Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2868 (2024-06-01 09:20:41)

refactor: Improve encapsulation of behaviour in `EditableTask` class
# Types of changes

Internal changes:

- [x] **Refactor** (prefix: `refactor` - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
- [x] **Tests** (prefix: `test` - additions and improvements to unit tests and the smoke tests)

## Description

- remove `addGlobalFilterOnSave` and `originalBlocking` from `EditableTask` methods' input and output
- move them to `private readonly` fields of `EditableTask` class

## Motivation and Context

- encapsulate behaviour that does not need to be exposed

## How has this been tested?

- Unit tests

## 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/EditTask.svelte b/src/ui/EditTask.svelte
index e259969b..d8f6e632 100644
--- a/src/ui/EditTask.svelte
+++ b/src/ui/EditTask.svelte
@@ -29,7 +29,7 @@
 
     let descriptionInput: HTMLTextAreaElement;
 
-    let { editableTask, addGlobalFilterOnSave, originalBlocking } = EditableTask.fromTask(task, allTasks);
+    let editableTask = EditableTask.fromTask(task, allTasks);
 
     let isDescriptionValid: boolean = true;
 
@@ -142,7 +142,7 @@
     };
 
     const _onSubmit = async () => {
-        const newTasks = await editableTask.applyEdits(task, originalBlocking, addGlobalFilterOnSave, allTasks);
+        const newTasks = await editableTask.applyEdits(task, allTasks);
         onSubmit(newTasks);
     };
 </script>
