Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1545 (2023-01-16 22:35:41)

fix: 'Add All Unknown Status Types' was creating duplicates
<!--- Provide a general summary of your changes in the Title above -->

# Description

Fix issue that 'Add All Unknown Status Types' was creating duplicates.

I thought that Set would prevent duplicates, if there were several tasks with an un-registered status symbol. It didn't.

## Motivation and Context

See above.

## How has this been tested?

With better tests.
And testing it manually in my main vault.

## Screenshots (if appropriate)

What the bug looked like. There should have been only 1 of each unique row.

![image](https://user-images.githubusercontent.com/4840096/212775083-a5196889-026c-48c5-84a1-8c4ffb36e068.png)


## 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)

Internal changes:

- [x] **Tests** (prefix: `test` - additions and improvements to unit tests and the smoke tests)

## 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`.
- [ ] My change requires a change to the documentation.
- [ ] I have [updated the documentation](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#updating-documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#maintaining-the-tests).

## Terms

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md) 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 follow this project's [contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md)
- [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/StatusRegistry.ts b/src/StatusRegistry.ts
index e66b195e..412ba290 100644
--- a/src/StatusRegistry.ts
+++ b/src/StatusRegistry.ts
@@ -174,27 +174,43 @@ export class StatusRegistry {
             return task.status;
         });
 
-        const uniqueStatuses = [...new Set(allStatuses)];
-        console.log(uniqueStatuses);
-
-        const unknownStatuses = uniqueStatuses.filter((s) => {
+        const unknownStatuses = allStatuses.filter((s) => {
             return !this.hasIndicator(s.indicator);
         });
 
+        // Use a separate StatusRegistry to keep track of duplicates,
+        // because Set is no use to us:
+        // https://stackoverflow.com/questions/29759480/how-to-customize-object-equality-for-javascript-set
+        const newStatusRegistry = new StatusRegistry();
+
         const namedUniqueStatuses: Status[] = [];
         unknownStatuses.forEach((s) => {
-            const statusConfiguration = new StatusConfiguration(
-                s.indicator,
-                `Unknown (${s.indicator})`,
-                s.nextStatusIndicator,
-                s.availableAsCommand,
-                s.type,
-            );
-            namedUniqueStatuses.push(new Status(statusConfiguration));
+            // Check if we have seen this indicator already:
+            if (newStatusRegistry.hasIndicator(s.indicator)) {
+                return;
+            }
+
+            // Go ahead and create a suitably-named copy,
+            // including the indicator in the name.
+            const newStatus = StatusRegistry.copyStatusWithNewName(s, `Unknown (${s.indicator})`);
+            namedUniqueStatuses.push(newStatus);
+            // And add it to our local registry, to prevent duplicates.
+            newStatusRegistry.add(newStatus);
         });
         return namedUniqueStatuses;
     }
 
+    private static copyStatusWithNewName(s: Status, newName: string) {
+        const statusConfiguration = new StatusConfiguration(
+            s.indicator,
+            newName,
+            s.nextStatusIndicator,
+            s.availableAsCommand,
+            s.type,
+        );
+        return new Status(statusConfiguration);
+    }
+
     /**
      * Filters the Status types by the indicator and returns the first one found.
      *


Test patch:
diff --git a/tests/StatusRegistry.test.ts b/tests/StatusRegistry.test.ts
index 4e8154f3..886fe7b8 100644
--- a/tests/StatusRegistry.test.ts
+++ b/tests/StatusRegistry.test.ts
@@ -254,12 +254,20 @@ describe('StatusRegistry', () => {
                 new TaskBuilder().statusValues('!', 'Unknown', 'X', false, StatusType.TODO).build(),
                 new TaskBuilder().statusValues('X', 'Unknown', '!', false, StatusType.DONE).build(),
                 new TaskBuilder().statusValues('d', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
+                // Include some tasks with duplicate statuses, to make sure duplicates are discarded
+                new TaskBuilder().statusValues('!', 'Unknown', 'X', false, StatusType.TODO).build(),
+                new TaskBuilder().statusValues('X', 'Unknown', '!', false, StatusType.DONE).build(),
+                new TaskBuilder().statusValues('d', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
+                // Check that it does not add copies of any core statuses
+                new TaskBuilder().statusValues('-', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
             ];
 
             // Act
             const unknownStatuses = registry.findUnknownStatuses(tasks);
 
             // Assert
+            expect(unknownStatuses.length).toEqual(3);
+
             let s1;
             s1 = unknownStatuses[0];
             expect(s1.type).toEqual(StatusType.TODO);