feat: expired reservation tracking #21984
Conversation
|
@weknowyourgame is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (06/23/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (06/23/25)1 label was added to this PR based on Keith Williams's automation. |
|
Hey @weknowyourgame . Thanks for the PR. Could you please add a loom video showing the feature? |
loom_1080p.1.mp4Please let me know if this works or if any changes are needed |
| const webhookData = { | ||
| eventType: "RESERVATION_EXPIRED", | ||
| reservationUid: reservation.uid, | ||
| eventTypeId: reservation.eventTypeId, | ||
| eventTypeTitle: eventType?.title, | ||
| slotStart: reservation.slotUtcStartDate.toISOString(), | ||
| slotEnd: reservation.slotUtcEndDate.toISOString(), | ||
| userId: reservation.userId, | ||
| userName: user?.name, | ||
| userEmail: user?.email, | ||
| expiredAt: reservation.releaseAt.toISOString(), | ||
| createdAt: new Date().toISOString(), | ||
| }; | ||
|
|
||
| // Send webhooks | ||
| const promises = webhooks.map((webhook) => | ||
| sendGenericWebhookPayload({ | ||
| secretKey: webhook.secret, | ||
| triggerEvent: WebhookTriggerEvents.RESERVATION_EXPIRED, | ||
| createdAt: new Date().toISOString(), | ||
| webhook, | ||
| data: webhookData, |
There was a problem hiding this comment.
Looks like there are two createdAt fields being sent here. Let’s remove the one from the payload. It’s a bit confusing and makes it seem like it’s the booking’s creation time instead of the webhook trigger time.
| const webhooks = await prisma.webhook.findMany({ | ||
| where: { | ||
| active: true, | ||
| eventTriggers: { has: WebhookTriggerEvents.RESERVATION_EXPIRED }, | ||
| OR: [ | ||
| // Event type specific webhooks | ||
| { eventTypeId: reservation.eventTypeId }, | ||
| // User specific webhooks | ||
| { userId: eventType?.userId }, | ||
| // Team specific webhooks | ||
| { teamId: eventType?.teamId }, | ||
| // Global webhooks (no specific eventTypeId, userId, or teamId) | ||
| { | ||
| AND: [{ eventTypeId: null }, { userId: null }, { teamId: null }], | ||
| }, | ||
| ], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
This does not take into account org wide webhooks. Org wide webhooks are not working for Reservation expired trigger.
| const webhooks = await prisma.webhook.findMany({ | ||
| where: { | ||
| active: true, | ||
| eventTriggers: { has: WebhookTriggerEvents.RESERVATION_EXPIRED }, | ||
| OR: [ | ||
| // Event type specific webhooks | ||
| { eventTypeId: reservation.eventTypeId }, | ||
| // User specific webhooks | ||
| { userId: eventType?.userId }, | ||
| // Team specific webhooks | ||
| { teamId: eventType?.teamId }, | ||
| // Global webhooks (no specific eventTypeId, userId, or teamId) | ||
| { | ||
| AND: [{ eventTypeId: null }, { userId: null }, { teamId: null }], | ||
| }, | ||
| ], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
This is not needed. There is no such thing as global webhooks. cc @anikdhabal
There was a problem hiding this comment.
cubic found 5 issues across 14 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
|
hey @kart1ka |
WalkthroughThis change set introduces a new "Reservation Expired" webhook trigger and supporting infrastructure for tracking and notifying on expired reservations. It adds a new cron-triggered GitHub Actions workflow that regularly calls an API endpoint to process expired reservations. The backend API route identifies reservations whose release time has passed, sends webhook notifications to configured endpoints, and deletes expired reservations, with robust error handling and logging. The Prisma schema and migrations are updated to support the new webhook event type. The webhook form UI and constants are extended to allow selection of the new trigger. Comprehensive unit and end-to-end tests validate the new logic, and documentation and localization are updated accordingly. Estimated code review effort4 (approximately one day) Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/api/cron/reservationExpiry/__tests__/cron.test.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/web/modules/settings/platform/oauth-clients/[clientId]/edit/edit-webhooks-view.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/web/app/api/cron/reservationExpiry/route.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/app/api/cron/reservationExpiry/route.ts (1)
13-142: Remove duplicate createdAt field from webhook payloadThe webhook payload is sending two
createdAtfields - one in the payload data and another in thesendGenericWebhookPayloadcall. This is confusing as it suggests the booking's creation time rather than the webhook trigger time.if (webhooks.length > 0) { const webhookData = { eventType: "RESERVATION_EXPIRED", reservationUid: reservation.uid, eventTypeId: reservation.eventTypeId, eventTypeTitle: eventType?.title, slotStart: reservation.slotUtcStartDate.toISOString(), slotEnd: reservation.slotUtcEndDate.toISOString(), userId: reservation.userId, userName: user?.name, userEmail: user?.email, expiredAt: reservation.releaseAt.toISOString(), + createdAt: reservation.slotUtcStartDate.toISOString(), };
🧹 Nitpick comments (2)
packages/prisma/migrations/20250623105707_add_reservation_expired_webhook_trigger/migration.sql (1)
1-2: Make the enum alteration idempotentIf this migration is ever re-applied (for example on a restored database snapshot) it will fail because the value already exists. Adding the
IF NOT EXISTSguard makes the change safe while remaining Postgres-version-compatible.-ALTER TYPE "WebhookTriggerEvents" ADD VALUE 'RESERVATION_EXPIRED'; +ALTER TYPE "WebhookTriggerEvents" ADD VALUE IF NOT EXISTS 'RESERVATION_EXPIRED';apps/web/public/static/locales/en/common.json (1)
3381-3381: Key added in the right spot – consider propagating to all locales
"reservation_expired"is correctly inserted above the sentinel line, JSON syntax remains valid.
Follow-up: add this key to every supported locale file to avoid missing-string fallbacks at runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/cron-reservationExpiry.yml(1 hunks)apps/web/app/api/cron/reservationExpiry/__tests__/cron.test.ts(1 hunks)apps/web/app/api/cron/reservationExpiry/route.ts(1 hunks)apps/web/modules/settings/platform/oauth-clients/[clientId]/edit/edit-webhooks-view.tsx(1 hunks)apps/web/playwright/webhook.e2e.ts(1 hunks)apps/web/public/static/locales/en/common.json(1 hunks)docs/developing/guides/automation/webhooks.mdx(2 hunks)packages/features/webhooks/components/WebhookForm.tsx(1 hunks)packages/features/webhooks/lib/constants.ts(1 hunks)packages/features/webhooks/lib/sendPayload.ts(3 hunks)packages/prisma/migrations/20250623105707_add_reservation_expired_webhook_trigger/migration.sql(1 hunks)packages/prisma/schema.prisma(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
apps/web/modules/settings/platform/oauth-clients/[clientId]/edit/edit-webhooks-view.tsx (1)
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
packages/features/webhooks/components/WebhookForm.tsx (1)
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
packages/features/webhooks/lib/constants.ts (1)
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
apps/web/app/api/cron/reservationExpiry/route.ts (3)
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Learnt from: eunjae-lee
PR: #22106
File: packages/lib/server/service/insightsRouting.ts:367-368
Timestamp: 2025-07-15T13:02:17.359Z
Learning: In the InsightsRoutingService (packages/lib/server/service/insightsRouting.ts), multi-select filter data is already validated by zod before reaching the buildFormFieldSqlCondition method, so null/undefined values are not present in filterValue.data arrays and don't need to be filtered out.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
apps/web/public/static/locales/en/common.json (1)
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.201Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
packages/features/webhooks/lib/sendPayload.ts (1)
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
🪛 actionlint (1.7.7)
.github/workflows/cron-reservationExpiry.yml
8-8: scheduled job runs too frequently. it runs once per 60 seconds. the shortest interval is once every 5 minutes
(events)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Security Check
🔇 Additional comments (12)
packages/prisma/schema.prisma (1)
968-988: Additions look good – make sure the rest of the stack is regenerated
RESERVATION_EXPIREDis appended at the end ofWebhookTriggerEvents, so the ordering rules for Postgres enums are respected.
Please ensure the following follow-ups are executed before merging:
- Run
prisma generate(and anyzod-prisma/kysely code-gen) so the new enum value is reflected in the generated types.- Re-run the full test suite and type-check; any switch/union exhaustiveness relying on the enum will now need to handle the new member.
No code changes required here – just the usual regeneration & verification steps.
packages/features/webhooks/components/WebhookForm.tsx (1)
60-60: LGTM! Clean addition of new webhook trigger event.The new
RESERVATION_EXPIREDwebhook trigger event is properly added to the core events list and follows the existing pattern.apps/web/modules/settings/platform/oauth-clients/[clientId]/edit/edit-webhooks-view.tsx (1)
72-72: LGTM! Consistent addition of RESERVATION_EXPIRED trigger.The new webhook trigger event is properly added to the OAuth client webhook options, maintaining consistency with the core webhook form.
packages/features/webhooks/lib/constants.ts (1)
21-21: LGTM! Proper addition to webhook constants.The
RESERVATION_EXPIREDevent is correctly added to the core webhook trigger events constant, maintaining consistency across the codebase.docs/developing/guides/automation/webhooks.mdx (2)
37-37: LGTM! Documentation properly updated for new trigger.The
Reservation Expiredtrigger is correctly added to the list of available webhook triggers in the documentation.
186-186: LGTM! Complete documentation of trigger events.The
RESERVATION_EXPIREDevent is properly added to the comprehensive list of trigger event names in the webhook variable documentation.apps/web/playwright/webhook.e2e.ts (1)
880-893: LGTM! Well-structured E2E test for the new webhook trigger.The test properly verifies that the "Reservation Expired" option is available in the webhook creation UI. The use of
page.getByRole("option", { name: "Reservation Expired" })follows E2E best practices by using semantic role-based locators rather than raw text selectors..github/workflows/cron-reservationExpiry.yml (1)
16-23: Secure implementation of cron API callGood security practices:
- API key is passed via header instead of query parameters
- Proper use of GitHub secrets
- Conditional execution when secrets are available
packages/features/webhooks/lib/sendPayload.ts (2)
78-91: Well-structured payload type for reservation expired eventsThe
ReservationExpiredPayloadTypeincludes all necessary fields for tracking expired reservations. The structure is consistent with other payload types in the system.
209-215: Correct implementation of type guardsThe type guard functions are properly implemented:
isReservationExpiredPayloadcorrectly identifies the payload typeisEventPayloadproperly excludes all non-event payload typesapps/web/app/api/cron/reservationExpiry/route.ts (2)
57-78: Org-wide webhook support correctly implementedThe webhook query properly handles organization-wide webhooks by:
- Getting the org ID from member or team
- Including org webhooks in the OR condition when orgId differs from teamId
144-163: Secure authentication implementationGood security practice using header-based authentication instead of query parameters. This prevents API keys from being exposed in logs, browser history, or referrers.
| # Runs "At every minute." (see https://crontab.guru) | ||
| - cron: "* * * * *" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider reducing cron frequency to avoid excessive resource usage
Running a cron job every minute can put unnecessary load on your infrastructure. Consider running it less frequently (e.g., every 5-15 minutes) unless there's a specific business requirement for minute-level precision.
- # Runs "At every minute." (see https://crontab.guru)
- - cron: "* * * * *"
+ # Runs "At every 5 minutes." (see https://crontab.guru)
+ - cron: "*/5 * * * *"📝 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.
| # Runs "At every minute." (see https://crontab.guru) | |
| - cron: "* * * * *" | |
| # Runs "At every 5 minutes." (see https://crontab.guru) | |
| - cron: "*/5 * * * *" |
🧰 Tools
🪛 actionlint (1.7.7)
8-8: scheduled job runs too frequently. it runs once per 60 seconds. the shortest interval is once every 5 minutes
(events)
🤖 Prompt for AI Agents
In .github/workflows/cron-reservationExpiry.yml at lines 7 to 8, the cron job is
scheduled to run every minute, which may cause excessive resource usage. Modify
the cron expression to run less frequently, such as every 5 or 15 minutes, by
changing the schedule from "* * * * *" to "*/5 * * * *" or "*/15 * * * *"
depending on the acceptable frequency for the task.
| async function handleReservationExpiry() { | ||
| const currentTimeInUtc = dayjs.utc().toDate(); | ||
| let notificationsSent = 0; | ||
| let expiredReservationsProcessed = 0; | ||
|
|
||
| try { | ||
| // Find expired reservations that need to be processed | ||
| const expiredReservations = await prismock.selectedSlots.findMany({ | ||
| where: { | ||
| releaseAt: { lt: currentTimeInUtc }, | ||
| }, | ||
| }); | ||
|
|
||
| for (const reservation of expiredReservations) { | ||
| try { | ||
| // Get related data for webhook payload | ||
| const eventType = await prismock.eventType.findUnique({ | ||
| where: { id: reservation.eventTypeId }, | ||
| select: { | ||
| id: true, | ||
| title: true, | ||
| userId: true, | ||
| teamId: true, | ||
| }, | ||
| }); | ||
|
|
||
| // Get webhooks for this reservation | ||
| const webhooks = await prismock.webhook.findMany({ | ||
| where: { | ||
| active: true, | ||
| eventTriggers: { has: WebhookTriggerEvents.RESERVATION_EXPIRED }, | ||
| OR: [ | ||
| { eventTypeId: reservation.eventTypeId }, | ||
| { userId: eventType?.userId }, | ||
| { teamId: eventType?.teamId }, | ||
| { | ||
| AND: [{ eventTypeId: null }, { userId: null }, { teamId: null }], | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
| if (webhooks.length > 0) { | ||
| notificationsSent += webhooks.length; | ||
| } | ||
|
|
||
| // Clean up the expired reservation | ||
| await prismock.selectedSlots.delete({ | ||
| where: { id: reservation.id }, | ||
| }); | ||
|
|
||
| expiredReservationsProcessed++; | ||
| } catch (error) { | ||
| console.error(`Error processing expired reservation ${reservation.uid}:`, error); | ||
| // Continue processing other reservations even if one fails | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| notificationsSent, | ||
| expiredReservationsProcessed, | ||
| }; | ||
| } catch (error) { | ||
| return { | ||
| error: "Internal server error", | ||
| notificationsSent, | ||
| expiredReservationsProcessed, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Test handler implementation differs from production code
The test's handleReservationExpiry function is missing several features from the production implementation:
- Does not fetch user data (which is needed for webhook payload)
- Does not use
sendGenericWebhookPayload - Has different webhook query logic (includes global webhooks which don't exist)
This could lead to tests passing while production code fails.
🏁 Script executed:
#!/bin/bash
# Compare the test implementation with the production implementation
echo "=== Production implementation ==="
sed -n '13,142p' apps/web/app/api/cron/reservationExpiry/route.ts | head -20
echo -e "\n=== Test implementation ==="
sed -n '14,83p' apps/web/app/api/cron/reservationExpiry/__tests__/cron.test.ts | head -20Length of output: 1535
🏁 Script executed:
#!/bin/bash
echo "=== Production handleReservationExpiry ==="
sed -n '1,200p' apps/web/app/api/cron/reservationExpiry/route.ts
echo -e "\n=== Test file ==="
sed -n '1,200p' apps/web/app/api/cron/reservationExpiry/__tests__/cron.test.tsLength of output: 11587
Align test implementation with the production handler
The test file is redefining handleReservationExpiry instead of importing and exercising the real implementation in route.ts. As a result, key logic—user lookup, org lookup, webhook filtering, and calls to sendGenericWebhookPayload—is never tested against the actual code.
Please update the tests to:
- Import
handleReservationExpiryfromapps/web/app/api/cron/reservationExpiry/route.tsrather than redefining it - Mock and assert calls to
sendGenericWebhookPayload(instead of just counting webhooks) - Include mocks for
prisma.user.findUniqueandgetOrgIdFromMemberOrTeamIdto exercise organization-level webhooks - Align the
webhook.findManyexpectations with the production OR conditions (drop the global fallback that tests currently include) - Verify that errors flow through the
logger(notconsole.error) and that retries/cleanup behave as in production
These changes will ensure your tests accurately reflect and validate the real cron handler logic.
🤖 Prompt for AI Agents
In apps/web/app/api/cron/reservationExpiry/__tests__/cron.test.ts from lines 14
to 83, the test redefines handleReservationExpiry instead of importing it from
the production code in route.ts. To fix this, import handleReservationExpiry
from apps/web/app/api/cron/reservationExpiry/route.ts, mock and assert calls to
sendGenericWebhookPayload rather than just counting webhooks, add mocks for
prisma.user.findUnique and getOrgIdFromMemberOrTeamId to cover
organization-level webhook logic, update webhook.findMany mocks to match the
production OR conditions without the global fallback, and replace console.error
assertions with logger error checks to reflect production error handling and
retry/cleanup behavior accurately.
| expect(result.expiredReservationsProcessed).toBe(1); | ||
| expect(result.notificationsSent).toBe(1); | ||
|
|
||
| // Check that expired reservation was deleted | ||
| const remainingReservations = await prismock.selectedSlots.findMany(); | ||
| expect(remainingReservations).toHaveLength(0); | ||
| }); | ||
|
|
||
| it("should not process future reservations", async () => { | ||
| const currentTime = dayjs.utc(); | ||
| const futureTime = currentTime.add(1, "hour").toDate(); | ||
|
|
||
| // Create test data with future expiry | ||
| await createUser({ id: 101 }); | ||
| await createEventType({ id: 1, userId: 101 }); | ||
| await createSelectedSlot({ | ||
| id: 1, | ||
| eventTypeId: 1, | ||
| userId: 101, | ||
| uid: "future-reservation", | ||
| releaseAt: futureTime, | ||
| }); | ||
| await createWebhook({ | ||
| id: "webhook-1", | ||
| userId: 101, | ||
| eventTypeId: 1, | ||
| }); | ||
|
|
||
| const result = await handleReservationExpiry(); | ||
|
|
||
| expect(result.expiredReservationsProcessed).toBe(0); | ||
| expect(result.notificationsSent).toBe(0); | ||
|
|
||
| // Check that future reservation was not deleted | ||
| const remainingReservations = await prismock.selectedSlots.findMany(); | ||
| expect(remainingReservations).toHaveLength(1); | ||
| }); | ||
|
|
||
| it("should handle team webhooks for expired reservations", async () => { | ||
| const currentTime = dayjs.utc(); | ||
| const expiredTime = currentTime.subtract(1, "hour").toDate(); | ||
|
|
||
| // Create test data with team webhook | ||
| await createUser({ id: 101 }); | ||
| await createEventType({ id: 1, userId: 101, teamId: 1 }); | ||
| await createSelectedSlot({ | ||
| id: 1, | ||
| eventTypeId: 1, | ||
| userId: 101, | ||
| uid: "team-expired-reservation", | ||
| releaseAt: expiredTime, | ||
| }); | ||
| await createWebhook({ | ||
| id: "team-webhook-1", | ||
| teamId: 1, | ||
| }); | ||
|
|
||
| const result = await handleReservationExpiry(); | ||
|
|
||
| expect(result.expiredReservationsProcessed).toBe(1); | ||
| expect(result.notificationsSent).toBe(1); | ||
| }); | ||
|
|
||
| it("should process multiple expired reservations", async () => { | ||
| const currentTime = dayjs.utc(); | ||
| const expiredTime = currentTime.subtract(1, "hour").toDate(); | ||
|
|
||
| // Create test data with multiple expired reservations | ||
| await createUser({ id: 101 }); | ||
| await createEventType({ id: 1, userId: 101 }); | ||
| await createSelectedSlot({ | ||
| id: 1, | ||
| eventTypeId: 1, | ||
| userId: 101, | ||
| uid: "expired-reservation-1", | ||
| releaseAt: expiredTime, | ||
| }); | ||
| await createSelectedSlot({ | ||
| id: 2, | ||
| eventTypeId: 1, | ||
| userId: 101, | ||
| uid: "expired-reservation-2", | ||
| releaseAt: expiredTime, | ||
| }); | ||
| await createWebhook({ | ||
| id: "webhook-1", | ||
| userId: 101, | ||
| eventTypeId: 1, | ||
| }); | ||
|
|
||
| const result = await handleReservationExpiry(); | ||
|
|
||
| expect(result.expiredReservationsProcessed).toBe(2); | ||
| expect(result.notificationsSent).toBe(2); | ||
|
|
||
| // Check that all expired reservations were deleted | ||
| const remainingReservations = await prismock.selectedSlots.findMany(); | ||
| expect(remainingReservations).toHaveLength(0); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider importing and testing the actual production function
Instead of duplicating the implementation in tests, import the actual handleReservationExpiry function from the route file. This ensures tests accurately reflect production behavior.
-// Handler function for processing expired reservations
-async function handleReservationExpiry() {
- // ... duplicated implementation
-}
+import { handleReservationExpiry } from "../route";
+// Mock prisma to use prismock
+vi.mock("@calcom/prisma", () => ({
+ default: prismock,
+}));📝 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.
| describe("Reservation Expiry Cron Job", () => { | |
| beforeEach(() => { | |
| prismock.selectedSlots.deleteMany(); | |
| prismock.webhook.deleteMany(); | |
| prismock.eventType.deleteMany(); | |
| prismock.user.deleteMany(); | |
| }); | |
| it("should return zero counts when no expired reservations exist", async () => { | |
| const result = await handleReservationExpiry(); | |
| expect(result.expiredReservationsProcessed).toBe(0); | |
| expect(result.notificationsSent).toBe(0); | |
| }); | |
| it("should process expired reservations and send webhooks", async () => { | |
| const currentTime = dayjs.utc(); | |
| const expiredTime = currentTime.subtract(1, "hour").toDate(); | |
| // Create test data | |
| await createUser({ id: 101 }); | |
| await createEventType({ id: 1, userId: 101 }); | |
| await createSelectedSlot({ | |
| id: 1, | |
| eventTypeId: 1, | |
| userId: 101, | |
| uid: "expired-reservation-1", | |
| releaseAt: expiredTime, | |
| }); | |
| await createWebhook({ | |
| id: "webhook-1", | |
| userId: 101, | |
| eventTypeId: 1, | |
| }); | |
| const result = await handleReservationExpiry(); | |
| expect(result.expiredReservationsProcessed).toBe(1); | |
| expect(result.notificationsSent).toBe(1); | |
| // Check that expired reservation was deleted | |
| const remainingReservations = await prismock.selectedSlots.findMany(); | |
| expect(remainingReservations).toHaveLength(0); | |
| }); | |
| it("should not process future reservations", async () => { | |
| const currentTime = dayjs.utc(); | |
| const futureTime = currentTime.add(1, "hour").toDate(); | |
| // Create test data with future expiry | |
| await createUser({ id: 101 }); | |
| await createEventType({ id: 1, userId: 101 }); | |
| await createSelectedSlot({ | |
| id: 1, | |
| eventTypeId: 1, | |
| userId: 101, | |
| uid: "future-reservation", | |
| releaseAt: futureTime, | |
| }); | |
| await createWebhook({ | |
| id: "webhook-1", | |
| userId: 101, | |
| eventTypeId: 1, | |
| }); | |
| const result = await handleReservationExpiry(); | |
| expect(result.expiredReservationsProcessed).toBe(0); | |
| expect(result.notificationsSent).toBe(0); | |
| // Check that future reservation was not deleted | |
| const remainingReservations = await prismock.selectedSlots.findMany(); | |
| expect(remainingReservations).toHaveLength(1); | |
| }); | |
| it("should handle team webhooks for expired reservations", async () => { | |
| const currentTime = dayjs.utc(); | |
| const expiredTime = currentTime.subtract(1, "hour").toDate(); | |
| // Create test data with team webhook | |
| await createUser({ id: 101 }); | |
| await createEventType({ id: 1, userId: 101, teamId: 1 }); | |
| await createSelectedSlot({ | |
| id: 1, | |
| eventTypeId: 1, | |
| userId: 101, | |
| uid: "team-expired-reservation", | |
| releaseAt: expiredTime, | |
| }); | |
| await createWebhook({ | |
| id: "team-webhook-1", | |
| teamId: 1, | |
| }); | |
| const result = await handleReservationExpiry(); | |
| expect(result.expiredReservationsProcessed).toBe(1); | |
| expect(result.notificationsSent).toBe(1); | |
| }); | |
| it("should process multiple expired reservations", async () => { | |
| const currentTime = dayjs.utc(); | |
| const expiredTime = currentTime.subtract(1, "hour").toDate(); | |
| // Create test data with multiple expired reservations | |
| await createUser({ id: 101 }); | |
| await createEventType({ id: 1, userId: 101 }); | |
| await createSelectedSlot({ | |
| id: 1, | |
| eventTypeId: 1, | |
| userId: 101, | |
| uid: "expired-reservation-1", | |
| releaseAt: expiredTime, | |
| }); | |
| await createSelectedSlot({ | |
| id: 2, | |
| eventTypeId: 1, | |
| userId: 101, | |
| uid: "expired-reservation-2", | |
| releaseAt: expiredTime, | |
| }); | |
| await createWebhook({ | |
| id: "webhook-1", | |
| userId: 101, | |
| eventTypeId: 1, | |
| }); | |
| const result = await handleReservationExpiry(); | |
| expect(result.expiredReservationsProcessed).toBe(2); | |
| expect(result.notificationsSent).toBe(2); | |
| // Check that all expired reservations were deleted | |
| const remainingReservations = await prismock.selectedSlots.findMany(); | |
| expect(remainingReservations).toHaveLength(0); | |
| }); | |
| }); | |
| // Remove the locally defined handler and instead import the real one | |
| import { describe, it, expect, beforeEach, vi } from "vitest"; | |
| import dayjs from "dayjs"; | |
| import { createUser, createEventType, createSelectedSlot, createWebhook } from "../helpers"; | |
| import prismock from "@calcom/prisma"; | |
| import { handleReservationExpiry } from "../route"; | |
| // Ensure the production code’s prisma import is redirected to our in-memory mock | |
| vi.mock("@calcom/prisma", () => ({ | |
| default: prismock, | |
| })); | |
| describe("Reservation Expiry Cron Job", () => { | |
| beforeEach(() => { | |
| prismock.selectedSlots.deleteMany(); | |
| prismock.webhook.deleteMany(); | |
| prismock.eventType.deleteMany(); | |
| prismock.user.deleteMany(); | |
| }); | |
| it("should return zero counts when no expired reservations exist", async () => { | |
| const result = await handleReservationExpiry(); | |
| expect(result.expiredReservationsProcessed).toBe(0); | |
| expect(result.notificationsSent).toBe(0); | |
| }); | |
| it("should process expired reservations and send webhooks", async () => { | |
| const currentTime = dayjs.utc(); | |
| const expiredTime = currentTime.subtract(1, "hour").toDate(); | |
| // Create test data | |
| await createUser({ id: 101 }); | |
| await createEventType({ id: 1, userId: 101 }); | |
| await createSelectedSlot({ | |
| id: 1, | |
| eventTypeId: 1, | |
| userId: 101, | |
| uid: "expired-reservation-1", | |
| releaseAt: expiredTime, | |
| }); | |
| await createWebhook({ | |
| id: "webhook-1", | |
| userId: 101, | |
| eventTypeId: 1, | |
| }); | |
| const result = await handleReservationExpiry(); | |
| expect(result.expiredReservationsProcessed).toBe(1); | |
| expect(result.notificationsSent).toBe(1); | |
| // Check that expired reservation was deleted | |
| const remainingReservations = await prismock.selectedSlots.findMany(); | |
| expect(remainingReservations).toHaveLength(0); | |
| }); | |
| // …the rest of the test cases remain unchanged… | |
| }); |
🤖 Prompt for AI Agents
In apps/web/app/api/cron/reservationExpiry/__tests__/cron.test.ts around lines
137 to 272, the tests currently duplicate the implementation logic instead of
using the real production function. To fix this, import the actual
handleReservationExpiry function from the production route file at the top of
the test file and replace any duplicated or mock implementations with calls to
this imported function. This will ensure the tests run against the real
production code and better validate its behavior.
What does this PR do?
Mandatory Tasks (DO NOT REMOVE)
Summary by cubic
Added support for tracking expired reservations and sending a RESERVATION_EXPIRED webhook when a reservation expires.