Skip to content

Commit

Permalink
Ensure monthly and yearly recurrences
Browse files Browse the repository at this point in the history
When a recurrence rule states "every month" and the next month does not
have the day of the month that the task is scheduled, that month is
skipped and the next valid month is used instead.

For example: When I mark a task that recurs "every month" done on
January 31, the next occurrence will be scheduled on March 31, since
February does not have 31 days. This is in line with the rrule
specification, but not what users expect. This can be triggered by
accident, for example when the task is based on the done date and the
done date is on the 31st.

To fix this, the number of skipped months is parsed (defaults to one).
While the actualy number of skipped months is higher, the base date is
adjusted one day into the past. In practice, this means that the example
from above would no schedule the recurring task on the 28th of
February (or the 29th for a leap year).

The same applies for yearly recurrences, for example based on the 29th
of February.

This is a proof of concept and is still missing:

1. Documentation in code
2. End user documentation to explain the behavior
3. Extensive testing to ensure this change does not introduce regressions

Fixes #614, #1087
  • Loading branch information
schemar committed Sep 30, 2022
1 parent cc35589 commit 0aebc5e
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 11 deletions.
102 changes: 91 additions & 11 deletions src/Recurrence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class Recurrence {
...this.rrule.origOptions,
dtstart: today.startOf('day').utc(true).toDate(),
});
next = ruleBasedOnToday.after(today.endOf('day').utc(true).toDate());
next = this.nextAfter(today.endOf('day'), ruleBasedOnToday);
} else {
// The next occurrence should happen based on the original reference
// date if possible. Otherwise, base it on today if we do not have a
Expand All @@ -138,17 +138,12 @@ export class Recurrence {
// Reference date can be `null` to mean "today".
// Moment only accepts `undefined`, not `null`.
.moment(this.referenceDate ?? undefined)
.endOf('day')
.utc(true);
.endOf('day');

next = this.rrule.after(after.toDate());
next = this.nextAfter(after, this.rrule);
}

if (next !== null) {
// Re-add the timezone that RRule disregarded:
const localTimeZone = window.moment.utc(next).local(true);
const nextOccurrence = localTimeZone.startOf('day');

// Keep the relative difference between the reference date and
// start/scheduled/due.
let startDate: Moment | null = null;
Expand All @@ -162,23 +157,23 @@ export class Recurrence {
const originalDifference = window.moment.duration(this.startDate.diff(this.referenceDate));

// Cloning so that original won't be manipulated:
startDate = window.moment(nextOccurrence);
startDate = window.moment(next);
// Rounding days to handle cross daylight-savings-time recurrences.
startDate.add(Math.round(originalDifference.asDays()), 'days');
}
if (this.scheduledDate) {
const originalDifference = window.moment.duration(this.scheduledDate.diff(this.referenceDate));

// Cloning so that original won't be manipulated:
scheduledDate = window.moment(nextOccurrence);
scheduledDate = window.moment(next);
// Rounding days to handle cross daylight-savings-time recurrences.
scheduledDate.add(Math.round(originalDifference.asDays()), 'days');
}
if (this.dueDate) {
const originalDifference = window.moment.duration(this.dueDate.diff(this.referenceDate));

// Cloning so that original won't be manipulated:
dueDate = window.moment(nextOccurrence);
dueDate = window.moment(next);
// Rounding days to handle cross daylight-savings-time recurrences.
dueDate.add(Math.round(originalDifference.asDays()), 'days');
}
Expand Down Expand Up @@ -212,4 +207,89 @@ export class Recurrence {

return this.toText() === other.toText(); // this also checks baseOnToday
}

private nextAfter(after: Moment, rrule: RRule): Date {
after.utc(true);
let next = window.moment(rrule.after(after.toDate()));

const monthMatch = this.toText().match(/every( \d+)? month(s)?(.*)?/);
if (monthMatch !== null) {
next = Recurrence.nextAfterMonths(after, next, rrule, monthMatch[1]);
}

const yearMatch = this.toText().match(/every( \d+)? year(s)?(.*)?/);
if (yearMatch !== null) {
next = Recurrence.nextAfterYears(after, next, rrule, yearMatch[1]);
}

return Recurrence.addTimezone(next).toDate();
}

private static nextAfterMonths(
after: Moment,
next: Moment,
rrule: RRule,
skippingMonths: string | undefined,
): Moment {
let parsedSkippingMonths: Number = 1;
if (skippingMonths !== undefined) {
parsedSkippingMonths = Number.parseInt(skippingMonths.trim(), 10);
}

while (Recurrence.isSkippingTooManyMonths(after, next, parsedSkippingMonths)) {
next = Recurrence.fromOneDayEarlier(after, rrule);
}

return next;
}

private static isSkippingTooManyMonths(after: Moment, next: Moment, skippingMonths: Number): boolean {
let diff = next.month() - after.month();
if (diff < 0) {
diff = diff + 12;
}

return diff > skippingMonths;
}

private static nextAfterYears(
after: Moment,
next: Moment,
rrule: RRule,
skippingYears: string | undefined,
): Moment {
let parsedSkippingYears: Number = 1;
if (skippingYears !== undefined) {
parsedSkippingYears = Number.parseInt(skippingYears.trim(), 10);
}
while (Recurrence.isSkippingTooManyYears(after, next, parsedSkippingYears)) {
next = Recurrence.fromOneDayEarlier(after, rrule);
}

return next;
}

private static isSkippingTooManyYears(after: Moment, next: Moment, skippingYears: Number): boolean {
const diff = next.year() - after.year();

return diff > skippingYears;
}

private static fromOneDayEarlier(after: Moment, rrule: RRule): Moment {
after.subtract(1, 'days').endOf('day');

const options = rrule.origOptions;
options.dtstart = after.startOf('day').toDate();
rrule = new RRule(options);

const next = window.moment(rrule.after(after.toDate()));

return next;
}

private static addTimezone(date: Moment): Moment {
const localTimeZone = window.moment.utc(date).local(true);

return localTimeZone.startOf('day');
}
}
72 changes: 72 additions & 0 deletions tests/Recurrence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,78 @@ describe('Recurrence', () => {
dueDate: null,
});
});