diff --git a/src/ui/EditableTask.ts b/src/ui/EditableTask.ts
index 9e51a0dc..8a96f95d 100644
--- a/src/ui/EditableTask.ts
+++ b/src/ui/EditableTask.ts
@@ -15,6 +15,9 @@ type EditableTaskPriority = 'none' | 'lowest' | 'low' | 'medium' | 'high' | 'hig
  *
  */
 export class EditableTask {
+    private readonly addGlobalFilterOnSave: boolean;
+    private readonly originalBlocking: Task[];
+
     // NEW_TASK_FIELD_EDIT_REQUIRED
     description: string;
     status: Status;
@@ -31,6 +34,9 @@ export class EditableTask {
     blocking: Task[];
 
     private constructor(editableTask: {
+        addGlobalFilterOnSave: boolean;
+        originalBlocking: Task[];
+
         // NEW_TASK_FIELD_EDIT_REQUIRED
         description: string;
         status: Status;
@@ -46,6 +52,9 @@ export class EditableTask {
         blockedBy: Task[];
         blocking: Task[];
     }) {
+        this.addGlobalFilterOnSave = editableTask.addGlobalFilterOnSave;
+        this.originalBlocking = editableTask.originalBlocking;
+
         this.description = editableTask.description;
         this.status = editableTask.status;
         this.priority = editableTask.priority;
@@ -67,22 +76,14 @@ export class EditableTask {
      * @param task
      * @param allTasks
      */
-    public static fromTask(
-        task: Task,
-        allTasks: Task[],
-    ): {
-        addGlobalFilterOnSave: boolean;
-        originalBlocking: Task[];
-        editableTask: EditableTask;
-    } {
+    public static fromTask(task: Task, allTasks: Task[]): EditableTask {
         const description = GlobalFilter.getInstance().removeAsWordFrom(task.description);
         // If we're displaying to the user the description without the global filter (i.e. it was removed in the method
         // above), or if the description did not include a global filter in the first place, we'll add the global filter
         // when saving the task.
-        let addGlobalFilterOnSave = false;
-        if (description != task.description || !GlobalFilter.getInstance().includedIn(task.description)) {
-            addGlobalFilterOnSave = true;
-        }
+        const addGlobalFilterOnSave =
+            description != task.description || !GlobalFilter.getInstance().includedIn(task.description);
+
         let priority: EditableTaskPriority = 'none';
         if (task.priority === Priority.Lowest) {
             priority = 'lowest';
@@ -108,26 +109,25 @@ export class EditableTask {
 
         const originalBlocking = allTasks.filter((cacheTask) => cacheTask.dependsOn.includes(task.id));
 
-        return {
-            editableTask: new EditableTask({
-                // NEW_TASK_FIELD_EDIT_REQUIRED
-                description,
-                status: task.status,
-                priority,
-                recurrenceRule: task.recurrence ? task.recurrence.toText() : '',
-                createdDate: task.created.formatAsDate(),
-                startDate: task.start.formatAsDate(),
-                scheduledDate: task.scheduled.formatAsDate(),
-                dueDate: task.due.formatAsDate(),
-                doneDate: task.done.formatAsDate(),
-                cancelledDate: task.cancelled.formatAsDate(),
-                forwardOnly: true,
-                blockedBy: blockedBy,
-                blocking: originalBlocking,
-            }),
+        return new EditableTask({
             addGlobalFilterOnSave,
             originalBlocking,
-        };
+
+            // NEW_TASK_FIELD_EDIT_REQUIRED
+            description,
+            status: task.status,
+            priority,
+            recurrenceRule: task.recurrence ? task.recurrence.toText() : '',
+            createdDate: task.created.formatAsDate(),
+            startDate: task.start.formatAsDate(),
+            scheduledDate: task.scheduled.formatAsDate(),
+            dueDate: task.due.formatAsDate(),
+            doneDate: task.done.formatAsDate(),
+            cancelledDate: task.cancelled.formatAsDate(),
+            forwardOnly: true,
+            blockedBy: blockedBy,
+            blocking: originalBlocking,
+        });
     }
 
     /**
@@ -136,19 +136,12 @@ export class EditableTask {
      * There are cases where the output of the edits is more than one task, for example, completing a {@link Task} with {@link Recurrence}.
      *
      * @param task
-     * @param originalBlocking
-     * @param addGlobalFilterOnSave
      * @param allTasks
      */
-    public async applyEdits(
-        task: Task,
-        originalBlocking: Task[],
-        addGlobalFilterOnSave: boolean,
-        allTasks: Task[],
-    ): Promise<Task[]> {
+    public async applyEdits(task: Task, allTasks: Task[]): Promise<Task[]> {
         // NEW_TASK_FIELD_EDIT_REQUIRED
         let description = this.description.trim();
-        if (addGlobalFilterOnSave) {
+        if (this.addGlobalFilterOnSave) {
             description = GlobalFilter.getInstance().prependTo(description);
         }
 
@@ -202,14 +195,14 @@ export class EditableTask {
         let removedBlocking: Task[] = [];
         let addedBlocking: Task[] = [];
 
-        if (this.blocking.toString() !== originalBlocking.toString() || this.blocking.length !== 0) {
+        if (this.blocking.toString() !== this.originalBlocking.toString() || this.blocking.length !== 0) {
             if (task.id === '') {
                 id = generateUniqueId(allTasks.filter((task) => task.id !== '').map((task) => task.id));
             }
 
-            removedBlocking = originalBlocking.filter((task) => !this.blocking.includes(task));
+            removedBlocking = this.originalBlocking.filter((task) => !this.blocking.includes(task));
 
-            addedBlocking = this.blocking.filter((task) => !originalBlocking.includes(task));
+            addedBlocking = this.blocking.filter((task) => !this.originalBlocking.includes(task));
         }
 
         // First create an updated task, with all edits except Status:


Test patch:
diff --git a/tests/ui/EditTaskHelpers.test.ts b/tests/ui/EditTaskHelpers.test.ts
index 506de7e7..fa43e1a5 100644
--- a/tests/ui/EditTaskHelpers.test.ts
+++ b/tests/ui/EditTaskHelpers.test.ts
@@ -50,7 +50,7 @@ describe('labelContentWithAccessKey() tests', () => {
 
 describe('parseAndValidateRecurrence() tests', () => {
     const emptyTask = new TaskBuilder().description('').build();
-    const { editableTask } = EditableTask.fromTask(emptyTask, [emptyTask]);
+    const editableTask = EditableTask.fromTask(emptyTask, [emptyTask]);
 
     const noRecurrenceRule = (editableTask: EditableTask) => {
         editableTask.recurrenceRule = '';
diff --git a/tests/ui/EditableTask.test.ts b/tests/ui/EditableTask.test.ts
index ece07b62..884dfaa9 100644
--- a/tests/ui/EditableTask.test.ts
+++ b/tests/ui/EditableTask.test.ts
@@ -14,22 +14,17 @@ function testEditableTaskDescriptionAndGlobalFilterOnSave({
     globalFilter,
     taskDescription,
     expectedEditableTaskDescription,
-    expectedAddGlobalFilterOnSave,
 }: {
     globalFilter: string;
     taskDescription: string;
     expectedEditableTaskDescription: string;
-    expectedAddGlobalFilterOnSave: boolean;
 }) {
     GlobalFilter.getInstance().set(globalFilter);
     const taskWithoutGlobalFilter = new TaskBuilder().description(taskDescription).build();
 
-    const { editableTask, addGlobalFilterOnSave } = EditableTask.fromTask(taskWithoutGlobalFilter, [
-        taskWithoutGlobalFilter,
-    ]);
+    const editableTask = EditableTask.fromTask(taskWithoutGlobalFilter, [taskWithoutGlobalFilter]);
 
     expect(editableTask.description).toEqual(expectedEditableTaskDescription);
-    expect(addGlobalFilterOnSave).toEqual(expectedAddGlobalFilterOnSave);
 }
 
 describe('EditableTask tests', () => {
@@ -46,10 +41,11 @@ describe('EditableTask tests', () => {
     it('should create an editable task without dependencies', () => {
         const taskToEdit = TaskBuilder.createFullyPopulatedTask();
 
-        const { editableTask, originalBlocking } = EditableTask.fromTask(taskToEdit, [taskToEdit]);
+        const editableTask = EditableTask.fromTask(taskToEdit, [taskToEdit]);
 
         expect(editableTask).toMatchInlineSnapshot(`
             EditableTask {
+              "addGlobalFilterOnSave": false,
               "blockedBy": [],
               "blocking": [],
               "cancelledDate": "2023-07-06",
@@ -58,6 +54,7 @@ describe('EditableTask tests', () => {
               "doneDate": "2023-07-05",
               "dueDate": "2023-07-04",
               "forwardOnly": true,
+              "originalBlocking": [],
               "priority": "medium",
               "recurrenceRule": "every day when done",
               "scheduledDate": "2023-07-03",
@@ -73,7 +70,6 @@ describe('EditableTask tests', () => {
               },
             }
         `);
-        expect(originalBlocking).toEqual([]);
     });
 
     it('should create an editable task with dependencies', () => {
@@ -85,11 +81,10 @@ describe('EditableTask tests', () => {
             .build();
         const allTasks = [taskToEdit, blockingTask, blockedTask];
 
-        const { editableTask, originalBlocking } = EditableTask.fromTask(taskToEdit, allTasks);
+        const editableTask = EditableTask.fromTask(taskToEdit, allTasks);
 
         expect(editableTask.blocking).toEqual([blockedTask]);
         expect(editableTask.blockedBy).toEqual([blockingTask]);
-        expect(originalBlocking).toEqual([blockedTask]);
     });
 
     it('should remember to add global filter when it is absent in task description', () => {
@@ -97,7 +92,6 @@ describe('EditableTask tests', () => {
             globalFilter: '#todo',
             taskDescription: 'global filter is absent',
             expectedEditableTaskDescription: 'global filter is absent',
-            expectedAddGlobalFilterOnSave: true,
         });
     });
 
@@ -106,7 +100,6 @@ describe('EditableTask tests', () => {
             globalFilter: '#important',
             taskDescription: '#important is the global filter',
             expectedEditableTaskDescription: 'is the global filter',
-            expectedAddGlobalFilterOnSave: true,
         });
     });
 
@@ -115,7 +108,6 @@ describe('EditableTask tests', () => {
             globalFilter: GlobalFilter.empty,
             taskDescription: 'global filter has not been set',
             expectedEditableTaskDescription: 'global filter has not been set',
-            expectedAddGlobalFilterOnSave: false,
         });
     });
 
@@ -123,8 +115,8 @@ describe('EditableTask tests', () => {
         const task = new TaskBuilder().build();
         const allTasks = [task];
 
-        const { editableTask, originalBlocking, addGlobalFilterOnSave } = EditableTask.fromTask(task, allTasks);
-        const appliedEdits = await editableTask.applyEdits(task, originalBlocking, addGlobalFilterOnSave, [task]);
+        const editableTask = EditableTask.fromTask(task, allTasks);
+        const appliedEdits = await editableTask.applyEdits(task, [task]);
 
         expect(appliedEdits).toEqual([task]);
     });
@@ -133,8 +125,8 @@ describe('EditableTask tests', () => {
         const task = TaskBuilder.createFullyPopulatedTask();
         const allTasks = [task];
 
-        const { editableTask, originalBlocking, addGlobalFilterOnSave } = EditableTask.fromTask(task, allTasks);
-        const appliedEdits = await editableTask.applyEdits(task, originalBlocking, addGlobalFilterOnSave, [task]);
+        const editableTask = EditableTask.fromTask(task, allTasks);
+        const appliedEdits = await editableTask.applyEdits(task, [task]);
 
         expect(appliedEdits).toEqual([task]);
     });
@@ -143,7 +135,7 @@ describe('EditableTask tests', () => {
         const task = TaskBuilder.createFullyPopulatedTask();
         const allTasks = [task];
 
-        const { editableTask, originalBlocking, addGlobalFilterOnSave } = EditableTask.fromTask(task, allTasks);
+        const editableTask = EditableTask.fromTask(task, allTasks);
 
         editableTask.description = '';
         editableTask.status = Status.TODO;
@@ -159,7 +151,7 @@ describe('EditableTask tests', () => {
         editableTask.blockedBy = [];
         editableTask.blocking = [];
 
-        const appliedEdits = await editableTask.applyEdits(task, originalBlocking, addGlobalFilterOnSave, allTasks);
+        const appliedEdits = await editableTask.applyEdits(task, allTasks);
 
         expect(appliedEdits.length).toEqual(1);
         expect(appliedEdits[0]).toMatchInlineSnapshot(`
@@ -212,11 +204,11 @@ describe('EditableTask tests', () => {
     it('should set a date in YYYY-MM-DD format', async () => {
         const task = new TaskBuilder().build();
         const allTasks: Task[] = [];
-        const { editableTask, originalBlocking, addGlobalFilterOnSave } = EditableTask.fromTask(task, allTasks);
+        const editableTask = EditableTask.fromTask(task, allTasks);
 
         editableTask.dueDate = '2024-07-13';
 
-        const editedTasks = await editableTask.applyEdits(task, originalBlocking, addGlobalFilterOnSave, allTasks);
+        const editedTasks = await editableTask.applyEdits(task, allTasks);
         // TODO Why does this have the time 12:00?
         //      When I edit a task in the plugin, in the modal, and then group by the following, the time is midnight,
         //      so where is the time dropped in production code?
@@ -230,7 +222,7 @@ describe('EditableTask tests', () => {
     it('should honour the forwardOnly value', async () => {
         const task = new TaskBuilder().build();
         const allTasks: Task[] = [];
-        const { editableTask, originalBlocking, addGlobalFilterOnSave } = EditableTask.fromTask(task, allTasks);
+        const editableTask = EditableTask.fromTask(task, allTasks);
 
         jest.setSystemTime(new Date('2024-05-22')); // Wednesday 22nd May
 
@@ -239,11 +231,11 @@ describe('EditableTask tests', () => {
         const tuesdayAfter = moment('2024-05-21T12:00:00.000Z');
 
         editableTask.forwardOnly = true;
-        const tasksFutureDay = await editableTask.applyEdits(task, originalBlocking, addGlobalFilterOnSave, allTasks);
+        const tasksFutureDay = await editableTask.applyEdits(task, allTasks);
         expect(tasksFutureDay[0].dueDate).toEqualMoment(tuesdayBefore);
 
         editableTask.forwardOnly = false;
-        const tasksClosestDay = await editableTask.applyEdits(task, originalBlocking, addGlobalFilterOnSave, allTasks);
+        const tasksClosestDay = await editableTask.applyEdits(task, allTasks);
         expect(tasksClosestDay[0].dueDate).toEqualMoment(tuesdayAfter);
     });
 });