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

Events v2: Skeleton GraphQL resolvers for basic operations #8033

Merged
merged 23 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1b7d031
add some basic operations and unit tests for interfacing with events
rikukissa Nov 18, 2024
9f83721
Merge branch 'develop' of github.com:opencrvs/opencrvs-core into ocrv…
rikukissa Nov 19, 2024
f259bc3
feat(events): create first draft of types
makelicious Nov 18, 2024
d2a03f8
feat(events): unify and clean up types
makelicious Nov 19, 2024
5d1351f
add basic structure for graphql endpoints
rikukissa Nov 20, 2024
a05906d
add tennic club membership example data
rikukissa Nov 20, 2024
68f8adb
Merge branch 'feat/event-types' of github.com:opencrvs/opencrvs-core …
rikukissa Nov 20, 2024
a9f98a9
implement initial graphql resolvers for basic event operations
rikukissa Nov 20, 2024
7914c63
implement authorisation and first GraphQL -> MongoDB request chain
rikukissa Nov 20, 2024
81f470f
add license headers to all files
rikukissa Nov 21, 2024
d8addb9
Merge branch 'feat/event-types' of github.com:opencrvs/opencrvs-core …
rikukissa Nov 21, 2024
55a2d63
Merge branch 'feat/event-types' of github.com:opencrvs/opencrvs-core …
rikukissa Nov 21, 2024
1f90403
fix tests
rikukissa Nov 21, 2024
269970d
add events package to gateway build
rikukissa Nov 21, 2024
e8add12
fix the test pipeline to not remove events package if its used in pac…
rikukissa Nov 21, 2024
307a04f
cleanup
rikukissa Nov 21, 2024
d31792f
Merge branch 'feat/event-types' of github.com:opencrvs/opencrvs-core …
rikukissa Nov 21, 2024
a5e2c8d
fix docker build dependencies
rikukissa Nov 21, 2024
e04fef8
trick knip not to complain about duplicate exports
rikukissa Nov 21, 2024
d031ed5
remove unused export
rikukissa Nov 22, 2024
6d7b57b
update docker compose config
rikukissa Nov 22, 2024
c3c97ac
Merge branch 'feat/event-types' into ocrvs-7824-b
rikukissa Nov 25, 2024
c604312
Merge branch 'develop' into ocrvs-7824-b
rikukissa Nov 25, 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
41 changes: 29 additions & 12 deletions .github/workflows/lint-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,36 @@ jobs:
with:
node-version-file: .nvmrc

