Webhook subscriptions - #122
Merged
Merged
Conversation
Fix ESLint no-unsafe-call on getInstancePlayerInfo in instances test (avoid .catch(console.error) widening types). Made-with: Cursor
Made-with: Cursor
…s in tests Made-with: Cursor
…Auth in admin tests Made-with: Cursor
Made-with: Cursor
…achUserAuth) Made-with: Cursor
Barecheck - Code coverage reportTotal: 89.61%Your code coverage diff: 0.19% ▴ Uncovered files and lines |
…I coverage) Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces Discord webhook subscription management endpoints (create/update/delete/status) backed by Postgres, adds Discord invocation context auth handling, and extends player search with pagination via offset.
Changes:
- Add Discord webhook subscription service +
/subscriptions/discord/webhooksroutes and OpenAPI schema components. - Add auth context plumbing (
req.auth,req.discord) and refactor protected-resource/admin checks to use parsed auth context. - Add
offsetpagination to player search query/service and update related tests/docs/config.
Reviewed changes
Copilot reviewed 45 out of 46 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Includes scripts/**/*.ts in TS compilation. |
| src/services/subscriptions/discord-webhooks.ts | Implements Discord webhook destination/rule upsert, status, and deactivation logic. |
| src/services/subscriptions/discord-webhooks.test.ts | Adds unit tests for webhook registration/upsert/status/deactivation behaviors. |
| src/services/search/player-search.ts | Adds offset support to SQL query and membershipId shortcut behavior. |
| src/services/search/player-search.test.ts | Updates service tests to pass offset and minor assertion tweaks. |
| src/services/reporting/standing.test.ts | Removes non-null assertions in expectations. |
| src/services/player-instances/instances.test.ts | Adjusts non-null assertions and error handling in tests. |
| src/services/manifest/urls.ts | Removes non-null assertion on result[activityId]. |
| src/services/instance/instance.test.ts | Removes non-null assertion when reading fixture insert result. |
| src/schema/errors/ErrorCode.ts | Adds InvalidDiscordAuthError error code. |
| src/schema/components/DiscordSubscriptionWebhook.ts | Adds Zod/OpenAPI schemas for Discord webhook subscription APIs. |
| src/routes/subscriptions/index.ts | Adds /subscriptions router. |
| src/routes/subscriptions/discord/webhooks.ts | Adds GET/PUT/DELETE routes for Discord webhook subscription management. |
| src/routes/subscriptions/discord/index.ts | Wires Discord subscription webhook routes under /subscriptions/discord. |
| src/routes/player/search.ts | Adds offset query param and includes it in response params. |
| src/routes/player/search.test.ts | Updates route tests to remove non-null assertions. |
| src/routes/player/membershipId/teammates.ts | Switches protected-resource check to use req.auth. |
| src/routes/player/membershipId/profile.ts | Switches protected-resource check to use req.auth. |
| src/routes/player/membershipId/instances.ts | Switches protected-resource check to use req.auth. |
| src/routes/player/membershipId/history.ts | Switches protected-resource check to use req.auth. |
| src/routes/instance.ts | Refactors control-flow formatting while preserving behavior. |
| src/routes/index.ts | Mounts new /subscriptions router. |
| src/routes/authorize/user.test.ts | Removes non-null assertion on process.env.JWT_SECRET in test. |
| src/routes/authorize/admin.test.ts | Removes non-null assertion on process.env.JWT_SECRET in test. |
| src/integrations/postgres/transactional.ts | Simplifies queryRow return (`rows[0] |
| src/integrations/postgres/reader.ts | Simplifies queryRow return (`rows[0] |
| src/integrations/discord/context-jwt.ts | Adds JWT sign/verify helpers for Discord invocation context. |
| src/index.ts | Adds attachUserAuth and attachDiscordContext middleware to app pipeline. |
| src/core/RaidHubRouterTypes.ts | Extends handler request type with auth and discord contexts. |
| src/core/RaidHubRoute.ts | Adds $mock support for auth via authFromHeaders. |
| src/auth/user-context.ts | Adds Bearer JWT parsing + Express middleware attaching req.auth. |
| src/auth/user-context.test.ts | Adds tests for authFromHeaders. |
| src/auth/protected-resource.ts | Refactors protected-resource checks to use parsed auth context. |
| src/auth/express-discord-context.ts | Adds Express type augmentation for req.discord. |
| src/auth/express-auth-context.ts | Adds Express type augmentation for req.auth. |
| src/auth/discord-context.ts | Adds Express middleware attaching req.discord from Authorization: Discord <jwt>. |
| src/auth/api-keys.ts | Removes non-null assertion from API_KEYS_PATH use and updates linting scope. |
| src/auth/admin.ts | Refactors admin protection to use req.auth?.isAdmin. |
| src/auth/admin.test.ts | Updates middleware chain to include attachUserAuth and sets a test JWT secret fallback. |
| package.json | Switches typecheck to bunx tsc, updates eslint config to type-checked rules, and adjusts lint/lint-staged patterns. |
| open-api/openapi.json | Updates generated OpenAPI for new subscription endpoints and search offset. |
| example.env | Documents API_KEYS_PATH and adds Discord integration env vars. |
| api-keys.example.json | Adds example API key file for local/prod usage. |
| README.md | Updates local setup instructions for API keys and PROD behavior. |
| .github/raidhub-services-version.env | Bumps referenced RaidHub-Services commit. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR review feedback by registering InvalidDiscordAuthError, documenting 401 responses for Discord webhook routes, hardening Discord auth header parsing, adding API key path fallback, and reconciling/de-duping webhook targets with bounded input size. Made-with: Cursor
Refresh generated OpenAPI output so docs/checks reflect the new Discord webhook error contracts and schema updates. Made-with: Cursor
Use an explicit unknown-to-request cast in the mock request setup so tsc accepts the test harness while preserving the header-array coverage. Made-with: Cursor
Match repository formatting expectations for discord-context middleware so the format check passes in CI. Made-with: Cursor
Contributor
Author
|
@sentry review |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Delete the Discord webhook if the Postgres transaction fails after create, since the API cannot run inside a DB transaction. Run destination reactivation and rule updates in a single transaction during upsert. Extract shared update path for reuse. Made-with: Cursor
Contributor
Author
|
Addressed the two Sentry review items in
Also factored |
Made-with: Cursor
Made-with: Cursor
Comment on lines
+358
to
+368
| try { | ||
| createdWebhook = await createDiscordWebhook({ | ||
| channelId: input.channelId, | ||
| name: input.name | ||
| }) | ||
| const webhookUrl = validateDiscordWebhookUrl(createdWebhook.url) | ||
| const requireFresh = input.filters?.requireFresh ?? false | ||
| const requireCompleted = input.filters?.requireCompleted ?? false | ||
| const playerMembershipIds = normalizeTargetIds(input.targets?.playerMembershipIds) | ||
| const clanGroupIds = normalizeTargetIds(input.targets?.clanGroupIds) | ||
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Sentry follow-up: registerDiscordWebhook always created a new channel webhook but could leave the previous Discord webhook orphaned when a row already existed (e.g. direct service calls). Look up stored webhook_id, best-effort DELETE, then create and persist the new webhook. Tests cover the DELETE+POST sequence and queue an extra queryRow for prior-id lookups. Made-with: Cursor
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.