Repository: obsidian-tasks-group/obsidian-tasks
Pull request #3126 (2024-10-18 13:46:15)

fix: rerender search results when child list items are edited
# Types of changes

Done by pairing with @claremacrae 

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)

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

- make cache update when there are differences in child list items
- make Task check child list items

## Motivation and Context

- #60 

## How has this been tested?

- unit tests
- manual test in local 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/Obsidian/Cache.ts b/src/Obsidian/Cache.ts
index 02a4fb8b..3f3387dd 100644
--- a/src/Obsidian/Cache.ts
+++ b/src/Obsidian/Cache.ts
@@ -377,7 +377,7 @@ export class Cache {
 
         // If there are no changes in any of the tasks, there's
         // nothing to do, so just return.
-        if (Task.tasksListsIdentical(oldTasks, newTasks)) {
+        if (ListItem.listsAreIdentical(oldTasks, newTasks)) {
             // This code kept for now, to allow for debugging during development.
             // It is too verbose to release to users.
             // if (this.getState() == State.Warm) {
diff --git a/src/Task/ListItem.ts b/src/Task/ListItem.ts
index b97584cd..df0029fd 100644
--- a/src/Task/ListItem.ts
+++ b/src/Task/ListItem.ts
@@ -48,4 +48,46 @@ export class ListItem {
     get isRoot(): boolean {
         return this.parent === null;
     }
+
+    /**
+     * Compare all the fields in another ListItem, to detect any differences from this one.
+     *
+     * If any field is different in any way, it will return false.
+     *
+     * @note Use {@link Task.identicalTo} to compare {@link Task} objects.
+     *
+     * @param other - if this is in fact a {@link Task}, the result of false.
+     */
+    identicalTo(other: ListItem) {
+        if (this.constructor.name !== other.constructor.name) {
+            return false;
+        }
+
+        if (this.originalMarkdown !== other.originalMarkdown) {
+            return false;
+        }
+
+        return ListItem.listsAreIdentical(this.children, other.children);
+    }
+
+    /**
+     * Compare two lists of ListItem objects, and report whether their
+     * contents, including any children, are identical and in the same order.
+     *
+     * This can be useful for optimising code if it is guaranteed that
+     * there are no possible differences in the tasks in a file
+     * after an edit, for example.
+     *
+     * If any field is different in any task or list item, it will return false.
+     *
+     * @param list1
+     * @param list2
+     */
+    static listsAreIdentical(list1: ListItem[], list2: ListItem[]): boolean {
+        if (list1.length !== list2.length) {
+            return false;
+        }
+
+        return list1.every((item, index) => item.identicalTo(list2[index]));
+    }
 }
diff --git a/src/Task/Task.ts b/src/Task/Task.ts
index 1533efa3..3d516c90 100644
--- a/src/Task/Task.ts
+++ b/src/Task/Task.ts
@@ -784,26 +784,6 @@ export class Task extends ListItem {
         return linkText;
     }
 
-    /**
-     * Compare two lists of Task objects, and report whether their
-     * tasks are identical in the same order.
-     *
-     * This can be useful for optimising code if it is guaranteed that
-     * there are no possible differences in the tasks in a file
-     * after an edit, for example.
-     *
-     * If any field is different in any task, it will return false.
-     *
-     * @param oldTasks
-     * @param newTasks
-     */
-    static tasksListsIdentical(oldTasks: Task[], newTasks: Task[]): boolean {
-        if (oldTasks.length !== newTasks.length) {
-            return false;
-        }
-        return oldTasks.every((oldTask, index) => oldTask.identicalTo(newTasks[index]));
-    }
-
     /**
      * Compare all the fields in another Task, to detect any differences from this one.
      *
@@ -816,6 +796,11 @@ export class Task extends ListItem {
      * @param other
      */
     public identicalTo(other: Task) {
+        // First compare child Task and ListItem objects, and any other data in ListItem:
+        if (!super.identicalTo(other)) {
+            return false;
+        }
+
         // NEW_TASK_FIELD_EDIT_REQUIRED
 
         // Based on ideas from koala. AquaCat and javalent in Discord:


Test patch:
diff --git a/tests/Task/ListItem.test.ts b/tests/Task/ListItem.test.ts
index 7bd45ab3..b813e7e6 100644
--- a/tests/Task/ListItem.test.ts
+++ b/tests/Task/ListItem.test.ts
@@ -6,6 +6,9 @@ import { TasksFile } from '../../src/Scripting/TasksFile';
 import { Task } from '../../src/Task/Task';
 import { TaskLocation } from '../../src/Task/TaskLocation';
 import { ListItem } from '../../src/Task/ListItem';
+import { TaskBuilder } from '../TestingTools/TaskBuilder';
+import { fromLine } from '../TestingTools/TestHelpers';
+import { createChildListItem } from './ListItemHelpers';
 
 window.moment = moment;
 
@@ -96,3 +99,107 @@ describe('list item tests', () => {
         }
     });
 });
+
+describe('identicalTo', () => {
+    it('should test same markdown', () => {
+        const listItem1 = new ListItem('- same description', null);
+        const listItem2 = new ListItem('- same description', null);
+        expect(listItem1.identicalTo(listItem2)).toEqual(true);
+    });
+
+    it('should test different markdown', () => {
+        const listItem1 = new ListItem('- description', null);
+        const listItem2 = new ListItem('- description two', null);
+        expect(listItem1.identicalTo(listItem2)).toEqual(false);
+    });
+
+    it('should recognise list items with different number of children', () => {
+        const item1 = new ListItem('- item', null);
+        createChildListItem('- child of item1', item1);
+
+        const item2 = new ListItem('- item', null);
+
+        expect(item2.identicalTo(item1)).toEqual(false);
+    });
+
+    it('should recognise list items with different children', () => {
+        const item1 = new ListItem('- item', null);
+        createChildListItem('- child of item1', item1);
+
+        const item2 = new ListItem('- item', null);
+        createChildListItem('- child of item2', item2);
+
+        expect(item2.identicalTo(item1)).toEqual(false);
+    });
+
+    it('should recognise ListItem and Task as different', () => {
+        const listItem = new ListItem('- [ ] description', null);
+        const task = fromLine({ line: '- [ ] description' });
+
+        expect(listItem.identicalTo(task)).toEqual(false);
+    });
+});
+
+describe('checking if list item lists are identical', () => {
+    it('should treat empty lists as identical', () => {
+        const list1: ListItem[] = [];
+        const list2: ListItem[] = [];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
+    });
+
+    it('should treat different sized lists as different', () => {
+        const list1: ListItem[] = [];
+        const list2: ListItem[] = [new ListItem('- x', null)];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
+    });
+
+    it('should detect matching list items as same', () => {
+        const list1: ListItem[] = [new ListItem('- 1', null)];
+        const list2: ListItem[] = [new ListItem('- 1', null)];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
+    });
+
+    it('- should detect non-matching list items as different', () => {
+        const list1: ListItem[] = [new ListItem('- 1', null)];
+        const list2: ListItem[] = [new ListItem('- 2', null)];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
+    });
+});
+
+describe('checking if task lists are identical', () => {
+    it('should treat empty lists as identical', () => {
+        const list1: Task[] = [];
+        const list2: Task[] = [];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
+    });
+
+    it('should treat different sized lists as different', () => {
+        const list1: Task[] = [];
+        const list2: Task[] = [new TaskBuilder().build()];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
+    });
+
+    it('should detect matching tasks as same', () => {
+        const list1: Task[] = [new TaskBuilder().description('1').build()];
+        const list2: Task[] = [new TaskBuilder().description('1').build()];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
+    });
+
+    it('should detect non-matching tasks as different', () => {
+        const list1: Task[] = [new TaskBuilder().description('1').build()];
+        const list2: Task[] = [new TaskBuilder().description('2').build()];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
+    });
+});
+
+describe('checking if mixed lists are identical', () => {
+    it('should recognise mixed lists as unequal', () => {
+        const list1 = [new ListItem('- [ ] description', null)];
+        const list2 = [fromLine({ line: '- [ ] description' })];
+
+        expect(ListItem.listsAreIdentical(list1, list1)).toEqual(true);
+        expect(ListItem.listsAreIdentical(list1, list2)).toEqual(false);
+        expect(ListItem.listsAreIdentical(list2, list1)).toEqual(false);
+        expect(ListItem.listsAreIdentical(list2, list2)).toEqual(true);
+    });
+});
diff --git a/tests/Task/ListItemHelpers.ts b/tests/Task/ListItemHelpers.ts
new file mode 100644
index 00000000..a5aa27ce
--- /dev/null
+++ b/tests/Task/ListItemHelpers.ts
@@ -0,0 +1,7 @@
+import { ListItem } from '../../src/Task/ListItem';
+
+export function createChildListItem(originalMarkdown: string, parent: ListItem) {
+    // This exists purely to silence WebStorm about typescript:S1848
+    // See https://sonarcloud.io/organizations/obsidian-tasks-group/rules?open=typescript%3AS1848&rule_key=typescript%3AS1848
+    new ListItem(originalMarkdown, parent);
+}
diff --git a/tests/Task/Task.test.ts b/tests/Task/Task.test.ts
index d52ff887..5e7c87f8 100644
--- a/tests/Task/Task.test.ts
+++ b/tests/Task/Task.test.ts
@@ -22,6 +22,7 @@ import type { TasksDate } from '../../src/DateTime/TasksDate';
 import { example_kanban } from '../Obsidian/__test_data__/example_kanban';
 import { jason_properties } from '../Obsidian/__test_data__/jason_properties';
 import { OnCompletion } from '../../src/Task/OnCompletion';
+import { createChildListItem } from './ListItemHelpers';
 
 window.moment = moment;
 
@@ -1730,30 +1731,21 @@ describe('identicalTo', () => {
         expect(task2.status.identicalTo(task1.status)).toEqual(true);
         expect(task2.identicalTo(task1)).toEqual(true);
     });
-});
 