it('creates a recurrence the next month, even on the 31st', () => {
// Arrange
const recurrence = Recurrence.fromText({
recurrenceRuleText: 'every month',
startDate: null,
scheduledDate: null,
dueDate: moment('2022-01-31').startOf('day'),
});

// Act
const next = recurrence!.next();

// Assert
expect(next!.startDate).toBeNull();
expect(next!.scheduledDate).toBeNull();
expect(next!.dueDate!.isSame(moment('2022-02-28'))).toStrictEqual(true);
});

it('creates a recurrence 3 months in', () => {
// Arrange
const recurrence = Recurrence.fromText({
recurrenceRuleText: 'every 3 months',
startDate: null,
scheduledDate: null,
dueDate: moment('2022-01-31').startOf('day'),
});

// Act
const next = recurrence!.next();

// Assert
expect(next!.startDate).toBeNull();
expect(next!.scheduledDate).toBeNull();
expect(next!.dueDate!.isSame(moment('2022-04-30'))).toStrictEqual(true);
});

it('creates a recurrence the next month, even across years', () => {
// Arrange
const recurrence = Recurrence.fromText({
recurrenceRuleText: 'every 2 months',
startDate: null,
scheduledDate: null,
dueDate: moment('2023-12-31').startOf('day'),
});

// Act
const next = recurrence!.next();

// Assert
expect(next!.startDate).toBeNull();
expect(next!.scheduledDate).toBeNull();
expect(next!.dueDate!.isSame(moment('2024-02-29'))).toStrictEqual(true);
});

it('creates a recurrence in 2 years, even on Feb 29th', () => {
// Arrange
const recurrence = Recurrence.fromText({
recurrenceRuleText: 'every 2 years',
startDate: null,
scheduledDate: null,
dueDate: moment('2024-02-29').startOf('day'),
});

// Act
const next = recurrence!.next();

// Assert
expect(next!.startDate).toBeNull();
expect(next!.scheduledDate).toBeNull();
expect(next!.dueDate!.isSame(moment('2026-02-28'))).toStrictEqual(true);
});
});

describe('identicalTo', () => {
Expand Down

0 comments on commit 0aebc5e

Please sign in to comment.