feat: booking with phone number

[Udit-takkar]

trafficstars

What does this PR do?

Fixes https://github.com/calcom/cal.com/issues/14534 Follow up: https://github.com/calcom/cal.com/issues/15415

How to create event type with only phone number?

1. Create a Team Event type

2. Make Email field not required and toggle off to hide the field

Turn the toggle on for attendee phone number

3. Fill the form

• Added 4 Unit Tests

  • [Event Type with only Attendee Phone number as required field and Email as hidden field] succesfully creates a booking when the users are available as per the common schedule selected in the event-type
  • [Event Type with both Attendee Phone number and Email as required fields] succesfully creates a booking when the users are available as per the common schedule selected in the event-type
  • [Event Type that requires confirmation with only Attendee Phone number as required field and Email as optional field] succesfully creates a booking when the users are available as per the common schedule selected in the event-type
  • [Event Type with Both Email and Attendee Phone Number as required fields] should send rescheduling emails when round robin is rescheduled to same host

• Added 2 E2E tests

  • Can create a booking for Round Robin EventType with both phone number and email required
  • Can create a booking for Collective EventType with only phone number

Type of change

• New feature (non-breaking change which adds functionality)

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
ai	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 11, 2024 6:49pm
platform-starter-kit	❌ Failed (Inspect)			Sep 11, 2024 6:49pm

3 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
cal	⬜️ Ignored (Inspect)	Visit Preview		Sep 11, 2024 6:49pm
calcom-web-canary	⬜️ Ignored (Inspect)	Visit Preview		Sep 11, 2024 6:49pm
qa	⬜️ Ignored (Inspect)	Visit Preview		Sep 11, 2024 6:49pm

[github-actions[bot]]

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

[deploysentinel[bot]]

Current Playwright Test Results Summary

✅ 318 Passing - ❌ 3 Failing - ⚠️ 16 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 05/21/2024 03:16:11pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: eb72e57f767ecb478d8e9ef043eb156f97e42a5f

Started: 05/21/2024 03:12:32pm UTC

❌ Failures

📄   apps/web/playwright/profile.e2e.ts • 1 Failure

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Update Profile Can update a users email (verification enabled) Retry 2 • Retry 1 • Initial Attempt Error: page.waitForURL: Timeout 30000ms exceeded.... page.waitForURL: Timeout 30000ms exceeded. =========================== logs =========================== waiting for navigation to "/event-types" until "load" navigated to "http://localhost:3000/auth/login" ============================================================	38.81% (104) 104 / 268 runs failed over last 7 days	33.58% (90) 90 / 268 runs flaked over last 7 days

---

📄   apps/web/playwright/managedBooking/advancedOptions.e2e.ts • 1 Failure

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Check advanced options in a managed team event type Check advanced options in a managed team event type with offer seats Retry 2 • Retry 1 • Initial Attempt Error: Test timeout of 60000ms exceeded. Test timeout of 60000ms exceeded.	2.53% (6) 6 / 237 runs failed over last 7 days	5.49% (13) 13 / 237 runs flaked over last 7 days

---

📄   apps/web/playwright/manage-booking-questions.e2e.ts • 1 Failure

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Manage Booking Questions For Team EventType Do a booking with a user added question and verify a few thing in b/w Retry 2 • Retry 1 • Initial Attempt Error: Timed out 30000ms waiting for expect(received).toBeVisible()... Timed out 30000ms waiting for expect(received).toBeVisible() Call log: - expect.toBeVisible with timeout 30000ms - waiting for locator('[data-fob-field-name]:not(.hidden)').nth(5).locator('[name="how-are-you"]') - waiting for locator('[data-fob-field-name]:not(.hidden)').nth(5).locator('[name="how-are-you"]')	6.78% (16) 16 / 236 runs failed over last 7 days	1.27% (3) 3 / 236 runs flaked over last 7 days

---

