diff --git a/backend/app/plex/client.py b/backend/app/plex/client.py index a8a92dde..d3313323 100644 --- a/backend/app/plex/client.py +++ b/backend/app/plex/client.py @@ -197,7 +197,12 @@ class PlexClient: ) def unscrobble(self, rating_key: str) -> None: - """Mark the item *unwatched* on the token's Plex account (clears viewCount).""" + """Mark the item *unwatched* on the token's Plex account. + + Clears the view count AND the resume point (`viewOffset`) — measured against the live server + on 2026-09-09, from both a watched+offset state and an offset-only one. This one call is the + whole un-mark; `/:/timeline?time=0` and `/:/progress?time=0` were both tried there and do + NOT clear an offset, so do not reach for them.""" self._write( "GET", f"/:/unscrobble?identifier={self._LIBRARY_IDENTIFIER}" diff --git a/backend/app/routes/plex/playback.py b/backend/app/routes/plex/playback.py index d77eb462..1a25750c 100644 --- a/backend/app/routes/plex/playback.py +++ b/backend/app/routes/plex/playback.py @@ -406,10 +406,20 @@ def item_state( ) prev_status = st.status if st is not None else "new" if status == "new": + # Does Plex hold anything for this item that the un-mark has to clear? Two things qualify: a + # view count, and a resume point. An IN-PROGRESS item carries status "new" WITH a position — + # that is how the importer maps a bare `viewOffset` (`_plex_watch_to_state`) — so keying the + # push on the status alone silently skipped the single case "Clear resume point" exists for: + # the un-mark never reached Plex, the offset stayed, and the next pull imported it straight + # back (Protector, cleared twice by the user and back at 92s both times). `hidden` is + # Siftlode-only and still pushes nothing, exactly as this endpoint's docstring says. + mirrored_on_plex = st is not None and ( + st.status == "watched" or (st.position_seconds or 0) > 0 + ) if st is not None: db.delete(st) db.commit() - if prev_status != "new": # something → unwatched: mirror the un-mark to Plex + if mirrored_on_plex: _push_watch(background, db, user.id, it.id, rating_key, "unwatched") return {"status": "new"} if st is None: diff --git a/backend/tests/test_plex_clear_resume_push.py b/backend/tests/test_plex_clear_resume_push.py new file mode 100644 index 00000000..b088a84d --- /dev/null +++ b/backend/tests/test_plex_clear_resume_push.py @@ -0,0 +1,83 @@ +""" "Clear resume point" has to reach Plex — the case the status-only guard skipped. + +`POST /api/plex/item/{rk}/state {"status": "new"}` backs the *Clear resume point* / *Mark unwatched* +buttons. It deletes the local row and mirrors the un-mark to Plex — but it used to decide whether to +push from the PREVIOUS STATUS alone (`prev_status != "new"`), and an in-progress item's row is +`status="new"` carrying a position: that is how the importer maps a bare `viewOffset` +(`_plex_watch_to_state`). So for the one case the button exists for, nothing was ever sent to Plex, +Plex kept the offset, and the next pull imported it straight back — the user cleared *Protector* +twice and it returned to Continue watching at 92s both times (2026-09-09). + +One `unscrobble` clears the view count AND the offset (measured against the live server), so the +push itself needed no change; what needed fixing is WHEN it is scheduled. + +DB lane (`db` fixture) — skipped when no Postgres is reachable. +""" + +from types import SimpleNamespace + +from fastapi.testclient import TestClient + +import app.state as state +from app.auth import current_user +from app.db import get_db +from app.main import app +from app.models import PlexItem, PlexLibrary, PlexState, User +from app.plex import watch_sync as plex_watch + + +def _seed(db, *, status: str, position: int): + lib = PlexLibrary(plex_key="1", title="Movies", kind="movie", enabled=True) + db.add(lib) + db.flush() + it = PlexItem( + rating_key="98614", library_id=lib.id, kind="movie", title="Protector", duration_s=5495 + ) + db.add(it) + user = User(email="clear@example.com") + db.add(user) + db.flush() + db.add(PlexState(user_id=user.id, item_id=it.id, status=status, position_seconds=position)) + db.commit() + return it, user + + +def _clear(db, user, it, monkeypatch) -> list[tuple]: + """Drive the endpoint with the Plex link satisfied and the background push recorded.""" + pushes: list[tuple] = [] + monkeypatch.setattr(state, "_configured_cache", True) # pass the instance-setup request gate + monkeypatch.setattr(plex_watch, "link_for_push", lambda db_, uid: SimpleNamespace(id=1)) + monkeypatch.setattr( + plex_watch, + "push_state_to_plex", + lambda user_id, item_id, rating_key, action, *rest: pushes.append((rating_key, action)), + ) + app.dependency_overrides[get_db] = lambda: db + app.dependency_overrides[current_user] = lambda: user + try: + resp = TestClient(app).post(f"/api/plex/item/{it.rating_key}/state", json={"status": "new"}) + assert resp.status_code == 200, resp.text + finally: + app.dependency_overrides.clear() + return pushes + + +def test_clearing_an_in_progress_item_tells_plex(db, monkeypatch): + # The regression: status "new" + a position IS the in-progress shape, and it must push. + it, user = _seed(db, status="new", position=92) + assert _clear(db, user, it, monkeypatch) == [("98614", "unwatched")] + assert db.query(PlexState).count() == 0 + + +def test_clearing_a_watched_item_still_tells_plex(db, monkeypatch): + # The case the old guard did cover — kept, so the fix cannot narrow into a new hole. + it, user = _seed(db, status="watched", position=0) + assert _clear(db, user, it, monkeypatch) == [("98614", "unwatched")] + + +def test_unhiding_touches_plex_not_at_all(db, monkeypatch): + # `hidden` is a Siftlode-only concept "with no Plex analogue, so it never touches Plex" — the + # endpoint's own docstring. The old status-only guard pushed here anyway, un-marking an item on + # Plex that the user had merely hidden. + it, user = _seed(db, status="hidden", position=0) + assert _clear(db, user, it, monkeypatch) == []