fix(plex,channels): deep-review fixes — durable watch-sync + no shared-link clobber

Deep-review of the quickfix batch surfaced two data/preference-loss bugs plus
smaller items; all fixed with tests.

- watch_sync _pull_apply: don't let the Plex-authoritative pull overwrite a DIRTY
  local row (a change whose immediate push failed) — that stranded the value
  forever since _repush_dirty only retries dirty rows. Leave it dirty for re-push.
- PlexProvider / ChannelsProvider: a partial shared link opened in a fresh tab
  persisted mount-time DEFAULTS over the account's stored scope/sort/filters
  (base read before the account was pinned). Persist/keep only URL-present dims;
  re-hydrate the omitted ones from storage.
- playback resume write-back: guard against out-of-order delivery (a superseded
  older push now drops) and evict the throttle map so it can't grow unbounded.
- PlexPlayer: announce mid-playback buffering to assistive tech (role=status +
  sr-only label); new plex.player.buffering (HU+EN).
- columnSort: hoist the shared key:dir sort codec used by both URL-state modules.
- Tests: dirty-guard + flag-settle + per-key throttle scoping + superseded-push
  (backend); hold-repeat/suppress-click/unmount + partial-link no-clobber +
  refetch-doesn't-clobber-undo (frontend). Backend pytest 475, FE vitest 519.
This commit is contained in:
2026-08-27 23:54:44 +02:00
parent 7fbb996297
commit f80ed115ad
14 changed files with 403 additions and 60 deletions
@@ -17,6 +17,7 @@ vi.mock("./NavigationProvider", () => ({
}));
import { ChannelsProvider, useChannels, useChannelsActions } from "./ChannelsProvider";
import { setActiveAccount } from "../lib/api/core";
const wrapper = ({ children }: { children: ReactNode }) => (
<ChannelsProvider>{children}</ChannelsProvider>
@@ -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",
});
});
});
+26 -8
View File
@@ -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
@@ -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(
<QueryClientProvider client={qc}>
<PersonalViewsModal />
</QueryClientProvider>,
);
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(
<QueryClientProvider client={qc}>
<PersonalViewsModal />
</QueryClientProvider>,
);
// 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"));
+13 -2
View File
@@ -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) && (
<div className="absolute inset-0 grid place-items-center px-6 text-center text-sm pointer-events-none">
<div
role="status"
aria-live="polite"
className="absolute inset-0 grid place-items-center px-6 text-center text-sm pointer-events-none"
>
{loadError ? (
<span className="max-w-md text-danger">{loadError}</span>
) : (
<div className="flex flex-col items-center gap-3 text-white/80">
<Loader2 className="w-10 h-10 animate-spin" aria-hidden />
{!ready && <span>{t("plex.player.loading")}</span>}
{/* 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 ? (
<span>{t("plex.player.loading")}</span>
) : buffering ? (
<span className="sr-only">{t("plex.player.buffering")}</span>
) : null}
</div>
)}
</div>
+36 -10
View File
@@ -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<string>(() => urlSeed?.scope ?? readPlexScope());
const [show, setShowState] = useState<string>(() => urlSeed?.show ?? readPlexShow());
const [sort, setSortState] = useState<SortState>(() => urlSeed?.sort ?? readPlexSort());
const [scope, setScopeState] = useState<string>(() => urlSeed?.state.scope ?? readPlexScope());
const [show, setShowState] = useState<string>(() => urlSeed?.state.show ?? readPlexShow());
const [sort, setSortState] = useState<SortState>(() => urlSeed?.state.sort ?? readPlexSort());
// The expanded Plex filters live as one JSON blob (the string-only account store serializes it).
const [filtersRaw, setFiltersRaw] = useState<string>(() =>
urlSeed ? JSON.stringify(urlSeed.filters) : readPlexFiltersRaw(),
urlSeed ? JSON.stringify(urlSeed.state.filters) : readPlexFiltersRaw(),
);
const filters = useMemo<PlexFilters>(() => {
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;
@@ -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(<PriorityStepper priority={0} onPriority={onPriority} onSet={vi.fn()} />);
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(<PriorityStepper priority={0} onPriority={onPriority} onSet={vi.fn()} />);
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(
<PriorityStepper priority={0} onPriority={onPriority} onSet={vi.fn()} />,
);
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
});
});