Merge pull request #3087 from ilandikov/fix-keep-done-and-cancelled

fix: retain manually added Done & Cancelled dates in modal

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>

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(