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

Issue:
Edit Task modal: switching status to Done or Cancelled changes the existing Done or Cancelled 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

If a Done date has been set, switching status from Todo to Done should not overwrite an already set Done date. Same thing for Cancelled date & status.

### Current behaviour

If a Done date has been set, switching status from Todo to Done **_overwrites_** an already set Done date. Same thing for Cancelled date & status.

### Steps to reproduce

1. Open Edit Task modal on a task that is not Done status
2. Set Done date to any value other than today's
3. Change status to Done

Same thing for Cancelled date & status.

### Which Operating Systems are you using?

- [ ] Android
- [ ] iPhone/iPad
- [ ] Linux
- [X] macOS
- [ ] Windows

### Obsidian Version

1.7.1

### Tasks Plugin Version

7.10.0

### Checks

- [ ] I have tried it with all other plugins disabled and the error still occurs

### Possible solution

Setting status to Done or Cancelled should not overwrite an existing Done or Cancelled date but only set it if it was missing.

Pull request:
fix: retain manually added Done & Cancelled dates in modal
# Types of changes

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)

Internal changes:

- [x] **Tests** (prefix: `test` - additions and improvements to unit tests and the smoke tests)

## Description

- Done/Cancelled date have to be kept if they are already present in the respective date editor while the status is set to Done/Cancelled respectively. 

_Don't overwrite a user-entered Done date if changing status to DONE_
_Don't overwrite a user-entered Cancelled date if changing status to CANCELLED_


## Motivation and Context

- Fixes #3085 

## How has this been tested?

- new unit tests added
- manual test in demo vault

## Checklist

