fix: emit webhooks for campaign subscription changes - #417
Conversation
WalkthroughCampaign unsubscribe and subscribe flows now use a shared contact subscription helper. The helper updates subscription fields, emits a Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Deploying usesend with
|
| Latest commit: |
3d78b2a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://74c1f7cc.usesend.pages.dev |
| Branch Preview URL: | https://codex-fix-campaign-contact-w.usesend.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/server/service/contact-service.ts (1)
345-367: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSES hook updates still bypass
contact.updated
apps/web/src/server/service/ses-hook-parser.tsstill calls these helpers for bounce/complaint handling, so those subscription changes won’t emitcontact.updated. If downstream consumers need that event, route this path throughupdateContactSubscription.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/server/service/contact-service.ts` around lines 345 - 367, Update unsubscribeContact and subscribeContact to route subscription changes through updateContactSubscription instead of directly calling db.contact.update, ensuring SES bounce and complaint handling emits the contact.updated event while preserving the existing subscribed and unsubscribeReason values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/web/src/server/service/contact-service.ts`:
- Around line 345-367: Update unsubscribeContact and subscribeContact to route
subscription changes through updateContactSubscription instead of directly
calling db.contact.update, ensuring SES bounce and complaint handling emits the
contact.updated event while preserving the existing subscribed and
unsubscribeReason values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7375a6d7-8ec1-420a-8695-c8af66359259
📒 Files selected for processing (3)
apps/web/src/server/service/campaign-service.tsapps/web/src/server/service/contact-service.tsapps/web/src/server/service/contact-service.unit.test.ts
e65e0a0 to
3d78b2a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/src/server/service/campaign-service.unit.test.ts (2)
24-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the
$transactionmock callback type annotation.
ReturnType<typeof vi.fn>resolves to aMockinstance type, not a function signature. The annotation is semantically incorrect even though it works at runtime. Use a proper function type instead.♻️ Proposed fix
- $transaction: vi.fn(async (callback: ReturnType<typeof vi.fn>) => + $transaction: vi.fn(async (callback: (tx: typeof mockTx) => Promise<unknown>) => callback(mockTx), ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/server/service/campaign-service.unit.test.ts` around lines 24 - 26, Update the $transaction mock in the campaign service unit test to annotate its callback parameter with a proper callable function type, such as a function accepting the transaction mock and returning the expected result, instead of ReturnType<typeof vi.fn>. Preserve the existing callback(mockTx) behavior.
233-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider asserting the return value and mocking
updateContactSubscriptionreturn for the subscribe test.The
unsubscribeContacttest (lines 204-231) thoroughly asserts the return value and mocksmockUpdateContactSubscriptionwith a resolved value. ThesubscribeContacttest does neither —mockUpdateContactSubscriptionreturnsundefinedby default and no return value is asserted. IfsubscribeContactreturns or uses the updated contact, this gap could mask regressions.♻️ Suggested additions
mockDb.contact.findUnique.mockResolvedValue({ id: "contact_1", contactBookId: "book_1", email: "alice@example.com", subscribed: false, }); + const updatedContact = { ...mockDb.contact.findUnique.mock.results[0].value, subscribed: true }; + mockUpdateContactSubscription.mockResolvedValue(updatedContact); const id = "contact_1-campaign_1"; const hash = createHash("sha256").update(`${id}-test-secret`).digest("hex"); - await subscribeContact(id, hash); + const result = await subscribeContact(id, hash); expect(mockUpdateContactSubscription).toHaveBeenCalledWith({ contactId: "contact_1", subscribed: true, unsubscribeReason: null, }); expect(mockDb.campaign.update).toHaveBeenCalledWith({ where: { id: "campaign_1" }, data: { unsubscribed: { decrement: 1 } }, }); + // If subscribeContact returns the updated contact: + // expect(result).toBe(updatedContact);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/server/service/campaign-service.unit.test.ts` around lines 233 - 254, Update the “updates the contact through the webhook-emitting service on re-subscribe” test to mock mockUpdateContactSubscription with the expected resolved updated-contact value, then capture the result of subscribeContact and assert it matches that value, mirroring the unsubscribeContact test’s return-value coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/src/server/service/campaign-service.unit.test.ts`:
- Around line 24-26: Update the $transaction mock in the campaign service unit
test to annotate its callback parameter with a proper callable function type,
such as a function accepting the transaction mock and returning the expected
result, instead of ReturnType<typeof vi.fn>. Preserve the existing
callback(mockTx) behavior.
- Around line 233-254: Update the “updates the contact through the
webhook-emitting service on re-subscribe” test to mock
mockUpdateContactSubscription with the expected resolved updated-contact value,
then capture the result of subscribeContact and assert it matches that value,
mirroring the unsubscribeContact test’s return-value coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 062484d6-d49b-4f63-b2ee-0560c9fe1ce3
📒 Files selected for processing (4)
apps/web/src/server/service/campaign-service.tsapps/web/src/server/service/campaign-service.unit.test.tsapps/web/src/server/service/contact-service.tsapps/web/src/server/service/contact-service.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/server/service/contact-service.unit.test.ts
- apps/web/src/server/service/campaign-service.ts
- apps/web/src/server/service/contact-service.ts
1 participant
Summary
contact.updatedafter campaign unsubscribe and re-subscribe state changesFixes #413.
Impact
Webhook consumers can now synchronize per-contact suppression state when subscription status changes through campaign links, RFC 8058 one-click unsubscribe, or campaign delivery events. Existing campaign unsubscribe counters remain unchanged.
Verification
pnpm --filter=web exec vitest run -c vitest.unit.config.ts src/server/service/contact-service.unit.test.ts(16 passed)pnpm --filter=web exec eslint src/server/service/contact-service.ts src/server/service/campaign-service.ts src/server/service/contact-service.unit.test.ts --max-warnings 0git diff --checkMigration notes
No database migrations or configuration changes.
Screenshots
Not applicable; server-side behavior only.
Summary by cubic
Emit
contact.updatedwebhooks when campaign actions change a contact’s subscription so downstream systems can sync suppression state in real time. Also standardizes unsubscribe reason handling and returns the updated contact on unsubscribe.Bug Fixes
contact.updatedafter unsubscribe and re-subscribe via campaign flows (including RFC 8058 one-click).unsubscribeContactnow returns the updated contact.Refactors
updateContactSubscriptionto centralize subscription updates and webhook emission; used by campaignunsubscribeContactandsubscribeContact.Written for commit 3d78b2a. Summary will update on new commits.
Summary by CodeRabbit