Merge pull request #3558 from obsidian-tasks-group/test-issue-3553

refactor: Add failing test and prepare to fix #3553

Author: claremacrae

src/Task/Occurrence.ts

@@ -1,5 +1,6 @@
 import type { Moment } from 'moment/moment';
 import { compareByDate } from '../DateTime/DateTools';
+import { getSettings } from '../Config/Settings';
 
 /**
  * A set of dates on a single instance of {@link Recurrence}.
@@ -11,6 +12,20 @@ export class Occurrence {
     public readonly scheduledDate: Moment | null;
     public readonly dueDate: Moment | null;
 
+    constructor({
+        startDate = null,
+        scheduledDate = null,
+        dueDate = null,
+    }: {
+        startDate?: Moment | null;
+        scheduledDate?: Moment | null;
+        dueDate?: Moment | null;
+    }) {
+        this.startDate = startDate ?? null;
+        this.scheduledDate = scheduledDate ?? null;
+        this.dueDate = dueDate ?? null;
+    }
+
     /**
      * The reference date is used to calculate future occurrences.
      *
@@ -24,21 +39,8 @@ export class Occurrence {
      * same relative distance to the due date as the original task. For example
      * "starts one week before it is due".
      */
-    public readonly referenceDate: Moment | null;
-
-    constructor({
-        startDate = null,
-        scheduledDate = null,
-        dueDate = null,
-    }: {
-        startDate?: Moment | null;
-        scheduledDate?: Moment | null;
-        dueDate?: Moment | null;
-    }) {
-        this.startDate = startDate ?? null;
-        this.scheduledDate = scheduledDate ?? null;
-        this.dueDate = dueDate ?? null;
-        this.referenceDate = this.getReferenceDate();
+    public get referenceDate(): moment.Moment | null {
+        return this.getReferenceDate();
     }
 
     /**
@@ -87,9 +89,8 @@ export class Occurrence {
      * If the occurrence has no reference date, an empty {@link Occurrence} will be returned.
      *
      * @param nextReferenceDate
-     * @param removeScheduledDate - Optional boolean to remove the scheduled date from the next occurrence so long as a start or due date exists.
      */
-    public next(nextReferenceDate: Date, removeScheduledDate: boolean = false): Occurrence {
+    public next(nextReferenceDate: Date): Occurrence {
         // Only if a reference date is given. A reference date will exist if at
         // least one of the other dates is set.
         if (this.referenceDate === null) {
@@ -103,7 +104,9 @@ export class Occurrence {
         const hasStartDate = this.startDate !== null;
         const hasDueDate = this.dueDate !== null;
         const canRemoveScheduledDate = hasStartDate || hasDueDate;
-        const shouldRemoveScheduledDate = removeScheduledDate && canRemoveScheduledDate;
+
+        const { removeScheduledDateOnRecurrence } = getSettings();
+        const shouldRemoveScheduledDate = removeScheduledDateOnRecurrence && canRemoveScheduledDate;
 
         const startDate = this.nextOccurrenceDate(this.startDate, nextReferenceDate);
         const scheduledDate = shouldRemoveScheduledDate

src/Task/Recurrence.ts

@@ -72,16 +72,15 @@ export class Recurrence {
      * Returns the dates of the next occurrence or null if there is no next occurrence.
      *
      * @param today - Optional date representing the completion date. Defaults to today.
-     * @param removeScheduledDate - Optional boolean to remove the scheduled date from the next occurrence so long as a start or due date exists.
      */
-    public next(today = window.moment(), removeScheduledDate: boolean = false): Occurrence | null {
+    public next(today = window.moment()): Occurrence | null {
         const nextReferenceDate = this.nextReferenceDate(today);
 
         if (nextReferenceDate === null) {
             return null;
         }
 
-        return this.occurrence.next(nextReferenceDate, removeScheduledDate);
+        return this.occurrence.next(nextReferenceDate);
     }
 
     public identicalTo(other: Recurrence) {

src/Task/Task.ts

@@ -377,8 +377,7 @@ export class Task extends ListItem {
             return [toggledTask];
         }
 
-        const { removeScheduledDateOnRecurrence } = getSettings();
-        const nextOccurrence = this.recurrence.next(today, removeScheduledDateOnRecurrence);
+        const nextOccurrence = this.recurrence.next(today);
         if (nextOccurrence === null) {
             return [toggledTask];
         }

tests/Task/Recurrence.test.ts

@@ -5,6 +5,7 @@ import moment from 'moment';
 import { Occurrence } from '../../src/Task/Occurrence';
 import { Recurrence } from '../../src/Task/Recurrence';
 import { RecurrenceBuilder } from '../TestingTools/RecurrenceBuilder';
+import { updateSettings } from '../../src/Config/Settings';
 
 window.moment = moment;
 
@@ -230,6 +231,14 @@ describe('identicalTo', () => {
 });
 
 describe('Recurrence - with removeScheduledDateOnRecurrence', () => {
+    beforeEach(() => {
+        updateSettings({ removeScheduledDateOnRecurrence: true });
+    });
+
+    afterEach(() => {
+        updateSettings({ removeScheduledDateOnRecurrence: false });
+    });
+
     it('should remove the scheduledDate when removeScheduledDate is true', () => {
         // Arrange
         const recurrence = Recurrence.fromText({
@@ -242,7 +251,7 @@ describe('Recurrence - with removeScheduledDateOnRecurrence', () => {
         });
 
         // Act
-        const next = recurrence!.next(undefined, true);
+        const next = recurrence!.next();
 
         // Assert
         expect(next!.startDate).toEqualMoment(moment('2022-02-01'));
@@ -260,9 +269,46 @@ describe('Recurrence - with removeScheduledDateOnRecurrence', () => {
         });
 
         // Act
-        const next = recurrence!.next(undefined, true);
+        const next = recurrence!.next();
 
         // Assert
         expect(next!.scheduledDate).not.toBeNull();
     });
+
+    describe('dropScheduledDate and when done', () => {
+        beforeEach(() => {
+            jest.useFakeTimers();
+            jest.setSystemTime(new Date('2022-01-10'));
+        });
+        afterEach(() => {
+            jest.useRealTimers();
+        });
+
+        it.failing('calculates correct start date with "dropScheduledDate" and "when done", with no due date', () => {
+            // Arrange
+
+            // The task is being completed on the 10th of January.
+            // And the user has turned on the setting to remove the scheduled date in the new instance.
+            // This means that the user will apply a scheduled date at some point in the future,
+            // when they are ready to work on the task.
+            // Therefore, in the new task, as there is no Due date, the only date the user will see
+            // is the Start date.
+            // And so it makes sense to calculate the new 'happens' date using the Start date,
+            // not the old scheduled date.
+            const recurrence = Recurrence.fromText({
+                recurrenceRuleText: 'every 3 days when done',
+                occurrence: new Occurrence({
+                    startDate: moment('2022-01-01').startOf('day'),
+                    scheduledDate: moment('2022-01-04').startOf('day'),
+                }),
+            });
+
+            // Act
+            const next = recurrence!.next();
+
+            // Assert
+            expect(next!.startDate).toEqualMoment(moment('2022-01-13'));
+            expect(next!.scheduledDate).toBeNull();
+        });
+    });
 });