refactor: Only send dirty fields for event type update call #13292

alishaz-polymath · 2024-01-18T05:59:43Z

What does this PR do?

Earlier we would pass all event type fields even if the change is in one field. This PR updates the logic to only pass in dirty fields instead of all fields. This is a pre-requisite to Managed Events V2, so this PR would need to be merged before we can continue sorting out the Managed Events V2.

Here's a list of all fields that currently identify as dirty accurately:

Fixes #13291
Fixes #13183

Requirement/Documentation

If there is a requirement document, please, share it here.
If there is ab UI/UX design document, please, share it here.

Type of change

Chore (refactoring code, technical debt, workflow improvements)
New feature (non-breaking change which adds functionality)

How should this be tested?

Rigorously and carefully, touches core (event-types) and so it should be tested against all kinds of event-types, with the idea of saving changes to an event type. In managed events, it should only over-write the fields changed by Admin and not all the fields.

Mandatory Tasks

Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

vercel · 2024-01-18T05:59:54Z

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

Name	Status	Preview	Comments	Updated (UTC)
api	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 15, 2024 4:19pm
dev	❌ Failed (Inspect)			Feb 15, 2024 4:19pm

6 Ignored Deployments

Name	Status	Preview	Updated (UTC)
ai	⬜️ Ignored (Inspect)	Visit Preview	Feb 15, 2024 4:19pm
cal	⬜️ Ignored (Inspect)	Visit Preview	Feb 15, 2024 4:19pm
cal-demo	⬜️ Ignored (Inspect)	Visit Preview	Feb 15, 2024 4:19pm
calcom-web-canary	⬜️ Ignored (Inspect)	Visit Preview	Feb 15, 2024 4:19pm
qa	⬜️ Ignored (Inspect)	Visit Preview	Feb 15, 2024 4:19pm
ui	⬜️ Ignored (Inspect)	Visit Preview	Feb 15, 2024 4:19pm

github-actions · 2024-01-18T05:59:56Z

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

github-actions · 2024-01-18T06:10:44Z

📦 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! 🙌

deploysentinel · 2024-01-18T06:13:09Z

Current Playwright Test Results Summary

✅ 447 Passing - ⚠️ 8 Flaky

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

