From f1ac0727941ee9f27e81f623b9331d0a8f7d5100 Mon Sep 17 00:00:00 2001 From: NixReaper Date: Sat, 5 Sep 2026 20:08:26 -0500 Subject: [PATCH] Add GET /collections browse endpoint; fix embedded collection poster_path Completes browse/discover across all five entity types: movies, series, people, companies, and now collections. No filter (no categorical field like company_type exists for a collection) -- just page/page_size, always alphabetical by name. While adding poster_path to CollectionBriefOut for the browse listing, found the same class of bug fixed earlier for company logo_path: movie_service's embedded `collection` field never looked up its poster artwork either, so it stayed null even after ingest_collection populated one. Fixed alongside since it's the same schema field this feature was already touching. Also removed PageMeta from schemas/common.py -- scaffolding from early spec work that was never actually wired into any endpoint; all five browse endpoints shipped this session use a flat page/page_size/ total_results/total_pages shape instead. Co-Authored-By: Claude Sonnet 5 --- app/api/v1/collections.py | 13 +++- app/repositories/collection_repository.py | 23 ++++++- app/schemas/collection.py | 9 +++ app/schemas/common.py | 8 +-- app/services/collection_service.py | 28 ++++++++- app/services/movie_service.py | 19 +++--- docs/api.md | 9 ++- tests/integration/test_collection_browse.py | 69 +++++++++++++++++++++ tests/integration/test_movie_ingestion.py | 27 +++++++- 9 files changed, 184 insertions(+), 21 deletions(-) create mode 100644 tests/integration/test_collection_browse.py diff --git a/app/api/v1/collections.py b/app/api/v1/collections.py index 0c4f1aa..62c066a 100644 --- a/app/api/v1/collections.py +++ b/app/api/v1/collections.py @@ -2,13 +2,13 @@ from __future__ import annotations -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, Query from sqlalchemy.ext.asyncio import AsyncSession from app.api.deps import not_found from app.core.database import get_db from app.core.security import enforce_rate_limit -from app.schemas.collection import CollectionDetailOut +from app.schemas.collection import CollectionDetailOut, CollectionListOut from app.services import collection_service router = APIRouter( @@ -16,6 +16,15 @@ router = APIRouter( ) +@router.get("", response_model=CollectionListOut) +async def list_collections( + page: int = Query(1, ge=1), + page_size: int = Query(20, ge=1, le=50), + session: AsyncSession = Depends(get_db), +) -> CollectionListOut: + return await collection_service.list_collections(session, page=page, page_size=page_size) + + @router.get("/{aurora_id}", response_model=CollectionDetailOut) async def get_collection( aurora_id: str, session: AsyncSession = Depends(get_db) diff --git a/app/repositories/collection_repository.py b/app/repositories/collection_repository.py index d1d1a53..337636c 100644 --- a/app/repositories/collection_repository.py +++ b/app/repositories/collection_repository.py @@ -4,7 +4,7 @@ from __future__ import annotations import uuid -from sqlalchemy import select +from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from app.models.collection import Collection, CollectionMovie, CollectionSeries @@ -37,3 +37,24 @@ async def get_collection_series(session: AsyncSession, collection_id: uuid.UUID) ) result = await session.execute(stmt) return list(result.scalars().all()) + + +async def list_collections( + session: AsyncSession, *, limit: int, offset: int +) -> tuple[list[Collection], int]: + """Browse collections -- see movie_repository.list_movies for the + window-function total-count approach this mirrors. Collections have no + categorical field to filter by and no popularity/date to sort by, so + this always orders alphabetically by name (with an id tiebreak for + stable pagination).""" + + stmt = ( + select(Collection, func.count().over().label("total_count")) + .order_by(Collection.name.asc(), Collection.id.asc()) + .limit(limit) + .offset(offset) + ) + rows = (await session.execute(stmt)).all() + if not rows: + return [], 0 + return [row[0] for row in rows], rows[0].total_count diff --git a/app/schemas/collection.py b/app/schemas/collection.py index b1565ab..10e2087 100644 --- a/app/schemas/collection.py +++ b/app/schemas/collection.py @@ -6,6 +6,7 @@ from datetime import datetime from pydantic import BaseModel +from app.schemas.common import CollectionBriefOut from app.schemas.movie import MovieSummaryOut from app.schemas.tv import SeriesSummaryOut @@ -22,3 +23,11 @@ class CollectionDetailOut(BaseModel): created_at: datetime updated_at: datetime + + +class CollectionListOut(BaseModel): + page: int + page_size: int + total_results: int + total_pages: int + results: list[CollectionBriefOut] = [] diff --git a/app/schemas/common.py b/app/schemas/common.py index 2318ce4..ace5833 100644 --- a/app/schemas/common.py +++ b/app/schemas/common.py @@ -101,13 +101,7 @@ class CompanyBriefOut(BaseModel): class CollectionBriefOut(BaseModel): id: str name: str - - -class PageMeta(BaseModel): - page: int - page_size: int - total: int - total_pages: int + poster_path: str | None = None class ErrorDetail(BaseModel): diff --git a/app/services/collection_service.py b/app/services/collection_service.py index 4d4333c..ccd075a 100644 --- a/app/services/collection_service.py +++ b/app/services/collection_service.py @@ -7,9 +7,10 @@ import uuid from sqlalchemy.ext.asyncio import AsyncSession from app.core.ids import aurora_id, parse_aurora_id +from app.models.collection import Collection from app.models.enums import ArtworkType from app.repositories import collection_repository, polymorphic -from app.schemas.collection import CollectionDetailOut +from app.schemas.collection import CollectionDetailOut, CollectionListOut from app.schemas.common import CollectionBriefOut from app.services.common import primary_artwork_path from app.services.movie_service import build_movie_summaries @@ -55,13 +56,24 @@ async def get_collection_detail( ) +async def _collection_summary_from_row( + session: AsyncSession, collection: Collection +) -> CollectionBriefOut: + posters = await polymorphic.get_artwork(session, "collection", collection.id, ArtworkType.POSTER) + return CollectionBriefOut( + id=aurora_id("collection", str(collection.id)), + name=collection.name, + poster_path=primary_artwork_path(posters, ArtworkType.POSTER), + ) + + async def build_collection_summary( session: AsyncSession, collection_id: uuid.UUID ) -> CollectionBriefOut | None: collection = await collection_repository.get_collection_by_id(session, collection_id) if collection is None: return None - return CollectionBriefOut(id=aurora_id("collection", str(collection.id)), name=collection.name) + return await _collection_summary_from_row(session, collection) async def build_collection_summaries( @@ -73,3 +85,15 @@ async def build_collection_summaries( if summary is not None: out.append(summary) return out + + +async def list_collections(session: AsyncSession, *, page: int, page_size: int) -> CollectionListOut: + collections, total = await collection_repository.list_collections( + session, limit=page_size, offset=(page - 1) * page_size + ) + results = [await _collection_summary_from_row(session, c) for c in collections] + total_pages = (total + page_size - 1) // page_size if total else 0 + + return CollectionListOut( + page=page, page_size=page_size, total_results=total, total_pages=total_pages, results=results + ) diff --git a/app/services/movie_service.py b/app/services/movie_service.py index 616cfc7..4d8b9bf 100644 --- a/app/services/movie_service.py +++ b/app/services/movie_service.py @@ -54,6 +54,17 @@ async def get_movie_detail(session: AsyncSession, movie_aurora_id: str) -> Movie cast, crew = split_credits(credits_) + collection_out: CollectionBriefOut | None = None + if movie.collection is not None: + collection_posters = await polymorphic.get_artwork( + session, "collection", movie.collection.id, ArtworkType.POSTER + ) + collection_out = CollectionBriefOut( + id=aurora_id("collection", str(movie.collection.id)), + name=movie.collection.name, + poster_path=primary_artwork_path(collection_posters, ArtworkType.POSTER), + ) + company_out: list[CompanyBriefOut] = [] for company, _role in companies: logos = await polymorphic.get_artwork(session, "company", company.id, ArtworkType.LOGO) @@ -87,13 +98,7 @@ async def get_movie_detail(session: AsyncSession, movie_aurora_id: str) -> Movie cast=cast, crew=crew, companies=company_out, - collection=( - CollectionBriefOut( - id=aurora_id("collection", str(movie.collection.id)), name=movie.collection.name - ) - if movie.collection - else None - ), + collection=collection_out, images=to_images_out(artwork), videos=[to_video_out(v) for v in videos], external_ids=external_ids, diff --git a/docs/api.md b/docs/api.md index 1764622..d17a290 100644 --- a/docs/api.md +++ b/docs/api.md @@ -94,7 +94,7 @@ unknown status) is a 422, not a silently-empty result. Response: "cast": [{ "id": "aur_credit_...", "person": { "id": "aur_person_...", "name": "Keanu Reeves", "profile_path": null }, "character": "Neo", "billing_order": 0 }], "crew": [{ "id": "aur_credit_...", "person": { "id": "aur_person_...", "name": "Lana Wachowski" }, "department": "Directing", "job": "Director" }], "companies": [{ "id": "aur_company_...", "name": "Village Roadshow Pictures", "company_type": "production_company" }], - "collection": { "id": "aur_collection_...", "name": "The Matrix Collection" }, + "collection": { "id": "aur_collection_...", "name": "The Matrix Collection", "poster_path": "https://..." }, "images": { "posters": [...], "backdrops": [...], "logos": [...] }, "videos": [{ "id": "aur_vid_...", "type": "trailer", "site": "YouTube", "url": "https://www.youtube.com/watch?v=...", "is_official": true }], "external_ids": { "tmdb": "603", "imdb": "tt0133093" }, @@ -165,6 +165,7 @@ pagination and response shape as `/movies` and `/series`. | Method | Path | |---|---| +| GET | `/api/v1/collections` | | GET | `/api/v1/collections/{aurora_id}` | | GET | `/api/v1/companies` | | GET | `/api/v1/companies/{aurora_id}` | @@ -175,6 +176,12 @@ pagination and response shape as `/movies` and `/series`. `page_size` pagination. No `sort_by` — companies have no popularity or date field to sort by, so results are always alphabetical by name. +`GET /api/v1/collections` is the same idea for collections/franchises, but +with no filter at all (no categorical field like `company_type` exists for +a collection) — just `page`/`page_size`, always alphabetical by name. This +is the last entity type to get a browse endpoint: movies, series, people, +companies, and collections are all browsable now. + Collection detail's `movies` list only includes movies already ingested into Aurora — `POST /api/v1/admin/ingest/collection` links a franchise's member movies purely by external-id lookup, the same never-fabricate-a-stub diff --git a/tests/integration/test_collection_browse.py b/tests/integration/test_collection_browse.py new file mode 100644 index 0000000..cacf1e1 --- /dev/null +++ b/tests/integration/test_collection_browse.py @@ -0,0 +1,69 @@ +"""Integration tests for GET /api/v1/collections (browse/discover, section +17 -- additive beyond the spec's minimum endpoint list): pagination against +real PostgreSQL, exercised through collection_service.list_collections +directly. Mirrors test_company_browse.py; collections have no categorical +field to filter by and no popularity/date field to sort by, so there's no +filter case and always alphabetical-by-name ordering to test.""" + +from __future__ import annotations + +import pytest +from sqlalchemy.ext.asyncio import AsyncSession + +from app.repositories.provider_repository import get_or_create_provider +from app.repositories.reference_repository import get_or_create_collection +from app.services import collection_service + +_COLLECTIONS = [ + ("3001", "Zeta Trilogy"), + ("3002", "Alpha Saga"), + ("3003", "Mid Collection"), + ("3004", "Beta Universe"), +] + + +async def _seed_collections(db_session: AsyncSession) -> None: + provider = await get_or_create_provider(db_session, "tmdb", "TMDB") + for external_id, name in _COLLECTIONS: + await get_or_create_collection( + db_session, provider_id=provider.id, external_id=external_id, name=name + ) + await db_session.commit() + + +@pytest.mark.asyncio +async def test_list_collections_defaults_to_alphabetical_by_name(db_session: AsyncSession): + await _seed_collections(db_session) + + out = await collection_service.list_collections(db_session, page=1, page_size=20) + + assert out.total_results == 4 + assert out.total_pages == 1 + assert [c.name for c in out.results] == [ + "Alpha Saga", + "Beta Universe", + "Mid Collection", + "Zeta Trilogy", + ] + + +@pytest.mark.asyncio +async def test_list_collections_paginates(db_session: AsyncSession): + await _seed_collections(db_session) + + page1 = await collection_service.list_collections(db_session, page=1, page_size=2) + page2 = await collection_service.list_collections(db_session, page=2, page_size=2) + + assert page1.total_results == 4 + assert page1.total_pages == 2 + assert [c.name for c in page1.results] == ["Alpha Saga", "Beta Universe"] + assert [c.name for c in page2.results] == ["Mid Collection", "Zeta Trilogy"] + + +@pytest.mark.asyncio +async def test_list_collections_returns_empty_page_when_none_exist(db_session: AsyncSession): + out = await collection_service.list_collections(db_session, page=1, page_size=20) + + assert out.results == [] + assert out.total_results == 0 + assert out.total_pages == 0 diff --git a/tests/integration/test_movie_ingestion.py b/tests/integration/test_movie_ingestion.py index a871e95..47f604d 100644 --- a/tests/integration/test_movie_ingestion.py +++ b/tests/integration/test_movie_ingestion.py @@ -21,8 +21,10 @@ from app.models.movie import Movie from app.models.provider import ProviderRecord from app.providers.base import MetadataProvider from app.services import movie_service +from app.services.ingestion.collection_ingestion import ingest_collection from app.services.ingestion.movie_ingestion import ingest_movie -from tests.unit.test_normalizer import FAKE_MOVIE +from tests.integration.test_collection_ingestion import FakeTMDbCollectionProvider +from tests.unit.test_normalizer import FAKE_COLLECTION, FAKE_MOVIE class FakeTMDbProvider(MetadataProvider): @@ -154,6 +156,29 @@ async def test_movie_detail_surfaces_company_logo_path(db_session: AsyncSession) assert roadshow.logo_path.endswith("/village-roadshow-logo.png") +@pytest.mark.asyncio +async def test_movie_detail_surfaces_collection_poster_path(db_session: AsyncSession): + """Same class of bug as company logo_path: movie_service built the + embedded collection field without ever looking up its poster artwork, + so poster_path stayed None even after a full ingest_collection run + populated one. The bare belongs_to_collection stub created by + ingest_movie alone has no poster -- only ingest_collection provides it.""" + + provider = FakeTMDbCollectionProvider( + movie_payloads={"603": FAKE_MOVIE}, collection_payload=FAKE_COLLECTION + ) + movie_id = await ingest_movie(db_session, provider, "603") + await db_session.flush() + await ingest_collection(db_session, provider, "2344") + await db_session.commit() + + detail = await movie_service.get_movie_detail(db_session, aurora_id("movie", str(movie_id))) + + assert detail.collection is not None + assert detail.collection.poster_path is not None + assert detail.collection.poster_path.endswith("/collection-poster.jpg") + + @pytest.mark.asyncio async def test_movie_recommendations_only_include_already_ingested_movies( db_session: AsyncSession,