Upgrade Apollo server to v4 #8036

rikukissa · 2024-11-21T09:22:11Z

Our previous version had lost support. I also want to upgrade so that we can use API federation for new events APIs :)

github-actions · 2024-11-21T09:22:27Z

Oops! Looks like you forgot to update the changelog. When updating CHANGELOG.md, please consider the following:

Changelog is read by country implementors who might not always be familiar with all technical details of OpenCRVS. Keep language high-level, user friendly and avoid technical references to internals.
Answer "What's new?", "Why was the change made?" and "Why should I care?" for each change.
If it's a breaking change, include a migration guide answering "What do I need to do to upgrade?".

packages/gateway/src/features/fhir/FHIRAPI.ts

Zangetsu101 · 2024-11-22T07:02:28Z

packages/gateway/src/features/fhir/patientAPI.ts

  }

  getPatient(id: string) {
-    const inBundle = getResourceFromBundleById(this.context.record, id)
+    const inBundle = getResourceFromBundleById(this.context.record!, id)


if we are trying to get the resource, then shouldn't it be findResource...?

I don't think this is even used anywhere 😄 This would throw an error if there isn't a patient which to me seems logical given the method is named getPatient. So will give you a patient with you know their id or else throw an error

Zangetsu101 · 2024-11-22T07:03:44Z

packages/gateway/src/features/search/root-resolvers.ts

+class RateLimitError extends GraphQLError {
+  constructor(message = 'You are being rate limited') {
+    super(message, {
+      extensions: {
+        code: 'DAILY_QUOTA_EXCEEDED',
+        http: {
+          status: 429
+        }
+      }
+    })


nice! This should make the graphql requests actually fail by responding with something other than 200 right?

I think there is potential for that but I need to test how it works 👍

Zangetsu101 · 2024-11-22T07:04:38Z

packages/gateway/src/features/user/root-resolvers.ts

+    getUser: async (_, { userId }, { dataSources }) => {
+      const user = await dataSources.usersAPI.getUserById(userId!)
+      return user
+    },


Are we intentionally removing the rate limit from this endpoint?

No, good catch! I removed it while debugging something

Zangetsu101 · 2024-11-22T07:06:32Z

packages/gateway/src/features/user/usersAPI.ts

@@ -39,11 +34,11 @@ export class UsersAPI extends RESTDataSource {
    }

    try {
-      const response = this.post('getUser', { email })
+      const response = await this.post('getUser', { body: { email } })


You probably don't want to await here as that would just break the caching functionality. We want to cache the Promise in this case and not the Resolved value so that when multiple requests are made, we can just return the cached Promise that we created on the first request

Same goes for the other requests in this file

Cool yea, I was wondering about that.. Makes sense!

makelicious

Looks good! Comments are mostly stylistic, check the code suggestion whether it is needed at least.

makelicious · 2024-11-22T07:13:34Z

packages/gateway/src/features/fhir/documentsAPI.ts

-    this.baseURL = `${FHIR_URL}/DocumentReference`
-  }
+const fhirPath = parse(FHIR_URL).path || ''
+export default class DocumentsAPI extends OpenCRVSRestDataSource {


Since we capitalise CRVS, API and others, maybe rest could be also capitalised.

makelicious · 2024-11-22T07:15:04Z

packages/gateway/src/features/fhir/patientAPI.ts

  }

  getPatient(id: string) {
-    const inBundle = getResourceFromBundleById(this.context.record, id)
+    const inBundle = getResourceFromBundleById(this.context.record!, id)


could assertion be managed by types?

packages/gateway/src/features/fhir/paymentsAPI.ts

packages/gateway/src/features/user/root-resolvers.ts

packages/gateway/src/features/user/usersAPI.ts

packages/gateway/src/features/search/root-resolvers.ts

packages/gateway/src/features/fhir/documentsAPI.ts

packages/gateway/src/features/fhir/FHIRAPI.ts

ocrvs-bot · 2024-11-25T09:06:37Z

Your environment is deployed to https://upgrade-apollo-to-4.opencrvs.dev

…ade-apollo-to-4

…re into upgrade-apollo-to-4

…ade-apollo-to-4

…re into upgrade-apollo-to-4

rikukissa added the 🚀 Ready to deploy Deployment automation should pick this PR up and start auto-deploying it label Nov 21, 2024

rikukissa temporarily deployed to upgrade-apollo-to-4 November 21, 2024 09:22 — with GitHub Actions Inactive

github-actions bot temporarily deployed to upgrade-apollo-to-4 November 21, 2024 09:22 Inactive

rikukissa temporarily deployed to upgrade-apollo-to-4 November 21, 2024 13:32 — with GitHub Actions Inactive

rikukissa temporarily deployed to upgrade-apollo-to-4 November 21, 2024 13:40 — with GitHub Actions Inactive

rikukissa marked this pull request as ready for review November 21, 2024 13:41

rikukissa temporarily deployed to upgrade-apollo-to-4 November 21, 2024 13:41 — with GitHub Actions Inactive

rikukissa changed the title ~~Upgrade Apollo~~ Upgrade Apollo server to v4 Nov 21, 2024

rikukissa requested review from makelicious and Zangetsu101 November 21, 2024 13:42

upgrade apollo

1ce14c8

rikukissa force-pushed the upgrade-apollo-to-4 branch from d193747 to 1ce14c8 Compare November 22, 2024 06:45

rikukissa temporarily deployed to upgrade-apollo-to-4 November 22, 2024 06:45 — with GitHub Actions Inactive

add missing license header

ad70247

rikukissa temporarily deployed to upgrade-apollo-to-4 November 22, 2024 06:47 — with GitHub Actions Inactive

Zangetsu101 requested changes Nov 22, 2024

View reviewed changes

makelicious approved these changes Nov 22, 2024

View reviewed changes

fix gateway tests

3af330d

rikukissa temporarily deployed to upgrade-apollo-to-4 November 22, 2024 07:27 — with GitHub Actions Inactive

cleanup

1c11fb0

rikukissa temporarily deployed to upgrade-apollo-to-4 November 22, 2024 08:12 — with GitHub Actions Inactive

Merge branch 'develop' into upgrade-apollo-to-4

ad2662a

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 08:40 — with GitHub Actions Inactive

remove unused module

5e98bb9

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 08:54 — with GitHub Actions Inactive

update types

c6bf9d8

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 08:58 — with GitHub Actions Inactive

Merge branch 'develop' of github.com:opencrvs/opencrvs-core into upgr…

0b9ef5d

…ade-apollo-to-4

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 09:26 — with GitHub Actions Inactive

fix all references to Event to be EventType

a46e289

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 12:43 — with GitHub Actions Inactive

cache promises and not promise return values

93d58ab

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 12:47 — with GitHub Actions Inactive

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 13:14 — with GitHub Actions Inactive

Zangetsu101 approved these changes Nov 25, 2024

View reviewed changes

rikukissa force-pushed the upgrade-apollo-to-4 branch from 0ddb03a to 70e82f2 Compare November 25, 2024 13:54

rikukissa had a problem deploying to upgrade-apollo-to-4 November 25, 2024 13:55 — with GitHub Actions Error

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 13:55 — with GitHub Actions Inactive

add knip.json

9c8b4b3

rikukissa force-pushed the upgrade-apollo-to-4 branch from 5b4417a to 9c8b4b3 Compare November 25, 2024 14:05

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 14:06 — with GitHub Actions Inactive

fix final EventType reference

8883809

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 14:40 — with GitHub Actions Inactive

Merge branch 'develop' into upgrade-apollo-to-4

e373060

rikukissa temporarily deployed to upgrade-apollo-to-4 November 25, 2024 14:41 — with GitHub Actions Inactive

rikukissa added 3 commits November 26, 2024 10:57

move FHIR bundle record caching to be inside a data source

bca00fc

Merge branch 'upgrade-apollo-to-4' of github.com:opencrvs/opencrvs-co…

8270541

…re into upgrade-apollo-to-4

Merge branch 'develop' of github.com:opencrvs/opencrvs-core into upgr…

8eef240

…ade-apollo-to-4

rikukissa temporarily deployed to upgrade-apollo-to-4 November 26, 2024 09:00 — with GitHub Actions Inactive

add records api to test mock

07d9344

rikukissa temporarily deployed to upgrade-apollo-to-4 November 26, 2024 09:23 — with GitHub Actions Inactive

rikukissa added 2 commits November 26, 2024 11:26

create separate methods for fetching and getting a record from the state

98f31ae

Merge branch 'develop' into upgrade-apollo-to-4

0a62b4a

rikukissa temporarily deployed to upgrade-apollo-to-4 November 26, 2024 09:27 — with GitHub Actions Inactive

rikukissa added 2 commits November 26, 2024 11:30

cleanup

b3bdc6b

Merge branch 'upgrade-apollo-to-4' of github.com:opencrvs/opencrvs-co…

a02b744

…re into upgrade-apollo-to-4

rikukissa deployed to upgrade-apollo-to-4 November 26, 2024 09:32 — with GitHub Actions Active

rikukissa merged commit cb97021 into develop Nov 26, 2024
67 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Apollo server to v4 #8036

Upgrade Apollo server to v4 #8036

rikukissa commented Nov 21, 2024 •

edited

Loading

github-actions bot commented Nov 21, 2024

Zangetsu101 Nov 22, 2024

rikukissa Nov 22, 2024

Zangetsu101 Nov 22, 2024

rikukissa Nov 22, 2024

Zangetsu101 Nov 22, 2024

rikukissa Nov 22, 2024

Zangetsu101 Nov 22, 2024

Zangetsu101 Nov 22, 2024

rikukissa Nov 22, 2024

makelicious left a comment

makelicious Nov 22, 2024

makelicious Nov 22, 2024

ocrvs-bot commented Nov 25, 2024

Upgrade Apollo server to v4 #8036

Upgrade Apollo server to v4 #8036

Conversation

rikukissa commented Nov 21, 2024 • edited Loading

github-actions bot commented Nov 21, 2024

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

makelicious left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocrvs-bot commented Nov 25, 2024

rikukissa commented Nov 21, 2024 •

edited

Loading