Skip to content

fix: Use Google recurring events protocol#12651

Merged
hariombalhara merged 16 commits intocalcom:mainfrom
haranrk:11655_recurring_events
Jan 16, 2024
Merged

fix: Use Google recurring events protocol#12651
hariombalhara merged 16 commits intocalcom:mainfrom
haranrk:11655_recurring_events

Conversation

@haranrk
Copy link
Copy Markdown
Contributor

@haranrk haranrk commented Dec 3, 2023

What does this PR do?

Uses the native recurring cal API of google calendar to make recurring bookings.
Features -

  • Cancel all remaining events - will cancel the entire recurring event set with one API call
  • Modifying a booking will only change that instance of the recurring event
  • Creating a new event will create the recurring events with one API call

Partially fixes #11655 as only the support for Google Calendar is added. Though it should be quick to support other Calendars.

https://www.loom.com/share/e66dad98c4a54d229b480bebf15b074f

Requirement/Documentation

Previous recurring event flow

  • recurring-event ->
    • For every event
      • run handleNewBooking -> eventManager -> calendarManager -> googleCalendarService
      • check if the user is available during the proposed event time (this would mean that even if the user is unavailable for a particular instance of the recurring event, only that instance will not be created, but the other instances will)
      • a new standalone event is created

New recurring event flow

  • recurring-event ->
    • Check if the user is available for every instance of the proposed recurring event
    • For every event
      • run handleNewBooking -> eventManager -> calendarManager -> googleCalendarService
      • in the first iteration of the loop, a recurring event on gcal is created. this would create all the instances on gcal.
      • subsequent iterations of the loop do all the additional cal.com housekeeping necessary like creating videoconferencing links, making new cal.com db records etc.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Mandatory Tasks

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

Checklist

@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 3, 2023

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

A member of the Team first needs to authorize it.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Dec 3, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions github-actions Bot added the ❗️ migrations contains migration files label Dec 3, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 3, 2023

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

@emrysal emrysal changed the title 11655 recurring events fix: 11655 recurring events Dec 3, 2023
@emrysal emrysal added the osshack Submission for 2023 OSShack label Dec 3, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 3, 2023

📦 Next.js Bundle Analysis for @calcom/web

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

Five Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load % of Budget (350 KB)
/auth/oauth2/authorize 117.36 KB 282.13 KB 80.61% (🟢 -0.17%)
/getting-started/[[...step]] 403.93 KB 568.7 KB 162.49% (🟢 -0.17%)
/settings/teams/[id]/members 380.4 KB 545.17 KB 155.76% (🟡 +0.26%)
/settings/teams/[id]/onboard-members 157.11 KB 321.88 KB 91.96% (🟢 -0.17%)
/settings/teams/new 189.78 KB 354.55 KB 101.30% (🟢 -0.17%)
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored.

@haranrk haranrk marked this pull request as ready for review December 3, 2023 17:49
@keithwillcode keithwillcode changed the title fix: 11655 recurring events fix: Use Google recurring events protocol Dec 3, 2023
@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 3, 2023

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

Name Status Preview Comments Updated (UTC)
dev ❌ Failed (Inspect) Dec 3, 2023 6:50pm

@CarinaWolli CarinaWolli self-requested a review December 4, 2023 19:29
@PeerRich PeerRich added Medium priority Created by Linear-GitHub Sync 🚨 needs approval This feature request has not been reviewed yet by the Product Team and needs approval beforehand labels Dec 4, 2023
@CarinaWolli
Copy link
Copy Markdown
Member

For every event of the recurring booking a new recurring google calendar event is created for me.

So I end up with a ton of calendar events in my google calendar:
Screenshot 2023-12-05 at 5 18 40 PM

@haranrk I don't see this happening in your loom. Any idea why this is happening?

@haranrk
Copy link
Copy Markdown
Contributor Author

haranrk commented Dec 5, 2023

@CarinaWolli, I just pulled the latest changes and tested it on my end and it worked correctly. Can you give the details of the event you created?

In the interest of speed, would it be possible to get on a video conferencing call with you to identify the bug? https://cal.com/haranrk/30min

