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: ics compat with older ical specced calendars #19582

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

emrysal
Copy link
Contributor

@emrysal emrysal commented Feb 26, 2025

What does this PR do?

  • Remove Dayjs dependency (.utc conversion)
  • Better TS support
  • Fixed various tests; title/subtitle are invalid parameters so this test may not have been running for some time.
  • Removed seconds from the generated .ics file, .ics does not have second accuracy.

@emrysal emrysal changed the title Bugfix/ics compat with older ical specced calendars fix: ics compat with older ical specced calendars Feb 26, 2025
@graphite-app graphite-app bot requested a review from a team February 26, 2025 17:09
@keithwillcode keithwillcode added core area: core, team members only foundation labels Feb 26, 2025
@dosubot dosubot bot added the 🐛 bug Something isn't working label Feb 26, 2025
Copy link

graphite-app bot commented Feb 26, 2025

Graphite Automations

"Add foundation team as reviewer" took an action on this PR • (02/26/25)

1 reviewer was added to this PR based on Keith Williams's automation.

Copy link

vercel bot commented Feb 26, 2025

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

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Feb 26, 2025 5:14pm
calcom-web-canary ⬜️ Ignored (Inspect) Feb 26, 2025 5:14pm

Copy link
Contributor

E2E results are ready!

@emrysal emrysal enabled auto-merge (squash) February 26, 2025 17:58
startInputType: "utc",
productId: "calcom/ics",
title: event.title,
description: getRichDescription(event, t),
duration: { minutes: dayjs(event.endTime).diff(dayjs(event.startTime), "minute") },
Copy link
Member

Choose a reason for hiding this comment

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

Is duration isnt' needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, dtend takes it's place, duration is newer spec that allows for dynamic duration which we don't need :)

Copy link
Member

@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.

Overall LGTM!

@emrysal emrysal merged commit 48b8afd into main Feb 26, 2025
62 of 103 checks passed
@emrysal emrysal deleted the bugfix/ics-compat-with-older-ical-specced-calendars branch February 26, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working core area: core, team members only foundation ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants