Skip to content

Bugfix/recurring dst change#5172

Merged
zomars merged 8 commits intomainfrom
bugfix/recurring-dst-change
Oct 24, 2022
Merged

Bugfix/recurring dst change#5172
zomars merged 8 commits intomainfrom
bugfix/recurring-dst-change

Conversation

@emrysal
Copy link
Copy Markdown
Contributor

@emrysal emrysal commented Oct 24, 2022

This PR fixes the DST issues regarding recurring events.

Also, it fixes a couple of issues:

  1. "events remaining" on recurring events now reflect past events within the series

  2. Booking page was not applying user's preferences to display time

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 24, 2022

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

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Oct 24, 2022 at 10:34PM (UTC)

@leog
Copy link
Copy Markdown
Contributor

leog commented Oct 24, 2022

Hey @emrysal. I was actually working on replacing RRule.js with a dayjs plugin here: #5171. Maybe you could apply your changes to support DST in that PR to solve that issue and also we can remove the heavyweight dependency at the same time. WDYT?

@emrysal
Copy link
Copy Markdown
Contributor Author

emrysal commented Oct 24, 2022

@leog I had a look at dayjs-recur but I would say (looking at both repo's) that rrule is better. Recur is a dead project (last activity was in March) - and has a total of 8 stars. It also only supports a very limited subset of features compared to rrule - and also very importantly rrule is a port of the python project for recurring rules which is good for debugging (you can use python docs to learn about rrule.js features).

Rrule also has some bugs that need fixing but the performance issues we've been seeing can be resolved on our end. It's a much more stable project.

@leog
Copy link
Copy Markdown
Contributor

leog commented Oct 24, 2022

Ok, I agree, thanks @emrysal for the insight.

Added an extra seeded example for recurring
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.

Works great. Added 2 more relevant fixes brought from the closed PR #5171.

Copy link
Copy Markdown
Contributor

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

Ship it

@zomars zomars disabled auto-merge October 24, 2022 22:37
@zomars zomars merged commit 5305f31 into main Oct 24, 2022
@zomars zomars deleted the bugfix/recurring-dst-change branch October 24, 2022 22:37
Udit-takkar pushed a commit that referenced this pull request Oct 26, 2022
* Structural fix to recurring times

* Remove conversion regression

* Revert current time -> startTime based utcOffset

* Fixing remaining events count

* Using user's preference for recurring tooltip

* Missing refactor

* Showing better datetime in booking page
Added an extra seeded example for recurring

Co-authored-by: Leo Giovanetti <hello@leog.me>
haffla pushed a commit to tourlane/cal.com that referenced this pull request Nov 22, 2022
* Structural fix to recurring times

* Remove conversion regression

* Revert current time -> startTime based utcOffset

* Fixing remaining events count

* Using user's preference for recurring tooltip

* Missing refactor

* Showing better datetime in booking page
Added an extra seeded example for recurring

Co-authored-by: Leo Giovanetti <hello@leog.me>
haffla pushed a commit to tourlane/cal.com that referenced this pull request Nov 22, 2022
* Structural fix to recurring times

* Remove conversion regression

* Revert current time -> startTime based utcOffset

* Fixing remaining events count

* Using user's preference for recurring tooltip

* Missing refactor

* Showing better datetime in booking page
Added an extra seeded example for recurring

Co-authored-by: Leo Giovanetti <hello@leog.me>
@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.

4 participants