- [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

- [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/ui/StatusEditor.svelte b/src/ui/StatusEditor.svelte
index e53d71db..cff563b9 100644
--- a/src/ui/StatusEditor.svelte
+++ b/src/ui/StatusEditor.svelte
@@ -1,4 +1,5 @@
 <script lang="ts">
+    import type { TasksDate } from '../DateTime/TasksDate';
     import type { Status } from '../Statuses/Status';
     import type { Task } from '../Task/Task';
     import type { EditableTask } from './EditableTask';
@@ -11,6 +12,22 @@
 
     let statusSymbol = task.status.symbol;
 
+    function setStatusRelatedDate(currentValue: string, isInStatus: boolean, editedValue: TasksDate) {
+        const dateFieldIsEmpty = currentValue === '';
+
+        if (isInStatus && dateFieldIsEmpty) {
+            // the date field is empty and the status was set (set the date from the task with the applied status)
+            return editedValue.formatAsDate();
+        }
+
+        if (!isInStatus && !dateFieldIsEmpty) {
+            // the date field is not empty but another status was set (clean the date field)
+            return '';
+        }
+
+        return currentValue;
+    }
+
     const _onStatusChange = () => {
         // Use statusSymbol to find the status to save to editableTask.status
         const selectedStatus: Status | undefined = statusOptions.find((s) => s.symbol === statusSymbol);
@@ -26,9 +43,17 @@
         const taskWithEditedStatusApplied = task.handleNewStatus(selectedStatus).pop();
 
         if (taskWithEditedStatusApplied) {
-            // Update the doneDate field, in case changing the status changed the value:
-            editableTask.doneDate = taskWithEditedStatusApplied.done.formatAsDate();
-            editableTask.cancelledDate = taskWithEditedStatusApplied.cancelled.formatAsDate();
+            editableTask.doneDate = setStatusRelatedDate(
+                editableTask.doneDate,
+                selectedStatus.isCompleted(),
+                taskWithEditedStatusApplied.done,
+            );
+
+            editableTask.cancelledDate = setStatusRelatedDate(
+                editableTask.cancelledDate,
+                selectedStatus.isCancelled(),
+                taskWithEditedStatusApplied.cancelled,
+            );
         }
     };
 </script>


Test patch:
diff --git a/tests/ui/EditTask.test.ts b/tests/ui/EditTask.test.ts
index 404131ff..18f790e0 100644
--- a/tests/ui/EditTask.test.ts
+++ b/tests/ui/EditTask.test.ts
@@ -6,8 +6,8 @@ import moment from 'moment';
 import { taskFromLine } from '../../src/Commands/CreateOrEditTaskParser';
 import { GlobalFilter } from '../../src/Config/GlobalFilter';
 import { resetSettings, updateSettings } from '../../src/Config/Settings';
-import { StatusRegistry } from '../../src/Statuses/StatusRegistry';
 import { DateFallback } from '../../src/DateTime/DateFallback';
+import { StatusRegistry } from '../../src/Statuses/StatusRegistry';
 import type { Task } from '../../src/Task/Task';
 import EditTask from '../../src/ui/EditTask.svelte';
 import { verifyWithFileExtension } from '../TestingTools/ApprovalTestHelpers';
@@ -158,6 +158,35 @@ async function renderTaskModalAndChangeStatus(line: string, newStatusSymbol: str
     return { waitForClose, container, submit };
 }
 
+/**
+ * Simulate the behaviour of:
+ *   - clicking on a line in Obsidian,
+ *   - opening the Edit task modal,
+ *   - editing a field,
+ *   - changing the status,
+ *   - and clicking Apply.
+ * @param line
+ * @param elementId - specifying the field to edit
+ * @param newValue - the new value for the field
+ * @param newStatusSymbol - new Status symbol value
+ */
+async function renderChangeDateAndStatus(line: string, elementId: string, newValue: string, newStatusSymbol: string) {
+    const task = taskFromLine({ line: line, path: '' });
+    const { waitForClose, onSubmit } = constructSerialisingOnSubmit(task);
+    const { result, container } = renderAndCheckModal(task, onSubmit);
+
+    const inputElement = getAndCheckRenderedElement<HTMLInputElement>(container, elementId);
+    await editInputElement(inputElement, newValue);
+
+    const statusSelector = getAndCheckRenderedElement<HTMLSelectElement>(container, 'status-type');
+    await fireEvent.change(statusSelector, {
+        target: { value: newStatusSymbol },
+    });
+
+    const submit = getAndCheckApplyButton(result);
+    return { waitForClose, container, submit };
+}
+
 function getElementValue(container: HTMLElement, elementId: string) {
     const element = getAndCheckRenderedElement<HTMLInputElement>(container, elementId);
     return element.value;
@@ -334,34 +363,142 @@ describe('Task editing', () => {
             resetSettings();
         });
 
-        const line = '- [ ] simple';
         it('should change status to Done and add doneDate', async () => {
-            const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(line, 'x');
+            const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
+                '- [ ] expecting done date to be added',
+                'x',
+            );
             expect(getElementValue(container, 'done')).toEqual(today);
 
             submit.click();
-            expect(await waitForClose).toMatchInlineSnapshot('"- [x] simple ✅ 2024-02-29"');
+            expect(await waitForClose).toMatchInlineSnapshot('"- [x] expecting done date to be added ✅ 2024-02-29"');
+        });
+
+        it('should change status to Done and keep doneDate', async () => {
+            const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
+                '- [ ] expecting done date to be kept ✅ 2024-09-19',
+                'x',
+            );
+            expect(getElementValue(container, 'done')).toEqual('2024-09-19');
+
+            submit.click();
+            expect(await waitForClose).toMatchInlineSnapshot('"- [x] expecting done date to be kept ✅ 2024-09-19"');
         });
 
         it('should change status to Todo and remove doneDate', async () => {
             const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
-                '- [x] simple ✅ 2024-02-29',
+                '- [x] expecting done date to be removed ✅ 2024-02-29',
                 ' ',
             );
             expect(getElementValue(container, 'done')).toEqual('');
 
             submit.click();
-            expect(await waitForClose).toMatchInlineSnapshot('"- [ ] simple"');
+            expect(await waitForClose).toMatchInlineSnapshot('"- [ ] expecting done date to be removed"');
         });
 
         it('should change status to Cancelled and add cancelledDate', async () => {
-            const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(line, '-');
+            const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
+                '- [ ] expecting cancelled date to be added',
+                '-',
+            );
             expect(getElementValue(container, 'cancelled')).toEqual(today);
 
             submit.click();
-            expect(await waitForClose).toMatchInlineSnapshot('"- [-] simple ❌ 2024-02-29"');
+            expect(await waitForClose).toMatchInlineSnapshot(
+                '"- [-] expecting cancelled date to be added ❌ 2024-02-29"',
+            );
+        });
+
+        it('should change status to Cancelled and keep cancelledDate', async () => {
+            const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
+                '- [ ] expecting cancelled date to be kept ❌ 2024-09-20',
+                '-',
+            );
+            expect(getElementValue(container, 'cancelled')).toEqual('2024-09-20');
+
+            submit.click();
+            expect(await waitForClose).toMatchInlineSnapshot(
+                '"- [-] expecting cancelled date to be kept ❌ 2024-02-29"',
+            );
         });
 
