Events v2: Skeleton GraphQL resolvers for basic operations #8033

rikukissa · 2024-11-20T15:02:41Z

Country config: opencrvs/opencrvs-countryconfig#328

…s-7824

…into ocrvs-7824-b

github-actions · 2024-11-20T15:02:56Z

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?".

rikukissa · 2024-11-20T15:04:54Z

packages/gateway/tsconfig.json

@@ -3,7 +3,8 @@
    "skipLibCheck": true,
    "baseUrl": "./src",
    "paths": {
-      "@gateway/*": ["./*"]
+      "@gateway/*": ["./*"],
+      "@events/*": ["../../events/src/*"]


This is a little nasty – it is because gateway imports the appRouter from events

import type { AppRouter } from '@opencrvs/events/src/router'

which again imports modules inside Events with the @events/... paths.

❯ yarn test:compilation yarn run v1.22.4 $ tsc --noEmit ../events/src/service/events.ts:12:27 - error TS2307: Cannot find module '@events/storage/mongodb' or its corresponding type declarations. 12 import { getClient } from '@events/storage/mongodb' ~~~~~~~~~~~~~~~~~~~~~~~~~

rikukissa · 2024-11-20T15:07:05Z

packages/commons/src/events/Event.ts

-      ])
-    })
-  )
+  type: z.string()


The only field the user can really send at the point of Event creation is type if we take on the approach of only submitting actions like registerEvent(eventId, registrationData) from the client

rikukissa · 2024-11-20T16:35:09Z

packages/gateway/src/v2-events/events/root-resolvers.ts

