Repository: obsidian-tasks-group/obsidian-tasks (ts)

Issue:
`sort by due` and similar should put tasks with invalid dates before the tasks with valid dates.
### Please check that this issue hasn't been reported before.

- [X] I searched previous [Bug Reports](https://github.com/obsidian-tasks-group/obsidian-tasks/issues?q=is%3Aopen+is%3Aissue+label%3A%22type%3A+bug%22+sort%3Acomments-desc) didn't find any similar reports.

### Expected Behavior

_Note: This issue refers to `due` dates, but the same applies to all the date fields._

`sort by due` and similar should put tasks with invalid dates before the tasks with valid dates, like this:

1. Tasks with invalid due dates
1. Tasks with valid due dates, earliest to latest
1. Tasks with no dates.




### Current behaviour

The sort order is currently:

1. Tasks with valid due dates, earliest to latest
2. Tasks with invalid due dates
3. Tasks with no dates.


### Steps to reproduce

1. Create a note, and call it `Invalid dates with Sorting and Grouping`
2. View it in an editing mode
3. Use `Paste and Match Style` in the following content:

~~~
# Invalid dates with Sorting and Grouping

## Tasks

Numbers indicate the sort order I feel is useful, as invalid dates require action to fix them, so they should be prioritised.

- [ ] #task 1 invalid due date 📅 1999-02-32
- [ ] #task 2 valid over-due date 📅 2023-01-01
- [ ] #task 3 valid future-due date 📅 2050-01-01
- [ ] #task 4 undated

## Sort by due - built-in

The invalid task should be before the dated ones.

```tasks
sort by due

path includes {{query.file.path}}
hide backlinks
hide postpone button
hide task count
```
~~~

4. View the file in Live Preview or Reading mode.

**Actual output**

![image](https://github.com/obsidian-tasks-group/obsidian-tasks/assets/4840096/ea4d0ca7-bca8-492f-a95a-fa17685fed58)


**Expected output**

![image](https://github.com/obsidian-tasks-group/obsidian-tasks/assets/4840096/7900e87f-dcbe-4bf4-8dd8-9a79c983a4f7)


### Which Operating Systems are you using?

- [ ] Android
- [ ] iPhone/iPad
- [ ] Linux
- [X] macOS
- [ ] Windows

### Obsidian Version

1.5.3

### Tasks Plugin Version

5.6.0

### Checks

- [ ] I have tried it with all other plugins disabled and the error still occurs

### Possible solution

Change the logic in `compareByDate()` in [DateTools.ts](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/src/lib/DateTools.ts) so that invalid dates are sorted before valid ones.

But before doing that, use refactoring to unwrap the else conditions, to reduce the nesting and simplify the code.



Pull request:
fix!!: Sort invalid dates before valid ones in query results
<!--- Provide a general summary of your changes in the Title above -->

# Description

- `sort by due` - and sort by other date fields - now puts tasks with an invalid date before the valid ones, so invalid dates are more visible in long lists.
- update documentation to record this change

I'm treating this as a **breaking change** because it does change search results, although I am confident it is an improvement.

## Motivation and Context

Fix #2589.

## 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. -->

- Updating existing tests
- Exploratory testing

## Screenshots (if appropriate)

## 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] **Breaking change** (prefix: `feat!!` or `fix!!` - fix or feature that would cause existing functionality to not work as expected)
- [x] **Documentation** (prefix: `docs` - improvements to any documentation content **for users**)
- [x] **Sample vault** (prefix: `vault` - improvements to the [Tasks-Demo sample vault](https://github.com/obsidian-tasks-group/obsidian-tasks/tree/main/resources/sample_vaults/Tasks-Demo))

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.
- [x] 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/Queries/Sorting.md b/docs/Queries/Sorting.md
index 1c60f8fb..a5bb09b3 100644
--- a/docs/Queries/Sorting.md
+++ b/docs/Queries/Sorting.md
@@ -124,6 +124,17 @@ sort by function task.status.nextSymbol
 
 ## Sort by Dates in Tasks
 
+### How dates are sorted
+
+When sorting tasks by date, such as with `sort by due`, tasks are sorted in this order:
+
+1. Tasks with **invalid** `due` dates come first
+2. Tasks with valid `due` dates, **earliest** to **latest**
+3. Tasks with **no due date** come last.
+
+> [!NOTE]
+> Prior to Tasks X.Y.Z, tasks with invalid dates were sorted **after** the tasks with valid dates.
+
 ### Done Date
 
 - `sort by done` (the date when the task was done)
diff --git a/docs/Support and Help/Breaking Changes.md b/docs/Support and Help/Breaking Changes.md
index 1216967c..e0da7548 100644
--- a/docs/Support and Help/Breaking Changes.md
+++ b/docs/Support and Help/Breaking Changes.md
@@ -27,3 +27,4 @@ To help users updating across multiple Tasks releases, we collect here links to
   - The meaning of final backslash (`\`) characters on query lines [[Line Continuations#Appendix Updating pre-5.0.0 searches with trailing backslashes|has changed]].
 - Tasks [X.Y.Z](https://github.com/obsidian-tasks-group/obsidian-tasks/releases/tag/X.Y.Z) (21 January 2024):
   - Code snippets used to change the look of the edit and postpone buttons must be changed, as explained in [[How to style buttons]], which gives several examples.
+  - When sorting tasks, invalid dates are now sorted before valid dates. See [[Sorting#How dates are sorted]].
diff --git a/src/lib/DateTools.ts b/src/lib/DateTools.ts
index b7558322..32fca914 100644
--- a/src/lib/DateTools.ts
+++ b/src/lib/DateTools.ts
@@ -10,9 +10,9 @@ export function compareByDate(a: moment.Moment | null, b: moment.Moment | null):
     }
 
     if (a.isValid() && !b.isValid()) {
-        return -1;
-    } else if (!a.isValid() && b.isValid()) {
         return 1;
+    } else if (!a.isValid() && b.isValid()) {
+        return -1;
     }
 
     if (a.isAfter(b)) {


Test patch:
diff --git a/resources/sample_vaults/Tasks-Demo/Manual Testing/Invalid dates with Sorting and Grouping.md b/resources/sample_vaults/Tasks-Demo/Manual Testing/Invalid dates with Sorting and Grouping.md
index b4b774ce..68c63049 100644
--- a/resources/sample_vaults/Tasks-Demo/Manual Testing/Invalid dates with Sorting and Grouping.md
+++ b/resources/sample_vaults/Tasks-Demo/Manual Testing/Invalid dates with Sorting and Grouping.md
@@ -45,9 +45,9 @@ hide task count
 
 ### Sort by due - built-in
 
-The invalid task should be before the dated ones.
+The invalid task is sorted before the dated ones, since Tasks X.Y.Z.
 
-[Bug report](https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2589): **`sort by due` and similar should put tasks with invalid dates before the tasks with valid dates.**
+==FIXED==: [Bug report](https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2589): **`sort by due` and similar should put tasks with invalid dates before the tasks with valid dates.**
 
 ```tasks
 sort by due
diff --git a/tests/lib/DateTools.test.ts b/tests/lib/DateTools.test.ts
index d7e7344b..7599ce71 100644
--- a/tests/lib/DateTools.test.ts
+++ b/tests/lib/DateTools.test.ts
@@ -29,6 +29,7 @@ describe('compareBy', () => {
 
         expectDateComparesBefore(invalidDate, null); // invalid dates sort before no date
         expectDateComparesEqual(invalidDate, invalidDate);
-        expectDateComparesAfter(invalidDate, earlierDate); // invalid dates sort after valid ones
+        expectDateComparesBefore(invalidDate, earlierDate); // invalid dates sort before valid ones
+        expectDateComparesAfter(laterDate, invalidDate); // invalid dates sort before valid ones
     });
 });