feat: Add email input dialog to /booking/uid page#22542
feat: Add email input dialog to /booking/uid page#22542iharsh02 wants to merge 48 commits intocalcom:mainfrom
Conversation
|
@iharsh02 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the """ WalkthroughThe changes introduce an email verification step before allowing booking cancellation or viewing booking details when the current user is not authenticated and no valid email parameter is present in the URL. The Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes ✨ 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. Comment |
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
github-actions
bot
added
Medium priority
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/16/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (07/16/25)1 label was added to this PR based on Keith Williams's automation. |
![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
🔭 Outside diff range comments (1)
apps/web/modules/bookings/views/bookings-single-view.tsx (1)
865-872: Avoid unsafe cast & ensure fallbacks for optional fields
bookingInfo?.user?.emailis optional. Casting it tostringhides theundefinedcase at compile-time but the runtime value can still beundefined, which will break the new e-mail-verification flow insideCancelBooking.
Likewise,bookingInfo?.attendeescan beundefined, yetCancelBookingpresumably iterates over it.- attendees: bookingInfo?.attendees, - userEmail: bookingInfo?.user?.email as string, + attendees: bookingInfo.attendees ?? [], + userEmail: + bookingInfo?.user?.email ?? + bookingInfo?.userPrimaryEmail ?? + "", // empty string handled inside CancelBookingAdjust the
CancelBookingprop types if they currently mark these fields as required.
This prevents runtime errors and keeps the types honest.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/components/booking/CancelBooking.tsx(6 hunks)apps/web/modules/bookings/views/bookings-single-view.tsx(1 hunks)apps/web/playwright/booking-seats.e2e.ts(2 hunks)apps/web/public/static/locales/en/common.json(1 hunks)packages/features/schedules/components/Schedule.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- packages/features/schedules/components/Schedule.tsx
🧰 Additional context used
🧠 Learnings (3)
apps/web/modules/bookings/views/bookings-single-view.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#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)
undefined
<retrieved_learning>
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.
</retrieved_learning>
apps/web/components/booking/CancelBooking.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#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.
🧬 Code Graph Analysis (1)
apps/web/components/booking/CancelBooking.tsx (1)
packages/ui/components/form/index.ts (1)
Input(21-21)
⏰ 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). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Security Check
🔇 Additional comments (5)
apps/web/public/static/locales/en/common.json (1)
2504-2506: Colon alignment & style inconsistency—remove leading spaces before:Across this i18n file, key-value pairs are written without a space before the colon.
The two new entriesproceed_with_cancellation_descriptionandproceed_with_cancellation_errorintroduce a leading space before the colon, breaking the prevailing style and creating noisy diffs later.-"proceed_with_cancellation_description" : "Please enter the email address used for this booking to proceed with cancellation", -"proceed_with_cancellation_error" : "Email doesn't match the booking email", +"proceed_with_cancellation_description": "Please enter the email address used for this booking to proceed with cancellation", +"proceed_with_cancellation_error": "Email doesn't match the booking email",[ suggest_nitpick ]
apps/web/playwright/booking-seats.e2e.ts (1)
127-170: LGTM! Test correctly implements the new email verification flow.The test properly handles the email verification requirement by:
- Destructuring the
userobject to access the email- Following the correct sequence: confirm → verify email → submit
This aligns with the new
CancelBookingcomponent behavior.apps/web/components/booking/CancelBooking.tsx (3)
12-13: Imports and type updates look good.The Dialog components and Input are correctly imported, and the booking type additions support the email verification functionality.
Also applies to: 80-81
124-126: Well-structured refactoring with proper state management.The extraction of cancellation logic into
handleCancellationimproves code organization. TheverifyAndCancelfunction correctly implements conditional verification based on whethercurrentUserEmailis available (logged-in users skip verification).Also applies to: 137-183
286-324: Well-implemented verification dialog.The Dialog component is properly structured with:
- Appropriate test IDs that match the e2e tests
- Clear error handling and display
- Proper state cleanup on cancel
- Accessible form inputs
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
…erification feature Co-Authored-By: unknown <>
Devin AI is completing this stale PRThis PR by @iharsh02 has been marked as stale. A Devin session has been created to complete the remaining work. Devin will review the PR, address any feedback, and push updates to complete this PR. |
- Add verifyBookingEmail tRPC endpoint for server-side email validation - Replace client-side email validation with server-side validation - Remove async/await from updateSearchParams as per review feedback - Address review comment by @kart1ka for server-side validation Co-authored-by: iharsh02 <iharsh02@users.noreply.github.com> Co-Authored-By: unknown <>
Summary of ChangesI've completed the remaining work on this PR to address the review feedback from @kart1ka. Key ChangesServer-side email validation (addressing the main review comment):
Minor fixes (addressing other review comments):
Files Added
Files Modified
The PR is now ready for another review. The email verification dialog still works as before, but now the validation happens securely on the server side. Completed by Devin on behalf of @iharsh02 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 8 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/components/booking/CancelBooking.tsx">
<violation number="1" location="apps/web/components/booking/CancelBooking.tsx:182">
P2: handleCancellation lacks try/catch or res.ok guard, so fetch/JSON errors leave loading stuck true and throw unhandled rejections</violation>
</file>
<file name="packages/trpc/server/routers/publicViewer/verifyBookingEmail.schema.ts">
<violation number="1" location="packages/trpc/server/routers/publicViewer/verifyBookingEmail.schema.ts:10">
P2: Email input is validated without trimming; pasted emails with whitespace will be rejected as invalid</violation>
</file>
<file name="apps/web/modules/bookings/views/bookings-single-view.tsx">
<violation number="1" location="apps/web/modules/bookings/views/bookings-single-view.tsx:531">
P1: Email verification state persists across email changes; unverified emails can view booking details after one successful verify.</violation>
<violation number="2" location="apps/web/modules/bookings/views/bookings-single-view.tsx:565">
P1: Email-gated booking details are only hidden in the UI; SSR still delivers full booking data to unauthenticated users</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| eventType={{ ...eventType, metadata: eventTypeMetaDataSchemaWithTypedApps.parse(eventType.metadata) }} | ||
| /> | ||
| <main className={classNames(shouldAlignCentrally ? "mx-auto" : "", isEmbed ? "" : "max-w-3xl")}> | ||
| {showBookingInfo ? ( |
There was a problem hiding this comment.
P1: Email-gated booking details are only hidden in the UI; SSR still delivers full booking data to unauthenticated users
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/modules/bookings/views/bookings-single-view.tsx, line 565:
<comment>Email-gated booking details are only hidden in the UI; SSR still delivers full booking data to unauthenticated users</comment>
<file context>
@@ -478,7 +562,8 @@ export default function Success(props: PageProps) {
eventType={{ ...eventType, metadata: eventTypeMetaDataSchemaWithTypedApps.parse(eventType.metadata) }}
/>
- <main className={classNames(shouldAlignCentrally ? "mx-auto" : "", isEmbed ? "" : "max-w-3xl")}>
+ {showBookingInfo ? (
+ <main className={classNames(shouldAlignCentrally ? "mx-auto" : "", isEmbed ? "" : "max-w-3xl")}>
<div className={classNames("overflow-y-auto", isEmbed ? "" : "z-50 ")}>
</file context>
packages/trpc/server/routers/publicViewer/verifyBookingEmail.schema.ts
Outdated
Show resolved
Hide resolved
- Add findByUidIncludeUserEmailAndAttendeeEmails method to BookingRepository - Update verifyBookingEmail handler to use repository instead of prisma directly - Follow Cal.com's repository pattern for data access Co-Authored-By: unknown <>
Devin AI is completing this stale PRThis PR by @iharsh02 has been marked as stale. A Devin session has been created to complete the remaining work. Devin will review the PR, address any feedback, and push updates to complete this PR. |
- Add .trim() to email validation schema to handle pasted emails with whitespace - Add try/catch error handling to handleCancellation in CancelBooking.tsx - Reset isEmailVerified state when email param changes to prevent state persistence bug - Add loading state to verify button for better UX Co-authored-by: iharsh02 <iharsh02@users.noreply.github.com> Co-Authored-By: unknown <>
Summary of Completed WorkI've addressed the review feedback from @cubic-dev-ai and completed the remaining work on this PR. Here's what was done: Fixes Applied
Note on P1 SSR IssueThe SSR data exposure issue mentioned in the review (booking details being delivered via SSR to unauthenticated users) would require server-side changes to Testing
Co-authored-by: @iharsh02 (original PR author) |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/modules/bookings/views/bookings-single-view.tsx">
<violation number="1" location="apps/web/modules/bookings/views/bookings-single-view.tsx:485">
P2: Manual email verification is immediately cleared by the new `emailParam !== lastVerifiedEmail` effect because `lastVerifiedEmail` is never updated in the manual success path, causing booking info to disappear until revalidation succeeds.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }, [emailVerificationResult, emailParam]); | ||
|
|
||
| useEffect(() => { | ||
| if (emailParam !== lastVerifiedEmail) { |
There was a problem hiding this comment.
P2: Manual email verification is immediately cleared by the new emailParam !== lastVerifiedEmail effect because lastVerifiedEmail is never updated in the manual success path, causing booking info to disappear until revalidation succeeds.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/modules/bookings/views/bookings-single-view.tsx, line 485:
<comment>Manual email verification is immediately cleared by the new `emailParam !== lastVerifiedEmail` effect because `lastVerifiedEmail` is never updated in the manual success path, causing booking info to disappear until revalidation succeeds.</comment>
<file context>
@@ -474,10 +475,17 @@ export default function Success(props: PageProps) {
+ }, [emailVerificationResult, emailParam]);
+
+ useEffect(() => {
+ if (emailParam !== lastVerifiedEmail) {
+ setIsEmailVerified(false);
+ }
</file context>
8 participants
What does this PR do?
Follow-up PR - #18770
This PR adds an email verification dialog to the
/booking/uidpage that:cancelledByis always set when cancelling a bookingUpdates since last revision
Addressed review feedback from @kart1ka:
verifyBookingEmailtRPC endpoint that validates emails against the database instead of client-side validationasyncfromupdateSearchParamsfunctionawaitfromrouter.replacecallLatest update: Moved prisma query to proper repository pattern:
findByUidIncludeUserEmailAndAttendeeEmailsmethod toBookingRepositoryFiles changed:
packages/trpc/server/routers/publicViewer/verifyBookingEmail.schema.ts(new)packages/trpc/server/routers/publicViewer/verifyBookingEmail.handler.ts(new)packages/features/bookings/repositories/BookingRepository.ts(modified)Visual Demo (For contributors especially)
cal-5045.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
/booking/{uid}directlycancelledByis set correctlyHuman Review Checklist
findByUidIncludeUserEmailAndAttendeeEmailsrepository method returns correct dataverifyBookingEmailtRPC endpoint correctly validates emails against booking attendees and hostChecklist
Link to Devin run: https://app.devin.ai/sessions/733eb255221f498981f687837f05b1bc
Requested by: unknown ()