Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2399 (2023-11-06 20:51:19)

feat: "Review and check your Statuses" now shows any problems
<!--- Provide a general summary of your changes in the Title above -->

# Description

Added an extra column to "Review and check your Statuses" report, that shows things worth investigating.

## Motivation and Context

- To help with user support.

<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->

## How has this been tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->

- Manually in the demo vault
- In new tests

## Screenshots (if appropriate)

This is the report that's generated in Unit Tests, with a variety of problem data cases:

![image](https://github.com/obsidian-tasks-group/obsidian-tasks/assets/4840096/3f36a7da-cafa-4193-b634-6cf39d68b05a)


## 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] **New feature** (prefix: `feat` - non-breaking change which adds functionality)

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`.
- [x] My change requires a change to the documentation.
- [ ] I have [updated the documentation](https://publish.obsidian.md/tasks-contributing/Documentation/About+Documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://publish.obsidian.md/tasks-contributing/Testing/About+Testing).

## Terms

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://publish.obsidian.md/tasks-contributing) 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://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/docs/Getting Started/Statuses/Recurring Tasks and Custom Statuses.md b/docs/Getting Started/Statuses/Recurring Tasks and Custom Statuses.md
index 6ff104f7..ab085fd8 100644
--- a/docs/Getting Started/Statuses/Recurring Tasks and Custom Statuses.md
+++ b/docs/Getting Started/Statuses/Recurring Tasks and Custom Statuses.md
@@ -100,7 +100,7 @@ Let's look at the **second of those two lines**:
   - ... because the next status symbol after `x` is ``.
 - And the Due date has advanced a day.
 
-## When DONE is not followed by TODO
+## When DONE is not followed by TODO or IN_PROGRESS
 
 In the following example, `DONE` is followed by `CANCELLED`.
 
diff --git a/src/StatusRegistryReport.ts b/src/StatusRegistryReport.ts
index 949aeefb..1a240a50 100644
--- a/src/StatusRegistryReport.ts
+++ b/src/StatusRegistryReport.ts
@@ -49,6 +49,12 @@ ${mermaidText}`;
 }
 
 export function getPrintableSymbol(symbol: string) {
+    // Do not put backticks around an empty symbol, as the two backticks are rendered
+    // by Obsidian as ordinary characters and the meaning is unclear.
+    // Better to just display nothing in this situation.
+    if (symbol === '') {
+        return symbol;
+    }
     const result = symbol !== ' ' ? symbol : 'space';
     return '`' + result + '`';
 }
diff --git a/src/StatusSettingsReport.ts b/src/StatusSettingsReport.ts
index 01ff4a9f..fffadb85 100644
--- a/src/StatusSettingsReport.ts
+++ b/src/StatusSettingsReport.ts
@@ -2,21 +2,105 @@ import { StatusSettings } from './Config/StatusSettings';
 import { MarkdownTable } from './lib/MarkdownTable';
 import type { StatusConfiguration } from './StatusConfiguration';
 import { getPrintableSymbol } from './StatusRegistryReport';