- name: Extract dependencies for ${{ matrix.package }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

if: steps.check-scripts.outputs.skip != 'true'
id: extract-dependencies
run: |
DEPENDENCIES=$(node -e "
const { execSync } = require('child_process');
const output = execSync('yarn --silent workspaces info', { encoding: 'utf-8' });
const json = JSON.parse(output.replaceAll('@opencrvs', 'packages'));

const getDependencies = (pkg) =>
json[pkg].workspaceDependencies.concat(
json[pkg].workspaceDependencies.flatMap(getDependencies)
);

console.log(
getDependencies('${{ matrix.package }}').join(' ')
);
")
echo "DEPENDENCIES=${DEPENDENCIES}" >> $GITHUB_ENV
echo "Found dependencies: $DEPENDENCIES"

- name: Remove other package directories
if: steps.check-scripts.outputs.skip != 'true'
run: |
for dir in packages/*; do
if [ "$dir" != "${{ matrix.package }}" ] && [ "$dir" != "packages/commons" ] && [ "$dir" != "packages/components" ]; then
if [ "${{ matrix.package }}" == "packages/client" ] && [ "$dir" == "packages/gateway" ] ; then
echo "Skipping $dir"
else
echo "Removing $dir"
rm -rf "$dir"
fi
if echo "${{ matrix.package }} $DEPENDENCIES" | grep -q -w "$dir"; then
echo "Skipping $dir"
else
echo "Removing $dir"
rm -rf "$dir"
fi
done

Expand All @@ -119,15 +138,13 @@ jobs:

# TODO: Move out of the matrix to be built once and shared
- name: Build common package
if: steps.check-scripts.outputs.skip != 'true'
if: steps.check-scripts.outputs.skip != 'true' && contains(env.DEPENDENCIES, 'packages/commons')
run: cd packages/commons && yarn build

- name: Build components client and login
if: steps.check-scripts.outputs.skip != 'true'
if: steps.check-scripts.outputs.skip != 'true' && contains(env.DEPENDENCIES, 'packages/components')
run: |
if [[ "${{ matrix.package }}" == "packages/client" || "${{ matrix.package }}" == "packages/login" ]]; then
cd packages/components && yarn build
fi
cd packages/components && yarn build

# TODO: should run parallel to unit tests as can take as much as unit tests
- name: Run linting
Expand Down
13 changes: 13 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ services:
- CONFIG_SMS_CODE_EXPIRY_SECONDS=600
- CONFIG_TOKEN_EXPIRY_SECONDS=604800
- NODE_ENV=development
- EVENTS_URL=http://events:5555/
- FHIR_URL=http://hearth:3447/fhir
- USER_MANAGEMENT_URL=http://user-mgnt:3030/
- SEARCH_URL=http://search:9090/
Expand All @@ -86,6 +87,18 @@ services:
- CHECK_INVALID_TOKEN=true
- MINIO_BUCKET=ocrvs
- DOCUMENTS_URL=http://documents:9050
events:
image: opencrvs/ocrvs-events:${VERSION}
#platform: linux/amd64
build:
context: .
dockerfile: ./packages/events/Dockerfile
restart: unless-stopped
depends_on:
- base
environment:
- MONGO_URL=mongodb://mongo1/events

# User facing services
workflow:
image: opencrvs/ocrvs-workflow:${VERSION}
Expand Down
4 changes: 4 additions & 0 deletions packages/client/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@ FROM opencrvs/ocrvs-base:${BRANCH}

USER node

COPY --chown=node:node packages/gateway /app/packages/gateway
COPY --chown=node:node packages/events /app/packages/events

WORKDIR /app/packages/components
COPY --chown=node:node packages/components /app/packages/components

RUN yarn install --frozen-lockfile && yarn build
ENV CONTENT_SECURITY_POLICY_WILDCARD "{{CONTENT_SECURITY_POLICY_WILDCARD}}"
ENV COUNTRY_CONFIG_URL_INTERNAL "{{COUNTRY_CONFIG_URL_INTERNAL}}"
Expand Down
3 changes: 2 additions & 1 deletion packages/client/Dockerfile.dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ packages/*
!packages/commons
!packages/components
!packages/client
!packages/gateway
!packages/gateway
!packages/events
1 change: 1 addition & 0 deletions packages/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
"xregexp": "^4.2.0"
},
"devDependencies": {
"@opencrvs/gateway": "^1.5.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

does the client need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

"@graphql-codegen/add": "^5.0.0",
"@graphql-codegen/cli": "^5.0.0",
"@graphql-codegen/introspection": "^3.0.0",
Expand Down
5 changes: 5 additions & 0 deletions packages/commons/src/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,8 @@ export const getTokenPayload = (token: string): ITokenPayload => {
}
return decoded
}

export const getUserId = (token: string): string => {
const tokenPayload = getTokenPayload(token.split(' ')[1])
return tokenPayload.sub
}
33 changes: 21 additions & 12 deletions packages/commons/src/events/Action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export const ActionConfig = z.object({
})

export const ActionInputBase = z.object({
type: z.enum(actionTypes as NonEmptyArray<ActionType>),
fields: z.array(
z.object({
id: z.string(),
Expand All @@ -62,18 +61,28 @@ export const ActionInputBase = z.object({
)
})

export const ActionInput = z.union([
ActionInputBase.extend({
type: z.enum([ActionType.CREATE])
}),
ActionInputBase.extend({
type: z.enum([ActionType.REGISTER]),
identifiers: z.object({
trackingId: z.string(),
registrationNumber: z.string()
})
export const CreateActionInput = ActionInputBase.extend({})
export const NotifyActionInput = ActionInputBase.extend({})
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

export const DeclareActionInput = ActionInputBase.extend({})
export const RegisterActionInput = ActionInputBase.extend({
identifiers: z.object({
trackingId: z.string(),
registrationNumber: z.string()
})
])
})

export const ActionInput = z
.union([
CreateActionInput.extend({ type: z.enum([ActionType.CREATE]) }),
NotifyActionInput.extend({ type: z.enum([ActionType.NOTIFY]) }),
DeclareActionInput.extend({ type: z.enum([ActionType.DECLARE]) }),
RegisterActionInput.extend({ type: z.enum([ActionType.REGISTER]) })
])
.and(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there semantic difference between extend and and?

Copy link
Collaborator

@naftis naftis Nov 22, 2024

Choose a reason for hiding this comment

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

.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

z.object({
createdBy: z.string()
})
)

export type ActionInput = z.infer<typeof ActionInput>

Expand Down
20 changes: 1 addition & 19 deletions packages/commons/src/events/Event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,7 @@ import { Label, Summary } from './utils'
* A subset of an event. Describes fields that can be sent to the system with the intention of either creating or mutating a an event
*/
export const EventInput = z.object({
type: z.string(),
fields: z.array(
z.object({
id: z.string(),
value: z.union([
z.string(),
z.number(),
z.array(
// @TODO: Check if we could make this stricter
z.object({
optionValues: z.array(z.string()),
type: z.string(),
data: z.string(),
fileSize: z.number()
})
)
])
})
)
type: z.string()
Copy link
Member Author

@rikukissa rikukissa Nov 20, 2024

Choose a reason for hiding this comment

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

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

})
export type EventInput = z.infer<typeof EventInput>

Expand Down
45 changes: 18 additions & 27 deletions packages/events/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@
import { vi } from 'vitest'
import { appRouter, t } from './router'
import {
setupServer,
getClient,
resetServer
resetServer,
setupServer
} from './storage/__mocks__/mongodb'
import { ActionType } from '@opencrvs/commons'

const { createCallerFactory } = t

Expand All @@ -31,15 +30,17 @@ afterEach(async () => {

function createClient() {
const createCaller = createCallerFactory(appRouter)
const caller = createCaller({})
const caller = createCaller({
user: { id: '1' }
})
return caller
}

const client = createClient()
test('event can be created and fetched', async () => {
const event = await client.event.create({
transactionId: '1',
event: { type: 'birth', fields: [] }
event: { type: 'birth' }
})

const fetchedEvent = await client.event.get(event.id)
Expand All @@ -52,12 +53,12 @@ test('creating an event is an idempotent operation', async () => {

await client.event.create({
transactionId: '1',
event: { type: 'birth', fields: [] }
event: { type: 'birth' }
})

await client.event.create({
transactionId: '1',
event: { type: 'birth', fields: [] }
event: { type: 'birth' }
})

expect(await db.collection('events').find().toArray()).toHaveLength(1)
Expand All @@ -66,13 +67,12 @@ test('creating an event is an idempotent operation', async () => {
test('stored events can be modified', async () => {
const originalEvent = await client.event.create({
transactionId: '1',
event: { type: 'birth', fields: [] }
event: { type: 'birth' }
})

const event = await client.event.patch({
id: originalEvent.id,
type: 'death',
fields: []
type: 'death'
})

expect(event.updatedAt).not.toBe(originalEvent.updatedAt)
Expand All @@ -82,28 +82,19 @@ test('stored events can be modified', async () => {
test('actions can be added to created events', async () => {
const originalEvent = await client.event.create({
transactionId: '1',
event: { type: 'birth', fields: [] }
event: { type: 'birth' }
})

const event = await client.event.actions.create({
const event = await client.event.actions.declare({
eventId: originalEvent.id,
transactionId: '2',
action: {
type: ActionType.REGISTER,
fields: [],
identifiers: {
trackingId: '123',
registrationNumber: '456'
}
fields: []
}
})

expect(event.actions).toContainEqual(
expect.objectContaining({
type: ActionType.REGISTER,
identifiers: {
trackingId: '123',
registrationNumber: '456'
}
})
)
expect(event.actions).toEqual([
expect.objectContaining({ type: 'CREATE' }),
expect.objectContaining({ type: 'DECLARE' })
])
})
26 changes: 25 additions & 1 deletion packages/events/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,33 @@ require('app-module-path').addPath(require('path').join(__dirname, '../'))

import { appRouter } from './router'
import { createHTTPServer } from '@trpc/server/adapters/standalone'
import { getUserId } from '@opencrvs/commons/authentication'
import { TRPCError } from '@trpc/server'

const server = createHTTPServer({
router: appRouter
router: appRouter,
createContext: function createContext(opts) {
const token = opts.req.headers.authorization
if (!token) {
throw new TRPCError({
code: 'UNAUTHORIZED'
})
}

const userId = getUserId(token)

if (!userId) {
throw new TRPCError({
code: 'UNAUTHORIZED'
})
}

return {
user: {
id: userId
}
}
}
})

server.listen(5555)
Loading
Loading