From f7a06e4644eedab3df8689c384fb823dd3bbc817 Mon Sep 17 00:00:00 2001 From: shilch Date: Sun, 23 Jan 2022 12:59:43 +0000 Subject: [PATCH] Enable a list to be moved across namespaces (#1096) Co-authored-by: Simon Hilchenbach Reviewed-on: https://kolaente.dev/vikunja/api/pulls/1096 Reviewed-by: konrad Co-authored-by: shilch Co-committed-by: shilch --- .vscode/settings.json | 5 +++ docs/content/doc/development/mage.md | 4 +- magefile.go | 3 +- pkg/models/list.go | 1 + pkg/models/list_rights.go | 19 +++++++++ pkg/models/list_test.go | 59 ++++++++++++++++++++++++++++ pkg/swagger/docs.go | 2 +- 7 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 000000000..d4aee1e68 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "go.testEnvVars": { + "VIKUNJA_SERVICE_ROOTPATH": "${workspaceRoot}" + } +} \ No newline at end of file diff --git a/docs/content/doc/development/mage.md b/docs/content/doc/development/mage.md index d2d309f56..ab059b131 100644 --- a/docs/content/doc/development/mage.md +++ b/docs/content/doc/development/mage.md @@ -11,9 +11,9 @@ menu: # Mage Vikunja uses [Mage](https://magefile.org/) to script common development tasks and even releasing. -Mage is a pure go solution which allows for greater flexibility and things like better paralelization. +Mage is a pure go solution which allows for greater flexibility and things like better parallelization. -This document explains what taks are available and what they do. +This document explains what tasks are available and what they do. {{< table_of_contents >}} diff --git a/magefile.go b/magefile.go index 1924e278e..026425473 100644 --- a/magefile.go +++ b/magefile.go @@ -14,6 +14,7 @@ // You should have received a copy of the GNU Affero General Public Licensee // along with this program. If not, see . +//go:build mage // +build mage package main @@ -349,7 +350,7 @@ func (Test) Unit() { mg.Deps(initVars) setApiPackages() // We run everything sequentially and not in parallel to prevent issues with real test databases - args := append([]string{"test", Goflags[0], "-p", "1", "-timeout", "20m"}, ApiPackages...) + args := append([]string{"test", Goflags[0], "-p", "1", "-coverprofile", "cover.out", "-timeout", "20m"}, ApiPackages...) runAndStreamOutput("go", args...) } diff --git a/pkg/models/list.go b/pkg/models/list.go index 6c0230fc7..513f388b2 100644 --- a/pkg/models/list.go +++ b/pkg/models/list.go @@ -630,6 +630,7 @@ func UpdateList(s *xorm.Session, list *List, auth web.Auth, updateListBackground "is_archived", "identifier", "hex_color", + "namespace_id", "position", } if list.Description != "" { diff --git a/pkg/models/list_rights.go b/pkg/models/list_rights.go index 5b4144f77..adbd48450 100644 --- a/pkg/models/list_rights.go +++ b/pkg/models/list_rights.go @@ -116,6 +116,25 @@ func (l *List) CanUpdate(s *xorm.Session, a web.Auth) (canUpdate bool, err error return false, nil } + // Get the list + ol, err := GetListSimpleByID(s, l.ID) + if err != nil { + return false, err + } + + // Check if we're moving the list into a different namespace. + // If that is the case, we need to verify permissions to do so. + if l.NamespaceID != 0 && l.NamespaceID != ol.NamespaceID { + newNamespace := &Namespace{ID: l.NamespaceID} + can, err := newNamespace.CanWrite(s, a) + if err != nil { + return false, err + } + if !can { + return false, ErrGenericForbidden{} + } + } + fid := getSavedFilterIDFromListID(l.ID) if fid > 0 { sf, err := getSavedFilterSimpleByID(s, fid) diff --git a/pkg/models/list_test.go b/pkg/models/list_test.go index 0fde472a9..843aad961 100644 --- a/pkg/models/list_test.go +++ b/pkg/models/list_test.go @@ -163,6 +163,65 @@ func TestList_CreateOrUpdate(t *testing.T) { assert.True(t, IsErrListIdentifierIsNotUnique(err)) _ = s.Close() }) + t.Run("change namespace", func(t *testing.T) { + t.Run("own", func(t *testing.T) { + usr := &user.User{ + ID: 6, + Username: "user6", + Email: "user6@example.com", + } + + db.LoadAndAssertFixtures(t) + s := db.NewSession() + list := List{ + ID: 6, + Title: "Test6", + Description: "Lorem Ipsum", + NamespaceID: 7, // from 6 + } + can, err := list.CanUpdate(s, usr) + assert.NoError(t, err) + assert.True(t, can) + err = list.Update(s, usr) + assert.NoError(t, err) + err = s.Commit() + assert.NoError(t, err) + db.AssertExists(t, "lists", map[string]interface{}{ + "id": list.ID, + "title": list.Title, + "description": list.Description, + "namespace_id": list.NamespaceID, + }, false) + }) + // FIXME: The check for whether the namespace is archived is missing in namespace.CanWrite + // t.Run("archived own", func(t *testing.T) { + // db.LoadAndAssertFixtures(t) + // s := db.NewSession() + // list := List{ + // ID: 1, + // Title: "Test1", + // Description: "Lorem Ipsum", + // NamespaceID: 16, // from 1 + // } + // can, err := list.CanUpdate(s, usr) + // assert.NoError(t, err) + // assert.False(t, can) // namespace is archived and thus not writeable + // _ = s.Close() + // }) + t.Run("others", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + list := List{ + ID: 1, + Title: "Test1", + Description: "Lorem Ipsum", + NamespaceID: 2, // from 1 + } + can, _ := list.CanUpdate(s, usr) + assert.False(t, can) // namespace is not writeable by us + _ = s.Close() + }) + }) }) } diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index 08487fd0a..35eb0dad3 100644 --- a/pkg/swagger/docs.go +++ b/pkg/swagger/docs.go @@ -9215,5 +9215,5 @@ func (s *s) ReadDoc() string { } func init() { - swag.Register(swag.Name, &s{}) + swag.Register("swagger", &s{}) }