From 413006e9bad7dcad2c44f8e99ae124453cd0d11c Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 5 Jun 2026 10:04:02 +0200 Subject: [PATCH] feat(api/v2): add task labels (create/list/delete) on /api/v2 Port the LabelTask resource (labels attached to a task) from the frozen /api/v1 to the Huma-backed /api/v2 as nested routes under /tasks/{projecttask}/labels: - GET list the labels on a task (read access to the task) - POST attach a label to a task (write access to the task + access to the label) - DELETE detach a label from a task (write access to the task) There is no read-one or update for a label-task relation, so no max_permission. Adds doc tags and marks the server-set created timestamp readOnly on the shared model. Permissions stay enforced at the model layer via the existing Can* methods through handler.Do*. --- pkg/models/label_task.go | 4 +- pkg/routes/api/v2/label_tasks.go | 119 ++++++++++++++++ pkg/webtests/huma_label_task_test.go | 201 +++++++++++++++++++++++++++ 3 files changed, 322 insertions(+), 2 deletions(-) create mode 100644 pkg/routes/api/v2/label_tasks.go create mode 100644 pkg/webtests/huma_label_task_test.go diff --git a/pkg/models/label_task.go b/pkg/models/label_task.go index 8a09a3c01..ad8b46c2c 100644 --- a/pkg/models/label_task.go +++ b/pkg/models/label_task.go @@ -36,9 +36,9 @@ type LabelTask struct { ID int64 `xorm:"bigint autoincr not null unique pk" json:"-"` TaskID int64 `xorm:"bigint INDEX not null" json:"-" param:"projecttask"` // The label id you want to associate with a task. - LabelID int64 `xorm:"bigint INDEX not null" json:"label_id" param:"label"` + LabelID int64 `xorm:"bigint INDEX not null" json:"label_id" param:"label" doc:"The id of the label to associate with the task."` // A timestamp when this task was created. You cannot change this value. - Created time.Time `xorm:"created not null" json:"created"` + Created time.Time `xorm:"created not null" json:"created" readOnly:"true" doc:"A timestamp when this label was added to the task. You cannot change this value."` web.CRUDable `xorm:"-" json:"-"` web.Permissions `xorm:"-" json:"-"` diff --git a/pkg/routes/api/v2/label_tasks.go b/pkg/routes/api/v2/label_tasks.go new file mode 100644 index 000000000..1ea47800a --- /dev/null +++ b/pkg/routes/api/v2/label_tasks.go @@ -0,0 +1,119 @@ +// 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/models" + "code.vikunja.io/api/pkg/web/handler" + + "github.com/danielgtaylor/huma/v2" +) + +// Element type is *models.LabelWithTaskID because that's what +// models.LabelTask.ReadAll returns; TaskID is json:"-", so the wire shape +// matches plain Label. +type labelTaskListBody struct { + Body Paginated[*models.LabelWithTaskID] +} + +// RegisterLabelTaskRoutes wires the nested labels-on-a-task routes onto the +// Huma API: list, attach and detach. There is no read-one or update — a +// label-task is just a relation, so it has no max_permission. +func RegisterLabelTaskRoutes(api huma.API) { + tags := []string{"labels"} + + Register(api, huma.Operation{ + OperationID: "task-labels-list", + Summary: "List the labels on a task", + Description: "Returns the labels attached to the given task, paginated. Requires read access to the task.", + Method: http.MethodGet, + Path: "/tasks/{projecttask}/labels", + Tags: tags, + }, labelTasksList) + + Register(api, huma.Operation{ + OperationID: "task-labels-create", + Summary: "Add a label to a task", + Description: "Attaches an existing label to the given task. Requires write access to the task and access to the label. Fails if the label is already on the task.", + Method: http.MethodPost, + Path: "/tasks/{projecttask}/labels", + Tags: tags, + }, labelTasksCreate) + + Register(api, huma.Operation{ + OperationID: "task-labels-delete", + Summary: "Remove a label from a task", + Description: "Detaches a label from the given task. Requires write access to the task.", + Method: http.MethodDelete, + Path: "/tasks/{projecttask}/labels/{label}", + Tags: tags, + }, labelTasksDelete) +} + +func init() { AddRouteRegistrar(RegisterLabelTaskRoutes) } + +func labelTasksList(ctx context.Context, in *struct { + TaskID int64 `path:"projecttask"` + ListParams +}) (*labelTaskListBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + result, _, total, err := handler.DoReadAll(ctx, &models.LabelTask{TaskID: in.TaskID}, a, in.Q, in.Page, in.PerPage) + if err != nil { + return nil, translateDomainError(err) + } + items, ok := result.([]*models.LabelWithTaskID) + if !ok { + return nil, fmt.Errorf("labelTasks.ReadAll returned unexpected type %T (expected []*models.LabelWithTaskID)", result) + } + return &labelTaskListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil +} + +func labelTasksCreate(ctx context.Context, in *struct { + TaskID int64 `path:"projecttask"` + Body models.LabelTask +}) (*singleBody[models.LabelTask], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + in.Body.TaskID = in.TaskID // parent from the path, not the body + if err := handler.DoCreate(ctx, &in.Body, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.LabelTask]{Body: &in.Body}, nil +} + +func labelTasksDelete(ctx context.Context, in *struct { + TaskID int64 `path:"projecttask"` + LabelID int64 `path:"label"` +}) (*emptyBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + if err := handler.DoDelete(ctx, &models.LabelTask{TaskID: in.TaskID, LabelID: in.LabelID}, a); err != nil { + return nil, translateDomainError(err) + } + return &emptyBody{}, nil +} diff --git a/pkg/webtests/huma_label_task_test.go b/pkg/webtests/huma_label_task_test.go new file mode 100644 index 000000000..1f191a67e --- /dev/null +++ b/pkg/webtests/huma_label_task_test.go @@ -0,0 +1,201 @@ +// 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 ( + "encoding/json" + "net/http" + "testing" + + "code.vikunja.io/api/pkg/models" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestLabelTask is the nested-path test for labels-on-a-task under +// /api/v2/tasks/{projecttask}/labels. It ports the full v1 model-level matrix +// from pkg/models/label_task_test.go so the v2 HTTP surface independently +// proves the permission contract once v1's routes are removed. +// +// Permission topology for testuser1 (see pkg/db/fixtures): +// - task 1 (project 1): owned by user1 → write/admin. Has label #4 attached. +// - task 14 (project 5): no access → forbidden. +// - task 15 (project 6): shared via team 2 read-only → readable, not writable. +// - task 16 (project 7): shared via team 3 with write → writable. +// - task 34 (project 20): private to user13 → no access. +// +// Labels user1 may attach: #1 (own) and #4 (visible via accessible task 1). +// Label #9999 does not exist → no access → forbidden on attach. +func TestLabelTask(t *testing.T) { + // task 1 is owned by testuser1. + owned := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks/1/labels", + idParam: "label", + t: t, + } + require.NoError(t, owned.ensureEnv()) + // Each setupTestEnv() regenerates the JWT signing secret, so every harness + // below must reuse owned's Echo instance to keep its token valid. + forbidden := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks/14/labels", + idParam: "label", + t: t, + e: owned.e, + } + readShared := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks/15/labels", + idParam: "label", + t: t, + e: owned.e, + } + writeShared := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks/16/labels", + idParam: "label", + t: t, + e: owned.e, + } + private := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/tasks/34/labels", + idParam: "label", + t: t, + e: owned.e, + } + + t.Run("ReadAll", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := owned.testReadAllWithUser(nil, nil) + require.NoError(t, err) + // task 1 has exactly label #4 attached. + ids := labelTaskIDsFromReadAll(t, rec.Body.Bytes()) + assert.ElementsMatch(t, []int64{4}, ids, + "ReadAll on task 1 must return exactly {4}; body: %s", rec.Body.String()) + }) + t.Run("Read-only share can list", func(t *testing.T) { + // ReadAll only requires read access to the task; a read share suffices. + _, err := readShared.testReadAllWithUser(nil, nil) + require.NoError(t, err) + }) + t.Run("Search", func(t *testing.T) { + rec, err := owned.testReadAllWithUser(map[string][]string{"q": {"4"}}, nil) + require.NoError(t, err) + ids := labelTaskIDsFromReadAll(t, rec.Body.Bytes()) + assert.ElementsMatch(t, []int64{4}, ids) + }) + t.Run("Forbidden", func(t *testing.T) { + _, err := forbidden.testReadAllWithUser(nil, nil) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Nonexisting task", func(t *testing.T) { + noTask := webHandlerTestV2{user: &testuser1, basePath: "/api/v2/tasks/9999/labels", idParam: "label", t: t, e: owned.e} + _, err := noTask.testReadAllWithUser(nil, nil) + require.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeTaskDoesNotExist) + }) + }) + + t.Run("Create", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := owned.testCreateWithUser(nil, nil, `{"label_id":1}`) + require.NoError(t, err) + assert.Equal(t, http.StatusCreated, rec.Code) + assert.Contains(t, rec.Body.String(), `"label_id":1`) + }) + t.Run("Write share can attach", func(t *testing.T) { + // task 16 is write-shared; user1 has access to label #1. + rec, err := writeShared.testCreateWithUser(nil, nil, `{"label_id":1}`) + require.NoError(t, err) + assert.Equal(t, http.StatusCreated, rec.Code) + }) + t.Run("Already on task", func(t *testing.T) { + // label #4 is already attached to task 1. + _, err := owned.testCreateWithUser(nil, nil, `{"label_id":4}`) + require.Error(t, err) + assert.Equal(t, http.StatusBadRequest, getHTTPErrorCode(err)) + assertHandlerErrorCode(t, err, models.ErrCodeLabelIsAlreadyOnTask) + }) + t.Run("Nonexisting label", func(t *testing.T) { + // CanCreate looks the label up first, so a missing label surfaces as + // 404 (label not found) rather than a permission error. + _, err := owned.testCreateWithUser(nil, nil, `{"label_id":9999}`) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + assertHandlerErrorCode(t, err, models.ErrCodeLabelDoesNotExist) + }) + t.Run("Read-only share cannot attach", func(t *testing.T) { + // task 15 is read-only shared → CanCreate needs write to the task. + _, err := readShared.testCreateWithUser(nil, nil, `{"label_id":1}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Forbidden task", func(t *testing.T) { + _, err := private.testCreateWithUser(nil, nil, `{"label_id":1}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + + t.Run("Delete", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + // label #4 is attached to task 1. + rec, err := owned.testDeleteWithUser(nil, map[string]string{"label": "4"}) + require.NoError(t, err) + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.Empty(t, rec.Body.String()) + }) + t.Run("Nonexisting relation", func(t *testing.T) { + // label #2 (own, never attached by any subtest) is not on task 1 → + // CanDelete requires the relation to exist, so it refuses. + _, err := owned.testDeleteWithUser(nil, map[string]string{"label": "2"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Read-only share cannot detach", func(t *testing.T) { + _, err := readShared.testDeleteWithUser(nil, map[string]string{"label": "1"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Forbidden task", func(t *testing.T) { + _, err := private.testDeleteWithUser(nil, map[string]string{"label": "1"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) +} + +// labelTaskIDsFromReadAll extracts the label IDs from a v2 paginated list body +// so the attached set can be asserted exactly rather than via substring match. +func labelTaskIDsFromReadAll(t *testing.T, body []byte) []int64 { + t.Helper() + var resp struct { + Items []struct { + ID int64 `json:"id"` + } `json:"items"` + } + require.NoError(t, json.Unmarshal(body, &resp), "ReadAll body must be a paginated envelope: %s", string(body)) + ids := make([]int64, 0, len(resp.Items)) + for _, it := range resp.Items { + ids = append(ids, it.ID) + } + return ids +}