From 1f039c4cda218089d6522b7acfcd4985b111c516 Mon Sep 17 00:00:00 2001 From: konrad Date: Wed, 19 Feb 2020 21:57:56 +0000 Subject: [PATCH] Task Comments (#138) Add swagger docs Add integration tests Add tests Add task comment test fixtures Add config option to enable/disable task comments Add custom error if a task comment does not exist Fix lint Add getting author when getting a single comment Fix getting comments/comments author Add rights check to ReadAll + actually get the comment author Add migration and table definitions Add routes Add ReadOne method Add basic crud rights Signed-off-by: kolaente Implement basic crudable functions for task comments Signed-off-by: kolaente Start adding task comments Signed-off-by: kolaente Co-authored-by: kolaente Reviewed-on: https://kolaente.dev/vikunja/api/pulls/138 --- config.yml.sample | 2 + docs/content/doc/setup/config.md | 2 + docs/content/doc/usage/errors.md | 1 + pkg/config/config.go | 2 + pkg/db/fixtures/task_comments.yml | 84 ++++++++ pkg/db/fixtures/tasks.yml | 9 + pkg/integrations/task_comment_test.go | 284 ++++++++++++++++++++++++++ pkg/migration/20200219183248.go | 51 +++++ pkg/models/error.go | 30 ++- pkg/models/models.go | 1 + pkg/models/task_comment_rights.go | 44 ++++ pkg/models/task_comments.go | 209 +++++++++++++++++++ pkg/models/task_comments_test.go | 126 ++++++++++++ pkg/models/tasks_rights.go | 2 +- pkg/models/unit_tests.go | 1 + pkg/routes/routes.go | 13 ++ 16 files changed, 859 insertions(+), 2 deletions(-) create mode 100644 pkg/db/fixtures/task_comments.yml create mode 100644 pkg/integrations/task_comment_test.go create mode 100644 pkg/migration/20200219183248.go create mode 100644 pkg/models/task_comment_rights.go create mode 100644 pkg/models/task_comments.go create mode 100644 pkg/models/task_comments_test.go diff --git a/config.yml.sample b/config.yml.sample index 85b80ec15e..1b44369d0c 100644 --- a/config.yml.sample +++ b/config.yml.sample @@ -28,6 +28,8 @@ service: enabletaskattachments: true # The time zone all timestamps are in timezone: GMT + # Whether task comments should be enabled or not + enabletaskcomments: true database: # Database type to use. Supported types are mysql, postgres and sqlite. diff --git a/docs/content/doc/setup/config.md b/docs/content/doc/setup/config.md index dd4ebc4415..bfff15fa36 100644 --- a/docs/content/doc/setup/config.md +++ b/docs/content/doc/setup/config.md @@ -71,6 +71,8 @@ service: enabletaskattachments: true # The time zone all timestamps are in timezone: GMT + # Whether task comments should be enabled or not + enabletaskcomments: true database: # Database type to use. Supported types are mysql, postgres and sqlite. diff --git a/docs/content/doc/usage/errors.md b/docs/content/doc/usage/errors.md index a312284f9f..7010deeb79 100644 --- a/docs/content/doc/usage/errors.md +++ b/docs/content/doc/usage/errors.md @@ -48,6 +48,7 @@ This document describes the different errors Vikunja can return. | 4012 | 400 | The task attachment is too large. | | 4013 | 400 | The task sort param is invalid. | | 4014 | 400 | The task sort order is invalid. | +| 4015 | 404 | The task comment does not exist. | | 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. | diff --git a/pkg/config/config.go b/pkg/config/config.go index 92604e8b2e..a2a9cd0629 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -45,6 +45,7 @@ const ( ServiceEnableRegistration Key = `service.enableregistration` ServiceEnableTaskAttachments Key = `service.enabletaskattachments` ServiceTimeZone Key = `service.timezone` + ServiceEnableTaskComments Key = `service.enabletaskcomments` DatabaseType Key = `database.type` DatabaseHost Key = `database.host` @@ -171,6 +172,7 @@ func InitDefaultConfig() { ServiceEnableRegistration.setDefault(true) ServiceEnableTaskAttachments.setDefault(true) ServiceTimeZone.setDefault("GMT") + ServiceEnableTaskComments.setDefault(true) // Database DatabaseType.setDefault("sqlite") diff --git a/pkg/db/fixtures/task_comments.yml b/pkg/db/fixtures/task_comments.yml new file mode 100644 index 0000000000..76f599fe57 --- /dev/null +++ b/pkg/db/fixtures/task_comments.yml @@ -0,0 +1,84 @@ +- id: 1 + comment: Lorem Ipsum Dolor Sit Amet + author_id: 1 + task_id: 1 + created: 1582135626 + updated: 1582135626 +- id: 2 + comment: comment 2 + author_id: 5 + task_id: 14 + created: 1582135626 + updated: 1582135626 +- id: 3 + comment: comment 3 + author_id: 5 + task_id: 15 + created: 1582135626 + updated: 1582135626 +- id: 4 + comment: comment 4 + author_id: 6 + task_id: 16 + created: 1582135626 + updated: 1582135626 +- id: 5 + comment: comment 5 + author_id: 6 + task_id: 17 + created: 1582135626 + updated: 1582135626 +- id: 6 + comment: comment 6 + author_id: 6 + task_id: 18 + created: 1582135626 + updated: 1582135626 +- id: 7 + comment: comment 7 + author_id: 6 + task_id: 19 + created: 1582135626 + updated: 1582135626 +- id: 8 + comment: comment 8 + author_id: 6 + task_id: 20 + created: 1582135626 + updated: 1582135626 +- id: 9 + comment: comment 9 + author_id: 6 + task_id: 21 + created: 1582135626 + updated: 1582135626 +- id: 10 + comment: comment 10 + author_id: 6 + task_id: 22 + created: 1582135626 + updated: 1582135626 +- id: 11 + comment: comment 11 + author_id: 6 + task_id: 23 + created: 1582135626 + updated: 1582135626 +- id: 12 + comment: comment 12 + author_id: 6 + task_id: 24 + created: 1582135626 + updated: 1582135626 +- id: 13 + comment: comment 13 + author_id: 6 + task_id: 25 + created: 1582135626 + updated: 1582135626 +- id: 14 + comment: comment 14 + author_id: 6 + task_id: 26 + created: 1582135626 + updated: 1582135626 diff --git a/pkg/db/fixtures/tasks.yml b/pkg/db/fixtures/tasks.yml index 3881811988..d7f7fae80b 100644 --- a/pkg/db/fixtures/tasks.yml +++ b/pkg/db/fixtures/tasks.yml @@ -243,4 +243,13 @@ percent_done: 0.5 created: 1543626724 updated: 1543626724 + # This task is forbidden for user1 +- id: 34 + text: 'task #34' + created_by_id: 13 + list_id: 20 + index: 20 + created: 1543626724 + updated: 1543626724 + diff --git a/pkg/integrations/task_comment_test.go b/pkg/integrations/task_comment_test.go new file mode 100644 index 0000000000..e1a8bc5a19 --- /dev/null +++ b/pkg/integrations/task_comment_test.go @@ -0,0 +1,284 @@ +// Vikunja is a to-do 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 integrations + +import ( + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/web/handler" + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestTaskComments(t *testing.T) { + testHandler := webHandlerTest{ + user: &testuser1, + strFunc: func() handler.CObject { + return &models.TaskComment{} + }, + t: t, + } + // Only run specific nested tests: + // ^TestTaskComments$/^Update$/^Update_task_items$/^Removing_Assignees_null$ + t.Run("Update", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "1", "commentid": "1"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "99999", "commentid": "9999"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeTaskDoesNotExist) + }) + t.Run("Rights check", func(t *testing.T) { + t.Run("Forbidden", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "14", "commentid": "2"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via Team readonly", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "15", "commentid": "3"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via Team write", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "16", "commentid": "4"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Shared Via Team admin", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "17", "commentid": "5"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + + t.Run("Shared Via User readonly", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "18", "commentid": "6"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via User write", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "19", "commentid": "7"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Shared Via User admin", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "20", "commentid": "8"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + + t.Run("Shared Via NamespaceTeam readonly", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "21", "commentid": "9"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via NamespaceTeam write", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "22", "commentid": "10"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Shared Via NamespaceTeam admin", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "23", "commentid": "11"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + + t.Run("Shared Via NamespaceUser readonly", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "24", "commentid": "12"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via NamespaceUser write", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "25", "commentid": "13"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Shared Via NamespaceUser admin", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "26", "commentid": "14"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + }) + }) + t.Run("Delete", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "1", "commentid": "1"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "99999", "commentid": "9999"}) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeTaskDoesNotExist) + }) + t.Run("Rights check", func(t *testing.T) { + t.Run("Forbidden", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "14", "commentid": "2"}) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via Team readonly", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "15", "commentid": "3"}) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via Team write", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "16", "commentid": "4"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + t.Run("Shared Via Team admin", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "17", "commentid": "5"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + + t.Run("Shared Via User readonly", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "18", "commentid": "6"}) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via User write", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "19", "commentid": "7"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + t.Run("Shared Via User admin", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "20", "commentid": "8"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + + t.Run("Shared Via NamespaceTeam readonly", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "21", "commentid": "9"}) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via NamespaceTeam write", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "22", "commentid": "10"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + t.Run("Shared Via NamespaceTeam admin", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "23", "commentid": "11"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + + t.Run("Shared Via NamespaceUser readonly", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "24", "commentid": "12"}) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via NamespaceUser write", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "25", "commentid": "13"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + t.Run("Shared Via NamespaceUser admin", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "26", "commentid": "14"}) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Successfully deleted.`) + }) + }) + }) + t.Run("Create", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "1"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "9999"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeTaskDoesNotExist) + }) + t.Run("Rights check", func(t *testing.T) { + t.Run("Forbidden", func(t *testing.T) { + // Owned by user13 + _, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "34"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via Team readonly", func(t *testing.T) { + _, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "15"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via Team write", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "16"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Shared Via Team admin", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "17"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + + t.Run("Shared Via User readonly", func(t *testing.T) { + _, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "18"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via User write", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "19"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Shared Via User admin", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "20"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + + t.Run("Shared Via NamespaceTeam readonly", func(t *testing.T) { + _, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "21"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via NamespaceTeam write", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "22"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Shared Via NamespaceTeam admin", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "23"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + + t.Run("Shared Via NamespaceUser readonly", func(t *testing.T) { + _, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "24"}, `{"comment":"Lorem Ipsum"}`) + assert.Error(t, err) + assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`) + }) + t.Run("Shared Via NamespaceUser write", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "25"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + t.Run("Shared Via NamespaceUser admin", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "26"}, `{"comment":"Lorem Ipsum"}`) + assert.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`) + }) + }) + }) +} diff --git a/pkg/migration/20200219183248.go b/pkg/migration/20200219183248.go new file mode 100644 index 0000000000..c2e2ac4288 --- /dev/null +++ b/pkg/migration/20200219183248.go @@ -0,0 +1,51 @@ +// 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 ( + "code.vikunja.io/api/pkg/timeutil" + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +type taskComments20200219183248 struct { + ID int64 `xorm:"autoincr pk unique not null" json:"id" param:"comment"` + Comment string `xorm:"text not null" json:"comment"` + AuthorID int64 `xorm:"not null" json:"-"` + TaskID int64 `xorm:"not null" json:"-" param:"task"` + + Created timeutil.TimeStamp `xorm:"created"` + Updated timeutil.TimeStamp `xorm:"updated"` +} + +func (s taskComments20200219183248) TableName() string { + return "task_comments" +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20200219183248", + Description: "Add task comments table", + Migrate: func(tx *xorm.Engine) error { + return tx.Sync2(taskComments20200219183248{}) + }, + Rollback: func(tx *xorm.Engine) error { + return tx.DropTables(taskComments20200219183248{}) + }, + }) + +} diff --git a/pkg/models/error.go b/pkg/models/error.go index e464a137d5..77b70ba6d6 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -276,7 +276,7 @@ const ErrCodeTaskDoesNotExist = 4002 // HTTPError holds the http error description func (err ErrTaskDoesNotExist) HTTPError() web.HTTPError { - return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTaskDoesNotExist, Message: "This list task does not exist"} + return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTaskDoesNotExist, Message: "This task does not exist"} } // ErrBulkTasksMustBeInSameList represents a "ErrBulkTasksMustBeInSameList" kind of error. @@ -602,6 +602,34 @@ func (err ErrInvalidSortOrder) HTTPError() web.HTTPError { } } +// ErrTaskCommentDoesNotExist represents an error where a task comment does not exist +type ErrTaskCommentDoesNotExist struct { + ID int64 + TaskID int64 +} + +// IsErrTaskCommentDoesNotExist checks if an error is ErrTaskCommentDoesNotExist. +func IsErrTaskCommentDoesNotExist(err error) bool { + _, ok := err.(ErrTaskCommentDoesNotExist) + return ok +} + +func (err ErrTaskCommentDoesNotExist) Error() string { + return fmt.Sprintf("Task comment does not exist [ID: %d, TaskID: %d]", err.ID, err.TaskID) +} + +// ErrCodeTaskCommentDoesNotExist holds the unique world-error code of this error +const ErrCodeTaskCommentDoesNotExist = 4015 + +// HTTPError holds the http error description +func (err ErrTaskCommentDoesNotExist) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusNotFound, + Code: ErrCodeTaskCommentDoesNotExist, + Message: "This task comment does not exist", + } +} + // ================= // Namespace errors // ================= diff --git a/pkg/models/models.go b/pkg/models/models.go index bfa3f1331e..7a0e22efa4 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -50,6 +50,7 @@ func GetTables() []interface{} { &LinkSharing{}, &TaskRelation{}, &TaskAttachment{}, + &TaskComment{}, } } diff --git a/pkg/models/task_comment_rights.go b/pkg/models/task_comment_rights.go new file mode 100644 index 0000000000..b1fec2d53e --- /dev/null +++ b/pkg/models/task_comment_rights.go @@ -0,0 +1,44 @@ +// 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 models + +import "code.vikunja.io/web" + +// CanRead checks if a user can read a comment +func (tc *TaskComment) CanRead(a web.Auth) (bool, error) { + t := Task{ID: tc.TaskID} + return t.CanRead(a) +} + +// CanDelete checks if a user can delete a comment +func (tc *TaskComment) CanDelete(a web.Auth) (bool, error) { + t := Task{ID: tc.TaskID} + return t.CanWrite(a) +} + +// CanUpdate checks if a user can update a comment +func (tc *TaskComment) CanUpdate(a web.Auth) (bool, error) { + t := Task{ID: tc.TaskID} + return t.CanWrite(a) +} + +// CanCreate checks if a user can create a new comment +func (tc *TaskComment) CanCreate(a web.Auth) (bool, error) { + t := Task{ID: tc.TaskID} + return t.CanWrite(a) +} diff --git a/pkg/models/task_comments.go b/pkg/models/task_comments.go new file mode 100644 index 0000000000..41ae22eb06 --- /dev/null +++ b/pkg/models/task_comments.go @@ -0,0 +1,209 @@ +// 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 models + +import ( + "code.vikunja.io/api/pkg/timeutil" + "code.vikunja.io/api/pkg/user" + "code.vikunja.io/web" +) + +// TaskComment represents a task comment +type TaskComment struct { + ID int64 `xorm:"autoincr pk unique not null" json:"id" param:"commentid"` + Comment string `xorm:"text not null" json:"comment"` + AuthorID int64 `xorm:"not null" json:"-"` + Author *user.User `xorm:"-" json:"author"` + TaskID int64 `xorm:"not null" json:"-" param:"task"` + + Created timeutil.TimeStamp `xorm:"created" json:"created"` + Updated timeutil.TimeStamp `xorm:"updated" json:"updated"` + + web.CRUDable `xorm:"-" json:"-"` + web.Rights `xorm:"-" json:"-"` +} + +// TableName holds the table name for the task comments table +func (tc *TaskComment) TableName() string { + return "task_comments" +} + +// Create creates a new task comment +// @Summary Create a new task comment +// @Description Create a new task comment. The user doing this need to have at least write access to the task this comment should belong to. +// @tags task +// @Accept json +// @Produce json +// @Security JWTKeyAuth +// @Param relation body models.TaskComment true "The task comment object" +// @Param taskID path int true "Task ID" +// @Success 200 {object} models.TaskComment "The created task comment object." +// @Failure 400 {object} code.vikunja.io/web.HTTPError "Invalid task comment object provided." +// @Failure 500 {object} models.Message "Internal error" +// @Router /tasks/{taskID}/comments [put] +func (tc *TaskComment) Create(a web.Auth) (err error) { + // Check if the task exists + _, err = GetTaskSimple(&Task{ID: tc.TaskID}) + if err != nil { + return err + } + + tc.AuthorID = a.GetID() + _, err = x.Insert(tc) + return +} + +// Delete removes a task comment +// @Summary Remove a task comment +// @Description Remove a task comment. The user doing this need to have at least write access to the task this comment belongs to. +// @tags task +// @Accept json +// @Produce json +// @Security JWTKeyAuth +// @Param taskID path int true "Task ID" +// @Param commentID path int true "Comment ID" +// @Success 200 {object} models.Message "The task comment was successfully deleted." +// @Failure 400 {object} code.vikunja.io/web.HTTPError "Invalid task comment object provided." +// @Failure 404 {object} code.vikunja.io/web.HTTPError "The task comment was not found." +// @Failure 500 {object} models.Message "Internal error" +// @Router /tasks/{taskID}/comments/{commentID} [delete] +func (tc *TaskComment) Delete() error { + deleted, err := x.ID(tc.ID).NoAutoCondition().Delete(tc) + if deleted == 0 { + return ErrTaskCommentDoesNotExist{ID: tc.ID} + } + return err +} + +// Update updates a task text by its ID +// @Summary Update an existing task comment +// @Description Update an existing task comment. The user doing this need to have at least write access to the task this comment belongs to. +// @tags task +// @Accept json +// @Produce json +// @Security JWTKeyAuth +// @Param taskID path int true "Task ID" +// @Param commentID path int true "Comment ID" +// @Success 200 {object} models.TaskComment "The updated task comment object." +// @Failure 400 {object} code.vikunja.io/web.HTTPError "Invalid task comment object provided." +// @Failure 404 {object} code.vikunja.io/web.HTTPError "The task comment was not found." +// @Failure 500 {object} models.Message "Internal error" +// @Router /tasks/{taskID}/comments/{commentID} [post] +func (tc *TaskComment) Update() error { + updated, err := x.ID(tc.ID).Cols("comment").Update(tc) + if updated == 0 { + return ErrTaskCommentDoesNotExist{ID: tc.ID} + } + return err +} + +// ReadOne handles getting a single comment +// @Summary Remove a task comment +// @Description Remove a task comment. The user doing this need to have at least read access to the task this comment belongs to. +// @tags task +// @Accept json +// @Produce json +// @Security JWTKeyAuth +// @Param taskID path int true "Task ID" +// @Param commentID path int true "Comment ID" +// @Success 200 {object} models.TaskComment "The task comment object." +// @Failure 400 {object} code.vikunja.io/web.HTTPError "Invalid task comment object provided." +// @Failure 404 {object} code.vikunja.io/web.HTTPError "The task comment was not found." +// @Failure 500 {object} models.Message "Internal error" +// @Router /tasks/{taskID}/comments/{commentID} [get] +func (tc *TaskComment) ReadOne() (err error) { + exists, err := x.Get(tc) + if err != nil { + return + } + if !exists { + return ErrTaskCommentDoesNotExist{ + ID: tc.ID, + TaskID: tc.TaskID, + } + } + + // Get the author + author := &user.User{} + _, err = x. + Where("id = ?", tc.AuthorID). + Get(author) + tc.Author = author + return +} + +// ReadAll returns all comments for a task +// @Summary Get all task comments +// @Description Get all task comments. The user doing this need to have at least read access to the task. +// @tags task +// @Accept json +// @Produce json +// @Security JWTKeyAuth +// @Param taskID path int true "Task ID" +// @Success 200 {array} models.TaskComment "The array with all task comments" +// @Failure 500 {object} models.Message "Internal error" +// @Router /tasks/{taskID}/comments [get] +func (tc *TaskComment) ReadAll(auth web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) { + + // Check if the user has access to the task + canRead, err := tc.CanRead(auth) + if err != nil { + return nil, 0, 0, err + } + if !canRead { + return nil, 0, 0, ErrGenericForbidden{} + } + + // Because we can't extend the type in general, we need to do this here. + // Not a good solution, but saves performance. + type TaskCommentWithAuthor struct { + TaskComment + AuthorFromDB *user.User `xorm:"extends" json:"-"` + } + + comments := []*TaskComment{} + err = x. + Where("task_id = ? AND comment like ?", tc.TaskID, "%"+search+"%"). + Join("LEFT", "users", "users.id = task_comments.author_id"). + Limit(getLimitFromPageIndex(page, perPage)). + Find(&comments) + if err != nil { + return + } + + // Get all authors + authors := make(map[int64]*user.User) + err = x. + Select("users.*"). + Table("task_comments"). + Where("task_id = ? AND comment like ?", tc.TaskID, "%"+search+"%"). + Join("INNER", "users", "users.id = task_comments.author_id"). + Find(&authors) + if err != nil { + return + } + + for _, comment := range comments { + comment.Author = authors[comment.AuthorID] + } + + numberOfTotalItems, err = x. + Where("task_id = ? AND comment like ?", tc.TaskID, "%"+search+"%"). + Count(&TaskCommentWithAuthor{}) + return comments, len(comments), numberOfTotalItems, err +} diff --git a/pkg/models/task_comments_test.go b/pkg/models/task_comments_test.go new file mode 100644 index 0000000000..6127d2d0fd --- /dev/null +++ b/pkg/models/task_comments_test.go @@ -0,0 +1,126 @@ +// 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 models + +import ( + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/user" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestTaskComment_Create(t *testing.T) { + u := &user.User{ID: 1} + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{ + Comment: "test", + TaskID: 1, + } + err := tc.Create(u) + assert.NoError(t, err) + }) + t.Run("nonexisting task", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{ + Comment: "test", + TaskID: 99999, + } + err := tc.Create(u) + assert.Error(t, err) + assert.True(t, IsErrTaskDoesNotExist(err)) + }) +} + +func TestTaskComment_Delete(t *testing.T) { + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{ID: 1} + err := tc.Delete() + assert.NoError(t, err) + }) + t.Run("nonexisting comment", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{ID: 9999} + err := tc.Delete() + assert.Error(t, err) + assert.True(t, IsErrTaskCommentDoesNotExist(err)) + }) +} + +func TestTaskComment_Update(t *testing.T) { + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{ + ID: 1, + Comment: "testing", + } + err := tc.Update() + assert.NoError(t, err) + }) + t.Run("nonexisting comment", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{ + ID: 9999, + } + err := tc.Update() + assert.Error(t, err) + assert.True(t, IsErrTaskCommentDoesNotExist(err)) + }) +} + +func TestTaskComment_ReadOne(t *testing.T) { + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{ID: 1} + err := tc.ReadOne() + assert.NoError(t, err) + assert.Equal(t, "Lorem Ipsum Dolor Sit Amet", tc.Comment) + assert.NotEmpty(t, tc.Author.ID) + }) + t.Run("nonexisting", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{ID: 9999} + err := tc.ReadOne() + assert.Error(t, err) + assert.True(t, IsErrTaskCommentDoesNotExist(err)) + }) +} + +func TestTaskComment_ReadAll(t *testing.T) { + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{TaskID: 1} + u := &user.User{ID: 1} + result, resultCount, total, err := tc.ReadAll(u, "", 0, -1) + resultComment := result.([]*TaskComment) + assert.NoError(t, err) + assert.Equal(t, 1, resultCount) + assert.Equal(t, int64(1), total) + assert.Equal(t, int64(1), resultComment[0].ID) + assert.Equal(t, "Lorem Ipsum Dolor Sit Amet", resultComment[0].Comment) + assert.NotEmpty(t, resultComment[0].Author.ID) + }) + t.Run("no access to task", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + tc := &TaskComment{TaskID: 14} + u := &user.User{ID: 1} + _, _, _, err := tc.ReadAll(u, "", 0, -1) + assert.Error(t, err) + assert.True(t, IsErrGenericForbidden(err)) + }) +} diff --git a/pkg/models/tasks_rights.go b/pkg/models/tasks_rights.go index 7e92a08270..4771a995e2 100644 --- a/pkg/models/tasks_rights.go +++ b/pkg/models/tasks_rights.go @@ -64,7 +64,7 @@ func (t *Task) canDoTask(a web.Auth) (bool, error) { return false, err } - // A user can do a task if he has write acces to its list + // A user can do a task if it has write acces to its list l := &List{ID: lI.ListID} return l.CanWrite(a) } diff --git a/pkg/models/unit_tests.go b/pkg/models/unit_tests.go index c3a35acebf..442a6be0f6 100644 --- a/pkg/models/unit_tests.go +++ b/pkg/models/unit_tests.go @@ -46,6 +46,7 @@ func SetupTests() { "namespaces", "task_assignees", "task_attachments", + "task_comments", "task_relations", "task_reminders", "tasks", diff --git a/pkg/routes/routes.go b/pkg/routes/routes.go index e5814b32f3..142a479ee7 100644 --- a/pkg/routes/routes.go +++ b/pkg/routes/routes.go @@ -305,6 +305,19 @@ func registerAPIRoutes(a *echo.Group) { a.GET("/tasks/:task/attachments/:attachment", apiv1.GetTaskAttachment) } + if config.ServiceEnableTaskComments.GetBool() { + taskCommentHandler := &handler.WebHandler{ + EmptyStruct: func() handler.CObject { + return &models.TaskComment{} + }, + } + a.GET("/tasks/:task/comments", taskCommentHandler.ReadAllWeb) + a.PUT("/tasks/:task/comments", taskCommentHandler.CreateWeb) + a.DELETE("/tasks/:task/comments/:commentid", taskCommentHandler.DeleteWeb) + a.POST("/tasks/:task/comments/:commentid", taskCommentHandler.UpdateWeb) + a.GET("/tasks/:task/comments/:commentid", taskCommentHandler.ReadOneWeb) + } + labelHandler := &handler.WebHandler{ EmptyStruct: func() handler.CObject { return &models.Label{}