fix(booking-page): hide duplicate phone fields for attendee phone location#29761
fix(booking-page): hide duplicate phone fields for attendee phone location#29761akshitj11 wants to merge 3 commits into
Conversation
…ation When location is attendee phone, sync and hide other phone booking questions so users are not prompted for the same number twice. Fixes calcom#23117
github-actions
Bot
added
booking-page
|
Welcome to Cal.diy, @akshitj11! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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
🧹 Nitpick comments (1)
apps/web/modules/bookings/components/BookEventForm/BookingFields.tsx (1)
56-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the two phone-sync paths to avoid behavioral divergence.
There are now two sync mechanisms:
syncPhoneFields(event-driven, with touched checks and dedup) and the render-time block (no touched checks, no dedup). This DRY violation means the two paths can silently diverge. Consolidate into a single function called from both theonValueChangehandler and auseEffectwatchinglocationResponse.♻️ Proposed consolidation
+ const syncPhoneFromLocation = useCallback( + (locationValue: unknown) => { + const parsed = PhoneLocationSchema.safeParse(locationValue); + if (!parsed.success) return; + const phone = (parsed.data.optionValue ?? "").trim(); + if (!phone || phone === lastSyncedPhoneRef.current) return; + + otherPhoneFieldNames.forEach((name) => { + const targetTouched = !!(formState.touchedFields as TouchedFields)?.responses?.[name]; + if (!targetTouched) { + setValue(`responses.${name}`, phone, { + shouldDirty: false, + shouldValidate: false, + }); + } + }); + + lastSyncedPhoneRef.current = phone; + }, + [otherPhoneFieldNames, formState.touchedFields, setValue] + ); + + useEffect(() => { + syncPhoneFromLocation(locationResponse); + }, [locationResponse, syncPhoneFromLocation]);Then use
syncPhoneFromLocationin theonValueChangehandler and remove the inlinesetValuefrom the render block.Also applies to: 154-168
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/bookings/components/BookEventForm/BookingFields.tsx` around lines 56 - 78, Consolidate the phone synchronization logic into a single function named syncPhoneFromLocation, preserving the existing parsing, deduplication, and touched-field checks. Call this function from both the location onValueChange handler and the useEffect watching locationResponse, and remove the render-time inline setValue synchronization block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/modules/bookings/components/BookEventForm/BookingFields.tsx`:
- Around line 161-165: Update the render-time phone field synchronization near
setValue to respect formState.touchedFields, matching syncPhoneFields behavior.
Only set responses.${field.name} when the field has not been touched, while
preserving the existing non-empty phone condition and setValue options.
- Around line 154-168: Move the phone response synchronization out of the
fields.map render path in BookingFields and into a useEffect that watches
locationResponse (and the relevant field data). Preserve the existing
conditions, trimmed phone value, setValue options, and return null behavior so
the duplicated phone field remains hidden during render.
---
Nitpick comments:
In `@apps/web/modules/bookings/components/BookEventForm/BookingFields.tsx`:
- Around line 56-78: Consolidate the phone synchronization logic into a single
function named syncPhoneFromLocation, preserving the existing parsing,
deduplication, and touched-field checks. Call this function from both the
location onValueChange handler and the useEffect watching locationResponse, and
remove the render-time inline setValue synchronization block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5c86aca-8537-437c-adfb-b9067e869ca9
📒 Files selected for processing (1)
apps/web/modules/bookings/components/BookEventForm/BookingFields.tsx
Extract shouldHideDuplicatePhoneField helper and cover attendee-phone location cases so duplicate phone inputs stay hidden on the booking form.
Consolidate phone field sync into syncPhoneFromLocation with touched-field checks and hide duplicate phone fields during render only.
|
CodeRabbit fixes pushed. Please add the |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
CodeRabbit feedback addressed in commits
Please add the |
|
|
2 participants
What does this PR do?
When an event type uses Attendee phone as the location and also has additional phone booking questions, the booking page showed duplicate phone inputs — the same data point collected twice.
This PR hides duplicate phone fields when location is set to attendee phone and syncs the phone value from the location field into other phone responses before submit. Logic is extracted to
shouldHideDuplicatePhoneFieldwith unit tests.apps/web/modules/bookings/components/BookEventForm/BookingFields.tsxphoneFieldUtils.tsandphoneFieldUtils.test.tsVisual 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):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
yarn vitest run apps/web/modules/bookings/components/BookEventForm/phoneFieldUtils.test.ts(5 tests pass).