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

david-mears-2 · 2024-07-30T08:58:18Z

This PR sets up the basic infrastructure required for the web app to consume data from the R API:

Running the R API image from a dev dependencies script.
Displaying the only information that the R API currently exposes, the version number, in a title / tooltip. Hover over the logo in the sidebar to see if it's working.

codecov · 2024-07-30T17:12:09Z

Codecov Report

Attention: Patch coverage is 64.66165% with 47 lines in your changes missing coverage. Please review.

Project coverage is 53.50%. Comparing base (ec4dfd5) to head (0701f44).
Report is 1 commits behind head on main.

Files	Patch %	Lines
server/utils/defineEventHandlerWithErrors.ts	0.00%	15 Missing and 1 partial ⚠️
server/utils/rApi.ts	72.34%	13 Missing ⚠️
server/api/versions.get.ts	0.00%	7 Missing and 1 partial ⚠️
server/utils/helpers.ts	16.66%	5 Missing ⚠️
lib/prisma.ts	0.00%	4 Missing ⚠️
components/WebsocketConnection.client.vue	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
+ Coverage   50.62%   53.50%   +2.87%     
==========================================
  Files          16       22       +6     
  Lines         401      514     +113     
  Branches       25       39      +14     
==========================================
+ Hits          203      275      +72     
- Misses        192      231      +39     
- Partials        6        8       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

david-mears-2 · 2024-07-31T21:56:09Z

.env.example

@@ -1,2 +1,3 @@
 CI=0
 DATABASE_URL="postgresql://daedalus-web-app-user:changeme@localhost:5432/daedalus-web-app"
+NUXT_PUBLIC_R_API_BASE=http://localhost:8001/


This doesn't mean the API is public, it's just a way of automatically setting a config variable in nuxt.

So it needs to have the NUXT_PUBLIC_ prefix to become available in Nuxt?

It ought to work without 'PUBLIC' but I couldn't immediately make it do so. Documentation: https://nuxt.com/docs/guide/going-further/runtime-config#environment-variables

OK I managed to remove the 'PUBLIC' part :)

david-mears-2 · 2024-07-31T22:00:09Z

tests/unit/components/AppHeader.spec.ts

-// @vitest-environment nuxt
-


These comments used to tell vitest to use a nuxt environment for these tests, but after I got stuck for a while trying to debug a test and finally worked out that it wasn't using a nuxt environment, I decided to set that as the default in vitest.config.ts.

… for tests" This reverts commit 9f0e716.

This reverts commit d1c4f09.

EmmaLRussell

Works for me!

There seem to be a lot of DB script changes showing in the diff? Do these belong in the parent branch? Do you need to push the latest of that?

Missing unit tests for the server endpoints? Codecov is grumbling as well as the failing Playwright workflow.

Feels to me like the last missing piece is how a store is going to fit into all this - one for the next sprint perhaps.

EmmaLRussell · 2024-08-01T13:36:57Z

.env.example

@@ -1,2 +1,3 @@
 CI=0
 DATABASE_URL="postgresql://daedalus-web-app-user:changeme@localhost:5432/daedalus-web-app"
+NUXT_PUBLIC_R_API_BASE=http://localhost:8001/


So it needs to have the NUXT_PUBLIC_ prefix to become available in Nuxt?

EmmaLRussell · 2024-08-01T13:37:53Z

README.md

@@ -29,31 +29,36 @@ The tests under e2e, which are run by playwright, are for testing the full-stack

 ## Local development

-### Setup
+### If it's the first time setting up


Could this have a corresponding heading further down at the start of the bits that you need to do every time?

That's what I intended with 'Other ways of serving the app and dependencies' heading, I can rename that to be more clearly a contrasting heading

Ah, right I thought that was 'alternatives' - I really meant running dependencies and running the app, which you need to do every time you, say, restart your machine, as opposed to installing Docker which you only have to do once.

Updated README

EmmaLRussell · 2024-08-01T13:55:09Z

lib/prisma.ts

@@ -18,5 +18,6 @@ const prisma = globalThis.prismaGlobal ?? prismaClientSingleton();

 export default prisma;

