-
Notifications
You must be signed in to change notification settings - Fork 233
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
Reproduce and fix issues related to RECURRENCE-ID
#659
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## main #659 +/- ##
===================================
Coverage 61% 61%
===================================
Files 99 99
Lines 4559 4561 +2
Branches 1083 1085 +2
===================================
+ Hits 2791 2801 +10
+ Misses 1302 1296 -6
+ Partials 466 464 -2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tnx
…to `RECURRENCE-ID`
…date/time, not only to the date.
…ences that replace others from the result set, we missed those outside the observed range, which is fixed by this change.
…t), which was lost due to prior code optimizations.
79cb5b3
to
287fcfc
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine, tnx
Thank you for the review! |
This PR simplifies the handling of
RECURRENCE-ID
and by doing so fixes two bugs:RECURRENCE-ID
and therefore replace others was constructed from the result set insideGetOccurrences
. So if they were lying outside the observed range, they were missed, which could cause incorrect result sets.