Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1454 (2023-01-01 20:37:07)

refactor: Update 'Toggle task done' command to use StatusRegistry
<!--- Provide a general summary of your changes in the Title above -->

# Description

Update the 'Toggle task done' command to use StatusRegistry, and honour custom statuses.

This fixes a bug that existed in the obsidian-tasks-x code that:

- when there was a global filter,
- and there was a task that:
    - used a custom status with a Capital letter,
    - and did not match the global filter
- then when the task was toggled with 'Toggle task done':
    - the original custom status character was lower-cased
    - so the wrong next status was chosen.

## Motivation and Context

Moving towards removing `Task.originalStatusCharacter`.

## How has this been tested?

By adding new tests and running existing ones.

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

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)

## 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/Commands/ToggleDone.ts b/src/Commands/ToggleDone.ts
index 2e2a360a..4ffff322 100644
--- a/src/Commands/ToggleDone.ts
+++ b/src/Commands/ToggleDone.ts
@@ -1,4 +1,5 @@
 import { Editor, MarkdownView, View } from 'obsidian';
+import { StatusRegistry } from '../StatusRegistry';
 
 import { Task, TaskRegularExpressions } from '../Task';
 
@@ -76,8 +77,9 @@ export const toggleLine = (line: string, path: string) => {
         const regexMatch = line.match(TaskRegularExpressions.taskRegex);
         if (regexMatch !== null) {
             // Toggle the status of the checklist item.
-            const statusString = regexMatch[3].toLowerCase(); // Note for future: I do not think this toLowerCase is necessary and there is an issue about how it breaks some theme or snippet.
-            const newStatusString = statusString === ' ' ? 'x' : ' ';
+            const statusString = regexMatch[3];
+            const status = StatusRegistry.getInstance().byIndicator(statusString);
+            const newStatusString = status.nextStatusIndicator;
             toggledLine = line.replace(TaskRegularExpressions.taskRegex, `$1- [${newStatusString}] $4`);
         } else if (TaskRegularExpressions.listItemRegex.test(line)) {
             // Convert the list item to a checklist item.
diff --git a/src/Task.ts b/src/Task.ts
index 56457b36..9e2bc41c 100644
--- a/src/Task.ts
+++ b/src/Task.ts
@@ -552,7 +552,7 @@ export class Task {
             ...this,
             status: newStatus,
             doneDate: newDoneDate,
-            originalStatusCharacter: newStatus.isCompleted() ? 'x' : ' ',
+            originalStatusCharacter: newStatus.indicator,
         });
 
         const newTasks: Task[] = [];


Test patch:
diff --git a/tests/Commands/ToggleDone.test.ts b/tests/Commands/ToggleDone.test.ts
index 6393494e..592b8195 100644
--- a/tests/Commands/ToggleDone.test.ts
+++ b/tests/Commands/ToggleDone.test.ts
@@ -5,6 +5,8 @@
 import moment from 'moment';
 import { calculateCursorOffset, toggleLine } from '../../src/Commands/ToggleDone';
 import { resetSettings, updateSettings } from '../../src/Config/Settings';
+import { StatusRegistry } from '../../src/StatusRegistry';
+import { Status, StatusConfiguration } from '../../src/Status';
 
 window.moment = moment;
 
@@ -161,5 +163,54 @@ describe('ToggleDone', () => {
         );
     });
 
+    describe('should honour next status character', () => {
+        afterEach(() => {
+            resetSettings();
+        });
+
+        // Arrange
+        const statusRegistry = StatusRegistry.getInstance();
+        statusRegistry.clearStatuses();
+        statusRegistry.add(new Status(new StatusConfiguration('P', 'Pro', 'C', false)));
+        statusRegistry.add(new Status(new StatusConfiguration('C', 'Con', 'P', false)));
+
+        it('when there is no global filter', () => {
+            const line1 = '- [P] this is a task starting at Pro';
+
+            // Assert
+            const line2 = toggleLine(line1, 'x.md');
+            expect(line2).toStrictEqual('- [C] this is a task starting at Pro');
+
+            const line3 = toggleLine(line2, 'x.md');
+            expect(line3).toStrictEqual('- [P] this is a task starting at Pro');
+        });
+
+        it('when there is a global filter and task with global filter is toggled', () => {
+            updateSettings({ globalFilter: '#task' });
+
+            const line1 = '- [C] #task this is a task starting at Con';
+
+            // Assert
+            const line2 = toggleLine(line1, 'x.md');
+            expect(line2).toStrictEqual('- [P] #task this is a task starting at Con');
+
+            const line3 = toggleLine(line2, 'x.md');
+            expect(line3).toStrictEqual('- [C] #task this is a task starting at Con');
+        });
+
+        it('when there is a global filter and task without global filter is toggled', () => {
+            updateSettings({ globalFilter: '#task' });
+
+            const line1 = '- [P] this is a task starting at Pro, not matching the global filter';
+
+            // Assert
+            const line2 = toggleLine(line1, 'x.md');
+            expect(line2).toStrictEqual('- [C] this is a task starting at Pro, not matching the global filter');
+
+            const line3 = toggleLine(line2, 'x.md');
+            expect(line3).toStrictEqual('- [P] this is a task starting at Pro, not matching the global filter');
+        });
+    });
+
     todaySpy.mockClear();
 });