-
Notifications
You must be signed in to change notification settings - Fork 10.4k
test: add no show webhooks integration tests #21697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: add no show webhooks integration tests #21697
Conversation
@kart1ka 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/04/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (06/04/25)1 label was added to this PR based on Keith Williams's automation. |
E2E results are ready! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Left a question for you
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
Outdated
Show resolved
Hide resolved
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
Outdated
Show resolved
Hide resolved
This PR is being marked as stale due to inactivity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cubic reviewed 1 file and found no issues. Review PR in cubic.dev.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are going through the test requirements: -
We need to test the following scenarios:
- When scheduling a webhook expect the correct Tasker job to be created
- On another test, make sure the task payload is correct
- Given a Task, ensure the task handler runs properly with the correct payload
- Make sure a Task is deleted when the webhook is deleted
Please make sure to fix the test name and make it specific to these scenarios only.
Also, is this the correct place to add the integration test? Does this test file run when executing: yarn test -- --integrationTestsOnly?
I would suggest taking some help from Devin, if you found any difficulties. Devin is so good with test
Pull request was converted to draft
I have changed the names of the tests as per the request. And Yes, the test file runs when executing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cubic found 3 issues across 1 file. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai
to give specific feedback.
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
Outdated
Show resolved
Hide resolved
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
Outdated
Show resolved
Hide resolved
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
Outdated
Show resolved
Hide resolved
@kart1ka pls address the cubic reviews. Also where is this test:- |
@anikdhabal I have addressed the cubic comments. I haven’t added the test for checking if a Task is deleted when a webhook is removed, since that logic hasn’t been implemented yet. I’ve opened a separate PR to fix it. Ref - #21345. I have mentioned the same in the description as well. |
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA new integration test suite was introduced in Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts
: For Prisma queries, only select data you need; never useinclude
, always useselect
Ensure thecredential.key
field is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()
in hot paths like loops
Files:
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
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: vijayraghav-io
PR: calcom/cal.com#21072
File: .env.example:354-357
Timestamp: 2025-07-18T08:49:18.779Z
Learning: For E2E integration tests that require real third-party service credentials (like Outlook calendar), it's acceptable to temporarily include actual test account credentials in .env.example during feature development and validation, provided there's a clear plan to replace them with placeholder values before final release. Test credentials should be for dedicated test tenants/accounts, not production systems.
📚 Learning: the office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is spec...
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
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.
Applied to files:
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
📚 Learning: applies to **/*.ts : for prisma queries, only select data you need; never use `include`, always use ...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`
Applied to files:
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
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.
Applied to files:
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
📚 Learning: in the insights routing funnel component (packages/features/insights/components/routingfunnel.tsx), ...
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.539Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
Applied to files:
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
⏰ 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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
const updatedBooking = await prisma.booking.findUnique({ | ||
where: { | ||
id: testBookingId, | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use select
to fetch only required fields
According to our Prisma query guidelines, always use select
to fetch only the data you need. Since you only need the noShowHost
field for the assertion, specify it explicitly.
const updatedBooking = await prisma.booking.findUnique({
where: {
id: testBookingId,
},
+ select: {
+ noShowHost: true,
+ },
});
📝 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 updatedBooking = await prisma.booking.findUnique({ | |
where: { | |
id: testBookingId, | |
}, | |
}); | |
const updatedBooking = await prisma.booking.findUnique({ | |
where: { | |
id: testBookingId, | |
}, | |
select: { | |
noShowHost: true, | |
}, | |
}); |
🤖 Prompt for AI Agents
In
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
around lines 294 to 298, the Prisma query fetches the entire booking record but
only the noShowHost field is needed. Modify the findUnique call to include a
select clause that explicitly requests only the noShowHost field to optimize the
query and follow Prisma guidelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, thank you for your contribution @kart1ka - appreciated as always.
What does this PR do?
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
yarn test packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.integration-test.ts
Summary by cubic
Added integration tests for no-show webhooks to verify task creation, payloads, and webhook triggering logic. These tests ensure correct handling of host and guest no-show scenarios.