Repository: obsidian-tasks-group/obsidian-tasks (ts)

Issue:
'When done' causing wrong done date
### Please check that this issue hasn't been reported before.

- [X] I searched previous [Bug Reports](https://github.com/obsidian-tasks-group/obsidian-tasks/issues?q=is%3Aopen+is%3Aissue+label%3A%22type%3A+bug%22+sort%3Acomments-desc) didn't find any similar reports.

### Expected Behavior

When I check the below task as done by clicking the check field, it should add today's done date (2024-05-31) and create the recurring task.

`- [ ] 🦷 Dentist Check up Appointment 🔁 every 6 months when done ⏳ 2024-05-31`

which would turn into:

`- [x] 🦷 Dentist Check up Appointment 🔁 every 6 months when done ⏳ 2024-05-31 ✅ 2024-05-31`
`- [ ] 🦷 Dentist Check up Appointment 🔁 every 6 months when done ➕ 2024-05-31 ⏳ 2024-11-30`


### Current behaviour

When checked, the task adds yesterday as it's completion date instead.

`- [x] 🦷 Dentist Check up Appointment 🔁 every 6 months when done ⏳ 2024-05-31 ✅ 2024-05-30`
`- [ ] 🦷 Dentist Check up Appointment 🔁 every 6 months when done ➕ 2024-05-31 ⏳ 2024-11-30`

I have tried it without the '`when done`' words in the recurrence rule and it works correctly.

### Steps to reproduce

1. I have been able to reproduce this in sandbox vault with only Tasks installed

_Maintainer's note: See https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2867#issuecomment-2192679323 for reproduction steps._

### Which Operating Systems are you using?

- [ ] Android
- [ ] iPhone/iPad
- [ ] Linux
- [ ] macOS
- [X] Windows

### Obsidian Version

1.6.1 (Insider Release), Installer version 1.5.3

### Tasks Plugin Version

7.3.0

### Checks

- [X] I have tried it with all other plugins disabled and the error still occurs

### Possible solution

_No response_

Pull request:
fix: recurrence with 'when done' could give wrong 'done' date
# Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)
  - Issue/discussion: #2867

## Description

Prevent unintended mutation of 'today' that broke the saved done date.

The underlying cause of this us that `today.startOf('day')` was being called in `nextReferenceDateFromToday()` - which was adjust the reference date passed in, in the case that the next recurrence was a non-existent date and so needed to be adjusted.

## Motivation and Context

Fixes #2867

<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->

## How has this been tested?

- Adding some tests - and then later removing them, when I discovered that the problem could be found with existing test cases
- Repeating the 'steps to reproduce' in #2867, to confirm it really is fixed.

## Checklist

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [x] My change has adequate [Unit Test coverage](https://publish.obsidian.md/tasks-contributing/Testing/About+Testing).

## Terms

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://publish.obsidian.md/tasks-contributing) and
[Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md).
Put an x in the boxes to confirm you agree.
-->

- [x] My contribution follows this project's [contributing guide](https://publish.obsidian.md/tasks-contributing)
- [x] I agree to follow this project's [Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md)


Patch:
diff --git a/src/Task/Recurrence.ts b/src/Task/Recurrence.ts
index 64d1fdbb..ccb5793b 100644
--- a/src/Task/Recurrence.ts
+++ b/src/Task/Recurrence.ts
@@ -200,7 +200,7 @@ export class Recurrence {
     private nextReferenceDate(today: Moment): Date {
         if (this.baseOnToday) {
             // The next occurrence should happen based off the current date.
-            return this.nextReferenceDateFromToday(today).toDate();
+            return this.nextReferenceDateFromToday(today.clone()).toDate();
         } else {
             return this.nextReferenceDateFromOriginalReferenceDate().toDate();
         }


Test patch:
diff --git a/tests/Task/Task.test.ts b/tests/Task/Task.test.ts
index c14b7de9..c3eaa90f 100644
--- a/tests/Task/Task.test.ts
+++ b/tests/Task/Task.test.ts
@@ -1094,12 +1094,14 @@ describe('toggle done', () => {
 
         // Testing 'when done' does not skip when next occurrence is a non-existent date
         {
+            // This also showed the existence of #2867 - which caused the done date to be '2021-08-30'
             interval: 'every month when done',
             scheduled: '1999-01-23',
             today: '2021-08-31',
             nextScheduled: '2021-09-30',
         },
         {
+            // This also showed the existence of #2867 - which caused the done date to be '2020-02-28'
             interval: 'every 2 years when done',
             start: '1999-01-23',
             today: '2020-02-29', // is a leap year
@@ -1183,6 +1185,14 @@ describe('toggle done', () => {
         const nextTask: Task = tasks[0];
 
         expect(doneTask.status.symbol).toEqual(doneSymbol ?? 'x');
+        if (today) {
+            expect({
+                doneDate: doneTask.doneDate?.format('YYYY-MM-DD'),
+            }).toMatchObject({
+                doneDate: today,
+            });
+        }
+
         expect(nextTask.status.symbol).toEqual(nextSymbol ?? ' ');
         expect({
             nextDue: nextTask.dueDate?.format('YYYY-MM-DD'),