From d39007baa048578d871e6a6d99810d418ac8d527 Mon Sep 17 00:00:00 2001 From: konrad Date: Sat, 29 Dec 2018 14:29:50 +0000 Subject: [PATCH] Task assignees (#44) --- Featurecreep.md | 2 +- REST-Tests/auth.http | 2 +- REST-Tests/lists.http | 25 +++--- REST-Tests/users.http | 2 +- docs/docs.go | 14 +++- docs/swagger/swagger.json | 12 +++ docs/swagger/swagger.yaml | 8 ++ pkg/models/bulk_list_task.go | 5 ++ pkg/models/list_rights.go | 7 +- pkg/models/list_tasks.go | 57 ++++++++++++- pkg/models/list_tasks_create_update.go | 108 +++++++++++++++++++++---- pkg/models/list_tasks_delete.go | 11 ++- pkg/models/list_tasks_rights.go | 12 +-- pkg/models/main_test.go | 16 ---- pkg/models/models.go | 2 + 15 files changed, 226 insertions(+), 57 deletions(-) diff --git a/Featurecreep.md b/Featurecreep.md index 2f8137679d..32c3425706 100644 --- a/Featurecreep.md +++ b/Featurecreep.md @@ -99,8 +99,8 @@ Sorry for some of them being in German, I'll tranlate them at some point. * [x] Timeline/Calendar view -> Dazu tasks die in einem Bestimmten Bereich due sind, macht dann das Frontend * [x] Tasks innerhalb eines definierbarem Bereich, sollte aber trotzdem der server machen, so à la "Gib mir alles für diesen Monat" * [x] Bulk-edit -> Transactions +* [x] Assignees * [ ] Labels -* [ ] Assignees * [ ] Attachments * [ ] Task-Templates innerhalb namespaces und Listen (-> Mehrere, die auswählbar sind) * [ ] Ein Task muss von mehreren Assignees abgehakt werden bis er als done markiert wird diff --git a/REST-Tests/auth.http b/REST-Tests/auth.http index 5c7e342816..33c7c7a676 100644 --- a/REST-Tests/auth.http +++ b/REST-Tests/auth.http @@ -15,7 +15,7 @@ POST http://localhost:8080/api/v1/register Content-Type: application/json { - "username": "user5", + "username": "user", "password": "1234", "email": "5@knt.li" } diff --git a/REST-Tests/lists.http b/REST-Tests/lists.http index 7c05e80e71..ead1c66f2f 100644 --- a/REST-Tests/lists.http +++ b/REST-Tests/lists.http @@ -1,35 +1,34 @@ # Get all lists -GET http://localhost:8080/api/v1/namespaces/1/lists +GET http://localhost:8080/api/v1/namespaces/1 Authorization: Bearer {{auth_token}} ### # Get one list -GET http://localhost:8080/api/v1/lists/1163 +GET http://localhost:8080/api/v1/lists/1172 Authorization: Bearer {{auth_token}} ### # Add a new list -PUT http://localhost:8080/api/v1/namespaces/6/lists +PUT http://localhost:8080/api/v1/namespaces/1/lists Authorization: Bearer {{auth_token}} Content-Type: application/json { - "title": "sffffc me only" + "title": "test" } ### # Add a new item -PUT http://localhost:8080/api/v1/lists/15 +PUT http://localhost:8080/api/v1/lists/1 Authorization: Bearer {{auth_token}} Content-Type: application/json { - "text": "this is a subtask 2", - "description": "Schinken", - "parentTaskID": 34 + "text": "Task", + "description": "Schinken" } ### @@ -83,11 +82,11 @@ Authorization: Bearer {{auth_token}} ### # Give a user access to that list -PUT http://localhost:8080/api/v1/lists/30/users +PUT http://localhost:8080/api/v1/lists/1172/users Authorization: Bearer {{auth_token}} Content-Type: application/json -{"user_id":3, "right":1} +{"user_id":1, "right":1} ### @@ -131,11 +130,13 @@ GET http://localhost:8080/api/v1/tasks/caldav ### # Update a task -POST http://localhost:8080/api/v1/tasks/3491 +POST http://localhost:8080/api/v1/tasks/1 Authorization: Bearer {{auth_token}} Content-Type: application/json -{"startDate":1546804000, "endDate": 1546805000} +{"assignees":[ + {"id": 1} +]} ### diff --git a/REST-Tests/users.http b/REST-Tests/users.http index aa6be12872..e33f4a04b8 100644 --- a/REST-Tests/users.http +++ b/REST-Tests/users.http @@ -1,6 +1,6 @@ # Get all users -GET http://localhost:8080/api/v1/users +GET http://localhost:8080/api/v1/user Authorization: Bearer {{auth_token}} ###### diff --git a/docs/docs.go b/docs/docs.go index 242b238264..855150b4af 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -1,6 +1,6 @@ // GENERATED BY THE COMMAND ABOVE; DO NOT EDIT // This file was generated by swaggo/swag at -// 2018-12-28 01:18:16.824999107 +0100 CET m=+0.098072896 +// 2018-12-29 15:14:06.225275112 +0100 CET m=+0.295589005 package docs @@ -3012,6 +3012,12 @@ var doc = `{ "models.BulkTask": { "type": "object", "properties": { + "assignees": { + "type": "array", + "items": { + "$ref": "#/definitions/models.User" + } + }, "created": { "type": "integer" }, @@ -3116,6 +3122,12 @@ var doc = `{ "models.ListTask": { "type": "object", "properties": { + "assignees": { + "type": "array", + "items": { + "$ref": "#/definitions/models.User" + } + }, "created": { "type": "integer" }, diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index e2c5f8a57e..bcf7bb02ec 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -2998,6 +2998,12 @@ "models.BulkTask": { "type": "object", "properties": { + "assignees": { + "type": "array", + "items": { + "$ref": "#/definitions/models.User" + } + }, "created": { "type": "integer" }, @@ -3102,6 +3108,12 @@ "models.ListTask": { "type": "object", "properties": { + "assignees": { + "type": "array", + "items": { + "$ref": "#/definitions/models.User" + } + }, "created": { "type": "integer" }, diff --git a/docs/swagger/swagger.yaml b/docs/swagger/swagger.yaml index 872058397c..354064767c 100644 --- a/docs/swagger/swagger.yaml +++ b/docs/swagger/swagger.yaml @@ -13,6 +13,10 @@ definitions: type: object models.BulkTask: properties: + assignees: + items: + $ref: '#/definitions/models.User' + type: array created: type: integer createdBy: @@ -82,6 +86,10 @@ definitions: type: object models.ListTask: properties: + assignees: + items: + $ref: '#/definitions/models.User' + type: array created: type: integer createdBy: diff --git a/pkg/models/bulk_list_task.go b/pkg/models/bulk_list_task.go index 1d95b9ba52..9f31b47d0d 100644 --- a/pkg/models/bulk_list_task.go +++ b/pkg/models/bulk_list_task.go @@ -95,6 +95,11 @@ func (bt *BulkTask) Update() (err error) { // When a repeating task is marked as done, we update all deadlines and reminders and set it as undone updateDone(oldtask, &bt.ListTask) + // Update the assignees + if err := oldtask.updateTaskAssignees(bt.Assignees); err != nil { + return err + } + // For whatever reason, xorm dont detect if done is updated, so we need to update this every time by hand // Which is why we merge the actual task struct with the one we got from the // The user struct overrides values in the actual one. diff --git a/pkg/models/list_rights.go b/pkg/models/list_rights.go index 7bc4f92f4f..c3a2bdc83a 100644 --- a/pkg/models/list_rights.go +++ b/pkg/models/list_rights.go @@ -69,7 +69,12 @@ func (l *List) CanRead(a web.Auth) bool { return true } - return l.checkListTeamRight(user, TeamRightRead) + if l.checkListTeamRight(user, TeamRightRead) { + return true + } + + // Users who are able to write should also be able to read + return l.CanWrite(a) } // CanDelete checks if the user can delete a list diff --git a/pkg/models/list_tasks.go b/pkg/models/list_tasks.go index 37c1a25441..b52319a7b8 100644 --- a/pkg/models/list_tasks.go +++ b/pkg/models/list_tasks.go @@ -36,6 +36,7 @@ type ListTask struct { Priority int64 `xorm:"int(11)" json:"priority"` StartDateUnix int64 `xorm:"int(11) INDEX" json:"startDate"` EndDateUnix int64 `xorm:"int(11) INDEX" json:"endDate"` + Assignees []*User `xorm:"-" json:"assignees"` Sorting string `xorm:"-" json:"-" param:"sort"` // Parameter to sort by StartDateSortUnix int64 `xorm:"-" json:"-" param:"startdatefilter"` @@ -57,6 +58,25 @@ func (ListTask) TableName() string { return "tasks" } +// ListTaskAssginee represents an assignment of a user to a task +type ListTaskAssginee struct { + ID int64 `xorm:"int(11) autoincr not null unique pk"` + TaskID int64 `xorm:"int(11) not null"` + UserID int64 `xorm:"int(11) not null"` + Created int64 `xorm:"created"` +} + +// TableName makes a pretty table name +func (ListTaskAssginee) TableName() string { + return "task_assignees" +} + +// ListTaskAssigneeWithUser is a helper type to deal with user joins +type ListTaskAssigneeWithUser struct { + TaskID int64 + User `xorm:"extends"` +} + // GetTasksByListID gets all todotasks for a list func GetTasksByListID(listID int64) (tasks []*ListTask, err error) { err = x.Where("list_id = ?", listID).Find(&tasks) @@ -72,9 +92,11 @@ func GetTasksByListID(listID int64) (tasks []*ListTask, err error) { // make a map so we can put in subtasks more easily taskMap := make(map[int64]*ListTask, len(tasks)) - // Get all users and put them into the array + // Get all users & task ids and put them into the array var userIDs []int64 + var taskIDs []int64 for _, i := range tasks { + taskIDs = append(taskIDs, i.ID) found := false for _, u := range userIDs { if i.CreatedByID == u { @@ -90,6 +112,18 @@ func GetTasksByListID(listID int64) (tasks []*ListTask, err error) { taskMap[i.ID] = i } + // Get all assignees + taskAssignees, err := getRawTaskAssigneesForTasks(taskIDs) + if err != nil { + return + } + // Put the assignees in the task map + for _, a := range taskAssignees { + if a != nil { + taskMap[a.TaskID].Assignees = append(taskMap[a.TaskID].Assignees, &a.User) + } + } + var users []User err = x.In("id", userIDs).Find(&users) if err != nil { @@ -130,6 +164,16 @@ func GetTasksByListID(listID int64) (tasks []*ListTask, err error) { return } +func getRawTaskAssigneesForTasks(taskIDs []int64) (taskAssignees []*ListTaskAssigneeWithUser, err error) { + taskAssignees = []*ListTaskAssigneeWithUser{nil} + err = x.Table("task_assignees"). + Select("task_id, users.*"). + In("task_id", taskIDs). + Join("INNER", "users", "task_assignees.user_id = users.id"). + Find(&taskAssignees) + return +} + // GetListTaskByID returns all tasks a list has func GetListTaskByID(listTaskID int64) (listTask ListTask, err error) { if listTaskID < 1 { @@ -151,6 +195,17 @@ func GetListTaskByID(listTaskID int64) (listTask ListTask, err error) { } listTask.CreatedBy = u + // Get assignees + taskAssignees, err := getRawTaskAssigneesForTasks([]int64{listTaskID}) + if err != nil { + return + } + for _, u := range taskAssignees { + if u != nil { + listTask.Assignees = append(listTask.Assignees, &u.User) + } + } + return } diff --git a/pkg/models/list_tasks_create_update.go b/pkg/models/list_tasks_create_update.go index 459b478959..7a7a053a3c 100644 --- a/pkg/models/list_tasks_create_update.go +++ b/pkg/models/list_tasks_create_update.go @@ -36,21 +36,21 @@ import ( // @Failure 403 {object} code.vikunja.io/web.HTTPError "The user does not have access to the list" // @Failure 500 {object} models.Message "Internal error" // @Router /lists/{id} [put] -func (i *ListTask) Create(a web.Auth) (err error) { +func (t *ListTask) Create(a web.Auth) (err error) { doer, err := getUserWithError(a) if err != nil { return err } - i.ID = 0 + t.ID = 0 // Check if we have at least a text - if i.Text == "" { + if t.Text == "" { return ErrListTaskCannotBeEmpty{} } // Check if the list exists - l := &List{ID: i.ListID} + l := &List{ID: t.ListID} if err = l.GetSimpleByID(); err != nil { return } @@ -60,9 +60,14 @@ func (i *ListTask) Create(a web.Auth) (err error) { return err } - i.CreatedByID = u.ID - i.CreatedBy = u - if _, err = x.Insert(i); err != nil { + t.CreatedByID = u.ID + t.CreatedBy = u + if _, err = x.Insert(t); err != nil { + return err + } + + // Update the assignees + if err := t.updateTaskAssignees(t.Assignees); err != nil { return err } @@ -84,29 +89,34 @@ func (i *ListTask) Create(a web.Auth) (err error) { // @Failure 403 {object} code.vikunja.io/web.HTTPError "The user does not have access to the task (aka its list)" // @Failure 500 {object} models.Message "Internal error" // @Router /tasks/{id} [post] -func (i *ListTask) Update() (err error) { +func (t *ListTask) Update() (err error) { // Check if the task exists - ot, err := GetListTaskByID(i.ID) + ot, err := GetListTaskByID(t.ID) if err != nil { return } // When a repeating task is marked as done, we update all deadlines and reminders and set it as undone - updateDone(&ot, i) + updateDone(&ot, t) + + // Update the assignees + if err := ot.updateTaskAssignees(t.Assignees); err != nil { + return err + } // For whatever reason, xorm dont detect if done is updated, so we need to update this every time by hand // Which is why we merge the actual task struct with the one we got from the // The user struct overrides values in the actual one. - if err := mergo.Merge(&ot, i, mergo.WithOverride); err != nil { + if err := mergo.Merge(&ot, t, mergo.WithOverride); err != nil { return err } // And because a false is considered to be a null value, we need to explicitly check that case here. - if i.Done == false { + if t.Done == false { ot.Done = false } - _, err = x.ID(i.ID). + _, err = x.ID(t.ID). Cols("text", "description", "done", @@ -118,7 +128,7 @@ func (i *ListTask) Update() (err error) { "start_date_unix", "end_date_unix"). Update(ot) - *i = ot + *t = ot return } @@ -133,3 +143,73 @@ func updateDone(oldTask *ListTask, newTask *ListTask) { newTask.Done = false } } + +// Create a bunch of task assignees +func (t *ListTask) updateTaskAssignees(assignees []*User) (err error) { + + // Get old assignees to delete + var found bool + var assigneesToDelete []int64 + for _, oldAssignee := range t.Assignees { + found = false + for _, newAssignee := range assignees { + if newAssignee.ID == oldAssignee.ID { + found = true // If a new assignee is already in the list with old assignees + break + } + } + + // Put all assignees which are only on the old list to the trash + if !found { + assigneesToDelete = append(assigneesToDelete, oldAssignee.ID) + } + } + + // Delete all assignees not passed + if len(assigneesToDelete) > 0 { + _, err = x.In("user_id", assigneesToDelete). + And("task_id = ?", t.ID). + Delete(ListTaskAssginee{}) + if err != nil { + return err + } + } + + // Get the list to perform later checks + list := List{ID: t.ListID} + err = list.ReadOne() + if err != nil { + return + } + + // Loop through our users and add them +AddNewAssignee: + for _, u := range assignees { + // Check if the user is already assigned and assign him only if not + for _, oldAssignee := range t.Assignees { + if oldAssignee.ID == u.ID { + // continue outer loop + continue AddNewAssignee + } + } + + // Check if the user exists and has access to the list + newAssignee, err := GetUserByID(u.ID) + if err != nil { + return err + } + if !list.CanRead(&newAssignee) { + return ErrUserDoesNotHaveAccessToList{list.ID, u.ID} + } + + _, err = x.Insert(ListTaskAssginee{ + TaskID: t.ID, + UserID: u.ID, + }) + if err != nil { + return err + } + } + + return +} diff --git a/pkg/models/list_tasks_delete.go b/pkg/models/list_tasks_delete.go index 5cc52d2c17..34958f3a30 100644 --- a/pkg/models/list_tasks_delete.go +++ b/pkg/models/list_tasks_delete.go @@ -33,15 +33,20 @@ import ( // @Failure 403 {object} code.vikunja.io/web.HTTPError "The user does not have access to the list" // @Failure 500 {object} models.Message "Internal error" // @Router /tasks/{id} [delete] -func (i *ListTask) Delete() (err error) { +func (t *ListTask) Delete() (err error) { // Check if it exists - _, err = GetListTaskByID(i.ID) + _, err = GetListTaskByID(t.ID) if err != nil { return } - if _, err = x.ID(i.ID).Delete(ListTask{}); err != nil { + if _, err = x.ID(t.ID).Delete(ListTask{}); err != nil { + return err + } + + // Delete assignees + if _, err = x.Where("task_id = ?", t.ID).Delete(ListTaskAssginee{}); err != nil { return err } diff --git a/pkg/models/list_tasks_rights.go b/pkg/models/list_tasks_rights.go index ef42c1b8a1..2b6f9d972d 100644 --- a/pkg/models/list_tasks_rights.go +++ b/pkg/models/list_tasks_rights.go @@ -22,11 +22,11 @@ import ( ) // CanDelete checks if the user can delete an task -func (i *ListTask) CanDelete(a web.Auth) bool { +func (t *ListTask) CanDelete(a web.Auth) bool { doer := getUserForRights(a) // Get the task - lI, err := GetListTaskByID(i.ID) + lI, err := GetListTaskByID(t.ID) if err != nil { log.Log.Error("Error occurred during CanDelete for ListTask: %s", err) return false @@ -39,11 +39,11 @@ func (i *ListTask) CanDelete(a web.Auth) bool { } // CanUpdate determines if a user has the right to update a list task -func (i *ListTask) CanUpdate(a web.Auth) bool { +func (t *ListTask) CanUpdate(a web.Auth) bool { doer := getUserForRights(a) // Get the task - lI, err := GetListTaskByID(i.ID) + lI, err := GetListTaskByID(t.ID) if err != nil { log.Log.Error("Error occurred during CanDelete for ListTask: %s", err) return false @@ -56,11 +56,11 @@ func (i *ListTask) CanUpdate(a web.Auth) bool { } // CanCreate determines if a user has the right to create a list task -func (i *ListTask) CanCreate(a web.Auth) bool { +func (t *ListTask) CanCreate(a web.Auth) bool { doer := getUserForRights(a) // A user can create an task if he has write acces to its list - l := &List{ID: i.ListID} + l := &List{ID: t.ListID} l.ReadOne() return l.CanWrite(doer) } diff --git a/pkg/models/main_test.go b/pkg/models/main_test.go index 188824bd0e..3fcddf3428 100644 --- a/pkg/models/main_test.go +++ b/pkg/models/main_test.go @@ -14,22 +14,6 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -// Vikunja is a todo-list application to facilitate your life. -// Copyright 2018 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 models import ( diff --git a/pkg/models/models.go b/pkg/models/models.go index 433e119df9..5b95173c41 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -68,6 +68,7 @@ func init() { new(Namespace), new(ListUser), new(NamespaceUser), + new(ListTaskAssginee), ) tablesWithPointer = append(tables, @@ -81,6 +82,7 @@ func init() { &Namespace{}, &ListUser{}, &NamespaceUser{}, + &ListTaskAssginee{}, ) }