refactor(plex): dedup movie/show watch-state predicates into query.py (E3 S1)
The `show` scope (all|watched|in_progress|unwatched) filter and the status CASE were hand-duplicated between unified_library (the grid) and facets.wsq — the exact drift risk the review flagged (a hidden movie kept out of the grid must not inflate facets). Extract them into four shared query.py helpers (movie_status_case, show_status_case, apply_movie_state_filter, apply_show_state_filter), model-agnostic (callers pass the joined state expressions), and point all three call sites at them. Pure behaviour-equivalence refactor: browse.py -67 lines, no query changes. Green under the E3 S1 parity net + query_conversion/facets/field-scoped/lang tests (348 DB-lane pass, ruff clean).
This commit is contained in:
@@ -6,7 +6,7 @@ Pure query fragments — every function returns SQLAlchemy expressions or a para
|
|||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
|
|
||||||
from sqlalchemy import and_, func, or_, type_coerce
|
from sqlalchemy import and_, case, func, or_, type_coerce
|
||||||
from sqlalchemy.dialects.postgresql import JSONB
|
from sqlalchemy.dialects.postgresql import JSONB
|
||||||
|
|
||||||
TS_CONFIG = "public.unaccent_simple"
|
TS_CONFIG = "public.unaccent_simple"
|
||||||
@@ -189,3 +189,62 @@ class LibraryFilters:
|
|||||||
|
|
||||||
def like_escape(s: str) -> str:
|
def like_escape(s: str) -> str:
|
||||||
return s.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
|
return s.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
|
||||||
|
|
||||||
|
|
||||||
|
# --- Watch-state scope (shared by the /library grid AND the /facets aggregation) ---------------
|
||||||
|
#
|
||||||
|
# The `show` scope (all | watched | in_progress | unwatched) filters the browse result by per-user
|
||||||
|
# watch state. The SAME predicate has to be applied in two places — `unified_library` (the grid) and
|
||||||
|
# `facets.wsq` (so the facet counts reflect exactly the visible set) — and they MUST stay identical:
|
||||||
|
# a hidden movie kept out of the grid must not inflate the facets either. These helpers are the one
|
||||||
|
# home for that predicate so the two call sites can't drift (E3 S1; pinned by test_plex_facets_grid_
|
||||||
|
# parity). Callers pass the already-joined state expressions, so this module stays model-agnostic:
|
||||||
|
# * movies — `st` is an aliased PlexState outer-joined on (item_id, user_id).
|
||||||
|
# * shows — `tot`/`wat`/`inp` are the coalesced totals from `rollup.agg_subquery`.
|
||||||
|
|
||||||
|
|
||||||
|
def movie_status_case(st):
|
||||||
|
"""The per-movie status label (new | in_progress | watched) from its PlexState alias `st`."""
|
||||||
|
return case(
|
||||||
|
(st.status == "watched", "watched"),
|
||||||
|
(
|
||||||
|
and_(st.position_seconds > 0, or_(st.status.is_(None), st.status != "watched")),
|
||||||
|
"in_progress",
|
||||||
|
),
|
||||||
|
else_="new",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def show_status_case(tot, wat, inp):
|
||||||
|
"""The per-show status label rolled up from its episodes' totals (total/watched/in-progress)."""
|
||||||
|
return case(
|
||||||
|
(and_(tot > 0, wat >= tot), "watched"),
|
||||||
|
(or_(wat > 0, inp > 0), "in_progress"),
|
||||||
|
else_="new",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def apply_movie_state_filter(query, st, show: str):
|
||||||
|
"""Restrict a movie query to the `show` scope via its PlexState alias `st`. The default (`all`)
|
||||||
|
excludes hidden movies — the exclusion that must match between grid and facets."""
|
||||||
|
if show == "watched":
|
||||||
|
return query.where(st.status == "watched")
|
||||||
|
if show == "in_progress":
|
||||||
|
return query.where(
|
||||||
|
st.position_seconds > 0, or_(st.status.is_(None), st.status != "watched")
|
||||||
|
)
|
||||||
|
if show == "unwatched":
|
||||||
|
return query.where(or_(st.status.is_(None), st.status.notin_(["watched", "hidden"])))
|
||||||
|
return query.where(or_(st.status.is_(None), st.status != "hidden"))
|
||||||
|
|
||||||
|
|
||||||
|
def apply_show_state_filter(query, tot, wat, inp, show: str):
|
||||||
|
"""Restrict a show query to the `show` scope via its rollup totals. `all` is a no-op (shows have
|
||||||
|
no hidden concept); the caller only joins the rollup for the three non-`all` scopes."""
|
||||||
|
if show == "watched":
|
||||||
|
return query.where(tot > 0, wat >= tot)
|
||||||
|
if show == "in_progress":
|
||||||
|
return query.where(or_(wat > 0, inp > 0), wat < tot)
|
||||||
|
if show == "unwatched":
|
||||||
|
return query.where(wat == 0, inp == 0)
|
||||||
|
return query
|
||||||
|
|||||||
@@ -11,7 +11,6 @@ from sqlalchemy import (
|
|||||||
Integer,
|
Integer,
|
||||||
String,
|
String,
|
||||||
and_,
|
and_,
|
||||||
case,
|
|
||||||
cast,
|
cast,
|
||||||
delete,
|
delete,
|
||||||
func,
|
func,
|
||||||
@@ -50,10 +49,14 @@ from app.plex.query import (
|
|||||||
TS_CONFIG,
|
TS_CONFIG,
|
||||||
LibraryFilters,
|
LibraryFilters,
|
||||||
apply_meta_filters,
|
apply_meta_filters,
|
||||||
|
apply_movie_state_filter,
|
||||||
|
apply_show_state_filter,
|
||||||
field_recheck,
|
field_recheck,
|
||||||
genres_col,
|
genres_col,
|
||||||
like_escape,
|
like_escape,
|
||||||
meta_filter_conds,
|
meta_filter_conds,
|
||||||
|
movie_status_case,
|
||||||
|
show_status_case,
|
||||||
split_csv,
|
split_csv,
|
||||||
title_col,
|
title_col,
|
||||||
vector_col,
|
vector_col,
|
||||||
@@ -154,17 +157,7 @@ def unified_library(
|
|||||||
selects = []
|
selects = []
|
||||||
if want_movies and grid_field:
|
if want_movies and grid_field:
|
||||||
st = aliased(PlexState)
|
st = aliased(PlexState)
|
||||||
m_status = case(
|
m_status = movie_status_case(st)
|
||||||
(st.status == "watched", "watched"),
|
|
||||||
(
|
|
||||||
and_(
|
|
||||||
st.position_seconds > 0,
|
|
||||||
or_(st.status.is_(None), st.status != "watched"),
|
|
||||||
),
|
|
||||||
"in_progress",
|
|
||||||
),
|
|
||||||
else_="new",
|
|
||||||
)
|
|
||||||
mq = (
|
mq = (
|
||||||
select(
|
select(
|
||||||
PlexItem.rating_key.label("rk"),
|
PlexItem.rating_key.label("rk"),
|
||||||
@@ -188,19 +181,7 @@ def unified_library(
|
|||||||
.outerjoin(st, and_(st.item_id == PlexItem.id, st.user_id == user.id))
|
.outerjoin(st, and_(st.item_id == PlexItem.id, st.user_id == user.id))
|
||||||
.where(PlexItem.kind == "movie", PlexItem.library_id.in_(movie_lib_ids))
|
.where(PlexItem.kind == "movie", PlexItem.library_id.in_(movie_lib_ids))
|
||||||
)
|
)
|
||||||
if show == "watched":
|
mq = apply_movie_state_filter(mq, st, show)
|
||||||
mq = mq.where(st.status == "watched")
|
|
||||||
elif show == "in_progress":
|
|
||||||
mq = mq.where(
|
|
||||||
st.position_seconds > 0,
|
|
||||||
or_(st.status.is_(None), st.status != "watched"),
|
|
||||||
)
|
|
||||||
elif show == "unwatched":
|
|
||||||
mq = mq.where(
|
|
||||||
or_(st.status.is_(None), st.status.notin_(["watched", "hidden"]))
|
|
||||||
)
|
|
||||||
else:
|
|
||||||
mq = mq.where(or_(st.status.is_(None), st.status != "hidden"))
|
|
||||||
mq = apply_meta_filters(mq, PlexItem, p, lang)
|
mq = apply_meta_filters(mq, PlexItem, p, lang)
|
||||||
if f.duration_min is not None:
|
if f.duration_min is not None:
|
||||||
mq = mq.where(PlexItem.duration_s >= f.duration_min)
|
mq = mq.where(PlexItem.duration_s >= f.duration_min)
|
||||||
@@ -216,11 +197,7 @@ def unified_library(
|
|||||||
tot = func.coalesce(agg.c.total, 0)
|
tot = func.coalesce(agg.c.total, 0)
|
||||||
wat = func.coalesce(agg.c.watched, 0)
|
wat = func.coalesce(agg.c.watched, 0)
|
||||||
inp = func.coalesce(agg.c.inprog, 0)
|
inp = func.coalesce(agg.c.inprog, 0)
|
||||||
s_status = case(
|
s_status = show_status_case(tot, wat, inp)
|
||||||
(and_(tot > 0, wat >= tot), "watched"),
|
|
||||||
(or_(wat > 0, inp > 0), "in_progress"),
|
|
||||||
else_="new",
|
|
||||||
)
|
|
||||||
sq = (
|
sq = (
|
||||||
select(
|
select(
|
||||||
PlexShow.rating_key.label("rk"),
|
PlexShow.rating_key.label("rk"),
|
||||||
@@ -244,12 +221,7 @@ def unified_library(
|
|||||||
.outerjoin(agg, agg.c.show_id == PlexShow.id)
|
.outerjoin(agg, agg.c.show_id == PlexShow.id)
|
||||||
.where(PlexShow.library_id.in_(show_lib_ids))
|
.where(PlexShow.library_id.in_(show_lib_ids))
|
||||||
)
|
)
|
||||||
if show == "watched":
|
sq = apply_show_state_filter(sq, tot, wat, inp, show)
|
||||||
sq = sq.where(tot > 0, wat >= tot)
|
|
||||||
elif show == "in_progress":
|
|
||||||
sq = sq.where(or_(wat > 0, inp > 0), wat < tot)
|
|
||||||
elif show == "unwatched":
|
|
||||||
sq = sq.where(wat == 0, inp == 0)
|
|
||||||
sq = apply_meta_filters(sq, PlexShow, p, lang)
|
sq = apply_meta_filters(sq, PlexShow, p, lang)
|
||||||
if tsq is not None:
|
if tsq is not None:
|
||||||
sq = sq.where(vector_match(PlexShow, lang, tsq))
|
sq = sq.where(vector_match(PlexShow, lang, tsq))
|
||||||
@@ -393,23 +365,11 @@ def facets(
|
|||||||
query = query.where(model.duration_s >= pp["duration_min"])
|
query = query.where(model.duration_s >= pp["duration_min"])
|
||||||
if with_dur and pp.get("duration_max") is not None:
|
if with_dur and pp.get("duration_max") is not None:
|
||||||
query = query.where(model.duration_s <= pp["duration_max"])
|
query = query.where(model.duration_s <= pp["duration_max"])
|
||||||
# Always join watch-state and mirror the grid's status branches (unified_library) exactly —
|
# Join watch-state and reuse the grid's exact movie state filter (unified_library) —
|
||||||
# hidden movies never appear in the grid, so they must not inflate facet counts/bounds either.
|
# hidden movies never appear in the grid, so they must not inflate facet counts/bounds either.
|
||||||
st = aliased(PlexState)
|
st = aliased(PlexState)
|
||||||
query = query.outerjoin(st, and_(st.item_id == model.id, st.user_id == uid))
|
query = query.outerjoin(st, and_(st.item_id == model.id, st.user_id == uid))
|
||||||
if show == "watched":
|
query = apply_movie_state_filter(query, st, show)
|
||||||
query = query.where(st.status == "watched")
|
|
||||||
elif show == "in_progress":
|
|
||||||
query = query.where(
|
|
||||||
st.position_seconds > 0,
|
|
||||||
or_(st.status.is_(None), st.status != "watched"),
|
|
||||||
)
|
|
||||||
elif show == "unwatched":
|
|
||||||
query = query.where(
|
|
||||||
or_(st.status.is_(None), st.status.notin_(["watched", "hidden"]))
|
|
||||||
)
|
|
||||||
else:
|
|
||||||
query = query.where(or_(st.status.is_(None), st.status != "hidden"))
|
|
||||||
else:
|
else:
|
||||||
if show in ("watched", "in_progress", "unwatched"):
|
if show in ("watched", "in_progress", "unwatched"):
|
||||||
agg = rollup.agg_subquery(db, uid).subquery()
|
agg = rollup.agg_subquery(db, uid).subquery()
|
||||||
@@ -417,12 +377,7 @@ def facets(
|
|||||||
tot = func.coalesce(agg.c.total, 0)
|
tot = func.coalesce(agg.c.total, 0)
|
||||||
wat = func.coalesce(agg.c.watched, 0)
|
wat = func.coalesce(agg.c.watched, 0)
|
||||||
inp = func.coalesce(agg.c.inprog, 0)
|
inp = func.coalesce(agg.c.inprog, 0)
|
||||||
if show == "watched":
|
query = apply_show_state_filter(query, tot, wat, inp, show)
|
||||||
query = query.where(tot > 0, wat >= tot)
|
|
||||||
elif show == "in_progress":
|
|
||||||
query = query.where(or_(wat > 0, inp > 0), wat < tot)
|
|
||||||
else:
|
|
||||||
query = query.where(wat == 0, inp == 0)
|
|
||||||
return query
|
return query
|
||||||
|
|
||||||
genre_counts: dict[str, int] = {}
|
genre_counts: dict[str, int] = {}
|
||||||
|
|||||||
Reference in New Issue
Block a user