kodev-org
/
vikunja
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(api/v2): align project max_permission to the shared embed pattern 

Read-one now returns a projectReadBody embedding models.Project with
max_permission always populated from CanRead, matching the labels/views
value-embed pattern instead of gating it behind expand=permissions.
CanRead yields a real permission for every readable project (Favorites
pseudo-project and saved-filter-backed ones included), so the field is
always meaningful on a read.

Project remains the no-ETag exception: the response carries user-scoped
favorite/subscription state that changes without bumping Updated, so it
is served fresh.

Update routes its body through the read shape so AutoPatch's GET→PUT echo
of the read-only max_permission validates. Create/Update return null for
max_permission (not computed there) rather than a misleading 0 (=read).
This commit is contained in:
kolaente 2026-06-04 23:44:23 +02:00 committed by kolaente
parent 25665f887f
commit bec991288b
2 changed files with 73 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
pkg/routes/api/v2/projects.go
View File

@ -49,7 +49,7 @@ func RegisterProjectRoutes(api huma.API) {
Register(api, huma.Operation{
OperationID: "projects-read",
Summary: "Get a project",
Description: "Returns a single project the caller can read, including its views and the caller's favorite/subscription state. Resolves the Favorites pseudo-project and saved-filter-backed projects. Pass expand=permissions to include the caller's max_permission; otherwise max_permission is null. Served fresh on every call (no conditional/ETag) because the response carries user-scoped state that changes without bumping the project's updated timestamp.",
Description: "Returns a single project the caller can read, including its views, the caller's favorite/subscription state and the caller's max_permission. Resolves the Favorites pseudo-project and saved-filter-backed projects. Served fresh on every call (no conditional/ETag) because the response carries user-scoped state that changes without bumping the project's updated timestamp.",
Method: http.MethodGet,
Path: "/projects/{id}",
Tags: tags,
@ -109,36 +109,33 @@ func projectsList(ctx context.Context, in *struct {
return &projectListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil
}
// projectReadBody is the read shape. Unlike labels/views, models.Project
// already carries a max_permission field (v1 surfaces it via expand on the
// list route), so the embed just reuses it rather than declaring a sibling.
type projectReadBody struct {
models.Project
}
func projectsRead(ctx context.Context, in *struct {
ID int64 `path:"id"`
Expand string `query:"expand" enum:"permissions" doc:"If set to \"permissions\", the project includes the max permission the requesting user has on it (max_permission)."`
}) (*singleBody[models.Project], error) {
ID int64 `path:"id"`
}) (*singleBody[projectReadBody], error) {
a, err := authFromCtx(ctx)
if err != nil {
return nil, err
}
project := &models.Project{ID: in.ID, Expand: models.ProjectExpandable(in.Expand)}
project := &models.Project{ID: in.ID}
maxPermission, err := handler.DoReadOne(ctx, project, a)
if err != nil {
return nil, translateDomainError(err)
}
// ReadOne doesn't act on Expand itself; the caller's max permission comes
// from DoReadOne's CanRead result. Surface it on the model only when asked,
// matching the list operation's expand=permissions behaviour. When not
// expanded, force PermissionUnknown so max_permission marshals as null
// instead of defaulting to the zero value (0 = PermissionRead), which would
// be a misleading "read" permission for projects the caller may fully own.
if models.ProjectExpandable(in.Expand) == models.ProjectExpandableRights {
project.MaxPermission = models.Permission(maxPermission)
} else {
project.MaxPermission = models.PermissionUnknown
}
// No ETag/conditional read here: a project response carries user-scoped,
// derived state (subscription, favorite, views, computed archived state)
// that changes without bumping project.Updated. An ETag built from Updated
// would hand out stale 304s and hide those changes, so the read is always
// served fresh.
return &singleBody[models.Project]{Body: project}, nil
// CanRead returns a real permission for every readable project (including
// the Favorites pseudo-project and saved-filter-backed ones), so the field
// is always meaningful here — surfaced unconditionally like labels/views.
project.MaxPermission = models.Permission(maxPermission)
// No ETag/conditional read: a project response carries user-scoped, derived
// state (subscription, favorite, views, computed archived state) that
// changes without bumping project.Updated, so it's always served fresh.
return &singleBody[projectReadBody]{Body: &projectReadBody{Project: *project}}, nil
}
func projectsCreate(ctx context.Context, in *struct {
@ -151,22 +148,28 @@ func projectsCreate(ctx context.Context, in *struct {
if err := handler.DoCreate(ctx, &in.Body, a); err != nil {
return nil, translateDomainError(err)
}
// Create/Update don't compute the caller's permission; null says "read it"
// rather than echoing the zero value (0 = read), misleading for the owner.
in.Body.MaxPermission = models.PermissionUnknown
return &singleBody[models.Project]{Body: &in.Body}, nil
}
// Body matches the read shape so AutoPatch's GET→PUT echo of max_permission validates.
func projectsUpdate(ctx context.Context, in *struct {
ID int64 `path:"id"`
Body models.Project
Body projectReadBody
}) (*singleBody[models.Project], error) {
a, err := authFromCtx(ctx)
if err != nil {
return nil, err
}
in.Body.ID = in.ID // URL wins over body
if err := handler.DoUpdate(ctx, &in.Body, a); err != nil {
project := &in.Body.Project
project.ID = in.ID // URL wins over body
if err := handler.DoUpdate(ctx, project, a); err != nil {
return nil, translateDomainError(err)
}
return &singleBody[models.Project]{Body: &in.Body}, nil
project.MaxPermission = models.PermissionUnknown // see projectsCreate
return &singleBody[models.Project]{Body: project}, nil
}
func projectsDelete(ctx context.Context, in *struct {

59
pkg/webtests/huma_project_test.go
View File

@ -18,6 +18,7 @@ package webtests
import (
"encoding/json"
"fmt"
"net/http"
"net/url"
"testing"
@ -136,21 +137,14 @@ func TestHumaProject(t *testing.T) {
assert.NotContains(t, rec.Body.String(), `"owner":{"id":2,"name":"","username":"user2",`)
// Tasks are never embedded on a plain project read.
assert.NotContains(t, rec.Body.String(), `"tasks":`)
// Without expand=permissions, max_permission must be null rather than
// defaulting to 0 (which would falsely read as PermissionRead).
assert.Contains(t, rec.Body.String(), `"max_permission":null`)
// max_permission is always present on a read and reflects the caller's
// permission. User1 owns Test1 → admin (2).
assert.Contains(t, rec.Body.String(), `"max_permission":2`)
// The project read is served fresh on every call; no ETag is sent
// because the response carries derived state that changes without
// bumping project.Updated.
assert.Empty(t, rec.Result().Header.Get("ETag"))
})
t.Run("Expand permissions", func(t *testing.T) {
// User 1 owns Test1 → admin (2); expand surfaces it as max_permission.
// v2 replaces v1's x-max-permission response header with this field.
rec, err := testHandler.testReadOneWithUser(url.Values{"expand": []string{"permissions"}}, map[string]string{"project": "1"})
require.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"max_permission":2`)
})
t.Run("Nonexisting", func(t *testing.T) {
// Projects return 404 here (CanRead → GetProjectSimpleByID → ErrProjectDoesNotExist),
// unlike labels which return 403 from the read branch.
@ -168,17 +162,15 @@ func TestHumaProject(t *testing.T) {
})
// readOneWithMaxPermission reads a shared project and asserts the
// granted level via expand=permissions, the v2 equivalent of v1's
// x-max-permission header assertion.
// granted level via the always-present max_permission field, the v2
// equivalent of v1's x-max-permission header assertion.
readOneWithMaxPermission := func(t *testing.T, projectID, title string, want models.Permission) {
rec, err := testHandler.testReadOneWithUser(nil, map[string]string{"project": projectID})
require.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"`+title+`"`)
recExpanded, err := testHandler.testReadOneWithUser(url.Values{"expand": []string{"permissions"}}, map[string]string{"project": projectID})
require.NoError(t, err)
var p models.Project
require.NoError(t, json.Unmarshal(recExpanded.Body.Bytes(), &p))
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &p))
assert.Equal(t, want, p.MaxPermission)
}
@ -235,6 +227,9 @@ func TestHumaProject(t *testing.T) {
assert.Contains(t, rec.Body.String(), `"owner":{"id":1`)
// Tasks are not embedded in the create response.
assert.NotContains(t, rec.Body.String(), `"tasks":`)
// Create doesn't compute the caller's permission; null over a
// misleading 0 (read) for the owner. Computed on a subsequent read.
assert.Contains(t, rec.Body.String(), `"max_permission":null`)
})
t.Run("Normal with description", func(t *testing.T) {
rec, err := testHandler.testCreateWithUser(nil, nil, `{"title":"Lorem","description":"Ipsum"}`)
@ -332,6 +327,8 @@ func TestHumaProject(t *testing.T) {
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
// The description should not be wiped but returned as it was.
assert.Contains(t, rec.Body.String(), `"description":"Lorem Ipsum`)
// Update doesn't recompute the permission; null, like create.
assert.Contains(t, rec.Body.String(), `"max_permission":null`)
})
t.Run("Normal with updating the description", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"project": "1"}, `{"title":"TestLoremIpsum","description":"Lorem Ipsum dolor sit amet"}`)
@ -506,3 +503,35 @@ func TestHumaProject(t *testing.T) {
})
})
}
// TestHumaProject_PATCHMergePatch confirms AutoPatch round-trips: it GETs the
// read body (which carries the read-only max_permission) and re-PUTs it, so the
// update body sharing the read shape must accept that echo without 422.
func TestHumaProject_PATCHMergePatch(t *testing.T) {
e, err := setupTestEnv()
require.NoError(t, err)
token := humaTokenFor(t, &testuser1)
rec := humaRequest(t, e, http.MethodPost, "/api/v2/projects",
`{"title":"before","description":"keep me"}`, token, "")
require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String())
var created struct {
ID int64 `json:"id"`
}
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &created))
// PATCH only title; AutoPatch must leave description alone.
rec = humaRequest(t, e, http.MethodPatch, fmt.Sprintf("/api/v2/projects/%d", created.ID),
`{"title":"after"}`, token, "application/merge-patch+json")
require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String())
rec = humaRequest(t, e, http.MethodGet, fmt.Sprintf("/api/v2/projects/%d", created.ID), "", token, "")
require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String())
var after struct {
Title string `json:"title"`
Description string `json:"description"`
}
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &after))
assert.Equal(t, "after", after.Title)
assert.Equal(t, "keep me", after.Description, "description must survive the PATCH")
}