From e4332898326a5c76adac1b4e6fd5fab359a6f6f2 Mon Sep 17 00:00:00 2001 From: konrad Date: Fri, 24 Apr 2020 15:23:03 +0000 Subject: [PATCH] Task Position (#412) Fix misspell Fix sorting tasks with null values Fix sorting by priority for postgres Merge branch 'master' into feature/position Add community link Update golang.org/x/crypto commit hash to 44a6062 (#429) Update golang.org/x/crypto commit hash to 44a6062 Reviewed-on: https://kolaente.dev/vikunja/api/pulls/429 Update module lib/pq to v1.4.0 (#428) Update module lib/pq to v1.4.0 Reviewed-on: https://kolaente.dev/vikunja/api/pulls/428 Fix updating position Add ordering tasks in buckets by position Make task sort by string Merge branch 'master' into feature/position Update golang.org/x/crypto commit hash to 3c4aac8 (#419) Update golang.org/x/crypto commit hash to 3c4aac8 Reviewed-on: https://kolaente.dev/vikunja/api/pulls/419 Merge branch 'master' into feature/position Fix moving tasks back into the empty (ID: 0) bucket Add adding a default position when creating new tasks Update golang.org/x/crypto commit hash to a76a400 (#411) Update golang.org/x/crypto commit hash to a76a400 Reviewed-on: https://kolaente.dev/vikunja/api/pulls/411 Remove unused code Fix tests Add migration for position attribute Add position attribute Co-authored-by: kolaente Co-authored-by: renovate Reviewed-on: https://kolaente.dev/vikunja/api/pulls/412 --- pkg/integrations/task_collection_test.go | 28 +- pkg/migration/20200420215928.go | 65 ++ pkg/models/error.go | 2 +- pkg/models/kanban.go | 10 +- pkg/models/task_collection.go | 5 +- pkg/models/task_collection_sort.go | 152 +---- pkg/models/task_collection_sort_test.go | 777 +---------------------- pkg/models/tasks.go | 102 +-- 8 files changed, 163 insertions(+), 978 deletions(-) create mode 100644 pkg/migration/20200420215928.go diff --git a/pkg/integrations/task_collection_test.go b/pkg/integrations/task_collection_test.go index fcd04ce2e..00d3deaee 100644 --- a/pkg/integrations/task_collection_test.go +++ b/pkg/integrations/task_collection_test.go @@ -95,33 +95,33 @@ func TestTaskCollection(t *testing.T) { t.Run("by priority", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"priority"}}, urlParams) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `{"id":33,"text":"task #33 with percent done","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0.5,"identifier":"test1-17","index":17,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-4","index":4,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) + assert.Contains(t, rec.Body.String(), `{"id":33,"text":"task #33 with percent done","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0.5,"identifier":"test1-17","index":17,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":0,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-4","index":4,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) }) t.Run("by priority desc", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"priority"}, "order_by": []string{"desc"}}, urlParams) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `[{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1`) + assert.Contains(t, rec.Body.String(), `[{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1`) }) t.Run("by priority asc", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"priority"}, "order_by": []string{"asc"}}, urlParams) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `{"id":33,"text":"task #33 with percent done","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0.5,"identifier":"test1-17","index":17,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-4","index":4,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) + assert.Contains(t, rec.Body.String(), `{"id":33,"text":"task #33 with percent done","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0.5,"identifier":"test1-17","index":17,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":0,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-4","index":4,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) }) // should equal duedate asc t.Run("by due_date", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"due_date_unix"}}, urlParams) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"done_at":null,"due_date":"2018-11-30T22:25:24Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-6","index":6,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":3,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) + assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"done_at":null,"due_date":"2018-11-30T22:25:24Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-6","index":6,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":3,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) }) t.Run("by duedate desc", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"due_date_unix"}, "order_by": []string{"desc"}}, urlParams) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `[{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":6,"text":"task #6 lower due date`) + assert.Contains(t, rec.Body.String(), `[{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":6,"text":"task #6 lower due date`) }) t.Run("by duedate asc", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"due_date_unix"}, "order_by": []string{"asc"}}, urlParams) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"done_at":null,"due_date":"2018-11-30T22:25:24Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-6","index":6,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":3,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) + assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"done_at":null,"due_date":"2018-11-30T22:25:24Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-6","index":6,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":3,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) }) t.Run("invalid sort parameter", func(t *testing.T) { _, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"loremipsum"}}, urlParams) @@ -207,7 +207,7 @@ func TestTaskCollection(t *testing.T) { // If no start date but an end date is specified, this should be null // since we don't have any tasks in the fixtures with an end date > // the current date. - assert.Equal(t, "null\n", rec.Body.String()) + assert.Equal(t, "[]\n", rec.Body.String()) }) }) }) @@ -270,33 +270,33 @@ func TestTaskCollection(t *testing.T) { t.Run("by priority", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"priority"}}, nil) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `{"id":33,"text":"task #33 with percent done","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0.5,"identifier":"test1-17","index":17,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-4","index":4,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) + assert.Contains(t, rec.Body.String(), `{"id":33,"text":"task #33 with percent done","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0.5,"identifier":"test1-17","index":17,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":0,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-4","index":4,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) }) t.Run("by priority desc", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"priority"}, "order_by": []string{"desc"}}, nil) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `[{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1`) + assert.Contains(t, rec.Body.String(), `[{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1`) }) t.Run("by priority asc", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"priority"}, "order_by": []string{"asc"}}, nil) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `{"id":33,"text":"task #33 with percent done","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0.5,"identifier":"test1-17","index":17,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-4","index":4,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) + assert.Contains(t, rec.Body.String(), `{"id":33,"text":"task #33 with percent done","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0.5,"identifier":"test1-17","index":17,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":0,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":4,"text":"task #4 low prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":1,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-4","index":4,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":3,"text":"task #3 high prio","description":"","done":false,"done_at":null,"due_date":null,"reminder_dates":null,"list_id":1,"repeat_after":0,"priority":100,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-3","index":3,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) }) // should equal duedate asc t.Run("by due_date", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"due_date_unix"}}, nil) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"done_at":null,"due_date":"2018-11-30T22:25:24Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-6","index":6,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":3,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) + assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"done_at":null,"due_date":"2018-11-30T22:25:24Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-6","index":6,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":3,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) }) t.Run("by duedate desc", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"due_date_unix"}, "order_by": []string{"desc"}}, nil) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `[{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":6,"text":"task #6 lower due date`) + assert.Contains(t, rec.Body.String(), `[{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":6,"text":"task #6 lower due date`) }) t.Run("by duedate asc", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"sort_by": []string{"due_date_unix"}, "order_by": []string{"asc"}}, nil) assert.NoError(t, err) - assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"done_at":null,"due_date":"2018-11-30T22:25:24Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-6","index":6,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":3,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) + assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"done_at":null,"due_date":"2018-11-30T22:25:24Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-6","index":6,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":3,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"done_at":null,"due_date":"2018-12-01T03:58:44Z","reminder_dates":null,"list_id":1,"repeat_after":0,"priority":0,"start_date":null,"end_date":null,"assignees":null,"labels":null,"hex_color":"","percent_done":0,"identifier":"test1-5","index":5,"related_tasks":{},"attachments":null,"created":"2018-12-01T01:12:04Z","updated":"2018-12-01T01:12:04Z","bucket_id":2,"position":0,"created_by":{"id":1,"username":"user1","created":null,"updated":null}}]`) }) t.Run("invalid parameter", func(t *testing.T) { // Invalid parameter should not sort at all @@ -372,7 +372,7 @@ func TestTaskCollection(t *testing.T) { // If no start date but an end date is specified, this should be null // since we don't have any tasks in the fixtures with an end date > // the current date. - assert.Equal(t, "null\n", rec.Body.String()) + assert.Equal(t, "[]\n", rec.Body.String()) }) }) }) diff --git a/pkg/migration/20200420215928.go b/pkg/migration/20200420215928.go new file mode 100644 index 000000000..8ffbc0544 --- /dev/null +++ b/pkg/migration/20200420215928.go @@ -0,0 +1,65 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2020 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package migration + +import ( + "code.vikunja.io/api/pkg/models" + "math" + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +type task20200420215928 struct { + Position float64 `xorm:"double null" json:"position"` +} + +func (s task20200420215928) TableName() string { + return "tasks" +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20200420215928", + Description: "Add position property to task", + Migrate: func(tx *xorm.Engine) error { + err := tx.Sync2(task20200420215928{}) + if err != nil { + return err + } + + // Create a position according to their id -> gives a starting position + tasks := []*models.Task{} + err = tx.Find(&tasks) + if err != nil { + return err + } + + for _, task := range tasks { + task.Position = float64(task.ID) * math.Pow(2, 16) + _, err = tx.Where("id = ?", task.ID).Update(task) + if err != nil { + return err + } + } + + return nil + }, + Rollback: func(tx *xorm.Engine) error { + return tx.DropTables(task20200420215928{}) + }, + }) +} diff --git a/pkg/models/error.go b/pkg/models/error.go index 3b266af96..952a15eba 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -573,7 +573,7 @@ func (err ErrTaskAttachmentIsTooLarge) HTTPError() web.HTTPError { // ErrInvalidSortParam represents an error where the provided sort param is invalid type ErrInvalidSortParam struct { - SortBy sortProperty + SortBy string } // IsErrInvalidSortParam checks if an error is ErrInvalidSortParam. diff --git a/pkg/models/kanban.go b/pkg/models/kanban.go index 40a4db2d4..affb952f3 100644 --- a/pkg/models/kanban.go +++ b/pkg/models/kanban.go @@ -123,7 +123,15 @@ func (b *Bucket) ReadAll(auth web.Auth, search string, page int, perPage int) (r } // Get all tasks for this list - tasks, _, _, err := getTasksForLists([]*List{{ID: b.ListID}}, &taskOptions{}) + opts := &taskOptions{ + sortby: []*sortParam{ + { + sortBy: taskPropertyPosition, + orderBy: orderAscending, + }, + }, + } + tasks, _, _, err := getTasksForLists([]*List{{ID: b.ListID}}, opts) if err != nil { return } diff --git a/pkg/models/task_collection.go b/pkg/models/task_collection.go index d1668866b..07c991273 100644 --- a/pkg/models/task_collection.go +++ b/pkg/models/task_collection.go @@ -67,7 +67,8 @@ func validateTaskField(fieldName string) error { taskPropertyPercentDone, taskPropertyUID, taskPropertyCreated, - taskPropertyUpdated: + taskPropertyUpdated, + taskPropertyPosition: return nil } return ErrInvalidTaskField{TaskField: fieldName} @@ -106,7 +107,7 @@ func (tf *TaskCollection) ReadAll(a web.Auth, search string, page int, perPage i var sort = make([]*sortParam, 0, len(tf.SortBy)) for i, s := range tf.SortBy { param := &sortParam{ - sortBy: sortProperty(s), + sortBy: s, orderBy: orderAscending, } // This checks if tf.OrderBy has an entry with the same index as the current entry from tf.SortBy diff --git a/pkg/models/task_collection_sort.go b/pkg/models/task_collection_sort.go index 16e5fc59a..5413c0d17 100644 --- a/pkg/models/task_collection_sort.go +++ b/pkg/models/task_collection_sort.go @@ -16,21 +16,12 @@ package models -import ( - "code.vikunja.io/api/pkg/timeutil" - "fmt" - "reflect" - "sort" -) - type ( sortParam struct { - sortBy sortProperty + sortBy string orderBy sortOrder // asc or desc } - sortProperty string - sortOrder string ) @@ -52,12 +43,9 @@ const ( taskPropertyUID string = "uid" taskPropertyCreated string = "created" taskPropertyUpdated string = "updated" + taskPropertyPosition string = "position" ) -func (p sortProperty) String() string { - return string(p) -} - const ( orderInvalid sortOrder = "invalid" orderAscending sortOrder = "asc" @@ -82,139 +70,5 @@ func (sp *sortParam) validate() error { if sp.orderBy != orderDescending && sp.orderBy != orderAscending { return ErrInvalidSortOrder{OrderBy: sp.orderBy} } - return validateTaskField(string(sp.sortBy)) -} - -type taskComparator func(lhs, rhs *Task) int64 - -func mustMakeComparator(fieldName string) taskComparator { - field, ok := reflect.TypeOf(&Task{}).Elem().FieldByName(fieldName) - if !ok { - panic(fmt.Sprintf("Field '%s' has not been found on Task", fieldName)) - } - - extractProp := func(task *Task) interface{} { - return reflect.ValueOf(task).Elem().FieldByIndex(field.Index).Interface() - } - - // Special case for handling TimeStamp types - if field.Type.Name() == "TimeStamp" { - return func(lhs, rhs *Task) int64 { - return int64(extractProp(lhs).(timeutil.TimeStamp)) - int64(extractProp(rhs).(timeutil.TimeStamp)) - } - } - - switch field.Type.Kind() { - case reflect.Int64: - return func(lhs, rhs *Task) int64 { - return extractProp(lhs).(int64) - extractProp(rhs).(int64) - } - case reflect.Float64: - return func(lhs, rhs *Task) int64 { - floatLHS, floatRHS := extractProp(lhs).(float64), extractProp(rhs).(float64) - if floatLHS > floatRHS { - return 1 - } else if floatLHS < floatRHS { - return -1 - } - return 0 - } - case reflect.String: - return func(lhs, rhs *Task) int64 { - strLHS, strRHS := extractProp(lhs).(string), extractProp(rhs).(string) - if strLHS > strRHS { - return 1 - } else if strLHS < strRHS { - return -1 - } - return 0 - } - case reflect.Bool: - 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 - } - return 0 - } - default: - panic(fmt.Sprintf("Unsupported type for sorting: %s", field.Type.Name())) - } -} - -// This is a map of properties that can be sorted by -// and their appropriate comparator function. -// The comparator function sorts in ascending mode. -var propertyComparators = map[string]taskComparator{ - taskPropertyID: mustMakeComparator("ID"), - taskPropertyText: mustMakeComparator("Text"), - taskPropertyDescription: mustMakeComparator("Description"), - taskPropertyDone: mustMakeComparator("Done"), - taskPropertyDoneAtUnix: mustMakeComparator("DoneAt"), - taskPropertyDueDateUnix: mustMakeComparator("DueDate"), - taskPropertyCreatedByID: mustMakeComparator("CreatedByID"), - taskPropertyListID: mustMakeComparator("ListID"), - taskPropertyRepeatAfter: mustMakeComparator("RepeatAfter"), - taskPropertyPriority: mustMakeComparator("Priority"), - taskPropertyStartDateUnix: mustMakeComparator("StartDate"), - taskPropertyEndDateUnix: mustMakeComparator("EndDate"), - taskPropertyHexColor: mustMakeComparator("HexColor"), - taskPropertyPercentDone: mustMakeComparator("PercentDone"), - taskPropertyUID: mustMakeComparator("UID"), - taskPropertyCreated: mustMakeComparator("Created"), - taskPropertyUpdated: mustMakeComparator("Updated"), -} - -// Creates a taskComparator that sorts by the first comparator and falls back to -// the second one (and so on...) if the properties were equal. -func combineComparators(comparators ...taskComparator) taskComparator { - return func(lhs, rhs *Task) int64 { - for _, compare := range comparators { - res := compare(lhs, rhs) - if res != 0 { - return res - } - } - return 0 - } -} - -func sortTasks(tasks []*Task, by []*sortParam) { - - // Always sort at least by id asc so we have a consistent order of items every time - // If we would not do this, we would get a different order for items with the same content every time - // the slice is sorted. To circumvent this, we always order at least by ID. - if len(by) == 0 || - (len(by) > 0 && by[len(by)-1].sortBy != sortProperty(taskPropertyID)) { // Don't sort by ID last if the id parameter is already passed as the last parameter. - by = append(by, &sortParam{sortBy: sortProperty(taskPropertyID), orderBy: orderAscending}) - } - - comparators := make([]taskComparator, 0, len(by)) - for _, param := range by { - comparator, ok := propertyComparators[string(param.sortBy)] - if !ok { - panic("No suitable comparator for sortBy found! Param was " + param.sortBy) - } - - // This is a descending sort, so we need to negate the comparator (i.e. switch the inputs). - if param.orderBy == orderDescending { - oldComparator := comparator - comparator = func(lhs, rhs *Task) int64 { - return oldComparator(lhs, rhs) * -1 - } - } - - comparators = append(comparators, comparator) - } - - combinedComparator := combineComparators(comparators...) - - sort.Slice(tasks, func(i, j int) bool { - lhs, rhs := tasks[i], tasks[j] - - res := combinedComparator(lhs, rhs) - return res <= 0 - }) + return validateTaskField(sp.sortBy) } diff --git a/pkg/models/task_collection_sort_test.go b/pkg/models/task_collection_sort_test.go index d9f2fc8a1..606b66647 100644 --- a/pkg/models/task_collection_sort_test.go +++ b/pkg/models/task_collection_sort_test.go @@ -17,10 +17,7 @@ package models import ( - "github.com/mohae/deepcopy" "github.com/stretchr/testify/assert" - "math/rand" - "reflect" "testing" ) @@ -62,11 +59,12 @@ func TestSortParamValidation(t *testing.T) { taskPropertyUID, taskPropertyCreated, taskPropertyUpdated, + taskPropertyPosition, } { t.Run(test, func(t *testing.T) { s := &sortParam{ orderBy: orderAscending, - sortBy: sortProperty(test), + sortBy: test, } err := s.validate() assert.NoError(t, err) @@ -92,774 +90,3 @@ func TestSortParamValidation(t *testing.T) { assert.True(t, IsErrInvalidTaskField(err)) }) } - -var ( - task1 = &Task{ - ID: 1, - Text: "aaa", - Description: "Lorem Ipsum", - Done: true, - DoneAt: 1543626000, - ListID: 1, - UID: "JywtBPCESImlyKugvaZWrxmXAFAWXFISMeXYImEh", - Created: 1543626724, - Updated: 1543626724, - } - task2 = &Task{ - ID: 2, - Text: "bbb", - Description: "Arem Ipsum", - Done: true, - DoneAt: 1543626724, - CreatedByID: 1, - ListID: 2, - PercentDone: 0.3, - StartDate: 1543626724, - Created: 1553626724, - Updated: 1553626724, - } - task3 = &Task{ - ID: 3, - Text: "ccc", - DueDate: 1583626724, - Priority: 100, - ListID: 3, - HexColor: "000000", - PercentDone: 0.1, - Updated: 1555555555, - } - task4 = &Task{ - ID: 4, - Text: "ddd", - Priority: 1, - StartDate: 1643626724, - ListID: 1, - } - task5 = &Task{ - ID: 5, - Text: "eef", - Priority: 50, - UID: "shggzCHQWLhGNMNsOGOCOjcVkInOYjTAnORqTkdL", - DueDate: 1543636724, - Updated: 1565555555, - } - task6 = &Task{ - ID: 6, - Text: "eef", - DueDate: 1543616724, - RepeatAfter: 6400, - CreatedByID: 2, - HexColor: "ffffff", - } - task7 = &Task{ - ID: 7, - Text: "mmmn", - Description: "Zoremis", - StartDate: 1544600000, - EndDate: 1584600000, - UID: "tyzCZuLMSKhwclJOsDyDcUdyVAPBDOPHNTBOLTcW", - } - task8 = &Task{ - ID: 8, - Text: "b123", - EndDate: 1544700000, - } - task9 = &Task{ - ID: 9, - Done: true, - DoneAt: 1573626724, - Text: "a123", - RepeatAfter: 86000, - StartDate: 1544600000, - EndDate: 1544700000, - } - task10 = &Task{ - ID: 10, - Text: "zzz", - Priority: 10, - PercentDone: 1, - } -) - -type taskSortTestCase struct { - name string - wantAsc []*Task - wantDesc []*Task - sortProperty string -} - -var taskSortTestCases = []taskSortTestCase{ - { - name: "id", - sortProperty: taskPropertyID, - wantAsc: []*Task{ - task1, - task2, - task3, - task4, - task5, - task6, - task7, - task8, - task9, - task10, - }, - wantDesc: []*Task{ - task10, - task9, - task8, - task7, - task6, - task5, - task4, - task3, - task2, - task1, - }, - }, - { - name: "text", - sortProperty: taskPropertyText, - wantAsc: []*Task{ - task9, - task1, - task8, - task2, - task3, - task4, - task5, - task6, - task7, - task10, - }, - wantDesc: []*Task{ - task10, - task7, - task5, - task6, - task4, - task3, - task2, - task8, - task1, - task9, - }, - }, - { - name: "description", - sortProperty: taskPropertyDescription, - wantAsc: []*Task{ - task3, - task4, - task5, - task6, - task8, - task9, - task10, - task2, - task1, - task7, - }, - wantDesc: []*Task{ - task7, - task1, - task2, - task3, - task4, - task5, - task6, - task8, - task9, - task10, - }, - }, - { - name: "done", - sortProperty: taskPropertyDone, - wantAsc: []*Task{ - // These are not - task3, - task4, - task5, - task6, - task7, - task8, - task10, - // These are done - task1, - task2, - task9, - }, - wantDesc: []*Task{ - // These are done - task1, - task2, - task9, - // These are not - task3, - task4, - task5, - task6, - task7, - task8, - task10, - }, - }, - { - name: "done at", - sortProperty: taskPropertyDoneAtUnix, - wantAsc: []*Task{ - task3, - task4, - task5, - task6, - task7, - task8, - task10, - task1, - task2, - task9, - }, - wantDesc: []*Task{ - task9, - task2, - task1, - task3, - task4, - task5, - task6, - task7, - task8, - task10, - }, - }, - { - name: "due date", - sortProperty: taskPropertyDueDateUnix, - wantAsc: []*Task{ - task1, - task2, - task4, - task7, - task8, - task9, - task10, - task6, - task5, - task3, - }, - wantDesc: []*Task{ - task3, - task5, - task6, - task1, - task2, - task4, - task7, - task8, - task9, - task10, - }, - }, - { - name: "created by id", - sortProperty: taskPropertyCreatedByID, - wantAsc: []*Task{ - task1, - task3, - task4, - task5, - task7, - task8, - task9, - task10, - task2, - task6, - }, - wantDesc: []*Task{ - task6, - task2, - task1, - task3, - task4, - task5, - task7, - task8, - task9, - task10, - }, - }, - { - name: "list id", - sortProperty: taskPropertyListID, - wantAsc: []*Task{ - task5, - task6, - task7, - task8, - task9, - task10, - task1, - task4, - task2, - task3, - }, - wantDesc: []*Task{ - task3, - task2, - task1, - task4, - task5, - task6, - task7, - task8, - task9, - task10, - }, - }, - { - name: "repeat after", - sortProperty: taskPropertyRepeatAfter, - wantAsc: []*Task{ - task1, - task2, - task3, - task4, - task5, - task7, - task8, - task10, - task6, - task9, - }, - wantDesc: []*Task{ - task9, - task6, - task1, - task2, - task3, - task4, - task5, - task7, - task8, - task10, - }, - }, - { - name: "priority", - sortProperty: taskPropertyPriority, - wantAsc: []*Task{ - task1, - task2, - task6, - task7, - task8, - task9, - task4, - task10, - task5, - task3, - }, - wantDesc: []*Task{ - task3, - task5, - task10, - task4, - task1, - task2, - task6, - task7, - task8, - task9, - }, - }, - { - name: "start date", - sortProperty: taskPropertyStartDateUnix, - wantAsc: []*Task{ - task1, - task3, - task5, - task6, - task8, - task10, - task2, - task7, - task9, - task4, - }, - wantDesc: []*Task{ - task4, - task7, - task9, - task2, - task1, - task3, - task5, - task6, - task8, - task10, - }, - }, - { - name: "end date", - sortProperty: taskPropertyEndDateUnix, - wantAsc: []*Task{ - task1, - task2, - task3, - task4, - task5, - task6, - task10, - task8, - task9, - task7, - }, - wantDesc: []*Task{ - task7, - task8, - task9, - task1, - task2, - task3, - task4, - task5, - task6, - task10, - }, - }, - { - name: "hex color", - sortProperty: taskPropertyHexColor, - wantAsc: []*Task{ - task1, - task2, - task4, - task5, - task7, - task8, - task9, - task10, - task3, - task6, - }, - wantDesc: []*Task{ - task6, - task3, - task1, - task2, - task4, - task5, - task7, - task8, - task9, - task10, - }, - }, - { - name: "percent done", - sortProperty: taskPropertyPercentDone, - wantAsc: []*Task{ - task1, - task4, - task5, - task6, - task7, - task8, - task9, - task3, - task2, - task10, - }, - wantDesc: []*Task{ - task10, - task2, - task3, - task1, - task4, - task5, - task6, - task7, - task8, - task9, - }, - }, - { - name: "uid", - sortProperty: taskPropertyUID, - wantAsc: []*Task{ - task2, - task3, - task4, - task6, - task8, - task9, - task10, - task1, - task5, - task7, - }, - wantDesc: []*Task{ - task7, - task5, - task1, - task2, - task3, - task4, - task6, - task8, - task9, - task10, - }, - }, - { - name: "created", - sortProperty: taskPropertyCreated, - wantAsc: []*Task{ - task3, - task4, - task5, - task6, - task7, - task8, - task9, - task10, - task1, - task2, - }, - wantDesc: []*Task{ - task2, - task1, - task3, - task4, - task5, - task6, - task7, - task8, - task9, - task10, - }, - }, - { - name: "updated", - sortProperty: taskPropertyUpdated, - wantAsc: []*Task{ - task4, - task6, - task7, - task8, - task9, - task10, - task1, - task2, - task3, - task5, - }, - wantDesc: []*Task{ - task5, - task3, - task2, - task1, - task4, - task6, - task7, - task8, - task9, - task10, - }, - }, -} - -func TestTaskSort(t *testing.T) { - - assertTestSliceMatch := func(t *testing.T, got, want []*Task) { - if !reflect.DeepEqual(got, want) { - t.Error("Slices do not match in order") - t.Error("Got\t| Want") - for in, task := range got { - fail := "" - if task.ID != want[in].ID { - fail = "wrong" - } - t.Errorf("\t%d\t| %d \t%s", task.ID, want[in].ID, fail) - } - } - } - - for _, testCase := range taskSortTestCases { - t.Run(testCase.name, func(t *testing.T) { - t.Run("asc default", func(t *testing.T) { - by := []*sortParam{ - { - sortBy: sortProperty(testCase.sortProperty), - }, - } - - got := deepcopy.Copy(testCase.wantAsc).([]*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, by) - - assertTestSliceMatch(t, got, testCase.wantAsc) - }) - t.Run("asc", func(t *testing.T) { - by := []*sortParam{ - { - sortBy: sortProperty(testCase.sortProperty), - orderBy: orderAscending, - }, - } - - got := deepcopy.Copy(testCase.wantAsc).([]*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, by) - - assertTestSliceMatch(t, got, testCase.wantAsc) - }) - t.Run("desc", func(t *testing.T) { - by := []*sortParam{ - { - sortBy: sortProperty(testCase.sortProperty), - orderBy: orderDescending, - }, - } - - got := deepcopy.Copy(testCase.wantDesc).([]*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, by) - - assertTestSliceMatch(t, got, testCase.wantDesc) - }) - }) - } - - // Other cases - 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, - } - sortParams := []*sortParam{ - { - sortBy: sortProperty(taskPropertyDone), - orderBy: orderAscending, - }, - { - sortBy: sortProperty(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, - task5, - task6, - task4, - task3, - task8, - // Done - task2, - task1, - task9, - } - sortParams := []*sortParam{ - { - sortBy: sortProperty(taskPropertyDone), - orderBy: orderAscending, - }, - { - sortBy: sortProperty(taskPropertyText), - 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 Descending and Text Ascending", func(t *testing.T) { - want := []*Task{ - // Done - task9, - task1, - task2, - // Not done - task8, - task3, - task4, - task5, - task6, - task7, - task10, - } - sortParams := []*sortParam{ - { - sortBy: sortProperty(taskPropertyDone), - orderBy: orderDescending, - }, - { - sortBy: sortProperty(taskPropertyText), - orderBy: orderAscending, - }, - } - - 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) - - }) -} diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index 2f6076302..3c9ac3281 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -24,10 +24,11 @@ import ( "code.vikunja.io/api/pkg/utils" "code.vikunja.io/web" "github.com/imdario/mergo" - "sort" + "math" "strconv" "time" "xorm.io/builder" + "xorm.io/xorm/schemas" ) // Task represents an task in a todolist @@ -88,6 +89,14 @@ type Task struct { // BucketID is the ID of the kanban bucket this task belongs to. BucketID int64 `xorm:"int(11) null" json:"bucket_id"` + // The position of the task - any task list can be sorted as usual by this parameter. + // When accessing tasks via kanban buckets, this is primarily used to sort them based on a range + // We're using a float64 here to make it possible to put any task within any two other tasks (by changing the number). + // You would calculate the new position between two tasks with something like task3.position = (task2.position - task1.position) / 2. + // A 64-Bit float leaves plenty of room to initially give tasks a position with 2^16 difference to the previous task + // which also leaves a lot of room for rearranging and sorting later. + Position float64 `xorm:"double null" json:"position"` + // The user who initially created the task. CreatedBy *user.User `xorm:"-" json:"created_by" valid:"-"` @@ -143,7 +152,7 @@ func (t *Task) ReadAll(a web.Auth, search string, page int, perPage int) (result return nil, 0, 0, nil } -func getRawTasksForLists(lists []*List, opts *taskOptions) (taskMap map[int64]*Task, resultCount int, totalItems int64, err error) { +func getRawTasksForLists(lists []*List, opts *taskOptions) (tasks []*Task, resultCount int, totalItems int64, err error) { // Get all list IDs and get the tasks var listIDs []int64 @@ -151,6 +160,15 @@ func getRawTasksForLists(lists []*List, opts *taskOptions) (taskMap map[int64]*T listIDs = append(listIDs, l.ID) } + // Add the id parameter as the last parameter to sorty by default, but only if it is not already passed as the last parameter. + if len(opts.sortby) == 0 || + len(opts.sortby) > 0 && opts.sortby[len(opts.sortby)-1].sortBy != taskPropertyID { + opts.sortby = append(opts.sortby, &sortParam{ + sortBy: taskPropertyID, + orderBy: orderAscending, + }) + } + // Since xorm does not use placeholders for order by, it is possible to expose this with sql injection if we're directly // passing user input to the db. // As a workaround to prevent this, we check for valid column names here prior to passing it to the db. @@ -160,7 +178,19 @@ func getRawTasksForLists(lists []*List, opts *taskOptions) (taskMap map[int64]*T if err := param.validate(); err != nil { return nil, 0, 0, err } - orderby += param.sortBy.String() + " " + param.orderBy.String() + orderby += param.sortBy + " " + param.orderBy.String() + + // Postgres sorts by default entries with null values after ones with values. + // To make that consistent with the sort order we have and other dbms, we're adding a separate clause here. + if x.Dialect().URI().DBType == schemas.POSTGRES { + if param.orderBy == orderAscending { + orderby += " NULLS FIRST" + } + if param.orderBy == orderDescending { + orderby += " NULLS LAST" + } + } + if (i + 1) < len(opts.sortby) { orderby += ", " } @@ -184,8 +214,6 @@ func getRawTasksForLists(lists []*List, opts *taskOptions) (taskMap map[int64]*T } } - taskMap = make(map[int64]*Task) - // Then return all tasks for that lists query := x. OrderBy(orderby) @@ -208,7 +236,8 @@ func getRawTasksForLists(lists []*List, opts *taskOptions) (taskMap map[int64]*T query = query.Limit(limit, start) } - err = query.Find(&taskMap) + tasks = []*Task{} + err = query.Find(&tasks) if err != nil { return nil, 0, 0, err } @@ -219,23 +248,25 @@ func getRawTasksForLists(lists []*List, opts *taskOptions) (taskMap map[int64]*T return nil, 0, 0, err } - return taskMap, len(taskMap), totalItems, nil + return tasks, len(tasks), totalItems, nil } func getTasksForLists(lists []*List, opts *taskOptions) (tasks []*Task, resultCount int, totalItems int64, err error) { - taskMap, resultCount, totalItems, err := getRawTasksForLists(lists, opts) + tasks, resultCount, totalItems, err = getRawTasksForLists(lists, opts) if err != nil { return nil, 0, 0, err } - tasks, err = addMoreInfoToTasks(taskMap) + taskMap := make(map[int64]*Task, len(tasks)) + for _, t := range tasks { + taskMap[t.ID] = t + } + + err = addMoreInfoToTasks(taskMap) if err != nil { return nil, 0, 0, err } - // Because the list is fully unsorted (since we're dealing with maps) - // we have to manually sort the tasks again here. - sortTasks(tasks, opts.sortby) return tasks, resultCount, totalItems, err } @@ -271,25 +302,28 @@ func (bt *BulkTask) GetTasksByIDs() (err error) { } } - taskMap := make(map[int64]*Task, len(bt.Tasks)) - err = x.In("id", bt.IDs).Find(&taskMap) + err = x.In("id", bt.IDs).Find(&bt.Tasks) if err != nil { return } - bt.Tasks, err = addMoreInfoToTasks(taskMap) return } // GetTasksByUIDs gets all tasks from a bunch of uids func GetTasksByUIDs(uids []string) (tasks []*Task, err error) { - taskMap := make(map[int64]*Task) - err = x.In("uid", uids).Find(&taskMap) + tasks = []*Task{} + err = x.In("uid", uids).Find(&tasks) if err != nil { return } - tasks, err = addMoreInfoToTasks(taskMap) + taskMap := make(map[int64]*Task, len(tasks)) + for _, t := range tasks { + taskMap[t.ID] = t + } + + err = addMoreInfoToTasks(taskMap) return } @@ -301,7 +335,7 @@ func getRemindersForTasks(taskIDs []int64) (reminders []*TaskReminder, err error // This function takes a map with pointers and returns a slice with pointers to tasks // It adds more stuff like assignees/labels/etc to a bunch of tasks -func addMoreInfoToTasks(taskMap map[int64]*Task) (tasks []*Task, err error) { +func addMoreInfoToTasks(taskMap map[int64]*Task) (err error) { // No need to iterate over users and stuff if the list doesn't has tasks if len(taskMap) == 0 { @@ -351,7 +385,7 @@ func addMoreInfoToTasks(taskMap map[int64]*Task) (tasks []*Task, err error) { In("task_id", taskIDs). Find(&attachments) if err != nil { - return nil, err + return } fileIDs := []int64{} @@ -446,19 +480,6 @@ func addMoreInfoToTasks(taskMap map[int64]*Task) (tasks []*Task, err error) { taskMap[rt.TaskID].RelatedTasks[rt.RelationKind] = append(taskMap[rt.TaskID].RelatedTasks[rt.RelationKind], fullRelatedTasks[rt.OtherTaskID]) } - // make a complete slice from the map - tasks = []*Task{} - for _, t := range taskMap { - tasks = append(tasks, t) - } - - // Sort the output. In Go, contents on a map are put on that map in no particular order. - // Because of this, tasks are not sorted anymore in the output, this leads to confiusion. - // To avoid all this, we need to sort the slice afterwards - sort.Slice(tasks, func(i, j int) bool { - return tasks[i].ID < tasks[j].ID - }) - return } @@ -533,6 +554,10 @@ func (t *Task) Create(a web.Auth) (err error) { t.Index = latestTask.Index + 1 t.CreatedByID = u.ID t.CreatedBy = u + // If no position was supplied, set a default one + if t.Position == 0 { + t.Position = float64(latestTask.ID+1) * math.Pow(2, 16) + } if _, err = x.Insert(t); err != nil { return err } @@ -673,6 +698,10 @@ func (t *Task) Update() (err error) { if t.BucketID == 0 { ot.BucketID = 0 } + // Position + if t.Position == 0 { + ot.Position = 0 + } _, err = x.ID(t.ID). Cols("text", @@ -688,6 +717,7 @@ func (t *Task) Update() (err error) { "percent_done", "list_id", "bucket_id", + "position", ). Update(ot) *t = ot @@ -877,16 +907,16 @@ func (t *Task) ReadOne() (err error) { return } - tasks, err := addMoreInfoToTasks(taskMap) + err = addMoreInfoToTasks(taskMap) if err != nil { return } - if len(tasks) == 0 { + if len(taskMap) == 0 { return ErrTaskDoesNotExist{t.ID} } - *t = *tasks[0] + *t = *taskMap[t.ID] return }