ami/tl-vikunja
1
0
Fork 0
Code Activity

Add support for archiving lists and namespaces (#152)

Browse Source 
Add query param to get all lists including archived ones

Add query param to get all namespaces including archived ones

Fix getting lists by namespace only not archived lists

Fix misspell

Fix lint

Merge branch 'master' into feature/archive-lists-namespaces

Add docs for error codes

Fix archive error codes

Don't let archived lists show up in general lists

Fix updating description

Fix updating lists with link shares

More comments

Fix un-archiving lists

Move check for archiving a list to canWrite Check

Add more tests

Add more checks

Add checks for namespaces and lists

Add namespace edit

Add tests

Add migrations and filter

Add basic tests

Add is archived property to lists and namespaces

Co-authored-by: kolaente <k@knt.li>
Reviewed-on: https://kolaente.dev/vikunja/api/pulls/152
This commit is contained in:
konrad
2020-03-15 21:50:39 +00:00
parent 4472020ee9
commit 5126330a10
20 changed files with 654 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
pkg/models/error.go
View File

@ -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
// ============

5
pkg/models/label.go
View File

@ -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
}

110
pkg/models/list.go
View File

@ -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

29
pkg/models/list_rights.go
View File

@ -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

39
pkg/models/namespace.go
View File

@ -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
}
}
}

3
pkg/models/namespace_rights.go
View File

@ -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
}

5
pkg/models/task_collection.go
View File

@ -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
}