diff --git a/backend/app/plex/watch_sync.py b/backend/app/plex/watch_sync.py index 96304701..85ef6399 100644 --- a/backend/app/plex/watch_sync.py +++ b/backend/app/plex/watch_sync.py @@ -369,6 +369,14 @@ def _pull_apply( # `_same_state`; a row that reaches here really is out of step with the source of truth. The # `_same_state` + `synced_to_plex` short-circuit above still absorbs the freshly-pushed case (Plex # echoes the same shape a beat later) so a just-pushed row never bounces. + if st is not None and not st.synced_to_plex: + # Durability guard: this row holds a local change that never reached Plex — its immediate push + # failed, so it's still dirty. Do NOT let the Plex-authoritative pull overwrite it and clear the + # flag: that would strand the local value forever, since `_repush_dirty` only retries dirty rows. + # Leave it dirty; `_repush_dirty` (later in this same run) pushes the local value up to Plex. This + # closes the "Plex flapped mid-session" lost-update — the one case the Plex-wins comment's + # "almost always already pushed" assumption above excludes. + return None if st is None: st = PlexState(user_id=user_id, item_id=item_id) db.add(st) diff --git a/backend/app/routes/plex/playback.py b/backend/app/routes/plex/playback.py index 7314087a..fab04797 100644 --- a/backend/app/routes/plex/playback.py +++ b/backend/app/routes/plex/playback.py @@ -4,7 +4,7 @@ import hashlib import os import tempfile import threading -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone import httpx from fastapi import ( @@ -171,15 +171,33 @@ _push_throttle_lock = threading.Lock() def _should_push_resume(user_id: int, item_id: int, now: datetime, final: bool) -> bool: """Whether to mirror this resume checkpoint to Plex now. `final` always pushes; a periodic tick pushes only when ≥_PUSH_THROTTLE_S has elapsed since the last push for this (user, item). Records - `now` as the last-push time whenever it returns True so the window is measured from the real push.""" + `now` as the last-push time whenever it returns True so the window is measured from the real push + (and so `_resume_push_superseded` can drop an older, still-pending push — see there).""" with _push_throttle_lock: last = _last_push_at.get((user_id, item_id)) if final or last is None or (now - last).total_seconds() >= _PUSH_THROTTLE_S: _last_push_at[(user_id, item_id)] = now + # Opportunistic eviction so the map can't grow unbounded over a long-lived process (one + # entry per item ever checkpointed). Anything older than a few windows is dead — a push + # completes in seconds, so a stale entry protects nothing. + if len(_last_push_at) > 2048: + cutoff = now - timedelta(seconds=_PUSH_THROTTLE_S * 6) + for k in [k for k, t in _last_push_at.items() if t < cutoff]: + del _last_push_at[k] return True return False +def _resume_push_superseded(user_id: int, item_id: int, intent: datetime) -> bool: + """True if a newer resume push for this (user, item) was scheduled after `intent`. Called at the + HTTP boundary of a background resume push: because periodic and seek/pause pushes are independent + background tasks with no ordering guard, an older still-pending periodic push could otherwise land + at Plex AFTER a newer backward-seek push and — with the Plex-authoritative pull — durably revive the + stale position. Skipping a superseded push keeps the last-scheduled position the one Plex settles on.""" + with _push_throttle_lock: + return _last_push_at.get((user_id, item_id), intent) > intent + + @router.get("/person-image") def person_image(u: str, request: Request) -> Response: """Proxy a cast/crew photo from Plex's public metadata CDN (host-whitelisted; no token needed). @@ -202,6 +220,26 @@ def person_image(u: str, request: Request) -> Response: return imgcache.response(r.content, ct) +def _push_resume_guarded( + user_id: int, + item_id: int, + rating_key: str, + time_ms: int, + duration_ms: int, + expect_ts: datetime | None, + intent: datetime, +) -> None: + """Background entrypoint for a resume push that drops itself if a newer resume push for the same + (user, item) was scheduled after it (see `_resume_push_superseded`), then delegates to the real + push. This narrows the periodic-vs-seek write-ordering race to the (rare) case where the older push + has already left the process before the newer one is scheduled.""" + if _resume_push_superseded(user_id, item_id, intent): + return + plex_watch.push_state_to_plex( + user_id, item_id, rating_key, "resume", time_ms, duration_ms, expect_ts + ) + + def _push_watch( background: BackgroundTasks, db: Session, @@ -211,10 +249,14 @@ def _push_watch( action: str, time_ms: int = 0, duration_ms: int = 0, + intent: datetime | None = None, ) -> None: """Schedule a best-effort Siftlode→Plex watch push, but only for a user whose owner link has push enabled — non-owner users never touch Plex. Guarding here (with the request session) avoids - spinning up a throwaway background session on every state change for the common no-link case.""" + spinning up a throwaway background session on every state change for the common no-link case. + + A `resume` push with an `intent` timestamp is routed through `_push_resume_guarded` so a superseded + (older) push can't land out of order; `watched`/`unwatched` scrobbles go straight through.""" if plex_watch.link_for_push(db, user_id) is None: return # Capture the row's freshest timestamp NOW so the background push can tell if a newer local edit @@ -226,6 +268,12 @@ def _push_watch( .scalars() .first() ) + marker = plex_watch._state_marker(st) + if action == "resume" and intent is not None: + background.add_task( + _push_resume_guarded, user_id, item_id, rating_key, time_ms, duration_ms, marker, intent + ) + return background.add_task( plex_watch.push_state_to_plex, user_id, @@ -234,7 +282,7 @@ def _push_watch( action, time_ms, duration_ms, - plex_watch._state_marker(st), + marker, ) @@ -284,6 +332,10 @@ def item_progress( st.position_seconds = 0 st.progress_updated_at = now st.synced_to_plex = False + # Terminal state for resume write-back: drop the throttle/ordering entry so the map doesn't + # retain a dead key for every finished item. + with _push_throttle_lock: + _last_push_at.pop((user.id, it.id), None) db.commit() # Immediate on the watched transition; skip a repeat scrobble if it was already watched # (a later checkpoint of the same finished item would otherwise bump Plex's viewCount again). @@ -321,6 +373,7 @@ def item_progress( "resume", position * 1000, duration * 1000, + intent=now, ) return {"position_seconds": position} diff --git a/backend/tests/test_plex_watch_sync.py b/backend/tests/test_plex_watch_sync.py index e4047066..da20bfee 100644 --- a/backend/tests/test_plex_watch_sync.py +++ b/backend/tests/test_plex_watch_sync.py @@ -14,7 +14,11 @@ from sqlalchemy import select from app.models import PlexItem, PlexLibrary, PlexState, User from app.plex.watch_sync import _pull_apply -from app.routes.plex.playback import _PUSH_THROTTLE_S, _should_push_resume +from app.routes.plex.playback import ( + _PUSH_THROTTLE_S, + _resume_push_superseded, + _should_push_resume, +) def _seed_item_and_user(db): @@ -29,9 +33,10 @@ def _seed_item_and_user(db): return item.id, user.id -def test_pull_plex_wins_over_newer_local_resume(db): - """The core S2 change: a locally-in-progress item whose local edit is NEWER than Plex's still - resolves to PLEX's position on the incremental pull — Plex is authoritative, no LWW.""" +def test_pull_plex_wins_over_diverging_synced_local_resume(db): + """The core S2 change: a SYNCED local row (its state already reached Plex) that later diverges from + Plex = a real external Plex-side change, so the incremental pull resolves to PLEX's position — Plex + is authoritative, no LWW. (A DIRTY local row is protected instead — see the dirty-guard test.)""" item_id, user_id = _seed_item_and_user(db) now = datetime.now(timezone.utc) db.add( @@ -41,7 +46,7 @@ def test_pull_plex_wins_over_newer_local_resume(db): status="new", position_seconds=300, progress_updated_at=now, # local touched just now — "newer" than Plex - synced_to_plex=False, + synced_to_plex=True, # but already pushed → a later Plex divergence is an external change ) ) db.flush() @@ -62,8 +67,8 @@ def test_pull_plex_wins_over_newer_local_resume(db): assert st.synced_to_plex is True -def test_pull_plex_watched_wins_over_local_in_progress(db): - """Plex marking the item watched propagates even over a fresh local in-progress row.""" +def test_pull_plex_watched_wins_over_synced_local_in_progress(db): + """Plex marking the item watched propagates over a SYNCED local in-progress row (external change).""" item_id, user_id = _seed_item_and_user(db) now = datetime.now(timezone.utc) db.add( @@ -73,7 +78,7 @@ def test_pull_plex_watched_wins_over_local_in_progress(db): status="new", position_seconds=300, progress_updated_at=now, - synced_to_plex=False, + synced_to_plex=True, ) ) db.flush() @@ -125,6 +130,104 @@ def test_pull_no_bounce_on_matching_row(db): assert st.position_seconds == 100, "no churn to the stored position" +def test_pull_keeps_dirty_local_on_divergence(db): + """Durability guard: a DIRTY row (a local change whose immediate push failed → synced_to_plex=False) + must NOT be overwritten by a diverging Plex signal, and must stay dirty so `_repush_dirty` can push + the local value up. Overwriting-and-clearing here would strand the local change forever.""" + item_id, user_id = _seed_item_and_user(db) + now = datetime.now(timezone.utc) + db.add( + PlexState( + user_id=user_id, + item_id=item_id, + status="watched", # user just marked watched locally; the scrobble to Plex failed + position_seconds=0, + watched_at=now, + progress_updated_at=now, + synced_to_plex=False, # dirty — never reached Plex + ) + ) + db.flush() + + # Plex still lists it in on_deck at an old in-progress offset (it never got the watched scrobble). + changed = _pull_apply(db, user_id, item_id, "new", 240, plex_ts=now - timedelta(seconds=60)) + db.flush() + + assert changed is None, "a dirty local row must not be clobbered by the pull" + st = ( + db.execute( + select(PlexState).where(PlexState.user_id == user_id, PlexState.item_id == item_id) + ) + .scalars() + .one() + ) + assert st.status == "watched", "local watched preserved" + assert st.position_seconds == 0 + assert st.synced_to_plex is False, "stays dirty so _repush_dirty retries the push" + + +def test_pull_settles_flag_on_matching_dirty_row(db): + """Anti-bounce, dirty branch: a row that already MATCHES Plex but is still dirty gets its + synced_to_plex flag settled to True (no churn), so `_repush_dirty` stops re-pushing it forever.""" + item_id, user_id = _seed_item_and_user(db) + now = datetime.now(timezone.utc) + db.add( + PlexState( + user_id=user_id, + item_id=item_id, + status="new", + position_seconds=100, + progress_updated_at=now, + synced_to_plex=False, # dirty, but already agrees with Plex + ) + ) + db.flush() + + changed = _pull_apply(db, user_id, item_id, "new", 102, plex_ts=now + timedelta(seconds=2)) + db.flush() + + assert changed is None, "a matching row makes no change" + st = ( + db.execute( + select(PlexState).where(PlexState.user_id == user_id, PlexState.item_id == item_id) + ) + .scalars() + .one() + ) + assert st.position_seconds == 100, "no churn to the stored position" + assert st.synced_to_plex is True, "the dirty flag is settled so it isn't re-pushed endlessly" + + +def test_should_push_resume_scopes_by_user_and_item(): + """The throttle window is keyed by the FULL (user, item) tuple: pushing for one key must not + suppress a distinct user or a distinct item within the same window.""" + now = datetime.now(timezone.utc) + assert _should_push_resume(9101, 9201, now, final=False) is True # (u1, i1) first tick + # Same window, different item for the same user → independent, still pushes. + assert _should_push_resume(9101, 9202, now, final=False) is True + # Same window, different user, same item → independent, still pushes. + assert _should_push_resume(9102, 9201, now, final=False) is True + # And the original key is still throttled within its window (proves the above weren't just "always"). + assert _should_push_resume(9101, 9201, now + timedelta(seconds=1), final=False) is False + + +def test_resume_push_superseded_drops_older_pending_push(): + """The write-ordering guard: an older still-pending resume push is dropped once a newer push for the + same (user, item) has been scheduled, so a reordered stale position can't land at Plex last.""" + key = (9301, 9301) + t0 = datetime.now(timezone.utc) + # Schedule an initial push at t0 (records t0 as the last-push time for this key). + assert _should_push_resume(*key, t0, final=False) is True + # Nothing newer yet → the t0 push is current, not superseded. + assert _resume_push_superseded(*key, t0) is False + # A newer push (e.g. a backward seek) is scheduled at t1 > t0. + t1 = t0 + timedelta(seconds=1) + assert _should_push_resume(*key, t1, final=True) is True + # Now the still-pending t0 push is superseded and must drop; the t1 push is current. + assert _resume_push_superseded(*key, t0) is True + assert _resume_push_superseded(*key, t1) is False + + def test_should_push_resume_final_always_pushes(): """A final checkpoint (pause / seek / stop) always pushes, bypassing the throttle.""" now = datetime.now(timezone.utc) diff --git a/frontend/src/components/ChannelsProvider.dom.test.tsx b/frontend/src/components/ChannelsProvider.dom.test.tsx index 5ad4a6f0..acca1831 100644 --- a/frontend/src/components/ChannelsProvider.dom.test.tsx +++ b/frontend/src/components/ChannelsProvider.dom.test.tsx @@ -17,6 +17,7 @@ vi.mock("./NavigationProvider", () => ({ })); import { ChannelsProvider, useChannels, useChannelsActions } from "./ChannelsProvider"; +import { setActiveAccount } from "../lib/api/core"; const wrapper = ({ children }: { children: ReactNode }) => ( {children} @@ -99,4 +100,33 @@ describe("ChannelsProvider — S8 shareable URL", () => { act(() => result.current.setView("subscribed")); expect(new URLSearchParams(window.location.search).has("ctab")).toBe(false); }); + + it("a PARTIAL shared link (fresh tab) does not clobber the account's stored sort/filters", () => { + // Regression: on a fresh tab the account pins AFTER mount, so the seed's base for the OMITTED + // dimensions was read as defaults. Persisting the whole seed used to overwrite the account's real + // stored sort + filter map. The fix persists ONLY the URL-present dimensions. + localStorage.setItem( + "siftlode.channelSort.1", + JSON.stringify({ key: "subscribers", dir: "desc" }), + ); + localStorage.setItem("siftlode.channelsFilterMap.1", JSON.stringify({ sync: "needsFull" })); + sessionStorage.removeItem("siftlode.activeAccount"); // account not pinned yet at mount + window.history.replaceState({ sfPage: "channels" }, "", "/?ctab=hidden"); // only the tab in the URL + + const { result } = render(); + act(() => setActiveAccount(1)); // App pins the account after /api/me + + // The present dim (tab) is adopted from the link... + expect(result.current.view).toBe("hidden"); + // ...but the omitted sort + filters keep the account's real stored values, in state AND storage. + expect(result.current.sort).toEqual({ key: "subscribers", dir: "desc" }); + expect(result.current.filters).toEqual({ sync: "needsFull" }); + expect(JSON.parse(localStorage.getItem("siftlode.channelSort.1")!)).toEqual({ + key: "subscribers", + dir: "desc", + }); + expect(JSON.parse(localStorage.getItem("siftlode.channelsFilterMap.1")!)).toEqual({ + sync: "needsFull", + }); + }); }); diff --git a/frontend/src/components/ChannelsProvider.tsx b/frontend/src/components/ChannelsProvider.tsx index 35ad9f02..3504a49d 100644 --- a/frontend/src/components/ChannelsProvider.tsx +++ b/frontend/src/components/ChannelsProvider.tsx @@ -73,14 +73,25 @@ function readStoredFilters(): FilterMap { // present, the URL wins over this account's stored view — read once at mount and applied over the slice // hooks (see the provider). The keys are channels-unique, so their mere presence identifies a shared // manager view; no ?page check is needed (the mirror below is page-gated so it never pollutes others). -function readChannelsUrlSeed(): ChannelsUrlState | null { +// The seed = the URL-merged state PLUS which dimensions the URL actually carried. `present` matters +// because the base below is read at MOUNT, before the account is pinned (getAccountRaw returns null), +// so an OMITTED dimension's base is a default, not this account's stored value. The provider seeds and +// persists only the PRESENT dimensions and lets the absent ones re-hydrate from storage normally — +// otherwise a partial shared link silently resets the omitted stored sort / filter map. +type ChannelsSeedPresent = { view: boolean; sort: boolean; filters: boolean }; +type ChannelsUrlSeed = { state: ChannelsUrlState; present: ChannelsSeedPresent }; +function readChannelsUrlSeed(): ChannelsUrlSeed | null { const params = new URLSearchParams(window.location.search); if (!hasChannelsParams(params)) return null; - return channelsParamsToState(params, { + const state = channelsParamsToState(params, { view: clampView(getAccountRaw(LS.channelsView)), sort: hydrateSort(getAccountRaw(LS.channelSort), [], null), filters: readStoredFilters(), }); + return { + state, + present: { view: params.has("ctab"), sort: params.has("csort"), filters: params.has("cf") }, + }; } type ChannelsState = { @@ -185,9 +196,13 @@ export function ChannelsProvider({ children }: { children: ReactNode }) { useLayoutEffect(() => { if (!urlSeed || seededRef.current) return; seededRef.current = true; - if (urlSeed.view !== view) setViewRaw(urlSeed.view); - setFilters(urlSeed.filters); - setSort(urlSeed.sort); + // Only apply the dimensions the URL actually carried. An omitted dimension must NOT go through its + // setter (which would markDirty and block the account-pin re-hydrate from loading the stored value) + // — leave it to hydrate from storage normally. Fixes a partial link wiping the stored sort/filters. + const { state, present } = urlSeed; + if (present.view && state.view !== view) setViewRaw(state.view); + if (present.filters) setFilters(state.filters); + if (present.sort) setSort(state.sort); // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -198,9 +213,12 @@ export function ChannelsProvider({ children }: { children: ReactNode }) { useEffect(() => { if (!urlSeed || accountId == null || persistedRef.current) return; persistedRef.current = true; - setAccountRaw(LS.channelsView, urlSeed.view); - setAccountRaw(LS.channelsFilterMap, JSON.stringify(urlSeed.filters)); - setAccountRaw(LS.channelSort, JSON.stringify(urlSeed.sort)); + // Persist ONLY the URL-present dimensions — the omitted ones were seeded from mount-time defaults + // (no account yet), so writing them here would overwrite the account's real stored sort/filter map. + const { state, present } = urlSeed; + if (present.view) setAccountRaw(LS.channelsView, state.view); + if (present.filters) setAccountRaw(LS.channelsFilterMap, JSON.stringify(state.filters)); + if (present.sort) setAccountRaw(LS.channelSort, JSON.stringify(state.sort)); }, [urlSeed, accountId]); // Latest-value ref so the STABLE setView can compare against the current tab without depending on diff --git a/frontend/src/components/PersonalViewsModal.dom.test.tsx b/frontend/src/components/PersonalViewsModal.dom.test.tsx index 9b9a18dc..93d7be25 100644 --- a/frontend/src/components/PersonalViewsModal.dom.test.tsx +++ b/frontend/src/components/PersonalViewsModal.dom.test.tsx @@ -80,11 +80,12 @@ function mount() { const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); qc.setQueryData(qk.tags(), [{ id: 1, name: "Docu" }]); vi.spyOn(api, "tags").mockResolvedValue([{ id: 1, name: "Docu" }] as never); - return render( + const utils = render( , ); + return { ...utils, qc }; } beforeEach(() => { @@ -92,6 +93,8 @@ beforeEach(() => { h.saveCurrent.mockClear(); h.rename.mockClear(); h.activeViewId = null; + // Fresh baseline each test — one test mutates CTX.views to simulate a refetch. + CTX.views = [{ id: 5, name: "Docs", filters: VIEW_FILTERS, position: 0, is_default: false }]; }); afterEach(() => { cleanup(); @@ -119,6 +122,39 @@ describe("PersonalViewsModal — S7 editable detail pane", () => { expect(h.updateFilters.mock.calls[1]![1]).toMatchObject({ tags: [1] }); }); + it("a same-view refetch (new-identity filters) does not wipe the undo history", async () => { + // Guards the [selectedId]-only re-seed effect (its exhaustive-deps disable): after a chip prune the + // real provider refetches and hands back the SAME view id with a fresh-identity filters object. The + // draft must NOT re-seed on that (it would destroy the undo history and the Undo control). A deps + // regression that added selected.filters would make this fail. + const { rerender, qc } = mount(); + fireEvent.click(await screen.findByLabelText("feed.chips.remove")); + await waitFor(() => expect(h.updateFilters).toHaveBeenCalledTimes(1)); + expect(await screen.findByLabelText("common.undo")).toBeTruthy(); + + // Simulate the post-write refetch: same id 5, new object identity, carrying the pruned value. + CTX.views = [ + { + id: 5, + name: "Docs", + filters: { ...VIEW_FILTERS, tags: [] }, + position: 0, + is_default: false, + }, + ]; + rerender( + + + , + ); + + // Undo history survived the refetch — the control is still there and still re-persists on click. + const undo = await screen.findByLabelText("common.undo"); + fireEvent.click(undo); + await waitFor(() => expect(h.updateFilters).toHaveBeenCalledTimes(2)); + expect(h.updateFilters.mock.calls[1]![1]).toMatchObject({ tags: [1] }); + }); + it("Save-current stores the live filters into the view when it isn't already the active one", async () => { mount(); fireEvent.click(await screen.findByText("views.saveCurrent")); diff --git a/frontend/src/components/PlexPlayer.tsx b/frontend/src/components/PlexPlayer.tsx index 383625d7..c8e775c2 100644 --- a/frontend/src/components/PlexPlayer.tsx +++ b/frontend/src/components/PlexPlayer.tsx @@ -760,13 +760,24 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) { mid-playback stall or in-flight seek (see the state); `loadError` surfaces a fatal failure so it can't hide behind a running video. Spinner on load/stall, error text otherwise. */} {(!ready || buffering || loadError) && ( -
+
{loadError ? ( {loadError} ) : (
- {!ready && {t("plex.player.loading")}} + {/* Announce the stall to assistive tech. Initial load shows visible text; a mid-playback + rebuffer/seek (ready, no error) has no visible label, so give it an sr-only one via the + aria-live region so a screen reader distinguishes "buffering" from a frozen frame. */} + {!ready ? ( + {t("plex.player.loading")} + ) : buffering ? ( + {t("plex.player.buffering")} + ) : null}
)}
diff --git a/frontend/src/components/PlexProvider.tsx b/frontend/src/components/PlexProvider.tsx index ed4b97ed..53d46024 100644 --- a/frontend/src/components/PlexProvider.tsx +++ b/frontend/src/components/PlexProvider.tsx @@ -131,15 +131,31 @@ function readPlexFilters(): PlexFilters { return EMPTY_PLEX_FILTERS; } } -function readPlexUrlSeed(): PlexUrlState | null { +// A shared-link seed = the URL-merged state PLUS which dimensions the URL actually carried. `present` +// matters because the base below is read at MOUNT, before the account is pinned (getAccountRaw returns +// null then), so the base for an OMITTED dimension is a hard default, NOT this account's stored value. +// The provider must therefore persist/keep only the PRESENT dimensions on account-pin and re-read the +// account's real stored value for the absent ones — otherwise a partial link silently resets them. +type PlexSeedPresent = { scope: boolean; sort: boolean; show: boolean; filters: boolean }; +type PlexUrlSeed = { state: PlexUrlState; present: PlexSeedPresent }; +function readPlexUrlSeed(): PlexUrlSeed | null { const params = new URLSearchParams(window.location.search); if (!hasPlexParams(params)) return null; - return plexParamsToState(params, { + const state = plexParamsToState(params, { scope: readPlexScope(), sort: readPlexSort(), show: readPlexShow(), filters: readPlexFilters(), }); + return { + state, + present: { + scope: params.has("pscope"), + sort: params.has("psort"), + show: params.has("pshow"), + filters: params.has("pf"), + }, + }; } export function PlexProvider({ children }: { children: ReactNode }) { @@ -148,12 +164,12 @@ export function PlexProvider({ children }: { children: ReactNode }) { const [urlSeed] = useState(readPlexUrlSeed); // The former per-library picker is now a cross-library SCOPE: movie | show | both (unified library). // (Reuses the old LS.plexLibrary key; an old stored library-id value falls back to "both".) - const [scope, setScopeState] = useState(() => urlSeed?.scope ?? readPlexScope()); - const [show, setShowState] = useState(() => urlSeed?.show ?? readPlexShow()); - const [sort, setSortState] = useState(() => urlSeed?.sort ?? readPlexSort()); + const [scope, setScopeState] = useState(() => urlSeed?.state.scope ?? readPlexScope()); + const [show, setShowState] = useState(() => urlSeed?.state.show ?? readPlexShow()); + const [sort, setSortState] = useState(() => urlSeed?.state.sort ?? readPlexSort()); // The expanded Plex filters live as one JSON blob (the string-only account store serializes it). const [filtersRaw, setFiltersRaw] = useState(() => - urlSeed ? JSON.stringify(urlSeed.filters) : readPlexFiltersRaw(), + urlSeed ? JSON.stringify(urlSeed.state.filters) : readPlexFiltersRaw(), ); const filters = useMemo(() => { try { @@ -180,10 +196,20 @@ export function PlexProvider({ children }: { children: ReactNode }) { // non-shared slices (q/field) from storage. Mirrors FeedFiltersProvider's initialHadUrlFilters branch. const markDirty = useAccountRehydrate(() => { if (urlSeed) { - setAccountRaw(LS.plexLibrary, urlSeed.scope); - setAccountRaw(LS.plexShow, urlSeed.show); - setAccountRaw(LS.plexSort, JSON.stringify(urlSeed.sort)); - setAccountRaw(LS.plexFilters, JSON.stringify(urlSeed.filters)); + // Per dimension: a dimension the URL carried (present) is the shared view — adopt it and persist + // it to this now-pinned account (so it wins and survives a later plain reload). A dimension the + // URL OMITTED was seeded from a mount-time default (no account yet), so do NOT persist that + // default over the account's real stored value — re-read the stored value instead, exactly like a + // normal load. This is the fix for a partial shared link silently resetting the omitted slices. + const { state, present } = urlSeed; + if (present.scope) setAccountRaw(LS.plexLibrary, state.scope); + else setScopeState(readPlexScope()); + if (present.show) setAccountRaw(LS.plexShow, state.show); + else setShowState(readPlexShow()); + if (present.sort) setAccountRaw(LS.plexSort, JSON.stringify(state.sort)); + else setSortState(readPlexSort()); + if (present.filters) setAccountRaw(LS.plexFilters, JSON.stringify(state.filters)); + else setFiltersRaw(readPlexFiltersRaw()); setQState(readPlexQ()); setFieldState(readPlexField()); return; diff --git a/frontend/src/components/PriorityStepper.dom.test.tsx b/frontend/src/components/PriorityStepper.dom.test.tsx index 6e8e6f4f..6225ee6a 100644 --- a/frontend/src/components/PriorityStepper.dom.test.tsx +++ b/frontend/src/components/PriorityStepper.dom.test.tsx @@ -12,7 +12,10 @@ vi.mock("react-i18next", async (importOriginal) => { return { ...actual, useTranslation: () => ({ t: (key: string) => key }) }; }); -afterEach(cleanup); +afterEach(() => { + cleanup(); + vi.useRealTimers(); // any fake-timer test resets here so it can't leak into the next +}); const raise = () => screen.getByRole("button", { name: "channels.row.raisePriority" }); const lower = () => screen.getByRole("button", { name: "channels.row.lowerPriority" }); @@ -78,4 +81,57 @@ describe("PriorityStepper", () => { fireEvent.click(raise()); expect(onPriority).toHaveBeenCalledWith(1); }); + + it("holding a step button auto-repeats (accelerating) and suppresses the trailing click", () => { + vi.useFakeTimers(); + const onPriority = vi.fn(); + render(); + + fireEvent.pointerDown(raise()); + vi.advanceTimersByTime(349); + expect(onPriority).not.toHaveBeenCalled(); // a tap below the 350ms threshold doesn't repeat + vi.advanceTimersByTime(1); // cross the threshold → first auto-repeat tick + expect(onPriority).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(1000); // several accelerating repeats while held + const heldCount = onPriority.mock.calls.length; + expect(heldCount).toBeGreaterThan(3); + expect(onPriority.mock.calls.every(([d]) => d === 1)).toBe(true); + + fireEvent.pointerUp(raise()); + vi.advanceTimersByTime(1000); + expect(onPriority.mock.calls.length).toBe(heldCount); // stops repeating on release + + fireEvent.click(raise()); // the click that trails a hold is swallowed (no double-count) + expect(onPriority.mock.calls.length).toBe(heldCount); + }); + + it("a quick tap (released before the hold threshold) still nudges exactly once", () => { + vi.useFakeTimers(); + const onPriority = vi.fn(); + render(); + + fireEvent.pointerDown(lower()); + vi.advanceTimersByTime(100); // below the threshold → no auto-repeat armed + fireEvent.pointerUp(lower()); + fireEvent.click(lower()); // the tap's click is NOT suppressed + expect(onPriority.mock.calls).toEqual([[-1]]); + }); + + it("clears the auto-repeat timer on unmount (no fire after the row is gone)", () => { + vi.useFakeTimers(); + const onPriority = vi.fn(); + const { unmount } = render( + , + ); + + fireEvent.pointerDown(raise()); + vi.advanceTimersByTime(350); // repeat armed + first tick + const before = onPriority.mock.calls.length; + expect(before).toBeGreaterThan(0); + + unmount(); + vi.advanceTimersByTime(1000); + expect(onPriority.mock.calls.length).toBe(before); // no post-unmount timer fire + }); }); diff --git a/frontend/src/i18n/locales/en/plex.json b/frontend/src/i18n/locales/en/plex.json index 964f7af6..44a30df4 100644 --- a/frontend/src/i18n/locales/en/plex.json +++ b/frontend/src/i18n/locales/en/plex.json @@ -154,6 +154,7 @@ }, "player": { "loading": "Loading…", + "buffering": "Buffering…", "back": "Back", "skipIntro": "Skip intro", "skipCredits": "Skip credits", diff --git a/frontend/src/i18n/locales/hu/plex.json b/frontend/src/i18n/locales/hu/plex.json index c24b9be4..57ab000e 100644 --- a/frontend/src/i18n/locales/hu/plex.json +++ b/frontend/src/i18n/locales/hu/plex.json @@ -154,6 +154,7 @@ }, "player": { "loading": "Betöltés…", + "buffering": "Pufferelés…", "back": "Vissza", "skipIntro": "Intro átugrása", "skipCredits": "Stáblista átugrása", diff --git a/frontend/src/lib/channelsUrlState.ts b/frontend/src/lib/channelsUrlState.ts index b1fa43bf..74c5e08a 100644 --- a/frontend/src/lib/channelsUrlState.ts +++ b/frontend/src/lib/channelsUrlState.ts @@ -5,7 +5,7 @@ // feed's URL-stripping can't delete them out from under the manager. Only non-default values are // emitted, so URLs stay clean. The manager's name search is deliberately EPHEMERAL (like the feed/plex // rail search) and stays OUT of the shared contract. -import { parseSortState, type SortState } from "./columnSort"; +import { decodeSortParam, encodeSortParam, type SortState } from "./columnSort"; import { type FilterMap } from "../components/tableHeader"; // The content state S8 mirrors. `view` is the raw tab string (the provider clamps it); `sort` is the @@ -20,18 +20,8 @@ export interface ChannelsUrlState { const KEYS = ["ctab", "csort", "cf"] as const; const VIEWS = ["subscribed", "discovery", "hidden", "blocked"]; -// Sort ⇄ `key:dir` (e.g. `subscribers:desc`). Column sort keys are simple words (no colon), so a plain -// split is safe. There is no default sort (fallback null), so any sort emits. -function encodeSort(s: SortState): string | null { - return s ? `${s.key}:${s.dir}` : null; -} -function decodeSort(raw: string): SortState { - const i = raw.indexOf(":"); - const key = i === -1 ? raw : raw.slice(0, i); - const dir = i === -1 ? "asc" : raw.slice(i + 1); - return parseSortState({ key, dir }); -} - +// Sort ⇄ `key:dir` (e.g. `subscribers:desc`) via the shared codec in columnSort. There is no default +// sort here (fallback null), so any sort emits — no default short-circuit needed. // The FilterMap mixes scalar column filters (priority=select, name=text, the sync chip) with array ones // (tags=multi), so it's carried as a compact JSON blob rather than a nested query string — JSON round- // trips both shapes exactly (and future multi columns for free), and the values are opaque enum/id @@ -72,7 +62,7 @@ export function channelsParamUpdates( if (!state) return { ctab: null, csort: null, cf: null }; return { ctab: state.view && state.view !== "subscribed" ? state.view : null, - csort: encodeSort(state.sort), + csort: encodeSortParam(state.sort), cf: encodeFilters(state.filters), }; } @@ -92,7 +82,7 @@ export function channelsParamsToState( const csort = params.get("csort"); return { view: tab && VIEWS.includes(tab) ? tab : base.view, - sort: csort != null ? (decodeSort(csort) ?? base.sort) : base.sort, + sort: csort != null ? (decodeSortParam(csort) ?? base.sort) : base.sort, filters: params.has("cf") ? decodeFilters(params.get("cf") ?? "") : base.filters, }; } diff --git a/frontend/src/lib/columnSort.ts b/frontend/src/lib/columnSort.ts index e9ae944f..cd2a9a71 100644 --- a/frontend/src/lib/columnSort.ts +++ b/frontend/src/lib/columnSort.ts @@ -34,6 +34,22 @@ export function parseSortState(raw: unknown): SortState { return typeof key === "string" && (dir === "asc" || dir === "desc") ? { key, dir } : null; } +/** Encode a SortState to the compact `key:dir` wire form the shareable-URL modules (plex/channels) + * put in the address bar. Sort keys are simple words (no colon), so the plain join round-trips. Returns + * null for no-sort. A module with a default sort short-circuits that to null at its own call site. */ +export function encodeSortParam(s: SortState): string | null { + return s ? `${s.key}:${s.dir}` : null; +} + +/** Inverse of `encodeSortParam`: parse `key:dir` (or a bare key → asc) from the URL back into a + * SortState, healing anything malformed to null. One codec for both URL-state modules — no drift. */ +export function decodeSortParam(raw: string): SortState { + const i = raw.indexOf(":"); + const key = i === -1 ? raw : raw.slice(0, i); + const dir = i === -1 ? "asc" : raw.slice(i + 1); + return parseSortState({ key, dir }); +} + /** The header-click cycle: unsorted → ascending → descending → unsorted. */ export function cycleSort(current: SortState, key: string): SortState { if (!current || current.key !== key) return { key, dir: "asc" }; diff --git a/frontend/src/lib/plexUrlState.ts b/frontend/src/lib/plexUrlState.ts index 4bbcb0a3..b02041bf 100644 --- a/frontend/src/lib/plexUrlState.ts +++ b/frontend/src/lib/plexUrlState.ts @@ -6,7 +6,7 @@ // keys are Plex-unique (like `?v=`/`?pl`), so no other provider ever reads or clears them. Only non- // default values are emitted, so URLs stay clean. import { parsePlexFilters, plexFiltersToParams, type PlexFilters } from "./api"; -import { parseSortState, type SortState } from "./columnSort"; +import { decodeSortParam, encodeSortParam, type SortState } from "./columnSort"; import { PLEX_SORT_FALLBACK } from "./plexSort"; // The content state S4 mirrors. `sort` is the RAW provider SortState (scope-healed only at the point of @@ -22,17 +22,11 @@ const KEYS = ["pscope", "psort", "pshow", "pf"] as const; const SCOPES = ["movie", "show", "both"]; const SHOWS = ["all", "unwatched", "in_progress", "watched"]; -// Sort ⇄ `key:dir` (e.g. `year:desc`). Plex sort keys are simple words (no colon), so a plain split is -// safe. The default (`title:asc`) emits nothing. +// Sort ⇄ `key:dir` (e.g. `year:desc`) via the shared codec in columnSort; the Plex default (`title:asc`) +// emits nothing. function encodeSort(s: SortState): string | null { if (!s || (s.key === PLEX_SORT_FALLBACK?.key && s.dir === PLEX_SORT_FALLBACK?.dir)) return null; - return `${s.key}:${s.dir}`; -} -function decodeSort(raw: string): SortState { - const i = raw.indexOf(":"); - const key = i === -1 ? raw : raw.slice(0, i); - const dir = i === -1 ? "asc" : raw.slice(i + 1); - return parseSortState({ key, dir }); + return encodeSortParam(s); } /** The address-bar delta for the Plex content state: each namespaced key mapped to its emitted value, @@ -61,7 +55,7 @@ export function plexParamsToState(params: URLSearchParams, base: PlexUrlState): const scope = params.get("pscope"); const show = params.get("pshow"); const psort = params.get("psort"); - const sort = psort != null ? (decodeSort(psort) ?? base.sort) : base.sort; + const sort = psort != null ? (decodeSortParam(psort) ?? base.sort) : base.sort; return { scope: scope && SCOPES.includes(scope) ? scope : base.scope, sort,