fix: recurring events with calendar connections by CarinaWolli · Pull Request #12250 · calcom/cal.diy

CarinaWolli · 2023-11-06T19:33:26Z

What does this PR do?

Fixes that when booking a recurring event with a calendar connection it was always throwing 'no available users found':

This error was also thrown when user was available on all the recurring dates.

Type of change

Bug fix (non-breaking change which fixes an issue)

How should this be tested?

Connect Google Calendar -> toggle same calendar to check for conflicts as the one that is set to add the events on
Create recurring event type
Book a slot were you know that all future slot are available too
This should pass now

Mandatory Tasks

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

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 PR needs changes to the documentation
I haven't checked if my changes generate no new warnings
I haven't added tests that prove my fix is effective or that my feature works
I haven't checked if new and existing unit tests pass locally with my changes

vercel · 2023-11-06T19:33:35Z

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	Nov 7, 2023 2:41pm
api	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 7, 2023 2:41pm
dev	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 7, 2023 2:41pm

4 Ignored Deployments

Name	Status	Preview	Updated (UTC)
cal	⬜️ Ignored (Inspect)	Visit Preview	Nov 7, 2023 2:41pm
cal-demo	⬜️ Ignored (Inspect)	Visit Preview	Nov 7, 2023 2:41pm
qa	⬜️ Ignored (Inspect)	Visit Preview	Nov 7, 2023 2:41pm
ui	⬜️ Ignored (Inspect)	Visit Preview	Nov 7, 2023 2:41pm

github-actions · 2023-11-06T19:33:38Z

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

github-actions · 2023-11-06T19:41:23Z

📦 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 · 2023-11-06T19:42:24Z

Current Playwright Test Results Summary

✅ 313 Passing - ⚠️ 7 Flaky

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

