Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add R API container to dev dependencies script, and consume R API from app #10

Merged
merged 51 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
1587ce4
WIP - front end can talk to R API
david-mears-2 Jul 29, 2024
704d4b7
Add .nvmrc
david-mears-2 Jul 29, 2024
985a75f
Update scripts
david-mears-2 Jul 29, 2024
3c98f7c
Add db:dev:migrate npm command
david-mears-2 Jul 29, 2024
ee643a0
Use utility instead of proxy api for R API; display version numbers
david-mears-2 Jul 30, 2024
18cb9e4
Streamline setup instructions
david-mears-2 Jul 30, 2024
d4c216e
Use composable instead of 'util' for R API consumption
david-mears-2 Jul 30, 2024
e8567db
Change linting rule
david-mears-2 Jul 30, 2024
e8ffe19
wip, I was mid-way through adapting the code to allow mocking for tests
david-mears-2 Jul 31, 2024
7ee6a28
Revert "wip, I was mid-way through adapting the code to allow mocking…
david-mears-2 Jul 31, 2024
d14a0eb
Return to using web app server API to talk to R API
david-mears-2 Jul 31, 2024
7669aa7
Try running Playwright with dependent services
david-mears-2 Aug 1, 2024
f187eeb
Revert "Try running Playwright with dependent services"
david-mears-2 Aug 1, 2024
1897a64
Deduplicate README
david-mears-2 Aug 1, 2024
77e31a4
Log more, for debugging
david-mears-2 Aug 2, 2024
5aa5870
Add ssh
david-mears-2 Aug 2, 2024
234bac2
increase retries and timeout
david-mears-2 Aug 2, 2024
418189f
Copy env vars
david-mears-2 Aug 2, 2024
94612e8
Try using the repo env
david-mears-2 Aug 2, 2024
a978237
Next iter of env var setup
david-mears-2 Aug 2, 2024
b2ce44c
ssh
david-mears-2 Aug 2, 2024
2bff65b
ssh iter
david-mears-2 Aug 2, 2024
fb2bf48
env var iter
david-mears-2 Aug 2, 2024
7f44bc5
Use script for env vars for github actions
david-mears-2 Aug 2, 2024
776d3a8
Run service dependencies
david-mears-2 Aug 2, 2024
1eabcfc
Ensure dependency services can find git branch name on CI environment
david-mears-2 Aug 2, 2024
1c9e574
remove debug
david-mears-2 Aug 2, 2024
276e91e
update tests - add r one
david-mears-2 Aug 2, 2024
76bef3b
Remove flaky low-value e2e test of sidebar behaviour
david-mears-2 Aug 2, 2024
0357db4
Use production mode for playwright web server
david-mears-2 Aug 2, 2024
68a2047
Add testing of Nuxt API endpoints, using external mocking service
david-mears-2 Aug 2, 2024
e1921c8
Test Mockoon is running on CI
david-mears-2 Aug 2, 2024
2c94900
Add timeout to long-running test
david-mears-2 Aug 2, 2024
1a4715e
Increase timeout on long-running test
david-mears-2 Aug 2, 2024
ee66623
Add env var for unit-tests.yml
david-mears-2 Aug 6, 2024
18d6ea1
Remove test timeout
david-mears-2 Aug 6, 2024
11801e2
Remove commented-out earlier test writing attempts
david-mears-2 Aug 6, 2024
871bb3d
Add documentation for Mockoon and how to unit test our API endpoints
david-mears-2 Aug 6, 2024
43d47b0
Fix unit test github action
david-mears-2 Aug 6, 2024
d45d437
Tweaks to unit test
david-mears-2 Aug 6, 2024
c70b3aa
Merge pull request #16 from jameel-institute/friday-pm-playwright
david-mears-2 Aug 6, 2024
4d8f286
Use non-public (server-side only) nuxt config variable
david-mears-2 Aug 6, 2024
54aa577
Update README
david-mears-2 Aug 6, 2024
6e0ed26
Simplify api dir structure
david-mears-2 Aug 6, 2024
1a82a3b
Refactor API to use types, pass on R API status codes/texts
david-mears-2 Aug 7, 2024
a2bfe29
Add more detail to integration test
david-mears-2 Aug 7, 2024
c862e0f
Update .github/workflows/integration-tests.yml
david-mears-2 Aug 8, 2024
5d875f7
Update README.md
david-mears-2 Aug 8, 2024
5de1f22
Update README.md
david-mears-2 Aug 8, 2024
6cfc4bd
Add wrapper for defineEventHandler to handle errors
david-mears-2 Aug 9, 2024
0701f44
Merge branch 'jidea-38-containerisation-r-api' of github.com:jameel-i…
david-mears-2 Aug 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env.example
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
CI=0
DATABASE_URL="postgresql://daedalus-web-app-user:changeme@localhost:5432/daedalus-web-app"
NUXT_R_API_BASE=http://localhost:8001/
36 changes: 36 additions & 0 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: Integration tests
on:
push:
branches:
- main
pull_request:
branches:
- '*'
jobs:
test:
timeout-minutes: 60
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set up Node
uses: actions/setup-node@v4
with:
node-version: 20
cache: npm
- name: Install dependencies
run: npm ci
- name: Start API server with Mockoon to mock R API
uses: mockoon/cli-action@v2
with:
data-file: ./mocks/mockoon.json
port: 8001
- name: Check mocked R API server is running
run: curl -s http://localhost:8001/mock-smoke
- name: Set environment variables for app
run: scripts/ci/copy-env-vars-to-dot-env-file
env:
NUXT_R_API_BASE: ${{ vars.NUXT_R_API_BASE }}
# Note - non-secret variables are stored under the 'vars' context, while secrets will be stored under the 'secrets' context
Comment on lines +30 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is going to be used in multiple workflows, you might want to pull this out into a shared action, like these:
https://github.com/mrc-ide/packit/tree/05560383aef00035a62eae605094e016eb6bc03c/.github/actions/build-api

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll bear it in mind for if it gets used in more than two workflows!