⚠️ Flakes

📄   apps/web/playwright/integrations-stripe.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Stripe integration When event is paid and confirmed Payment should confirm pending payment booking Retry 1 • Initial Attempt	4.55% (10) 10 / 220 runs failed over last 7 days	5.45% (12) 12 / 220 runs flaked over last 7 days

---

📄   apps/web/playwright/signup.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Signup Flow Test Email verification sent if enabled Retry 1 • Initial Attempt	0.81% (2) 2 / 247 runs failed over last 7 days	28.74% (71) 71 / 247 runs flaked over last 7 days

---

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 9 Flakes

Top 1 Common Error Messages

null	9 Test Cases Affected

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Popup Tests should open embed iframe on click - Configured with light theme Retry 1 • Initial Attempt	2.55% (6) 6 / 235 runs failed over last 7 days	60.43% (142) 142 / 235 runs flaked over last 7 days
Popup Tests should be able to reschedule Retry 1 • Initial Attempt	-163.22% (-142) -142 / 87 runs failed over last 7 days	163.22% (142) 142 / 87 runs flaked over last 7 days
Popup Tests should open Routing Forms embed on click Retry 1 • Initial Attempt	-163.22% (-142) -142 / 87 runs failed over last 7 days	163.22% (142) 142 / 87 runs flaked over last 7 days
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe according to system theme when no theme is configured through Embed API Retry 1 • Initial Attempt	-159.77% (-139) -139 / 87 runs failed over last 7 days	160.92% (140) 140 / 87 runs flaked over last 7 days
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe according to system theme when configured with 'auto' theme using Embed API Retry 1 • Initial Attempt	-161.63% (-139) -139 / 86 runs failed over last 7 days	161.63% (139) 139 / 86 runs flaked over last 7 days
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe(Booker Profile Page) with dark theme when configured with dark theme using Embed API Retry 1 • Initial Attempt	-161.63% (-139) -139 / 86 runs failed over last 7 days	161.63% (139) 139 / 86 runs flaked over last 7 days
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe(Event Booking Page) with dark theme when configured with dark theme using Embed API Retry 1 • Initial Attempt	-161.63% (-139) -139 / 86 runs failed over last 7 days	161.63% (139) 139 / 86 runs flaked over last 7 days
Popup Tests prendered embed should be loaded and apply the config given to it Retry 1 • Initial Attempt	-161.63% (-139) -139 / 86 runs failed over last 7 days	161.63% (139) 139 / 86 runs flaked over last 7 days
Popup Tests should open on clicking child element Retry 1 • Initial Attempt	-159.30% (-137) -137 / 86 runs failed over last 7 days	159.30% (137) 137 / 86 runs flaked over last 7 days

---

📄   packages/embeds/embed-core/playwright/tests/namespacing.e2e.ts • 4 Flakes

Top 1 Common Error Messages

null	4 Test Cases Affected

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Namespacing Inline Embed Double install Embed Snippet with inline embed using a namespace Retry 1 • Initial Attempt	0.43% (1) 1 / 234 run failed over last 7 days	61.54% (144) 144 / 234 runs flaked over last 7 days
Namespacing Inline Embed Add inline embed using a namespace without reload Retry 1 • Initial Attempt	0% (0) 0 / 234 runs failed over last 7 days	63.68% (149) 149 / 234 runs flaked over last 7 days
Namespacing Inline Embed Double install Embed Snippet with inline embed without a namespace(i.e. default namespace) Retry 1 • Initial Attempt	0% (0) 0 / 234 runs failed over last 7 days	64.53% (151) 151 / 234 runs flaked over last 7 days
Namespacing Different namespaces can have different init configs Retry 1 • Initial Attempt	0% (0) 0 / 233 runs failed over last 7 days	61.37% (143) 143 / 233 runs flaked over last 7 days

---

