Skip to content

Conversation

@hemantmm
Copy link
Contributor

@hemantmm hemantmm commented Sep 10, 2025

closes: #23722

What does this PR do?

This PR consolidates phone input handling by using a single attendeePhoneNumber field instead of separate fields for SMS reminder number and AI agent call phone number. Users will now see only one phone input field on booking forms.

Key Changes:

  • Skip rendering legacy phone fields (smsReminderNumber, aiAgentCallPhoneNumber) in BookingFields.tsx
  • Consolidate phone number sources in getBookingFields.ts to surface a single attendeePhoneNumber field
  • Preserve user's existing required setting when updating the attendeePhoneNumber field

Visual Demo (For contributors especially)

Video Demo (if applicable):

Before:

Screen.Recording.2025-09-10.at.10.50.54.mov

After:

Screen.Recording.2025-09-10.at.10.47.21.mov

Updates since last revision

Completed by Devin to address review feedback and resolve merge conflicts:

  1. Merged with upstream/main - Resolved conflicts after .eslintrc.js was deleted and BookingFields.tsx was moved to apps/web/modules/bookings/components/BookEventForm/
  2. Fixed required property issue - Changed required: isPhoneNumberRequired to required: bookingFields[attendeePhoneNumberIndex].required || isPhoneNumberRequired to preserve user's existing configuration (addresses cubic-dev-ai review feedback)
  3. Applied lint fixes - Import organization cleanup

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  1. Create an event type with a workflow that requires SMS notifications or AI phone calls
  2. Navigate to the booking page for that event type
  3. Verify only one phone number field (attendeePhoneNumber) is displayed instead of multiple phone fields
  4. If the user had previously set the phone field as required, verify it remains required

Human Review Checklist

  • Verify the skip logic for legacy phone fields in apps/web/modules/bookings/components/BookEventForm/BookingFields.tsx is correct
  • Verify the required property preservation logic in packages/features/bookings/lib/getBookingFields.ts works as expected
  • Test with workflows that use SMS/WhatsApp attendee notifications and AI phone calls

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

Link to Devin run: https://app.devin.ai/sessions/598091a8c87e4189b79221e96219f8f9
Requested by: unknown ()
Original author: @hemantmm

@hemantmm hemantmm requested a review from a team as a code owner September 10, 2025 07:01
@vercel
Copy link

vercel bot commented Sep 10, 2025

@hemantmm is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 10, 2025

Walkthrough

Removed react-hook-form context usage from BookingFields.tsx and added a guard to skip legacy phone fields (smsReminderNumber, aiAgentCallPhoneNumber). getBookingFields.ts consolidates phone handling by introducing phoneNumberSources and isPhoneNumberRequired, surfacing a single attendeePhoneNumber and merging legacy SMS/AI phone fields. Two exported functions (getBookingFieldsWithSystemFields, ensureBookingInputsHaveSystemFields) had isOrgTeamEvent removed from their parameter lists. ee/workflows/lib/reminders/aiPhoneCallManager.ts no longer reads CAL_AI_AGENT_PHONE_NUMBER_FIELD. .eslintrc.js adds a no-restricted-imports rule blocking tRPC imports in packages/app-store.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The PR includes changes that appear unrelated to the linked issues, notably an .eslintrc.js rule adding a tRPC import restriction and removal of the isOrgTeamEvent parameter from getBookingFields/ensureBookingInputsHaveSystemFields signatures; these are not part of the "one phone input" objective and may be considered out-of-scope or breaking public API consumers. Remove or move the lint-rule change and the API-signature modifications into separate PRs or explicitly document and justify them here, update any callers before merging, and run integration tests to confirm there are no breaking effects.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title "fix: phone-input" is concise and directly describes the primary change to phone input behavior in this changeset, matching the PR's intent to consolidate phone fields; it is short, specific, and appropriate for history scanning.
Linked Issues Check ✅ Passed The code changes implement a unified phone-input approach: getBookingFields consolidates phoneNumberSources and surfaces a single attendeePhoneNumber, BookingFields.tsx guards/omits legacy sms/AI fields, and aiPhoneCallManager now relies on attendeePhoneNumber; these changes directly address the linked issues' goal of showing one phone input across workflows and booking questions.
Description Check ✅ Passed The PR description clearly links the change to fixing phone-input behavior, references the linked issues it closes, and includes visual demos, so it is related to the changeset despite the testing section containing placeholders.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@graphite-app graphite-app bot requested a review from a team September 10, 2025 07:01
@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Sep 10, 2025
@github-actions github-actions bot added High priority Created by Linear-GitHub Sync 🧹 Improvements Improvements to existing features. Mostly UX/UI labels Sep 10, 2025
@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Sep 10, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx (1)

