Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1532 (2023-01-14 17:00:34)

refactor: Add factory methods to StatusConfiguration
<!--- Provide a general summary of your changes in the Title above -->

# Description

<!--- Describe your changes in detail -->

- Create static factory functions/named constrictors in StatusConfiguration for each of the static constants in Status
- Where easy to do so, update existing code to use the new functions

## Motivation and Context

This is working toward making the core status somewhat modifiable.

## How has this been tested?

- Using existing tests, and adding some more.
- Tested manually in Obsidian

## 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/Config/CustomStatusModal.ts b/src/Config/CustomStatusModal.ts
index 359e2955..21dc0b46 100644
--- a/src/Config/CustomStatusModal.ts
+++ b/src/Config/CustomStatusModal.ts
@@ -1,5 +1,4 @@
 import { Modal, Notice, Setting, TextComponent } from 'obsidian';
-import { Status } from '../Status';
 import { StatusConfiguration } from '../StatusConfiguration';
 import type TasksPlugin from '../main';
 
@@ -84,7 +83,7 @@ export class CustomStatusModal extends Modal {
                 CustomStatusModal.setValid(statusNextSymbolText, this.statusConfiguration().validateNextIndicator());
             });
 
-        if (Status.tasksPluginCanCreateCommandsForStatuses()) {
+        if (StatusConfiguration.tasksPluginCanCreateCommandsForStatuses()) {
             new Setting(settingDiv)
                 .setName('Available as command')
                 .setDesc(
diff --git a/src/Config/SettingsTab.ts b/src/Config/SettingsTab.ts
index c7733a76..487d7412 100644
--- a/src/Config/SettingsTab.ts
+++ b/src/Config/SettingsTab.ts
@@ -1,5 +1,4 @@
 import { Notice, PluginSettingTab, Setting, debounce } from 'obsidian';
-import { Status } from 'Status';
 import { StatusConfiguration } from '../StatusConfiguration';
 import type TasksPlugin from '../main';
 import { StatusRegistry } from '../StatusRegistry';
@@ -369,10 +368,10 @@ export class SettingsTab extends PluginSettingTab {
     insertTaskCoreStatusSettings(containerEl: HTMLElement, settings: SettingsTab) {
         // TODO Make these statuses editable
         const coreStatuses: StatusSettings = new StatusSettings();
-        StatusSettings.addCustomStatus(coreStatuses, Status.TODO.configuration);
-        StatusSettings.addCustomStatus(coreStatuses, Status.IN_PROGRESS.configuration);
-        StatusSettings.addCustomStatus(coreStatuses, Status.DONE.configuration);
-        StatusSettings.addCustomStatus(coreStatuses, Status.CANCELLED.configuration);
+        StatusSettings.addCustomStatus(coreStatuses, StatusConfiguration.makeTodo());
+        StatusSettings.addCustomStatus(coreStatuses, StatusConfiguration.makeInProgress());
+        StatusSettings.addCustomStatus(coreStatuses, StatusConfiguration.makeDone());
+        StatusSettings.addCustomStatus(coreStatuses, StatusConfiguration.makeCancelled());
 
         /* -------------------- One row per status in the settings -------------------- */
         coreStatuses.customStatusTypes.forEach((status_type) => {
@@ -464,7 +463,7 @@ function createRowForTaskStatus(
 
     const taskStatusPreview = containerEl.createEl('pre');
     taskStatusPreview.addClass('row-for-status');
-    taskStatusPreview.textContent = StatusSettingsHelpers.statusPreviewText(statusType);
+    taskStatusPreview.textContent = statusType.previewText();
 
     const setting = new Setting(containerEl);
 
diff --git a/src/Config/StatusSettingsHelpers.ts b/src/Config/StatusSettingsHelpers.ts
index 2a202076..1b6f23da 100644
--- a/src/Config/StatusSettingsHelpers.ts
+++ b/src/Config/StatusSettingsHelpers.ts
@@ -3,20 +3,6 @@
  * It intentionally does not import any Obsidian types, so that tests can
  * be written for its contents.
  */
-import { Status } from '../Status';
-import type { StatusConfiguration } from '../StatusConfiguration';
-
-/**
- * Return a one-line summary of the status, for presentation to users.
- * @param status
- */
-export function statusPreviewText(status: StatusConfiguration) {
-    let commandNotice = '';
-    if (Status.tasksPluginCanCreateCommandsForStatuses() && status.availableAsCommand) {
-        commandNotice = 'Available as a command.';
-    }
-    return `- [${status.indicator}] ${status.name}, next status is '${status.nextStatusIndicator}'. ${commandNotice}`;
-}
 
 /**
  * Status supported by the Minimal theme. {@link https://github.com/kepano/obsidian-minimal}
diff --git a/src/Status.ts b/src/Status.ts
index 0d2b0dde..e5ce2402 100644
--- a/src/Status.ts
+++ b/src/Status.ts
@@ -21,7 +21,7 @@ export class Status {
      * @type {Status}
      * @memberof Status
      */
-    public static DONE: Status = new Status(new StatusConfiguration('x', 'Done', ' ', true));
+    public static DONE: Status = new Status(StatusConfiguration.makeDone());
 
     /**
      * A default status of empty, used when things go wrong.
@@ -29,7 +29,7 @@ export class Status {
      * @static
      * @memberof Status
      */
-    public static EMPTY: Status = new Status(new StatusConfiguration('', 'EMPTY', '', true));
+    public static EMPTY: Status = new Status(StatusConfiguration.makeEmpty());
 
     /**
      * The default Todo status. Goes to Done when toggled.
@@ -39,25 +39,7 @@ export class Status {
      * @type {Status}
      * @memberof Status
      */
-    public static TODO: Status = new Status(new StatusConfiguration(' ', 'Todo', 'x', true));
-
-    /**
-     * The default Cancelled status. Goes to Todo when toggled.
-     *
-     * @static
-     * @type {Status}
-     * @memberof Status
-     */
-    public static CANCELLED: Status = new Status(new StatusConfiguration('-', 'Cancelled', ' ', true));
-
-    /**
-     * The default In Progress status. Goes to Done when toggled.
-     *
-     * @static
-     * @type {Status}
-     * @memberof Status
-     */
-    public static IN_PROGRESS: Status = new Status(new StatusConfiguration('/', 'In Progress', 'x', true));
+    public static TODO: Status = new Status(StatusConfiguration.makeTodo());
 
     /**
      * The configuration stored in the data.json file.
@@ -67,17 +49,6 @@ export class Status {
      */
     public readonly configuration: StatusConfiguration;
 
-    /**
-     * Whether Tasks can yet create 'Toggle Status' commands for statuses
-     *
-     * This is not yet possible, and so some UI features are temporarily hidden.
-     * See https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1486
-     * Once that issue is addressed, this method can be removed.
-     */
-    public static tasksPluginCanCreateCommandsForStatuses(): boolean {
-        return false;
-    }
-
     /**
      * The indicator used between the two square brackets in the markdown task.
      *
diff --git a/src/StatusConfiguration.ts b/src/StatusConfiguration.ts
index e5039f92..2a36ff63 100644
--- a/src/StatusConfiguration.ts
+++ b/src/StatusConfiguration.ts
@@ -6,6 +6,42 @@
  * @class StatusConfiguration
  */
 export class StatusConfiguration {
+    /**
+     * The default Done status. Goes to Todo when toggled.
+     */
+    static makeDone(): StatusConfiguration {
+        return new StatusConfiguration('x', 'Done', ' ', true);
+    }
+
+    /**
+     * A default status of empty, used when things go wrong.
+     */
+    static makeEmpty(): StatusConfiguration {
+        return new StatusConfiguration('', 'EMPTY', '', true);
+    }
+
+    /**
+     * The default Todo status. Goes to Done when toggled.
+     * User may later be able to override this to go to In Progress instead.
+     */
+    static makeTodo(): StatusConfiguration {
+        return new StatusConfiguration(' ', 'Todo', 'x', true);
+    }
+
+    /**
+     * The default Cancelled status. Goes to Todo when toggled.
+     */
+    static makeCancelled(): StatusConfiguration {
+        return new StatusConfiguration('-', 'Cancelled', ' ', true);
+    }
+
+    /**
+     * The default In Progress status. Goes to Done when toggled.
+     */
+    static makeInProgress(): StatusConfiguration {
+        return new StatusConfiguration('/', 'In Progress', 'x', true);
+    }
+
     /**
      * The indicator used between the two square brackets in the markdown task.
      *
@@ -55,6 +91,28 @@ export class StatusConfiguration {
         this.availableAsCommand = availableAsCommand;
     }
 
+    /**
+     * Return a one-line summary of the status, for presentation to users.
+     */
+    public previewText() {
+        let commandNotice = '';
+        if (StatusConfiguration.tasksPluginCanCreateCommandsForStatuses() && this.availableAsCommand) {
+            commandNotice = 'Available as a command.';
+        }
+        return `- [${this.indicator}] ${this.name}, next status is '${this.nextStatusIndicator}'. ${commandNotice}`;
+    }
+
+    /**
+     * Whether Tasks can yet create 'Toggle Status' commands for statuses
+     *
+     * This is not yet possible, and so some UI features are temporarily hidden.
+     * See https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1486
+     * Once that issue is addressed, this method can be removed.
+     */
+    public static tasksPluginCanCreateCommandsForStatuses(): boolean {
+        return false;
+    }
+
     /**
      * Determine whether the date in this object is valid, and return error message(s) for display if not.
      */
diff --git a/src/StatusRegistry.ts b/src/StatusRegistry.ts
index d34e8707..032ce119 100644
--- a/src/StatusRegistry.ts
+++ b/src/StatusRegistry.ts
@@ -1,5 +1,5 @@
 import { Status } from './Status';
-import type { StatusConfiguration } from './StatusConfiguration';
+import { StatusConfiguration } from './StatusConfiguration';
 
 /**
  * Tracks all the registered statuses a task can have.
@@ -170,7 +170,13 @@ export class StatusRegistry {
      * @memberof StatusRegistry
      */
     private addDefaultStatusTypes(): void {
-        const defaultStatuses = [Status.TODO, Status.IN_PROGRESS, Status.DONE, Status.CANCELLED, Status.EMPTY];
+        const defaultStatuses = [
+            new Status(StatusConfiguration.makeTodo()),
+            new Status(StatusConfiguration.makeInProgress()),
+            new Status(StatusConfiguration.makeDone()),
+            new Status(StatusConfiguration.makeCancelled()),
+            new Status(StatusConfiguration.makeEmpty()),
+        ];
 
         defaultStatuses.forEach((status) => {
             this.add(status);


Test patch:
diff --git a/tests/Status.test.ts b/tests/Status.test.ts
index 9e216f49..046bc347 100644
--- a/tests/Status.test.ts
+++ b/tests/Status.test.ts
@@ -9,6 +9,12 @@ jest.mock('obsidian');
 window.moment = moment;
 
 describe('Status', () => {
+    describe('default configurations', () => {
+        expect(Status.DONE.configuration.previewText()).toEqual("- [x] Done, next status is ' '. ");
+        expect(Status.EMPTY.configuration.previewText()).toEqual("- [] EMPTY, next status is ''. ");
+        expect(Status.TODO.configuration.previewText()).toEqual("- [ ] Todo, next status is 'x'. ");
+    });
+
     it('should initialize with valid properties', () => {
         // Arrange
         const indicator = '/';
diff --git a/tests/StatusConfiguration.test.ts b/tests/StatusConfiguration.test.ts
index 51605683..22546faa 100644
--- a/tests/StatusConfiguration.test.ts
+++ b/tests/StatusConfiguration.test.ts
@@ -1,6 +1,19 @@
 import { StatusConfiguration } from '../src/StatusConfiguration';
 
 describe('StatusConfiguration', () => {
+    describe('preview text', () => {
+        const configuration = new StatusConfiguration('P', 'Pro', 'Con', true);
+        expect(configuration.previewText()).toEqual("- [P] Pro, next status is 'Con'. ");
+    });
+
+    describe('factory methods for default statuses', () => {
+        expect(StatusConfiguration.makeDone().previewText()).toEqual("- [x] Done, next status is ' '. ");
+        expect(StatusConfiguration.makeEmpty().previewText()).toEqual("- [] EMPTY, next status is ''. ");
+        expect(StatusConfiguration.makeTodo().previewText()).toEqual("- [ ] Todo, next status is 'x'. ");
+        expect(StatusConfiguration.makeCancelled().previewText()).toEqual("- [-] Cancelled, next status is ' '. ");
+        expect(StatusConfiguration.makeInProgress().previewText()).toEqual("- [/] In Progress, next status is 'x'. ");
+    });
+
     function checkValidation(statusConfiguration: StatusConfiguration, expectedMessages: string[]) {
         const errors = statusConfiguration.validate();
         expect(errors).toEqual(expectedMessages);
diff --git a/tests/StatusRegistry.test.ts b/tests/StatusRegistry.test.ts
index 81760a98..e33b2b35 100644
--- a/tests/StatusRegistry.test.ts
+++ b/tests/StatusRegistry.test.ts
@@ -33,16 +33,16 @@ describe('StatusRegistry', () => {
         // Assert
         expect(statusRegistry).not.toBeNull();
 
-        expect(doneStatus.indicator).toEqual(Status.DONE.indicator);
+        expect(doneStatus.indicator).toEqual(StatusConfiguration.makeDone().indicator);
 
-        expect(statusRegistry.byIndicator('x').indicator).toEqual(Status.DONE.indicator);
-        expect(statusRegistry.byIndicator('').indicator).toEqual(Status.EMPTY.indicator);
-        expect(statusRegistry.byIndicator(' ').indicator).toEqual(Status.TODO.indicator);
-        expect(statusRegistry.byIndicator('-').indicator).toEqual(Status.CANCELLED.indicator);
-        expect(statusRegistry.byIndicator('/').indicator).toEqual(Status.IN_PROGRESS.indicator);
+        expect(statusRegistry.byIndicator('x').indicator).toEqual(StatusConfiguration.makeDone().indicator);
+        expect(statusRegistry.byIndicator('').indicator).toEqual(StatusConfiguration.makeEmpty().indicator);
+        expect(statusRegistry.byIndicator(' ').indicator).toEqual(StatusConfiguration.makeTodo().indicator);
+        expect(statusRegistry.byIndicator('-').indicator).toEqual(StatusConfiguration.makeCancelled().indicator);
+        expect(statusRegistry.byIndicator('/').indicator).toEqual(StatusConfiguration.makeInProgress().indicator);
 
         // Detect unrecognised indicator:
-        expect(statusRegistry.byIndicator('?').indicator).toEqual(Status.EMPTY.indicator);
+        expect(statusRegistry.byIndicator('?').indicator).toEqual(StatusConfiguration.makeEmpty().indicator);
     });
 
     it('should allow lookup of next status for a status', () => {
@@ -112,17 +112,17 @@ describe('StatusRegistry', () => {
 
             // Assert
             expect(task).not.toBeNull();
-            expect(task!.status.indicator).toEqual(Status.TODO.indicator);
+            expect(task!.status.indicator).toEqual(StatusConfiguration.makeTodo().indicator);
 
             // In Tasks, TODO toggles to DONE, for consistency with earlier releases.
             // const toggledInProgress = task?.toggle()[0];
             // expect(toggledInProgress?.status.indicator).toEqual(Status.IN_PROGRESS.indicator);
 
             const toggledDone = task?.toggle()[0];
-            expect(toggledDone?.status.indicator).toEqual(Status.DONE.indicator);
+            expect(toggledDone?.status.indicator).toEqual(StatusConfiguration.makeDone().indicator);
 
             const toggledTodo = toggledDone?.toggle()[0];
-            expect(toggledTodo?.status.indicator).toEqual(Status.TODO.indicator);
+            expect(toggledTodo?.status.indicator).toEqual(StatusConfiguration.makeTodo().indicator);
         });
 
         it('should allow task to toggle from cancelled to todo', () => {
@@ -147,10 +147,10 @@ describe('StatusRegistry', () => {
 
             // Assert
             expect(task).not.toBeNull();
-            expect(task!.status.indicator).toEqual(Status.CANCELLED.indicator);
+            expect(task!.status.indicator).toEqual(StatusConfiguration.makeCancelled().indicator);
 
             const toggledTodo = task?.toggle()[0];
-            expect(toggledTodo?.status.indicator).toEqual(Status.TODO.indicator);
+            expect(toggledTodo?.status.indicator).toEqual(StatusConfiguration.makeTodo().indicator);
         });
 
         it('should allow task to toggle through custom transitions', () => {