From 1555f04bd251b496673847f72e149d39ef03f436 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Sat, 7 Dec 2019 16:56:18 +0100
Subject: [PATCH] Fix sorting tasks by bool values

There was a bug where it would return all tasks with a true value before the ones with a false value. This is the exact opposite of what the db does, leading to wrongly sorted values
---
 pkg/models/task_collection_sort.go      |  4 +-
 pkg/models/task_collection_sort_test.go | 66 +++++++++++++++++++------
 2 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/pkg/models/task_collection_sort.go b/pkg/models/task_collection_sort.go
index f2cae7b7c8a..a729e32fdb7 100644
--- a/pkg/models/task_collection_sort.go
+++ b/pkg/models/task_collection_sort.go
@@ -146,9 +146,9 @@ func mustMakeComparator(fieldName string) taskComparator {
 		return func(lhs, rhs *Task) int64 {
 			boolLHS, boolRHS := extractProp(lhs).(bool), extractProp(rhs).(bool)
 			if !boolLHS && boolRHS {
-				return 1
-			} else if boolLHS && !boolRHS {
 				return -1
+			} else if boolLHS && !boolRHS {
+				return 1
 			}
 			return 0
 		}
diff --git a/pkg/models/task_collection_sort_test.go b/pkg/models/task_collection_sort_test.go
index d009e99c5c7..0697e094ab7 100644
--- a/pkg/models/task_collection_sort_test.go
+++ b/pkg/models/task_collection_sort_test.go
@@ -277,10 +277,6 @@ var taskSortTestCases = []taskSortTestCase{
 		name:         "done",
 		sortProperty: taskPropertyDone,
 		wantAsc: []*Task{
-			// These are done
-			task1,
-			task2,
-			task9,
 			// These are not
 			task3,
 			task4,
@@ -289,8 +285,16 @@ var taskSortTestCases = []taskSortTestCase{
 			task7,
 			task8,
 			task10,
+			// These are done
+			task1,
+			task2,
+			task9,
 		},
 		wantDesc: []*Task{
+			// These are done
+			task1,
+			task2,
+			task9,
 			// These are not
 			task3,
 			task4,
@@ -299,10 +303,6 @@ var taskSortTestCases = []taskSortTestCase{
 			task7,
 			task8,
 			task10,
-			// These are done
-			task1,
-			task2,
-			task9,
 		},
 	},
 	{
@@ -749,13 +749,46 @@ func TestTaskSort(t *testing.T) {
 	}
 
 	// Other cases
-	t.Run("Order by Done Ascending and Text Descending", func(t *testing.T) {
+	t.Run("Order by Done Ascending and ID Descending", func(t *testing.T) {
 		want := []*Task{
+			// Not done
+			task10,
+			task8,
+			task7,
+			task6,
+			task5,
+			task4,
+			task3,
+
 			// Done
+			task9,
 			task2,
 			task1,
-			task9,
+		}
+		sortParams := []*sortParam{
+			{
+				sortBy:  taskPropertyDone,
+				orderBy: orderAscending,
+			},
+			{
+				sortBy:  taskPropertyID,
+				orderBy: orderDescending,
+			},
+		}
 
+		got := deepcopy.Copy(want).([]*Task)
+
+		// Destroy wanted order to obtain some slice we can sort
+		rand.Shuffle(len(got), func(i, j int) {
+			got[i], got[j] = got[j], got[i]
+		})
+
+		sortTasks(got, sortParams)
+
+		assertTestSliceMatch(t, got, want)
+	})
+	t.Run("Order by Done Ascending and Text Descending", func(t *testing.T) {
+		want := []*Task{
 			// Not done
 			task10,
 			task7,
@@ -764,6 +797,10 @@ func TestTaskSort(t *testing.T) {
 			task4,
 			task3,
 			task8,
+			// Done
+			task2,
+			task1,
+			task9,
 		}
 		sortParams := []*sortParam{
 			{
@@ -789,6 +826,10 @@ func TestTaskSort(t *testing.T) {
 	})
 	t.Run("Order by Done Descending and Text Ascending", func(t *testing.T) {
 		want := []*Task{
+			// Done
+			task9,
+			task1,
+			task2,
 			// Not done
 			task8,
 			task3,
@@ -797,11 +838,6 @@ func TestTaskSort(t *testing.T) {
 			task6,
 			task7,
 			task10,
-
-			// Done
-			task9,
-			task1,
-			task2,
 		}
 		sortParams := []*sortParam{
 			{