36-41: Fix reschedule readOnly guard: undefined treated as “has bookingData”.

bookingData !== null evaluates true when bookingData is undefined, unintentionally locking fields read-only. Use a nullish check.

Apply:

-          bookingData !== null;
+          bookingData != null;
🧹 Nitpick comments (2)
packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx (1)

46-49: Minor: simplify view gating.

Inline and use .some for clarity.

-        const fieldViews = field.views;
-        if (fieldViews && !fieldViews.find((view) => view.id === currentView)) {
+        if (field.views && !field.views.some((view) => view.id === currentView)) {
           return null;
         }
packages/features/bookings/lib/getBookingFields.ts (1)

356-372: Make attendeePhoneNumber visible and merge sources — de-dupe to avoid duplicates.

Multiple steps in the same workflow can duplicate the same source id.

-        sources: [...(bookingFields[attendeePhoneNumberIndex].sources || []), ...phoneNumberSources],
+        sources: Array.from(
+          new Map(
+            [...(bookingFields[attendeePhoneNumberIndex].sources || []), ...phoneNumberSources].map((s) => [s.id, s])
+          ).values()
+        ),

I can add a unit test that asserts unique source ids on attendeePhoneNumber.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2cc8b1c and 6e520ad.

📒 Files selected for processing (3)
  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx (2 hunks)
  • packages/features/bookings/lib/getBookingFields.ts (2 hunks)
  • packages/features/ee/workflows/lib/reminders/aiPhoneCallManager.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • packages/features/ee/workflows/lib/reminders/aiPhoneCallManager.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx
**/*.{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/Booker/components/BookEventForm/BookingFields.tsx
  • packages/features/bookings/lib/getBookingFields.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/Booker/components/BookEventForm/BookingFields.tsx
  • packages/features/bookings/lib/getBookingFields.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/bookings/lib/getBookingFields.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/workflows/update.handler.ts:738-763
Timestamp: 2025-08-15T00:07:30.058Z
Learning: In calcom/cal.com workflows, Cal AI phone call actions (CAL_AI_PHONE_CALL) intentionally always require the phone number field when the action is present, unlike SMS/WhatsApp actions which respect the step.numberRequired flag. This is the intended behavior per maintainer Udit-takkar in PR #22995.
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.
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
📚 Learning: 2025-08-27T13:32:46.887Z
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.

Applied to files:

  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx
  • packages/features/bookings/lib/getBookingFields.ts
📚 Learning: 2025-07-15T12:59:34.389Z
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/Booker/components/BookEventForm/BookingFields.tsx
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.

Applied to files:

  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx
📚 Learning: 2025-08-29T22:57:30.382Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/form-builder/FormBuilder.tsx:11-11
Timestamp: 2025-08-29T22:57:30.382Z
Learning: FormBuilder.tsx in packages/features/form-builder/ does not have "use client" directive at the top despite using client-side React hooks and event handlers, which suggests it should be a client component.

Applied to files:

  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).

Applied to files:

  • packages/features/bookings/lib/getBookingFields.ts
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.

Applied to files:

  • packages/features/bookings/lib/getBookingFields.ts
📚 Learning: 2025-08-15T00:07:30.058Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/workflows/update.handler.ts:738-763
Timestamp: 2025-08-15T00:07:30.058Z
Learning: In calcom/cal.com workflows, Cal AI phone call actions (CAL_AI_PHONE_CALL) intentionally always require the phone number field when the action is present, unlike SMS/WhatsApp actions which respect the step.numberRequired flag. This is the intended behavior per maintainer Udit-takkar in PR #22995.

Applied to files:

  • packages/features/bookings/lib/getBookingFields.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/getBookingFields.ts (1)
packages/features/bookings/lib/SystemField.ts (2)
  • SMS_REMINDER_NUMBER_FIELD (16-16)
  • CAL_AI_AGENT_PHONE_NUMBER_FIELD (17-17)
⏰ 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). (2)
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx (2)

55-56: rescheduleReason stays editable — good.

Matches intent to allow editing reason during reschedule while other system fields remain read-only.


58-63: Enums verified as canonical
SystemField.Enum.smsReminderNumber and SystemField.Enum.aiAgentCallPhoneNumber match the constants in packages/features/bookings/lib/SystemField.ts.

packages/features/bookings/lib/getBookingFields.ts (2)

148-150: Phone sources accumulator — ok.

Initialization and flag use look good.


374-396: Surface attendeePhoneNumber when legacy fields exist — good.

Ensures single visible phone input in mixed/legacy configs.

Comment on lines +165 to +176
// Check for AI agent call workflows that require phone numbers
if (step.action === "CAL_AI_PHONE_CALL") {
const workflowId = workflow.workflow.id;
phoneNumberSources.push(
getAIAgentCallPhoneNumberSource({
workflowId,
isAIAgentCallPhoneNumberRequired: !!step.numberRequired,
})
);
if (step.numberRequired) {
isPhoneNumberRequired = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

CAL_AI_PHONE_CALL must always require a phone number.

Per workflows behavior, AI phone calls require phone regardless of step.numberRequired. Current code makes it conditional.

Apply:

       if (step.action === "CAL_AI_PHONE_CALL") {
         const workflowId = workflow.workflow.id;
         phoneNumberSources.push(
           getAIAgentCallPhoneNumberSource({
             workflowId,
-            isAIAgentCallPhoneNumberRequired: !!step.numberRequired,
+            isAIAgentCallPhoneNumberRequired: true,
           })
         );
-        if (step.numberRequired) {
-          isPhoneNumberRequired = true;
-        }
+        isPhoneNumberRequired = 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.

Suggested change
// Check for AI agent call workflows that require phone numbers
if (step.action === "CAL_AI_PHONE_CALL") {
const workflowId = workflow.workflow.id;
phoneNumberSources.push(
getAIAgentCallPhoneNumberSource({
workflowId,
isAIAgentCallPhoneNumberRequired: !!step.numberRequired,
})
);
if (step.numberRequired) {
isPhoneNumberRequired = true;
}
// Check for AI agent call workflows that require phone numbers
if (step.action === "CAL_AI_PHONE_CALL") {
const workflowId = workflow.workflow.id;
phoneNumberSources.push(
getAIAgentCallPhoneNumberSource({
workflowId,
isAIAgentCallPhoneNumberRequired: true,
})
);
isPhoneNumberRequired = true;
}
🤖 Prompt for AI Agents
In packages/features/bookings/lib/getBookingFields.ts around lines 165 to 176,
the code treats CAL_AI_PHONE_CALL as requiring a phone only when
step.numberRequired is true; change it so AI phone call steps always require a
phone: when step.action === "CAL_AI_PHONE_CALL" always push
getAIAgentCallPhoneNumberSource with isAIAgentCallPhoneNumberRequired set to
true (or !!true) and set isPhoneNumberRequired = true unconditionally (remove
the conditional check on step.numberRequired).

Copy link
Contributor

@kart1ka kart1ka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments. We are going to need tests to verify these changes.

@kart1ka kart1ka marked this pull request as draft September 15, 2025 04:23
@hemantmm hemantmm marked this pull request as ready for review September 17, 2025 08:26
@hemantmm hemantmm requested a review from kart1ka September 17, 2025 17:06
@hemantmm
Copy link
Contributor Author

@kart1ka, can you have a look at this PR?

Copy link
Contributor

@Devanshusharma2005 Devanshusharma2005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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: 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 3261a79 and a5470e8.

📒 Files selected for processing (1)
  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx
**/*.{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/Booker/components/BookEventForm/BookingFields.tsx
**/*.{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/Booker/components/BookEventForm/BookingFields.tsx
🧠 Learnings (4)
📓 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.
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
📚 Learning: 2025-08-27T13:32:46.887Z
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.

Applied to files:

  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx
📚 Learning: 2025-07-15T12:59:34.389Z
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/Booker/components/BookEventForm/BookingFields.tsx
📚 Learning: 2025-08-29T22:57:30.382Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/form-builder/FormBuilder.tsx:11-11
Timestamp: 2025-08-29T22:57:30.382Z
Learning: FormBuilder.tsx in packages/features/form-builder/ does not have "use client" directive at the top despite using client-side React hooks and event handlers, which suggests it should be a client component.

Applied to files:

  • packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx
🔇 Additional comments (1)
packages/features/bookings/Booker/components/BookEventForm/BookingFields.tsx (1)

100-100: LGTM on field views check.

Straightforward alias; behavior unchanged.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2025

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Oct 9, 2025
@hemantmm hemantmm requested a review from Udit-takkar November 3, 2025 12:28
@hemantmm
Copy link
Contributor Author

hemantmm commented Nov 3, 2025

@anikdhabal, can you review this PR?

Copy link
Member

@Pallava-Joshi Pallava-Joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey can you please fix the merge conflicts here. marking it draft until.

@Pallava-Joshi Pallava-Joshi marked this pull request as draft December 5, 2025 16:49
@Pallava-Joshi
Copy link
Member

@cubic-dev-ai leave a review.

@cubic-dev-ai
Copy link
Contributor

cubic-dev-ai bot commented Dec 5, 2025

@cubic-dev-ai leave a review.

@Pallava-Joshi I've started the AI code review. It'll take a few minutes to complete.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 4 files

Prompt for AI agents (all 2 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name=".eslintrc.js">

<violation number="1" location=".eslintrc.js:99">
P0: Duplicate object keys: the new `group` and `message` properties are overwritten by the existing ones below. This pattern should be a separate object in the `patterns` array. The tRPC restriction will have no effect.</violation>
</file>

<file name="packages/features/bookings/lib/getBookingFields.ts">

<violation number="1" location="packages/features/bookings/lib/getBookingFields.ts:368">
P2: The `required` property could incorrectly override a user&#39;s configuration. If the user has set `attendeePhoneNumber` to required, but no workflow step has `numberRequired: true`, this would reset it to `false`. Consider preserving the existing setting like in the consolidation block below: `required: bookingFields[attendeePhoneNumberIndex].required || isPhoneNumberRequired`.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

@keithwillcode keithwillcode added Stale and removed Stale labels Jan 15, 2026
@github-actions
Copy link
Contributor

Devin AI is completing this stale PR

This PR by @hemantmm has been marked as stale. A Devin session has been created to complete the remaining work.

View Devin Session


Devin will review the PR, address any feedback, and push updates to complete this PR.

devin-ai-integration bot and others added 2 commits January 15, 2026 22:26
- Skip legacy phone fields (smsReminderNumber, aiAgentCallPhoneNumber) in BookingFields.tsx
- Preserve user's required setting when updating attendeePhoneNumber field
- Apply lint fixes for import organization

Co-authored-by: hemantmm <[email protected]>
Co-Authored-By: unknown <>
@devin-ai-integration
Copy link
Contributor

Completed PR Updates

I've completed the work on this PR to address the review feedback and resolve merge conflicts. Here's a summary of the changes:

Changes Made

  1. Merged with upstream/main - The PR branch was significantly behind main. Resolved conflicts after:

    • .eslintrc.js was deleted in upstream (the tRPC import restriction change is no longer needed as the file structure changed)
    • BookingFields.tsx was moved from packages/features/bookings/Booker/components/BookEventForm/ to apps/web/modules/bookings/components/BookEventForm/
  2. Fixed required property issue (addresses @cubic-dev-ai review feedback)

    • Changed required: isPhoneNumberRequired to required: bookingFields[attendeePhoneNumberIndex].required || isPhoneNumberRequired
    • This preserves the user's existing configuration if they had set attendeePhoneNumber to required
  3. Applied lint fixes - Import organization cleanup via biome

Files Modified

  • apps/web/modules/bookings/components/BookEventForm/BookingFields.tsx - Skip legacy phone fields
  • packages/features/bookings/lib/getBookingFields.ts - Preserve user's required setting

The PR is now ready for review. The core functionality remains the same as @hemantmm's original implementation - consolidating phone inputs to show only attendeePhoneNumber instead of multiple phone fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working community Created by Linear-GitHub Sync High priority Created by Linear-GitHub Sync 🧹 Improvements Improvements to existing features. Mostly UX/UI size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

One Phone Input no matter the workflows / questions

7 participants