(Last updated on 11/07/2023 02:52:50pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: f61971a

Started: 11/07/2023 02:43:19pm UTC

⚠️ Flakes

📄 apps/web/playwright/event-types.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Event Types tests user can add multiple organizer address Retry 1 • Initial Attempt	0.44% (1) 1 / 228 run failed over last 7 days	10.09% (23) 23 / 228 runs flaked over last 7 days

📄 apps/web/playwright/teams.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
Teams - NonOrg Can create teams via Wizard Retry 1 • Initial Attempt	0% (0) 0 / 215 runs failed over last 7 days	1.40% (3) 3 / 215 runs flaked over last 7 days
Teams - NonOrg Non admin team members cannot create team in org Retry 1 • Initial Attempt	0% (0) 0 / 215 runs failed over last 7 days	40.47% (87) 87 / 215 runs flaked over last 7 days

📄 apps/web/playwright/insights.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
Insights should be able to go to insights as members Retry 1 • Initial Attempt	0% (0) 0 / 228 runs failed over last 7 days	4.82% (11) 11 / 228 runs flaked over last 7 days
Insights should test download button Retry 1 • Initial Attempt	0% (0) 0 / 228 runs failed over last 7 days	6.14% (14) 14 / 228 runs flaked over last 7 days

📄 packages/embeds/embed-core/playwright/tests/inline.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Inline Iframe Inline Iframe - Configured with Dark Theme Retry 1 • Initial Attempt	1.31% (3) 3 / 229 runs failed over last 7 days	28.38% (65) 65 / 229 runs flaked over last 7 days

📄 packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Popup Tests should be able to reschedule Retry 1 • Initial Attempt	20.51% (48) 48 / 234 runs failed over last 7 days	73.93% (173) 173 / 234 runs flaked over last 7 days

View Detailed Build Results

hariombalhara · 2023-11-07T07:28:17Z

  const dateStrings = times.map((t) => {
    // finally; show in local timeZone again
-    return processDate(t.tz(timeZone), language, { selectedTimeFormat, withDefaultTimeFormat });
+    return processDate(t.tz(timeZone), language, { selectedTimeFormat, withDefaultTimeFormat }, timeZone);


What is the exact bug that this change fixes?

hariombalhara · 2023-11-07T08:37:55Z

+  date: string | null | Dayjs,
+  language: string,
+  options?: ExtraOptions,
+  timeZone?: string


If we really need timezone to show the correct time here, shouldn't we make it required and fix everywhere it's missing.

hariombalhara · 2023-11-07T08:42:29Z

-      } else {
-        foundConflict = checkForConflicts(bufferedBusyTimes, input.dateFrom, duration);
-      }
+      foundConflict = checkForConflicts(bufferedBusyTimes, input.dateFrom, duration);


The change looks good. I was already removing it as part of https://github.com/calcom/cal.com/pull/11774/files

But the interesting thing is that I was thinking that it was dead code but actually it was failing some bookings.
Did some investigation as to how just enabling a calendar was failing Recurring bookings

When Calendar is enabled, we don't just fetch the busyTimes for the current slot being booked but instead we fetch till the end of the month(some weird reason for this). This isn't true for busyTimes fetched from DB. There it's just for the range of the timeslot being booked as expected. The behaviour changed here 5 months back. So, I believe it has been broken for 5 straight months now.

Due to this, when a single timeslot was checked for conflicts with busytimes, it was actually checking conflicts with busyTime of a recurring timeslot that was just booked in the previous iteration and thus it was always failing.

So, great find @CarinaWolli 👏

hariombalhara

Requesting changes to get clarity on timezone related change, otherwise it's good.

keithwillcode · 2023-11-07T09:28:21Z

Great find, @CarinaWolli. Any tests we could add to make sure this continues to work?

CarinaWolli · 2023-11-06T19:50:14Z

-        // DONE: Decreased computational complexity from O(2^n) to O(n) by refactoring this loop to stop
-        // running at the first unavailable time.
-        let i = 0;
-        while (!foundConflict && i < allBookingDates.length) {


The issue here was that every recurring date called the handler starting with with the last booking (see: book/recurring-event.ts)

So when the handler was called for the first booking (recurringDatesInfo?.currentRecurringIndex === 0) the calendar events for the other dates already existed and the checkForConflicts function found a conflict

CarinaWolli · 2023-11-06T19:58:57Z

      : options?.selectedTimeFormat || detectBrowserTimeFormat
  );
-  return `${formattedTime}, ${dayjs(date).toDate().toLocaleString(language, { dateStyle: "full" })}`;
+  return `${formattedTime}, ${dayjs(date)


Dates on booking page were shown incorrectly (day)

Example:
Should show Nov 8, 15 and 22. But shows:

CarinaWolli · 2023-11-07T13:34:51Z

Great find, @CarinaWolli. Any tests we could add to make sure this continues to work?

I don't think we have a setup yet for testing calendar connections. I think @joeauyeung is working on that. That would be needed to test this

keithwillcode · 2023-11-07T14:45:50Z

Great find, @CarinaWolli. Any tests we could add to make sure this continues to work?

I don't think we have a setup yet for testing calendar connections. I think @joeauyeung is working on that. That would be needed to test this

Ok. I thought by looking at the code it would be possible for us to unit test/mock the data this function receives, not necessarily coming from a real calendar connection.

We'll push this PR through to get the bug fixed but let's look into this possibility in a follow-up PR.

Reason for change requested was addressed

Co-authored-by: CarinaWolli <wollencarina@gmail.com>

CarinaWolli added 4 commits November 3, 2023 19:14

remove not needed if to check recurring conflicts

d8e4f54

show days in correct tz

123eb5c

Merge branch 'main' into fix/recurring-booking-improvements

1bb0c19

remove console log

21fd486

zomars added the core area: core, team members only label Nov 6, 2023

CarinaWolli marked this pull request as ready for review November 6, 2023 19:59

CarinaWolli requested a review from hariombalhara November 6, 2023 19:59

hariombalhara reviewed Nov 7, 2023

View reviewed changes

hariombalhara previously requested changes Nov 7, 2023

View reviewed changes

hariombalhara mentioned this pull request Nov 7, 2023

[CAL-2522] We need to have some solid reasons to write E2E for third party apps which might need more effort to maintain #11509

Closed

CarinaWolli commented Nov 7, 2023

View reviewed changes

CarinaWolli added 3 commits November 7, 2023 09:01

make timeZone mendatory

adbe7f8

add missing timeZone

3e15c68

fix required parameter cannot follow optional

ff70830

vercel Bot deployed to Preview – ai November 7, 2023 14:20 View deployment

vercel Bot deployed to Preview – dev November 7, 2023 14:21 View deployment

vercel Bot had a problem deploying to Preview – api November 7, 2023 14:21 Failure

fix type error

f61971a

vercel Bot deployed to Preview – ai November 7, 2023 14:38 View deployment

vercel Bot deployed to Preview – dev November 7, 2023 14:41 View deployment

vercel Bot deployed to Preview – api November 7, 2023 14:41 View deployment

keithwillcode added the High priority Created by Linear-GitHub Sync label Nov 7, 2023

keithwillcode added the bookings area: bookings, availability, timezones, double booking label Nov 7, 2023

keithwillcode self-requested a review November 7, 2023 14:45

keithwillcode approved these changes Nov 7, 2023

View reviewed changes

keithwillcode enabled auto-merge (squash) November 7, 2023 14:46

keithwillcode merged commit 8b89d66 into main Nov 7, 2023

keithwillcode deleted the fix/recurring-booking-improvements branch November 7, 2023 15:09

keithwillcode linked an issue Nov 11, 2023 that may be closed by this pull request

[CAL-2517] Recurring Event - Error while booking a meeting. #10673

Closed

zomars pushed a commit that referenced this pull request Jan 29, 2024

fix: recurring events with calendar connections (#12250)

629de80

Co-authored-by: CarinaWolli <wollencarina@gmail.com>

Conversation

CarinaWolli commented Nov 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Type of change

How should this be tested?

Mandatory Tasks

Checklist

Uh oh!

vercel Bot commented Nov 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Nov 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Nov 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Next.js Bundle Analysis for @calcom/web

Uh oh!

deploysentinel Bot commented Nov 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Top 1 Common Error Messages

Test Case Results

Top 1 Common Error Messages

Test Case Results

Test Case Results

Test Case Results

Uh oh!

hariombalhara Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

hariombalhara Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

hariombalhara Nov 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hariombalhara left a comment

Choose a reason for hiding this comment

Uh oh!

keithwillcode commented Nov 7, 2023

Uh oh!

CarinaWolli Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

keithwillcode Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

CarinaWolli Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

CarinaWolli commented Nov 7, 2023

Uh oh!

keithwillcode commented Nov 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CarinaWolli commented Nov 6, 2023 •

edited

Loading

vercel Bot commented Nov 6, 2023 •

edited

Loading

github-actions Bot commented Nov 6, 2023 •

edited

Loading

github-actions Bot commented Nov 6, 2023 •

edited

Loading

deploysentinel Bot commented Nov 6, 2023 •

edited

Loading

hariombalhara Nov 7, 2023 •

edited

Loading