fix: location change during rescheduling #15810

dilpreetsio · 2024-07-17T14:45:04Z

What does this PR do?

Fixes [CAL-4049] Choosing a different location when rescheduling not working #15806 (GitHub issue number)

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
I have added a Docs issue here 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?

Create an event type with multiple locations
Create a booking
Reschedule the booking with new location

Inperson-location.mov

Meet.links.mov

vercel · 2024-07-17T14:45:11Z

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

A member of the Team first needs to authorize it.

github-actions · 2024-07-17T14:45:18Z

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 "Fix location change during rescheduling". 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-07-17T14:50:11Z

Graphite Automations

"Add community label" took an action on this PR • (07/17/24)

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

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

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

Amit91848 · 2024-07-17T15:55:32Z

Type checks are failing @dilpreetsio , please fix those 🙌

hariombalhara · 2024-07-18T07:45:44Z

apps/web/modules/event-types/views/event-types-single-view.tsx

@@ -227,6 +227,7 @@ const EventTypePage = (props: EventTypeSetupProps & { allActiveWorkflows?: Workf
 const [pendingRoute, setPendingRoute] = useState("");
 const leaveWithoutAssigningHosts = useRef(false);
 const [animationParentRef] = useAutoAnimate<HTMLDivElement>();
+


Let's remove this whitespace change.

hariombalhara · 2024-07-18T08:32:47Z

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

- readOnly = false;
- }
+ // TODO: We need to ensure that location field is properly updated in Booking Success Page, email, calendar, webhook after reschedule
+ readOnly = false;


Made this change because if (locationResponse?.value === "attendeeInPerson" || "phone") { condition is faulty.

It is always truthy. So, it seems like location change is enabled for attendeeInPerson and phone but in fact it is enabled in all cases.

Introduced in #11651

I think with this PR we should intend to be able to make any change to location field and test it accordingly.

hariombalhara · 2024-07-18T08:33:14Z

packages/features/form-builder/schema.ts

+ "system", // Deletable-No, CanChangeIdentifier-No, Toggleable-No, CanMarkOptional-No.
+ "system-but-optional", // Deletable-No, CanChangeIdentifier-No, Toggleable-Yes, CanMarkOptional-Yes
+ // TODO: Seems to be unused and could probably be deleted
+ "system-but-hidden", // Deletable-No, CanChangeIdentifier-No, Always Hidden and thus Optional
+ "user", // Fully editable. Deletable-Yes, CanChangeIdentifier-Yes, Toggleable-Yes, CanMarkOptional-Yes
+ // TODO: Seems to be unused and could probably be deleted


Updated the comments to make it more readable.

hariombalhara · 2024-07-18T09:18:26Z

packages/features/form-builder/Components.tsx

@@ -391,7 +391,7 @@ export const Components: Record<FieldType, Component> = {
 if (!value) {
 setValue({
 value: options[0]?.value,
- optionValue: "",
+ optionValue: options[0]?.optionValue || "",


I don't understand this and other change in this component.

optionValue can only be updated through ComponentForField at line 469.

The booking view expects the optionValue to have the a valid value.

cal.com/apps/web/modules/bookings/views/bookings-single-view.tsx

Line 132 in 2ad3592

"optionValue" in bookingInfo.responses.location

Currently the optionValue is always empty regardless of the option selected.

hariombalhara

Thanks @dilpreetsio for the PR.

I don't understand right now how the changes are fixing the bug. While testing I could see the same behaviour even without the changes in components.tsx file.

Could you confirm that the behaviour(as per the recording your shared in the PR description) didn't exist in main?

dilpreetsio · 2024-07-18T09:43:45Z

@hariombalhara Yes the behaviour on main is not as expected.

Screen.Recording.2024-07-18.at.3.12.10.PM.mov

Amit91848 · 2024-07-18T09:57:02Z

Same issue was flagged earlier #15108 (comment). Also add tests to ensure this doesn't break in the future.

github-actions · 2024-08-15T00:18:31Z

This PR is being marked as stale due to inactivity.

Amit91848 · 2024-09-03T06:59:28Z

Thanks for your contribution but closing in favour of #16162

github-actions bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Jul 17, 2024

dosubot bot added the booking-page area: booking page, public booking page, booker label Jul 17, 2024

graphite-app bot added the community Created by Linear-GitHub Sync label Jul 17, 2024

graphite-app bot requested a review from a team July 17, 2024 14:47

dilpreetsio force-pushed the reschedule-location branch from f7744f2 to 1516ca5 Compare July 17, 2024 14:50

Amit91848 changed the title ~~Fix location change during rescheduling~~ fix: location change during rescheduling Jul 17, 2024

fix: location change during rescheduling

9bd23d4

dilpreetsio force-pushed the reschedule-location branch from 1516ca5 to 9bd23d4 Compare July 17, 2024 17:43

hariombalhara reviewed Jul 18, 2024

View reviewed changes

Improve comments and fix location check

d83c103

hariombalhara reviewed Jul 18, 2024

View reviewed changes

remove white-space

120ae97

hariombalhara reviewed Jul 18, 2024

View reviewed changes

hariombalhara requested changes Jul 18, 2024

View reviewed changes

Amit91848 mentioned this pull request Jul 18, 2024

feat: add "somewhere else" as a location option #15108

Merged

3 tasks

PeerRich added this to the Community Only milestone Jul 31, 2024

github-actions bot added the Stale label Aug 15, 2024

anikdhabal marked this pull request as draft August 23, 2024 18:41

github-actions bot removed the Stale label Aug 24, 2024

Amit91848 closed this Sep 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: location change during rescheduling #15810

fix: location change during rescheduling #15810

dilpreetsio commented Jul 17, 2024

vercel bot commented Jul 17, 2024

github-actions bot commented Jul 17, 2024 •

edited

Loading

graphite-app bot commented Jul 17, 2024

Amit91848 commented Jul 17, 2024

hariombalhara Jul 18, 2024

hariombalhara Jul 18, 2024

hariombalhara Jul 18, 2024

hariombalhara Jul 18, 2024

dilpreetsio Jul 18, 2024

hariombalhara left a comment •

edited

Loading

dilpreetsio commented Jul 18, 2024

Amit91848 commented Jul 18, 2024

github-actions bot commented Aug 15, 2024

Amit91848 commented Sep 3, 2024

fix: location change during rescheduling #15810

fix: location change during rescheduling #15810

Conversation

dilpreetsio commented Jul 17, 2024

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

How should this be tested?

vercel bot commented Jul 17, 2024

github-actions bot commented Jul 17, 2024 • edited Loading

graphite-app bot commented Jul 17, 2024

Graphite Automations

Amit91848 commented Jul 17, 2024

hariombalhara Jul 18, 2024

Choose a reason for hiding this comment

hariombalhara Jul 18, 2024

Choose a reason for hiding this comment

hariombalhara Jul 18, 2024

Choose a reason for hiding this comment

hariombalhara Jul 18, 2024

Choose a reason for hiding this comment

dilpreetsio Jul 18, 2024

Choose a reason for hiding this comment

hariombalhara left a comment • edited Loading

Choose a reason for hiding this comment

dilpreetsio commented Jul 18, 2024

Amit91848 commented Jul 18, 2024

github-actions bot commented Aug 15, 2024

Amit91848 commented Sep 3, 2024

github-actions bot commented Jul 17, 2024 •

edited

Loading

hariombalhara left a comment •

edited

Loading