-describe('checking if task lists are identical', () => {
-    it('should treat empty lists as identical', () => {
-        const list1: Task[] = [];
-        const list2: Task[] = [];
-        expect(Task.tasksListsIdentical(list1, list2)).toBe(true);
-    });
+    it('should recognise different numbers of child items', () => {
+        const task1 = new TaskBuilder().build();
+        const task2 = new TaskBuilder().build();
+        createChildListItem('- child of task2', task2);
 
-    it('should treat different sized lists as different', () => {
-        const list1: Task[] = [];
-        const list2: Task[] = [new TaskBuilder().build()];
-        expect(Task.tasksListsIdentical(list1, list2)).toBe(false);
+        expect(task2.identicalTo(task1)).toEqual(false);
     });
 
-    it('should detect matching tasks as same', () => {
-        const list1: Task[] = [new TaskBuilder().description('1').build()];
-        const list2: Task[] = [new TaskBuilder().description('1').build()];
-        expect(Task.tasksListsIdentical(list1, list2)).toBe(true);
-    });
+    it('should recognise different description in child list items', () => {
+        const task1 = new TaskBuilder().build();
+        const task2 = new TaskBuilder().build();
+        createChildListItem('- child of task1', task1);
+        createChildListItem('- child of task2', task2);
 
-    it('should detect non-matching tasks as different', () => {
-        const list1: Task[] = [new TaskBuilder().description('1').build()];
-        const list2: Task[] = [new TaskBuilder().description('2').build()];
-        expect(Task.tasksListsIdentical(list1, list2)).toBe(false);
+        expect(task2.identicalTo(task1)).toEqual(false);
     });
 });