- name: Test
run: npm run test:integration
11 changes: 9 additions & 2 deletions .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Playwright Tests
name: Playwright tests
on:
push:
branches:
Expand All @@ -13,12 +13,19 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set environment variables for app
run: scripts/ci/copy-env-vars-to-dot-env-file
env:
NUXT_R_API_BASE: ${{ vars.NUXT_R_API_BASE }}
# Note - non-secret variables are stored under the 'vars' context, while secrets will be stored under the 'secrets' context
- name: Run service dependencies
run: scripts/run-dependencies
- name: Set up Node
uses: actions/setup-node@v4
with:
node-version: 20
cache: npm
- name: Install dependencies
- name: Install npm dependencies
run: npm ci
- name: Install Playwright Browsers
run: npx playwright install --with-deps && npx playwright install msedge
Expand Down
7 changes: 6 additions & 1 deletion .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Test
name: Unit tests
on:
push:
branches:
Expand All @@ -20,6 +20,11 @@ jobs:
cache: npm
- name: Install dependencies
run: npm ci
- name: Set environment variables for app
run: scripts/ci/copy-env-vars-to-dot-env-file
env:
NUXT_R_API_BASE: ${{ vars.NUXT_R_API_BASE }}
# Note - non-secret variables are stored under the 'vars' context, while secrets will be stored under the 'secrets' context
- name: Test
run: npm run test:unit:coverage
- name: Upload coverage to codecov
Expand Down
1 change: 1 addition & 0 deletions .nvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
v20
75 changes: 46 additions & 29 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,79 +2,96 @@

npx nuxi analyze

# Env vars

CI=0 or CI=1

# Tests

Run unit tests and component tests:
To run unit tests:
```bash
npm run test:unit
# Or with coverage:
npm run test:unit:coverage
```

