From f8e0c7dab71744c90415e38f9120951136909a6f Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 24 Sep 2024 15:24:45 -0700 Subject: [PATCH 1/7] test: - Make StatusField.test.ts more robust by registering statuses --- tests/Query/Filter/StatusField.test.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Query/Filter/StatusField.test.ts b/tests/Query/Filter/StatusField.test.ts index 565669cf4b..a77fbd59ad 100644 --- a/tests/Query/Filter/StatusField.test.ts +++ b/tests/Query/Filter/StatusField.test.ts @@ -9,6 +9,8 @@ import { } from '../../CustomMatchers/CustomMatchersForSorting'; import { StatusConfiguration, StatusType } from '../../../src/Statuses/StatusConfiguration'; import { fromLine } from '../../TestingTools/TestHelpers'; +import { StatusRegistry } from '../../../src/Statuses/StatusRegistry'; +import type { StatusCollection } from '../../../src/Statuses/StatusCollection'; describe('status', () => { it('done', () => { @@ -88,6 +90,22 @@ describe('sorting by status', () => { }); describe('grouping by status', () => { + beforeAll(() => { + StatusRegistry.getInstance().resetToDefaultStatuses(); + const importantCycle: StatusCollection = [ + ['!', 'todo', 'X', 'TODO'], + ['X', 'done', '!', 'DONE'], + ]; + importantCycle.forEach((entry) => { + const status = Status.createFromImportedValue(entry); + StatusRegistry.getInstance().add(status); + }); + }); + + afterAll(() => { + StatusRegistry.getInstance().resetToDefaultStatuses(); + }); + it('supports grouping methods correctly', () => { expect(new StatusField()).toSupportGroupingWithProperty('status'); }); @@ -105,6 +123,11 @@ describe('grouping by status', () => { // Assert const tasks = [fromLine({ line: taskLine })]; + + // Check this symbol has been registered, so we are not passing by luck: + const symbol = tasks[0].status.symbol; + expect(StatusRegistry.getInstance().bySymbol(symbol).type).not.toEqual(StatusType.EMPTY); + expect({ grouper, tasks }).groupHeadingsToBe(groups); }); From db2d82fb775eac32c1e632db61224c4370a09d87 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 24 Sep 2024 15:29:51 -0700 Subject: [PATCH 2/7] fix: 'group by status' behaviour now matches its documentation fixes #3030 --- src/Query/Filter/StatusField.ts | 5 +---- ...Statuses.test.Status_Transitions_status-types.approved.md | 2 +- tests/Query/Filter/StatusField.test.ts | 4 ++-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Query/Filter/StatusField.ts b/src/Query/Filter/StatusField.ts index 7715539d13..9e1434ec8a 100644 --- a/src/Query/Filter/StatusField.ts +++ b/src/Query/Filter/StatusField.ts @@ -62,10 +62,7 @@ export class StatusField extends FilterInstructionsBasedField { public grouper(): GrouperFunction { return (task: Task) => { - // Backwards-compatibility note: In Tasks 1.22.0 and earlier, the only - // names used by 'group by status' were 'Todo' and 'Done' - and - // any character other than a space was considered to be 'Done'. - return [StatusField.oldStatusName(task)]; + return [task.isDone ? 'Done' : 'Todo']; }; } } diff --git a/tests/DocumentationSamples/DocsSamplesForStatuses.test.Status_Transitions_status-types.approved.md b/tests/DocumentationSamples/DocsSamplesForStatuses.test.Status_Transitions_status-types.approved.md index d0b803183d..f20029778b 100644 --- a/tests/DocumentationSamples/DocsSamplesForStatuses.test.Status_Transitions_status-types.approved.md +++ b/tests/DocumentationSamples/DocsSamplesForStatuses.test.Status_Transitions_status-types.approved.md @@ -14,7 +14,7 @@ | Matches `status.name includes in progress` | no | YES | no | no | no | | Matches `status.name includes done` | no | no | YES | no | no | | Matches `status.name includes cancelled` | no | no | no | YES | no | -| Name for `group by status` | Todo | Done | Done | Done | Done | +| Name for `group by status` | Todo | Todo | Done | Done | Done | | Name for `group by status.type` | %%2%%TODO | %%1%%IN_PROGRESS | %%3%%DONE | %%4%%CANCELLED | %%5%%NON_TASK | | Name for `group by status.name` | Todo | In Progress | Done | Cancelled | My custom status | diff --git a/tests/Query/Filter/StatusField.test.ts b/tests/Query/Filter/StatusField.test.ts index a77fbd59ad..76a020c288 100644 --- a/tests/Query/Filter/StatusField.test.ts +++ b/tests/Query/Filter/StatusField.test.ts @@ -114,9 +114,9 @@ describe('grouping by status', () => { ['- [ ] a', ['Todo']], ['- [x] a', ['Done']], ['- [X] a', ['Done']], - ['- [/] a', ['Done']], + ['- [/] a', ['Todo']], ['- [-] a', ['Done']], - ['- [!] a', ['Done']], + ['- [!] a', ['Todo']], ])('task "%s" should have groups: %s', (taskLine: string, groups: string[]) => { // Arrange const grouper = new StatusField().createNormalGrouper(); From 6afc9dc8ce78b9ab818428edf6250978b0e65a65 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 24 Sep 2024 15:46:58 -0700 Subject: [PATCH 3/7] test: - Apply custom statuses to all tests in StatusField.test.ts --- tests/Query/Filter/StatusField.test.ts | 32 +++++++++++++------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/Query/Filter/StatusField.test.ts b/tests/Query/Filter/StatusField.test.ts index 76a020c288..1de1c3e323 100644 --- a/tests/Query/Filter/StatusField.test.ts +++ b/tests/Query/Filter/StatusField.test.ts @@ -12,6 +12,22 @@ import { fromLine } from '../../TestingTools/TestHelpers'; import { StatusRegistry } from '../../../src/Statuses/StatusRegistry'; import type { StatusCollection } from '../../../src/Statuses/StatusCollection'; +beforeAll(() => { + StatusRegistry.getInstance().resetToDefaultStatuses(); + const importantCycle: StatusCollection = [ + ['!', 'todo', 'X', 'TODO'], + ['X', 'done', '!', 'DONE'], + ]; + importantCycle.forEach((entry) => { + const status = Status.createFromImportedValue(entry); + StatusRegistry.getInstance().add(status); + }); +}); + +afterAll(() => { + StatusRegistry.getInstance().resetToDefaultStatuses(); +}); + describe('status', () => { it('done', () => { // Arrange @@ -90,22 +106,6 @@ describe('sorting by status', () => { }); describe('grouping by status', () => { - beforeAll(() => { - StatusRegistry.getInstance().resetToDefaultStatuses(); - const importantCycle: StatusCollection = [ - ['!', 'todo', 'X', 'TODO'], - ['X', 'done', '!', 'DONE'], - ]; - importantCycle.forEach((entry) => { - const status = Status.createFromImportedValue(entry); - StatusRegistry.getInstance().add(status); - }); - }); - - afterAll(() => { - StatusRegistry.getInstance().resetToDefaultStatuses(); - }); - it('supports grouping methods correctly', () => { expect(new StatusField()).toSupportGroupingWithProperty('status'); }); From a5fca714a411cc5d3d0f2f94a6e1bd32a0bdd8aa Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 24 Sep 2024 15:56:24 -0700 Subject: [PATCH 4/7] fix: 'sort by status' behaviour now matches its documentation fixes #3030 --- src/Query/Filter/StatusField.ts | 5 +---- tests/Query/Filter/StatusField.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Query/Filter/StatusField.ts b/src/Query/Filter/StatusField.ts index 9e1434ec8a..3781a6abfc 100644 --- a/src/Query/Filter/StatusField.ts +++ b/src/Query/Filter/StatusField.ts @@ -32,9 +32,6 @@ export class StatusField extends FilterInstructionsBasedField { * Return a function to compare two Task objects, for use in sorting by status. */ public comparator(): Comparator { - // Backwards-compatibility note: In Tasks 1.22.0 and earlier, the - // only available status names were 'Todo' and 'Done'. - // And 'Todo' sorted before 'Done'. return (a: Task, b: Task) => { const oldStatusNameA = StatusField.oldStatusName(a); const oldStatusNameB = StatusField.oldStatusName(b); @@ -49,7 +46,7 @@ export class StatusField extends FilterInstructionsBasedField { } private static oldStatusName(a: Task): string { - if (a.status.symbol === ' ') { + if (!a.isDone) { return 'Todo'; } else { return 'Done'; diff --git a/tests/Query/Filter/StatusField.test.ts b/tests/Query/Filter/StatusField.test.ts index 1de1c3e323..a62831a79b 100644 --- a/tests/Query/Filter/StatusField.test.ts +++ b/tests/Query/Filter/StatusField.test.ts @@ -85,13 +85,13 @@ describe('sorting by status', () => { expectTaskComparesBefore(sorter, todoTask, TestHelpers.fromLine({ line: '- [-] Z' })); expectTaskComparesBefore(sorter, todoTask, TestHelpers.fromLine({ line: '- [x] Z' })); expectTaskComparesBefore(sorter, todoTask, TestHelpers.fromLine({ line: '- [X] Z' })); - expectTaskComparesBefore(sorter, todoTask, TestHelpers.fromLine({ line: '- [!] Z' })); + expectTaskComparesEqual(sorter, todoTask, TestHelpers.fromLine({ line: '- [!] Z' })); expectTaskComparesEqual(sorter, doneTask, doneTask); expectTaskComparesEqual(sorter, doneTask, TestHelpers.fromLine({ line: '- [-] Z' })); expectTaskComparesEqual(sorter, doneTask, TestHelpers.fromLine({ line: '- [x] Z' })); expectTaskComparesEqual(sorter, doneTask, TestHelpers.fromLine({ line: '- [X] Z' })); - expectTaskComparesEqual(sorter, doneTask, TestHelpers.fromLine({ line: '- [!] Z' })); + expectTaskComparesAfter(sorter, doneTask, TestHelpers.fromLine({ line: '- [!] Z' })); }); it('sort by status reverse', () => { From d204d490c2bd5ddc3f6600175bf1948dd62a8e9d Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 24 Sep 2024 16:02:31 -0700 Subject: [PATCH 5/7] refactor: - Now 'sort by status' has been updated, can revert grouper() implementation --- src/Query/Filter/StatusField.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Query/Filter/StatusField.ts b/src/Query/Filter/StatusField.ts index 3781a6abfc..0055b31585 100644 --- a/src/Query/Filter/StatusField.ts +++ b/src/Query/Filter/StatusField.ts @@ -59,7 +59,7 @@ export class StatusField extends FilterInstructionsBasedField { public grouper(): GrouperFunction { return (task: Task) => { - return [task.isDone ? 'Done' : 'Todo']; + return [StatusField.oldStatusName(task)]; }; } } From 63ac3d9886270e2f4f18354de5f161767b2ef748 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 24 Sep 2024 16:08:07 -0700 Subject: [PATCH 6/7] docs: Update 'Status Types' page for fix of #3030 --- docs/Getting Started/Statuses/Status Types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Getting Started/Statuses/Status Types.md b/docs/Getting Started/Statuses/Status Types.md index c5f5446cc9..290a469bb4 100644 --- a/docs/Getting Started/Statuses/Status Types.md +++ b/docs/Getting Started/Statuses/Status Types.md @@ -107,7 +107,7 @@ The tasks shown are purely examples for context. The `~` column is just an arbit | Matches `status.name includes in progress` | no | YES | no | no | no | | Matches `status.name includes done` | no | no | YES | no | no | | Matches `status.name includes cancelled` | no | no | no | YES | no | -| Name for `group by status` | Todo | Done | Done | Done | Done | +| Name for `group by status` | Todo | Todo | Done | Done | Done | | Name for `group by status.type` | %%2%%TODO | %%1%%IN_PROGRESS | %%3%%DONE | %%4%%CANCELLED | %%5%%NON_TASK | | Name for `group by status.name` | Todo | In Progress | Done | Cancelled | My custom status | From 26f90e40ffd75dbf0a119951586664dc37064173 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 24 Sep 2024 16:17:15 -0700 Subject: [PATCH 7/7] jsdoc: Clarify StatusField.comparator() and StatusField.grouper() --- src/Query/Filter/StatusField.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Query/Filter/StatusField.ts b/src/Query/Filter/StatusField.ts index 0055b31585..18407854fe 100644 --- a/src/Query/Filter/StatusField.ts +++ b/src/Query/Filter/StatusField.ts @@ -30,6 +30,7 @@ export class StatusField extends FilterInstructionsBasedField { /** * Return a function to compare two Task objects, for use in sorting by status. + * TODO and IN_PROGRESS types are sorted before the other types. */ public comparator(): Comparator { return (a: Task, b: Task) => { @@ -57,6 +58,12 @@ export class StatusField extends FilterInstructionsBasedField { return true; } + /** + * Return a function to name tasks, for use in grouping by status. + * TODO and IN_PROGRESS types are grouped in 'Todo'. + * Other status types are grouped in 'Done'. + */ + public grouper(): GrouperFunction { return (task: Task) => { return [StatusField.oldStatusName(task)];