Skip to content

Caldav/refactor: Mutiple Fixes and General code improvement#8031

Merged
emrysal merged 18 commits intomainfrom
caldav/refactor
Apr 12, 2023
Merged

Caldav/refactor: Mutiple Fixes and General code improvement#8031
emrysal merged 18 commits intomainfrom
caldav/refactor

Conversation

@alishaz-polymath
Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath commented Mar 30, 2023

What does this PR do?

Refactoring the CalDAV and Apple Calendar code to improve overall stability and support for various caldav services

  • Handle failure better so that it doesn't stop the conflict checks for other working integrations
  • Improves the Sanitization function to match the colons, semicolons, and equal signs only when they are followed by whitespace and then immediately followed by another colon, semicolon, or equal sign on the next line. This is an improvement on the current sanitizer function making it broader spectrum and avoiding errors that caused Shared Apple Calendar (other's ownership) and a few other CalDAV service providers to fail. In general, this should allow support for more CalDAV service providers.
  • We handle the expanded recurring events now, which should block off all the occurrences of the expanded recurring event in the given timeframe
  • Some CalDAV service providers fails silently when we use EXPAND property in the fetching of the Calendar Objects (eg. Zoho) and they do this in the back-end. Since there is no way to know for sure if EXPAND property is supported or not, we first send the request with the EXPAND property and then if that returns empty (silent failure) we make a call without the EXPAND property.
  • We assume that any ICAL object returned without any extension is a .ics file and treats it as such. This adds support to the Service Providers which send valid ICAL objects without an extension.

Fixes #7796 #7570 #5604 #5238 #8098 #8166

To follow up:

  • Improve the timezone registration when timezone is missing
  • DST compatibility
  • Tests when adding a CalDAV integration

Follow ups issues to fix: #5570

Environment: Staging(main branch)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)

How should this be tested?

  • Connect with a shared Apple Calendar with someone else's ownership. It should now work.
  • Connect a failed integration. It shouldn't stop working calendar integrations from conflict checking anymore.
  • Tested Support for various CalDAV integrations from Vates (Bluemind) to Qloud MSP (NextCloud) and Zoho. Although, reschedule in Zoho isn't working but that's on their end, and the issue is being checked by their team.

Checklist

  • I haven't performed a self-review of my own code and corrected any misspellings
  • I haven't added tests that prove my fix is effective or that my feature works

@alishaz-polymath alishaz-polymath requested a review from a team March 30, 2023 16:58
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 30, 2023

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

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 12, 2023 1:08pm
web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 12, 2023 1:08pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Apr 12, 2023 1:08pm

@alishaz-polymath alishaz-polymath marked this pull request as draft March 30, 2023 16:58
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 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 Mar 30, 2023

Current Playwright Test Results Summary

✅ 52 Passing - ⚠️ 2 Flaky

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

(Last updated on 04/12/2023 01:24:11pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 16f0b3c

Started: 04/12/2023 01:17:18pm UTC

⚠️ Flakes

📄   apps/web/playwright/wipe-my-cal.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Wipe my Cal App Test Browse upcoming bookings and validate button shows and triggering wipe my cal button
Retry 1Initial Attempt
0% (0) 0 / 207 runs
failed over last 7 days
0.97% (2) 2 / 207 runs
flaked over last 7 days

📄   apps/web/playwright/embed-code-generator.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Embed Code Generator Tests Event Type Edit Page open Embed Dialog for the Event Type
Retry 2Retry 1Initial Attempt
4.05% (9) 9 / 222 runs
failed over last 7 days
22.52% (50) 50 / 222 runs
flaked over last 7 days

View Detailed Build Results


@github-actions
Copy link
Copy Markdown
Contributor

📦 Next.js Bundle Analysis for @calcom/web

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

One Page Changed Size

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

Page Size (compressed) First Load % of Budget (350 KB)
/auth/setup 79.95 KB 313 KB 89.43% (🟢 -0.19%)
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.

@github-actions
Copy link
Copy Markdown
Contributor

📦 Next.js Bundle Analysis for @calcom/web

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

Forty-three 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)
/apps 165.04 KB 398.09 KB 113.74% (🟡 +1.06%)
/apps/[slug] 186.66 KB 419.71 KB 119.92% (🟡 +1.06%)
/apps/[slug]/[...pages] 390.55 KB 623.6 KB 178.17% (🟡 +0.84%)
/apps/categories 147.76 KB 380.81 KB 108.80% (🟡 +1.06%)
/apps/categories/[category] 151.7 KB 384.75 KB 109.93% (🟡 +1.06%)
/apps/installed/[category] 195.05 KB 428.1 KB 122.31% (🟡 +1.06%)
/availability 153.3 KB 386.35 KB 110.39% (🟡 +1.06%)
/availability/[schedule] 266.93 KB 499.98 KB 142.85% (🟡 +1.06%)
/availability/troubleshoot 148.45 KB 381.5 KB 109.00% (🟡 +1.06%)
/bookings/[status] 271.34 KB 504.39 KB 144.11% (🟡 +1.06%)
/event-types 347.67 KB 580.71 KB 165.92% (🟡 +1.06%)
/event-types/[type] 381.79 KB 614.84 KB 175.67% (🟡 +0.97%)
/insights 383.9 KB 616.95 KB 176.27% (🟡 +1.06%)
/more 147.39 KB 380.44 KB 108.70% (🟡 +1.06%)
/settings/admin 152.84 KB 385.89 KB 110.26% (🟡 +1.06%)
/settings/admin/apps 164.03 KB 397.08 KB 113.45% (🟡 +1.06%)
/settings/admin/apps/[category] 164.02 KB 397.07 KB 113.45% (🟡 +1.06%)
/settings/admin/flags 155.74 KB 388.79 KB 111.08% (🟡 +1.06%)
/settings/admin/impersonation 153.13 KB 386.18 KB 110.34% (🟡 +1.06%)
/settings/billing 152.96 KB 386.01 KB 110.29% (🟡 +1.06%)
/settings/developer/api-keys 182.09 KB 415.14 KB 118.61% (🟡 +1.06%)
/settings/developer/webhooks 155.61 KB 388.66 KB 111.04% (🟡 +1.06%)
/settings/developer/webhooks/[id] 186.23 KB 419.28 KB 119.79% (🟡 +1.06%)
/settings/developer/webhooks/new 186.09 KB 419.14 KB 119.75% (🟡 +1.06%)
/settings/my-account/appearance 167.6 KB 400.65 KB 114.47% (🟡 +1.06%)
/settings/my-account/calendars 187.13 KB 420.18 KB 120.05% (🟡 +1.06%)
/settings/my-account/conferencing 158.75 KB 391.8 KB 111.94% (🟡 +1.06%)
/settings/my-account/general 261.95 KB 495 KB 141.43% (🟡 +1.06%)
/settings/my-account/profile 270.42 KB 503.47 KB 143.85% (🟡 +1.06%)
/settings/security/impersonation 154.97 KB 388.02 KB 110.86% (🟡 +1.06%)
/settings/security/password 190.4 KB 423.45 KB 120.99% (🟡 +1.05%)
/settings/security/sso 162.68 KB 395.73 KB 113.07% (🟡 +1.06%)
/settings/security/two-factor-auth 157.49 KB 390.54 KB 111.58% (🟡 +1.06%)
/settings/teams 152.59 KB 385.64 KB 110.18% (🟡 +1.06%)
/settings/teams/[id]/appearance 167.45 KB 400.5 KB 114.43% (🟡 +1.06%)
/settings/teams/[id]/billing 152.83 KB 385.88 KB 110.25% (🟡 +1.06%)
/settings/teams/[id]/members 293.53 KB 526.58 KB 150.45% (🟡 +1.06%)
/settings/teams/[id]/profile 265.12 KB 498.17 KB 142.33% (🟡 +1.06%)
/settings/teams/[id]/sso 162.77 KB 395.82 KB 113.09% (🟡 +1.06%)
/settings/teams/new 92.5 KB 325.55 KB 93.02% (🟡 +0.97%)
/teams 147.61 KB 380.66 KB 108.76% (🟡 +1.06%)
/workflows 160.6 KB 393.65 KB 112.47% (🟡 +1.06%)
/workflows/[workflow] 290.32 KB 523.37 KB 149.53% (🟡 +1.06%)
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.

@alishaz-polymath alishaz-polymath changed the title Caldav/refactor --WIP Caldav/refactor: Mutiple Fixes and General code improvement Apr 11, 2023
@alishaz-polymath alishaz-polymath requested a review from leog April 11, 2023 10:57
@alishaz-polymath alishaz-polymath marked this pull request as ready for review April 11, 2023 10:57
Copy link
Copy Markdown
Member Author

@alishaz-polymath alishaz-polymath 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 added

function getFileExtension(url: string): string {
// Return null if the URL does not have a file extension
if (!hasFileExtension(url)) return null;
if (!hasFileExtension(url)) return "ics";
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We assume that an extensionless object is of the .ics extension as is the case for most if not all Calendar Objects

Comment on lines +317 to +319
try {
const jcalData = ICAL.parse(sanitizeCalendarObject(object));
vcalendar = new ICAL.Component(jcalData);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We place ICAL.parse in a try-catch as .parse function throws error when the object is not of iCalendar 2.0 standard, and it ends up breaking the working calendars as well. The try-catch fails it without breaking the flow.

return;
// const vevent = vcalendar.getFirstSubcomponent("vevent");
const vevents = vcalendar.getAllSubcomponents("vevent");
vevents.forEach((vevent) => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

A single Calendar Object might contain multiple events when we're talking about expanded recurring events in particular. Placing them in a forEach ensures we don't miss the next events. This fixes the earlier issue where only the first occurrence of a recurring event was being considered for the freebusy check.

@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

📦 Next.js Bundle Analysis for @calcom/web

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

Two 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)
/apps/[slug]/[...pages] 387.8 KB 620.6 KB 177.31% (🟡 +0.15%)
/auth/setup 80.87 KB 313.67 KB 89.62% (🟡 +0.19%)
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.

@emrysal emrysal enabled auto-merge (squash) April 12, 2023 15:15
Copy link
Copy Markdown
Contributor

@leog leog 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, approving.

@emrysal emrysal merged commit a8368aa into main Apr 12, 2023
@emrysal emrysal deleted the caldav/refactor branch April 12, 2023 17:30
@PeerRich PeerRich added the core area: core, team members only label Jul 24, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Conflict checks stopped working [CAL-390] Meetings in Apple calendar are offset when calculating available time slots

4 participants