+        it('should change status to Todo and remove cancelledDate', async () => {
+            const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
+                '- [-] expecting cancelled date to be removed ❌ 2024-02-29',
+                ' ',
+            );
+            expect(getElementValue(container, 'cancelled')).toEqual('');
+
+            submit.click();
+            expect(await waitForClose).toMatchInlineSnapshot('"- [ ] expecting cancelled date to be removed"');
+        });
+
+        /**
+         * Test opening task modal for a given line, changing a date to a value, changing the status,
+         * clicking Apply, verifying the final line.
+         *
+         * @param line
+         * @param dateElementToChange
+         * @param dateValue
+         * @param newStatusSymbol
+         * @param expectedTaskAfterEdits
+         */
+        async function testDateInputAndStatusChange(
+            line: string,
+            dateElementToChange: string,
+            dateValue: string,
+            newStatusSymbol: string,
+            expectedTaskAfterEdits: string,
+        ) {
+            const { waitForClose, container, submit } = await renderChangeDateAndStatus(
+                line,
+                dateElementToChange,
+                dateValue,
+                newStatusSymbol,
+            );
+
+            expect(getElementValue(container, dateElementToChange)).toEqual(dateValue);
+
+            submit.click();
+            expect(await waitForClose).toEqual(expectedTaskAfterEdits);
+        }
+
+        it.each([
+            [
+                '- [ ] input done date, change status to done and expect the date to be kept',
+                'done',
+                '2024-09-20',
+                'x',
+                '- [x] input done date, change status to done and expect the date to be kept ✅ 2024-09-20',
+            ],
+            [
+                '- [ ] input cancelled date, change status to cancelled and expect the date to be kept',
+                'cancelled',
+                '2024-09-21',
+                '-',
+                // TODO the difference between the date in the modal and the saved one is a bug:
+                // https://github.com/obsidian-tasks-group/obsidian-tasks/issues/3089
+                '- [-] input cancelled date, change status to cancelled and expect the date to be kept ❌ 2024-02-29',
+            ],
+        ])(
+            'for "%s" task, change %s date to %s and status to %s',
+            async (
+                line: string,
+                dateElementToChange: string,
+                dateValue: string,
+                newStatusSymbol: string,
+                expectedTaskAfterEdits: string,
+            ) => {
+                await testDateInputAndStatusChange(
+                    line,
+                    dateElementToChange,
+                    dateValue,
+                    newStatusSymbol,
+                    expectedTaskAfterEdits,
+                );
+            },
+        );
+
         it('should create new instance of recurring task, with doneDate set to today', async () => {
             updateSettings({ recurrenceOnNextLine: false });
             const { waitForClose, submit } = await renderTaskModalAndChangeStatus(