From 3271a1e1affdc567d6aa1c58b47587e05753d92d Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 3 Jun 2026 19:20:47 +0200 Subject: [PATCH] feat(api/v2): add nested task comment CRUD Add TaskComment CRUD on /api/v2 under /tasks/{task}/comments, mirroring the project_views nested-resource shape. The resource is feature-gated by config.ServiceEnableTaskComments, checked inside the registrar so it runs after config has loaded. Self-registers via init()+AddRouteRegistrar; no routes.go change. ReadAll exposes the order_by (asc/desc) query param. Adds doc:/readOnly: tags to the shared TaskComment model fields and a TestHumaTaskComment webtest covering list/read/create/update/delete plus negatives (non-author forbidden, comment under the wrong task -> 404). --- pkg/models/task_comments.go | 12 +- pkg/routes/api/v2/task_comments.go | 189 +++++++++++++++++++++++++ pkg/webtests/huma_task_comment_test.go | 186 ++++++++++++++++++++++++ 3 files changed, 381 insertions(+), 6 deletions(-) create mode 100644 pkg/routes/api/v2/task_comments.go create mode 100644 pkg/webtests/huma_task_comment_test.go diff --git a/pkg/models/task_comments.go b/pkg/models/task_comments.go index e079f54e0..5ad811073 100644 --- a/pkg/models/task_comments.go +++ b/pkg/models/task_comments.go @@ -30,18 +30,18 @@ import ( // 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" valid:"dbtext,required"` + ID int64 `xorm:"autoincr pk unique not null" json:"id" param:"commentid" readOnly:"true" doc:"The unique, numeric id of this comment."` + Comment string `xorm:"text not null" json:"comment" valid:"dbtext,required" doc:"The comment text. May contain HTML; mentions are parsed and notify the mentioned users."` AuthorID int64 `xorm:"not null" json:"-"` - Author *user.User `xorm:"-" json:"author"` + Author *user.User `xorm:"-" json:"author" readOnly:"true" doc:"The user who wrote the comment. Set from the authenticated user on create; ignored on write."` TaskID int64 `xorm:"index not null" json:"-" param:"task"` - Reactions ReactionMap `xorm:"-" json:"reactions"` + Reactions ReactionMap `xorm:"-" json:"reactions" readOnly:"true" doc:"The reactions on this comment, keyed by reaction value. Managed through the reactions endpoints, not by writing here."` OrderBy string `xorm:"-" json:"-" query:"order_by"` - Created time.Time `xorm:"created" json:"created"` - Updated time.Time `xorm:"updated" json:"updated"` + Created time.Time `xorm:"created" json:"created" readOnly:"true" doc:"A timestamp when this comment was created. You cannot change this value."` + Updated time.Time `xorm:"updated" json:"updated" readOnly:"true" doc:"A timestamp when this comment was last updated. You cannot change this value."` web.CRUDable `xorm:"-" json:"-"` web.Permissions `xorm:"-" json:"-"` diff --git a/pkg/routes/api/v2/task_comments.go b/pkg/routes/api/v2/task_comments.go new file mode 100644 index 000000000..58afe5c33 --- /dev/null +++ b/pkg/routes/api/v2/task_comments.go @@ -0,0 +1,189 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present 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 Affero 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 Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package apiv2 + +import ( + "context" + "fmt" + "net/http" + + "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/web/handler" + + "github.com/danielgtaylor/huma/v2" + "github.com/danielgtaylor/huma/v2/conditional" +) + +// taskCommentListBody is the list-response envelope. models.TaskComment.ReadAll +// returns []*models.TaskComment, so that's the element type. +type taskCommentListBody struct { + Body Paginated[*models.TaskComment] +} + +// RegisterTaskCommentRoutes wires the nested TaskComment CRUD onto the Huma API. +// Every operation binds two path params: {task} → TaskID and {commentid} → ID. +// +// The resource is feature-gated by config.ServiceEnableTaskComments. The flag is +// checked here rather than in the central wiring: the registrar runs at +// RegisterAll time, after the config has loaded, so a disabled instance simply +// registers no comment routes. +func RegisterTaskCommentRoutes(api huma.API) { + if !config.ServiceEnableTaskComments.GetBool() { + return + } + + tags := []string{"task_comments"} + + Register(api, huma.Operation{ + OperationID: "task-comments-list", + Summary: "List the comments of a task", + Description: "Returns the comments of the given task, paginated. Requires read access to the task. Pass order_by=desc to sort newest-first (default is oldest-first).", + Method: http.MethodGet, + Path: "/tasks/{task}/comments", + Tags: tags, + }, taskCommentsList) + + Register(api, huma.Operation{ + OperationID: "task-comments-read", + Summary: "Get a single comment of a task", + Description: "Returns one comment of a task. The comment must belong to the task in the path. Sends an ETag; pass it as If-None-Match on a later read to get a 304 Not Modified.", + Method: http.MethodGet, + Path: "/tasks/{task}/comments/{commentid}", + Tags: tags, + }, taskCommentsRead) + + Register(api, huma.Operation{ + OperationID: "task-comments-create", + Summary: "Create a comment on a task", + Description: "Adds a comment to the given task. The parent task is taken from the URL, not the body, and the author is the authenticated user. Requires write access to the task.", + Method: http.MethodPost, + Path: "/tasks/{task}/comments", + Tags: tags, + }, taskCommentsCreate) + + Register(api, huma.Operation{ + OperationID: "task-comments-update", + Summary: "Update a comment of a task", + Description: "Replaces a comment's text. The comment must belong to the task in the path, and only its author may update it. Use PATCH for a partial update.", + Method: http.MethodPut, + Path: "/tasks/{task}/comments/{commentid}", + Tags: tags, + }, taskCommentsUpdate) + + Register(api, huma.Operation{ + OperationID: "task-comments-delete", + Summary: "Delete a comment of a task", + Description: "Deletes a comment of a task. The comment must belong to the task in the path, and only its author may delete it.", + Method: http.MethodDelete, + Path: "/tasks/{task}/comments/{commentid}", + Tags: tags, + }, taskCommentsDelete) +} + +func init() { AddRouteRegistrar(RegisterTaskCommentRoutes) } + +func taskCommentsList(ctx context.Context, in *struct { + TaskID int64 `path:"task"` + OrderBy string `query:"order_by" enum:"asc,desc" default:"asc" doc:"Sort order by creation time: 'asc' (oldest first, default) or 'desc' (newest first)."` + ListParams +}) (*taskCommentListBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + result, _, total, err := handler.DoReadAll(ctx, &models.TaskComment{TaskID: in.TaskID, OrderBy: in.OrderBy}, a, in.Q, in.Page, in.PerPage) + if err != nil { + return nil, translateDomainError(err) + } + items, ok := result.([]*models.TaskComment) + if !ok { + return nil, fmt.Errorf("taskComments.ReadAll returned unexpected type %T (expected []*models.TaskComment)", result) + } + return &taskCommentListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil +} + +func taskCommentsRead(ctx context.Context, in *struct { + TaskID int64 `path:"task"` + ID int64 `path:"commentid"` + conditional.Params +}) (*singleReadBody[models.TaskComment], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + // ReadOne resolves the comment scoped to its parent task — the TaskID guards + // against reading a comment of one task through another (IDOR). + comment := &models.TaskComment{ID: in.ID, TaskID: in.TaskID} + if _, err := handler.DoReadOne(ctx, comment, a); err != nil { + return nil, translateDomainError(err) + } + // PreconditionFailed wants the unquoted etag; response header uses RFC 9110 quoted form. + etag := fmt.Sprintf("%d-%d", comment.ID, comment.Updated.UnixNano()) + if in.HasConditionalParams() { + if err := in.PreconditionFailed(etag, comment.Updated); err != nil { + return nil, err + } + } + return &singleReadBody[models.TaskComment]{ETag: `"` + etag + `"`, Body: comment}, nil +} + +func taskCommentsCreate(ctx context.Context, in *struct { + TaskID int64 `path:"task"` + Body models.TaskComment +}) (*singleBody[models.TaskComment], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + in.Body.TaskID = in.TaskID // URL wins over body + if err := handler.DoCreate(ctx, &in.Body, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.TaskComment]{Body: &in.Body}, nil +} + +func taskCommentsUpdate(ctx context.Context, in *struct { + TaskID int64 `path:"task"` + ID int64 `path:"commentid"` + Body models.TaskComment +}) (*singleBody[models.TaskComment], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + in.Body.ID = in.ID // URL wins over body + in.Body.TaskID = in.TaskID // parent from the path scopes the update + if err := handler.DoUpdate(ctx, &in.Body, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.TaskComment]{Body: &in.Body}, nil +} + +func taskCommentsDelete(ctx context.Context, in *struct { + TaskID int64 `path:"task"` + ID int64 `path:"commentid"` +}) (*emptyBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + if err := handler.DoDelete(ctx, &models.TaskComment{ID: in.ID, TaskID: in.TaskID}, a); err != nil { + return nil, translateDomainError(err) + } + return &emptyBody{}, nil +} diff --git a/pkg/webtests/huma_task_comment_test.go b/pkg/webtests/huma_task_comment_test.go new file mode 100644 index 000000000..b9350773e --- /dev/null +++ b/pkg/webtests/huma_task_comment_test.go @@ -0,0 +1,186 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present 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 Affero 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 Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package webtests + +import ( + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHumaTaskComment is the nested feature-gated reference test for /api/v2. +// Comments live under /tasks/{task}/comments/{commentid}, so the harness binds +// two path params: basePath carries the literal {task} and idParam picks +// {commentid}. +// +// The resource is gated behind config.ServiceEnableTaskComments, which +// InitDefaultConfig (called by setupTestEnv) defaults to true — so the +// registrar registers the routes and these tests can reach them. +// +// Fixtures: task 1 (project 1, owned by testuser1) has comment 1 (author 1). +// task 35 (project 21, owned by testuser1) has comment 15 (author 1) and +// comment 17 (author -2, a link share) — used for the link-share read case. +func TestHumaTaskComment(t *testing.T) { + // task 1 belongs to project 1, owned by testuser1. + onTask1 := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks/1/comments", + idParam: "commentid", + t: t, + } + require.NoError(t, onTask1.ensureEnv()) + // task 35 also belongs to testuser1; share the Echo instance so the JWT + // signing secret stays valid (each setupTestEnv() regenerates it). + onTask35 := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks/35/comments", + idParam: "commentid", + t: t, + e: onTask1.e, + } + // task 2 also belongs to project 1; used for the wrong-parent negative. + onTask2 := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks/2/comments", + idParam: "commentid", + t: t, + e: onTask1.e, + } + // user6 has no access to project 1, so it is neither author nor writer on + // task 1's comment 1 — used for the non-author forbidden negatives. + asUser6 := webHandlerTestV2{ + user: &testuser6, + basePath: "/api/v2/tasks/1/comments", + idParam: "commentid", + t: t, + e: onTask1.e, + } + + t.Run("ReadAll", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := onTask1.testReadAllWithUser(nil, nil) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Lorem Ipsum Dolor Sit Amet`) + // comments from other tasks must not leak in. + assert.NotContains(t, rec.Body.String(), `comment 2`) + }) + t.Run("Link share author resolves", func(t *testing.T) { + rec, err := onTask35.testReadAllWithUser(nil, nil) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `comment 15`) + assert.Contains(t, rec.Body.String(), `comment 17`) + }) + t.Run("order_by desc", func(t *testing.T) { + // order_by is an exposed query param; just assert it is accepted. + rec, err := onTask35.testReadAllWithUser(url.Values{"order_by": []string{"desc"}}, nil) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `comment 15`) + }) + t.Run("Forbidden", func(t *testing.T) { + // user6 cannot read task 1. + _, err := asUser6.testReadAllWithUser(nil, nil) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + + t.Run("ReadOne", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := onTask1.testReadOneWithUser(nil, map[string]string{"commentid": "1"}) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `Lorem Ipsum Dolor Sit Amet`) + assert.Contains(t, rec.Body.String(), `"id":1`) + assert.NotEmpty(t, rec.Result().Header.Get("ETag")) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := onTask1.testReadOneWithUser(nil, map[string]string{"commentid": "9999"}) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Comment from another task", func(t *testing.T) { + // comment 1 belongs to task 1; reading it under task 2 must 404. + _, err := onTask2.testReadOneWithUser(nil, map[string]string{"commentid": "1"}) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Forbidden", func(t *testing.T) { + _, err := asUser6.testReadOneWithUser(nil, map[string]string{"commentid": "1"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + + t.Run("Create", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := onTask1.testCreateWithUser(nil, nil, `{"comment":"A brand new comment"}`) + require.NoError(t, err) + assert.Equal(t, http.StatusCreated, rec.Code) + assert.Contains(t, rec.Body.String(), `"comment":"A brand new comment"`) + // author is set server-side from the authenticated user. + assert.Contains(t, rec.Body.String(), `"username":"user1"`) + }) + t.Run("Forbidden", func(t *testing.T) { + _, err := asUser6.testCreateWithUser(nil, nil, `{"comment":"Nope"}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + + t.Run("Update", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := onTask1.testUpdateWithUser(nil, map[string]string{"commentid": "1"}, `{"comment":"Edited comment"}`) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"comment":"Edited comment"`) + }) + t.Run("Comment from another task", func(t *testing.T) { + // comment 1 is on task 1; updating it under task 2 must 404. + _, err := onTask2.testUpdateWithUser(nil, map[string]string{"commentid": "1"}, `{"comment":"x"}`) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Forbidden non-author", func(t *testing.T) { + // user6 is not the author of comment 1 (and has no write access). + _, err := asUser6.testUpdateWithUser(nil, map[string]string{"commentid": "1"}, `{"comment":"x"}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + + t.Run("Delete", func(t *testing.T) { + t.Run("Forbidden non-author", func(t *testing.T) { + _, err := asUser6.testDeleteWithUser(nil, map[string]string{"commentid": "1"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Comment from another task", func(t *testing.T) { + _, err := onTask2.testDeleteWithUser(nil, map[string]string{"commentid": "1"}) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Normal", func(t *testing.T) { + // Run last: comment 1 is the author's own, so this succeeds and + // removes the fixture row used by the read/update cases above. + rec, err := onTask1.testDeleteWithUser(nil, map[string]string{"commentid": "1"}) + require.NoError(t, err) + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.Empty(t, rec.Body.String()) + }) + }) +}