Add GET /collections browse endpoint; fix embedded collection poster_path
CI / lint (push) Canceled after 0s
CI / typecheck (push) Canceled after 0s
CI / unit-tests (push) Canceled after 0s
CI / integration-tests (push) Canceled after 0s
CI / container-build (push) Canceled after 0s
CI / dependency-audit (push) Canceled after 0s
CI / lint (push) Canceled after 0s
CI / typecheck (push) Canceled after 0s
CI / unit-tests (push) Canceled after 0s
CI / integration-tests (push) Canceled after 0s
CI / container-build (push) Canceled after 0s
CI / dependency-audit (push) Canceled after 0s
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 <[email protected]>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
4cf3556433
commit
f1ac072794
@@ -2,13 +2,13 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends
|
from fastapi import APIRouter, Depends, Query
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from app.api.deps import not_found
|
from app.api.deps import not_found
|
||||||
from app.core.database import get_db
|
from app.core.database import get_db
|
||||||
from app.core.security import enforce_rate_limit
|
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
|
from app.services import collection_service
|
||||||
|
|
||||||
router = APIRouter(
|
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)
|
@router.get("/{aurora_id}", response_model=CollectionDetailOut)
|
||||||
async def get_collection(
|
async def get_collection(
|
||||||
aurora_id: str, session: AsyncSession = Depends(get_db)
|
aurora_id: str, session: AsyncSession = Depends(get_db)
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
from sqlalchemy import select
|
from sqlalchemy import func, select
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from app.models.collection import Collection, CollectionMovie, CollectionSeries
|
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)
|
result = await session.execute(stmt)
|
||||||
return list(result.scalars().all())
|
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
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ from datetime import datetime
|
|||||||
|
|
||||||
from pydantic import BaseModel
|
from pydantic import BaseModel
|
||||||
|
|
||||||
|
from app.schemas.common import CollectionBriefOut
|
||||||
from app.schemas.movie import MovieSummaryOut
|
from app.schemas.movie import MovieSummaryOut
|
||||||
from app.schemas.tv import SeriesSummaryOut
|
from app.schemas.tv import SeriesSummaryOut
|
||||||
|
|
||||||
@@ -22,3 +23,11 @@ class CollectionDetailOut(BaseModel):
|
|||||||
|
|
||||||
created_at: datetime
|
created_at: datetime
|
||||||
updated_at: datetime
|
updated_at: datetime
|
||||||
|
|
||||||
|
|
||||||
|
class CollectionListOut(BaseModel):
|
||||||
|
page: int
|
||||||
|
page_size: int
|
||||||
|
total_results: int
|
||||||
|
total_pages: int
|
||||||
|
results: list[CollectionBriefOut] = []
|
||||||
|
|||||||
@@ -101,13 +101,7 @@ class CompanyBriefOut(BaseModel):
|
|||||||
class CollectionBriefOut(BaseModel):
|
class CollectionBriefOut(BaseModel):
|
||||||
id: str
|
id: str
|
||||||
name: str
|
name: str
|
||||||
|
poster_path: str | None = None
|
||||||
|
|
||||||
class PageMeta(BaseModel):
|
|
||||||
page: int
|
|
||||||
page_size: int
|
|
||||||
total: int
|
|
||||||
total_pages: int
|
|
||||||
|
|
||||||
|
|
||||||
class ErrorDetail(BaseModel):
|
class ErrorDetail(BaseModel):
|
||||||
|
|||||||
@@ -7,9 +7,10 @@ import uuid
|
|||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from app.core.ids import aurora_id, parse_aurora_id
|
from app.core.ids import aurora_id, parse_aurora_id
|
||||||
|
from app.models.collection import Collection
|
||||||
from app.models.enums import ArtworkType
|
from app.models.enums import ArtworkType
|
||||||
from app.repositories import collection_repository, polymorphic
|
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.schemas.common import CollectionBriefOut
|
||||||
from app.services.common import primary_artwork_path
|
from app.services.common import primary_artwork_path
|
||||||
from app.services.movie_service import build_movie_summaries
|
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(
|
async def build_collection_summary(
|
||||||
session: AsyncSession, collection_id: uuid.UUID
|
session: AsyncSession, collection_id: uuid.UUID
|
||||||
) -> CollectionBriefOut | None:
|
) -> CollectionBriefOut | None:
|
||||||
collection = await collection_repository.get_collection_by_id(session, collection_id)
|
collection = await collection_repository.get_collection_by_id(session, collection_id)
|
||||||
if collection is None:
|
if collection is None:
|
||||||
return 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(
|
async def build_collection_summaries(
|
||||||
@@ -73,3 +85,15 @@ async def build_collection_summaries(
|
|||||||
if summary is not None:
|
if summary is not None:
|
||||||
out.append(summary)
|
out.append(summary)
|
||||||
return out
|
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
|
||||||
|
)
|
||||||
|
|||||||
@@ -54,6 +54,17 @@ async def get_movie_detail(session: AsyncSession, movie_aurora_id: str) -> Movie
|
|||||||
|
|
||||||
cast, crew = split_credits(credits_)
|
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] = []
|
company_out: list[CompanyBriefOut] = []
|
||||||
for company, _role in companies:
|
for company, _role in companies:
|
||||||
logos = await polymorphic.get_artwork(session, "company", company.id, ArtworkType.LOGO)
|
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,
|
cast=cast,
|
||||||
crew=crew,
|
crew=crew,
|
||||||
companies=company_out,
|
companies=company_out,
|
||||||
collection=(
|
collection=collection_out,
|
||||||
CollectionBriefOut(
|
|
||||||
id=aurora_id("collection", str(movie.collection.id)), name=movie.collection.name
|
|
||||||
)
|
|
||||||
if movie.collection
|
|
||||||
else None
|
|
||||||
),
|
|
||||||
images=to_images_out(artwork),
|
images=to_images_out(artwork),
|
||||||
videos=[to_video_out(v) for v in videos],
|
videos=[to_video_out(v) for v in videos],
|
||||||
external_ids=external_ids,
|
external_ids=external_ids,
|
||||||
|
|||||||
+8
-1
@@ -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 }],
|
"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" }],
|
"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" }],
|
"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": [...] },
|
"images": { "posters": [...], "backdrops": [...], "logos": [...] },
|
||||||
"videos": [{ "id": "aur_vid_...", "type": "trailer", "site": "YouTube", "url": "https://www.youtube.com/watch?v=...", "is_official": true }],
|
"videos": [{ "id": "aur_vid_...", "type": "trailer", "site": "YouTube", "url": "https://www.youtube.com/watch?v=...", "is_official": true }],
|
||||||
"external_ids": { "tmdb": "603", "imdb": "tt0133093" },
|
"external_ids": { "tmdb": "603", "imdb": "tt0133093" },
|
||||||
@@ -165,6 +165,7 @@ pagination and response shape as `/movies` and `/series`.
|
|||||||
|
|
||||||
| Method | Path |
|
| Method | Path |
|
||||||
|---|---|
|
|---|---|
|
||||||
|
| GET | `/api/v1/collections` |
|
||||||
| GET | `/api/v1/collections/{aurora_id}` |
|
| GET | `/api/v1/collections/{aurora_id}` |
|
||||||
| GET | `/api/v1/companies` |
|
| GET | `/api/v1/companies` |
|
||||||
| GET | `/api/v1/companies/{aurora_id}` |
|
| 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
|
`page_size` pagination. No `sort_by` — companies have no popularity or
|
||||||
date field to sort by, so results are always alphabetical by name.
|
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
|
Collection detail's `movies` list only includes movies already ingested
|
||||||
into Aurora — `POST /api/v1/admin/ingest/collection` links a franchise's
|
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
|
member movies purely by external-id lookup, the same never-fabricate-a-stub
|
||||||
|
|||||||
@@ -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
|
||||||
@@ -21,8 +21,10 @@ from app.models.movie import Movie
|
|||||||
from app.models.provider import ProviderRecord
|
from app.models.provider import ProviderRecord
|
||||||
from app.providers.base import MetadataProvider
|
from app.providers.base import MetadataProvider
|
||||||
from app.services import movie_service
|
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 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):
|
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")
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_movie_recommendations_only_include_already_ingested_movies(
|
async def test_movie_recommendations_only_include_already_ingested_movies(
|
||||||
db_session: AsyncSession,
|
db_session: AsyncSession,
|
||||||
|
|||||||
Reference in New Issue
Block a user