diff --git a/docs/content/doc/usage/errors.md b/docs/content/doc/usage/errors.md index 7010deeb..f565daa5 100644 --- a/docs/content/doc/usage/errors.md +++ b/docs/content/doc/usage/errors.md @@ -34,6 +34,7 @@ This document describes the different errors Vikunja can return. | 3005 | 400 | The list title cannot be empty. | | 3006 | 404 | The list share does not exist. | | 3007 | 400 | A list with this identifier already exists. | +| 3008 | 412 | The list is archived and can therefore only be accessed read only. This is also true for all tasks associated with this list. | | 4001 | 400 | The list task text cannot be empty. | | 4002 | 404 | The list task does not exist. | | 4003 | 403 | All bulk editing tasks must belong to the same list. | @@ -55,6 +56,7 @@ This document describes the different errors Vikunja can return. | 5009 | 403 | The user needs to have namespace read access to perform that action. | | 5010 | 403 | This team does not have access to that namespace. | | 5011 | 409 | This user has already access to that namespace. | +| 5012 | 412 | The namespace is archived and can therefore only be accessed read only. | | 6001 | 400 | The team name cannot be emtpy. | | 6002 | 404 | The team does not exist. | | 6004 | 409 | The team already has access to that namespace or list. | diff --git a/pkg/db/fixtures/label_task.yml b/pkg/db/fixtures/label_task.yml index c3fd91f7..218fef56 100644 --- a/pkg/db/fixtures/label_task.yml +++ b/pkg/db/fixtures/label_task.yml @@ -6,3 +6,11 @@ task_id: 2 label_id: 4 created: 0 +- id: 3 + task_id: 35 + label_id: 4 + created: 0 +- id: 4 + task_id: 36 + label_id: 4 + created: 0 diff --git a/pkg/db/fixtures/list.yml b/pkg/db/fixtures/list.yml index fa8edcb9..f5effdd3 100644 --- a/pkg/db/fixtures/list.yml +++ b/pkg/db/fixtures/list.yml @@ -180,3 +180,22 @@ namespace_id: 15 updated: 0 created: 0 +- + id: 21 + title: Test21 archived through namespace + description: Lorem Ipsum + identifier: test21 + owner_id: 1 + namespace_id: 16 + updated: 0 + created: 0 +- + id: 22 + title: Test22 archived individually + description: Lorem Ipsum + identifier: test22 + owner_id: 1 + namespace_id: 1 + is_archived: 1 + updated: 0 + created: 0 diff --git a/pkg/db/fixtures/namespaces.yml b/pkg/db/fixtures/namespaces.yml index 447d9c31..9d3c5511 100644 --- a/pkg/db/fixtures/namespaces.yml +++ b/pkg/db/fixtures/namespaces.yml @@ -76,3 +76,9 @@ owner_id: 13 updated: 0 created: 0 +- id: 16 + name: Archived testnamespace16 + owner_id: 1 + is_archived: 1 + updated: 0 + created: 0 diff --git a/pkg/db/fixtures/task_assignees.yml b/pkg/db/fixtures/task_assignees.yml index 9104a48c..ae4ca1cb 100644 --- a/pkg/db/fixtures/task_assignees.yml +++ b/pkg/db/fixtures/task_assignees.yml @@ -5,4 +5,12 @@ - id: 2 task_id: 30 user_id: 2 - created: 0 \ No newline at end of file + created: 0 +- id: 3 + task_id: 35 + user_id: 2 + created: 0 +- id: 4 + task_id: 36 + user_id: 2 + created: 0 diff --git a/pkg/db/fixtures/task_comments.yml b/pkg/db/fixtures/task_comments.yml index 76f599fe..385a3da3 100644 --- a/pkg/db/fixtures/task_comments.yml +++ b/pkg/db/fixtures/task_comments.yml @@ -82,3 +82,15 @@ task_id: 26 created: 1582135626 updated: 1582135626 +- id: 15 + comment: comment 15 + author_id: 1 + task_id: 35 + created: 1582135626 + updated: 1582135626 +- id: 16 + comment: comment 16 + author_id: 1 + task_id: 36 + created: 1582135626 + updated: 1582135626 diff --git a/pkg/db/fixtures/task_relations.yml b/pkg/db/fixtures/task_relations.yml index 6abf882a..08bd448a 100644 --- a/pkg/db/fixtures/task_relations.yml +++ b/pkg/db/fixtures/task_relations.yml @@ -10,3 +10,27 @@ relation_kind: 'parenttask' created_by_id: 1 created: 0 +- id: 3 + task_id: 35 + other_task_id: 1 + relation_kind: 'related' + created_by_id: 1 + created: 0 +- id: 4 + task_id: 35 + other_task_id: 1 + relation_kind: 'related' + created_by_id: 1 + created: 0 +- id: 5 + task_id: 36 + other_task_id: 1 + relation_kind: 'related' + created_by_id: 1 + created: 0 +- id: 6 + task_id: 36 + other_task_id: 1 + relation_kind: 'related' + created_by_id: 1 + created: 0 diff --git a/pkg/db/fixtures/tasks.yml b/pkg/db/fixtures/tasks.yml index d7f7fae8..40742e33 100644 --- a/pkg/db/fixtures/tasks.yml +++ b/pkg/db/fixtures/tasks.yml @@ -251,5 +251,19 @@ index: 20 created: 1543626724 updated: 1543626724 +- id: 35 + text: 'task #35' + created_by_id: 1 + list_id: 21 + index: 1 + created: 1543626724 + updated: 1543626724 +- id: 36 + text: 'task #36' + created_by_id: 1 + list_id: 22 + index: 1 + created: 1543626724 + updated: 1543626724 diff --git a/pkg/integrations/archived_test.go b/pkg/integrations/archived_test.go new file mode 100644 index 00000000..0099f801 --- /dev/null +++ b/pkg/integrations/archived_test.go @@ -0,0 +1,238 @@ +// Copyright 2020 Vikunja and contriubtors. All rights reserved. +// +// This file is part of Vikunja. +// +// Vikunja 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. +// +// Vikunja 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 Vikunja. If not, see . + +package integrations + +import ( + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/web/handler" + "github.com/stretchr/testify/assert" + "net/url" + "testing" +) + +// This tests the following behaviour: +// 1. A namespace should not be editable if it is archived. +// 1. With the exception being to un-archive it. +// 2. A list which belongs to an archived namespace cannot be edited. +// 3. An archived list should not be editable. +// 1. Except for un-archiving it. +// 4. It is not possible to un-archive a list individually if its namespace is archived. +// 5. Creating new lists on an archived namespace should not work. +// 6. Creating new tasks on an archived list should not work. +// 7. Creating new tasks on a list who's namespace is archived should not work. +// 8. Editing tasks on an archived list should not work. +// 9. Editing tasks on a list who's namespace is archived should not work. +// 10. Archived namespaces should not appear in the list with all namespaces. +// 11. Archived lists should not appear in the list with all lists. +// 12. Lists who's namespace is archived should not appear in the list with all lists. +// +// All of this is tested through integration tests because it's not yet clear if this will be implemented directly +// or with some kind of middleware. +// +// Maybe the inheritance of lists from namespaces could be solved with some kind of is_archived_inherited flag - +// that way I'd only need to implement the checking on a list level and update the flag for all lists once the +// namespace is archived. The archived flag would then be used to not accedentially unarchive lists which were +// already individually archived when the namespace was archived. +// Should still test it all though. +// +// Namespace 16 is archived +// List 21 belongs to namespace 16 +// List 22 is archived individually + +func TestArchived(t *testing.T) { + testListHandler := webHandlerTest{ + user: &testuser1, + strFunc: func() handler.CObject { + return &models.List{} + }, + t: t, + } + testNamespaceHandler := webHandlerTest{ + user: &testuser1, + strFunc: func() handler.CObject { + return &models.Namespace{} + }, + t: t, + } + testTaskHandler := webHandlerTest{ + user: &testuser1, + strFunc: func() handler.CObject { + return &models.Task{} + }, + t: t, + } + testLabelHandler := webHandlerTest{ + user: &testuser1, + strFunc: func() handler.CObject { + return &models.LabelTask{} + }, + t: t, + } + testAssigneeHandler := webHandlerTest{ + user: &testuser1, + strFunc: func() handler.CObject { + return &models.TaskAssginee{} + }, + t: t, + } + testRelationHandler := webHandlerTest{ + user: &testuser1, + strFunc: func() handler.CObject { + return &models.TaskRelation{} + }, + t: t, + } + testCommentHandler := webHandlerTest{ + user: &testuser1, + strFunc: func() handler.CObject { + return &models.TaskComment{} + }, + t: t, + } + + t.Run("namespace", func(t *testing.T) { + t.Run("not editable", func(t *testing.T) { + _, err := testNamespaceHandler.testUpdateWithUser(nil, map[string]string{"namespace": "16"}, `{"name":"TestIpsum","is_archived":true}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeNamespaceIsArchived) + }) + t.Run("unarchivable", func(t *testing.T) { + rec, err := testNamespaceHandler.testUpdateWithUser(nil, map[string]string{"namespace": "16"}, `{"name":"TestIpsum","is_archived":false}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"is_archived":false`) + }) + t.Run("no new lists", func(t *testing.T) { + _, err := testListHandler.testCreateWithUser(nil, map[string]string{"namespace": "16"}, `{"title":"Lorem"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeNamespaceIsArchived) + }) + t.Run("should not appear in the list", func(t *testing.T) { + rec, err := testNamespaceHandler.testReadAllWithUser(nil, nil) + assert.NoError(t, err) + assert.NotContains(t, rec.Body.String(), `"name":"Archived testnamespace16"`) + }) + t.Run("should appear in the list if explicitly requested", func(t *testing.T) { + rec, err := testNamespaceHandler.testReadAllWithUser(url.Values{"is_archived": []string{"true"}}, nil) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"name":"Archived testnamespace16"`) + }) + }) + + t.Run("list", func(t *testing.T) { + + taskTests := func(taskID string, errCode int, t *testing.T) { + t.Run("task", func(t *testing.T) { + t.Run("edit task", func(t *testing.T) { + _, err := testTaskHandler.testUpdateWithUser(nil, map[string]string{"listtask": taskID}, `{"text":"TestIpsum"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("delete", func(t *testing.T) { + _, err := testTaskHandler.testDeleteWithUser(nil, map[string]string{"listtask": taskID}) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("add new labels", func(t *testing.T) { + _, err := testLabelHandler.testCreateWithUser(nil, map[string]string{"listtask": taskID}, `{"label_id":1}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("remove lables", func(t *testing.T) { + _, err := testLabelHandler.testDeleteWithUser(nil, map[string]string{"listtask": taskID, "label": "4"}) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("add assignees", func(t *testing.T) { + _, err := testAssigneeHandler.testCreateWithUser(nil, map[string]string{"listtask": taskID}, `{"user_id":3}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("remove assignees", func(t *testing.T) { + _, err := testAssigneeHandler.testDeleteWithUser(nil, map[string]string{"listtask": taskID, "user": "2"}) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("add relation", func(t *testing.T) { + _, err := testRelationHandler.testCreateWithUser(nil, map[string]string{"task": taskID}, `{"other_task_id":1,"relation_kind":"related"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("remove relation", func(t *testing.T) { + _, err := testRelationHandler.testDeleteWithUser(nil, map[string]string{"task": taskID}, `{"other_task_id":2,"relation_kind":"related"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("add comment", func(t *testing.T) { + _, err := testCommentHandler.testCreateWithUser(nil, map[string]string{"task": taskID}, `{"comment":"Lorem"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + t.Run("remove comment", func(t *testing.T) { + var commentID = "15" + if taskID == "36" { + commentID = "16" + } + _, err := testCommentHandler.testDeleteWithUser(nil, map[string]string{"task": taskID, "commentid": commentID}) + assert.Error(t, err) + assertHandlerErrorCode(t, err, errCode) + }) + }) + } + + // The list belongs to an archived namespace + t.Run("archived namespace", func(t *testing.T) { + t.Run("not editable", func(t *testing.T) { + _, err := testListHandler.testUpdateWithUser(nil, map[string]string{"list": "21"}, `{"title":"TestIpsum","is_archived":true}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeNamespaceIsArchived) + }) + t.Run("no new tasks", func(t *testing.T) { + _, err := testTaskHandler.testCreateWithUser(nil, map[string]string{"list": "21"}, `{"text":"Lorem"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeNamespaceIsArchived) + }) + t.Run("not unarchivable", func(t *testing.T) { + _, err := testListHandler.testUpdateWithUser(nil, map[string]string{"list": "21"}, `{"title":"LoremIpsum","is_archived":false}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeNamespaceIsArchived) + }) + + taskTests("35", models.ErrCodeNamespaceIsArchived, t) + }) + // The list itself is archived + t.Run("archived individually", func(t *testing.T) { + t.Run("not editable", func(t *testing.T) { + _, err := testListHandler.testUpdateWithUser(nil, map[string]string{"list": "22"}, `{"title":"TestIpsum","is_archived":true}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeListIsArchived) + }) + t.Run("no new tasks", func(t *testing.T) { + _, err := testTaskHandler.testCreateWithUser(nil, map[string]string{"list": "22"}, `{"text":"Lorem"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeListIsArchived) + }) + t.Run("unarchivable", func(t *testing.T) { + rec, err := testListHandler.testUpdateWithUser(nil, map[string]string{"list": "22"}, `{"title":"LoremIpsum","is_archived":false}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"is_archived":false`) + }) + + taskTests("36", models.ErrCodeListIsArchived, t) + }) + }) +} diff --git a/pkg/integrations/integrations.go b/pkg/integrations/integrations.go index 41c48159..3b0b31a7 100644 --- a/pkg/integrations/integrations.go +++ b/pkg/integrations/integrations.go @@ -221,9 +221,13 @@ func (h *webHandlerTest) testUpdateWithUser(queryParams url.Values, urlParams ma return newTestRequestWithUser(h.t, http.MethodPost, hndl.UpdateWeb, h.user, payload, queryParams, urlParams) } -func (h *webHandlerTest) testDeleteWithUser(queryParams url.Values, urlParams map[string]string) (rec *httptest.ResponseRecorder, err error) { +func (h *webHandlerTest) testDeleteWithUser(queryParams url.Values, urlParams map[string]string, payload ...string) (rec *httptest.ResponseRecorder, err error) { + pl := "" + if len(payload) > 0 { + pl = payload[0] + } hndl := h.getHandler() - return newTestRequestWithUser(h.t, http.MethodDelete, hndl.DeleteWeb, h.user, "", queryParams, urlParams) + return newTestRequestWithUser(h.t, http.MethodDelete, hndl.DeleteWeb, h.user, pl, queryParams, urlParams) } func (h *webHandlerTest) testReadAllWithLinkShare(queryParams url.Values, urlParams map[string]string) (rec *httptest.ResponseRecorder, err error) { diff --git a/pkg/integrations/list_test.go b/pkg/integrations/list_test.go index 8d771356..7b0e50f6 100644 --- a/pkg/integrations/list_test.go +++ b/pkg/integrations/list_test.go @@ -38,10 +38,12 @@ func TestList(t *testing.T) { rec, err := testHandler.testReadAllWithUser(nil, nil) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `Test1`) - assert.NotContains(t, rec.Body.String(), `Test2`) + assert.NotContains(t, rec.Body.String(), `Test2"`) assert.Contains(t, rec.Body.String(), `Test3`) // Shared directly via users_list assert.Contains(t, rec.Body.String(), `Test4`) // Shared via namespace assert.NotContains(t, rec.Body.String(), `Test5`) + assert.NotContains(t, rec.Body.String(), `Test21`) // Archived through namespace + assert.NotContains(t, rec.Body.String(), `Test22`) // Archived directly }) t.Run("Search", func(t *testing.T) { rec, err := testHandler.testReadAllWithUser(url.Values{"s": []string{"Test1"}}, nil) @@ -52,6 +54,17 @@ func TestList(t *testing.T) { assert.NotContains(t, rec.Body.String(), `Test4`) assert.NotContains(t, rec.Body.String(), `Test5`) }) + t.Run("Normal with archived lists", func(t *testing.T) { + rec, err := testHandler.testReadAllWithUser(url.Values{"is_archived": []string{"true"}}, nil) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Test1`) + assert.NotContains(t, rec.Body.String(), `Test2"`) + assert.Contains(t, rec.Body.String(), `Test3`) // Shared directly via users_list + assert.Contains(t, rec.Body.String(), `Test4`) // Shared via namespace + assert.NotContains(t, rec.Body.String(), `Test5`) + assert.Contains(t, rec.Body.String(), `Test21`) // Archived through namespace + assert.Contains(t, rec.Body.String(), `Test22`) // Archived directly + }) }) t.Run("ReadOne", func(t *testing.T) { t.Run("Normal", func(t *testing.T) { diff --git a/pkg/migration/20200308205855.go b/pkg/migration/20200308205855.go new file mode 100644 index 00000000..fbe4d3a0 --- /dev/null +++ b/pkg/migration/20200308205855.go @@ -0,0 +1,44 @@ +// Vikunja is a todo-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 ( + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +type list20200308205855 struct { + IsArchived bool `xorm:"not null default false"` +} + +func (s list20200308205855) TableName() string { + return "list" +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20200308205855", + Description: "Add is archived flag to lists", + Migrate: func(tx *xorm.Engine) error { + return tx.Sync2(list20200308205855{}) + }, + Rollback: func(tx *xorm.Engine) error { + return tx.DropTables(list20200308205855{}) + }, + }) + +} diff --git a/pkg/migration/20200308210130.go b/pkg/migration/20200308210130.go new file mode 100644 index 00000000..c96a38be --- /dev/null +++ b/pkg/migration/20200308210130.go @@ -0,0 +1,44 @@ +// Vikunja is a todo-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 ( + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +type namepsace20200308210130 struct { + IsArchived bool `xorm:"not null default false"` +} + +func (s namepsace20200308210130) TableName() string { + return "namespaces" +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20200308210130", + Description: "Add is archived flag to namepaces", + Migrate: func(tx *xorm.Engine) error { + return tx.Sync2(namepsace20200308210130{}) + }, + Rollback: func(tx *xorm.Engine) error { + return tx.DropTables(namepsace20200308210130{}) + }, + }) + +} diff --git a/pkg/models/error.go b/pkg/models/error.go index 77b70ba6..5590cd95 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -231,6 +231,29 @@ func (err ErrListIdentifierIsNotUnique) HTTPError() web.HTTPError { } } +// ErrListIsArchived represents an error, where a list is archived +type ErrListIsArchived struct { + ListID int64 +} + +// IsErrListIsArchived checks if an error is a . +func IsErrListIsArchived(err error) bool { + _, ok := err.(ErrListIsArchived) + return ok +} + +func (err ErrListIsArchived) Error() string { + return fmt.Sprintf("List is archived [ListID: %d]", err.ListID) +} + +// ErrCodeListIsArchived holds the unique world-error code of this error +const ErrCodeListIsArchived = 3008 + +// HTTPError holds the http error description +func (err ErrListIsArchived) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusPreconditionFailed, Code: ErrCodeListIsArchived, Message: "This lists is archived. Editing or creating new tasks is not possible."} +} + // ================ // List task errors // ================ @@ -777,6 +800,29 @@ func (err ErrUserAlreadyHasNamespaceAccess) HTTPError() web.HTTPError { return web.HTTPError{HTTPCode: http.StatusConflict, Code: ErrCodeUserAlreadyHasNamespaceAccess, Message: "This user already has access to this namespace."} } +// ErrNamespaceIsArchived represents an error where a namespace is archived +type ErrNamespaceIsArchived struct { + NamespaceID int64 +} + +// IsErrNamespaceIsArchived checks if an error is a . +func IsErrNamespaceIsArchived(err error) bool { + _, ok := err.(ErrNamespaceIsArchived) + return ok +} + +func (err ErrNamespaceIsArchived) Error() string { + return fmt.Sprintf("Namespace is archived [NamespaceID: %d]", err.NamespaceID) +} + +// ErrCodeNamespaceIsArchived holds the unique world-error code of this error +const ErrCodeNamespaceIsArchived = 5012 + +// HTTPError holds the http error description +func (err ErrNamespaceIsArchived) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusPreconditionFailed, Code: ErrCodeNamespaceIsArchived, Message: "This namespaces is archived. Editing or creating new lists is not possible."} +} + // ============ // Team errors // ============ diff --git a/pkg/models/label.go b/pkg/models/label.go index bfdcd2c7..fab3280d 100644 --- a/pkg/models/label.go +++ b/pkg/models/label.go @@ -203,7 +203,10 @@ func getLabelByIDSimple(labelID int64) (*Label, error) { func getUserTaskIDs(u *user.User) (taskIDs []int64, err error) { // Get all lists - lists, _, _, err := getRawListsForUser("", u, -1, 0) + lists, _, _, err := getRawListsForUser(&listOptions{ + user: u, + page: -1, + }) if err != nil { return nil, err } diff --git a/pkg/models/list.go b/pkg/models/list.go index 9ddde6a0..3ff82c3f 100644 --- a/pkg/models/list.go +++ b/pkg/models/list.go @@ -21,6 +21,7 @@ import ( "code.vikunja.io/api/pkg/timeutil" "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" + "xorm.io/builder" ) // List represents a list of tasks @@ -43,6 +44,9 @@ type List struct { // Deprecated: you should use the dedicated task list endpoint because it has support for pagination and filtering Tasks []*Task `xorm:"-" json:"-"` + // Whether or not a list is archived. + IsArchived bool `xorm:"not null default false" json:"is_archived" query:"is_archived"` + // A timestamp when this list was created. You cannot change this value. Created timeutil.TimeStamp `xorm:"created not null" json:"created"` // A timestamp when this list was last updated. You cannot change this value. @@ -57,16 +61,24 @@ func GetListsByNamespaceID(nID int64, doer *user.User) (lists []*List, err error if nID == -1 { err = x.Select("l.*"). Table("list"). - Alias("l"). Join("LEFT", []string{"team_list", "tl"}, "l.id = tl.list_id"). Join("LEFT", []string{"team_members", "tm"}, "tm.team_id = tl.team_id"). Join("LEFT", []string{"users_list", "ul"}, "ul.list_id = l.id"). + Join("LEFT", []string{"namespaces", "n"}, "l.namespace_id = n.id"). Where("tm.user_id = ?", doer.ID). + Where("l.is_archived = false"). + Where("n.is_archived = false"). Or("ul.user_id = ?", doer.ID). GroupBy("l.id"). Find(&lists) } else { - err = x.Where("namespace_id = ?", nID).Find(&lists) + err = x.Select("l.*"). + Alias("l"). + Join("LEFT", []string{"namespaces", "n"}, "l.namespace_id = n.id"). + Where("l.is_archived = false"). + Where("n.is_archived = false"). + Where("namespace_id = ?", nID). + Find(&lists) } if err != nil { return nil, err @@ -86,6 +98,7 @@ func GetListsByNamespaceID(nID int64, doer *user.User) (lists []*List, err error // @Param page query int false "The page number. Used for pagination. If not provided, the first page of results is returned." // @Param per_page query int false "The maximum number of items per page. Note this parameter is limited by the configured maximum of items per page." // @Param s query string false "Search lists by title." +// @Param is_archived query bool false "If true, also returns all archived lists." // @Security JWTKeyAuth // @Success 200 {array} models.List "The lists" // @Failure 403 {object} code.vikunja.io/web.HTTPError "The user does not have access to the list" @@ -105,7 +118,13 @@ func (l *List) ReadAll(a web.Auth, search string, page int, perPage int) (result return lists, 0, 0, err } - lists, resultCount, totalItems, err := getRawListsForUser(search, &user.User{ID: a.GetID()}, page, perPage) + lists, resultCount, totalItems, err := getRawListsForUser(&listOptions{ + search: search, + user: &user.User{ID: a.GetID()}, + page: page, + perPage: perPage, + isArchived: l.IsArchived, + }) if err != nil { return nil, 0, 0, err } @@ -177,13 +196,30 @@ func GetListSimplByTaskID(taskID int64) (l *List, err error) { return &list, nil } +type listOptions struct { + search string + user *user.User + page int + perPage int + isArchived bool +} + // Gets the lists only, without any tasks or so -func getRawListsForUser(search string, u *user.User, page int, perPage int) (lists []*List, resultCount int, totalItems int64, err error) { - fullUser, err := user.GetUserByID(u.ID) +func getRawListsForUser(opts *listOptions) (lists []*List, resultCount int, totalItems int64, err error) { + fullUser, err := user.GetUserByID(opts.user.ID) if err != nil { return nil, 0, 0, err } + // Adding a 1=1 condition by default here because xorm always needs a condition and cannot handle nil conditions + var isArchivedCond builder.Cond = builder.Eq{"1": 1} + if !opts.isArchived { + isArchivedCond = builder.And( + builder.Eq{"l.is_archived": false}, + builder.Eq{"n.is_archived": false}, + ) + } + // Gets all Lists where the user is either owner or in a team which has access to the list // Or in a team which has namespace read access err = x.Select("l.*"). @@ -202,8 +238,9 @@ func getRawListsForUser(search string, u *user.User, page int, perPage int) (lis Or("ul.user_id = ?", fullUser.ID). Or("un.user_id = ?", fullUser.ID). GroupBy("l.id"). - Limit(getLimitFromPageIndex(page, perPage)). - Where("l.title LIKE ?", "%"+search+"%"). + Limit(getLimitFromPageIndex(opts.page, opts.perPage)). + Where("l.title LIKE ?", "%"+opts.search+"%"). + Where(isArchivedCond). Find(&lists) if err != nil { return nil, 0, 0, err @@ -225,8 +262,9 @@ func getRawListsForUser(search string, u *user.User, page int, perPage int) (lis Or("ul.user_id = ?", fullUser.ID). Or("un.user_id = ?", fullUser.ID). GroupBy("l.id"). - Limit(getLimitFromPageIndex(page, perPage)). - Where("l.title LIKE ?", "%"+search+"%"). + Limit(getLimitFromPageIndex(opts.page, opts.perPage)). + Where("l.title LIKE ?", "%"+opts.search+"%"). + Where(isArchivedCond). Count(&List{}) return lists, len(lists), totalItems, err } @@ -259,6 +297,38 @@ func AddListDetails(lists []*List) (err error) { return } +// NamespaceList is a meta type to be able to join a list with its namespace +type NamespaceList struct { + List List `xorm:"extends"` + Namespace Namespace `xorm:"extends"` +} + +// CheckIsArchived returns an ErrListIsArchived or ErrNamespaceIsArchived if the list or its namespace is archived. +func (l *List) CheckIsArchived() (err error) { + // When creating a new list, we check if the namespace is archived + if l.ID == 0 { + n := &Namespace{ID: l.NamespaceID} + return n.CheckIsArchived() + } + + nl := &NamespaceList{} + exists, err := x. + Table("list"). + Join("LEFT", "namespaces", "list.namespace_id = namespaces.id"). + Where("list.id = ? AND (list.is_archived = true OR namespaces.is_archived = true)", l.ID). + Get(nl) + if err != nil { + return + } + if exists && nl.List.ID != 0 && nl.List.IsArchived { + return ErrListIsArchived{ListID: l.ID} + } + if exists && nl.Namespace.ID != 0 && nl.Namespace.IsArchived { + return ErrNamespaceIsArchived{NamespaceID: nl.Namespace.ID} + } + return nil +} + // CreateOrUpdateList updates a list or creates it if it doesn't exist func CreateOrUpdateList(list *List) (err error) { @@ -285,7 +355,22 @@ func CreateOrUpdateList(list *List) (err error) { _, err = x.Insert(list) metrics.UpdateCount(1, metrics.ListCountKey) } else { - _, err = x.ID(list.ID).Update(list) + // We need to specify the cols we want to update here to be able to un-archive lists + colsToUpdate := []string{ + "title", + "is_archived", + } + if list.Description != "" { + colsToUpdate = append(colsToUpdate, "description") + } + if list.Identifier != "" { + colsToUpdate = append(colsToUpdate, "identifier") + } + + _, err = x. + ID(list.ID). + Cols(colsToUpdate...). + Update(list) } if err != nil { @@ -350,6 +435,11 @@ func updateListByTaskID(taskID int64) (err error) { // @Failure 500 {object} models.Message "Internal error" // @Router /namespaces/{namespaceID}/lists [put] func (l *List) Create(a web.Auth) (err error) { + err = l.CheckIsArchived() + if err != nil { + return err + } + doer, err := user.GetFromAuth(a) if err != nil { return err diff --git a/pkg/models/list_rights.go b/pkg/models/list_rights.go index 8bebf0f7..b27f5a8e 100644 --- a/pkg/models/list_rights.go +++ b/pkg/models/list_rights.go @@ -32,19 +32,33 @@ func (l *List) CanWrite(a web.Auth) (bool, error) { return false, err } + // We put the result of the is archived check in a separate variable to be able to return it later without + // needing to recheck it again + errIsArchived := originalList.CheckIsArchived() + + var canWrite bool + // Check if we're dealing with a share auth shareAuth, ok := a.(*LinkSharing) if ok { return originalList.ID == shareAuth.ListID && - (shareAuth.Right == RightWrite || shareAuth.Right == RightAdmin), nil + (shareAuth.Right == RightWrite || shareAuth.Right == RightAdmin), errIsArchived } // Check if the user is either owner or can write to the list if originalList.isOwner(&user.User{ID: a.GetID()}) { - return true, nil + canWrite = true } - return originalList.checkRight(a, RightWrite, RightAdmin) + if canWrite { + return canWrite, errIsArchived + } + + canWrite, err = originalList.checkRight(a, RightWrite, RightAdmin) + if err != nil { + return false, err + } + return canWrite, errIsArchived } // CanRead checks if a user has read access to a list @@ -68,8 +82,13 @@ func (l *List) CanRead(a web.Auth) (bool, error) { } // CanUpdate checks if the user can update a list -func (l *List) CanUpdate(a web.Auth) (bool, error) { - return l.CanWrite(a) +func (l *List) CanUpdate(a web.Auth) (canUpdate bool, err error) { + canUpdate, err = l.CanWrite(a) + // If the list is archived and the user tries to un-archive it, let the request through + if IsErrListIsArchived(err) && !l.IsArchived { + err = nil + } + return canUpdate, err } // CanDelete checks if the user can delete a list diff --git a/pkg/models/namespace.go b/pkg/models/namespace.go index 1bcf9eab..21299633 100644 --- a/pkg/models/namespace.go +++ b/pkg/models/namespace.go @@ -35,6 +35,9 @@ type Namespace struct { Description string `xorm:"longtext null" json:"description"` OwnerID int64 `xorm:"int(11) not null INDEX" json:"-"` + // Whether or not a namespace is archived. + IsArchived bool `xorm:"not null default false" json:"is_archived" query:"is_archived"` + // The user who owns this namespace Owner *user.User `xorm:"-" json:"owner" valid:"-"` @@ -103,6 +106,20 @@ func GetNamespaceByID(id int64) (namespace Namespace, err error) { return } +// CheckIsArchived returns an ErrNamespaceIsArchived if the namepace is archived. +func (n *Namespace) CheckIsArchived() error { + exists, err := x. + Where("id = ? AND is_archived = true", n.ID). + Exist(&Namespace{}) + if err != nil { + return err + } + if exists { + return ErrNamespaceIsArchived{NamespaceID: n.ID} + } + return nil +} + // ReadOne gets one namespace // @Summary Gets one namespace // @Description Returns a namespace by its ID. @@ -136,6 +153,7 @@ type NamespaceWithLists struct { // @Param page query int false "The page number. Used for pagination. If not provided, the first page of results is returned." // @Param per_page query int false "The maximum number of items per page. Note this parameter is limited by the configured maximum of items per page." // @Param s query string false "Search namespaces by name." +// @Param is_archived query bool false "If true, also returns all archived namespaces." // @Security JWTKeyAuth // @Success 200 {array} models.NamespaceWithLists "The Namespaces." // @Failure 500 {object} models.Message "Internal error" @@ -169,6 +187,7 @@ func (n *Namespace) ReadAll(a web.Auth, search string, page int, perPage int) (r Where("team_members.user_id = ?", doer.ID). Or("namespaces.owner_id = ?", doer.ID). Or("users_namespace.user_id = ?", doer.ID). + And("namespaces.is_archived = ?", n.IsArchived). GroupBy("namespaces.id"). Limit(getLimitFromPageIndex(page, perPage)). Where("namespaces.name LIKE ?", "%"+search+"%"). @@ -188,6 +207,7 @@ func (n *Namespace) ReadAll(a web.Auth, search string, page int, perPage int) (r Where("team_members.user_id = ?", doer.ID). Or("namespaces.owner_id = ?", doer.ID). Or("users_namespace.user_id = ?", doer.ID). + And("namespaces.is_archived = ?", n.IsArchived). GroupBy("users.id"). Find(&users) @@ -220,6 +240,7 @@ func (n *Namespace) ReadAll(a web.Auth, search string, page int, perPage int) (r Join("LEFT", []string{"users_list", "ul"}, "ul.list_id = l.id"). Where("tm.user_id = ?", doer.ID). Or("ul.user_id = ?", doer.ID). + And("l.is_archived = false"). GroupBy("l.id"). Find(&individualLists) if err != nil { @@ -272,6 +293,7 @@ func (n *Namespace) ReadAll(a web.Auth, search string, page int, perPage int) (r Where("team_members.user_id = ?", doer.ID). Or("namespaces.owner_id = ?", doer.ID). Or("users_namespace.user_id = ?", doer.ID). + And("namespaces.is_archived = false"). GroupBy("namespaces.id"). Where("namespaces.name LIKE ?", "%"+search+"%"). Count(&NamespaceWithLists{}) @@ -400,12 +422,19 @@ func (n *Namespace) Update() (err error) { return } + // Check if the namespace is archived and the update is not un-archiving it + if currentNamespace.IsArchived && n.IsArchived { + return ErrNamespaceIsArchived{NamespaceID: n.ID} + } + // Check if the (new) owner exists - n.OwnerID = n.Owner.ID - if currentNamespace.OwnerID != n.OwnerID { - n.Owner, err = user.GetUserByID(n.OwnerID) - if err != nil { - return + if n.Owner != nil { + n.OwnerID = n.Owner.ID + if currentNamespace.OwnerID != n.OwnerID { + n.Owner, err = user.GetUserByID(n.OwnerID) + if err != nil { + return + } } } diff --git a/pkg/models/namespace_rights.go b/pkg/models/namespace_rights.go index e4a518f6..e38a204a 100644 --- a/pkg/models/namespace_rights.go +++ b/pkg/models/namespace_rights.go @@ -64,7 +64,8 @@ func (n *Namespace) checkRight(a web.Auth, rights ...Right) (bool, error) { } // Get the namespace and check the right - err := n.GetSimpleByID() + nn := &Namespace{ID: n.ID} + err := nn.GetSimpleByID() if err != nil { return false, err } diff --git a/pkg/models/task_collection.go b/pkg/models/task_collection.go index 30955053..69ba194b 100644 --- a/pkg/models/task_collection.go +++ b/pkg/models/task_collection.go @@ -109,7 +109,10 @@ func (tf *TaskCollection) ReadAll(a web.Auth, search string, page int, perPage i // If the list ID is not set, we get all tasks for the user. // This allows to use this function in Task.ReadAll with a possibility to deprecate the latter at some point. if tf.ListID == 0 { - tf.Lists, _, _, err = getRawListsForUser("", &user.User{ID: a.GetID()}, -1, 0) + tf.Lists, _, _, err = getRawListsForUser(&listOptions{ + user: &user.User{ID: a.GetID()}, + page: -1, + }) if err != nil { return nil, 0, 0, err }