-if (process.env.NODE_ENV !== "production")
+if (process.env.NODE_ENV !== "production") {


I think you've switched off linting errors for one line ifs with braces - which is good - I think elsewhere we actually enforce these using somewhat different eslint config. Would be good to be as consistent as possible there.

This is now enforced, because lessOpinionated reverts us to the eslint 'curly' rule being set to 'error' in 'all' cases: https://eslint.org/docs/latest/rules/curly

EmmaLRussell · 2024-08-01T14:08:07Z

server/api/versions/index.get.ts

@@ -0,0 +1,28 @@
+import { fetchRApi } from "@/server/utils/rApi";


Could you simplify this location of this file somewhat? I get the impression from the docs, that you could just put this code in server/api/versions.ts - is it because you will have some POSTs which will need to go into a .post.ts file? But even then, I don't think you need to make a separate folder for each path.

OK, let's simplify the dir structure for now

EmmaLRussell · 2024-08-01T14:09:27Z

server/api/versions/index.get.ts

+    daedalusApi: response!.data!["daedalus.api"],
+    daedalusWebApp: packageJson.version,
+  };
+}, { maxAge: 60 /* Cache response server-side for 1 minute */ }); // https://nitro.unjs.io/guide/cache


This feels like it ought to be defined as a global setting somewhere, for responses which are appropriate for caching.

60 in particular? I think the appropriate cache duration might vary by endpoint. It feels premature to make that a shared setting, until we have at least two cached endpoints.

I ended up getting rid of the cacheing because it was causing confusion while debugging.

EmmaLRussell · 2024-08-01T14:16:24Z

server/utils/rApi.ts

