diff --git a/pkg/routes/api/v2/tasks.go b/pkg/routes/api/v2/tasks.go new file mode 100644 index 000000000..0c0f2d54c --- /dev/null +++ b/pkg/routes/api/v2/tasks.go @@ -0,0 +1,230 @@ +// 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" + "strconv" + "strings" + + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/web/handler" + + "github.com/danielgtaylor/huma/v2" + "github.com/danielgtaylor/huma/v2/conditional" +) + +// expandDoc lists the accepted expand values; shared between the by-id and +// by-index operations so the docs stay in sync. +const expandDoc = "Embed extra, more expensive data in each task. Repeatable. One of: subtasks, buckets, reactions, comments, comment_count, time_entries_count, is_unread. Expanding can return more tasks than the page limit (subtasks) and inflate the response." + +// parseTaskExpand turns the raw `expand` query values into validated +// TaskCollectionExpandable entries. Kept package-level for the TaskCollection +// list endpoint, which accepts the same option. An invalid value returns the +// model's own validation error, which translateDomainError maps to 422. +func parseTaskExpand(raw []string) ([]models.TaskCollectionExpandable, error) { + if len(raw) == 0 { + return nil, nil + } + expand := make([]models.TaskCollectionExpandable, 0, len(raw)) + for _, e := range raw { + v := models.TaskCollectionExpandable(e) + if err := v.Validate(); err != nil { + return nil, err + } + expand = append(expand, v) + } + return expand, nil +} + +// RegisterTaskRoutes wires Task CRUD onto the Huma API. The list lives on +// TaskCollection, not here. +func RegisterTaskRoutes(api huma.API) { + tags := []string{"tasks"} + + Register(api, huma.Operation{ + OperationID: "tasks-read", + Summary: "Get a task", + Description: "Returns a single task by its numeric id. Sends an ETag; pass it as If-None-Match on a later read to get a 304 Not Modified. " + expandDoc, + Method: "GET", + Path: "/tasks/{projecttask}", + Tags: tags, + }, tasksRead) + + Register(api, huma.Operation{ + OperationID: "tasks-read-by-index", + Summary: "Get a task by its project index", + Description: "Returns a single task addressed by its per-project index. The {project} segment accepts either a numeric project id or a textual project identifier (e.g. \"PROJ\"); a value made solely of digits is always treated as an id. " + expandDoc, + Method: "GET", + Path: "/projects/{project}/tasks/by-index/{index}", + Tags: tags, + }, tasksReadByIndex) + + Register(api, huma.Operation{ + OperationID: "tasks-create", + Summary: "Create a task", + Description: "Creates a task in the project from the URL. The authenticated user needs write access to that project and becomes the task's creator.", + Method: "POST", + Path: "/projects/{project}/tasks", + Tags: tags, + }, tasksCreate) + + Register(api, huma.Operation{ + OperationID: "tasks-update", + Summary: "Update a task", + Description: "Replaces all of a task's fields; requires write access. Setting project_id to a different project moves the task and also requires write access to the target project. Use PATCH for a partial update.", + Method: "PUT", + Path: "/tasks/{projecttask}", + Tags: tags, + }, tasksUpdate) + + Register(api, huma.Operation{ + OperationID: "tasks-delete", + Summary: "Delete a task", + Description: "Deletes a task. Requires write access to its project.", + Method: "DELETE", + Path: "/tasks/{projecttask}", + Tags: tags, + }, tasksDelete) +} + +func init() { AddRouteRegistrar(RegisterTaskRoutes) } + +type taskReadOneBody struct { + models.Task + MaxPermission models.Permission `json:"max_permission" readOnly:"true" doc:"The maximum permission the requesting user has on this task (0=read, 1=read/write, 2=admin)."` +} + +func tasksRead(ctx context.Context, in *struct { + ID int64 `path:"projecttask" doc:"The numeric id of the task."` + Expand []string `query:"expand,explode" enum:"subtasks,buckets,reactions,comments,comment_count,time_entries_count,is_unread" doc:"Embed extra data per task. Repeatable."` + conditional.Params +}) (*singleReadBody[taskReadOneBody], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + expand, err := parseTaskExpand(in.Expand) + if err != nil { + return nil, translateDomainError(err) + } + task := &models.Task{ID: in.ID, Expand: expand} + maxPermission, err := handler.DoReadOne(ctx, task, a) + if err != nil { + return nil, translateDomainError(err) + } + body := &taskReadOneBody{Task: *task, MaxPermission: models.Permission(maxPermission)} + return conditionalReadResponse(&in.Params, body, task.Updated, maxPermission) +} + +func tasksReadByIndex(ctx context.Context, in *struct { + Project string `path:"project" doc:"A numeric project id or a textual project identifier (e.g. \"PROJ\")."` + Index int64 `path:"index" doc:"The per-project task index."` + Expand []string `query:"expand,explode" enum:"subtasks,buckets,reactions,comments,comment_count,time_entries_count,is_unread" doc:"Embed extra data per task. Repeatable."` + conditional.Params +}) (*singleReadBody[taskReadOneBody], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + expand, err := parseTaskExpand(in.Expand) + if err != nil { + return nil, translateDomainError(err) + } + projectID, err := resolveProjectIdentifier(in.Project) + if err != nil { + return nil, err + } + + // ID 0 + ProjectID + Index makes the model resolve the id from the + // (project, index) pair in both CanRead and ReadOne. + task := &models.Task{ProjectID: projectID, Index: in.Index, Expand: expand} + maxPermission, err := handler.DoReadOne(ctx, task, a) + if err != nil { + return nil, translateDomainError(err) + } + body := &taskReadOneBody{Task: *task, MaxPermission: models.Permission(maxPermission)} + return conditionalReadResponse(&in.Params, body, task.Updated, maxPermission) +} + +func tasksCreate(ctx context.Context, in *struct { + Project int64 `path:"project" doc:"The numeric id of the project to create the task in."` + Body models.Task +}) (*singleBody[models.Task], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + task := &in.Body + task.ProjectID = in.Project // URL wins over body + if err := handler.DoCreate(ctx, task, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.Task]{Body: task}, nil +} + +// Body matches the read shape so AutoPatch's GET→PUT echo of max_permission validates. +func tasksUpdate(ctx context.Context, in *struct { + ID int64 `path:"projecttask"` + Body taskReadOneBody +}) (*singleBody[models.Task], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + task := &in.Body.Task + task.ID = in.ID // URL wins over body + if err := handler.DoUpdate(ctx, task, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.Task]{Body: task}, nil +} + +func tasksDelete(ctx context.Context, in *struct { + ID int64 `path:"projecttask"` +}) (*emptyBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + if err := handler.DoDelete(ctx, &models.Task{ID: in.ID}, a); err != nil { + return nil, translateDomainError(err) + } + return &emptyBody{}, nil +} + +// resolveProjectIdentifier turns the {project} path segment into a numeric +// project id. A pure-digit value is always an id (mirroring v1's +// ResolveProjectIdentifier middleware); anything else is looked up as a +// case-insensitive identifier and 404s if unknown. +func resolveProjectIdentifier(raw string) (int64, error) { + if id, err := strconv.ParseInt(raw, 10, 64); err == nil { + return id, nil + } + s := db.NewSession() + defer s.Close() + project := &models.Project{} + has, err := s.Where("identifier = ?", strings.ToUpper(raw)).Get(project) + if err != nil { + return 0, translateDomainError(err) + } + if !has { + return 0, huma.Error404NotFound("Project not found") + } + return project.ID, nil +} diff --git a/pkg/webtests/huma_task_test.go b/pkg/webtests/huma_task_test.go new file mode 100644 index 000000000..17583822e --- /dev/null +++ b/pkg/webtests/huma_task_test.go @@ -0,0 +1,280 @@ +// 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 ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "code.vikunja.io/api/pkg/models" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHumaTask mirrors v1's TestTask so v2 contract parity is readable +// side-by-side. Read/update/delete address a task by its numeric id; create +// and by-index live on project-scoped paths that don't fit the harness's +// basePath/{id} shape, so those use humaRequest against a shared env. +// +// Fixture topology the matrix relies on (pkg/db/fixtures/tasks.yml + +// project shares): +// - #1: user1's own task in project 1 (admin) — readable/updatable/deletable. +// - #14: project shared read-only via team — forbidden to write/delete. +// - #34: project 20, private to user13 — invisible to user1. +// - project 6: shared read-only; project 7/8: shared write/admin via team. +func TestHumaTask(t *testing.T) { + testHandler := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks", + idParam: "projecttask", + t: t, + } + + t.Run("ReadOne", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testReadOneWithUser(nil, map[string]string{"projecttask": "1"}) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"id":1`) + assert.Contains(t, rec.Body.String(), `"title":"task #1"`) + assert.Contains(t, rec.Body.String(), `"max_permission":2`) // owner = admin + assert.NotEmpty(t, rec.Result().Header.Get("ETag")) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := testHandler.testReadOneWithUser(nil, map[string]string{"projecttask": "99999"}) + require.Error(t, err) + // CanRead resolves the task before the project check, so a missing + // task surfaces as 404, not the 403 the label read uses. + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Forbidden - private project", func(t *testing.T) { + // Task #34 lives in project 20, private to user13. + _, err := testHandler.testReadOneWithUser(nil, map[string]string{"projecttask": "34"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + + // The v2 harness loads fixtures once and reuses the env across subtests, + // so each mutating subtest targets a distinct task to stay order-independent + // (unlike v1's webHandlerTest, which reloads fixtures per request). + t.Run("Update", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"projecttask": "3"}, `{"title":"Lorem Ipsum"}`) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"title":"Lorem Ipsum"`) + assert.NotContains(t, rec.Body.String(), `"title":"task #3 high prio"`) + }) + t.Run("Move to another project", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"projecttask": "4"}, `{"project_id":7}`) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"project_id":7`) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"projecttask": "99999"}, `{"title":"x"}`) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Forbidden - read-only share", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"projecttask": "14"}, `{"title":"x"}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Forbidden - move into a project the user can't write", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"projecttask": "5"}, `{"project_id":20}`) + require.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrorCodeGenericForbidden) + }) + }) + + t.Run("Delete", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"projecttask": "2"}) + require.NoError(t, err) + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.Empty(t, rec.Body.String()) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"projecttask": "99999"}) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Forbidden - read-only share", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"projecttask": "14"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Shared via team write", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"projecttask": "16"}) + require.NoError(t, err) + assert.Equal(t, http.StatusNoContent, rec.Code) + }) + }) +} + +// TestHumaTask_Create covers the project-scoped create path, which the harness +// basePath shape can't express. Mirrors v1's TestTask/Create matrix. +func TestHumaTask_Create(t *testing.T) { + h := webHandlerTestV2{user: &testuser1, t: t} + require.NoError(t, h.ensureEnv()) + + create := func(project, body string) *httptest.ResponseRecorder { + return humaRequest(t, h.e, http.MethodPost, "/api/v2/projects/"+project+"/tasks", body, humaTokenFor(t, &testuser1), "") + } + + t.Run("Normal", func(t *testing.T) { + rec := create("1", `{"title":"Lorem Ipsum"}`) + require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"title":"Lorem Ipsum"`) + assert.Contains(t, rec.Body.String(), `"project_id":1`) + }) + t.Run("Project id from body is ignored - URL wins", func(t *testing.T) { + rec := create("1", `{"title":"url wins","project_id":7}`) + require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"project_id":1`) + assert.NotContains(t, rec.Body.String(), `"project_id":7`) + }) + t.Run("Nonexisting project", func(t *testing.T) { + rec := create("9999", `{"title":"x"}`) + assert.Equal(t, http.StatusNotFound, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), fmt.Sprintf(`"code":%d`, models.ErrCodeProjectDoesNotExist)) + }) + t.Run("Forbidden - private project", func(t *testing.T) { + rec := create("20", `{"title":"x"}`) + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + t.Run("Forbidden - read-only share", func(t *testing.T) { + rec := create("6", `{"title":"x"}`) + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + t.Run("Shared via team write", func(t *testing.T) { + rec := create("7", `{"title":"Lorem Ipsum"}`) + require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"title":"Lorem Ipsum"`) + }) + t.Run("Empty title is rejected", func(t *testing.T) { + rec := create("1", `{"title":""}`) + assert.Equal(t, http.StatusUnprocessableEntity, rec.Code, "body: %s", rec.Body.String()) + }) +} + +// TestHumaTask_ReadByIndex covers the by-index route, including the textual +// project-identifier resolution that v1 does in echo middleware. Mirrors v1's +// TestTaskByProjectIndex and TestTask/ReadOneByIndex. +func TestHumaTask_ReadByIndex(t *testing.T) { + h := webHandlerTestV2{user: &testuser1, t: t} + require.NoError(t, h.ensureEnv()) + + get := func(project, index string) *httptest.ResponseRecorder { + return humaRequest(t, h.e, http.MethodGet, + fmt.Sprintf("/api/v2/projects/%s/tasks/by-index/%s", project, index), "", humaTokenFor(t, &testuser1), "") + } + + t.Run("By numeric project id", func(t *testing.T) { + rec := get("1", "1") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"id":1`) + assert.Contains(t, rec.Body.String(), `"index":1`) + }) + t.Run("By textual project identifier", func(t *testing.T) { + // Project 1 has identifier "TEST1". + rec := get("TEST1", "1") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"id":1`) + }) + t.Run("Identifier match is case-insensitive", func(t *testing.T) { + rec := get("test1", "1") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"id":1`) + }) + t.Run("Unknown identifier returns 404", func(t *testing.T) { + rec := get("does-not-exist", "1") + assert.Equal(t, http.StatusNotFound, rec.Code, "body: %s", rec.Body.String()) + }) + t.Run("Nonexistent index returns 404", func(t *testing.T) { + rec := get("1", "99999") + assert.Equal(t, http.StatusNotFound, rec.Code, "body: %s", rec.Body.String()) + }) + t.Run("No permission returns 403", func(t *testing.T) { + // Project 2 is inaccessible to user1; must be 403, not a 404 oracle. + rec := get("2", "1") + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) +} + +// TestHumaTask_Expand asserts the expand query param populates the extra, +// more expensive fields, is repeatable (explode), and rejects unknown values. +// comment_count and reactions are genuinely gated on the flag, so they prove +// the param is wired through; subtasks-as-related-tasks load regardless. +func TestHumaTask_Expand(t *testing.T) { + h := webHandlerTestV2{user: &testuser1, t: t} + require.NoError(t, h.ensureEnv()) + tok := humaTokenFor(t, &testuser1) + + t.Run("absent leaves expand-gated fields empty", func(t *testing.T) { + rec := humaRequest(t, h.e, http.MethodGet, "/api/v2/tasks/1", "", tok, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.NotContains(t, rec.Body.String(), `"comment_count":`) + assert.NotContains(t, rec.Body.String(), `"reactions":{`) + }) + t.Run("comment_count", func(t *testing.T) { + rec := humaRequest(t, h.e, http.MethodGet, "/api/v2/tasks/1?expand=comment_count", "", tok, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"comment_count":`, "comment_count must be present: %s", rec.Body.String()) + }) + t.Run("reactions", func(t *testing.T) { + // Task #1 has reaction fixture #1. + rec := humaRequest(t, h.e, http.MethodGet, "/api/v2/tasks/1?expand=reactions", "", tok, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"reactions":{`, "reactions must be embedded: %s", rec.Body.String()) + }) + t.Run("repeated param applies every value", func(t *testing.T) { + // explode binding: both ?expand= values take effect, not just the first. + rec := humaRequest(t, h.e, http.MethodGet, "/api/v2/tasks/1?expand=comment_count&expand=reactions", "", tok, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"comment_count":`) + assert.Contains(t, rec.Body.String(), `"reactions":{`) + }) + t.Run("invalid value is rejected", func(t *testing.T) { + rec := humaRequest(t, h.e, http.MethodGet, "/api/v2/tasks/1?expand=bogus", "", tok, "") + // enum on the query param makes Huma reject it as a 422 before the handler. + assert.Equal(t, http.StatusUnprocessableEntity, rec.Code, "body: %s", rec.Body.String()) + }) +} + +// TestHumaTask_ETagReturns304 covers the v2-only conditional-read behaviour. +func TestHumaTask_ETagReturns304(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodGet, "/api/v2/tasks/1", "", token, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + etag := rec.Header().Get("ETag") + require.NotEmpty(t, etag) + + req := httptest.NewRequest(http.MethodGet, "/api/v2/tasks/1", strings.NewReader("")) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("If-None-Match", etag) + rec = httptest.NewRecorder() + e.ServeHTTP(rec, req) + require.Equal(t, http.StatusNotModified, rec.Code, "body: %s", rec.Body.String()) +}