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).
This commit is contained in:
kolaente
kolaente
parent
67aca34124
commit
3271a1e1af
|
|
@ -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:"-"`
|
||||
|
|
|
|||
|
|
@ -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 <https://www.gnu.org/licenses/>.
|
||||
|
||||
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
|
||||
}
|
||||
|
|
@ -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 <https://www.gnu.org/licenses/>.
|
||||
|
||||
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())
|
||||
})
|
||||
})
|
||||
}
|
||||
Loading…
Reference in New Issue