To run integration tests, first start the Mockoon server:
```bash
npx mockoon-cli start --data ./mocks/mockoon.json
```
Then run:
```bash
npm run test:integration
```

Run server-side rendering tests:
```bash
npm run test:ssr
```

Run full-stack tests:

1. Ensure Mockoon server is not running
1. Run:
```bash
npm run test:e2e
```

The tests under e2e, which are run by playwright, are for testing the full-stack (client app and server app) in a browser environment. Since the server-rendered page may be different from the client-rendered page, for example, when some elements are configured to only render on the client side, relevant tests should wait for the elements to be present.

## Local development
# Local development

### Setup
## <a id="first-time"></a> Your first time setting up

Use Node 20.
Have Docker installed.
Copy `.env.example` to `.env`.
Build and run the database container:

Make sure Docker is installed.

Build and run the database container and R API container using this script:

```bash
./db/scripts/build
./db/scripts/run
scripts/run-dev-dependencies
```

Install the JS dependencies:
If you have trouble with the R API image, there might be helpful information in [its own README](https://github.com/jameel-institute/daedalus.api).

Run the below to copy `.env.example` to `.env` and then run the `dev:init` command, which installs the JS dependencies, runs any pending database migrations, and starts up the server in development mode on `http://localhost:3000`:

```bash
npm install
cp .env.example .env
npm run dev:init
```

Prisma ORM can only query the database once you 'generate' the Prisma Client, which generates into `node_modules/.prisma/client`. This should happen when you install the JS dependencies and whenever you run a migration, but if the Prisma client gets out of sync or doesn't generate, you can manually generate it:
## Everyday development workflow

If you've already done ['Your first time setting up'](#first-time), you can quickly get back to work by:

```bash
npx prisma generate
# In one terminal window
scripts/run-dev-dependencies --db-build-skip # Skips the build step for the db container, and tries to run an existing image
```

Start the development server on `http://localhost:3000`:

```bash
# In another terminal window
npm run dev
```

You can also expose it to your local network, so that you can try it out on a mobile device, using:
## Other ways of serving the app and dependencies

You can also expose the app to your local network, so that you can try it out on a mobile device, using:

```bash
npm run dev -- --host
```

The QR code shown will allow you to quickly access the app.

### DB
See the 'production' section of this README for how to run the app in production mode.

# DB

Our ORM is [Prisma](https://www.prisma.io/).

To create migrations to the database, first update the Prisma schema at ./prisma/schema.prisma as required, then run the below command to generate the corresponding SQL migration and to apply it to the database. [You should commit both](https://www.prisma.io/docs/orm/prisma-migrate/workflows/team-development#source-control) the Prisma schema and the migration file to Git.

```bash
npx prisma migrate dev
npm run db:dev:migrate
```

The same command is also used to apply migrations that already exist in ./prisma/migrations but which have not been applied to the database.
Expand All @@ -87,15 +104,15 @@ npx prisma generate

More helpful information about Prisma [development workflows](https://www.prisma.io/docs/orm/prisma-migrate/workflows/development-and-production#customizing-migrations) and resolving issues in [production environments](https://www.prisma.io/docs/orm/prisma-migrate/workflows/patching-and-hotfixing#fixing-failed-migrations-with-migrate-diff-and-db-execute).

#### For your IDE
## For your IDE

In VSCode, you can use the extension with ID 'Prisma.prisma' to get syntax highlighting etc.

## Linting and formatting
# Linting and formatting

Linting and formatting are handled jointly by [@nuxt/eslint](https://eslint.nuxt.com/packages/module) (an "all-in-one" ESLint "integration" for Nuxt) and by the more frequently-updated, conventionally- and widely-used [@antfu/eslint-config](https://github.com/antfu/eslint-config) (antfu works at NuxtLabs, and the package is given as an example in the `@nuxt/eslint` docs). The former handles linting only, while the latter also handles formatting, based on ESLint Stylistic.

### Commit hook
## Commit hook

You should install the lint-fixing commit hook (which will apply to this repo only) using:

Expand All @@ -105,24 +122,24 @@ npx simple-git-hooks

This will help us keep git history tidy, avoiding clogging up `git blame`s with linting-only commits.

### Inspecting the linting rules
## Inspecting the linting rules

This is a helpful tool for inspecting your config setup, so you can check which rules are applied, in which order:
```bash
npx @eslint/config-inspector
```

### For your IDE
## For your IDE

In VSCode, [make sure](https://eslint.nuxt.com/packages/module#vs-code) your ESlint VS Code extension (vscode-eslint) is at least v3.0.10 (released June 2024). Turn on the 'Format on Save' setting.

## CI
# CI

Playwright tests produce HTML reports when they run, whether on CI or not, showing visual snapshots at each timestep in each test. If you need to open these, follow the instructions [here](https://playwright.dev/docs/ci-intro#html-report), particularly '[Viewing the HTML report](https://playwright.dev/docs/ci-intro#viewing-the-html-report)'.

## Production
# Production

Build the application for production:
Build the Nuxt application for production:

```bash
npm run build
Expand Down
51 changes: 33 additions & 18 deletions components/SideBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,13 @@
<!-- Use CoreUI Sidebar Header component instead of footer so that stylings for CoreUI Sidebar Brand component work -->
<CSidebarBrand>
<div class="sidebar-brand-full">
<img class="img-fluid mb-1" src="~/assets/img/IMPERIAL_JAMEEL_INSTITUTE_LOCKUP-p-500.png" alt="Imperial College and Community Jameel logo">
<img
data-testid="logo"
class="img-fluid mb-1"
:title="versionTooltipContent"
src="~/assets/img/IMPERIAL_JAMEEL_INSTITUTE_LOCKUP-p-500.png"
alt="Imperial College and Community Jameel logo"
>
</div>
</CSidebarBrand>
</CSidebarHeader>
Expand All @@ -51,38 +57,47 @@

<script lang="ts" setup>
import { CIcon } from "@coreui/icons-vue";
import type { VersionData } from "@/types/daedalusApiResponseTypes";

const { data: versionData } = useFetch("/api/versions") as { data: Ref<VersionData> };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems likely that this pattern will be repeated a lot, so we'll probably eventually want a generic typs something like

export interface APIResponse<T> {
  data: Ref<T>
}

so then this line would become
const { data: versionData } = useFetch("/api/versions") as APIResponse<VersionData>;

..but then I suspect maybe we'll also eventually have some sort of useAPI which wraps the raw useFetch and just gives back the data you're interested in, and helps with error handling. So probably not one for this PR.

UPDATE: I see below that you've got ServerApiResponse, but I think that refers to responses from the R API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ServerApiResponse was named to try and distinguish it from the R API, so that'll have to be re-named... if we simply call it ApiResponse might that actually be clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though ServerApiResponse's purpose differs from what you're talking about - ServerApiResponse (well, types extended from it) are to be used inside the API routes to dictate the format of the response's data, rather than to be used wrapping a useFetch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, exactly. I think probably the response types from the R API (used in the web app backend) and the types from the web app backend (used in the front end) should probably live in different modules, and maybe a "shared" module for ones that happen to be the same, with whatever generic classes are useful in the two contexts. Maybe ServerApiResponse and RApiResponse (I think DaedalusApiResponse would be more correct, but very verbose, and you already have rApi module...)


const versionTooltipContent = computed(() => {
if (versionData.value) {
return `Model version: ${versionData.value.daedalusModel} \nR API version: ${versionData.value.daedalusApi} \nWeb app version: ${versionData.value.daedalusWebApp}`;
} else {
return undefined;
}
});

const visible = defineModel("visible", { type: Boolean, required: true });
const largeScreen = ref(true);

const breakpoint = 992; // CoreUI's "lg" breakpoint
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From here on in this file the changes are only formatting.

const resetSidebarPerScreenSize = () => {
// Set the default values for the sidebar based on the screen size.
if (window.innerWidth < breakpoint) {
visible.value = false;
largeScreen.value = false;
} else {
visible.value = true;
largeScreen.value = true;
}
};

const hideHasNeverBeenEmitted = ref(true);
function handleHide() {
const handleHide = () => {
if (hideHasNeverBeenEmitted.value) {
// If this is the first 'hide', which is emitted on page load, we un-do it.
// This is because the CoreUI Sidebar component emits a 'hide' event on page load, which we
// don't want to obey for larger screen sizes.
resetSidebarPerScreenSize();
hideHasNeverBeenEmitted.value = false;
}
else {
} else {
// If this is not the first 'hide', emitted on page load, we obey it and sync
// the parent component's value.
visible.value = false;
}
}

const breakpoint = 992; // CoreUI's "lg" breakpoint
function resetSidebarPerScreenSize() {
// Set the default values for the sidebar based on the screen size.
if (window.innerWidth < breakpoint) {
visible.value = false;
largeScreen.value = false;
}
else {
visible.value = true;
largeScreen.value = true;
}
}
};

onMounted(() => {
window.addEventListener("resize", resetSidebarPerScreenSize);
Expand Down
3 changes: 1 addition & 2 deletions components/WebsocketConnection.client.vue
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
function checkConnectionStatus() {
if (socket.connected) {
onConnect();
}
else {
} else {

Check warning on line 26 in components/WebsocketConnection.client.vue

View check run for this annotation

Codecov / codecov/patch

components/WebsocketConnection.client.vue#L26

Added line #L26 was not covered by tests
onDisconnect();
}
}
Expand Down
3 changes: 0 additions & 3 deletions db/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,5 @@ RUN chown -R postgres:postgres /etc/daedalus-web-app
ENV PATH="/usr/local/bin:$PATH"
RUN docker-entrypoint.sh --version

# Ensure the start script is executable
RUN chmod +x /daedalus-web-app-bin/start-with-config.sh

ENTRYPOINT ["/daedalus-web-app-bin/start-with-config.sh"]
CMD ["/etc/daedalus-web-app/postgresql.conf"]
Empty file modified db/bin/start-with-config.sh
100644 → 100755
Empty file.
37 changes: 37 additions & 0 deletions db/bin/wait-for-db
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#!/usr/bin/env bash

# Copied from https://github.com/mrc-ide/packit/blob/main/db/bin/wait-for-db

wait_for()
{
echo "waiting up to $TIMEOUT seconds for postgres"
start_ts=$(date +%s)
for i in $(seq $TIMEOUT); do
# Using pg_ready as:
#
# pg_isready -U $POSTGRES_USER -d $POSTGRES_DB
#
# seems heaps nicer but does not actually work properly
# because it pulls us up to soon.
psql -U $POSTGRES_USER -d $POSTGRES_DB -c "select 1;" > /dev/null 2>&1
result=$?
if [[ $result -eq 0 ]]; then
end_ts=$(date +%s)
echo "postgres is available after $((end_ts - start_ts)) seconds"
break
fi
sleep 1
echo "...still waiting"
done
return $result
}

# The variable expansion below is 100s by default, or the argument provided
# to this script
TIMEOUT="${1:-100}"
wait_for
RESULT=$?
if [[ $RESULT -ne 0 ]]; then
echo "postgres did not become available in time"
fi
exit $RESULT
2 changes: 1 addition & 1 deletion db/conf/postgresql.test.conf.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copied from db/conf/postgresql.test.conf.in
# Copied from https://github.com/mrc-ide/packit/blob/main/db/conf/postgresql.test.conf.in
# ----------------------------------------------------------------------------
# Test database config begins here.
# Values here override those earlier in the file
Expand Down
Loading