From 3dadac1c17ac43029299cc9882f0a725769cb77d Mon Sep 17 00:00:00 2001 From: randrini Date: Wed, 7 Oct 2026 11:20:26 +0200 Subject: [PATCH 1/4] feat(catalog): list an item's collections on its v2 detail page Add the additive `collections` member to the v2 item detail: the visible server collections a movie or series belongs to, each with id, title, poster_url, and item_count. A dedicated `getItemCollectionsCapability` document reports whether the row is wired. The lookup reuses `LibraryCollectionRepository.ListContainingItem` through a narrow `ItemCollectionIndex` seam on the existing collection handler, so the item detail never widens `LibraryCollectionService` for every fake. It filters by `CanAccessLibraryCollection` and resolves posters through the same viewer-aware path as the library Collections tab, so a hidden or out-of-scope collection never leaks. An episode or season answers its parent series' collections, because only movies and series can be members. The row is best-effort, mirroring the theme lookup: an unwired index or a lookup failure leaves the detail response a success with an empty row. Contract artifacts are regenerated, not hand-edited. New fixture cases append at the end of the positional list so committed request ids stay stable. --- .../fixtures/admin_item_metadata_updated.json | 3 +- .../api/v2/fixtures/get_catalog_item_ok.json | 10 +- ...ns_capability_authentication_required.json | 7 + .../get_item_collections_capability_ok.json | 5 + .../api/v2/fixtures/get_system_info_ok.json | 2 +- contracts/api/v2/fixtures/index.json | 38 +++ contracts/api/v2/openapi.json | 276 ++++++++++++++++++ docs/architecture/api-contract.md | 10 + docs/catalog-api.md | 25 ++ internal/api/handlers/library_collections.go | 12 + ...brary_collections_item_collections_test.go | 114 ++++++++ .../api/handlers/library_views_service.go | 55 ++++ internal/apiv2/catalog_items.go | 55 +++- internal/apiv2/catalog_items_test.go | 49 ++++ internal/apiv2/document_test.go | 1 + internal/apiv2/fixtures_test.go | 8 +- internal/apiv2/item_collections_capability.go | 39 +++ ...em_collections_capability_fixtures_test.go | 18 ++ .../apiv2/item_collections_capability_test.go | 52 ++++ internal/apiv2/library_views_test.go | 12 + internal/apiv2/router.go | 8 + 21 files changed, 793 insertions(+), 6 deletions(-) create mode 100644 contracts/api/v2/fixtures/get_item_collections_capability_authentication_required.json create mode 100644 contracts/api/v2/fixtures/get_item_collections_capability_ok.json create mode 100644 internal/api/handlers/library_collections_item_collections_test.go create mode 100644 internal/apiv2/item_collections_capability.go create mode 100644 internal/apiv2/item_collections_capability_fixtures_test.go create mode 100644 internal/apiv2/item_collections_capability_test.go diff --git a/contracts/api/v2/fixtures/admin_item_metadata_updated.json b/contracts/api/v2/fixtures/admin_item_metadata_updated.json index 202cd27a31..9a636aeb93 100644 --- a/contracts/api/v2/fixtures/admin_item_metadata_updated.json +++ b/contracts/api/v2/fixtures/admin_item_metadata_updated.json @@ -9,5 +9,6 @@ "crew": [], "versions": [], "ratings": [], - "subtitles": [] + "subtitles": [], + "collections": [] } diff --git a/contracts/api/v2/fixtures/get_catalog_item_ok.json b/contracts/api/v2/fixtures/get_catalog_item_ok.json index beca42f72c..546b0ed22c 100644 --- a/contracts/api/v2/fixtures/get_catalog_item_ok.json +++ b/contracts/api/v2/fixtures/get_catalog_item_ok.json @@ -81,5 +81,13 @@ "display": "8.3" } ], - "subtitles": [] + "subtitles": [], + "collections": [ + { + "id": "oscar-winners", + "title": "Oscar Winners", + "poster_url": "https://cdn.example.invalid/oscar-winners.jpg", + "item_count": 12 + } + ] } diff --git a/contracts/api/v2/fixtures/get_item_collections_capability_authentication_required.json b/contracts/api/v2/fixtures/get_item_collections_capability_authentication_required.json new file mode 100644 index 0000000000..bbc86532e6 --- /dev/null +++ b/contracts/api/v2/fixtures/get_item_collections_capability_authentication_required.json @@ -0,0 +1,7 @@ +{ + "type": "https://siloserver.org/docs/api/v2/problems/authentication_required", + "title": "Authentication required", + "status": 401, + "detail": "Authentication is required.", + "instance": "urn:silo:request:0000000000000000000002cc" +} diff --git a/contracts/api/v2/fixtures/get_item_collections_capability_ok.json b/contracts/api/v2/fixtures/get_item_collections_capability_ok.json new file mode 100644 index 0000000000..381e3b8fc7 --- /dev/null +++ b/contracts/api/v2/fixtures/get_item_collections_capability_ok.json @@ -0,0 +1,5 @@ +{ + "revision": "0c37f6ac3b88f5930ce78f264098826e315b407b99d69ada19e944f3ae2806aa", + "state": "available", + "allowed": true +} diff --git a/contracts/api/v2/fixtures/get_system_info_ok.json b/contracts/api/v2/fixtures/get_system_info_ok.json index 33afe8998c..a8adfd530b 100644 --- a/contracts/api/v2/fixtures/get_system_info_ok.json +++ b/contracts/api/v2/fixtures/get_system_info_ok.json @@ -1,7 +1,7 @@ { "server_version": "unavailable", "api_major": 2, - "contract_digest": "855d9197130c80371dc1c84364cef8c1ac3ffc2a0d9285e213e28b23ca23aafd", + "contract_digest": "187a3914d680ea0133f8cff2f02ce7982872ed2ea20b4cebb8b4069f4ba3fc19", "links": { "openapi": "/api/v2/openapi.json", "capabilities": "/api/v2/capabilities", diff --git a/contracts/api/v2/fixtures/index.json b/contracts/api/v2/fixtures/index.json index 48d6b3b08f..d507efe29d 100644 --- a/contracts/api/v2/fixtures/index.json +++ b/contracts/api/v2/fixtures/index.json @@ -6554,6 +6554,44 @@ "schema": "#/components/schemas/ImageCapabilities", "body_file": "get_image_capabilities_ok.json" }, + { + "name": "get_item_collections_capability_authentication_required", + "operation_id": "getItemCollectionsCapability", + "scenario": "The item-collections capability document without a credential.", + "request": { + "method": "GET", + "path": "/api/v2/capabilities/item-collections" + }, + "expected_status": 401, + "response_headers": { + "Cache-Control": "no-store", + "Content-Type": "application/problem+json" + }, + "response_media_type": "application/problem+json", + "schema": "#/components/schemas/Problem", + "body_file": "get_item_collections_capability_authentication_required.json" + }, + { + "name": "get_item_collections_capability_ok", + "operation_id": "getItemCollectionsCapability", + "scenario": "The item-collections capability document on a server whose item detail carries the collections row.", + "request": { + "method": "GET", + "path": "/api/v2/capabilities/item-collections", + "headers": { + "Authorization": "Bearer tok-member", + "X-Profile-Id": "p-owner" + } + }, + "expected_status": 200, + "response_headers": { + "Cache-Control": "private, no-cache", + "Content-Type": "application/json" + }, + "response_media_type": "application/json", + "schema": "#/components/schemas/ItemCollectionsCapability", + "body_file": "get_item_collections_capability_ok.json" + }, { "name": "get_item_markers_ok", "operation_id": "getItemMarkers", diff --git a/contracts/api/v2/openapi.json b/contracts/api/v2/openapi.json index 7a973ceaa2..b8d72152aa 100644 --- a/contracts/api/v2/openapi.json +++ b/contracts/api/v2/openapi.json @@ -20079,6 +20079,13 @@ }, "type": "array" }, + "collections": { + "description": "Visible collections containing this item (movies and series); empty, never null", + "items": { + "$ref": "#/components/schemas/ItemCollection" + }, + "type": "array" + }, "content_id": { "description": "Deterministic catalog identifier", "examples": [ @@ -20464,6 +20471,7 @@ "versions", "ratings", "subtitles", + "collections", "content_id", "type", "title", @@ -28455,6 +28463,73 @@ ], "type": "object" }, + "ItemCollection": { + "additionalProperties": false, + "properties": { + "id": { + "examples": [ + "01J9Z8C3W4R5T6Y7U8I9O0P1Q2" + ], + "type": "string" + }, + "item_count": { + "examples": [ + 3 + ], + "format": "int64", + "type": "integer" + }, + "poster_thumbhash": { + "type": "string" + }, + "poster_url": { + "description": "Presigned, short-lived; empty when none", + "type": "string" + }, + "title": { + "examples": [ + "Dune Saga" + ], + "type": "string" + } + }, + "required": [ + "id", + "title", + "poster_url", + "item_count" + ], + "type": "object" + }, + "ItemCollectionsCapability": { + "additionalProperties": false, + "properties": { + "allowed": { + "description": "Whether the current principal may use the capability", + "type": "boolean" + }, + "revision": { + "description": "Opaque revision of this document", + "type": "string" + }, + "state": { + "description": "Support and configuration state, not health", + "enum": [ + "available", + "disabled", + "not_configured", + "unsupported" + ], + "type": "string" + } + }, + "required": [ + "revision", + "state", + "allowed" + ], + "type": "object" + }, "ItemExtraInfo": { "additionalProperties": false, "properties": { @@ -122962,6 +123037,207 @@ "x-silo-service-backed": true } }, + "/api/v2/capabilities/item-collections": { + "get": { + "operationId": "getItemCollectionsCapability", + "parameters": [ + { + "description": "Optional first precondition, evaluated before If-None-Match: a tag that does not match the current representation is 412 precondition_failed.", + "in": "header", + "name": "If-Match", + "schema": { + "type": "string" + } + }, + { + "description": "The household profile acting for this request; it must belong to the authenticated account.", + "in": "header", + "name": "X-Profile-Id", + "required": true, + "schema": { + "examples": [ + "1" + ], + "type": "string" + } + }, + { + "description": "Verification proof for a PIN-locked profile, issued by POST /api/v2/profiles/{id}/verify-pin; required only when the declared profile is locked", + "in": "header", + "name": "X-Profile-Token", + "schema": { + "examples": [ + "pvt_5f3a9c1e7b2d4e8fa0c6" + ], + "type": "string" + } + }, + { + "in": "header", + "name": "If-None-Match", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItemCollectionsCapability" + } + } + }, + "description": "OK", + "headers": { + "Cache-Control": { + "schema": { + "type": "string" + } + }, + "ETag": { + "description": "The strong, opaque validator of the representation; send it back in If-Match on a guarded mutation or If-None-Match on a conditional read.", + "schema": { + "type": "string" + } + } + } + }, + "304": { + "description": "The representation named by If-None-Match is current; no body.", + "headers": { + "ETag": { + "description": "The strong, opaque validator of the representation; send it back in If-Match on a guarded mutation or If-None-Match on a conditional read.", + "schema": { + "type": "string" + } + } + } + }, + "400": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Bad Request" + }, + "401": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Unauthorized" + }, + "403": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Forbidden" + }, + "404": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Not Found" + }, + "406": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Not Acceptable" + }, + "412": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Precondition Failed", + "headers": { + "ETag": { + "description": "The strong, opaque validator of the representation; send it back in If-Match on a guarded mutation or If-None-Match on a conditional read.", + "schema": { + "type": "string" + } + } + } + }, + "422": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Unprocessable Entity" + }, + "429": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Too Many Requests" + }, + "500": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Internal Server Error" + }, + "503": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Service Unavailable" + } + }, + "security": [ + { + "bearerAuth": [] + } + ], + "summary": "Whether item detail carries the collections an item belongs to.", + "tags": [ + "catalog" + ], + "x-silo-class": "profile_scoped", + "x-silo-conditional": true, + "x-silo-service-backed": true + } + }, "/api/v2/capabilities/metadata-ai": { "get": { "operationId": "getMetadataAICapability", diff --git a/docs/architecture/api-contract.md b/docs/architecture/api-contract.md index 3e9c179de3..925fd750dc 100644 --- a/docs/architecture/api-contract.md +++ b/docs/architecture/api-contract.md @@ -1568,6 +1568,16 @@ contract; an unconfigured action is `409 capability_not_configured` rather than with `limit` validated `1..100`, integer identifiers are strings, and the work document's lists are empty arrays rather than `null`. +**Section catalog-items (Phase 4), stage C.** `getCatalogItem` gained the additive `collections` +member — the visible server collections a movie or series belongs to, with `id`, `title`, +`poster_url`, and `item_count` — plus its feature document `getItemCollectionsCapability` +(`/capabilities/item-collections`, `available` when the collection index is wired, `unsupported` +otherwise). There is no v1 counterpart, so no new ledger row. The reverse lookup reuses +`library_collection_items` (`LibraryCollectionRepository.ListContainingItem`); the reader filters +by `CanAccessLibraryCollection` and resolves posters through the same viewer-aware path as the +library Collections tab, so a hidden or out-of-scope collection never leaks. An episode or season +detail answers its parent series' collections, because only movies and series can be members. + ## v1 lifecycle and release sequence 1. Freeze v1 feature development. Critical fixes needed to keep the bridge usable may still land; diff --git a/docs/catalog-api.md b/docs/catalog-api.md index 31ee23a046..0e92ef27a4 100644 --- a/docs/catalog-api.md +++ b/docs/catalog-api.md @@ -564,6 +564,31 @@ book libraries, never carry an advisory age, so the limit never hides them. Frozen v1 responses do not expose these fields. +## Collections on title pages + +The v2 item detail of a movie or series carries `collections`, the visible +server collections the item belongs to. Each entry has: + +- `id`: the collection id, used to open its full item list + (`listCatalogItems?source=library_collection&collection_id=`). +- `title`: the collection's display name. +- `poster_url`: the collection's poster, presigned and short-lived, empty when + it has none. +- `item_count`: the collection's member count. + +The list is present on every v2 item detail as an array, empty when the item is +in none. Only movies and series can be members, so an episode detail answers +its parent series' collections and a season detail answers its series'. A +collection the viewer's library scope cannot reach, and a hidden collection, +are never listed. The `poster_url` follows the same viewer rule as the library +Collections tab: an uploaded or template poster is shared, while a generated +collage shows only members the viewer can access. + +`GET /api/v2/capabilities/item-collections` is the feature-detection document. +It answers `state: available` on a server whose item detail carries +`collections`, and `unsupported` otherwise. Clients that omit the row on an +older server without the operation keep their existing behavior. + ## Ratings on title pages Every client shows a title's external ratings the same way: the v2 item detail diff --git a/internal/api/handlers/library_collections.go b/internal/api/handlers/library_collections.go index 1c274db08b..2fbad60116 100644 --- a/internal/api/handlers/library_collections.go +++ b/internal/api/handlers/library_collections.go @@ -60,6 +60,18 @@ type LibraryCollectionHandler struct { JobRepo *adminjob.Repository EventsHub *evt.Hub SortPreferenceCleaner *userstore.CollectionSortPreferenceCleaner + + // itemCollectionIndex answers the item→collections reverse lookup behind + // the item detail "collections" field. Nil means use repo; tests inject a + // fake so the lookup is exercised without a database. + itemCollectionIndex itemCollectionIndex +} + +// itemCollectionIndex is the reverse membership lookup the item detail +// "collections" field needs: the visible collections that store a membership +// row for one item. *catalog.LibraryCollectionRepository implements it. +type itemCollectionIndex interface { + ListContainingItem(ctx context.Context, mediaItemID string) ([]*models.LibraryCollection, error) } var errLibraryCollectionInUse = catalog.ErrLibraryCollectionInUse diff --git a/internal/api/handlers/library_collections_item_collections_test.go b/internal/api/handlers/library_collections_item_collections_test.go new file mode 100644 index 0000000000..ddc8ca9203 --- /dev/null +++ b/internal/api/handlers/library_collections_item_collections_test.go @@ -0,0 +1,114 @@ +package handlers + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/Silo-Server/silo-server/internal/catalog" + "github.com/Silo-Server/silo-server/internal/models" +) + +// stubItemCollectionIndex is the reverse membership lookup without a database: +// it answers the collections seeded for a membership id and records the id it +// was asked for. +type stubItemCollectionIndex struct { + byMembership map[string][]*models.LibraryCollection + err error + lastMembership string +} + +func (s *stubItemCollectionIndex) ListContainingItem(_ context.Context, mediaItemID string) ([]*models.LibraryCollection, error) { + s.lastMembership = mediaItemID + if s.err != nil { + return nil, s.err + } + return s.byMembership[mediaItemID], nil +} + +// TestItemCollectionsReverseLookup pins the item detail "collections" field's +// service contract: a member of two visible collections lists both, a member +// of none answers an empty slice, and a collection outside the viewer's library +// scope is dropped. The stub index stands in for the SQL repository, so the +// test does not touch a database. Posters resolve through the real collection +// service, which drops a stored poster when it has no artwork resolver. +func TestItemCollectionsReverseLookup(t *testing.T) { + const membership = "movie:dune-1984" + alpha := &models.LibraryCollection{ID: "alpha", Title: "Dune Saga", Visibility: "visible", LibraryIDs: []int{1}, ItemCount: 3} + zeta := &models.LibraryCollection{ID: "zeta", Title: "Sci-Fi", Visibility: "visible", LibraryIDs: []int{2}, ItemCount: 9} + hidden := &models.LibraryCollection{ID: "hidden", Title: "Hidden", Visibility: "hidden", LibraryIDs: []int{1}} + offScope := &models.LibraryCollection{ID: "offscope", Title: "Restricted", Visibility: "visible", LibraryIDs: []int{99}} + + t.Run("member of two collections", func(t *testing.T) { + index := &stubItemCollectionIndex{byMembership: map[string][]*models.LibraryCollection{membership: {alpha, zeta}}} + h := &LibraryCollectionHandler{itemCollectionIndex: index} + got, err := h.ItemCollections(context.Background(), membership, catalog.AccessFilter{AllowedLibraryIDs: []int{1, 2}}) + if err != nil { + t.Fatalf("ItemCollections: %v", err) + } + if index.lastMembership != membership { + t.Fatalf("membership = %q, want %q", index.lastMembership, membership) + } + want := []ItemCollectionView{ + {ID: "alpha", Title: "Dune Saga", ItemCount: 3}, + {ID: "zeta", Title: "Sci-Fi", ItemCount: 9}, + } + if len(got) != len(want) { + t.Fatalf("collections = %+v, want %+v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("collections[%d] = %+v, want %+v", i, got[i], want[i]) + } + } + }) + + t.Run("member of none", func(t *testing.T) { + index := &stubItemCollectionIndex{byMembership: map[string][]*models.LibraryCollection{}} + h := &LibraryCollectionHandler{itemCollectionIndex: index} + got, err := h.ItemCollections(context.Background(), membership, catalog.AccessFilter{}) + if err != nil { + t.Fatalf("ItemCollections: %v", err) + } + if got == nil || len(got) != 0 { + t.Fatalf("collections = %#v, want an empty non-nil slice", got) + } + }) + + t.Run("inaccessible collections excluded", func(t *testing.T) { + // The viewer may see library 1 only. A collection in library 2 and a + // hidden collection are both dropped even though they contain the item. + index := &stubItemCollectionIndex{byMembership: map[string][]*models.LibraryCollection{membership: {alpha, zeta, hidden, offScope}}} + h := &LibraryCollectionHandler{itemCollectionIndex: index} + got, err := h.ItemCollections(context.Background(), membership, catalog.AccessFilter{AllowedLibraryIDs: []int{1}}) + if err != nil { + t.Fatalf("ItemCollections: %v", err) + } + if len(got) != 1 || got[0].ID != "alpha" { + t.Fatalf("collections = %+v, want only alpha", got) + } + }) + + t.Run("empty membership", func(t *testing.T) { + index := &stubItemCollectionIndex{} + h := &LibraryCollectionHandler{itemCollectionIndex: index} + got, err := h.ItemCollections(context.Background(), "", catalog.AccessFilter{}) + if err != nil { + t.Fatalf("ItemCollections: %v", err) + } + if len(got) != 0 || index.lastMembership != "" { + t.Fatalf("collections = %#v, lookup = %q", got, index.lastMembership) + } + }) + + t.Run("lookup failure is an API error", func(t *testing.T) { + index := &stubItemCollectionIndex{err: errors.New("boom")} + h := &LibraryCollectionHandler{itemCollectionIndex: index} + _, err := h.ItemCollections(context.Background(), membership, catalog.AccessFilter{}) + var apiErr *APIError + if !errors.As(err, &apiErr) || apiErr.Status != http.StatusInternalServerError { + t.Fatalf("err = %v, want a 500 APIError", err) + } + }) +} diff --git a/internal/api/handlers/library_views_service.go b/internal/api/handlers/library_views_service.go index e84779b77b..8ab8c4de11 100644 --- a/internal/api/handlers/library_views_service.go +++ b/internal/api/handlers/library_views_service.go @@ -55,6 +55,16 @@ type ItemUserStateView = itemUserStateResponse // CollectionItemView is one catalog item card in a collection listing. type CollectionItemView = itemListResponse +// ItemCollectionView is one collection an item belongs to: the chip the item +// detail "collections" row links to. +type ItemCollectionView struct { + ID string `json:"id"` + Title string `json:"title"` + PosterURL string `json:"poster_url"` + PosterThumbhash string `json:"poster_thumbhash,omitempty"` + ItemCount int `json:"item_count"` +} + // LibraryCollectionTabView is a library's Collections tab. type LibraryCollectionTabView = libraryTabResponse @@ -418,3 +428,48 @@ func (h *LibraryCollectionHandler) LibraryCollectionItems(ctx context.Context, l func collectionSpansLibrary(collection *models.LibraryCollection, libraryID int) bool { return collection.LibraryID == libraryID || slices.Contains(collection.LibraryIDs, libraryID) } + +// ItemCollections answers the reverse membership lookup behind the item +// detail "collections" field: the visible collections that store the member +// content id, as linkable chips. membership is a media item content id (a +// movie or series); the caller resolves an episode or season to its parent +// series first, because collections store movies and series, never episodes. +// A collection outside the viewer's library scope is filtered out, mirroring +// the collection-detail authorization. Poster resolution matches the library +// Collections tab. +func (h *LibraryCollectionHandler) ItemCollections(ctx context.Context, membership string, access catalog.AccessFilter) ([]ItemCollectionView, error) { + if membership == "" { + return []ItemCollectionView{}, nil + } + var index itemCollectionIndex + if h.itemCollectionIndex != nil { + index = h.itemCollectionIndex + } else if h.repo != nil { + index = h.repo + } + if index == nil { + return nil, apiError(http.StatusInternalServerError, "internal_error", "Failed to load item collections") + } + containing, err := index.ListContainingItem(ctx, membership) + if err != nil { + return nil, apiError(http.StatusInternalServerError, "internal_error", "Failed to load item collections") + } + viewable := make([]*models.LibraryCollection, 0, len(containing)) + for _, collection := range containing { + if catalog.CanAccessLibraryCollection(collection, access) { + viewable = append(viewable, collection) + } + } + viewable = h.withViewerPosters(ctx, viewable, access) + out := make([]ItemCollectionView, 0, len(viewable)) + for _, collection := range viewable { + out = append(out, ItemCollectionView{ + ID: collection.ID, + Title: collection.Title, + PosterURL: h.presignGPURLCtx(ctx, collection.PosterURL), + PosterThumbhash: collection.PosterThumbhash, + ItemCount: collection.ItemCount, + }) + } + return out, nil +} diff --git a/internal/apiv2/catalog_items.go b/internal/apiv2/catalog_items.go index 65335d4f0a..355ec51deb 100644 --- a/internal/apiv2/catalog_items.go +++ b/internal/apiv2/catalog_items.go @@ -390,6 +390,20 @@ type CatalogItemDetail struct { Audiobook *catalogpkg.AudiobookDetailExtension `json:"audiobook,omitempty"` Ebook *catalogpkg.EbookDetailExtension `json:"ebook,omitempty"` Manga *catalogpkg.MangaDetailExtension `json:"manga,omitempty"` + // Collections are the visible collections this item belongs to: the + // title page's "Collections" row, each linking to the collection's view. + // Present on every v2 item detail, as an empty array when the item is in + // none. + Collections []ItemCollection `json:"collections" doc:"Visible collections containing this item (movies and series); empty, never null"` +} + +// ItemCollection is one collection a title page lists under Collections. +type ItemCollection struct { + ID string `json:"id" example:"01J9Z8C3W4R5T6Y7U8I9O0P1Q2"` + Title string `json:"title" example:"Dune Saga"` + PosterURL string `json:"poster_url" doc:"Presigned, short-lived; empty when none"` + PosterThumbhash string `json:"poster_thumbhash,omitempty"` + ItemCount int `json:"item_count" example:"3"` } // CatalogRatingSource is one source's rating of an item. @@ -603,6 +617,7 @@ func registerCatalogItems(reg *Registry) { "The episodes of one season of a series by number.")), reg.listSeasonEpisodes) registerCatalogActions(reg) registerRatingsCapability(reg) + registerItemCollectionsCapability(reg) } // --- helpers --- @@ -1116,6 +1131,7 @@ func (reg *Registry) getCatalogItem(ctx context.Context, in *CatalogItemInput) ( return nil, serviceProblem(err) } out := catalogItemDetailOf(detail, reg.ratingSelection(ctx)) + out.Collections = NonNil(reg.itemCollections(ctx, detail, viewer.Access)) if reg.deps.ThemeSongs != nil && (detail.Type == themeOwnerMovie || detail.Type == themeOwnerSeries || detail.Type == themeOwnerSeason || detail.Type == themeOwnerEpisode) { themes, err := reg.deps.ThemeSongs.Discover(ctx, in.ID, true, viewer.Access) if err != nil { @@ -1130,6 +1146,43 @@ func (reg *Registry) getCatalogItem(ctx context.Context, in *CatalogItemInput) ( return &CatalogItemDetailOutput{Body: out}, nil } +// itemCollections answers the item's collections for the detail page. It is +// best-effort: when the index service is not wired, or the lookup fails, the +// result is nil and the detail page still succeeds with an empty row, +// following the theme lookup's non-fatal contract. +// +// Collections store movies and series; an episode contributes its parent +// series id, and a season contributes its series. Other types (audiobooks, +// ebooks) have no collection membership and answer empty. +func (reg *Registry) itemCollections(ctx context.Context, detail *catalogpkg.ItemDetail, access catalogpkg.AccessFilter) []ItemCollection { + index, ok := reg.deps.LibraryCollections.(ItemCollectionIndex) + if !ok || index == nil || detail == nil { + return nil + } + var membership string + switch detail.Type { + case themeOwnerMovie, themeOwnerSeries: + membership = detail.ContentID + case themeOwnerEpisode, themeOwnerSeason: + membership = detail.SeriesID + default: + return nil + } + if membership == "" { + return nil + } + views, err := index.ItemCollections(ctx, membership, access) + if err != nil { + slog.WarnContext(ctx, "catalog item collections lookup failed", "component", "apiv2", "item_id", detail.ContentID, "error", err) + return nil + } + out := make([]ItemCollection, 0, len(views)) + for _, c := range views { + out = append(out, ItemCollection{ID: c.ID, Title: c.Title, PosterURL: c.PosterURL, PosterThumbhash: c.PosterThumbhash, ItemCount: c.ItemCount}) + } + return out +} + func (reg *Registry) listCatalogItemVersions(ctx context.Context, in *CatalogItemInput) (*FileVersionCollectionOutput, error) { svc, p := reg.catalogItems() if p != nil { @@ -1340,7 +1393,7 @@ func catalogItemDetailOf(d *catalogpkg.ItemDetail, sel ratingsources.Selection) FolderPaths: d.FolderPaths, Subtitles: NonNil(d.Subtitles), Intro: d.Intro, Credits: d.Credits, Recap: d.Recap, Preview: d.Preview, EffectiveVersionResolution: d.EffectiveVersionResolution, EffectiveVersionHDR: d.EffectiveVersionHDR, EffectiveVersionCodecVideo: d.EffectiveVersionCodecVideo, EffectiveVersionEditionKey: d.EffectiveVersionEditionKey, - Audiobook: d.Audiobook, Ebook: d.Ebook, Manga: d.Manga, + Audiobook: d.Audiobook, Ebook: d.Ebook, Manga: d.Manga, Collections: []ItemCollection{}, } if d.EffectiveSubtitleTrackSignature != nil { out.EffectiveSubtitleTrackSignature = watchSignatureOf(*d.EffectiveSubtitleTrackSignature) diff --git a/internal/apiv2/catalog_items_test.go b/internal/apiv2/catalog_items_test.go index 71ef1e3b72..0b0cc0d461 100644 --- a/internal/apiv2/catalog_items_test.go +++ b/internal/apiv2/catalog_items_test.go @@ -108,6 +108,10 @@ func (f *fakeCatalog) ItemDetail(_ context.Context, v handlers.ItemViewer, id st return nil, f.err } f.lastViewer = v + if id == "episode:severance-s01e01" { + return &catalogpkg.ItemDetail{ContentID: id, Type: "episode", Title: "Good News About Hell", SeriesID: "series:severance", + Genres: []string{}, Cast: []catalogpkg.CastCredit{}, Crew: []catalogpkg.CrewCredit{}, Versions: []catalogpkg.FileVersion{}, Subtitles: []catalogpkg.SubtitleInfo{}}, nil + } if id != "movie:heat-1995" { return nil, notFoundItem() } @@ -650,3 +654,48 @@ func TestGetCatalogItemScopesVersionsToLibraryWhenEnabled(t *testing.T) { t.Fatalf("viewer = %+v", fake.lastViewer.Access) } } + +// TestGetCatalogItemCarriesCollections pins the additive item detail +// "collections" row: a movie's detail fetches the reverse lookup with its own +// content id, an episode resolves to its parent series, the chips carry +// poster and count, and an item in none answers an empty array, never null. +func TestGetCatalogItemCarriesCollections(t *testing.T) { + deps, _ := catalogDeps(t) + fake := deps.LibraryCollections.(*fakeLibraryViews) + fake.collections = []handlers.ItemCollectionView{{ID: "alpha", Title: "Dune Saga", PosterURL: "https://cdn.example.test/alpha.jpg", ItemCount: 3}} + h := newTestHandler(t, deps) + + rec := do(t, h, http.MethodGet, "/api/v2/catalog/items/movie:heat-1995", "", viewerHeaders()) + if rec.Code != http.StatusOK { + t.Fatal(rec.Code, rec.Body.String()) + } + var movie CatalogItemDetail + decodeJSON(t, rec.Body, &movie) + if fake.lastCollections != "movie:heat-1995" { + t.Fatalf("lookup = %q, want the movie id", fake.lastCollections) + } + if len(movie.Collections) != 1 || movie.Collections[0].ID != "alpha" || movie.Collections[0].Title != "Dune Saga" || + movie.Collections[0].ItemCount != 3 || movie.Collections[0].PosterURL != "https://cdn.example.test/alpha.jpg" { + t.Fatalf("movie collections = %+v", movie.Collections) + } + + rec = do(t, h, http.MethodGet, "/api/v2/catalog/items/episode:severance-s01e01", "", viewerHeaders()) + if rec.Code != http.StatusOK { + t.Fatal(rec.Code, rec.Body.String()) + } + var episode CatalogItemDetail + decodeJSON(t, rec.Body, &episode) + if fake.lastCollections != "series:severance" { + t.Fatalf("episode lookup = %q, want the parent series id", fake.lastCollections) + } + if len(episode.Collections) != 1 || episode.Collections[0].ID != "alpha" { + t.Fatalf("episode collections = %+v", episode.Collections) + } + + // An item in no collection answers an empty array, not null. + fake.collections = nil + rec = do(t, h, http.MethodGet, "/api/v2/catalog/items/movie:heat-1995", "", viewerHeaders()) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"collections":[]`) { + t.Fatalf("empty collections = %s", rec.Body.String()) + } +} diff --git a/internal/apiv2/document_test.go b/internal/apiv2/document_test.go index 886a8a6a92..3da97a3cfb 100644 --- a/internal/apiv2/document_test.go +++ b/internal/apiv2/document_test.go @@ -520,6 +520,7 @@ var libraryViewOperationIDs = []string{ "getCatalogSearchCapabilities", "listCatalogItems", "listAudiobookGroups", "getCatalogFilters", "searchCatalogFacet", "queryCatalogItems", "getCatalogItem", "listCatalogItemEpisodes", "listCatalogItemMangaFiles", "listCatalogItemVersions", "listSeriesSeasons", "getSeriesSeason", "listSeasonEpisodes", "getTrailersCapability", "refreshCatalogItemTrailers", "getMetadataAICapability", "translateCatalogItemDescription", "getRatingsCapability", + "getItemCollectionsCapability", "listPeople", "getPerson", "refreshPerson", "getLiteraryWork", "getLibraryLayout", "listLibrarySections", "getLibrarySectionItems", "getLibraryCollections", "listLibraryUserCollections", } diff --git a/internal/apiv2/fixtures_test.go b/internal/apiv2/fixtures_test.go index bb952bf32e..cc19b1e86c 100644 --- a/internal/apiv2/fixtures_test.go +++ b/internal/apiv2/fixtures_test.go @@ -1743,7 +1743,9 @@ func fixtureCases() []fixtureCase { cases = append(cases, watchTrickplayFixtureCases()...) cases = append(cases, adminTrickplayFixtureCases()...) cases = append(cases, deviceSignInFixtureCases()...) - return append(cases, externalSignInFixtureCases()...) + cases = append(cases, externalSignInFixtureCases()...) + cases = append(cases, itemCollectionsCapabilityFixtureCases()...) + return cases } // deviceSignInFixtureCases covers the TV sign-in additions: the opened @@ -1854,7 +1856,9 @@ func fixtureDeps() Dependencies { deps.PermissionGates[policy.PermissionMetadataCuration] = adminTranslationGate deps.LibraryJobs = &fixtureAdminCollectionJobs{fakeLibraryJobs: *deps.LibraryJobs.(*fakeLibraryJobs)} deps.LibrarySections = &fakeLibraryViews{} - deps.LibraryCollections = &fakeLibraryViews{} + deps.LibraryCollections = &fakeLibraryViews{collections: []handlers.ItemCollectionView{{ + ID: "oscar-winners", Title: "Oscar Winners", PosterURL: "https://cdn.example.invalid/oscar-winners.jpg", ItemCount: 12, + }}} home := &fakeHome{} deps.Calendar = home deps.HomeDismissals = home diff --git a/internal/apiv2/item_collections_capability.go b/internal/apiv2/item_collections_capability.go new file mode 100644 index 0000000000..501647d175 --- /dev/null +++ b/internal/apiv2/item_collections_capability.go @@ -0,0 +1,39 @@ +package apiv2 + +import ( + "context" + "net/http" +) + +// ItemCollectionsCapability reports that item detail carries the collections +// row: the visible collections the item belongs to. +type ItemCollectionsCapability struct { + Capability +} + +// ItemCollectionsCapabilityOutput is the getItemCollectionsCapability response. +type ItemCollectionsCapabilityOutput struct { + Status int + ETag string `header:"ETag"` + CacheControl string `header:"Cache-Control"` + Body ItemCollectionsCapability +} + +func registerItemCollectionsCapability(reg *Registry) { + Register(reg, viewerOperation(humaOp(http.MethodGet, Prefix+"/capabilities/item-collections", "getItemCollectionsCapability", "catalog", + "Whether item detail carries the collections an item belongs to.")), reg.getItemCollectionsCapability) +} + +// getItemCollectionsCapability answers available when the assembled server +// wires the collection index behind item detail, and unsupported when it does +// not. Route presence never changes with configuration. +func (reg *Registry) getItemCollectionsCapability(_ context.Context, _ *CapabilityInput) (*ItemCollectionsCapabilityOutput, error) { + state := StateUnsupported + if _, ok := reg.deps.LibraryCollections.(ItemCollectionIndex); ok { + state = StateAvailable + } + return &ItemCollectionsCapabilityOutput{ + CacheControl: cacheControlPrivateNoCache, + Body: ItemCollectionsCapability{Capability: Capability{State: state}}, + }, nil +} diff --git a/internal/apiv2/item_collections_capability_fixtures_test.go b/internal/apiv2/item_collections_capability_fixtures_test.go new file mode 100644 index 0000000000..da93f370ca --- /dev/null +++ b/internal/apiv2/item_collections_capability_fixtures_test.go @@ -0,0 +1,18 @@ +package apiv2 + +import "net/http" + +func itemCollectionsCapabilityFixtureCases() []fixtureCase { + viewer := with(bearer(memberToken), "X-Profile-Id", "p-owner") + problem := "#/components/schemas/Problem" + return []fixtureCase{ + {name: "get_item_collections_capability_ok", operationID: "getItemCollectionsCapability", + scenario: "The item-collections capability document on a server whose item detail carries the collections row.", + method: http.MethodGet, path: "/api/v2/capabilities/item-collections", headers: viewer, + status: http.StatusOK, assertHeaders: []string{"Content-Type", "Cache-Control"}, schema: "#/components/schemas/ItemCollectionsCapability"}, + {name: "get_item_collections_capability_authentication_required", operationID: "getItemCollectionsCapability", + scenario: "The item-collections capability document without a credential.", + method: http.MethodGet, path: "/api/v2/capabilities/item-collections", + status: http.StatusUnauthorized, assertHeaders: []string{"Content-Type", "Cache-Control"}, schema: problem}, + } +} diff --git a/internal/apiv2/item_collections_capability_test.go b/internal/apiv2/item_collections_capability_test.go new file mode 100644 index 0000000000..6519878c04 --- /dev/null +++ b/internal/apiv2/item_collections_capability_test.go @@ -0,0 +1,52 @@ +package apiv2 + +import ( + "context" + "net/http" + "testing" + + "github.com/Silo-Server/silo-server/internal/api/handlers" + "github.com/Silo-Server/silo-server/internal/usercollections" +) + +// TestGetItemCollectionsCapabilityState: the capability reports available when +// the collection index is wired behind item detail, and unsupported when it is +// not. Route presence never changes with configuration. +func TestGetItemCollectionsCapabilityState(t *testing.T) { + deps, _ := catalogDeps(t) + rec := do(t, newTestHandler(t, deps), http.MethodGet, "/api/v2/capabilities/item-collections", "", viewerHeaders()) + if rec.Code != http.StatusOK { + t.Fatal(rec.Code, rec.Body.String()) + } + var body ItemCollectionsCapability + decodeJSON(t, rec.Body, &body) + if body.State != StateAvailable || body.Allowed == nil || !*body.Allowed || body.Revision == "" { + t.Fatalf("capability = %+v", body) + } + + // A service without the collection index answers unsupported rather than + // dropping the route. + deps.LibraryCollections = collectionServiceWithoutIndex{} + rec = do(t, newTestHandler(t, deps), http.MethodGet, "/api/v2/capabilities/item-collections", "", viewerHeaders()) + if rec.Code != http.StatusOK { + t.Fatal(rec.Code, rec.Body.String()) + } + body = ItemCollectionsCapability{} + decodeJSON(t, rec.Body, &body) + if body.State != StateUnsupported { + t.Fatalf("capability = %+v", body) + } +} + +// collectionServiceWithoutIndex satisfies LibraryCollectionService but not +// ItemCollectionIndex, standing in for a server whose item detail carries no +// collections row. +type collectionServiceWithoutIndex struct{} + +func (collectionServiceWithoutIndex) LibraryCollectionsTab(context.Context, int, int, string) (handlers.LibraryCollectionTabView, error) { + return handlers.LibraryCollectionTabView{}, nil +} + +func (collectionServiceWithoutIndex) LibraryUserCollections(context.Context, int, int, string) ([]usercollections.ServerVisibleCollection, error) { + return nil, nil +} diff --git a/internal/apiv2/library_views_test.go b/internal/apiv2/library_views_test.go index bb34da2a29..6f03b6d03c 100644 --- a/internal/apiv2/library_views_test.go +++ b/internal/apiv2/library_views_test.go @@ -11,6 +11,7 @@ import ( "github.com/Silo-Server/silo-server/internal/adminjob" "github.com/Silo-Server/silo-server/internal/api/handlers" + catalogpkg "github.com/Silo-Server/silo-server/internal/catalog" "github.com/Silo-Server/silo-server/internal/models" "github.com/Silo-Server/silo-server/internal/usercollections" ) @@ -121,6 +122,17 @@ func (f *fakeLibraryAdmin) SetLibraryProviders(_ context.Context, id int, levels type fakeLibraryViews struct { err error + // collections backs ItemCollections; empty answers an empty slice. + collections []handlers.ItemCollectionView + lastCollections string +} + +func (f *fakeLibraryViews) ItemCollections(_ context.Context, membership string, _ catalogpkg.AccessFilter) ([]handlers.ItemCollectionView, error) { + if f.err != nil { + return nil, f.err + } + f.lastCollections = membership + return f.collections, nil } func fakeCard() handlers.SectionItemView { diff --git a/internal/apiv2/router.go b/internal/apiv2/router.go index c8d59b6393..8ef366db1c 100644 --- a/internal/apiv2/router.go +++ b/internal/apiv2/router.go @@ -962,6 +962,14 @@ type LibraryCollectionService interface { LibraryUserCollections(ctx context.Context, libraryID, userID int, profileID string) ([]usercollections.ServerVisibleCollection, error) } +// ItemCollectionIndex is the reverse membership lookup item detail adds as +// its "collections" field. It is a narrow type assertion on the same +// *handlers.LibraryCollectionHandler, so adding the capability does not widen +// LibraryCollectionService for every existing fake. +type ItemCollectionIndex interface { + ItemCollections(ctx context.Context, membership string, access mediacatalog.AccessFilter) ([]handlers.ItemCollectionView, error) +} + // CalendarService is the slice of *handlers.CalendarHandler getCalendar uses. type CalendarService interface { Calendar(ctx context.Context, q handlers.CalendarQuery, access mediacatalog.AccessFilter) (handlers.CalendarView, error) From 43b654c6b08c99eb2ea96d12bd043cae9c641df7 Mon Sep 17 00:00:00 2001 From: randrini Date: Wed, 7 Oct 2026 12:28:36 +0200 Subject: [PATCH 2/4] feat(web): show an item's collections on the title page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v2 item detail now carries a `collections` array — the visible collections a movie or series belongs to, each with id, title, poster_url and item_count. Render it as a Collections row on the movie and series detail pages: one poster chip per collection (poster, title, item count) linking to that collection's browse view, using the shared MediaCarousel so the row scrolls and sizes like the neighboring poster rows. The membership list arrives on the detail payload itself, so the row has no fetch of its own: the page's existing skeleton covers loading and its error state covers a failed read, and a title in no collections renders no row at all. The chip links through buildLibraryCollectionCatalogHref, the same href the sidebar and Collections tab use for a library collection. Regenerate the committed v2 web bindings so the new detail member and the item-collections capability are typed, and pin the mapper with a test: catalogItemDetailFromV2 builds ItemDetail field by field, so a member it forgets to copy would drop silently and the row would never render. --- web/src/api/types.ts | 19 ++ web/src/api/v2/catalog.ts | 1 + web/src/api/v2/catalogCollections.test.ts | 34 ++++ web/src/api/v2/operations.ts | 1 + web/src/api/v2/schema.ts | 176 ++++++++++++++++++ .../pages/ItemDetail/MovieContent.test.tsx | 34 ++++ web/src/pages/ItemDetail/MovieContent.tsx | 3 + .../pages/ItemDetail/SeriesContent.test.tsx | 38 ++++ web/src/pages/ItemDetail/SeriesContent.tsx | 3 + .../components/CollectionsSection.test.tsx | 83 +++++++++ .../components/CollectionsSection.tsx | 61 ++++++ 11 files changed, 453 insertions(+) create mode 100644 web/src/api/v2/catalogCollections.test.ts create mode 100644 web/src/pages/ItemDetail/components/CollectionsSection.test.tsx create mode 100644 web/src/pages/ItemDetail/components/CollectionsSection.tsx diff --git a/web/src/api/types.ts b/web/src/api/types.ts index c97181fc4a..bfb93c6113 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -1175,6 +1175,19 @@ export interface ItemVideo { is_official: boolean; } +/** + * One collection a title page lists under its Collections row: the visible + * collections this item belongs to, as the v2 item detail reports them. + */ +export interface ItemCollection { + id: string; + title: string; + /** Presigned, short-lived; empty when the collection has no poster. */ + poster_url: string; + poster_thumbhash?: string; + item_count: number; +} + /** Local extras file attached to an item; content_id is a watchable target. */ export interface ItemExtra { content_id: string; @@ -1305,6 +1318,12 @@ export interface ItemDetail { audiobook?: AudiobookDetailExtension; ebook?: EbookDetailExtension; manga?: MangaDetailExtension; + /** + * The visible collections this title belongs to. Present on every v2 item + * detail as an empty array when the item is in none; optional here so + * hand-built fixtures that predate the field still typecheck. + */ + collections?: ItemCollection[]; } export interface WatchDetail { diff --git a/web/src/api/v2/catalog.ts b/web/src/api/v2/catalog.ts index d53830c852..792f6c44c1 100644 --- a/web/src/api/v2/catalog.ts +++ b/web/src/api/v2/catalog.ts @@ -336,6 +336,7 @@ export function catalogItemDetailFromV2(item: CatalogItemDetailV2): ItemDetail { audiobook: item.audiobook, ebook: item.ebook, manga: item.manga, + collections: item.collections, }; } diff --git a/web/src/api/v2/catalogCollections.test.ts b/web/src/api/v2/catalogCollections.test.ts new file mode 100644 index 0000000000..4a64a52ce3 --- /dev/null +++ b/web/src/api/v2/catalogCollections.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; + +import itemFixture from "../../../../contracts/api/v2/fixtures/get_catalog_item_ok.json"; +import { catalogItemDetailFromV2 } from "@/api/v2/catalog"; + +// catalogItemDetailFromV2 builds ItemDetail field by field, so a contract +// member the mapper forgets to copy disappears silently: types compile, the +// API returns the row, and the detail page renders nothing. The Collections +// row is pinned here for the same reason the advisory badge is. +describe("catalogItemDetailFromV2 collections", () => { + it("carries the item's collections through the mapper", () => { + const detail = catalogItemDetailFromV2( + itemFixture as Parameters[0], + ); + + expect(detail.collections).toEqual([ + { + id: "oscar-winners", + title: "Oscar Winners", + poster_url: "https://cdn.example.invalid/oscar-winners.jpg", + item_count: 12, + }, + ]); + }); + + it("keeps an empty memberships list empty rather than dropping it", () => { + const detail = catalogItemDetailFromV2({ + ...(itemFixture as Parameters[0]), + collections: [], + }); + + expect(detail.collections).toEqual([]); + }); +}); diff --git a/web/src/api/v2/operations.ts b/web/src/api/v2/operations.ts index c2fecf3232..c5780368c4 100644 --- a/web/src/api/v2/operations.ts +++ b/web/src/api/v2/operations.ts @@ -274,6 +274,7 @@ export const v2Operations = { "GET /api/v2/capabilities/downloads": "getDownloadCapability", "GET /api/v2/capabilities/ebooks": "getEbookCapability", "GET /api/v2/capabilities/episode-release": "getEpisodeReleaseCapability", + "GET /api/v2/capabilities/item-collections": "getItemCollectionsCapability", "GET /api/v2/capabilities/metadata-ai": "getMetadataAICapability", "GET /api/v2/capabilities/password-reset": "getPasswordResetCapability", "GET /api/v2/capabilities/ratings": "getRatingsCapability", diff --git a/web/src/api/v2/schema.ts b/web/src/api/v2/schema.ts index 1de6518385..e8f2152829 100644 --- a/web/src/api/v2/schema.ts +++ b/web/src/api/v2/schema.ts @@ -5800,6 +5800,23 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v2/capabilities/item-collections": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Whether item detail carries the collections an item belongs to. */ + get: operations["getItemCollectionsCapability"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v2/capabilities/metadata-ai": { parameters: { query?: never; @@ -18740,6 +18757,8 @@ export interface components { badges?: string[]; /** @description Empty, never null */ cast: components["schemas"]["CastCredit"][]; + /** @description Visible collections containing this item (movies and series); empty, never null */ + collections: components["schemas"]["ItemCollection"][]; /** * @description Deterministic catalog identifier * @example movie:heat-1995 @@ -22002,6 +22021,31 @@ export interface components { server_name: string; show_tour: boolean; }; + ItemCollection: { + /** @example 01J9Z8C3W4R5T6Y7U8I9O0P1Q2 */ + id: string; + /** + * Format: int64 + * @example 3 + */ + item_count: number; + poster_thumbhash?: string; + /** @description Presigned, short-lived; empty when none */ + poster_url: string; + /** @example Dune Saga */ + title: string; + }; + ItemCollectionsCapability: { + /** @description Whether the current principal may use the capability */ + allowed: boolean; + /** @description Opaque revision of this document */ + revision: string; + /** + * @description Support and configuration state, not health + * @enum {string} + */ + state: "available" | "disabled" | "not_configured" | "unsupported"; + }; ItemExtraInfo: { content_id: string; /** Format: int64 */ @@ -81537,6 +81581,138 @@ export interface operations { }; }; }; + getItemCollectionsCapability: { + parameters: { + query?: never; + header: { + /** @description Optional first precondition, evaluated before If-None-Match: a tag that does not match the current representation is 412 precondition_failed. */ + "If-Match"?: string; + "If-None-Match"?: string; + /** @description The household profile acting for this request; it must belong to the authenticated account. */ + "X-Profile-Id": string; + /** @description Verification proof for a PIN-locked profile, issued by POST /api/v2/profiles/{id}/verify-pin; required only when the declared profile is locked */ + "X-Profile-Token"?: string; + }; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + "Cache-Control"?: string; + /** @description The strong, opaque validator of the representation; send it back in If-Match on a guarded mutation or If-None-Match on a conditional read. */ + ETag?: string; + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ItemCollectionsCapability"]; + }; + }; + /** @description The representation named by If-None-Match is current; no body. */ + 304: { + headers: { + /** @description The strong, opaque validator of the representation; send it back in If-Match on a guarded mutation or If-None-Match on a conditional read. */ + ETag?: string; + [name: string]: unknown; + }; + content?: never; + }; + /** @description Bad Request */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Unauthorized */ + 401: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Forbidden */ + 403: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Not Found */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Not Acceptable */ + 406: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Precondition Failed */ + 412: { + headers: { + /** @description The strong, opaque validator of the representation; send it back in If-Match on a guarded mutation or If-None-Match on a conditional read. */ + ETag?: string; + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Unprocessable Entity */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Too Many Requests */ + 429: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Internal Server Error */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + /** @description Service Unavailable */ + 503: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["Problem"]; + }; + }; + }; + }; getMetadataAICapability: { parameters: { query?: never; diff --git a/web/src/pages/ItemDetail/MovieContent.test.tsx b/web/src/pages/ItemDetail/MovieContent.test.tsx index 03946cd635..7c1e81e9cf 100644 --- a/web/src/pages/ItemDetail/MovieContent.test.tsx +++ b/web/src/pages/ItemDetail/MovieContent.test.tsx @@ -573,4 +573,38 @@ describe("MovieContent", () => { expect(firstCall?.[0]).toEqual([expect.objectContaining({ file_id: 2 })]); expect(firstCall?.[1]).toBe(true); }); + + it("renders a Collections row when the movie belongs to collections", () => { + const markup = renderToStaticMarkup( + + + , + ); + + expect(markup).toContain("Oscar Winners"); + expect(markup).toContain("12 items"); + expect(markup).toContain("collection_id=oscar-winners"); + }); + + it("renders no Collections row when the movie is in no collections", () => { + const markup = renderToStaticMarkup( + + + , + ); + + expect(markup).not.toContain("collection_id="); + expect(markup).not.toContain("Collections"); + }); }); diff --git a/web/src/pages/ItemDetail/MovieContent.tsx b/web/src/pages/ItemDetail/MovieContent.tsx index a53f57275c..272673ea59 100644 --- a/web/src/pages/ItemDetail/MovieContent.tsx +++ b/web/src/pages/ItemDetail/MovieContent.tsx @@ -27,6 +27,7 @@ import { useOnViewTranslation } from "@/hooks/useOnViewTranslation"; import MetadataBadges from "./components/MetadataBadges"; import TrailersSection from "./components/TrailersSection"; import ExtrasSection from "./components/ExtrasSection"; +import CollectionsSection from "./components/CollectionsSection"; import QualityBadges from "./components/QualityBadges"; import ScoreRow from "./components/ScoreRow"; import HeroCrewLine from "./components/HeroCrewLine"; @@ -454,6 +455,8 @@ export default function MovieContent({ {item.crew && item.crew.length > 0 && } + + {/* More Like This */} {similarLoading ? ( diff --git a/web/src/pages/ItemDetail/SeriesContent.test.tsx b/web/src/pages/ItemDetail/SeriesContent.test.tsx index f81e05195c..efcd756ee8 100644 --- a/web/src/pages/ItemDetail/SeriesContent.test.tsx +++ b/web/src/pages/ItemDetail/SeriesContent.test.tsx @@ -306,4 +306,42 @@ describe("SeriesContent", () => { expect(mocks.setRatingMutate).toHaveBeenCalledWith(5); expect(mocks.deleteRatingMutate).toHaveBeenCalledTimes(1); }); + + it("renders a Collections row when the series belongs to collections", () => { + const markup = renderToStaticMarkup( + + + + + , + ); + + expect(markup).toContain("Prestige Drama"); + expect(markup).toContain("4 items"); + expect(markup).toContain("collection_id=prestige-drama"); + }); + + it("renders no Collections row when the series is in no collections", () => { + const markup = renderToStaticMarkup( + + + + + , + ); + + expect(markup).not.toContain("collection_id="); + expect(markup).not.toContain("Collections"); + }); }); diff --git a/web/src/pages/ItemDetail/SeriesContent.tsx b/web/src/pages/ItemDetail/SeriesContent.tsx index e656c39351..bf9d41051b 100644 --- a/web/src/pages/ItemDetail/SeriesContent.tsx +++ b/web/src/pages/ItemDetail/SeriesContent.tsx @@ -27,6 +27,7 @@ import SeasonEpisodeGrid from "./components/SeasonEpisodeGrid"; import MetadataBadges from "./components/MetadataBadges"; import TrailersSection from "./components/TrailersSection"; import ExtrasSection from "./components/ExtrasSection"; +import CollectionsSection from "./components/CollectionsSection"; import ScoreRow from "./components/ScoreRow"; import HeroCrewLine from "./components/HeroCrewLine"; import MediaUserActionBar from "./components/MediaUserActionBar"; @@ -253,6 +254,8 @@ export default function SeriesContent({ )} {item.crew && item.crew.length > 0 && } + + {similarLoading ? ( ) : ( diff --git a/web/src/pages/ItemDetail/components/CollectionsSection.test.tsx b/web/src/pages/ItemDetail/components/CollectionsSection.test.tsx new file mode 100644 index 0000000000..769255cbd2 --- /dev/null +++ b/web/src/pages/ItemDetail/components/CollectionsSection.test.tsx @@ -0,0 +1,83 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { MemoryRouter } from "react-router"; +import { describe, expect, it } from "vitest"; + +import { UICustomizationContext } from "@/contexts/uiCustomizationContext"; +import type { ItemCollection } from "@/api/types"; +import CollectionsSection from "./CollectionsSection"; + +const COLLECTION: ItemCollection = { + id: "oscar-winners", + title: "Oscar Winners", + poster_url: "/collections/oscar-winners.jpg", + poster_thumbhash: "abc123", + item_count: 12, +}; + +function render(collections?: ItemCollection[], posterSize: "compact" | "standard" = "standard") { + return renderToStaticMarkup( + + + + + , + ); +} + +describe("CollectionsSection", () => { + it("renders a chip per collection with its poster, title, and item count", () => { + const markup = render([COLLECTION]); + + expect(markup).toContain('src="/collections/oscar-winners.jpg"'); + expect(markup).toContain("Oscar Winners"); + expect(markup).toContain("12 items"); + expect(markup).toContain("embla__viewport"); + }); + + it("pluralizes the item count for a single-item collection", () => { + const markup = render([{ ...COLLECTION, item_count: 1 }]); + + expect(markup).toContain("1 item"); + expect(markup).not.toContain("1 items"); + }); + + it("links each chip to the collection's browse view", () => { + const markup = render([ + COLLECTION, + { ...COLLECTION, id: "new-releases", title: "New Releases" }, + ]); + + expect(markup).toContain( + 'href="/catalog?source=library_collection&collection_id=oscar-winners&title=Oscar+Winners"', + ); + expect(markup).toContain( + 'href="/catalog?source=library_collection&collection_id=new-releases&title=New+Releases"', + ); + }); + + it("renders nothing when the item is in no collections", () => { + expect(render([])).toBe(""); + expect(render(undefined)).toBe(""); + }); + + it("sizes the chips with the viewer's poster density", () => { + expect(render([COLLECTION], "compact")).toContain( + "w-[120px] shrink-0 sm:w-[140px] lg:w-[160px]", + ); + expect(render([COLLECTION], "standard")).toContain( + "w-[140px] shrink-0 sm:w-[160px] lg:w-[185px]", + ); + }); +}); diff --git a/web/src/pages/ItemDetail/components/CollectionsSection.tsx b/web/src/pages/ItemDetail/components/CollectionsSection.tsx new file mode 100644 index 0000000000..844da7ebf5 --- /dev/null +++ b/web/src/pages/ItemDetail/components/CollectionsSection.tsx @@ -0,0 +1,61 @@ +import type { ItemCollection } from "@/api/types"; +import MediaCardArtwork from "@/components/MediaCardArtwork"; +import MediaCarousel from "@/components/MediaCarousel"; +import ViewTransitionLink from "@/components/ViewTransitionLink"; +import { useUICustomization } from "@/hooks/useUICustomization"; +import { carouselCardWidthClasses } from "@/lib/uiCustomization"; +import { buildLibraryCollectionCatalogHref } from "@/pages/catalogSearchParams"; + +interface CollectionsSectionProps { + collections?: ItemCollection[]; +} + +/** + * The "Collections" row on a movie or series page: one chip per collection the + * title belongs to, each opening that collection's browse view. The membership + * list arrives on the item detail itself, so the row has no fetch of its own — + * the page's loading and error states already cover it — and a title in no + * collections renders no row. + */ +export default function CollectionsSection({ collections }: CollectionsSectionProps) { + const { cardPresentation } = useUICustomization(); + + if (!collections || collections.length === 0) return null; + + const posterWidthClasses = carouselCardWidthClasses(cardPresentation.poster_size); + + return ( + + {collections.map((collection) => ( +
+ +
+ ))} + + ); +} + +function CollectionChip({ collection }: { collection: ItemCollection }) { + const href = buildLibraryCollectionCatalogHref(collection.id, collection.title); + const countLabel = `${collection.item_count} ${collection.item_count === 1 ? "item" : "items"}`; + + return ( + +
+ +
+
+
+ {collection.title} +
+
{countLabel}
+
+ + ); +} From 20e19eeb3e36d80fb33b6da142f18a8f141a5ed6 Mon Sep 17 00:00:00 2001 From: randrini Date: Wed, 7 Oct 2026 13:46:13 +0200 Subject: [PATCH 3/4] chore(api): regenerate media route manifest for item-collections The #230 item-collections capability route was added to the v2 router without regenerating internal/api/testdata/media_routes.txt, so TestMediaRouteManifest failed in CI. Regenerate it: the only change is the new GET /api/v2/capabilities/item-collections entry, once for each fixture router. --- internal/api/testdata/media_routes.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/api/testdata/media_routes.txt b/internal/api/testdata/media_routes.txt index 537e7ac0a1..f6ef8ea57c 100644 --- a/internal/api/testdata/media_routes.txt +++ b/internal/api/testdata/media_routes.txt @@ -425,6 +425,7 @@ GET /api/v2/calendar non-media GET /api/v2/capabilities/downloads non-media GET /api/v2/capabilities/ebooks non-media GET /api/v2/capabilities/episode-release non-media +GET /api/v2/capabilities/item-collections non-media GET /api/v2/capabilities/metadata-ai non-media GET /api/v2/capabilities/password-reset non-media GET /api/v2/capabilities/ratings non-media @@ -1660,6 +1661,7 @@ GET /api/v2/calendar non-media GET /api/v2/capabilities/downloads non-media GET /api/v2/capabilities/ebooks non-media GET /api/v2/capabilities/episode-release non-media +GET /api/v2/capabilities/item-collections non-media GET /api/v2/capabilities/metadata-ai non-media GET /api/v2/capabilities/password-reset non-media GET /api/v2/capabilities/ratings non-media From 8da71b62363e70caf732359ebd9df79bf9a7b956 Mon Sep 17 00:00:00 2001 From: randrini Date: Wed, 7 Oct 2026 15:38:39 +0200 Subject: [PATCH 4/4] fix(catalog): carry item collections on the metadata save response The metadata editor's PATCH response built its detail through the shared mapper, whose `collections` member defaults to empty, so editing an item that belongs to a collection returned `"collections":[]` until the client refetched the read detail. Populate the row on that path with the access scope the permission gate resolved, the same lookup the read detail uses. Correct the documented and commented semantics: the field reports stored server-collection memberships only (a smart collection derives its members from its query and stores no rows; personal collections are a separate surface), and `item_count` is the collection's total, not the viewer-visible count. Document `poster_thumbhash` and the metadata-save row. Strengthen the tests where the prior coverage was thin: assert the resolved access filter reaches the reverse lookup, cover season and series mapping, and cover the lookup-failure fallback. Add a Postgres-backed handler test for the real SQL that skips without SILO_TEST_DATABASE_URL. --- .../fixtures/admin_item_metadata_updated.json | 9 +- docs/catalog-api.md | 28 +++- ...brary_collections_item_collections_test.go | 153 ++++++++++++++++++ internal/apiv2/admin_catalog_item_metadata.go | 8 +- .../apiv2/admin_catalog_item_metadata_test.go | 36 +++++ internal/apiv2/catalog_items.go | 7 +- internal/apiv2/catalog_items_test.go | 55 ++++++- internal/apiv2/library_views_test.go | 11 +- 8 files changed, 290 insertions(+), 17 deletions(-) diff --git a/contracts/api/v2/fixtures/admin_item_metadata_updated.json b/contracts/api/v2/fixtures/admin_item_metadata_updated.json index 9a636aeb93..ec68bb4fa4 100644 --- a/contracts/api/v2/fixtures/admin_item_metadata_updated.json +++ b/contracts/api/v2/fixtures/admin_item_metadata_updated.json @@ -10,5 +10,12 @@ "versions": [], "ratings": [], "subtitles": [], - "collections": [] + "collections": [ + { + "id": "oscar-winners", + "title": "Oscar Winners", + "poster_url": "https://cdn.example.invalid/oscar-winners.jpg", + "item_count": 12 + } + ] } diff --git a/docs/catalog-api.md b/docs/catalog-api.md index 0e92ef27a4..5403bce684 100644 --- a/docs/catalog-api.md +++ b/docs/catalog-api.md @@ -574,15 +574,29 @@ server collections the item belongs to. Each entry has: - `title`: the collection's display name. - `poster_url`: the collection's poster, presigned and short-lived, empty when it has none. -- `item_count`: the collection's member count. +- `poster_thumbhash`: the poster's thumbhash, absent when the collection has no + poster. +- `item_count`: the collection's stored member total. The list is present on every v2 item detail as an array, empty when the item is -in none. Only movies and series can be members, so an episode detail answers -its parent series' collections and a season detail answers its series'. A -collection the viewer's library scope cannot reach, and a hidden collection, -are never listed. The `poster_url` follows the same viewer rule as the library -Collections tab: an uploaded or template poster is shared, while a generated -collage shows only members the viewer can access. +in none and when the membership lookup fails. It lists stored server-collection +memberships only: a smart collection derives its members from its query at read +time and stores no rows, so it is never listed, and personal (user) collections +are a separate surface this field does not include. `item_count` is the +collection's total membership, not the members the viewer may see. + +The membership key is the item's own id for a movie or series. An episode detail +answers its parent series' collections and a season detail answers its series', +so both report the series' memberships rather than their own; a title with no +series, such as an audiobook or ebook, answers empty. A collection the viewer's +library scope cannot reach, and a hidden collection, are never listed. +`poster_url` follows the same viewer rule as the library Collections tab: an +uploaded or template poster is shared, while a generated collage shows only +members the viewer can access. + +`PATCH /api/v2/admin/items/{id}/metadata` returns the same `collections` row on +its detail body, so a metadata save does not appear to drop the item's +memberships. `GET /api/v2/capabilities/item-collections` is the feature-detection document. It answers `state: available` on a server whose item detail carries diff --git a/internal/api/handlers/library_collections_item_collections_test.go b/internal/api/handlers/library_collections_item_collections_test.go index ddc8ca9203..fa37f291db 100644 --- a/internal/api/handlers/library_collections_item_collections_test.go +++ b/internal/api/handlers/library_collections_item_collections_test.go @@ -3,8 +3,15 @@ package handlers import ( "context" "errors" + "fmt" "net/http" + "os" + "slices" + "strings" "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" "github.com/Silo-Server/silo-server/internal/catalog" "github.com/Silo-Server/silo-server/internal/models" @@ -112,3 +119,149 @@ func TestItemCollectionsReverseLookup(t *testing.T) { } }) } + +// itemCollectionsTestPool mirrors the repo's Postgres test pattern: it skips +// unless SILO_TEST_DATABASE_URL names a disposable database, and refuses a +// database whose name does not look like a test/purge fixture. +func itemCollectionsTestPool(t *testing.T) *pgxpool.Pool { + t.Helper() + dsn := os.Getenv("SILO_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("SILO_TEST_DATABASE_URL is not set") + } + ctx := context.Background() + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatalf("connect test database: %v", err) + } + var databaseName string + if err := pool.QueryRow(ctx, "SELECT current_database()").Scan(&databaseName); err != nil { + t.Fatalf("identify test database: %v", err) + } + if !strings.Contains(strings.ToLower(databaseName), "test") && !strings.Contains(strings.ToLower(databaseName), "purge") { + t.Fatalf("refusing destructive handler fixture database %q", databaseName) + } + t.Cleanup(pool.Close) + return pool +} + +// TestItemCollectionsReverseLookupDB exercises the real SQL behind the handler: +// the item detail "collections" field answers visible stored memberships the +// viewer's library scope can reach, drops hidden and out-of-scope collections, +// reports the global item_count, and leaves a smart (live-query) collection out +// because it stores no membership rows. +func TestItemCollectionsReverseLookupDB(t *testing.T) { + pool := itemCollectionsTestPool(t) + ctx := context.Background() + suffix := time.Now().UnixNano() + slug := func(name string) string { return fmt.Sprintf("%s-%d", name, suffix) } + + var visibleLibrary, otherLibrary int + if err := pool.QueryRow(ctx, `INSERT INTO media_folders (type, name, enabled) VALUES ('movies', $1, TRUE) RETURNING id`, slug("ic-visible")).Scan(&visibleLibrary); err != nil { + t.Fatalf("seed visible library: %v", err) + } + if err := pool.QueryRow(ctx, `INSERT INTO media_folders (type, name, enabled) VALUES ('movies', $1, TRUE) RETURNING id`, slug("ic-other")).Scan(&otherLibrary); err != nil { + t.Fatalf("seed other library: %v", err) + } + t.Cleanup(func() { + _, _ = pool.Exec(context.Background(), `DELETE FROM media_folders WHERE id = ANY($1)`, []int{visibleLibrary, otherLibrary}) + }) + + member := slug("ic-member") + companion := slug("ic-companion") + for _, item := range []struct{ id, title string }{{member, "Member"}, {companion, "Companion"}} { + if _, err := pool.Exec(ctx, `INSERT INTO media_items (content_id, type, title, sort_title, year, genres) VALUES ($1, 'movie', $2, $2, 2001, '{}'::text[])`, item.id, item.title); err != nil { + t.Fatalf("seed item %s: %v", item.id, err) + } + } + t.Cleanup(func() { + _, _ = pool.Exec(context.Background(), `DELETE FROM media_items WHERE content_id = ANY($1)`, []string{member, companion}) + }) + + repo := catalog.NewLibraryCollectionRepository(pool) + itemRepo := catalog.NewItemRepository(pool) + service := catalog.NewLibraryCollectionService(repo, itemRepo, nil, nil) + h := &LibraryCollectionHandler{repo: repo, service: service} + + create := func(name, title, visibility string, libraryID int, items ...string) string { + t.Helper() + collection, err := repo.Create(ctx, catalog.CreateLibraryCollectionInput{ + LibraryID: libraryID, LibraryIDs: []int{libraryID}, Slug: slug(name), Title: title, + CollectionType: "manual", Visibility: visibility, + }) + if err != nil { + t.Fatalf("create %s: %v", name, err) + } + t.Cleanup(func() { _ = repo.Delete(context.Background(), collection.ID) }) + inputs := make([]catalog.LibraryCollectionItemInput, 0, len(items)) + for _, id := range items { + inputs = append(inputs, catalog.LibraryCollectionItemInput{MediaItemID: id}) + } + if err := repo.ReplaceItems(ctx, collection.ID, inputs); err != nil { + t.Fatalf("replace items %s: %v", name, err) + } + return collection.ID + } + + visible := create("visible", "Visible Set", "visible", visibleLibrary, member, companion) + hidden := create("hidden", "Hidden Set", "hidden", visibleLibrary, member) + offScope := create("offscope", "Off Scope Set", "visible", otherLibrary, member) + + // A smart collection derives its members from its query and stores no + // membership rows, so ListContainingItem never reports it. + smart, err := repo.Create(ctx, catalog.CreateLibraryCollectionInput{ + LibraryID: visibleLibrary, LibraryIDs: []int{visibleLibrary}, Slug: slug("smart"), Title: "Smart Set", + CollectionType: "smart", Visibility: "visible", + }) + if err != nil { + t.Fatalf("create smart: %v", err) + } + t.Cleanup(func() { _ = repo.Delete(context.Background(), smart.ID) }) + + // The viewer may reach only visibleLibrary, so hidden and off-scope drop out. + // visible remains (the smart collection never stored a row to match). + got, err := h.ItemCollections(ctx, member, catalog.AccessFilter{AllowedLibraryIDs: []int{visibleLibrary}}) + if err != nil { + t.Fatalf("ItemCollections: %v", err) + } + ids := make([]string, 0, len(got)) + for _, c := range got { + ids = append(ids, c.ID) + } + if !slices.Equal(ids, []string{visible}) { + t.Fatalf("collections = %v, want [%s]", ids, visible) + } + if got[0].ItemCount != 2 { + t.Fatalf("visible item_count = %d, want the global total 2", got[0].ItemCount) + } + + // An unrestricted viewer (no allowlist) still drops hidden collections; an + // off-scope collection is legitimately visible because no allowlist + // restricts libraries. The allowlisted call above already proved off-scope + // drops when the viewer's scope cannot reach its library. + unrestricted, err := h.ItemCollections(ctx, member, catalog.AccessFilter{}) + if err != nil { + t.Fatalf("unrestricted ItemCollections: %v", err) + } + sawOffScope := false + for _, c := range unrestricted { + if c.ID == hidden { + t.Fatalf("unrestricted viewer saw hidden collection %s", hidden) + } + if c.ID == offScope { + sawOffScope = true + } + } + if !sawOffScope { + t.Fatalf("unrestricted viewer should reach the visible off-scope collection %s", offScope) + } + + // A member in no collection answers an empty non-nil slice. + none, err := h.ItemCollections(ctx, slug("ic-nobody"), catalog.AccessFilter{}) + if err != nil { + t.Fatalf("no-member ItemCollections: %v", err) + } + if none == nil || len(none) != 0 { + t.Fatalf("no-member collections = %#v, want an empty non-nil slice", none) + } +} diff --git a/internal/apiv2/admin_catalog_item_metadata.go b/internal/apiv2/admin_catalog_item_metadata.go index 91c3b75ec4..39fc00ba81 100644 --- a/internal/apiv2/admin_catalog_item_metadata.go +++ b/internal/apiv2/admin_catalog_item_metadata.go @@ -88,5 +88,11 @@ func (reg *Registry) updateAdminItemMetadata(ctx context.Context, in *AdminItemM // The operation is gated on metadata curation, so the editor gets every // stored rating back. detail.ViewerCurates = true - return &CatalogItemDetailOutput{Body: catalogItemDetailOf(detail, reg.ratingSelection(ctx))}, nil + out := catalogItemDetailOf(detail, reg.ratingSelection(ctx)) + // The editor sees the same collections row the read detail shows, so a save + // does not appear to drop the item's memberships until a refetch. Access + // comes from the scope the permission gate resolved; the route is not + // profile-scoped, so there is no declared profile to resolve. + out.Collections = NonNil(reg.itemCollections(ctx, detail, handlers.AccessFilterFromContext(ctx, ""))) + return &CatalogItemDetailOutput{Body: out}, nil } diff --git a/internal/apiv2/admin_catalog_item_metadata_test.go b/internal/apiv2/admin_catalog_item_metadata_test.go index 0f7a0c98d6..5f475722a9 100644 --- a/internal/apiv2/admin_catalog_item_metadata_test.go +++ b/internal/apiv2/admin_catalog_item_metadata_test.go @@ -5,8 +5,10 @@ import ( "strings" "testing" + "github.com/Silo-Server/silo-server/internal/access" "github.com/Silo-Server/silo-server/internal/adminjob" "github.com/Silo-Server/silo-server/internal/api/handlers" + apimw "github.com/Silo-Server/silo-server/internal/api/middleware" catalogsvc "github.com/Silo-Server/silo-server/internal/catalog" "github.com/Silo-Server/silo-server/internal/models" "github.com/Silo-Server/silo-server/internal/policy" @@ -56,6 +58,40 @@ func TestAdminItemMetadataTransport(t *testing.T) { } } } + +// TestUpdateAdminItemMetadataCarriesCollections pins that a successful metadata +// save answers the same collections row the read detail does, using the +// viewer's resolved access, so an item in a collection does not appear to lose +// its memberships until a refetch. +func TestUpdateAdminItemMetadataCarriesCollections(t *testing.T) { + deps, _ := catalogDeps(t) + deps.AdminItemMetadata = &fakeAdminItemMetadata{} + deps.PermissionGates[policy.PermissionMetadataCuration] = adminTranslationGate + // The metadata route is not profile-scoped, so the lookup reads the scope + // the permission gate resolved. Restrict it so the test proves that scope, + // not the catalog seam's default, reaches the reverse lookup. + deps.ViewerAccess = apimw.NewViewerAccessMiddleware(policyResolver{scope: &access.Scope{AllowedLibraryIDs: []int{1, 2}}}) + fake := deps.LibraryCollections.(*fakeLibraryViews) + fake.collections = []handlers.ItemCollectionView{{ID: "alpha", Title: "Dune Saga", ItemCount: 3}} + h := newTestHandler(t, deps) + + rec := do(t, h, "PATCH", Prefix+"/admin/items/item-1/metadata", `{"title":"Updated"}`, bearer(memberToken)) + if rec.Code != 200 { + t.Fatalf("update: %d %s", rec.Code, rec.Body) + } + var detail CatalogItemDetail + decodeJSON(t, rec.Body, &detail) + if fake.lastCollections != "item-1" { + t.Fatalf("lookup = %q, want the edited item id", fake.lastCollections) + } + if got := fake.lastCollectionsAccess.AllowedLibraryIDs; len(got) != 2 || got[0] != 1 || got[1] != 2 { + t.Fatalf("lookup access = %+v, want the gate-resolved allowed libraries [1 2]", fake.lastCollectionsAccess) + } + if len(detail.Collections) != 1 || detail.Collections[0].ID != "alpha" || detail.Collections[0].Title != "Dune Saga" || detail.Collections[0].ItemCount != 3 { + t.Fatalf("collections = %+v, want the membership row on the save response", detail.Collections) + } +} + func adminCatalogItemMetadataFixtureCases() []fixtureCase { return []fixtureCase{ {name: "admin_item_refresh_queued", operationID: "refreshAdminItemMetadata", method: "POST", path: Prefix + "/admin/items/item-1/refresh-metadata", body: `{"mode":"complete"}`, headers: bearer(memberToken), status: 202, schema: "#/components/schemas/AdminTaskJob", assertHeaders: []string{"Content-Type", "Location", "Retry-After"}, scenario: "A delegated curator receives a persisted refresh job without administrator-only payloads."}, diff --git a/internal/apiv2/catalog_items.go b/internal/apiv2/catalog_items.go index 355ec51deb..69769dd720 100644 --- a/internal/apiv2/catalog_items.go +++ b/internal/apiv2/catalog_items.go @@ -1151,9 +1151,10 @@ func (reg *Registry) getCatalogItem(ctx context.Context, in *CatalogItemInput) ( // result is nil and the detail page still succeeds with an empty row, // following the theme lookup's non-fatal contract. // -// Collections store movies and series; an episode contributes its parent -// series id, and a season contributes its series. Other types (audiobooks, -// ebooks) have no collection membership and answer empty. +// The membership key is an item's own content id for a movie or series; an +// episode or season contributes its parent series id, because a membership row +// names the series rather than an episode. Any other type, and a title whose +// series id is empty, answers empty. func (reg *Registry) itemCollections(ctx context.Context, detail *catalogpkg.ItemDetail, access catalogpkg.AccessFilter) []ItemCollection { index, ok := reg.deps.LibraryCollections.(ItemCollectionIndex) if !ok || index == nil || detail == nil { diff --git a/internal/apiv2/catalog_items_test.go b/internal/apiv2/catalog_items_test.go index 0b0cc0d461..d311aefbed 100644 --- a/internal/apiv2/catalog_items_test.go +++ b/internal/apiv2/catalog_items_test.go @@ -3,6 +3,7 @@ package apiv2 import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "net/url" @@ -112,6 +113,14 @@ func (f *fakeCatalog) ItemDetail(_ context.Context, v handlers.ItemViewer, id st return &catalogpkg.ItemDetail{ContentID: id, Type: "episode", Title: "Good News About Hell", SeriesID: "series:severance", Genres: []string{}, Cast: []catalogpkg.CastCredit{}, Crew: []catalogpkg.CrewCredit{}, Versions: []catalogpkg.FileVersion{}, Subtitles: []catalogpkg.SubtitleInfo{}}, nil } + if id == "series:severance-S01" { + return &catalogpkg.ItemDetail{ContentID: id, Type: "season", Title: "Season 1", SeriesID: "series:severance", + Genres: []string{}, Cast: []catalogpkg.CastCredit{}, Crew: []catalogpkg.CrewCredit{}, Versions: []catalogpkg.FileVersion{}, Subtitles: []catalogpkg.SubtitleInfo{}}, nil + } + if id == "series:severance" { + return &catalogpkg.ItemDetail{ContentID: id, Type: "series", Title: "Severance", + Genres: []string{}, Cast: []catalogpkg.CastCredit{}, Crew: []catalogpkg.CrewCredit{}, Versions: []catalogpkg.FileVersion{}, Subtitles: []catalogpkg.SubtitleInfo{}}, nil + } if id != "movie:heat-1995" { return nil, notFoundItem() } @@ -657,8 +666,10 @@ func TestGetCatalogItemScopesVersionsToLibraryWhenEnabled(t *testing.T) { // TestGetCatalogItemCarriesCollections pins the additive item detail // "collections" row: a movie's detail fetches the reverse lookup with its own -// content id, an episode resolves to its parent series, the chips carry -// poster and count, and an item in none answers an empty array, never null. +// content id under the viewer's resolved access filter, an episode and a +// season resolve to their parent series, the chips carry poster and count, an +// item in none answers an empty array, never null, and a lookup failure leaves +// the detail a success with an empty row. func TestGetCatalogItemCarriesCollections(t *testing.T) { deps, _ := catalogDeps(t) fake := deps.LibraryCollections.(*fakeLibraryViews) @@ -674,6 +685,11 @@ func TestGetCatalogItemCarriesCollections(t *testing.T) { if fake.lastCollections != "movie:heat-1995" { t.Fatalf("lookup = %q, want the movie id", fake.lastCollections) } + // The viewer's resolved access filter reaches the lookup; the fake catalog + // resolves allowed libraries {1, 2}. + if got := fake.lastCollectionsAccess.AllowedLibraryIDs; len(got) != 2 || got[0] != 1 || got[1] != 2 { + t.Fatalf("lookup access = %+v, want allowed libraries [1 2]", fake.lastCollectionsAccess) + } if len(movie.Collections) != 1 || movie.Collections[0].ID != "alpha" || movie.Collections[0].Title != "Dune Saga" || movie.Collections[0].ItemCount != 3 || movie.Collections[0].PosterURL != "https://cdn.example.test/alpha.jpg" { t.Fatalf("movie collections = %+v", movie.Collections) @@ -692,10 +708,45 @@ func TestGetCatalogItemCarriesCollections(t *testing.T) { t.Fatalf("episode collections = %+v", episode.Collections) } + // A season answers its series' collections the same way. + rec = do(t, h, http.MethodGet, "/api/v2/catalog/items/series:severance-S01", "", viewerHeaders()) + if rec.Code != http.StatusOK { + t.Fatal(rec.Code, rec.Body.String()) + } + var season CatalogItemDetail + decodeJSON(t, rec.Body, &season) + if fake.lastCollections != "series:severance" { + t.Fatalf("season lookup = %q, want the parent series id", fake.lastCollections) + } + if len(season.Collections) != 1 || season.Collections[0].ID != "alpha" { + t.Fatalf("season collections = %+v", season.Collections) + } + + // A series answers its own collections by its own id. + rec = do(t, h, http.MethodGet, "/api/v2/catalog/items/series:severance", "", viewerHeaders()) + if rec.Code != http.StatusOK { + t.Fatal(rec.Code, rec.Body.String()) + } + var series CatalogItemDetail + decodeJSON(t, rec.Body, &series) + if fake.lastCollections != "series:severance" { + t.Fatalf("series lookup = %q, want the series id itself", fake.lastCollections) + } + if len(series.Collections) != 1 || series.Collections[0].ID != "alpha" { + t.Fatalf("series collections = %+v", series.Collections) + } + // An item in no collection answers an empty array, not null. fake.collections = nil rec = do(t, h, http.MethodGet, "/api/v2/catalog/items/movie:heat-1995", "", viewerHeaders()) if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"collections":[]`) { t.Fatalf("empty collections = %s", rec.Body.String()) } + + // A lookup failure is best-effort: the detail succeeds with an empty row. + fake.err = errors.New("boom") + rec = do(t, h, http.MethodGet, "/api/v2/catalog/items/movie:heat-1995", "", viewerHeaders()) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"collections":[]`) { + t.Fatalf("failed lookup collections = %d %s", rec.Code, rec.Body.String()) + } } diff --git a/internal/apiv2/library_views_test.go b/internal/apiv2/library_views_test.go index 6f03b6d03c..3e1496463e 100644 --- a/internal/apiv2/library_views_test.go +++ b/internal/apiv2/library_views_test.go @@ -123,15 +123,20 @@ func (f *fakeLibraryAdmin) SetLibraryProviders(_ context.Context, id int, levels type fakeLibraryViews struct { err error // collections backs ItemCollections; empty answers an empty slice. - collections []handlers.ItemCollectionView - lastCollections string + collections []handlers.ItemCollectionView + // lastCollections, lastCollectionsAccess record what the reverse lookup + // received, so a test can prove the membership id and the viewer access + // filter are what the caller resolved. + lastCollections string + lastCollectionsAccess catalogpkg.AccessFilter } -func (f *fakeLibraryViews) ItemCollections(_ context.Context, membership string, _ catalogpkg.AccessFilter) ([]handlers.ItemCollectionView, error) { +func (f *fakeLibraryViews) ItemCollections(_ context.Context, membership string, access catalogpkg.AccessFilter) ([]handlers.ItemCollectionView, error) { if f.err != nil { return nil, f.err } f.lastCollections = membership + f.lastCollectionsAccess = access return f.collections, nil }