From 231dc3913f4a96869bc56f02339d8f70b0be9319 Mon Sep 17 00:00:00 2001 From: konrad Date: Sat, 25 Apr 2020 20:32:02 +0000 Subject: [PATCH] Add real buckets for tasks which don't have one (#446) Add docs for error code Add moving new tasks into the default bucket when none was provided Add moving tasks in default bucket when deleting one Fix tests again Add test for removing a bucket Fix tests Prevent removing the last bucket Remove the empty pseudo bucket Add migration to create a new bucket for each list (and put all tasks in it Co-authored-by: kolaente Reviewed-on: https://kolaente.dev/vikunja/api/pulls/446 --- docs/content/doc/usage/errors.md | 1 + pkg/db/fixtures/buckets.yml | 104 +++++++++++++++++++++++ pkg/db/fixtures/tasks.yml | 28 ++++++ pkg/integrations/kanban_test.go | 28 +++--- pkg/integrations/task_collection_test.go | 8 +- pkg/migration/20200425182634.go | 75 ++++++++++++++++ pkg/models/error.go | 28 ++++++ pkg/models/kanban.go | 50 ++++++----- pkg/models/kanban_test.go | 61 +++++++++---- pkg/models/task_collection_test.go | 28 +++++- pkg/models/tasks.go | 9 ++ pkg/models/tasks_test.go | 2 + 12 files changed, 364 insertions(+), 58 deletions(-) create mode 100644 pkg/migration/20200425182634.go diff --git a/docs/content/doc/usage/errors.md b/docs/content/doc/usage/errors.md index f2fdb112fd..490b6f8896 100644 --- a/docs/content/doc/usage/errors.md +++ b/docs/content/doc/usage/errors.md @@ -128,3 +128,4 @@ This document describes the different errors Vikunja can return. |-----------|------------------|-------------| | 10001 | 404 | The bucket does not exist. | | 10002 | 400 | The bucket does not belong to that list. | +| 10003 | 412 | You cannot remove the last bucket on a list. | diff --git a/pkg/db/fixtures/buckets.yml b/pkg/db/fixtures/buckets.yml index d7d23b7291..20299e5405 100644 --- a/pkg/db/fixtures/buckets.yml +++ b/pkg/db/fixtures/buckets.yml @@ -101,3 +101,107 @@ created_by_id: 1 created: 1587244432 updated: 1587244432 +- id: 18 + title: testbucket18 + list_id: 5 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 19 + title: testbucket19 + list_id: 21 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 20 + title: testbucket20 + list_id: 22 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 21 + title: testbucket21 + list_id: 3 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +# Duplicate buckets to make deletion of one of them possible +- id: 22 + title: testbucket22 + list_id: 6 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 23 + title: testbucket23 + list_id: 7 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 24 + title: testbucket24 + list_id: 8 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 25 + title: testbucket25 + list_id: 9 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 26 + title: testbucket26 + list_id: 10 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 27 + title: testbucket27 + list_id: 11 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 28 + title: testbucket28 + list_id: 12 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 29 + title: testbucket29 + list_id: 13 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 30 + title: testbucket30 + list_id: 14 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 31 + title: testbucket31 + list_id: 15 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 32 + title: testbucket32 + list_id: 16 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +- id: 33 + title: testbucket33 + list_id: 17 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 +# This bucket is the last one in its list +- id: 34 + title: testbucket34 + list_id: 18 + created_by_id: 1 + created: 1587244432 + updated: 1587244432 diff --git a/pkg/db/fixtures/tasks.yml b/pkg/db/fixtures/tasks.yml index 7ee8f7c4c8..5e54030cc3 100644 --- a/pkg/db/fixtures/tasks.yml +++ b/pkg/db/fixtures/tasks.yml @@ -87,12 +87,14 @@ updated: 1543626724 start_date_unix: 1544600000 end_date_unix: 1544700000 + bucket_id: 1 - id: 10 text: 'task #10 basic' done: false created_by_id: 1 list_id: 1 index: 10 + bucket_id: 1 created: 1543626724 updated: 1543626724 - id: 11 @@ -101,6 +103,7 @@ created_by_id: 1 list_id: 1 index: 11 + bucket_id: 1 created: 1543626724 updated: 1543626724 - id: 12 @@ -109,6 +112,7 @@ created_by_id: 1 list_id: 1 index: 12 + bucket_id: 1 created: 1543626724 updated: 1543626724 - id: 13 @@ -117,6 +121,7 @@ created_by_id: 1 list_id: 2 index: 1 + bucket_id: 4 created: 1543626724 updated: 1543626724 - id: 14 @@ -125,6 +130,7 @@ created_by_id: 5 list_id: 5 index: 1 + bucket_id: 18 created: 1543626724 updated: 1543626724 - id: 15 @@ -133,6 +139,7 @@ created_by_id: 6 list_id: 6 index: 1 + bucket_id: 6 created: 1543626724 updated: 1543626724 - id: 16 @@ -141,6 +148,7 @@ created_by_id: 6 list_id: 7 index: 1 + bucket_id: 7 created: 1543626724 updated: 1543626724 - id: 17 @@ -149,6 +157,7 @@ created_by_id: 6 list_id: 8 index: 1 + bucket_id: 8 created: 1543626724 updated: 1543626724 - id: 18 @@ -157,6 +166,7 @@ created_by_id: 6 list_id: 9 index: 1 + bucket_id: 9 created: 1543626724 updated: 1543626724 - id: 19 @@ -165,6 +175,7 @@ created_by_id: 6 list_id: 10 index: 1 + bucket_id: 10 created: 1543626724 updated: 1543626724 - id: 20 @@ -173,6 +184,7 @@ created_by_id: 6 list_id: 11 index: 1 + bucket_id: 11 created: 1543626724 updated: 1543626724 - id: 21 @@ -181,6 +193,7 @@ created_by_id: 6 list_id: 12 index: 1 + bucket_id: 12 created: 1543626724 updated: 1543626724 - id: 22 @@ -189,6 +202,7 @@ created_by_id: 6 list_id: 13 index: 1 + bucket_id: 13 created: 1543626724 updated: 1543626724 - id: 23 @@ -197,6 +211,7 @@ created_by_id: 6 list_id: 14 index: 1 + bucket_id: 14 created: 1543626724 updated: 1543626724 - id: 24 @@ -205,6 +220,7 @@ created_by_id: 6 list_id: 15 index: 1 + bucket_id: 15 created: 1543626724 updated: 1543626724 - id: 25 @@ -213,6 +229,7 @@ created_by_id: 6 list_id: 16 index: 1 + bucket_id: 16 created: 1543626724 updated: 1543626724 - id: 26 @@ -221,6 +238,7 @@ created_by_id: 6 list_id: 17 index: 1 + bucket_id: 17 created: 1543626724 updated: 1543626724 - id: 27 @@ -229,6 +247,7 @@ created_by_id: 1 list_id: 1 index: 12 + bucket_id: 1 created: 1543626724 updated: 1543626724 - id: 28 @@ -238,6 +257,7 @@ repeat_after: 3600 list_id: 1 index: 13 + bucket_id: 1 created: 1543626724 updated: 1543626724 - id: 29 @@ -246,6 +266,7 @@ created_by_id: 1 list_id: 1 index: 14 + bucket_id: 1 created: 1543626724 updated: 1543626724 - id: 30 @@ -254,6 +275,7 @@ created_by_id: 1 list_id: 1 index: 15 + bucket_id: 1 created: 1543626724 updated: 1543626724 - id: 31 @@ -263,6 +285,7 @@ list_id: 1 index: 16 hex_color: f0f0f0 + bucket_id: 1 created: 1543626724 updated: 1543626724 - id: 32 @@ -271,6 +294,7 @@ created_by_id: 1 list_id: 3 index: 1 + bucket_id: 21 created: 1543626724 updated: 1543626724 - id: 33 @@ -280,6 +304,7 @@ list_id: 1 index: 17 percent_done: 0.5 + bucket_id: 1 created: 1543626724 updated: 1543626724 # This task is forbidden for user1 @@ -289,6 +314,7 @@ created_by_id: 13 list_id: 20 index: 20 + bucket_id: 5 created: 1543626724 updated: 1543626724 - id: 35 @@ -297,6 +323,7 @@ created_by_id: 1 list_id: 21 index: 1 + bucket_id: 19 created: 1543626724 updated: 1543626724 - id: 36 @@ -305,6 +332,7 @@ created_by_id: 1 list_id: 22 index: 1 + bucket_id: 20 created: 1543626724 updated: 1543626724 diff --git a/pkg/integrations/kanban_test.go b/pkg/integrations/kanban_test.go index 5d30e13e90..3e6f912b52 100644 --- a/pkg/integrations/kanban_test.go +++ b/pkg/integrations/kanban_test.go @@ -133,7 +133,7 @@ func TestBucket(t *testing.T) { }) t.Run("Delete", func(t *testing.T) { t.Run("Normal", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "1"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "1", "bucket": "1"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) @@ -145,70 +145,70 @@ func TestBucket(t *testing.T) { t.Run("Rights check", func(t *testing.T) { t.Run("Forbidden", func(t *testing.T) { // Owned by user13 - _, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "5"}) + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "20", "bucket": "5"}) assert.Error(t, err) assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) }) t.Run("Shared Via Team readonly", func(t *testing.T) { - _, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "6"}) + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "6", "bucket": "6"}) assert.Error(t, err) assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) }) t.Run("Shared Via Team write", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "7"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "7", "bucket": "7"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) t.Run("Shared Via Team admin", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "8"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "8", "bucket": "8"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) t.Run("Shared Via User readonly", func(t *testing.T) { - _, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "9"}) + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "9", "bucket": "9"}) assert.Error(t, err) assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) }) t.Run("Shared Via User write", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "10"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "10", "bucket": "10"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) t.Run("Shared Via User admin", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "11"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "11", "bucket": "11"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) t.Run("Shared Via NamespaceTeam readonly", func(t *testing.T) { - _, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "12"}) + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "12", "bucket": "12"}) assert.Error(t, err) assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) }) t.Run("Shared Via NamespaceTeam write", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "13"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "13", "bucket": "13"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) t.Run("Shared Via NamespaceTeam admin", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "14"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "14", "bucket": "14"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) t.Run("Shared Via NamespaceUser readonly", func(t *testing.T) { - _, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "15"}) + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "15", "bucket": "15"}) assert.Error(t, err) assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) }) t.Run("Shared Via NamespaceUser write", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "16"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "16", "bucket": "16"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) t.Run("Shared Via NamespaceUser admin", func(t *testing.T) { - rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"bucket": "17"}) + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"list": "17", "bucket": "17"}) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"message":"Successfully deleted."`) }) diff --git a/pkg/integrations/task_collection_test.go b/pkg/integrations/task_collection_test.go index 00d3deaee8..569833975a 100644 --- a/pkg/integrations/task_collection_test.go +++ b/pkg/integrations/task_collection_test.go @@ -95,7 +95,7 @@ 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,"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}}]`) + 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":1,"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) @@ -105,7 +105,7 @@ func TestTaskCollection(t *testing.T) { 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,"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}}]`) + 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":1,"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) { @@ -270,7 +270,7 @@ 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,"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}}]`) + 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":1,"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) @@ -280,7 +280,7 @@ func TestTaskCollection(t *testing.T) { 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,"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}}]`) + 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":1,"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) { diff --git a/pkg/migration/20200425182634.go b/pkg/migration/20200425182634.go new file mode 100644 index 0000000000..f53977638c --- /dev/null +++ b/pkg/migration/20200425182634.go @@ -0,0 +1,75 @@ +// 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" + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20200425182634", + Description: "Create one bucket for each list", + Migrate: func(tx *xorm.Engine) (err error) { + lists := []*models.List{} + err = tx.Find(&lists) + if err != nil { + return + } + + tasks := []*models.Task{} + err = tx.Find(&tasks) + if err != nil { + return + } + + // This map contains all buckets with their list ids as key + buckets := make(map[int64]*models.Bucket, len(lists)) + for _, l := range lists { + buckets[l.ID] = &models.Bucket{ + ListID: l.ID, + Title: "New Bucket", + // The bucket creator is just the same as the list's one + CreatedByID: l.OwnerID, + } + _, err = tx.Insert(buckets[l.ID]) + if err != nil { + return + } + + for _, t := range tasks { + if t.ListID != l.ID { + continue + } + + t.BucketID = buckets[l.ID].ID + _, err = tx.Where("id = ?", t.ID).Update(t) + if err != nil { + return + } + } + } + + return + }, + Rollback: func(tx *xorm.Engine) error { + return nil + }, + }) +} diff --git a/pkg/models/error.go b/pkg/models/error.go index 952a15ebac..1fe4321e22 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -1251,3 +1251,31 @@ func (err ErrBucketDoesNotBelongToList) HTTPError() web.HTTPError { Message: "This bucket does not belong to that list.", } } + +// ErrCannotRemoveLastBucket represents an error where a kanban bucket is the last on a list and thus cannot be removed. +type ErrCannotRemoveLastBucket struct { + BucketID int64 + ListID int64 +} + +// IsErrCannotRemoveLastBucket checks if an error is ErrCannotRemoveLastBucket. +func IsErrCannotRemoveLastBucket(err error) bool { + _, ok := err.(ErrCannotRemoveLastBucket) + return ok +} + +func (err ErrCannotRemoveLastBucket) Error() string { + return fmt.Sprintf("Cannot remove last bucket of list [BucketID: %d, ListID: %d]", err.BucketID, err.ListID) +} + +// ErrCodeCannotRemoveLastBucket holds the unique world-error code of this error +const ErrCodeCannotRemoveLastBucket = 10003 + +// HTTPError holds the http error description +func (err ErrCannotRemoveLastBucket) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusPreconditionFailed, + Code: ErrCodeCannotRemoveLastBucket, + Message: "You cannot remove the last bucket on this list.", + } +} diff --git a/pkg/models/kanban.go b/pkg/models/kanban.go index affb952f3a..498765d49c 100644 --- a/pkg/models/kanban.go +++ b/pkg/models/kanban.go @@ -20,7 +20,6 @@ import ( "code.vikunja.io/api/pkg/timeutil" "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" - "time" ) // Bucket represents a kanban bucket @@ -64,6 +63,15 @@ func getBucketByID(id int64) (b *Bucket, err error) { return } +func getDefaultBucket(listID int64) (bucket *Bucket, err error) { + bucket = &Bucket{} + _, err = x. + Where("list_id = ?", listID). + OrderBy("id asc"). + Get(bucket) + return +} + // ReadAll returns all buckets with their tasks for a certain list // @Summary Get all kanban buckets of a list // @Description Returns all kanban buckets with belong to a list including their tasks. @@ -81,23 +89,7 @@ func (b *Bucket) ReadAll(auth web.Auth, search string, page int, perPage int) (r // I'll probably just don't do it and instead make individual tasks archivable. // Get all buckets for this list - buckets := []*Bucket{ - { - // This is the default bucket for all tasks which are not associated to a bucket. - ID: 0, - Title: "Not associated to a bucket", - ListID: b.ListID, - Created: timeutil.FromTime(time.Now()), - Updated: timeutil.FromTime(time.Now()), - CreatedByID: auth.GetID(), - }, - } - - buckets[0].CreatedBy, err = user.GetFromAuth(auth) - if err != nil { - return - } - + buckets := []*Bucket{} err = x.Where("list_id = ?", b.ListID).Find(&buckets) if err != nil { return @@ -189,7 +181,7 @@ func (b *Bucket) Update() (err error) { // Delete removes a bucket, but no tasks // @Summary Deletes an existing bucket -// @Description Deletes an existing kanban bucket and dissociates all of its task. It does not delete any tasks. +// @Description Deletes an existing kanban bucket and dissociates all of its task. It does not delete any tasks. You cannot delete the last bucket on a list. // @tags task // @Accept json // @Produce json @@ -201,8 +193,26 @@ func (b *Bucket) Update() (err error) { // @Failure 500 {object} models.Message "Internal error" // @Router /lists/{listID}/buckets/{bucketID} [delete] func (b *Bucket) Delete() (err error) { + // Prevent removing the last bucket + total, err := x.Where("list_id = ?", b.ListID).Count(&Bucket{}) + if err != nil { + return + } + if total <= 1 { + return ErrCannotRemoveLastBucket{ + BucketID: b.ID, + ListID: b.ListID, + } + } + + // Get the default bucket + defaultBucket, err := getDefaultBucket(b.ListID) + if err != nil { + return + } + // Remove all associations of tasks to that bucket - _, err = x.Where("bucket_id = ?", b.ID).Cols("bucket_id").Update(&Task{BucketID: 0}) + _, err = x.Where("bucket_id = ?", b.ID).Cols("bucket_id").Update(&Task{BucketID: defaultBucket.ID}) if err != nil { return } diff --git a/pkg/models/kanban_test.go b/pkg/models/kanban_test.go index 52b77241e7..64c41ee94e 100644 --- a/pkg/models/kanban_test.go +++ b/pkg/models/kanban_test.go @@ -38,30 +38,55 @@ func TestBucket_ReadAll(t *testing.T) { assert.Equal(t, testuser.ID, buckets[0].CreatedBy.ID) assert.Equal(t, testuser.ID, buckets[1].CreatedBy.ID) assert.Equal(t, testuser.ID, buckets[2].CreatedBy.ID) - assert.Equal(t, testuser.ID, buckets[3].CreatedBy.ID) - // Assert our three test buckets + one for all tasks without a bucket - assert.Len(t, buckets, 4) + // Assert our three test buckets + assert.Len(t, buckets, 3) // Assert all tasks are in the right bucket - assert.Len(t, buckets[0].Tasks, 10) - assert.Len(t, buckets[1].Tasks, 2) + assert.Len(t, buckets[0].Tasks, 12) + assert.Len(t, buckets[1].Tasks, 3) assert.Len(t, buckets[2].Tasks, 3) - assert.Len(t, buckets[3].Tasks, 3) // Assert we have bucket 0, 1, 2, 3 but not 4 (that belongs to a different list) - assert.Equal(t, int64(1), buckets[1].ID) - assert.Equal(t, int64(2), buckets[2].ID) - assert.Equal(t, int64(3), buckets[3].ID) + assert.Equal(t, int64(1), buckets[0].ID) + assert.Equal(t, int64(2), buckets[1].ID) + assert.Equal(t, int64(3), buckets[2].ID) // Kinda assert all tasks are in the right buckets - assert.Equal(t, int64(0), buckets[0].Tasks[0].BucketID) - assert.Equal(t, int64(1), buckets[1].Tasks[0].BucketID) - assert.Equal(t, int64(1), buckets[1].Tasks[1].BucketID) - assert.Equal(t, int64(2), buckets[2].Tasks[0].BucketID) - assert.Equal(t, int64(2), buckets[2].Tasks[1].BucketID) - assert.Equal(t, int64(2), buckets[2].Tasks[2].BucketID) - assert.Equal(t, int64(3), buckets[3].Tasks[0].BucketID) - assert.Equal(t, int64(3), buckets[3].Tasks[1].BucketID) - assert.Equal(t, int64(3), buckets[3].Tasks[2].BucketID) + assert.Equal(t, int64(1), buckets[0].Tasks[0].BucketID) + assert.Equal(t, int64(1), buckets[0].Tasks[1].BucketID) + assert.Equal(t, int64(2), buckets[1].Tasks[0].BucketID) + assert.Equal(t, int64(2), buckets[1].Tasks[1].BucketID) + assert.Equal(t, int64(2), buckets[1].Tasks[2].BucketID) + assert.Equal(t, int64(3), buckets[2].Tasks[0].BucketID) + assert.Equal(t, int64(3), buckets[2].Tasks[1].BucketID) + assert.Equal(t, int64(3), buckets[2].Tasks[2].BucketID) +} + +func TestBucket_Delete(t *testing.T) { + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + b := &Bucket{ + ID: 2, // The second bucket only has 3 tasks + ListID: 1, + } + err := b.Delete() + assert.NoError(t, err) + + // Assert all tasks have been moved to bucket 1 as that one is the first + tasks := []*Task{} + err = x.Where("bucket_id = ?", 1).Find(&tasks) + assert.NoError(t, err) + assert.Len(t, tasks, 15) + }) + t.Run("last bucket in list", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + b := &Bucket{ + ID: 34, + ListID: 18, + } + err := b.Delete() + assert.Error(t, err) + assert.True(t, IsErrCannotRemoveLastBucket(err)) + }) } diff --git a/pkg/models/task_collection_test.go b/pkg/models/task_collection_test.go index 25abfac3d1..13b7225259 100644 --- a/pkg/models/task_collection_test.go +++ b/pkg/models/task_collection_test.go @@ -75,6 +75,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { Index: 14, CreatedByID: 1, ListID: 1, + BucketID: 1, Created: 1543626724, Updated: 1543626724, }, @@ -223,6 +224,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user1, ListID: 1, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 1, Created: 1543626724, Updated: 1543626724, StartDate: 1544600000, @@ -237,6 +239,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user1, ListID: 1, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 1, Created: 1543626724, Updated: 1543626724, } @@ -249,6 +252,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user1, ListID: 1, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 1, Created: 1543626724, Updated: 1543626724, } @@ -261,6 +265,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user1, ListID: 1, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 1, Created: 1543626724, Updated: 1543626724, } @@ -273,6 +278,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 6, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 6, Created: 1543626724, Updated: 1543626724, } @@ -285,6 +291,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 7, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 7, Created: 1543626724, Updated: 1543626724, } @@ -297,6 +304,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 8, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 8, Created: 1543626724, Updated: 1543626724, } @@ -309,6 +317,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 9, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 9, Created: 1543626724, Updated: 1543626724, } @@ -321,6 +330,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 10, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 10, Created: 1543626724, Updated: 1543626724, } @@ -333,6 +343,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 11, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 11, Created: 1543626724, Updated: 1543626724, } @@ -345,6 +356,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 12, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 12, Created: 1543626724, Updated: 1543626724, } @@ -357,6 +369,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 13, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 13, Created: 1543626724, Updated: 1543626724, } @@ -369,6 +382,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 14, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 14, Created: 1543626724, Updated: 1543626724, } @@ -381,6 +395,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 15, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 15, Created: 1543626724, Updated: 1543626724, } @@ -393,6 +408,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 16, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 16, Created: 1543626724, Updated: 1543626724, } @@ -405,6 +421,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user6, ListID: 17, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 17, Created: 1543626724, Updated: 1543626724, } @@ -417,6 +434,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user1, Reminders: []timeutil.TimeStamp{1543626724, 1543626824}, ListID: 1, + BucketID: 1, RelatedTasks: map[RelationKind][]*Task{}, Created: 1543626724, Updated: 1543626724, @@ -431,6 +449,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { ListID: 1, RelatedTasks: map[RelationKind][]*Task{}, RepeatAfter: 3600, + BucketID: 1, Created: 1543626724, Updated: 1543626724, } @@ -457,8 +476,9 @@ func TestTaskCollection_ReadAll(t *testing.T) { }, }, }, - Created: 1543626724, - Updated: 1543626724, + BucketID: 1, + Created: 1543626724, + Updated: 1543626724, } task30 := &Task{ ID: 30, @@ -473,6 +493,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { user2, }, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 1, Created: 1543626724, Updated: 1543626724, } @@ -486,6 +507,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user1, ListID: 1, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 1, Created: 1543626724, Updated: 1543626724, } @@ -498,6 +520,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { CreatedBy: user1, ListID: 3, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 21, Created: 1543626724, Updated: 1543626724, } @@ -511,6 +534,7 @@ func TestTaskCollection_ReadAll(t *testing.T) { ListID: 1, PercentDone: 0.5, RelatedTasks: map[RelationKind][]*Task{}, + BucketID: 1, Created: 1543626724, Updated: 1543626724, } diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index 3c9ac32816..e807c4b83d 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -544,6 +544,15 @@ func (t *Task) Create(a web.Auth) (err error) { return } + // Get the default bucket and move the task there + if t.BucketID == 0 { + defaultBucket, err := getDefaultBucket(t.ListID) + if err != nil { + return err + } + t.BucketID = defaultBucket.ID + } + // Get the index for this task latestTask := &Task{} _, err = x.Where("list_id = ?", t.ListID).OrderBy("id desc").Get(latestTask) diff --git a/pkg/models/tasks_test.go b/pkg/models/tasks_test.go index bed4c8ecec..c7dd36e525 100644 --- a/pkg/models/tasks_test.go +++ b/pkg/models/tasks_test.go @@ -47,6 +47,8 @@ func TestTask_Create(t *testing.T) { // Assert getting a new index assert.NotEmpty(t, task.Index) assert.Equal(t, int64(18), task.Index) + // Assert moving it into the default bucket + assert.Equal(t, int64(1), task.BucketID) }) t.Run("empty text", func(t *testing.T) {