(Last updated on 02/15/2024 04:36:34pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: a3d87ff

Started: 02/15/2024 04:27:34pm UTC

⚠️ Flakes

📄 apps/web/playwright/booking/multipleEmailQuestion.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Booking With Multiple Email Question and Each Other Question Booking With Multiple Email Question and Address Question Multiple Email required and Address required Retry 1 • Initial Attempt	0% (0) 0 / 241 runs failed over last 7 days	5.39% (13) 13 / 241 runs flaked over last 7 days

📄 apps/web/playwright/booking/selectQuestion.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Booking With Phone Question and Each Other Question Booking With Select Question and checkbox group Question Select required and checkbox group required Retry 1 • Initial Attempt	0% (0) 0 / 244 runs failed over last 7 days	5.33% (13) 13 / 244 runs flaked over last 7 days

📄 apps/web/playwright/booking/responsiveBooking.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Booking page with no questions Booking page with 1920x1080 resolution Retry 1 • Initial Attempt	0% (0) 0 / 244 runs failed over last 7 days	8.20% (20) 20 / 244 runs flaked over last 7 days

📄 apps/web/playwright/booking/checkboxGroupQuestion.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Booking With Checkbox Group Question and Each Other Question Booking With Checkbox Group Question and Address Question Booking With Checkbox Group Question and checkbox Question Checkbox Group and checkbox not required Retry 1 • Initial Attempt	0% (0) 0 / 246 runs failed over last 7 days	6.91% (17) 17 / 246 runs flaked over last 7 days

📄 apps/web/playwright/organization/organization-invitation.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Organization Email matching orgAutoAcceptEmail and a Verified Organization Team Invitation Retry 1 • Initial Attempt	0% (0) 0 / 246 runs failed over last 7 days	8.94% (22) 22 / 246 runs flaked over last 7 days

📄 apps/web/playwright/profile.e2e.ts • 2 Flakes

Top 1 Common Error Messages

`null`	2 Test Cases Affected

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Update Profile Cannot update a users email when existing user has same email (verification enabled) Retry 1 • Initial Attempt	0.38% (1) 1 / 260 run failed over last 7 days	51.15% (133) 133 / 260 runs flaked over last 7 days
Update Profile Can update a users email (verification enabled) Retry 2 • Retry 1 • Initial Attempt	11.54% (30) 30 / 260 runs failed over last 7 days	54.23% (141) 141 / 260 runs flaked over last 7 days

📄 apps/web/playwright/organization/booking.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Bookings Team Event Can create a booking for Round Robin EventType Retry 1 • Initial Attempt	0.43% (1) 1 / 230 run failed over last 7 days	0.43% (1) 1 / 230 run flaked over last 7 days

View Detailed Build Results

sean-brydon

I think we can really optimise this and leverage RHF to do this for us - I also noticed this method was including fields that were never touched when saving. Please see other comments to see my explanation

apps/web/pages/event-types/[type]/index.tsx

packages/lib/isEqual.ts

The bookerLayout changes now work as expected

alishaz-polymath · 2024-02-14T11:25:09Z

Hey @alishaz-polymath Almost everything works, well done!

On the Advanced Tab, if you make changes to the booking layout this works, and it updates. But if you continue to change other Advanced Tab before saving changes the booking layout change is not updated.

Checked against production and it does not happen there, so this is the one regression I could find.

@emrysal Can you please check now if it's still a problem? I tested with the latest changes and it appears to be gone now.

@CarinaWolli Can you check the Availability common schedule and Hosts are now working as expected?

The save button is still something I'm thinking over, because RHF isDirty is unreliable, as we can see. 🤔

CarinaWolli · 2024-02-14T21:26:53Z

apps/web/components/eventtype/EventTeamTab.tsx

@@ -472,10 +472,12 @@ const Hosts = ({
      initialValue.current = { hosts: getValues("hosts"), schedulingType, submitCount };
      return;
    }
-    resetField("hosts", {


hosts weren't reset when changing the scheduling type as it wasn't set dirty

Thank you 🙌 🚀

CarinaWolli · 2024-02-14T21:27:56Z

@CarinaWolli Can you check the Availability common schedule and Hosts are now working as expected?

I added one more fix to reset the hosts, now everything seems to work

alishaz-polymath · 2024-02-15T13:56:53Z

Closed thanks to unintended use of shortcut 😢
Reopened 🙏

alishaz-polymath · 2024-02-15T13:57:51Z

Still not finding an efficient solution for the SAVE button's disabled state. I could potentially extract the dirtyFields check and then maybe set it up in a useEffect inside the EventTypeSingleLayout.tsx file, but I'm wondering if it could be more efficient.
@emrysal Opinion?

emrysal

Approved 😎

CarinaWolli

Tested again and everything worked. Great work @alishaz-polymath 🙌🏻

CarinaWolli · 2024-02-15T16:21:12Z

apps/web/components/eventtype/EventAdvancedTab.tsx

@@ -183,7 +183,7 @@ export const EventAdvancedTab = ({ eventType, team }: Pick<EventTypeSetupProps,
              checked={useEventTypeDestinationCalendarEmail}
              onCheckedChange={(val) => {
                setUseEventTypeDestinationCalendarEmail(val);
-                formMethods.setValue("useEventTypeDestinationCalendarEmail", val);
+                formMethods.setValue("useEventTypeDestinationCalendarEmail", val, { shouldDirty: true });


came in from the latest merge with main

To block unintentional squash and merge

emrysal

Let's get this merged, nice work!

init

b6e2b5e

keithwillcode added the core area: core, team members only label Jan 18, 2024

alishaz-polymath changed the title ~~Chore: Only send dirty fields for event type update call~~ refactor: Only send dirty fields for event type update call Jan 18, 2024

vercel bot deployed to Preview – dev January 18, 2024 06:07 View deployment

typefix maybe

38f51d5

vercel bot deployed to Preview – api January 18, 2024 06:37 View deployment

vercel bot deployed to Preview – dev January 18, 2024 06:50 View deployment

alishaz-polymath and others added 2 commits January 18, 2024 10:53

fix type

53c8dd5

Merge branch 'main' into chore/only-update-dirty-fields-eventtype

58d83ce

vercel bot deployed to Preview – api January 18, 2024 07:03 View deployment

fix e2e

2b2eb24

vercel bot deployed to Preview – api January 18, 2024 07:34 View deployment

vercel bot deployed to Preview – dev January 18, 2024 07:49 View deployment

keithwillcode added this to the v3.8 milestone Jan 18, 2024

keithwillcode requested a review from a team January 18, 2024 18:57

alishaz-polymath mentioned this pull request Jan 18, 2024

fix: individual settings overwritten on editing locked fields of managed event #13297

Closed

1 task

Extract delete dialog to its own function to optimise rerender

7127bc0

vercel bot deployed to Preview – api January 19, 2024 15:58 View deployment

vercel bot deployed to Preview – dev January 19, 2024 16:11 View deployment

sean-brydon previously requested changes Jan 20, 2024

View reviewed changes

apps/web/pages/event-types/[type]/index.tsx Outdated Show resolved Hide resolved

packages/lib/isEqual.ts Outdated Show resolved Hide resolved

alishaz-polymath requested a review from emrysal February 14, 2024 11:25

add shouldDirty when resetting hosts after changing schedulingType

cff2871

CarinaWolli reviewed Feb 14, 2024

View reviewed changes

vercel bot deployed to Preview – dev February 14, 2024 21:36 View deployment

vercel bot deployed to Preview – api February 14, 2024 21:39 View deployment

vercel bot had a problem deploying to Preview – ai February 14, 2024 21:44 Failure

Merge branch 'main' into chore/only-update-dirty-fields-eventtype

a4f0533

vercel bot deployed to Preview – ai February 15, 2024 10:54 View deployment

vercel bot deployed to Preview – api February 15, 2024 11:00 View deployment

vercel bot deployed to Preview – dev February 15, 2024 11:03 View deployment

alishaz-polymath closed this Feb 15, 2024

alishaz-polymath reopened this Feb 15, 2024

emrysal reviewed Feb 15, 2024

View reviewed changes

add missing shouldDirty to useEventTypeDestinationCalendarEmail

a3d87ff

CarinaWolli previously approved these changes Feb 15, 2024

View reviewed changes

CarinaWolli self-requested a review February 15, 2024 16:24

vercel bot deployed to Preview – api February 15, 2024 16:24 View deployment

vercel bot deployed to Preview – dev February 15, 2024 16:32 View deployment

keithwillcode modified the milestones: v3.8, v3.9 Feb 15, 2024

emrysal enabled auto-merge (squash) February 15, 2024 22:43

emrysal approved these changes Feb 15, 2024

View reviewed changes

emrysal merged commit ee286a6 into main Feb 15, 2024
38 checks passed

emrysal deleted the chore/only-update-dirty-fields-eventtype branch February 15, 2024 22:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Only send dirty fields for event type update call #13292

refactor: Only send dirty fields for event type update call #13292

alishaz-polymath commented Jan 18, 2024 •

edited

Loading

vercel bot commented Jan 18, 2024 •

edited

Loading

github-actions bot commented Jan 18, 2024 •

edited

Loading

github-actions bot commented Jan 18, 2024 •

edited

Loading

deploysentinel bot commented Jan 18, 2024 •

edited

Loading

Test Case Results

Test Case Results

Test Case Results

Test Case Results

Test Case Results

Top 1 Common Error Messages

Test Case Results

Test Case Results

sean-brydon left a comment

alishaz-polymath commented Feb 14, 2024

CarinaWolli Feb 14, 2024

alishaz-polymath Feb 15, 2024

CarinaWolli commented Feb 14, 2024

alishaz-polymath commented Feb 15, 2024

alishaz-polymath commented Feb 15, 2024

emrysal left a comment

CarinaWolli left a comment

CarinaWolli Feb 15, 2024

emrysal left a comment

refactor: Only send dirty fields for event type update call #13292

refactor: Only send dirty fields for event type update call #13292

Conversation

alishaz-polymath commented Jan 18, 2024 • edited Loading

What does this PR do?

Requirement/Documentation

Type of change

How should this be tested?

Mandatory Tasks

Checklist

vercel bot commented Jan 18, 2024 • edited Loading

github-actions bot commented Jan 18, 2024 • edited Loading

github-actions bot commented Jan 18, 2024 • edited Loading

📦 Next.js Bundle Analysis for @calcom/web

deploysentinel bot commented Jan 18, 2024 • edited Loading

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

Test Case Results

Test Case Results

Test Case Results

Top 1 Common Error Messages

Test Case Results

Test Case Results

sean-brydon left a comment

Choose a reason for hiding this comment

alishaz-polymath commented Feb 14, 2024

CarinaWolli Feb 14, 2024

Choose a reason for hiding this comment

alishaz-polymath Feb 15, 2024

Choose a reason for hiding this comment

CarinaWolli commented Feb 14, 2024

alishaz-polymath commented Feb 15, 2024

alishaz-polymath commented Feb 15, 2024

emrysal left a comment

Choose a reason for hiding this comment

CarinaWolli left a comment

Choose a reason for hiding this comment

CarinaWolli Feb 15, 2024

Choose a reason for hiding this comment

emrysal left a comment

Choose a reason for hiding this comment

alishaz-polymath commented Jan 18, 2024 •

edited

Loading

vercel bot commented Jan 18, 2024 •

edited

Loading

github-actions bot commented Jan 18, 2024 •

edited

Loading

github-actions bot commented Jan 18, 2024 •

edited

Loading

deploysentinel bot commented Jan 18, 2024 •

edited

Loading