fix(webhooks): schedule a no-show task for every subscriber - #29835
fix(webhooks): schedule a no-show task for every subscriber#29835AlgoArtist06 wants to merge 1 commit into
Conversation
Task rows are unique on (referenceUid, type), and every Cal Video no-show task for a booking was created with referenceUid: booking.uid. Multiple active webhooks subscribed to the same no-show trigger therefore resolved to the same key, so only the first subscriber was scheduled and the resulting P2002 propagated out of handleConfirmation, preventing the BOOKING_CREATED payloads dispatched after that call from being sent. Scope the task reference to the subscriber so each active webhook gets its own independently scheduled task, and match every subscriber's reference when cancelling a booking's no-show tasks. Cancellation also accepts a webhookId so removing a trigger from one webhook no longer cancels its siblings' tasks, and it still matches the bare booking uid so tasks queued before this change stay cancellable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Welcome to Cal.diy, @AlgoArtist06! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthroughNo-show task references now combine the booking UID with the webhook ID for host and guest scheduling. Cancellation accepts optional webhook scoping and matches both legacy booking-only references and new scoped references, while restricting deletion to relevant no-show task types. Unit and integration tests cover multiple subscribers, distinct references, payloads, cancellation scope, legacy tasks, non-no-show triggers, and non-Cal Video bookings. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@packages/features/webhooks/lib/scheduleTrigger.ts`:
- Line 628: Update the deleteMany calls in the no-specific-trigger paths of
scheduleTrigger to retain referenceFilter matching while restricting type to
triggerHostNoShowWebhook or triggerGuestNoShowWebhook. Apply this constraint to
both affected deletion paths and leave targeted-trigger deletion behavior
unchanged.
- Around line 615-617: Update the referenceFilter construction in the no-show
cancellation flow to match both the webhook-specific reference UID and the
legacy booking-only reference when webhookId is supplied. Preserve
getNoShowTaskReferenceUidFilter(bookingUid) behavior for calls without
webhookId, ensuring updateTriggerForExistingBookings cancels both task formats.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: acbad9c3-1373-4b8d-bd2a-a8000b3d11a8
📒 Files selected for processing (6)
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.tspackages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.test.tspackages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.tspackages/features/webhooks/lib/cancelNoShowTasksForBooking.test.tspackages/features/webhooks/lib/noShowTaskReference.tspackages/features/webhooks/lib/scheduleTrigger.ts
| const referenceFilter = webhookId | ||
| ? { referenceUid: getNoShowTaskReferenceUid({ bookingUid, webhookId }) } | ||
| : getNoShowTaskReferenceUidFilter(bookingUid); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Include the legacy UID when cancelling one subscriber.
When webhookId is supplied, this filter excludes pre-deployment tasks keyed only by bookingUid. updateTriggerForExistingBookings now always supplies webhookId for a removed no-show subscription, leaving its legacy task scheduled.
Proposed fix
const referenceFilter = webhookId
- ? { referenceUid: getNoShowTaskReferenceUid({ bookingUid, webhookId }) }
+ ? {
+ OR: [
+ { referenceUid: bookingUid },
+ { referenceUid: getNoShowTaskReferenceUid({ bookingUid, webhookId }) },
+ ],
+ }
: getNoShowTaskReferenceUidFilter(bookingUid);As per PR objectives, cancellation must preserve legacy booking-only references.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const referenceFilter = webhookId | |
| ? { referenceUid: getNoShowTaskReferenceUid({ bookingUid, webhookId }) } | |
| : getNoShowTaskReferenceUidFilter(bookingUid); | |
| const referenceFilter = webhookId | |
| ? { | |
| OR: [ | |
| { referenceUid: bookingUid }, | |
| { referenceUid: getNoShowTaskReferenceUid({ bookingUid, webhookId }) }, | |
| ], | |
| } | |
| : getNoShowTaskReferenceUidFilter(bookingUid); |
🤖 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 `@packages/features/webhooks/lib/scheduleTrigger.ts` around lines 615 - 617,
Update the referenceFilter construction in the no-show cancellation flow to
match both the webhook-specific reference UID and the legacy booking-only
reference when webhookId is supplied. Preserve
getNoShowTaskReferenceUidFilter(bookingUid) behavior for calls without
webhookId, ensuring updateTriggerForExistingBookings cancels both task formats.
| where: { ...referenceFilter, type: "triggerGuestNoShowWebhook" }, | ||
| }); | ||
| } else { | ||
| await prisma.task.deleteMany({ where: referenceFilter }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Restrict these deletes to no-show task types.
Both paths delete every task type sharing the reference UID. Keep the reference matching, but add type: { in: ["triggerHostNoShowWebhook", "triggerGuestNoShowWebhook"] } when no specific trigger is supplied.
As per PR objectives, cancellation must delete only relevant no-show tasks.
Also applies to: 638-641
🤖 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 `@packages/features/webhooks/lib/scheduleTrigger.ts` at line 628, Update the
deleteMany calls in the no-specific-trigger paths of scheduleTrigger to retain
referenceFilter matching while restricting type to triggerHostNoShowWebhook or
triggerGuestNoShowWebhook. Apply this constraint to both affected deletion paths
and leave targeted-trigger deletion behavior unchanged.
1 participant
What does this PR do?
When an event type had more than one active webhook subscribed to the same Cal Video no-show trigger (
AFTER_HOSTS_CAL_VIDEO_NO_SHOW/AFTER_GUESTS_CAL_VIDEO_NO_SHOW), only the first subscriber's no-show webhook was scheduled. The rest were dropped, and on the booking-confirmation path the failure also aborted the remaining webhook work for that booking.Root cause
Taskis unique on(referenceUid, type):referenceUid String? @@unique([referenceUid, type])Every no-show task for a booking was created with
referenceUid: booking.uidand a type that only distinguishes host from guest. So N subscribers to the same trigger all resolved to the same(referenceUid, type)pair. The first insert won; every later one threwP2002.Because
scheduleNoShowTriggersawaitsPromise.all(...), thatP2002propagated out ofhandleConfirmation, so theBOOKING_CREATEDpayloads dispatched after the call were never sent — this is the second symptom described in the issue.The fix
Scope the task reference to the subscriber instead of only the booking, via a small shared helper (
packages/features/webhooks/lib/noShowTaskReference.ts):scheduleNoShowTriggers(booking flow) andscheduleNoShowTaskForBooking(webhook create/update flow). Each subscriber gets its own independently scheduled task, and no insert conflicts with another subscriber's.cancelNoShowTasksForBookingmatches every subscriber's reference for a booking, so cancelling a booking still clears all of its no-show tasks. It also accepts an optionalwebhookId, which the "trigger removed from one webhook" path now passes so that deactivating one subscriber no longer cancels its siblings' tasks.booking.uidform, so tasks already queued before this change stay cancellable after deploy. No schema change or migration is needed.Booking uids are base58 short-uuids, so the
_separator cannot appear in the booking portion of the reference and the prefix match stays unambiguous.Visual Demo (For contributors especially)
N/A — backend-only change with no UI surface. The behaviour is demonstrated by the tests below, including one that reproduces the original
P2002against a real PostgreSQL database.Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
No environment variables are required. Minimal test data is an event type with two active webhooks pointing at different URLs, both subscribed to the same no-show trigger, both with
time+timeUnitset, plus a Cal Video booking.Expected happy path: each subscriber gets its own
Taskrow (triggerHostNoShowWebhook/triggerGuestNoShowWebhook) with a distinctreferenceUid, and theBOOKING_CREATEDwebhooks for that booking are still delivered.Automated tests:
Results on this branch: 11 unit tests pass, 4 integration tests pass. The new integration test
schedules a task for every subscriber sharing the same no-show triggerfails withUnique constraint failed on the fields: ("referenceUid", type)when the source fix is reverted, which is the exact failure from the issue.yarn type-check:ci --forcepasses (9/9 packages, no TS errors).yarn lint(biome lint, the CI gate) reports no errors on the changed files, andbiome formatreports nothing to reformat.For full transparency:
biome checkalso raisesassist/source/organizeImportsonscheduleTrigger.tsandscheduleNoShowTriggers.integration-test.ts, but that is pre-existing onmainfor those same two files (verified against themaincopies), and applying it would reorder unrelated pre-existing imports. I left it out to keep the diff scoped to the fix.Note on the existing integration fixtures
The two pre-existing fixtures in
scheduleNoShowTriggers.integration-test.tsomitteduid, soreferenceUidwas written asNULL— and PostgreSQL treatsNULLs as distinct, which is why the collision never showed up there. I added auidto those fixtures (the flow's type already requiresuid: string, and real callers always pass one) so the test exercises the same key that production does.Checklist