feat: removed the confirm step in creating bookings #18380

theVedanta · 2024-12-27T02:49:04Z

What does this PR do?

Removed the confirm step changes in this PR because as I see it, once the URL params are passed and checked in the useEffect, the booking state is changed, where I've replaced it with the handleEventBooking();

Fixes [CAL-4943] If all information for "confirm" step is provided skip step #18307 (GitHub issue number)
Fixes CAL-4943 (Linear issue number - should be visible at the bottom of the GitHub issue description)

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
[N/A] 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.

LOOM LINK: https://www.loom.com/share/2967cdb902fb4945908e17c79bb379b8?sid=88459728-1b9e-46e9-af1a-9168cd8cc0cc

How should this be tested?

You can start by creating event types with varying complexity and check if any URL params are conflicting this direct booking without confirmation.

No ENV vars required to test.

vercel · 2024-12-27T02:49:09Z

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

A member of the Team first needs to authorize it.

CLAassistant · 2024-12-27T02:49:11Z

All committers have signed the CLA.

github-actions · 2024-12-27T02:49:16Z

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "added functionality to remove the confirm step in creating bookings". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

graphite-app · 2024-12-27T02:50:22Z

Graphite Automations

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

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

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

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

"Add community label" took an action on this PR • (12/27/24)

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

yarn.lock

Praashh · 2024-12-27T17:49:56Z

also can you please add a loom recording of the fix @theVedanta 🙏 ?

packages/prisma/.env

theVedanta · 2024-12-27T18:56:04Z

also can you please add a loom recording of the fix @theVedanta 🙏 ?

I've added it in the first comment or the PR description.

Praashh · 2024-12-27T19:48:27Z

also can you please add a loom recording of the fix @theVedanta 🙏 ?

I've added it in the first comment or the PR description.

Alright, thanks! One thing I wanted to ask is, if someone accidentally clicks the button, it’ll immediately book the meeting. I think it would be better to have a confirmation dialog box or something similar to prevent this.

theVedanta · 2024-12-27T20:30:12Z

also can you please add a loom recording of the fix @theVedanta 🙏 ?

I've added it in the first comment or the PR description.

Alright, thanks! One thing I wanted to ask is, if someone accidentally clicks the button, it’ll immediately book the meeting. I think it would be better to have a confirmation dialog box or something similar to prevent this.

This has been discussed in the GitHub issue before. Please check the issue once and let me know if a dialog box is required.

Praashh · 2024-12-28T06:12:21Z

also can you please add a loom recording of the fix @theVedanta 🙏 ?

I've added it in the first comment or the PR description.

Alright, thanks! One thing I wanted to ask is, if someone accidentally clicks the button, it’ll immediately book the meeting. I think it would be better to have a confirmation dialog box or something similar to prevent this.

This has been discussed in the GitHub issue before. Please check the issue once and let me know if a dialog box is required.

yeah, got the answer, thank you 🙏

packages/prisma/schema.prisma

Praashh

Can we have a test case for this feature @theVedanta?

theVedanta · 2024-12-31T22:20:00Z

Can we have a test case for this feature @theVedanta?

Sure, can you please provide some context and more information on how you want the tescases generaeted?

Praashh · 2025-01-01T05:11:50Z

Can we have a test case for this feature @theVedanta?

Sure, can you please provide some context and more information on how you want the tescases generaeted?

We need to test it on a variety of events, ensuring the behavior is expected for all events.

hariombalhara · 2025-01-01T10:15:55Z

packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx

@@ -95,7 +95,10 @@ export const BookEventForm = ({
          // still change the timeslot, and comes back to the form, all their values
          // still exist. This gets cleared when the form is submitted.
          const values = bookingForm.getValues();
-          setFormValues(values);
+
+          if (JSON.stringify(values) !== JSON.stringify(bookingData)) {


What does it do? Doesn't seem like the correct approach.

The comparison is sensitive to irrelevant changes like property order or case, leading to unexpected form behavior

oh ok I will correct this then.

Is this addressed?

Yes, this is fixed

hariombalhara · 2025-01-01T11:39:37Z

@theVedanta needs confirmation on skipping the Booking Form as well here cc @CarinaWolli

I think we should skip the form and the loader should be the there on the slot itself.

Also @theVedanta, keep in mind that users can configure custom booking questions under Advanced section of the event and mark some of them required. We would be able to submit the slot directly only if all required fields have the data.

hariombalhara

Left some comments

theVedanta · 2025-01-02T01:49:49Z

@theVedanta needs confirmation on skipping the Booking Form as well here cc @CarinaWolli

I think we should skip the form and the loader should be the there on the slot itself.

Also @theVedanta, keep in mind that users can configure custom booking questions under Advanced section of the event and mark some of them required. We would be able to submit the slot directly only if all required fields have the data.

I understand. I will make some changes soon, let me know if they are ok according to the tests.

PeerRich · 2025-01-03T22:05:53Z

correct we only skip the "confirm" step if all required inputs are prefilled

maybe even if all inputs are prefilled, no?

not only required but also the optional ones

github-actions · 2025-01-03T23:25:08Z

E2E results are ready!

Workflow #42871.1 latest results

…e updates and maximum limit is reached)

…eVedanta/cal.com into remove-confirm-before-booking

theVedanta · 2025-01-04T00:38:42Z

@PeerRich @Praashh @hariombalhara @CarinaWolli @keithwillcode I have made a change to the PR. Now, it checks for every field being filled in the form based on URL-Params. I can't figure out however, why the "maximum update depth reached" error occurs.
The app works fine when there are additional questions that do not already have a value in the URL Params.

This is what I mean with the help of an example:
When the meeting type has an additional question that is required, say, the Reference ID of a person.
If the URL Params contain the time, date, name, email, and other details of the user and not the Reference ID, the app works fine and stops to ask the user for it. However, when it is all filled in the URL-Params (including this hypothetical Reference ID, then we are running into a maximum-state-update error).

Can you please help me figure out the cause and potential fixes for this?

This is my first PR and I'm not very familiar with the workspace, so your help would be highly appreciated.

theVedanta · 2025-01-04T00:39:33Z

@PeerRich @Praashh @hariombalhara @CarinaWolli @keithwillcode I have made a change to the PR. Now, it checks for every field being filled in the form based on URL-Params. I can't figure out however, why the "maximum update depth reached" error occurs. The app works fine when there are additional questions that do not already have a value in the URL Params.

This is what I mean with the help of an example: When the meeting type has an additional question that is required, say, the Reference ID of a person. If the URL Params contain the time, date, name, email, and other details of the user and not the Reference ID, the app works fine and stops to ask the user for it. However, when it is all filled in the URL-Params (including this hypothetical Reference ID, then we are running into a maximum-state-update error).

Can you please help me figure out the cause and potential fixes for this?

This is my first PR and I'm not very familiar with the workspace, so your help would be highly appreciated.

Also, I have fixed the JSON.stringify comparison which was not necessary

added functionality to remove the confirm step in creating bookings

53cef29

graphite-app bot requested review from a team December 27, 2024 02:49

graphite-app bot added the community Created by Linear-GitHub Sync label Dec 27, 2024

github-actions bot added High priority Created by Linear-GitHub Sync ✨ feature New feature or request labels Dec 27, 2024

dosubot bot added bookings area: bookings, availability, timezones, double booking ❗️ migrations contains migration files labels Dec 27, 2024

theVedanta mentioned this pull request Dec 27, 2024

[CAL-4943] If all information for "confirm" step is provided skip step #18307

Open

theVedanta changed the title ~~added functionality to remove the confirm step in creating bookings~~ feat: removed the confirm step in creating bookings Dec 27, 2024

Praashh reviewed Dec 27, 2024

View reviewed changes

yarn.lock Outdated Show resolved Hide resolved

theVedanta added 2 commits December 27, 2024 10:57

changed the max-length update state exceeding error

872aad3

Revert yarn.lock to its state before unintended changes

b5dd4a0

Praashh reviewed Dec 27, 2024

View reviewed changes

packages/prisma/.env Outdated Show resolved Hide resolved

Revert packages/prisma/.env

e0ad39e

Praashh reviewed Dec 28, 2024

View reviewed changes

packages/prisma/schema.prisma Outdated Show resolved Hide resolved

Revert schema.prisma to its state before unintended changes

0f18bff

Praashh requested changes Dec 31, 2024

View reviewed changes

hariombalhara reviewed Jan 1, 2025

View reviewed changes

hariombalhara requested changes Jan 1, 2025

View reviewed changes

hbjORbj added the ready-for-e2e label Jan 3, 2025

github-actions bot marked this pull request as draft January 3, 2025 23:01

Merge branch 'main' into remove-confirm-before-booking

f7a3345

keithwillcode added the high-risk Requires approval by Foundation team label Jan 3, 2025

theVedanta added 2 commits January 3, 2025 18:30

check for required fields filled (functional, but updates trigger mor…

6cbe5a7

…e updates and maximum limit is reached)

Merge branch 'remove-confirm-before-booking' of https://github.com/th…

46af08b

…eVedanta/cal.com into remove-confirm-before-booking

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: removed the confirm step in creating bookings #18380

feat: removed the confirm step in creating bookings #18380

theVedanta commented Dec 27, 2024 •

edited

Loading

vercel bot commented Dec 27, 2024

CLAassistant commented Dec 27, 2024 •

edited

Loading

github-actions bot commented Dec 27, 2024

graphite-app bot commented Dec 27, 2024

Praashh commented Dec 27, 2024

theVedanta commented Dec 27, 2024

Praashh commented Dec 27, 2024

theVedanta commented Dec 27, 2024

Praashh commented Dec 28, 2024

Praashh left a comment

theVedanta commented Dec 31, 2024

Praashh commented Jan 1, 2025

hariombalhara Jan 1, 2025 •

edited

Loading

theVedanta Jan 2, 2025

hbjORbj Jan 3, 2025

theVedanta Jan 4, 2025

hariombalhara commented Jan 1, 2025

hariombalhara left a comment

theVedanta commented Jan 2, 2025

PeerRich commented Jan 3, 2025

github-actions bot commented Jan 3, 2025 •

edited

Loading

theVedanta commented Jan 4, 2025

theVedanta commented Jan 4, 2025

feat: removed the confirm step in creating bookings #18380

Are you sure you want to change the base?

feat: removed the confirm step in creating bookings #18380

Conversation

theVedanta commented Dec 27, 2024 • edited Loading

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

How should this be tested?

vercel bot commented Dec 27, 2024

CLAassistant commented Dec 27, 2024 • edited Loading

github-actions bot commented Dec 27, 2024

graphite-app bot commented Dec 27, 2024

Graphite Automations

Praashh commented Dec 27, 2024

theVedanta commented Dec 27, 2024

Praashh commented Dec 27, 2024

theVedanta commented Dec 27, 2024

Praashh commented Dec 28, 2024

Praashh left a comment

Choose a reason for hiding this comment

theVedanta commented Dec 31, 2024

Praashh commented Jan 1, 2025

hariombalhara Jan 1, 2025 • edited Loading

Choose a reason for hiding this comment

theVedanta Jan 2, 2025

Choose a reason for hiding this comment

hbjORbj Jan 3, 2025

Choose a reason for hiding this comment

theVedanta Jan 4, 2025

Choose a reason for hiding this comment

hariombalhara commented Jan 1, 2025

hariombalhara left a comment

Choose a reason for hiding this comment

theVedanta commented Jan 2, 2025

PeerRich commented Jan 3, 2025

github-actions bot commented Jan 3, 2025 • edited Loading

E2E results are ready!

theVedanta commented Jan 4, 2025

theVedanta commented Jan 4, 2025

theVedanta commented Dec 27, 2024 •

edited

Loading

CLAassistant commented Dec 27, 2024 •

edited

Loading

hariombalhara Jan 1, 2025 •

edited

Loading

github-actions bot commented Jan 3, 2025 •

edited

Loading