-
Notifications
You must be signed in to change notification settings - Fork 5
feat: notification methods on PlatformClient, backend types via submodule #39
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
97e126f
feat: notification methods on PlatformClient
ABujalance 8df588e
chore: track the backend contract from dev
ABujalance b97077d
ci: authenticate the cross-repo submodule fetch
ABujalance dbe6e66
ci: explain how to fix a missing backend-types credential
ABujalance 57825a2
ci: mint the backend-types credential from a GitHub App
ABujalance 9b074a3
feat: subscribe to automations and listen for notifications live
ABujalance 4adc76a
revert: drop the backend types submodule from this PR
ABujalance File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| --- | ||
| '@thatopen/services': minor | ||
| --- | ||
|
|
||
| Add notification methods to `PlatformClient`: `getNotifications`, | ||
| `getUnreadNotificationCount`, `markNotificationsRead`, | ||
| `markAllNotificationsRead`, `getNotificationSubscriptions` and | ||
| `unsubscribeFromAutomation`. All scoped to the signed-in user via the | ||
| bearer token an app already has. | ||
|
|
||
| The notification types are re-exported from the backend repo rather than | ||
| copied into `src/types`, so the contract cannot drift from what the API | ||
| sends. Run `yarn types:update` to move the pin. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| --- | ||
| '@thatopen/services': minor | ||
| --- | ||
|
|
||
| Add `subscribeToAutomation` and `updateAutomationSubscription`, so an app can | ||
| create a subscription rather than only listing and cancelling one. | ||
|
|
||
| Add `onNotification`, a live socket subscription for the signed-in user. | ||
| Unlike `onExecutionProgress` it stays connected for the session rather than | ||
| closing on a terminal event, and it returns a function that disconnects. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| [submodule "vendor/backend-api"] | ||
| path = vendor/backend-api | ||
| url = https://github.com/ThatOpen/platform_backend-api.git | ||
| branch = dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| #!/usr/bin/env bash | ||
| # Refresh the backend contract types. | ||
| # | ||
| # The shared DTOs live in the backend repo, so they arrive here as a git | ||
| # submodule. Two things this script does that a plain `git submodule update` | ||
| # does not: | ||
| # | ||
| # 1. Applies a sparse checkout, so only `src/common/dto` lands on disk | ||
| # instead of the whole backend repository. Sparse config lives in the | ||
| # submodule's .git and is not committed, so it has to be re-applied on | ||
| # every fresh clone — including in CI. | ||
| # 2. Moves the pin to the tip of the tracked branch. Deliberately a command | ||
| # you run, not a postinstall hook: an install that silently moved the | ||
| # pin would break builds with no commit explaining why. | ||
| # | ||
| # After running this, `git add vendor/backend-api` and commit the new pin. | ||
| set -euo pipefail | ||
|
|
||
| SUBMODULE_PATH="vendor/backend-api" | ||
| SPARSE_PATH="src/common/dto" | ||
| BACKEND_REPO="ThatOpen/platform_backend-api" | ||
| KEY_SECRET="BACKEND_TYPES_DEPLOY_KEY" | ||
| APP_ID_SECRET="BACKEND_TYPES_APP_ID" | ||
| APP_KEY_SECRET="BACKEND_TYPES_APP_PRIVATE_KEY" | ||
| TOKEN_SECRET="BACKEND_TYPES_TOKEN" | ||
|
|
||
| # The backend repo is private. Locally that is fine, git uses whatever | ||
| # credentials you already have. In CI there is no such thing, and the failure | ||
| # is a bare "Repository not found" that says nothing about what to do, so | ||
| # spell it out instead. | ||
| token_instructions() { | ||
| cat <<INSTRUCTIONS | ||
|
|
||
| The runner could not read ${BACKEND_REPO}. | ||
|
|
||
| This repo vendors the backend's shared DTOs as a submodule, and that repo | ||
| is private, so CI needs a token with read access to it. The default | ||
| GITHUB_TOKEN cannot see other repositories. | ||
|
|
||
| Preferred fix, a GitHub App. Nothing long-lived is granted: the workflow | ||
| mints a token scoped to that one repo, valid for an hour. | ||
|
|
||
| 1. https://github.com/organizations/ThatOpen/settings/apps/new | ||
| Name : anything unique, e.g. "ThatOpen CI types reader" | ||
| Homepage URL: https://github.com/ThatOpen | ||
| Webhook : UNTICK "Active", or it demands a webhook URL | ||
| Permissions -> Repository -> Contents: Read-only (nothing else) | ||
| Where installed: Only on this account | ||
|
|
||
| 2. On the App page, note the App ID, then "Generate a private key". | ||
| That downloads a .pem file. | ||
|
|
||
| 3. Install App -> ThatOpen -> Only select repositories -> | ||
| platform_backend-api | ||
|
|
||
| 4. Add two secrets here: | ||
| ${GITHUB_SERVER_URL:-https://github.com}/${GITHUB_REPOSITORY:-<this repo>}/settings/secrets/actions | ||
| ${APP_ID_SECRET} : the App ID from step 2 | ||
| ${APP_KEY_SECRET} : the whole .pem, BEGIN and END lines included | ||
|
|
||
| 5. Delete the .pem locally and re-run this job. | ||
|
|
||
| Quicker alternative, a fine-grained token. Expires within a year and is | ||
| tied to whoever made it: | ||
|
|
||
| 1. https://github.com/settings/personal-access-tokens/new | ||
| Resource owner : ThatOpen | ||
| Repository access : Only select repositories -> platform_backend-api | ||
| Repository permissions: Contents -> Read-only | ||
| 2. Store it as ${TOKEN_SECRET} in this repo's Actions secrets. Because | ||
| the owner is the organisation, it may sit in "pending approval" | ||
| until an org owner accepts it. | ||
|
|
||
| A read-only deploy key stored as ${KEY_SECRET} also works. | ||
|
|
||
| If one of these is already set, it has most likely been revoked or lost | ||
| access to ${BACKEND_REPO}. A token may simply have expired. | ||
|
|
||
| INSTRUCTIONS | ||
| } | ||
|
|
||
| fail_with_instructions() { | ||
| local headline="$1" | ||
| if [ -n "${GITHUB_ACTIONS:-}" ]; then | ||
| echo "::error title=Backend types unavailable::${headline} See the log for how to fix it." | ||
| token_instructions | ||
| if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then | ||
| { | ||
| echo "## Backend contract types could not be fetched" | ||
| echo | ||
| echo "**${headline}**" | ||
| echo '```' | ||
| token_instructions | ||
| echo '```' | ||
| } >>"$GITHUB_STEP_SUMMARY" | ||
| fi | ||
| else | ||
| echo "${headline}" | ||
| token_instructions | ||
| fi | ||
| exit 1 | ||
| } | ||
|
|
||
| # Only enforced in CI. A developer's own git credentials already cover the | ||
| # private repo, so requiring the token locally would be noise. | ||
| if [ -n "${CI:-}" ] && | ||
| [ -z "${BACKEND_TYPES_DEPLOY_KEY:-}" ] && | ||
| [ -z "${BACKEND_TYPES_TOKEN:-}" ]; then | ||
| fail_with_instructions \ | ||
| "Neither ${KEY_SECRET} nor ${TOKEN_SECRET} is set." | ||
| fi | ||
|
|
||
| # Both rewrite the submodule's https URL rather than changing .gitmodules, so | ||
| # a developer cloning over https locally is unaffected. Scoped to this | ||
| # process and undone on the way out, so the credential never becomes the | ||
| # identity for anything else in the job. | ||
| if [ -n "${BACKEND_TYPES_DEPLOY_KEY:-}" ]; then | ||
| KEY_FILE=$(mktemp) | ||
| printf '%s\n' "$BACKEND_TYPES_DEPLOY_KEY" >"$KEY_FILE" | ||
| chmod 600 "$KEY_FILE" | ||
| export GIT_SSH_COMMAND="ssh -i $KEY_FILE -o IdentitiesOnly=yes -o StrictHostKeyChecking=accept-new" | ||
| git config --global "url.git@github.com:.insteadOf" "https://github.com/" | ||
| trap 'rm -f "$KEY_FILE"; git config --global --unset-all "url.git@github.com:.insteadOf" || true' EXIT | ||
| elif [ -n "${BACKEND_TYPES_TOKEN:-}" ]; then | ||
| git config --global \ | ||
| "url.https://x-access-token:${BACKEND_TYPES_TOKEN}@github.com/.insteadOf" \ | ||
| "https://github.com/" | ||
| trap 'git config --global --unset-all "url.https://x-access-token:${BACKEND_TYPES_TOKEN}@github.com/.insteadOf" || true' EXIT | ||
| fi | ||
|
|
||
| if ! git submodule update --init "$SUBMODULE_PATH"; then | ||
| fail_with_instructions "Could not clone the backend types submodule." | ||
| fi | ||
|
|
||
| git -C "$SUBMODULE_PATH" sparse-checkout init --cone | ||
| git -C "$SUBMODULE_PATH" sparse-checkout set "$SPARSE_PATH" | ||
|
|
||
| if [ "${1:-}" = "--pin-only" ]; then | ||
| echo "Pinned at $(git -C "$SUBMODULE_PATH" rev-parse --short HEAD) (not moved)." | ||
| exit 0 | ||
| fi | ||
|
|
||
| if ! git submodule update --remote "$SUBMODULE_PATH"; then | ||
| fail_with_instructions "Could not update the backend types submodule." | ||
| fi | ||
| git -C "$SUBMODULE_PATH" sparse-checkout set "$SPARSE_PATH" | ||
|
|
||
| BRANCH=$(git config -f .gitmodules "submodule.$SUBMODULE_PATH.branch") | ||
| echo "Types updated from '$BRANCH' at $(git -C "$SUBMODULE_PATH" rev-parse --short HEAD)." | ||
| echo "Commit the new pin with: git add $SUBMODULE_PATH" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import { describe, it, expect, beforeEach, vi } from 'vitest'; | ||
|
|
||
| const handlers = new Map<string, (payload: unknown) => void>(); | ||
| const disconnect = vi.fn(); | ||
| const ioMock = vi.fn(() => ({ | ||
| on: (event: string, handler: (payload: unknown) => void) => { | ||
| handlers.set(event, handler); | ||
| }, | ||
| disconnect, | ||
| })); | ||
|
|
||
| vi.mock('socket.io-client', () => ({ io: (...args: unknown[]) => ioMock(...(args as [])) })); | ||
|
|
||
| const { PlatformClient } = await import('./platform-client'); | ||
|
|
||
| const API = 'https://api.example.com'; | ||
|
|
||
| describe('PlatformClient — live notifications', () => { | ||
| let client: InstanceType<typeof PlatformClient>; | ||
|
|
||
| beforeEach(() => { | ||
| handlers.clear(); | ||
| ioMock.mockClear(); | ||
| disconnect.mockClear(); | ||
| client = new PlatformClient('jwt-1', API); | ||
| }); | ||
|
|
||
| it('connects to the notifications namespace with the token', async () => { | ||
| await client.onNotification(() => {}); | ||
|
|
||
| const url = (ioMock.mock.calls[0] as unknown as string[])[0]; | ||
| expect(url).toContain('/notifications'); | ||
| expect(url).toContain('accessToken=jwt-1'); | ||
| // No /api on a socket URL; that prefix is for REST only. | ||
| expect(url).not.toContain('/api/'); | ||
| }); | ||
|
|
||
| // A provider-backed client must open the socket with a current token, not | ||
| // the one it happened to be constructed with. | ||
| it('resolves the token per connection when a provider is used', async () => { | ||
| const provider = vi.fn().mockResolvedValue('fresh-token'); | ||
| const providerClient = new PlatformClient(provider, API); | ||
|
|
||
| await providerClient.onNotification(() => {}); | ||
|
|
||
| expect(provider).toHaveBeenCalled(); | ||
| expect((ioMock.mock.calls[0] as unknown as string[])[0]).toContain( | ||
| 'accessToken=fresh-token', | ||
| ); | ||
| }); | ||
|
|
||
| it('maps each server event onto one callback shape', async () => { | ||
| const seen: unknown[] = []; | ||
| await client.onNotification((event) => seen.push(event)); | ||
|
|
||
| handlers.get('notification.created')?.({ id: 'n1' }); | ||
| handlers.get('notification.read')?.({ id: 'n2' }); | ||
| handlers.get('notifications.allRead')?.({ batch: 42 }); | ||
|
|
||
| expect(seen).toEqual([ | ||
| { type: 'created', id: 'n1' }, | ||
| { type: 'read', id: 'n2' }, | ||
| { type: 'allRead', batch: 42 }, | ||
| ]); | ||
| }); | ||
|
|
||
| it('returns a disconnect function', async () => { | ||
| const stop = await client.onNotification(() => {}); | ||
|
|
||
| expect(disconnect).not.toHaveBeenCalled(); | ||
| stop(); | ||
| expect(disconnect).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the fallback is reachable: this step has no
continue-on-error, so if the secrets are missing the job dies here and the||on line 35 never gets evaluated. And since the same step is in release.yml, publishing would be blocked too.Adding
continue-on-error: truehere would make it behave the way the comment above describes.