📄   apps/web/playwright/login.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
user can login & logout succesfully -- future login flow user & logout using dashboard Retry 1 • Initial Attempt	4.18% (10) 10 / 239 runs failed over last 7 days	35.56% (85) 85 / 239 runs flaked over last 7 days

---

View Detailed Build Results

---

[github-actions[bot]]

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

[graphite-app[bot]]

Graphite Automations

"Add foundation team as reviewer" took an action on this PR • (05/13/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add consumer team as reviewer" took an action on this PR • (05/13/24)

1 reviewer was added to this PR based on Keith Williams's automation.

[Udit-takkar]

• We should revaluate the email-manager since it is not just sending emails but also SMS messages. We could have a NotificationManager class to handle the logic of whether to send emails or SMS messages and to whom

Yeah this is what carina also suggested on my post on campsite. I am planning to add this in my new PR because this one's is already too big.

[Udit-takkar]

Have you considered any alternatives to not using the email placeholder if an attendee books via phone number? I had two ideas. Both of these ideas would transform the attendee email column to optional.

@joeauyeung Actually making email an optional field would require adding too many checks in the complete codebase and could potentially break some things so peer suggested adding a placeholder "[email protected]".

But yeah for the next version it would best to make the email field optional and adding a constraint like you mentioned. I'll open an issue for this

[joeauyeung]

Have you considered any alternatives to not using the email placeholder if an attendee books via phone number? I had two ideas. Both of these ideas would transform the attendee email column to optional.

@joeauyeung Actually making email an optional field would require adding too many checks in the complete codebase and could potentially break some things so peer suggested adding a placeholder "[email protected]".

But yeah for the next version it would best to make the email field optional and adding a constraint like you mentioned. I'll open an issue for this

I would argue we can reduce the amount of code changes we would have to make by having that logic live in an attendee repository class and calling a createAttendees() that contains the logic to check for an email or phone number.

[Udit-takkar]

I would argue we can reduce the amount of code changes we would have to make by having that logic live in an attendee repository class and calling a createAttendees() that contains the logic to check for an email or phone number.

@joeauyeung I tried setting email as optional parameter and check constraint but there's just too much code that assumes email is present and a lot of types like Person , TeamMember would become in compatible. Trying to fix them now

[Udit-takkar]

Found some issues when testing:

• [ ] When event has workflow with email to attendee we need to automatically add the email booking question

Just this one is left which I'll add later. (similar to upsertSmsReminderFieldForBooking)

[CarinaWolli]

I forgot to add that to my review:

Here are some other issues I found that I think can be handled in a follow up. Could you create issues for them, if not handled in this PR?

• Two phone number fields when 'SMS to attendee' workflow is enabled on the event type
• Either change 'requires booker email verification' to 'requires booker sms verification' or disable feature
• rename email-manager (it doesn't only manage emails)
• (from review above) When event has workflow with email to attendee we need to automatically add the email booking question

[CarinaWolli]

@Udit-takkar didn't we talk about making this a org-only feature for now?

[Udit-takkar]

@Udit-takkar didn't we talk about making this a org-only feature for now?

Yeah. should i wait for a credit system for sending SMS or just make it org only feature ?

[CarinaWolli]

@Udit-takkar didn't we talk about making this a org-only feature for now?

Yeah. should i wait for a credit system for sending SMS or just make it org only feature ?

Let's make it org-only for now

[Udit-takkar]

@PeerRich Keith suggested that we merge this after retreat.

[keithwillcode]

@zomars @emrysal Moving back to draft as we will wait until post-retreat to merge this.

[github-actions[bot]]

This PR is being marked as stale due to inactivity.

[PeerRich]

not stale, lets get this reviewed after merge conflicts are resolved

[Udit-takkar]

Since this is a significant change of the booking process. Can we at least add a couple of tests cases for our E2E booking flow tests?

Yeah sure. I already added few unit tests. is there any specific scenario that i should add?

Since email is not required anymore I feel like this is introducing a lot of nesting and overall it's becoming harder to track these fields. What's the rationale behind the DB design? What other alternatives were considered and why was this the winner?

The changes in DB schema are:-

I made email an optional field now and added a new field phoneNumber in Attendee model with a constraint that make sure atleast one of the two exist

ALTER TABLE "Attendee"
ADD CONSTRAINT check_email_or_phone
CHECK ("email" IS NOT NULL OR "phoneNumber" IS NOT NULL);

BOOKED_WITH_SMS is used as a placeholder email address when it's not present as a lot of our codebase depend upon bookerEmail and this is a very big change. Few things like creating a calendar event would also require email of attendee.

Eventually I have to completely remove BOOKED_WITH_SMS after some time on production. This feature is only limited to org team event types for now and we need it fast for cal ai feature which some customers are waiting for.

[zomars]

Yeah sure. I already few unit tests. is there any specific scenario that i should add?

Booking with only phone required. Booking with both phone and email required.

[Udit-takkar]

Booking with only phone required. Booking with both phone and email required.

Done.

[Udit-takkar]

It's really important that we make sure that we always send a SMS or Email. Can you make sure to test all SMS manually, I might have missed something when testing

Fixed and Tested more sms types

1. scheduled seats, rescheduled seats, cancelled seats
2. location changed
3. attendee requested

not adding sms for daily recording, transcript for now.

[PeerRich]

excited for this

[github-actions[bot]]

E2E results are ready!

• Workflow #31406.1 latest results

[zomars]

Really solid PR @Udit-takkar. When reviewing I was thinking about the long term maintainability of our codebase. We'll now have to start testing against emails & phone numbers.

• The big thing I noticed is you added a RequiresAtLeastOne type. Could we use this in the functions where we see email?: string; phoneNumber?: string?
• Thoughts on having a bookingAttendee class where we pass the booking attendee. I see the email / phone number as the attendee identifier so we could have a method like bookingAttendee.getIdentifier() to get those fields. Rather than calling stuff like getEmail or getPhoneNumber? Could also provide some safety if the email or phone number doesn't exist.
• I'm seeing two instances of getEventTypesFromDB can we stack a PR on top of this to remove the two instances?
• Could we create a prisma middleware when creating an attendee to ensure an email or phone number is present?

Pretty valid points. @joeauyeung I was also thinking something similar. We could even treat the email field as a phone number input in order to avoid changing something as fundamental as email in our booking process.

This is not a "simple" change at all.

I'd rather use phones as as "faux email" so you just type you phone number: +1 555 555 5555 but internally it gets handled as a email: [email protected] or something like that.

On the technical side, treating phone numbers as "faux emails" could simplify how the system works behind the scenes. By handling all identifiers in a similar way, you might reduce the need for complicated changes to the system, keeping things running smoothly without a major overhaul. It’s a way to make the switch without disrupting everything.

EDIT: I still don't feel 100% confident that shipping the current approach won't "break" anything. This might be because of lack of automated test or edge cases we aren't really aware of right not.

[PeerRich]

maybe [email protected]?

[Udit-takkar]

On the organizer calendar event the placeholder email is appearing as an attendee

Do you mean here ?

We need some email to create a calendar event. Do you suggest we don't add any email in guests when email is not passed?

[Udit-takkar]

When editing the booking fields, even if a field is hidden the required field check is still run. This might could be annoying when transitioning from email -> phone numbers for bookings

Okay. now I changed the logic.

[joeauyeung]

On the organizer calendar event the placeholder email is appearing as an attendee

Do you mean here ?

We need some email to create a calendar event. Do you suggest we don't add any email in guests when email is not passed?

I thought we didn't want to the organizers to see the placeholder emails. If we're ok with that then I'm ok. We just have to remember to create a catch all rule so organizers don't see this email from their calendar

[zomars]

Fixing conflicts before testing.