+import { Status } from './Status';
+import { StatusType } from './StatusConfiguration';
+
+function getFirstIndex(statusConfigurations: StatusConfiguration[], wantedSymbol: string) {
+    return statusConfigurations.findIndex((s) => s.symbol === wantedSymbol);
+}
+
+function checkIfConventionalType(status: StatusConfiguration, problems: string[]) {
+    // Check if conventional type is being used:
+    const conventionalType = Status.getTypeForUnknownSymbol(status.symbol);
+    if (status.type === conventionalType) {
+        return;
+    }
+
+    if (conventionalType === StatusType.TODO && status.symbol !== ' ') {
+        // This was likely a default TODO - ignore it.
+        return;
+    }
+
+    problems.push(
+        `For information, the conventional type for status symbol ${getPrintableSymbol(
+            status.symbol,
+        )} is ${getPrintableSymbol(conventionalType)}: you may wish to review this type.`,
+    );
+}
+
+function checkNextStatusSymbol(statuses: StatusConfiguration[], status: StatusConfiguration, problems: string[]) {
+    // Check if next symbol is known
+    const indexOfNextSymbol = getFirstIndex(statuses, status.nextStatusSymbol);
+    if (indexOfNextSymbol === -1) {
+        problems.push(
+            `Next symbol ${getPrintableSymbol(
+                status.nextStatusSymbol,
+            )} is unknown: create a status with symbol ${getPrintableSymbol(status.nextStatusSymbol)}.`,
+        );
+        return;
+    }
+
+    if (status.type !== StatusType.DONE) {
+        return;
+    }
+
+    // This type is DONE: check that next status type is TODO or IN_PROGRESS.
+    // See issues #2089 and #2304.
+    const nextStatus = statuses[indexOfNextSymbol];
+    if (nextStatus) {
+        if (nextStatus.type !== 'TODO' && nextStatus.type !== 'IN_PROGRESS') {
+            const helpURL =
+                'https://publish.obsidian.md/tasks/Getting+Started/Statuses/Recurring+Tasks+and+Custom+Statuses';
+            problems.push(
+                `This \`DONE\` status is followed by ${getPrintableSymbol(
+                    nextStatus.type,
+                )}, not \`TODO\` or \`IN_PROGRESS\`: this will not work well for recurring tasks. See [Recurring Tasks and Custom Statuses](${helpURL}).`,
+            );
+        }
+    } else {
+        problems.push('Unexpected failure to find the next status.');
+    }
+}
+
+function getProblemsForStatus(statuses: StatusConfiguration[], status: StatusConfiguration, index: number) {
+    const problems: string[] = [];
+    if (status.symbol === Status.EMPTY.symbol) {
+        problems.push('Empty symbol: this status will be ignored.');
+        return problems;
+    }
+
+    const firstIndex = getFirstIndex(statuses, status.symbol);
+    if (firstIndex != index) {
+        problems.push(`Duplicate symbol '${getPrintableSymbol(status.symbol)}': this status will be ignored.`);
+        return problems;
+    }
+
+    checkIfConventionalType(status, problems);
+    checkNextStatusSymbol(statuses, status, problems);
+    return problems;
+}
 
 export function tabulateStatusSettings(statusSettings: StatusSettings) {
     // Note: There is very similar code in verifyStatusesAsMarkdownTable() in DocsSamplesForStatuses.test.ts.
     //       Maybe try unifying the common code one day?
 
-    const table = new MarkdownTable(['Status Symbol', 'Next Status Symbol', 'Status Name', 'Status Type']);
+    const table = new MarkdownTable([
+        'Status Symbol',
+        'Next Status Symbol',
+        'Status Name',
+        'Status Type',
+        'Problems (if any)',
+    ]);
 
     const statuses: StatusConfiguration[] = StatusSettings.allStatuses(statusSettings);
-    for (const status of statuses) {
+    statuses.forEach((status, index) => {
         table.addRow([
             getPrintableSymbol(status.symbol),
             getPrintableSymbol(status.nextStatusSymbol),
             status.name,
             getPrintableSymbol(status.type),
+            getProblemsForStatus(statuses, status, index).join('<br>'),
         ]);
-    }
+    });
     return table.markdown;
 }


Test patch:
diff --git a/tests/StatusRegistryReport.test.StatusRegistryReport_should_create_a_report.approved.md b/tests/StatusRegistryReport.test.StatusRegistryReport_should_create_a_report.approved.md
index e7b54940..abb854c1 100644
--- a/tests/StatusRegistryReport.test.StatusRegistryReport_should_create_a_report.approved.md
+++ b/tests/StatusRegistryReport.test.StatusRegistryReport_should_create_a_report.approved.md
@@ -21,15 +21,15 @@ Obsidian may only render the table correctly in Reading Mode.
 
 These are the status values in the Core and Custom statuses sections.
 
