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.

Sign up for GitHub

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

Jump to bottom

recur_expansion: EXDATE can be DATE and DTSTART can be DATE-TIME #670

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

recur_expansion: EXDATE can be DATE and DTSTART can be DATE-TIME #670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions lib/ical/recur_expansion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,21 @@ class RecurExpansion {
}
}

/**
* Compare two ICAL.Time objects. When the second parameter is a timeless date
* and the first parameter is date-with-time, strip the time and compare only
* the days.
*
* @private
* @param {ICAL.Time} a The one object to compare
* @param {ICAL.Time} b The other object to compare
*/
_compare_special(a, b) {
if (!a.isDate && b.isDate)
return new Time({ year: a.year, month: a.month, day: a.day }).compare(b);
Comment on lines +211 to +212
Copy link
Owner

Choose a reason for hiding this comment

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

This also strips out the timezone, which might affect the comparison. Can you add zone handling here?

Copy link
Owner

Choose a reason for hiding this comment

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

A test or two around this case and zone handling would also be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

return a.compare(b);
}

/**
* Retrieve the next occurrence in the series.
* @return {ICAL.Time}
Expand Down Expand Up @@ -248,9 +263,10 @@ class RecurExpansion {

// check the negative rules
if (this.exDate) {
compare = this.exDate.compare(this.last);
//EXDATE can be in DATE format, but DTSTART is in DATE-TIME format
compare = this._compare_special(this.last, this.exDate);

if (compare < 0) {
if (compare > 0) {
this._nextExDay();
}

Expand Down Expand Up @@ -397,10 +413,12 @@ class RecurExpansion {
if (component.hasProperty('exdate')) {
this.exDates = this._extractDates(component, 'exdate');
// if we have a .last day we increment the index to beyond it.
// When DTSTART is in DATE-TIME format, EXDATE is in DATE format and EXDATE is
// the date of DTSTART, _compare_special finds this out and compareTime fails.
this.exDateInc = binsearchInsert(
this.exDates,
this.last,
(a, b) => a.compare(b)
this._compare_special
);

this.exDate = this.exDates[this.exDateInc];
Expand Down
8 changes: 8 additions & 0 deletions samples/rdate_exdate.ics
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
BEGIN:VCALENDAR
BEGIN:VEVENT
UID:123
DTSTART:20240609T030000Z
RRULE:FREQ=DAILY;INTERVAL=1;COUNT=4
EXDATE;VALUE=DATE:20240611
END:VEVENT
END:VCALENDAR
13 changes: 13 additions & 0 deletions test/recur_expansion_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,4 +329,17 @@ suite('recur_expansion', function() {

});

suite('EXDATE and DTSTART have different value type', function() {
createSubject('rdate_exdate.ics');
test('Compare EXDATE;VALUE=DATE and DTSTART;VALUE=DATE-TIME', function() {
let dates = [], next;
while ((next = subject.next()))
dates.push(next.toJSDate());
assert.deepEqual(dates, [
new Date('2024-06-09T03:00:00.000Z'),
new Date('2024-06-10T03:00:00.000Z'),
new Date('2024-06-12T03:00:00.000Z')
]);
});
});
});