+    throw createError({
+      statusCode: 500,
+      message: "Internal Server Error",
+      data: {
+        rApiStatusCode: error.response?.status,
+        rApiResponseBody: error.data,
+        rApiResponseErrors: errorInformationString(error),
+      },


I think it would be better (and is generally our standard practice) for error responses from the web app api to mirror those from the R API, i.e. same format, include an errors array. I think we generally tend to just pass error responses through, preserving status code and errors, and including null data - unless there's some particular reason why we don't want to surface R API errors to the front end - we may choose not to display them to the user of course.

I gave this a good try. Below I'll give a comparison of the responses from the R API and the web app API, in the scenario when an error occurs:

Web app API (much of the format of this is determined by Nuxt):

> curl http://localhost:3000/api/versions {"url":"/api/versions","statusCode":404,"statusMessage":"Not Found","message":"NOT_FOUND: Resource not found","stack":"","data":[{"error":"NOT_FOUND","detail":"Resource not found"}]}

R API (I am presuming much of the format of this is determined by porcelain):

curl http://localhost:8001/ {"status":"failure","errors":[{"error":"NOT_FOUND","detail":"Resource not found"}],"data":null}

EmmaLRussell · 2024-08-01T14:20:06Z

components/SideBar.vue

@@ -52,37 +58,45 @@
 <script lang="ts" setup>
 import { CIcon } from "@coreui/icons-vue";

+const { data: versionData } = useFetch("/api/versions");


Can we cast this to the relevant response type?

EmmaLRussell · 2024-08-01T14:21:24Z

server/api/versions/index.get.ts

+interface SuccessResponse {
+  daedalusModel: string
+  daedalusApi: string
+  daedalusWebApp: string
+}


It feels like this sort of api model type is going to be useful to the client, so it might be better to export it with a less generic name, and to define it in a models or types folder perhaps.

Friday pm playwright

david-mears-2 · 2024-08-06T14:34:01Z

There seem to be a lot of DB script changes showing in the diff? Do these belong in the parent branch? Do you need to push the latest of that?

In fact this PR introduces a new DB script 'wait-for-db', which is required by the scripts 'run dependencies' and 'run dev dependencies' which are new features from this branch. They also imply a few other small changes to db scripts (like making sure they are able to return a branch name while on CI).

david-mears-2 · 2024-08-07T17:24:59Z

components/SideBar.vue


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

+const breakpoint = 992; // CoreUI's "lg" breakpoint


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

david-mears-2 · 2024-08-07T17:26:08Z

mocks/mockoon.json

@@ -0,0 +1,163 @@
+{


This file is automatically generated (but can also be manually edited). If a crumb of context is needed, see the new file tests/integration/INTEGRATION_TESTING.md

david-mears-2 · 2024-08-07T17:29:00Z

scripts/ci/copy-env-vars-to-dot-env-file

+
+set -eux
+
+echo "NUXT_R_API_BASE=$NUXT_R_API_BASE" > .env


This file is a bit of infrastructure to make it convenient to add lots more env vars at once to GitHub Actions much easily (assuming we'll get more of them in the future)

Also, use a mixture of integration (with Mockoon) testing and unit testing to test the web server API and its interaction with the front end and the R API.

EmmaLRussell

Various meandering remarks, some of which change course partway though!
Pretty much all suggestions could be ignored or done in another branch - up to you!

Web app API (much of the format of this is determined by Nuxt):

curl http://localhost:3000/api/versions
{"url":"/api/versions","statusCode":404,"statusMessage":"Not Found","message":"NOT_FOUND: Resource not found","stack":"","data":[{"error":"NOT_FOUND","detail":"Resource not found"}]}

Ah sorry, I didn't realise that Nuxt was so bossy about api response format. This seems like a good compromise, allowing us to separate out error code and detail, and with potential for multiple errors. Let's see how it works for us.

I wouldn't bother with Mockoon myself, it seems like a brittle overhead that's going to be a pain to maintain. I get the point about e2e vs integration, and the idea of not going outside the bounds of the repo for the former... but to be honest we've always found it most instructive to hit the the real R API in our web API integration tests as well as e2e, since that gives us early warning of any incompatibilities, and it's easy for us to address those since we own the R APIs too.
UPDATE: Hmm, ok I may have been persuaded out of that position by seeing that a lot of the integration tests you've done are around error handling, and errors are quite difficult to engineer in the real API, and the error handling does involve multiple modules, so it really does need integration testing. So Mockoon does have a use, but I'd suggest using it fairly sparingly, for special cases like error handling, and use the e2e tests for most use cases.

EmmaLRussell · 2024-08-07T09:03:53Z

tests/unit/server/api/README.md

+
+However, for testing our own endpoints, there is no way to directly invoke the functions exported from our API endpoints, other than by sending requests to the endpoints they define. As a result, we are required to run our server from the test, which we achieve by using the @nuxt/test-utils/e2e module (and we use the module's in-built [`fetch`](https://nuxt.com/docs/getting-started/testing#fetchurl-1) function to send the requests). The fact that Vitest is running a whole test server means these tests are not zoomed-in unit tests that are able to test a small piece of code, but are more like integration tests, since they run the server entirely, and they also (as we'll come onto later) use a mock server to mock responses from external APIs (namely, the R API).
+
+Although the set-up just described is not very unit-like and is more like an integration test, it is the closest to unit-testing that the Nuxt eco-system seems to currently offer if you need to test calling any API endpoints which belong to your server.


I don't really think we need to be this confessional about it. It's ok to just have integration tests for the edges of the system I think!

This is an outdated README that doesn't exist in the PR files any more (though it did yesterday morning when you wrote this!). The up-to-date documentation is in the file called INTEGRATION_TESTING.md which is significantly different and post-dates your discovery of a nice way to do unit testing: https://github.com/jameel-institute/daedalus-web-app/pull/10/files#diff-946b7aec3a9dfaeccc6649ec573a1b47c732e2f13fa236fec90af741e7cf4a90

Sorry, I think i made this remark before I realised you hadn't resubmitted yet, and then I forgot to delete it! 😆

EmmaLRussell · 2024-08-08T13:13:37Z

.github/workflows/integration-tests.yml

+        with:
+          data-file: ./mocks/mockoon.json
+          port: 8001
+      - name: Check API server is running


Suggested change

- name: Check API server is running

- name: Check mocked R API server is running

I think this is correct - it's not the nuxt API you're checking here?

EmmaLRussell · 2024-08-08T13:21:14Z

.github/workflows/integration-tests.yml

+      - 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


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

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

EmmaLRussell · 2024-08-08T13:22:47Z

README.md

+To run unit tests, component tests and integration tests, first start the Mockoon server:
+```bash
+npx mockoon-cli start --data ./mocks/mockoon.json
+```


I shouldn't need mockoon if I just want to run the unit/component tests though?

True, true, true-chez. Updated.

README.md

EmmaLRussell · 2024-08-08T13:39:33Z

components/SideBar.vue

@@ -51,38 +57,47 @@

 <script lang="ts" setup>
 import { CIcon } from "@coreui/icons-vue";
+import type { VersionData } from "@/types/apiResponseTypes";
+
+const { data: versionData } = useFetch("/api/versions") as { data: Ref<VersionData> };


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?

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?

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.

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...)

EmmaLRussell · 2024-08-08T13:42:33Z

playwright.config.ts

  // Multiple web servers (or background processes) can be launched: https://playwright.dev/docs/api/class-testconfig#test-config-web-server
  webServer: {
-    command: 'NUXT_HOST="127.0.0.1" NUXT_PORT="3000" npm run dev',
+    command: "npm run build && cd .output && node ./server/index.mjs",


Why this change?

This changes the e2e tests from running in dev mode to running in production mode. That's a more realistic test. It also, I think, lets us by-pass an intermittent problem I had when the dev server was responding to requests before the site was built (with that greenish Nuxt interstitial loading page), which tricked Playwright into thinking it was ready to start running the tests.

EmmaLRussell · 2024-08-08T13:46:46Z

server/api/versions.get.ts

+  if (versionDataResponse.errors || !versionDataResponse.data) {
+    throw createError({
+      statusCode: versionDataResponse.statusCode,
+      statusMessage: versionDataResponse.statusText,
+      message: errorMessage(versionDataResponse.errors),
+      data: versionDataResponse.errors,
+    });
+  } else {
+    return versionDataResponse.data;
+  }


This shared bit of logic feels like it ought to be middleware, or whatever the Nuxt equivalent is! Or just handled in rApi?

I've extracted it in this commit:

6cfc4bd

Nuxt documentation for server utils is limited but it's here: https://nuxt.com/docs/guide/directory-structure/server#server-utilities

EmmaLRussell · 2024-08-08T13:52:03Z

tests/e2e/rApi.spec.ts

@@ -0,0 +1,22 @@
+import { expect, request, test } from "@playwright/test";


I'm assuming this test module is temporary since basically all e2e tests will eventually be interacting with the R APi!

Co-authored-by: Emma Russell <[email protected]>

…nstitute/daedalus-web-app into jidea-38-containerisation-r-api

EmmaLRussell

One tiny comment, but it all seems to work great so I'm going to approve and merge this so I can get on with the docker stuff against the latest main with API support.

EmmaLRussell · 2024-08-12T09:48:20Z

server/api/versions.get.ts

-  const versionDataResponse = await getVersionData(event);
+export default defineEventHandlerWithErrors(
+  // TODO: Consider cacheing this server-side https://nitro.unjs.io/guide/cache
+  defineEventHandler(async (event): Promise<VersionDataResponse> => {


I'm a bit surprised that you're still invoking defineEventHandler here, since your new custom defineEventHandlerWitErrors already wraps defineEventHandler - do you have one more layer of defineEventHandler than you need?

I found a way around this, here is the commit: b029800

It's not yet a commit that lives in a PR.

david-mears-2 force-pushed the jidea-38-containerisation-r-api branch from a7e9485 to 8f1a0dd Compare July 30, 2024 17:09

david-mears-2 force-pushed the jidea-38-containerisation-r-api branch 2 times, most recently from 06e142e to 1be396f Compare July 31, 2024 21:55

david-mears-2 commented Jul 31, 2024

View reviewed changes

david-mears-2 force-pushed the jidea-38-containerisation-r-api branch 2 times, most recently from 7c5be26 to cd0b746 Compare July 31, 2024 22:02

david-mears-2 changed the title ~~Jidea 38 containerisation r api~~ Add R API container to dev dependencies script, and consume R API from app Jul 31, 2024

david-mears-2 force-pushed the jidea-38-containerisation-r-api branch from cd0b746 to dd6c7bf Compare July 31, 2024 22:06

david-mears-2 mentioned this pull request Jul 31, 2024

Wire up a database #8

Merged

1 task

david-mears-2 marked this pull request as ready for review August 1, 2024 08:53

david-mears-2 requested a review from EmmaLRussell August 1, 2024 09:20

david-mears-2 added 14 commits August 1, 2024 14:16

WIP - front end can talk to R API

1587ce4

Add .nvmrc

704d4b7

Update scripts

985a75f

Add db:dev:migrate npm command

3c98f7c

Use utility instead of proxy api for R API; display version numbers

ee643a0

Streamline setup instructions

18cb9e4

Use composable instead of 'util' for R API consumption

d4c216e

Change linting rule

e8567db

wip, I was mid-way through adapting the code to allow mocking for tests

e8ffe19

Revert "wip, I was mid-way through adapting the code to allow mocking…

7ee6a28

… for tests" This reverts commit 9f0e716.

Return to using web app server API to talk to R API

d14a0eb

Try running Playwright with dependent services

7669aa7

Revert "Try running Playwright with dependent services"

f187eeb

This reverts commit d1c4f09.

Deduplicate README

1897a64

EmmaLRussell reviewed Aug 1, 2024

View reviewed changes

david-mears-2 force-pushed the jidea-38-containerisation-r-api branch from 48de4d3 to 1897a64 Compare August 2, 2024 11:05

Log more, for debugging

77e31a4

david-mears-2 and others added 6 commits August 6, 2024 11:24

Remove commented-out earlier test writing attempts

11801e2

Add documentation for Mockoon and how to unit test our API endpoints

871bb3d

Fix unit test github action

43d47b0

Tweaks to unit test

d45d437

Merge pull request #16 from jameel-institute/friday-pm-playwright

c70b3aa

Friday pm playwright

Use non-public (server-side only) nuxt config variable

4d8f286

david-mears-2 added 2 commits August 6, 2024 15:43

Update README

54aa577

Simplify api dir structure

6e0ed26

Base automatically changed from jidea-38-containerisation to main August 7, 2024 09:36

david-mears-2 force-pushed the jidea-38-containerisation-r-api branch from 93e284b to 72869ff Compare August 7, 2024 17:22

david-mears-2 commented Aug 7, 2024

View reviewed changes

david-mears-2 force-pushed the jidea-38-containerisation-r-api branch from 72869ff to 9cdbff2 Compare August 7, 2024 17:34

Refactor API to use types, pass on R API status codes/texts

1a82a3b

Also, use a mixture of integration (with Mockoon) testing and unit testing to test the web server API and its interaction with the front end and the R API.

david-mears-2 force-pushed the jidea-38-containerisation-r-api branch from 9cdbff2 to 1a82a3b Compare August 7, 2024 17:42

Add more detail to integration test

a2bfe29

david-mears-2 requested a review from EmmaLRussell August 7, 2024 18:19

EmmaLRussell reviewed Aug 8, 2024

View reviewed changes

david-mears-2 and others added 5 commits August 8, 2024 16:53

Update .github/workflows/integration-tests.yml

c862e0f

Co-authored-by: Emma Russell <[email protected]>

Update README.md

5d875f7

Co-authored-by: Emma Russell <[email protected]>

Update README.md

5de1f22

Add wrapper for defineEventHandler to handle errors

6cfc4bd

Merge branch 'jidea-38-containerisation-r-api' of github.com:jameel-i…

0701f44

…nstitute/daedalus-web-app into jidea-38-containerisation-r-api

david-mears-2 requested a review from EmmaLRussell August 9, 2024 10:52

EmmaLRussell approved these changes Aug 12, 2024

View reviewed changes

EmmaLRussell merged commit 3d142fc into main Aug 12, 2024
6 checks passed

EmmaLRussell deleted the jidea-38-containerisation-r-api branch August 12, 2024 09:50

david-mears-2 mentioned this pull request Sep 3, 2024

Convert parameter metadata into a front-end form #23

Merged

		@@ -0,0 +1,28 @@
		import { fetchRApi } from "@/server/utils/rApi";


		However, for testing our own endpoints, there is no way to directly invoke the functions exported from our API endpoints, other than by sending requests to the endpoints they define. As a result, we are required to run our server from the test, which we achieve by using the @nuxt/test-utils/e2e module (and we use the module's in-built [`fetch`](https://nuxt.com/docs/getting-started/testing#fetchurl-1) function to send the requests). The fact that Vitest is running a whole test server means these tests are not zoomed-in unit tests that are able to test a small piece of code, but are more like integration tests, since they run the server entirely, and they also (as we'll come onto later) use a mock server to mock responses from external APIs (namely, the R API).

		Although the set-up just described is not very unit-like and is more like an integration test, it is the closest to unit-testing that the Nuxt eco-system seems to currently offer if you need to test calling any API endpoints which belong to your server.

	- name: Check API server is running
	- name: Check mocked R API server is running

		@@ -0,0 +1,22 @@
		import { expect, request, test } from "@playwright/test";

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

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

Conversation

david-mears-2 commented Jul 30, 2024 • edited Loading

codecov bot commented Jul 30, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmmaLRussell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-mears-2 commented Aug 6, 2024

Choose a reason for hiding this comment

david-mears-2 Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmmaLRussell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-mears-2 Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmmaLRussell left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-mears-2 commented Jul 30, 2024 •

edited

Loading

codecov bot commented Jul 30, 2024 •

edited

Loading

david-mears-2 Aug 7, 2024 •

edited

Loading

david-mears-2 Aug 8, 2024 •

edited

Loading

EmmaLRussell left a comment •

edited

Loading