-| Status Symbol | Next Status Symbol | Status Name | Status Type |
-| ----- | ----- | ----- | ----- |
-| `space` | `x` | Todo | `TODO` |
-| `x` | `space` | Done | `DONE` |
-| `/` | `x` | In Progress | `IN_PROGRESS` |
-| `-` | `space` | Cancelled | `CANCELLED` |
-| `Q` | `A` | Question | `NON_TASK` |
-| `A` | `Q` | Answer | `NON_TASK` |
-| `` | `` |  | `TODO` |
+| Status Symbol | Next Status Symbol | Status Name | Status Type | Problems (if any) |
+| ----- | ----- | ----- | ----- | ----- |
+| `space` | `x` | Todo | `TODO` |  |
+| `x` | `space` | Done | `DONE` |  |
+| `/` | `x` | In Progress | `IN_PROGRESS` |  |
+| `-` | `space` | Cancelled | `CANCELLED` |  |
+| `Q` | `A` | Question | `NON_TASK` |  |
+| `A` | `Q` | Answer | `NON_TASK` |  |
+|  |  |  | `TODO` | Empty symbol: this status will be ignored. |
 
 ## Loaded Settings
 
diff --git a/tests/StatusSettingsReport.test.StatusSettingsReport_should_include_problems_in_table.approved.md b/tests/StatusSettingsReport.test.StatusSettingsReport_should_include_problems_in_table.approved.md
new file mode 100644
index 00000000..82770e0b
--- /dev/null
+++ b/tests/StatusSettingsReport.test.StatusSettingsReport_should_include_problems_in_table.approved.md
@@ -0,0 +1,16 @@
+| Status Symbol | Next Status Symbol | Status Name | Status Type | Problems (if any) |
+| ----- | ----- | ----- | ----- | ----- |
+| `space` | `x` | Todo | `TODO` |  |
+| `x` | `space` | Done | `DONE` |  |
+| `/` | `x` | In Progress | `IN_PROGRESS` |  |
+| `/` | `x` | In Progress DUPLICATE | `IN_PROGRESS` | Duplicate symbol '`/`': this status will be ignored. |
+| `X` | `space` | X - conventionally DONE, but this is CANCELLED | `CANCELLED` | For information, the conventional type for status symbol `X` is `DONE`: you may wish to review this type. |
+|  |  |  | `TODO` | Empty symbol: this status will be ignored. |
+| `p` | `q` | Unknown next symbol | `TODO` | Next symbol `q` is unknown: create a status with symbol `q`. |
+| `c` | `d` | Followed by d | `TODO` | Next symbol `d` is unknown: create a status with symbol `d`. |
+| `n` | `n` | Non-task | `NON_TASK` |  |
+| `1` | `space` | DONE followed by TODO | `DONE` |  |
+| `2` | `/` | DONE followed by IN_PROGRESS | `DONE` |  |
+| `3` | `x` | DONE followed by DONE | `DONE` | This `DONE` status is followed by `DONE`, not `TODO` or `IN_PROGRESS`: this will not work well for recurring tasks. See [Recurring Tasks and Custom Statuses](https://publish.obsidian.md/tasks/Getting+Started/Statuses/Recurring+Tasks+and+Custom+Statuses). |
+| `4` | `X` | DONE followed by CANCELLED | `DONE` | This `DONE` status is followed by `CANCELLED`, not `TODO` or `IN_PROGRESS`: this will not work well for recurring tasks. See [Recurring Tasks and Custom Statuses](https://publish.obsidian.md/tasks/Getting+Started/Statuses/Recurring+Tasks+and+Custom+Statuses). |
+| `5` | `n` | DONE followed by NON_TASK | `DONE` | This `DONE` status is followed by `NON_TASK`, not `TODO` or `IN_PROGRESS`: this will not work well for recurring tasks. See [Recurring Tasks and Custom Statuses](https://publish.obsidian.md/tasks/Getting+Started/Statuses/Recurring+Tasks+and+Custom+Statuses). |
diff --git a/tests/StatusSettingsReport.test.StatusSettingsReport_should_tabulate_StatusSettings.approved.md b/tests/StatusSettingsReport.test.StatusSettingsReport_should_tabulate_StatusSettings.approved.md
index 767ed174..fa1aa5a0 100644
--- a/tests/StatusSettingsReport.test.StatusSettingsReport_should_tabulate_StatusSettings.approved.md
+++ b/tests/StatusSettingsReport.test.StatusSettingsReport_should_tabulate_StatusSettings.approved.md
@@ -1,6 +1,6 @@
-| Status Symbol | Next Status Symbol | Status Name | Status Type |
-| ----- | ----- | ----- | ----- |
-| `space` | `x` | Todo | `TODO` |
-| `x` | `space` | Done | `DONE` |
-| `/` | `x` | In Progress | `IN_PROGRESS` |
-| `-` | `space` | Cancelled | `CANCELLED` |
+| Status Symbol | Next Status Symbol | Status Name | Status Type | Problems (if any) |
+| ----- | ----- | ----- | ----- | ----- |
+| `space` | `x` | Todo | `TODO` |  |
+| `x` | `space` | Done | `DONE` |  |
+| `/` | `x` | In Progress | `IN_PROGRESS` |  |
+| `-` | `space` | Cancelled | `CANCELLED` |  |
diff --git a/tests/StatusSettingsReport.test.ts b/tests/StatusSettingsReport.test.ts
index ac0e6ce7..67777077 100644
--- a/tests/StatusSettingsReport.test.ts
+++ b/tests/StatusSettingsReport.test.ts
@@ -1,6 +1,8 @@
 import { StatusSettings } from '../src/Config/StatusSettings';
 import { tabulateStatusSettings } from '../src/StatusSettingsReport';
+import type { StatusCollection } from '../src/StatusCollection';
 import { verifyWithFileExtension } from './TestingTools/ApprovalTestHelpers';
+import { coreStatusesData, createStatuses } from './TestingTools/StatusesTestHelpers';
 
 describe('StatusSettingsReport', () => {
     it('should tabulate StatusSettings', () => {
@@ -8,4 +10,25 @@ describe('StatusSettingsReport', () => {
         const markdown = tabulateStatusSettings(statusSettings);
         verifyWithFileExtension(markdown, '.md');
     });
+
+    it('should include problems in table', () => {
+        const customStatusesData: StatusCollection = [
+            ['/', 'In Progress', 'x', 'IN_PROGRESS'],
+            ['/', 'In Progress DUPLICATE', 'x', 'IN_PROGRESS'],
+            ['X', 'X - conventionally DONE, but this is CANCELLED', ' ', 'CANCELLED'],
+            ['', '', '', 'TODO'], // A new, unedited status
+            ['p', 'Unknown next symbol', 'q', 'TODO'],
+            ['c', 'Followed by d', 'd', 'TODO'],
+            ['n', 'Non-task', 'n', 'NON_TASK'],
+            ['1', 'DONE followed by TODO', ' ', 'DONE'],
+            ['2', 'DONE followed by IN_PROGRESS', '/', 'DONE'],
+            ['3', 'DONE followed by DONE', 'x', 'DONE'],
+            ['4', 'DONE followed by CANCELLED', 'X', 'DONE'],
+            ['5', 'DONE followed by NON_TASK', 'n', 'DONE'],
+        ];
+        const { statusSettings } = createStatuses(coreStatusesData, customStatusesData);
+
+        const markdown = tabulateStatusSettings(statusSettings);
+        verifyWithFileExtension(markdown, '.md');
+    });
 });