fix: removed condition for hidden emails to be checked even if it is required#24431
fix: removed condition for hidden emails to be checked even if it is required#24431Spandan-Mishra wants to merge 1 commit intocalcom:mainfrom
Conversation
|
@Spandan-Mishra is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough
Possibly related PRs
Pre-merge checks and finishing touches❌ 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/bookings/lib/getBookingResponsesSchema.ts (1)
105-107: Consider structured logging or log level adjustment.The error logging helps with debugging, but since execution continues safely with default values, consider whether this should be a warning instead of an error, or add more context about the field name to aid debugging.
Example adjustment:
} catch (error) { - console.error("Failed to parse radioInput value as JSON:", error); + console.warn(`Failed to parse radioInput value for field "${field.name}":`, error); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/lib/getBookingResponsesSchema.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/getBookingResponsesSchema.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/getBookingResponsesSchema.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/getBookingResponsesSchema.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
packages/features/bookings/lib/getBookingResponsesSchema.ts (1)
178-178: LGTM! The simplified condition correctly prevents validation of hidden email fields.The change from the previous ternary condition to the explicit check
!bookingField.hidden && checkOptionaldirectly addresses the reported issue. By gating email validation on both the field not being hidden and thecheckOptionalflag, hidden email fields will no longer trigger validation errors during booking confirmation, even when prefilled.This is consistent with the phone field validation pattern at line 284 and aligns with the comment at line 163: "If the field is hidden, then it can never be required."
Please verify this resolves the issue by following the testing steps from the PR description:
- Create an event type with its email field hidden
- Complete the booking page with required information
- Confirm booking succeeds without console errors about
email_validation_errorAdditionally, verify that when the email field is visible and has validation rules (excluded/required emails), those validations still work correctly.
|
@Spandan-Mishra i've fixed it here:- #24432 |
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Before:
2025-10-13.18-48-50.mp4
After:
2025-10-13.20-15-15.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?