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

Sort byparts before setting defaults in iterator #337

Closed
wants to merge 2 commits into from

Conversation

arunpandianp
Copy link
Contributor

https://github.com/mozilla-comm/ical.js/blob/master/lib/ical/recur_iterator.js#L1372
setup_defaults is returning the first element of bypart array, if the input bypart array are not sorted then the iteration is not always correct. In this PR i'm sorting the bypart arrays before calling setup_defaults. This also fixes #336

@coveralls
Copy link

coveralls commented Aug 19, 2017

Coverage Status

Coverage increased (+0.1%) to 97.073% when pulling 8289538 on arunpandianp:sortByParts into 0c50d3b on mozilla-comm:master.

@kewisch kewisch self-requested a review October 27, 2017 12:01
@kewisch
Copy link
Owner

kewisch commented Oct 6, 2019

Thanks, I think this makes sense. Can you rebase on master? There were some conflicts in recur_iterator.js

this.last.month = this.setup_defaults("BYMONTH", "MONTHLY", this.dtstart.month);

//setting day after month, as sorting BYMONTHDAY will differ based on the month
Copy link
Owner

Choose a reason for hiding this comment

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

If sorting bymonthday differs based on month, wouldn't we need to sort this differently every month while the rule is iteraed?

@kewisch kewisch added the needinfo More information has been requested label Oct 6, 2019
Copy link

github-actions bot commented Apr 2, 2024

It looks like we haven't heard back on this issue, therefore we are closing this issue. If this problem persists in the latest version of ical.js, please re-open this issue.

@github-actions github-actions bot closed this Apr 2, 2024
@github-actions github-actions bot removed the needinfo More information has been requested label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recurrence Missing instances if bymonthday is not sorted
3 participants