Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix availability free-busy data pull, location change and status in CALDAV | new branch (attempt#2) #3958

Merged
merged 1 commit into from
Aug 25, 2022

Conversation

alishaz-polymath
Copy link
Member

What does this PR do?

  • Fixes the incompatibility between the ICALstring returned in the calendar objects by TSDAV's fetchCalendarObjects and the expected input from ICAL.parse()
  • Adds a sanitizer for calendar object data retrieved by TSdav for further processing
  • Fixes Location Change reflecting on the connected CalDAV
  • Fixes Reschedule change reflecting on the connected CalDAV
  • Fixes Freebusy data consideration to allow booking over events with 'TRANSPARENT' (free) status

Fixes #3589 #2988 #2236 #1588

Environment: Staging(main branch)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • 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

@alishaz-polymath alishaz-polymath added 🐛 bug Something isn't working ♻️ autoupdate tells kodiak to keep this branch up-to-date labels Aug 25, 2022
@alishaz-polymath alishaz-polymath self-assigned this Aug 25, 2022
@vercel
Copy link

vercel bot commented Aug 25, 2022

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

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Aug 25, 2022 at 5:24AM (UTC)
1 Ignored Deployment
Name Status Preview Updated
swagger ⬜️ Ignored (Inspect) Aug 25, 2022 at 5:24AM (UTC)

Copy link
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.

Added Self review

@@ -251,9 +251,13 @@ export default abstract class BaseCalendarService implements Calendar {
objects.forEach((object) => {
if (object.data == null) return;

const jcalData = ICAL.parse(object.data);
const jcalData = ICAL.parse(sanitizeCalendarObject(object));
Copy link
Member Author

Choose a reason for hiding this comment

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

Depending on the CalDAV service provider, the object.data may or may not have \r or \r\n as the EOL. ICAL.parse expects it to have '\r\n' to work as expected. Also, the object.data received from the fetchCalendarObjects function is at times faulty in placing the EOL, which is why we need to make sure that it doesn't break the structure of expected input for ICAL.parse, hence the need for sanitization.

const vcalendar = new ICAL.Component(jcalData);
const vevent = vcalendar.getFirstSubcomponent("vevent");

// if event status is free or transparent, return
if (vevent?.getFirstPropertyValue("transp") === "TRANSPARENT") return;
Copy link
Member Author

Choose a reason for hiding this comment

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

If an event is added as a Free event, meaning we should allow booking over it, this is how we identify the free state and ignore any further processing

Comment on lines +5 to +9
.replaceAll("\r\n", "\r")
.replaceAll("\r", "\r\n")
.replaceAll(/(: \r\n|:\r\n|\r\n:|\r\n :)/gm, ":")
.replaceAll(/(; \r\n|;\r\n|\r\n;|\r\n ;)/gm, ";")
.replaceAll(/(= \r\n|=\r\n|\r\n=|\r\n =)/gm, "=");
Copy link
Member Author

Choose a reason for hiding this comment

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

We make sure the ICAL.parse works with the output of this return. Currently, this seems to be fixing all potential invalid breakpoints but this can be revisited and improved if there are other edge cases we might have missed here.

Comment on lines +5 to +6
.replaceAll("\r\n", "\r")
.replaceAll("\r", "\r\n")
Copy link
Member Author

Choose a reason for hiding this comment

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

The reason we're going from \r\n to \r and then back from \r to \r\n here is because some of the CalDAV services return the EOL as \r while others as \r\n and the ICAL.parse expects it to have \r\n as EOL, so this just ensures all the returned Calendar objects have \r\n as EOL.

Copy link
Member

Choose a reason for hiding this comment

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

Just a nitpick replaceAll is not supported on browsers. And it is generally deprecated.

@hariombalhara hariombalhara merged commit 7f94ce0 into main Aug 25, 2022
@hariombalhara hariombalhara deleted the fix/caldav-002-alt branch August 25, 2022 09:18
@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
♻️ autoupdate tells kodiak to keep this branch up-to-date 🐛 bug Something isn't working core area: core, team members only
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Availability fetching is bugged for CalDAV
4 participants