fix: skip email validation for hidden field#24382
fix: skip email validation for hidden field#24382Saumya-Kushwah wants to merge 3 commits intocalcom:mainfrom Saumya-Kushwah:fix/email-validation-hidden-field
Conversation
|
|
github-actions
bot
added
booking-page
|
@Saumya-Kushwah 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 |
dosubot
bot
added
the
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/features/bookings/lib/getBookingResponsesSchema.ts (2)
98-107: Harden JSON parsing by validating the parsed shapeBare catch is fine, but if JSON.parse(value) returns a non-object (e.g., a string), parsedValue becomes a primitive and later property access can misbehave. Guard the shape post-parse to keep the default when it’s not the expected object.
- let parsedValue = { - optionValue: "", - value: "", - }; - try { - parsedValue = JSON.parse(value); - } catch { - // Intentionally ignoring parse errors - using default empty parsedValue - } + let parsedValue: { optionValue: string; value: string } = { + optionValue: "", + value: "", + }; + try { + const candidate = JSON.parse(value); + if ( + candidate && + typeof candidate === "object" && + typeof (candidate as any).value === "string" && + typeof (candidate as any).optionValue === "string" + ) { + parsedValue = candidate as { optionValue: string; value: string }; + } + } catch { + // Intentionally ignoring parse errors - using default empty parsedValue + }
178-182: Skip email validation should use computed hidden and view applicabilityGood fix. For consistency and to cover hideWhenJustOneOption and view mismatch, use the computed hidden and isFieldApplicableToCurrentView flags you already derive above.
- // Skip validation if field is hidden - if (bookingField.hidden) { + // Skip validation if field is hidden or not applicable to the current view + if (hidden || !isFieldApplicableToCurrentView) { continue; }Optional: consider applying the same early-skip for other field types (e.g., multiemail) so hidden fields never trigger validation noise when prefilled. Please confirm if product spec expects hidden fields to be fully ignored during validation.
📜 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 (2)
packages/features/bookings/lib/getBookingResponsesSchema.ts(3 hunks)packages/prisma/.env(0 hunks)
💤 Files with no reviewable changes (1)
- packages/prisma/.env
🧰 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
🧠 Learnings (1)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
🧬 Code graph analysis (1)
packages/features/bookings/lib/getBookingResponsesSchema.ts (1)
packages/features/form-builder/schema.ts (1)
ALL_VIEWS(11-11)
⏰ 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: Tests / Unit
🔇 Additional comments (1)
packages/features/bookings/lib/getBookingResponsesSchema.ts (1)
9-9: Type alias simplification looks goodChanging View to ALL_VIEWS | string is fine; no functional impact.
2 participants
What does this PR do?
This PR fixes a regression where email validation was incorrectly running on hidden email fields, preventing users from completing bookings even when the email was prefilled
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):
Timeline.1.mov
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Setup:
Test Execution:
Result:
{email}email_validation_errorin console