From 87e43b469521c47891e2bb63eb8073bcb5babe5a Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 28 Dec 2018 00:39:06 +0100 Subject: [PATCH 1/7] Added bulk edit mode functions --- Featurecreep.md | 2 +- pkg/models/bulk_list_task.go | 121 +++++++++++++++++++++++++ pkg/models/error.go | 45 +++++++++ pkg/models/list_tasks.go | 43 +++++++++ pkg/models/list_tasks_create_update.go | 24 +++-- 5 files changed, 224 insertions(+), 11 deletions(-) create mode 100644 pkg/models/bulk_list_task.go diff --git a/Featurecreep.md b/Featurecreep.md index f32b3e379..dbe788762 100644 --- a/Featurecreep.md +++ b/Featurecreep.md @@ -98,9 +98,9 @@ Sorry for some of them being in German, I'll tranlate them at some point. * [x] Start/Enddatum für Tasks * [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 * [ ] Labels * [ ] Assignees -* [ ] Bulk-edit -> Transactions * [ ] 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/pkg/models/bulk_list_task.go b/pkg/models/bulk_list_task.go new file mode 100644 index 000000000..67654a7ec --- /dev/null +++ b/pkg/models/bulk_list_task.go @@ -0,0 +1,121 @@ +// 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 ( + "code.vikunja.io/api/pkg/log" + "code.vikunja.io/web" + "github.com/imdario/mergo" +) + +// BulkTask is the definition of a bulk update task +type BulkTask struct { + IDs []int64 `json:"task_ids"` + Tasks []*ListTask `json:"-"` + ListTask +} + +func (bt *BulkTask) checkIfTasksAreOnTheSameList() (err error) { + // Get the tasks + err = bt.GetTasksByIDs() + if err != nil { + return err + } + + if len(bt.Tasks) == 0 { + return ErrBulkTasksNeedAtLeastOne{} + } + + // Check if all tasks are in the same list + var firstListID = bt.Tasks[0].ID + for _, t := range bt.Tasks { + if t.ID != firstListID { + return ErrBulkTasksMustBeInSameList{firstListID, t.ID} + } + } + + return nil +} + +// CanUpdate checks if a user is allowed to update a task +func (bt *BulkTask) CanUpdate(a web.Auth) bool { + + err := bt.checkIfTasksAreOnTheSameList() + if err != nil { + log.Log.Error("Error occurred during CanUpdate for BulkTask: %s", err) + return false + } + + doer := getUserForRights(a) + // A user can update an task if he has write acces to its list + l := &List{ID: bt.Tasks[0].ListID} + l.ReadOne() + return l.CanWrite(doer) +} + +// Update updates a bunch of tasks at once +func (bt *BulkTask) Update() (err error) { + + sess := x.NewSession() + defer sess.Close() + + err = sess.Begin() + if err != nil { + return + } + + for _, oldtask := range bt.Tasks { + + // When a repeating task is marked as done, we update all deadlines and reminders and set it as undone + updateDone(oldtask, &bt.ListTask) + + // 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(oldtask, &bt.ListTask, 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 bt.ListTask.Done == false { + oldtask.Done = false + } + + _, err = x.ID(oldtask.ID). + Cols("text", + "description", + "done", + "due_date_unix", + "reminders_unix", + "repeat_after", + "parent_task_id", + "priority", + "start_date_unix", + "end_date_unix"). + Update(oldtask) + if err != nil { + return sess.Rollback() + } + } + + err = sess.Commit() + if err != nil { + return + } + + return +} diff --git a/pkg/models/error.go b/pkg/models/error.go index 49c86c181..a0a2040b3 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -429,6 +429,51 @@ func (err ErrListTaskDoesNotExist) HTTPError() web.HTTPError { return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeListTaskDoesNotExist, Message: "This list task does not exist"} } +// ErrBulkTasksMustBeInSameList represents a "ErrBulkTasksMustBeInSameList" kind of error. +type ErrBulkTasksMustBeInSameList struct { + ShouldBeID int64 + IsID int64 +} + +// IsErrBulkTasksMustBeInSameList checks if an error is a ErrBulkTasksMustBeInSameList. +func IsErrBulkTasksMustBeInSameList(err error) bool { + _, ok := err.(ErrBulkTasksMustBeInSameList) + return ok +} + +func (err ErrBulkTasksMustBeInSameList) Error() string { + return fmt.Sprintf("All bulk editing tasks must be in the same list. [Should be: %d, is: %d]", err.ShouldBeID, err.IsID) +} + +// ErrCodeBulkTasksMustBeInSameList holds the unique world-error code of this error +const ErrCodeBulkTasksMustBeInSameList = 4003 + +// HTTPError holds the http error description +func (err ErrBulkTasksMustBeInSameList) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrCodeBulkTasksMustBeInSameList, Message: "All tasks must be in the same list."} +} + +// ErrBulkTasksNeedAtLeastOne represents a "ErrBulkTasksNeedAtLeastOne" kind of error. +type ErrBulkTasksNeedAtLeastOne struct{} + +// IsErrBulkTasksNeedAtLeastOne checks if an error is a ErrBulkTasksNeedAtLeastOne. +func IsErrBulkTasksNeedAtLeastOne(err error) bool { + _, ok := err.(ErrBulkTasksNeedAtLeastOne) + return ok +} + +func (err ErrBulkTasksNeedAtLeastOne) Error() string { + return fmt.Sprintf("Need at least one task when bulk editing tasks") +} + +// ErrCodeBulkTasksNeedAtLeastOne holds the unique world-error code of this error +const ErrCodeBulkTasksNeedAtLeastOne = 4004 + +// HTTPError holds the http error description +func (err ErrBulkTasksNeedAtLeastOne) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrCodeBulkTasksNeedAtLeastOne, Message: "Need at least one tasks to do bulk editing."} +} + // ================= // Namespace errors // ================= diff --git a/pkg/models/list_tasks.go b/pkg/models/list_tasks.go index f973b3a64..1a1f60c1a 100644 --- a/pkg/models/list_tasks.go +++ b/pkg/models/list_tasks.go @@ -153,3 +153,46 @@ func GetListTaskByID(listTaskID int64) (listTask ListTask, err error) { return } + +// GetTasksByIDs returns all tasks for a list of ids +func (bt *BulkTask) GetTasksByIDs() (err error) { + for _, id := range bt.IDs { + if id < 1 { + return ErrListTaskDoesNotExist{id} + } + } + + err = x.In("id", bt.IDs).Find(&bt.Tasks) + if err != nil { + return err + } + + // We use a map, to avoid looping over two slices at once + var usermapids = make(map[int64]bool) // Bool ist just something, doesn't acutually matter + for _, list := range bt.Tasks { + usermapids[list.CreatedByID] = true + } + + // Make a slice from the map + var userids []int64 + for uid := range usermapids { + userids = append(userids, uid) + } + + // Get all users for the tasks + var users []*User + err = x.In("created_by_id", userids).Find(&users) + if err != nil { + return err + } + + for in, task := range bt.Tasks { + for _, u := range users { + if task.CreatedByID == u.ID { + bt.Tasks[in].CreatedBy = *u + } + } + } + + return +} diff --git a/pkg/models/list_tasks_create_update.go b/pkg/models/list_tasks_create_update.go index f3bea0a20..459b47895 100644 --- a/pkg/models/list_tasks_create_update.go +++ b/pkg/models/list_tasks_create_update.go @@ -91,16 +91,8 @@ func (i *ListTask) Update() (err error) { return } - // When a repeating task is marked, as done, we update all deadlines and reminders and set it as undone - if !ot.Done && i.Done && ot.RepeatAfter > 0 { - ot.DueDateUnix = ot.DueDateUnix + ot.RepeatAfter - - for in, r := range ot.RemindersUnix { - ot.RemindersUnix[in] = r + ot.RepeatAfter - } - - i.Done = false - } + // When a repeating task is marked as done, we update all deadlines and reminders and set it as undone + updateDone(&ot, i) // 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 @@ -129,3 +121,15 @@ func (i *ListTask) Update() (err error) { *i = ot return } + +func updateDone(oldTask *ListTask, newTask *ListTask) { + if !oldTask.Done && newTask.Done && oldTask.RepeatAfter > 0 { + oldTask.DueDateUnix = oldTask.DueDateUnix + oldTask.RepeatAfter // assuming we'll save the old task (merged) + + for in, r := range oldTask.RemindersUnix { + oldTask.RemindersUnix[in] = r + oldTask.RepeatAfter + } + + newTask.Done = false + } +} -- 2.45.1 From 5132e3d85613fc1eaab4eee2c9974003559a54e8 Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 28 Dec 2018 00:40:37 +0100 Subject: [PATCH 2/7] docs --- docs/errors.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/errors.md b/docs/errors.md index 54bccfed7..ce9ed0a39 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -21,6 +21,8 @@ This document describes the different errors Vikunja can return. | 3005 | 400 | The list title cannot be empty. | | 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. | +| 4004 | 403 | Need at least one task when bulk editing tasks. | | 5001 | 404 | The namspace does not exist. | | 5003 | 403 | The user does not have access to the specified namespace. | | 5006 | 400 | The namespace name cannot be empty. | -- 2.45.1 From ee8721e7255a37ab7637689b118a4821709822b4 Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 28 Dec 2018 00:54:27 +0100 Subject: [PATCH 3/7] Fixed comparison of list ids --- REST-Tests/lists.http | 16 ++++++++++++++-- pkg/models/bulk_list_task.go | 8 ++++---- pkg/models/list_tasks.go | 2 +- pkg/routes/routes.go | 7 +++++++ 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/REST-Tests/lists.http b/REST-Tests/lists.http index b0926768e..7c05e80e7 100644 --- a/REST-Tests/lists.http +++ b/REST-Tests/lists.http @@ -5,7 +5,7 @@ Authorization: Bearer {{auth_token}} ### # Get one list -GET http://localhost:8080/api/v1/lists/15 +GET http://localhost:8080/api/v1/lists/1163 Authorization: Bearer {{auth_token}} ### @@ -137,4 +137,16 @@ Content-Type: application/json {"startDate":1546804000, "endDate": 1546805000} -### \ No newline at end of file +### + +# Bulk update multiple tasks at once +POST http://localhost:8080/api/v1/tasks/bulk +Authorization: Bearer {{auth_token}} +Content-Type: application/json + +{ + "task_ids": [3518,3519,3521], + "text":"bulkupdated" +} + +### diff --git a/pkg/models/bulk_list_task.go b/pkg/models/bulk_list_task.go index 67654a7ec..8bc9a0902 100644 --- a/pkg/models/bulk_list_task.go +++ b/pkg/models/bulk_list_task.go @@ -41,10 +41,10 @@ func (bt *BulkTask) checkIfTasksAreOnTheSameList() (err error) { } // Check if all tasks are in the same list - var firstListID = bt.Tasks[0].ID + var firstListID = bt.Tasks[0].ListID for _, t := range bt.Tasks { - if t.ID != firstListID { - return ErrBulkTasksMustBeInSameList{firstListID, t.ID} + if t.ListID != firstListID { + return ErrBulkTasksMustBeInSameList{firstListID, t.ListID} } } @@ -95,7 +95,7 @@ func (bt *BulkTask) Update() (err error) { oldtask.Done = false } - _, err = x.ID(oldtask.ID). + _, err = sess.ID(oldtask.ID). Cols("text", "description", "done", diff --git a/pkg/models/list_tasks.go b/pkg/models/list_tasks.go index 1a1f60c1a..37c1a2544 100644 --- a/pkg/models/list_tasks.go +++ b/pkg/models/list_tasks.go @@ -181,7 +181,7 @@ func (bt *BulkTask) GetTasksByIDs() (err error) { // Get all users for the tasks var users []*User - err = x.In("created_by_id", userids).Find(&users) + err = x.In("id", userids).Find(&users) if err != nil { return err } diff --git a/pkg/routes/routes.go b/pkg/routes/routes.go index dc42536d3..89b51d9ab 100644 --- a/pkg/routes/routes.go +++ b/pkg/routes/routes.go @@ -222,6 +222,13 @@ func RegisterRoutes(e *echo.Echo) { a.DELETE("/tasks/:listtask", taskHandler.DeleteWeb) a.POST("/tasks/:listtask", taskHandler.UpdateWeb) + bulkTaskHandler := &handler.WebHandler{ + EmptyStruct: func() handler.CObject { + return &models.BulkTask{} + }, + } + a.POST("/tasks/bulk", bulkTaskHandler.UpdateWeb) + listTeamHandler := &handler.WebHandler{ EmptyStruct: func() handler.CObject { return &models.TeamList{} -- 2.45.1 From e315f6f704d01e07031e648ca4d266b37c6c1b41 Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 28 Dec 2018 01:04:07 +0100 Subject: [PATCH 4/7] Added blank test --- go.mod | 1 + go.sum | 1 + pkg/models/bulk_list_task_test.go | 65 +++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 pkg/models/bulk_list_task_test.go diff --git a/go.mod b/go.mod index 8d8ada0f8..5956a2381 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf github.com/client9/misspell v0.3.4 + github.com/cweill/gotests v1.5.2 // indirect github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/fzipp/gocyclo v0.0.0-20150627053110-6acd4345c835 github.com/garyburd/redigo v1.6.0 // indirect diff --git a/go.sum b/go.sum index 627f40144..22826341d 100644 --- a/go.sum +++ b/go.sum @@ -18,6 +18,7 @@ github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24 github.com/client9/misspell v0.3.4 h1:ta993UF76GwbvJcIo3Y68y/M3WxlpEHPWIGDkJYwzJI= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cweill/gotests v1.5.2 h1:kKqmKmS2wCV3tuLnfpbiuN8OlkosQZTpCfiqmiuNAsA= +github.com/cweill/gotests v1.5.2/go.mod h1:XZYOJkGVkCRoymaIzmp9Wyi3rUgfA3oOnkuljYrjFV8= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/denisenkom/go-mssqldb v0.0.0-20181014144952-4e0d7dc8888f h1:WH0w/R4Yoey+04HhFxqZ6VX6I0d7RMyw5aXQ9UTvQPs= diff --git a/pkg/models/bulk_list_task_test.go b/pkg/models/bulk_list_task_test.go new file mode 100644 index 000000000..065ecfb4b --- /dev/null +++ b/pkg/models/bulk_list_task_test.go @@ -0,0 +1,65 @@ +package models + +import ( + "testing" + + "code.vikunja.io/web" +) + +func TestBulkTask_CanUpdate(t *testing.T) { + type fields struct { + IDs []int64 + Tasks []*ListTask + ListTask ListTask + } + type args struct { + a web.Auth + } + tests := []struct { + name string + fields fields + args args + want bool + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bt := &BulkTask{ + IDs: tt.fields.IDs, + Tasks: tt.fields.Tasks, + ListTask: tt.fields.ListTask, + } + if got := bt.CanUpdate(tt.args.a); got != tt.want { + t.Errorf("BulkTask.CanUpdate() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestBulkTask_Update(t *testing.T) { + type fields struct { + IDs []int64 + Tasks []*ListTask + ListTask ListTask + } + tests := []struct { + name string + fields fields + wantErr bool + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bt := &BulkTask{ + IDs: tt.fields.IDs, + Tasks: tt.fields.Tasks, + ListTask: tt.fields.ListTask, + } + if err := bt.Update(); (err != nil) != tt.wantErr { + t.Errorf("BulkTask.Update() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} -- 2.45.1 From 4342851ff668e0d20b4d7c5d42dea4b63eda7c2e Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 28 Dec 2018 01:19:10 +0100 Subject: [PATCH 5/7] Added swagger docs --- docs/docs.go | 130 ++++++++++++++++++++++++++++++++++- docs/swagger/swagger.json | 128 ++++++++++++++++++++++++++++++++++ docs/swagger/swagger.yaml | 88 ++++++++++++++++++++++++ pkg/models/bulk_list_task.go | 12 ++++ 4 files changed, 357 insertions(+), 1 deletion(-) diff --git a/docs/docs.go b/docs/docs.go index 7bdde0a75..242b23826 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-25 21:44:18.815676554 +0100 CET m=+0.161606284 +// 2018-12-28 01:18:16.824999107 +0100 CET m=+0.098072896 package docs @@ -2133,6 +2133,68 @@ var doc = `{ } } }, + "/tasks/bulk": { + "post": { + "security": [ + { + "ApiKeyAuth": [] + } + ], + "description": "Updates a bunch of tasks at once. This includes marking them as done. Note: although you could supply another ID, it will be ignored. Use task_ids instead.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "task" + ], + "summary": "Update a bunch of tasks at once", + "parameters": [ + { + "description": "The task object. Looks like a normal task, the only difference is it uses an array of list_ids to update.", + "name": "task", + "in": "body", + "required": true, + "schema": { + "type": "object", + "$ref": "#/definitions/models.BulkTask" + } + } + ], + "responses": { + "200": { + "description": "The updated task object.", + "schema": { + "type": "object", + "$ref": "#/definitions/models.ListTask" + } + }, + "400": { + "description": "Invalid task object provided.", + "schema": { + "type": "object", + "$ref": "#/definitions/code.vikunja.io.web.HTTPError" + } + }, + "403": { + "description": "The user does not have access to the task (aka its list)", + "schema": { + "type": "object", + "$ref": "#/definitions/code.vikunja.io.web.HTTPError" + } + }, + "500": { + "description": "Internal error", + "schema": { + "type": "object", + "$ref": "#/definitions/models.Message" + } + } + } + } + }, "/tasks/caldav": { "get": { "security": [ @@ -2947,6 +3009,72 @@ var doc = `{ } } }, + "models.BulkTask": { + "type": "object", + "properties": { + "created": { + "type": "integer" + }, + "createdBy": { + "type": "object", + "$ref": "#/definitions/models.User" + }, + "description": { + "type": "string" + }, + "done": { + "type": "boolean" + }, + "dueDate": { + "type": "integer" + }, + "endDate": { + "type": "integer" + }, + "id": { + "type": "integer" + }, + "listID": { + "type": "integer" + }, + "parentTaskID": { + "type": "integer" + }, + "priority": { + "type": "integer" + }, + "reminderDates": { + "type": "array", + "items": { + "type": "integer" + } + }, + "repeatAfter": { + "type": "integer" + }, + "startDate": { + "type": "integer" + }, + "subtasks": { + "type": "array", + "items": { + "$ref": "#/definitions/models.ListTask" + } + }, + "task_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "text": { + "type": "string" + }, + "updated": { + "type": "integer" + } + } + }, "models.EmailConfirm": { "type": "object", "properties": { diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index f840769ac..e2c5f8a57 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -2120,6 +2120,68 @@ } } }, + "/tasks/bulk": { + "post": { + "security": [ + { + "ApiKeyAuth": [] + } + ], + "description": "Updates a bunch of tasks at once. This includes marking them as done. Note: although you could supply another ID, it will be ignored. Use task_ids instead.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "task" + ], + "summary": "Update a bunch of tasks at once", + "parameters": [ + { + "description": "The task object. Looks like a normal task, the only difference is it uses an array of list_ids to update.", + "name": "task", + "in": "body", + "required": true, + "schema": { + "type": "object", + "$ref": "#/definitions/models.BulkTask" + } + } + ], + "responses": { + "200": { + "description": "The updated task object.", + "schema": { + "type": "object", + "$ref": "#/definitions/models.ListTask" + } + }, + "400": { + "description": "Invalid task object provided.", + "schema": { + "type": "object", + "$ref": "#/definitions/code.vikunja.io/web.HTTPError" + } + }, + "403": { + "description": "The user does not have access to the task (aka its list)", + "schema": { + "type": "object", + "$ref": "#/definitions/code.vikunja.io/web.HTTPError" + } + }, + "500": { + "description": "Internal error", + "schema": { + "type": "object", + "$ref": "#/definitions/models.Message" + } + } + } + } + }, "/tasks/caldav": { "get": { "security": [ @@ -2933,6 +2995,72 @@ } } }, + "models.BulkTask": { + "type": "object", + "properties": { + "created": { + "type": "integer" + }, + "createdBy": { + "type": "object", + "$ref": "#/definitions/models.User" + }, + "description": { + "type": "string" + }, + "done": { + "type": "boolean" + }, + "dueDate": { + "type": "integer" + }, + "endDate": { + "type": "integer" + }, + "id": { + "type": "integer" + }, + "listID": { + "type": "integer" + }, + "parentTaskID": { + "type": "integer" + }, + "priority": { + "type": "integer" + }, + "reminderDates": { + "type": "array", + "items": { + "type": "integer" + } + }, + "repeatAfter": { + "type": "integer" + }, + "startDate": { + "type": "integer" + }, + "subtasks": { + "type": "array", + "items": { + "$ref": "#/definitions/models.ListTask" + } + }, + "task_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "text": { + "type": "string" + }, + "updated": { + "type": "integer" + } + } + }, "models.EmailConfirm": { "type": "object", "properties": { diff --git a/docs/swagger/swagger.yaml b/docs/swagger/swagger.yaml index a4fe31bab..872058397 100644 --- a/docs/swagger/swagger.yaml +++ b/docs/swagger/swagger.yaml @@ -11,6 +11,50 @@ definitions: username: type: string type: object + models.BulkTask: + properties: + created: + type: integer + createdBy: + $ref: '#/definitions/models.User' + type: object + description: + type: string + done: + type: boolean + dueDate: + type: integer + endDate: + type: integer + id: + type: integer + listID: + type: integer + parentTaskID: + type: integer + priority: + type: integer + reminderDates: + items: + type: integer + type: array + repeatAfter: + type: integer + startDate: + type: integer + subtasks: + items: + $ref: '#/definitions/models.ListTask' + type: array + task_ids: + items: + type: integer + type: array + text: + type: string + updated: + type: integer + type: object models.EmailConfirm: properties: token: @@ -1822,6 +1866,50 @@ paths: summary: Get tasks sorted and within a date range tags: - task + /tasks/bulk: + post: + consumes: + - application/json + description: 'Updates a bunch of tasks at once. This includes marking them as + done. Note: although you could supply another ID, it will be ignored. Use + task_ids instead.' + parameters: + - description: The task object. Looks like a normal task, the only difference + is it uses an array of list_ids to update. + in: body + name: task + required: true + schema: + $ref: '#/definitions/models.BulkTask' + type: object + produces: + - application/json + responses: + "200": + description: The updated task object. + schema: + $ref: '#/definitions/models.ListTask' + type: object + "400": + description: Invalid task object provided. + schema: + $ref: '#/definitions/code.vikunja.io/web.HTTPError' + type: object + "403": + description: The user does not have access to the task (aka its list) + schema: + $ref: '#/definitions/code.vikunja.io/web.HTTPError' + type: object + "500": + description: Internal error + schema: + $ref: '#/definitions/models.Message' + type: object + security: + - ApiKeyAuth: [] + summary: Update a bunch of tasks at once + tags: + - task /tasks/caldav: get: description: Returns a calDAV-parsable format with all tasks as calendar events. diff --git a/pkg/models/bulk_list_task.go b/pkg/models/bulk_list_task.go index 8bc9a0902..1d95b9ba5 100644 --- a/pkg/models/bulk_list_task.go +++ b/pkg/models/bulk_list_task.go @@ -68,6 +68,18 @@ func (bt *BulkTask) CanUpdate(a web.Auth) bool { } // Update updates a bunch of tasks at once +// @Summary Update a bunch of tasks at once +// @Description Updates a bunch of tasks at once. This includes marking them as done. Note: although you could supply another ID, it will be ignored. Use task_ids instead. +// @tags task +// @Accept json +// @Produce json +// @Security ApiKeyAuth +// @Param task body models.BulkTask true "The task object. Looks like a normal task, the only difference is it uses an array of list_ids to update." +// @Success 200 {object} models.ListTask "The updated task object." +// @Failure 400 {object} code.vikunja.io/web.HTTPError "Invalid task object provided." +// @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/bulk [post] func (bt *BulkTask) Update() (err error) { sess := x.NewSession() -- 2.45.1 From a2bde3817ab0d1fc0cd2bacbe4dfbd3ea9dd91cc Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 28 Dec 2018 22:32:41 +0100 Subject: [PATCH 6/7] Added tests for bulkupdate --- pkg/models/bulk_list_task_test.go | 78 ++++++++++++++++--------------- pkg/models/fixtures/tasks.yml | 26 ++++++++++- 2 files changed, 66 insertions(+), 38 deletions(-) diff --git a/pkg/models/bulk_list_task_test.go b/pkg/models/bulk_list_task_test.go index 065ecfb4b..751b474c1 100644 --- a/pkg/models/bulk_list_task_test.go +++ b/pkg/models/bulk_list_task_test.go @@ -2,53 +2,53 @@ package models import ( "testing" - - "code.vikunja.io/web" ) -func TestBulkTask_CanUpdate(t *testing.T) { - type fields struct { - IDs []int64 - Tasks []*ListTask - ListTask ListTask - } - type args struct { - a web.Auth - } - tests := []struct { - name string - fields fields - args args - want bool - }{ - // TODO: Add test cases. - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - bt := &BulkTask{ - IDs: tt.fields.IDs, - Tasks: tt.fields.Tasks, - ListTask: tt.fields.ListTask, - } - if got := bt.CanUpdate(tt.args.a); got != tt.want { - t.Errorf("BulkTask.CanUpdate() = %v, want %v", got, tt.want) - } - }) - } -} - func TestBulkTask_Update(t *testing.T) { type fields struct { IDs []int64 Tasks []*ListTask ListTask ListTask + User *User } tests := []struct { - name string - fields fields - wantErr bool + name string + fields fields + wantErr bool + wantForbidden bool }{ - // TODO: Add test cases. + { + name: "Test normal update", + fields: fields{ + IDs: []int64{10, 11, 12}, + ListTask: ListTask{ + Text: "bulkupdated", + }, + User: &User{ID: 1}, + }, + }, + { + name: "Test with one task on different list", + fields: fields{ + IDs: []int64{10, 11, 12, 13}, + ListTask: ListTask{ + Text: "bulkupdated", + }, + User: &User{ID: 1}, + }, + wantForbidden: true, + }, + { + name: "Test without any tasks", + fields: fields{ + IDs: []int64{}, + ListTask: ListTask{ + Text: "bulkupdated", + }, + User: &User{ID: 1}, + }, + wantForbidden: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -57,6 +57,10 @@ func TestBulkTask_Update(t *testing.T) { Tasks: tt.fields.Tasks, ListTask: tt.fields.ListTask, } + allowed := bt.CanUpdate(tt.fields.User) + if !allowed != tt.wantForbidden { + t.Errorf("BulkTask.Update() want forbidden, got %v, want %v", allowed, tt.wantForbidden) + } if err := bt.Update(); (err != nil) != tt.wantErr { t.Errorf("BulkTask.Update() error = %v, wantErr %v", err, tt.wantErr) } diff --git a/pkg/models/fixtures/tasks.yml b/pkg/models/fixtures/tasks.yml index 05e54b9c8..1f5ce52ec 100644 --- a/pkg/models/fixtures/tasks.yml +++ b/pkg/models/fixtures/tasks.yml @@ -60,4 +60,28 @@ created: 1543626724 updated: 1543626724 start_date_unix: 1544600000 - end_date_unix: 1544700000 \ No newline at end of file + end_date_unix: 1544700000 +- id: 10 + text: 'task #10 basic' + created_by_id: 1 + list_id: 1 + created: 1543626724 + updated: 1543626724 +- id: 11 + text: 'task #11 basic' + created_by_id: 1 + list_id: 1 + created: 1543626724 + updated: 1543626724 +- id: 12 + text: 'task #12 basic' + created_by_id: 1 + list_id: 1 + created: 1543626724 + updated: 1543626724 +- id: 13 + text: 'task #13 basic other list' + created_by_id: 1 + list_id: 2 + created: 1543626724 + updated: 1543626724 \ No newline at end of file -- 2.45.1 From 6d34ede730de91f65c1dff6187b63b4b9bfcf95f Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 28 Dec 2018 22:44:22 +0100 Subject: [PATCH 7/7] Fix failing tests because of changed fixtures --- pkg/models/list_task_readall_test.go | 167 ++++++++++++++++++++++++++- 1 file changed, 162 insertions(+), 5 deletions(-) diff --git a/pkg/models/list_task_readall_test.go b/pkg/models/list_task_readall_test.go index 86b638b3a..75431e9bf 100644 --- a/pkg/models/list_task_readall_test.go +++ b/pkg/models/list_task_readall_test.go @@ -7,6 +7,8 @@ package models import ( + "fmt" + "github.com/stretchr/testify/assert" "reflect" "sort" "testing" @@ -97,6 +99,30 @@ func sortTasksForTesting(by SortBy) (tasks []*ListTask) { StartDateUnix: 1544600000, EndDateUnix: 1544700000, }, + { + ID: 10, + Text: "task #10 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 11, + Text: "task #11 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 12, + Text: "task #12 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, } switch by { @@ -122,6 +148,7 @@ func sortTasksForTesting(by SortBy) (tasks []*ListTask) { } func TestListTask_ReadAll(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) type fields struct { ID int64 Text string @@ -254,6 +281,30 @@ func TestListTask_ReadAll(t *testing.T) { StartDateUnix: 1544600000, EndDateUnix: 1544700000, }, + { + ID: 10, + Text: "task #10 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 11, + Text: "task #11 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 12, + Text: "task #12 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, { ID: 4, Text: "task #4 low prio", @@ -311,7 +362,113 @@ func TestListTask_ReadAll(t *testing.T) { a: &User{ID: 1}, page: 0, }, - want: sortTasksForTesting(SortTasksByDueDateAsc), + want: []*ListTask{ + { + ID: 1, + Text: "task #1", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 2, + Text: "task #2 done", + Done: true, + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 3, + Text: "task #3 high prio", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + Priority: 100, + }, + { + ID: 4, + Text: "task #4 low prio", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + Priority: 1, + }, + { + ID: 7, + Text: "task #7 with start date", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + StartDateUnix: 1544600000, + }, + { + ID: 8, + Text: "task #8 with end date", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + EndDateUnix: 1544700000, + }, + { + ID: 9, + Text: "task #9 with start and end date", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + StartDateUnix: 1544600000, + EndDateUnix: 1544700000, + }, + { + ID: 10, + Text: "task #10 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 11, + Text: "task #11 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 12, + Text: "task #12 basic", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + }, + { + ID: 6, + Text: "task #6 lower due date", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + DueDateUnix: 1543616724, + }, + { + ID: 5, + Text: "task #5 higher due date", + CreatedByID: 1, + ListID: 1, + Created: 1543626724, + Updated: 1543626724, + DueDateUnix: 1543636724, + }, + }, wantErr: false, }, { @@ -460,20 +617,20 @@ func TestListTask_ReadAll(t *testing.T) { } if !reflect.DeepEqual(got, tt.want) { t.Errorf("ListTask.ReadAll() = %v, want %v", got, tt.want) - /*fmt.Println("Got:") + fmt.Println("Got:") gotslice := got.([]*ListTask) for _, g := range gotslice { - fmt.Println(g.Priority, g.Text) + fmt.Println(g.Text) //fmt.Println(g.StartDateUnix) //fmt.Println(g.EndDateUnix) } fmt.Println("Want:") wantslice := tt.want.([]*ListTask) for _, w := range wantslice { - fmt.Println(w.Priority, w.Text) + fmt.Println(w.Text) //fmt.Println(w.StartDateUnix) //fmt.Println(w.EndDateUnix) - }*/ + } } }) } -- 2.45.1