Rewrite of legacy webhooks#2348
Conversation
🦋 Changeset detectedLatest commit: 2893bb6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
8 Skipped Deployments
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2348 +/- ##
==========================================
+ Coverage 37.84% 38.65% +0.80%
==========================================
Files 1037 1047 +10
Lines 66937 68151 +1214
Branches 3533 3663 +130
==========================================
+ Hits 25335 26345 +1010
- Misses 41221 41425 +204
Partials 381 381
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR replaces parts of the legacy catch-all notify webhook flow with dedicated, typed Saleor async webhooks (GraphQL subscriptions) for account and fulfillment events, while marking legacy events as deprecated in the dashboard and keeping existing templates working.
Changes:
- Added 9 dedicated webhook endpoints (and unit tests) for account + fulfillment events using typed GraphQL subscription payloads.
- Introduced event metadata (
label,deprecated,replacedBy) and updated UI/config generation to hide deprecated events for new tenants while hydrating existing tenants with new non-deprecated events. - Expanded default templates, example payloads, and webhook management mappings/statuses to support the new events.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/smtp/src/pages/configuration/smtp/[configurationId]/event/[eventType].tsx | Uses new event metadata to render breadcrumb label. |
| apps/smtp/src/pages/api/webhooks/notify.ts | Logs legacy webhook usage and points to replacement event metadata. |
| apps/smtp/src/pages/api/webhooks/fulfillment-tracking-number-updated.ts | New dedicated fulfillment webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/fulfillment-tracking-number-updated.test.ts | Unit tests for the new fulfillment-tracking-number-updated handler. |
| apps/smtp/src/pages/api/webhooks/fulfillment-created.ts | New dedicated fulfillment-created webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/fulfillment-created.test.ts | Unit tests for the new fulfillment-created handler. |
| apps/smtp/src/pages/api/webhooks/fulfillment-canceled.ts | New dedicated fulfillment-canceled webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/fulfillment-canceled.test.ts | Unit tests for the new fulfillment-canceled handler. |
| apps/smtp/src/pages/api/webhooks/fulfillment-approved.ts | New dedicated fulfillment-approved webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/fulfillment-approved.test.ts | Unit tests for the new fulfillment-approved handler. |
| apps/smtp/src/pages/api/webhooks/account-set-password-requested.ts | New dedicated account-set-password-requested webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/account-set-password-requested.test.ts | Unit tests for the new account-set-password-requested handler. |
| apps/smtp/src/pages/api/webhooks/account-email-changed.ts | New dedicated account-email-changed webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/account-email-changed.test.ts | Unit tests for the new account-email-changed handler. |
| apps/smtp/src/pages/api/webhooks/account-delete-requested.ts | New dedicated account-delete-requested webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/account-delete-requested.test.ts | Unit tests for the new account-delete-requested handler. |
| apps/smtp/src/pages/api/webhooks/account-confirmation-requested.ts | New dedicated account-confirmation-requested webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/account-confirmation-requested.test.ts | Unit tests for the new account-confirmation-requested handler. |
| apps/smtp/src/pages/api/webhooks/account-change-email-requested.ts | New dedicated account-change-email-requested webhook handler (typed payload). |
| apps/smtp/src/pages/api/webhooks/account-change-email-requested.test.ts | Unit tests for the new account-change-email-requested handler. |
| apps/smtp/src/modules/webhook-management/webhook-status-dict.ts | Adds status keys for the new dedicated webhooks. |
| apps/smtp/src/modules/webhook-management/webhook-management-service.ts | Registers new webhooks and maps new events to their dedicated webhook names. |
| apps/smtp/src/modules/webhook-management/sync-webhook-status.test.ts | Updates sync-webhook-status tests to include new webhook status keys. |
| apps/smtp/src/modules/smtp/ui/smtp-events-section.tsx | Displays deprecation badge + replacement hint using new event metadata; sorts by metadata label. |
| apps/smtp/src/modules/smtp/ui/event-form.tsx | Updates template option labels to use metadata label. |
| apps/smtp/src/modules/smtp/default-templates.ts | Adds default templates + subjects for new account/fulfillment events. |
| apps/smtp/src/modules/smtp/configuration/smtp-default-empty-configurations.ts | Excludes deprecated events from default config for new tenants. |
| apps/smtp/src/modules/smtp/configuration/smtp-default-empty-configurations.test.ts | Tests default config excludes deprecated events and includes new replacements. |
| apps/smtp/src/modules/smtp/configuration/smtp-configuration.service.ts | Hydrates existing configs at read-time with missing non-deprecated events + defaults. |
| apps/smtp/src/modules/smtp/configuration/smtp-configuration.service.test.ts | Updates tests for hydration behavior and adds dedicated hydration assertions. |
| apps/smtp/src/modules/event-handlers/message-event-types.ts | Extends event types and replaces label map with richer metadata (deprecated/replacedBy). |
| apps/smtp/src/modules/event-handlers/default-payloads.ts | Adds example payloads for new dedicated account/fulfillment events. |
| apps/smtp/scripts/build-email-previews.ts | Uses metadata label when generating preview pages. |
| apps/smtp/generated/graphql.ts | Generated GraphQL types/fragments/subscriptions for new webhook payloads. |
| .changeset/dedicated-webhooks-replace-notify.md | Changeset documenting new dedicated webhooks and deprecation strategy. |
| const { payload, authData } = context; | ||
| const { fulfillment, order } = payload; | ||
|
|
||
| if (!order) { |
There was a problem hiding this comment.
fulfillment is optional in the payload type, but the handler doesn’t guard against it being null/undefined. Add an early return (200) when fulfillment is missing to avoid sending emails with incomplete fulfillment context.
|
|
||
| const { payload, authData } = context; | ||
| const { fulfillment, order } = payload; | ||
|
|
There was a problem hiding this comment.
fulfillment is optional in the payload type but isn’t validated. Add an early return when fulfillment is missing so templates don’t render emails without fulfillment details.
| if (!fulfillment) { | |
| logger.error("No fulfillment in payload. Aborting."); | |
| return res | |
| .status(200) | |
| .json({ error: "Fulfillment has not been specified in the event payload." }); | |
| } |
| const { payload, authData } = context; | ||
| const { user, channel, redirectUrl, token, shop } = payload; | ||
|
|
||
| const recipientEmail = user?.email; |
There was a problem hiding this comment.
redirectUrl and token are optional in the generated payload type, but they’re forwarded to templates that build an action link. Add explicit validation (and return 200 with an error) when either is missing to avoid sending emails with broken URLs.
| const { payload, authData } = context; | ||
| const { user, channel, redirectUrl, token, shop } = payload; | ||
|
|
||
| const recipientEmail = user?.email; | ||
|
|
There was a problem hiding this comment.
redirectUrl/token are optional on the payload type, but the handler forwards them to templates that rely on them for the CTA link. Please validate they’re present before sending the message (return 200 + error when missing).
| const channelSlug = order.channel?.slug; | ||
|
|
||
| if (!channelSlug) { | ||
| logger.error(`The order ${order.number} had no channel set. Aborting.`); | ||
|
|
There was a problem hiding this comment.
Test coverage: the handler has a branch for missing order.channel.slug (returns 200 with an error), but the added unit tests don’t cover it. Add a test case with a payload missing order.channel/slug to lock in this behavior.
|
|
||
| const { payload, authData } = context; | ||
| const { fulfillment, order } = payload; | ||
|
|
There was a problem hiding this comment.
fulfillment can be null per the generated payload type, but the handler doesn’t validate it before forwarding into templates. Consider returning early (200) when fulfillment is missing (similar to other webhook handlers that validate their root object).
| if (!fulfillment) { | |
| logger.error("No fulfillment in payload. Aborting."); | |
| return res | |
| .status(200) | |
| .json({ error: "Fulfillment has not been specified in the event payload." }); | |
| } |
| const { payload, authData } = context; | ||
| const { user, channel, redirectUrl, token, shop } = payload; | ||
|
|
There was a problem hiding this comment.
redirectUrl and token are optional in the generated payload type, but the default template constructs a reset link from them. Add explicit validation and return 200 with an error when either is missing to avoid sending broken emails.
| const { payload, authData } = context; | ||
| const { user, channel, redirectUrl, token, shop, newEmail } = payload; | ||
|
|
There was a problem hiding this comment.
The template content for this event depends on newEmail, redirectUrl, and token, but the handler only validates the current recipient email + channel. Since these fields are optional in the generated type, add validation and return 200 with an error when any are missing.
| const channelSlug = order.channel?.slug; | ||
|
|
||
| if (!channelSlug) { |
There was a problem hiding this comment.
Test coverage: there’s an explicit branch when order.channel.slug is missing, but the current unit tests only cover missing order and missing order.userEmail. Add a test for the missing-channel case.
| const channelSlug = order.channel?.slug; | ||
|
|
||
| if (!channelSlug) { |
There was a problem hiding this comment.
Test coverage: the missing order.channel.slug branch isn’t covered by the new tests. Add a unit test to assert the handler returns 200 with the channel error and does not call sendEventMessages.
No description provided.