+      url: env.EVENTS_URL,
+      transformer: superjson,
+      headers({ opList }) {
+        const headers = opList[0].context?.headers


Not at all happy about this. opList is a non-empty list but still..

Could we parse it and throw to be sure, have a default or acknowledge the possibly undefined?

a bit wonky that the opList cannot be strictly typed :/ but yeah works like this or ! with a comment or ^

…into ocrvs-7824-b

…kage.json

…into ocrvs-7824-b

ocrvs-bot · 2024-11-22T06:41:40Z

Your environment is deployed to https://ocrvs-7824-b.opencrvs.dev

makelicious

Looks good! Non functional comments

makelicious · 2024-11-22T07:33:37Z

packages/gateway/src/v2-events/events/type-resolvers.ts

+      if (obj.type === ActionType.REGISTER) {
+        return 'RegisterAction'
+      }
+      return 'CreateAction'


We might want to be explicit here e.g. not defaulting to anything but exhausting the options

could we do type satisfies never to make sure all cases are handled at the end

makelicious · 2024-11-22T07:36:20Z

packages/gateway/src/v2-events/events/root-resolvers.ts

+      url: env.EVENTS_URL,
+      transformer: superjson,
+      headers({ opList }) {
+        const headers = opList[0].context?.headers


Could we parse it and throw to be sure, have a default or acknowledge the possibly undefined?

makelicious · 2024-11-22T07:39:33Z

packages/gateway/src/environment.ts

@@ -29,6 +29,7 @@ export const env = cleanEnv(process.env, {
  APPLICATION_CONFIG_URL: url({ devDefault: 'http://localhost:2021/' }),
  NOTIFICATION_URL: url({ devDefault: 'http://localhost:2020/' }),
  WORKFLOW_URL: url({ devDefault: 'http://localhost:5050/' }),
+  EVENTS_URL: url({ devDefault: 'http://localhost:5555/' }),


Opinion: Since we are using both conventions, with and without trailing slash, I would opt for without when introducing new ones. What do you think?

I agree; an extra trailing slash in our current setup might convey to a newcomer that it has a meaning that it doesn't

makelicious · 2024-11-22T07:41:33Z

packages/events/src/router.ts

+        .mutation(async (options) => {
+          return addAction(options.input.eventId, {
+            ...options.input.action,
+            type: 'NOTIFY',


Using these through the ActionType.NOTIFY will make changing them much easier in the future.

makelicious · 2024-11-22T07:45:07Z

packages/commons/src/events/Action.ts

-      registrationNumber: z.string()
-    })
+export const CreateActionInput = ActionInputBase.extend({})
+export const NotifyActionInput = ActionInputBase.extend({})


Could you add a comment for this pattern? Is it a matter of taking copy of the base an separating the type extension?

RegisterActionInput.extend({ type: z.enum([ActionType.REGISTER]) }) since one could argue that this is not RegisterActionInput until it has the type

makelicious · 2024-11-22T07:45:59Z

packages/commons/src/events/Action.ts

+    DeclareActionInput.extend({ type: z.enum([ActionType.DECLARE]) }),
+    RegisterActionInput.extend({ type: z.enum([ActionType.REGISTER]) })
+  ])
+  .and(


Is there semantic difference between extend and and?

.and apparently creates { name: string } & { age: number } while .extend creates { name: string; age: number }

https://zod.dev/?id=merge (merge is equivalent to extend but has better example :D)
vs
https://zod.dev/?id=and

makelicious · 2024-11-22T07:48:28Z

.github/workflows/lint-and-test.yml

@@ -85,17 +85,36 @@ jobs:
        with:
          node-version-file: .nvmrc

+      - name: Extract dependencies for ${{ matrix.package }}


We could benefit from adding description for this documenting the reason/aim

naftis · 2024-11-22T08:33:11Z

packages/client/package.json

@@ -108,6 +108,7 @@
    "xregexp": "^4.2.0"
  },
  "devDependencies": {
+    "@opencrvs/gateway": "^1.5.0",


does the client need this?

It does now as I added the logic to look at workflow dependencies to figure out which packages cannot be optimised away in build and test processes. Client did seem to have a dependency to gateway for whatever reason

I tried to find the dependency but couldn't :O

rikukissa and others added 8 commits November 18, 2024 15:32

add some basic operations and unit tests for interfacing with events

1b7d031

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

9f83721

…s-7824

feat(events): create first draft of types

f259bc3

feat(events): unify and clean up types

d2a03f8

add basic structure for graphql endpoints

5d1351f

add tennic club membership example data

a05906d

Merge branch 'feat/event-types' of github.com:opencrvs/opencrvs-core …

68f8adb

…into ocrvs-7824-b

implement initial graphql resolvers for basic event operations

a9f98a9

rikukissa commented Nov 20, 2024

View reviewed changes

implement authorisation and first GraphQL -> MongoDB request chain

7914c63

rikukissa commented Nov 20, 2024

View reviewed changes

rikukissa added 2 commits November 21, 2024 07:53

add license headers to all files

81f470f

Merge branch 'feat/event-types' of github.com:opencrvs/opencrvs-core …

d8addb9

…into ocrvs-7824-b

makelicious force-pushed the feat/event-types branch from 1412548 to 2a46d72 Compare November 21, 2024 12:44

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

Merge branch 'feat/event-types' of github.com:opencrvs/opencrvs-core …

55a2d63

…into ocrvs-7824-b

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 13:37 — with GitHub Actions Inactive

fix tests

1f90403

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 13:48 — with GitHub Actions Inactive

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 14:05 — with GitHub Actions Inactive

rikukissa force-pushed the ocrvs-7824-b branch from 8943b95 to 375acd1 Compare November 21, 2024 14:08

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 14:08 — with GitHub Actions Inactive

rikukissa force-pushed the ocrvs-7824-b branch from 375acd1 to d12e766 Compare November 21, 2024 14:12

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 14:14 — with GitHub Actions Inactive

add events package to gateway build

269970d

rikukissa force-pushed the ocrvs-7824-b branch from d12e766 to 269970d Compare November 21, 2024 14:28

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 14:28 — with GitHub Actions Inactive

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 14:46 — with GitHub Actions Inactive

rikukissa force-pushed the ocrvs-7824-b branch from 0d0a79f to 43b1146 Compare November 21, 2024 15:52

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 15:53 — with GitHub Actions Inactive

fix the test pipeline to not remove events package if its used in pac…

e8add12

…kage.json

rikukissa force-pushed the ocrvs-7824-b branch from 43b1146 to e8add12 Compare November 21, 2024 15:56

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 15:56 — with GitHub Actions Inactive

rikukissa added 2 commits November 21, 2024 18:11

cleanup

307a04f

Merge branch 'feat/event-types' of github.com:opencrvs/opencrvs-core …

d31792f

…into ocrvs-7824-b

rikukissa requested review from makelicious and naftis November 21, 2024 16:12

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 16:12 — with GitHub Actions Inactive

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 16:22 — with GitHub Actions Inactive

fix docker build dependencies

a5e2c8d

rikukissa force-pushed the ocrvs-7824-b branch from 8e12154 to a5e2c8d Compare November 21, 2024 16:25

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 16:25 — with GitHub Actions Inactive

trick knip not to complain about duplicate exports

e04fef8

rikukissa temporarily deployed to ocrvs-7824-b November 21, 2024 16:33 — with GitHub Actions Inactive

remove unused export

d031ed5

rikukissa temporarily deployed to ocrvs-7824-b November 22, 2024 06:04 — with GitHub Actions Inactive

update docker compose config

6d7b57b

rikukissa temporarily deployed to ocrvs-7824-b November 22, 2024 06:07 — with GitHub Actions Inactive

rikukissa temporarily deployed to ocrvs-7824-b November 22, 2024 06:34 — with GitHub Actions Inactive

makelicious approved these changes Nov 22, 2024

View reviewed changes

naftis reviewed Nov 22, 2024

View reviewed changes

Merge branch 'feat/event-types' into ocrvs-7824-b

c3c97ac

rikukissa temporarily deployed to ocrvs-7824-b November 25, 2024 07:45 — with GitHub Actions Inactive

rikukissa changed the base branch from feat/event-types to develop November 25, 2024 08:39

Merge branch 'develop' into ocrvs-7824-b

c604312

rikukissa deployed to ocrvs-7824-b November 25, 2024 08:40 — with GitHub Actions Active

rikukissa merged commit 89ce49a into develop Nov 25, 2024
67 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Events v2: Skeleton GraphQL resolvers for basic operations #8033

Events v2: Skeleton GraphQL resolvers for basic operations #8033

rikukissa commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024

rikukissa Nov 20, 2024

rikukissa Nov 20, 2024 •

edited

Loading

rikukissa Nov 20, 2024

makelicious Nov 22, 2024

naftis Nov 22, 2024

ocrvs-bot commented Nov 22, 2024

makelicious left a comment

makelicious Nov 22, 2024

naftis Nov 22, 2024

makelicious Nov 22, 2024

makelicious Nov 22, 2024

naftis Nov 22, 2024

makelicious Nov 22, 2024

makelicious Nov 22, 2024

makelicious Nov 22, 2024

makelicious Nov 22, 2024

naftis Nov 22, 2024 •

edited

Loading

makelicious Nov 22, 2024

naftis Nov 22, 2024

rikukissa Nov 22, 2024

naftis Nov 22, 2024

Events v2: Skeleton GraphQL resolvers for basic operations #8033

Events v2: Skeleton GraphQL resolvers for basic operations #8033

Conversation

rikukissa commented Nov 20, 2024 • edited Loading

github-actions bot commented Nov 20, 2024

Choose a reason for hiding this comment

rikukissa Nov 20, 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

ocrvs-bot commented Nov 22, 2024

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

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

naftis Nov 22, 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

rikukissa commented Nov 20, 2024 •

edited

Loading

rikukissa Nov 20, 2024 •

edited

Loading

naftis Nov 22, 2024 •

edited

Loading