diff --git a/pkg/routes/api/v2/projects.go b/pkg/routes/api/v2/projects.go index 219fb5422..27a40ea20 100644 --- a/pkg/routes/api/v2/projects.go +++ b/pkg/routes/api/v2/projects.go @@ -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 { diff --git a/pkg/webtests/huma_project_test.go b/pkg/webtests/huma_project_test.go index b63dec8b1..c7d4ccb5b 100644 --- a/pkg/webtests/huma_project_test.go +++ b/pkg/webtests/huma_project_test.go @@ -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") +}