Skip to content

feat: Calendar Cache#11185

Merged
zomars merged 12 commits into
mainfrom
feat/gcal-availability-cache
Sep 14, 2023
Merged

feat: Calendar Cache#11185
zomars merged 12 commits into
mainfrom
feat/gcal-availability-cache

Conversation

@zomars
Copy link
Copy Markdown
Contributor

@zomars zomars commented Sep 6, 2023

What does this PR do?

  • Implements a basic third-party calendar cache that lasts for a minute (for v1)
  • First call will be as slow as now, but subsequent calls should be nearly instant.
  • Migrates gCal CalendarService from callbacks to async/awaits

Possible downsides

  • If a profile is a high traffic one, we might run into a race condition where the cache is de-synced with the actual availability and get a "Could not book the meeting." error.

TODO

  • Implement a cron for cleanup expired caches
  • Implement an opt-in feature mechanism
  • Implement a way to bypass cache altogether (either via request header or search parameter)
  • Make the cache more aggressive (save full months, even if only a day was requested)

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How should this be tested?

  • Run migrations
  • Install Google Calendar
  • Enable some calendars
  • As an ADMIN, enable the calendar-cache feature flag
  • Go to a booking page
  • Check if CalendarCache table is being populated correctly
  • Check availability works as normal

Mandatory Tasks

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

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 6, 2023

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 Sep 14, 2023 7:54pm
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 14, 2023 7:54pm
cal 🔄 Building (Inspect) Visit Preview 💬 Add feedback Sep 14, 2023 7:54pm
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 14, 2023 7:54pm
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 14, 2023 7:54pm
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
qa ⬜️ Ignored (Inspect) Visit Preview Sep 14, 2023 7:54pm
ui ⬜️ Ignored (Inspect) Visit Preview Sep 14, 2023 7:54pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 6, 2023

Thank you for following the naming conventions! 🙏

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 6, 2023

📦 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
Copy link
Copy Markdown

deploysentinel Bot commented Sep 6, 2023

Current Playwright Test Results Summary

✅ 121 Passing - ⚠️ 2 Flaky

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

(Last updated on 09/14/2023 07:57:17pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 56440b2

Started: 09/14/2023 07:53:42pm UTC

⚠️ Flakes

📄   apps/web/playwright/login.2fa.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
2FA Tests should allow a user to enable 2FA and login using 2FA
Retry 1Initial Attempt
1.70% (5) 5 / 294 runs
failed over last 7 days
30.27% (89) 89 / 294 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 1Initial Attempt
1.95% (6) 6 / 307 runs
failed over last 7 days
96.42% (296) 296 / 307 runs
flaked over last 7 days

View Detailed Build Results


@PeerRich
Copy link
Copy Markdown
Member

PeerRich commented Sep 6, 2023

ouhh exciting

@emrysal
Copy link
Copy Markdown
Contributor

emrysal commented Sep 7, 2023

The problem is that the current bottleneck is not the calendars - the bottleneck is the database right now 😓

alannnc
alannnc previously approved these changes Sep 14, 2023
Copy link
Copy Markdown
Contributor

@alannnc alannnc left a comment

Choose a reason for hiding this comment

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

Code looks good! Left some NIT changes. Still since it's moving important calendar service methods we still need to test it.

@alannnc
Copy link
Copy Markdown
Contributor

alannnc commented Sep 14, 2023

Code looks good! Left some NIT changes. Still since it's moving important calendar service methods we still need to test it.

Update: and nows need conflict resolving.

# Conflicts:
#	packages/app-store/googlecalendar/lib/CalendarService.ts
Copy link
Copy Markdown
Contributor Author

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Self review done

});

if (event && event.data.id && event.data.hangoutLink) {
await calendar.events.patch({
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.

There was a missing await here.

calendar.events.patch({
// Update the same event but this time we know the hangout link
calendarId: selectedCalendar,
auth: myGoogleAuth,
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.

calendar is already authed

Comment thread apps/web/vercel.json
Comment thread packages/app-store/googlecalendar/lib/CalendarService.ts
Comment thread packages/app-store/googlecalendar/lib/CalendarService.test.ts Outdated
Copy link
Copy Markdown
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

It looks good to me :)

@zomars zomars merged commit c95917c into main Sep 14, 2023
@zomars zomars deleted the feat/gcal-availability-cache branch September 14, 2023 22:19
ashwintelmore pushed a commit to ashwintelmore/cal.com that referenced this pull request Sep 19, 2023
aar2dee2 pushed a commit to rkreddy99/cal.com that referenced this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only High priority Created by Linear-GitHub Sync ❗️ migrations contains migration files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants