recur_expansion: EXDATE can be DATE and DTSTART can be DATE-TIME #670
Conversation
dilyanpalauzov
force-pushed
the
recur_expansion_exdate
branch
from
3655794 to
be9214c
Compare
Pull Request Test Coverage Report for Build 9821264905Details
💛 - Coveralls |
| if (!a.isDate && b.isDate) | ||
| return new Time({ year: a.year, month: a.month, day: a.day }).compare(b); |
There was a problem hiding this comment.
This also strips out the timezone, which might affect the comparison. Can you add zone handling here?
There was a problem hiding this comment.
A test or two around this case and zone handling would also be useful.
There was a problem hiding this comment.
I have added a test.
I am not going to add time zone handling, as I do not understand how it would work. This patch fixes a problem I faced in 2022. I do not work with iCalendar in JavaScript currently.
There was a problem hiding this comment.
This also strips out the timezone, which might affect the comparison. Can you add zone handling here?
The commit message contains:
When EXDATE is in DATE format and DTSTART is DATE-TIME, then to determine whether an occurrence shall be excluded, the occurrence shall be converted to DATE and then compared to EXDATE.
I would say in the conversion of VALUE=DATE-TIME to VALUE=DATE the timezone is irrelevant.
|
@dilyanpalauzov Did you have a chance to look into this? Would appreciate if you could take care of the review comments. |
dilyanpalauzov
force-pushed
the
recur_expansion_exdate
branch
2 times, most recently
from
1722292 to
184e9e9
Compare
When EXDATE is in DATE format and DTSTART is DATE-TIME, then to determine whether an occurrence shall be excluded, the occurrence shall be converted to DATE and then compared to EXDATE. This applies also for the very first EXDATE, when it coincides with DTSTART.
dilyanpalauzov
force-pushed
the
recur_expansion_exdate
branch
from
184e9e9 to
ac0ad56
Compare
|
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. |
|
?!?! |
When EXDATE is in DATE format and DTSTART is DATE-TIME, then to determine whether an occurrence shall be excluded, the occurrence shall be converted to DATE and then compared to EXDATE.
This applies also for the very first EXDATE, when it coincides with DTSTART.
I had this change locally since 2022. I have not uploaded it, as I have lost hope that PRs here will be handled.