Comment thread apps/web/pages/api/book/recurring-event.ts Outdated
Comment thread apps/web/pages/api/book/recurring-event.ts Outdated
Comment thread apps/web/pages/api/book/recurring-event.ts Outdated
Comment thread apps/web/tsconfig.json Outdated
Comment thread packages/app-store/googlecalendar/lib/CalendarService.ts
Comment thread packages/app-store/googlecalendar/lib/CalendarService.ts Outdated
Comment thread packages/app-store/googlecalendar/lib/CalendarService.ts
Comment thread packages/app-store/googlecalendar/lib/CalendarService.ts Outdated
Comment thread packages/prisma/schema.prisma Outdated
Comment thread packages/features/bookings/lib/handleCancelBooking.ts Outdated
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @haranrk 🙏 Left some comments

@haranrk
Copy link
Copy Markdown
Contributor Author

haranrk commented Dec 6, 2023

@CarinaWolli @hariombalhara made the changes, PTAL

@haranrk haranrk force-pushed the 11655_recurring_events branch from 9b9c17b to 917a71d Compare January 4, 2024 22:12
@github-actions github-actions Bot added app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar app-store-template Created by Linear-GitHub Sync calendar-apps area: calendar, google calendar, outlook, lark, microsoft 365, apple calendar High priority Created by Linear-GitHub Sync recurring-events labels Jan 4, 2024
@github-actions github-actions Bot removed the Stale label Jan 5, 2024
@haranrk
Copy link
Copy Markdown
Contributor Author

haranrk commented Jan 5, 2024

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara

PTAL at these and let me know how you would like me to resolve them

#12651 (files) #12651 (files)

Replied inline

recurringEventReq,
{
isNotAnApiCall: true,
skipAvailabilityCheck: key >= numSlotsToCheckForAvailability || thirdPartyRecurringEventId !== null,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I wanted to point out that for the calendar apps that don't implement recurring api(all except Google Calendar) at the moment, thirdPartyRecurringEventId comes as undefined which is not really a value but skipAvailabilityCheck is correctly calculated because null !== undefined.

We should explicitly handle this.


createdBookings.push(eachRecurringBooking);

if (!thirdPartyRecurringEventId) {
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition would be truthy here for every iteration of non-recurring event supported calendars

@PeerRich PeerRich removed the 🚨 needs approval This feature request has not been reviewed yet by the Product Team and needs approval beforehand label Jan 8, 2024
Copy link
Copy Markdown
Contributor Author

@haranrk haranrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hariombalhara I fixed the bug and rewrote the code to make it more easy to read. Since, the skipAvailabilityCheck is not being used anywhere else, I removed it.

availableUsers.filter((user) => user.isFixed).length !== users.filter((user) => user.isFixed).length
) {
throw new Error(ErrorCode.HostsUnavailableForBooking);
if (!req.body.allRecurringDates || req.body.isFirstRecurringSlot) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keithwillcode keithwillcode added this to the v3.7 milestone Jan 10, 2024
@keithwillcode keithwillcode added the community Created by Linear-GitHub Sync label Jan 11, 2024
@keithwillcode keithwillcode modified the milestones: v3.7, v3.8 Jan 15, 2024
Comment on lines +1242 to +1243
if (req.body.allRecurringDates) {
if (req.body.isFirstRecurringSlot) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (req.body.allRecurringDates) {
if (req.body.isFirstRecurringSlot) {
if (req.body.allRecurringDates && req.body.isFirstRecurringSlot) {

@hariombalhara hariombalhara merged commit 0a7f65c into calcom:main Jan 16, 2024
@hariombalhara
Copy link
Copy Markdown
Member

It's merged 🎉Thanks @haranrk 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar app-store-template Created by Linear-GitHub Sync calendar-apps area: calendar, google calendar, outlook, lark, microsoft 365, apple calendar community Created by Linear-GitHub Sync ❗️ .env changes contains changes to env variables High priority Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync ❗️ migrations contains migration files osshack Submission for 2023 OSShack recurring-events

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CAL-2564] recurring events: use proper "recurring" API from calendar APIs

7 participants