diff --git a/backend/tests/test_feed_views.py b/backend/tests/test_feed_views.py index cb05aa8e..ba89de02 100644 --- a/backend/tests/test_feed_views.py +++ b/backend/tests/test_feed_views.py @@ -105,11 +105,12 @@ def _seed_tagged(db, make_user): chans = {c: Channel(id=c, title=c, subscriber_count=10) for c in ("chA", "chB", "chC", "chD")} db.add_all(chans.values()) db.flush() - # channel -> (system) tag keys. chA carries two languages (L1+L2) and chC two "other" tags - # (O1+O2) so the per-category any/all match narrows non-trivially for language and other, not - # only topic (chA/chB share topic tags T1/T2). + # channel -> (system) tag keys. chA carries two languages (L1+L2); chC two "other" tags (O1+O2) + # AND chA carries O1 too — so the per-category any/all match narrows NON-TRIVIALLY (AND != OR) for + # language and other, not only topic: e.g. other {O1,O2} → OR {chA,chC} vs AND {chC}; language + # {L1,L2} → OR {chA,chB,chC} vs AND {chA}. (chA/chB share topic tags T1/T2 for the topic case.) links = { - "chA": ["L1", "L2", "T1"], + "chA": ["L1", "L2", "T1", "O1"], "chB": ["L1", "T2"], "chC": ["L2", "T1", "O1", "O2"], "chD": [], diff --git a/frontend/src/components/DateFilterSelect.tsx b/frontend/src/components/DateFilterSelect.tsx index 2338b5c7..9df1cd33 100644 --- a/frontend/src/components/DateFilterSelect.tsx +++ b/frontend/src/components/DateFilterSelect.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useLayoutEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import { CalendarDays, Check, ChevronDown } from "lucide-react"; import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; @@ -36,7 +36,9 @@ export default function DateFilterSelect({ // is closed, drop the custom reveal so the trigger reverts to "Any time" instead of lingering on // "Custom range" with empty fields. Guarded on !open so it never closes a freshly-picked Custom the // user hasn't filled in yet — that stays open until they close it (handled by onOpenChange below). - useEffect(() => { + // useLayoutEffect (not useEffect) so the reset commits BEFORE paint — otherwise the trigger flashes + // one stale "Custom range" frame between the external clear and this reset. + useLayoutEffect(() => { if (!open && customOpen && !filters.dateFrom && !filters.dateTo) setCustomOpen(false); }, [open, customOpen, filters.dateFrom, filters.dateTo]); diff --git a/frontend/src/components/FeedFiltersProvider.dom.test.ts b/frontend/src/components/FeedFiltersProvider.dom.test.ts new file mode 100644 index 00000000..2d54581c --- /dev/null +++ b/frontend/src/components/FeedFiltersProvider.dom.test.ts @@ -0,0 +1,45 @@ +// @vitest-environment jsdom +// +// loadAccountFilters is the localStorage seed path for the feed filters. It must migrate a persisted +// legacy global `tagMode` (older builds wrote it) into the per-category `tagModes` — the same coercion +// the saved-view and URL paths do — so an upgrading user's topic any/all match isn't silently lost. +import { describe, it, expect, beforeEach } from "vitest"; +import { loadAccountFilters } from "./FeedFiltersProvider"; +import { accountKey, LS } from "../lib/storage"; + +const ID = 7; + +beforeEach(() => localStorage.clear()); + +describe("loadAccountFilters — legacy tagMode migration on seed", () => { + it("migrates a persisted legacy tagMode:'and' from the last-session blob into topic", () => { + localStorage.setItem(accountKey(LS.filters, ID)!, JSON.stringify({ q: "hi", tagMode: "and" })); + const f = loadAccountFilters(ID); + expect(f.tagModes).toEqual({ language: "or", topic: "and", other: "or" }); + expect(f.q).toBe("hi"); // other fields preserved through the merge + }); + + it("migrates a legacy tagMode:'and' from the default-view mirror when no last-session blob exists", () => { + localStorage.setItem( + accountKey(LS.defaultViewFilters, ID)!, + JSON.stringify({ tagMode: "and" }), + ); + expect(loadAccountFilters(ID).tagModes.topic).toBe("and"); + }); + + it("leaves tagModes at defaults when nothing is stored", () => { + expect(loadAccountFilters(ID).tagModes).toEqual({ language: "or", topic: "or", other: "or" }); + }); + + it("preserves an already-migrated per-category tagModes blob", () => { + localStorage.setItem( + accountKey(LS.filters, ID)!, + JSON.stringify({ tagModes: { language: "and", topic: "or", other: "and" } }), + ); + expect(loadAccountFilters(ID).tagModes).toEqual({ + language: "and", + topic: "or", + other: "and", + }); + }); +}); diff --git a/frontend/src/components/FeedFiltersProvider.tsx b/frontend/src/components/FeedFiltersProvider.tsx index 2854e534..2a0d9ad3 100644 --- a/frontend/src/components/FeedFiltersProvider.tsx +++ b/frontend/src/components/FeedFiltersProvider.tsx @@ -18,7 +18,7 @@ import { } from "../lib/api"; import { filterParamUpdates, hasFilterParams, paramsToFilters } from "../lib/urlState"; import { useUrlMirror } from "../lib/useUrlMirror"; -import { accountKey, LS, readJSON, readMerged, writeJSON } from "../lib/storage"; +import { accountKey, LS, readJSON, writeJSON } from "../lib/storage"; import { useNavigation } from "./NavigationProvider"; // Owns the GLOBAL feed-filter state (the feed's scope/tags/content/sort/show), lifted out of the App @@ -55,7 +55,7 @@ const FILTERS_KEY = LS.filters; // default saved view wins over the last-session filters (SavedViewsWidget mirrors it here). When // the account isn't known yet (first load, before the tab is pinned), start from defaults; the // post-login effect loads the real ones once the id arrives. -function loadAccountFilters(id: number | null): FeedFilters { +export function loadAccountFilters(id: number | null): FeedFilters { // Your last-applied filters win, so a reload keeps whatever view you're on (a saved view you // picked, or manual tweaks) — setFilters persists them under LS.filters on every change. The // starred default view only SEEDS a fresh account that has never stored filters: it drives the @@ -68,18 +68,18 @@ function loadAccountFilters(id: number | null): FeedFilters { } catch { /* localStorage may be unavailable */ } - // Migrate a persisted legacy global `tagMode` into the per-category `tagModes` on every seed path - // (readMerged is a shallow merge that leaves the legacy scalar as ignored junk), matching what the - // saved-view (asFilters) and URL (paramsToFilters) paths already do. + // Migrate a persisted legacy global `tagMode` into the per-category `tagModes` on every seed path, + // matching what the saved-view (asFilters) and URL (paramsToFilters) paths already do. coerceTagModes + // MUST run on the RAW stored blob, not the object merged over DEFAULT_FILTERS: the merge injects + // DEFAULT_TAG_MODES (all "or"), which coerceTagModes prefers over the legacy scalar and would mask it. if (hasStored && fKey) { - const merged = readMerged(fKey, DEFAULT_FILTERS); - return { ...merged, tagModes: coerceTagModes(merged) }; + const raw = readJSON>(fKey, {}); + return { ...DEFAULT_FILTERS, ...raw, tagModes: coerceTagModes(raw) }; } const dvKey = accountKey(LS.defaultViewFilters, id); - const def = dvKey ? readJSON(dvKey, null) : null; + const def = dvKey ? readJSON | null>(dvKey, null) : null; if (def) { - const merged = { ...DEFAULT_FILTERS, ...def }; - return { ...merged, tagModes: coerceTagModes(merged) }; + return { ...DEFAULT_FILTERS, ...def, tagModes: coerceTagModes(def) }; } return { ...DEFAULT_FILTERS }; } diff --git a/frontend/src/components/SavedViewsProvider.tsx b/frontend/src/components/SavedViewsProvider.tsx index 108894af..496256af 100644 --- a/frontend/src/components/SavedViewsProvider.tsx +++ b/frontend/src/components/SavedViewsProvider.tsx @@ -10,6 +10,7 @@ import { type SavedView, } from "../lib/api"; import { filtersToParams } from "../lib/urlState"; +import { preferSticky } from "../lib/savedViews"; import { notifyActionError } from "../lib/actionErrors"; import { accountKey, LS, writeJSON } from "../lib/storage"; import { useConfirm } from "./ConfirmProvider"; @@ -85,12 +86,16 @@ export function SavedViewsProvider({ children }: { children: ReactNode }) { }, [viewsQuery.data]); const currentKey = keyOf(filters); - const activeView = useMemo(() => { - const matches = views.filter((v) => keyOf(v.filters) === currentKey); + const activeView = useMemo( // When several views capture identical filters, prefer the one the user explicitly applied // (sticky) so apply(B) isn't silently re-attributed to an earlier duplicate A; else the first. - return matches.find((v) => v.id === stickyId) ?? matches[0] ?? null; - }, [views, currentKey, stickyId]); + () => + preferSticky( + views.filter((v) => keyOf(v.filters) === currentKey), + stickyId, + ), + [views, currentKey, stickyId], + ); // An exact match becomes the sticky selection, so filters that happen to equal a view (deep-link, // manual, or an applied view) name that view in the chip and keep naming it after a later tweak. useEffect(() => { diff --git a/frontend/src/lib/savedViews.test.ts b/frontend/src/lib/savedViews.test.ts new file mode 100644 index 00000000..3ebe93d5 --- /dev/null +++ b/frontend/src/lib/savedViews.test.ts @@ -0,0 +1,26 @@ +import { describe, it, expect } from "vitest"; +import { preferSticky } from "./savedViews"; + +const v = (id: number) => ({ id, name: `v${id}` }); + +describe("preferSticky", () => { + it("returns null when nothing matches", () => { + expect(preferSticky([], 3)).toBeNull(); + expect(preferSticky([], null)).toBeNull(); + }); + + it("returns the only match regardless of stickyId", () => { + expect(preferSticky([v(5)], null)?.id).toBe(5); + expect(preferSticky([v(5)], 99)?.id).toBe(5); + }); + + it("prefers the sticky view among several identical-filter matches", () => { + // A (id 1) is earlier in the list, B (id 2) was explicitly applied (sticky). + expect(preferSticky([v(1), v(2)], 2)?.id).toBe(2); + }); + + it("falls back to the first match when the sticky id isn't among the matches", () => { + expect(preferSticky([v(1), v(2)], 99)?.id).toBe(1); + expect(preferSticky([v(1), v(2)], null)?.id).toBe(1); + }); +}); diff --git a/frontend/src/lib/savedViews.ts b/frontend/src/lib/savedViews.ts new file mode 100644 index 00000000..cce4e46a --- /dev/null +++ b/frontend/src/lib/savedViews.ts @@ -0,0 +1,12 @@ +// Pure selection helpers for saved ("personal") views, split out of SavedViewsProvider so the +// tie-break logic is unit-testable without mounting the provider (its React Query / context deps). + +/** Among the views whose filters match the live filters, pick the one to treat as active: prefer the + * explicitly-applied ("sticky") view when several capture identical filters, so apply(B) isn't + * silently re-attributed to an earlier duplicate A; otherwise the first match. Null if none match. */ +export function preferSticky( + matches: T[], + stickyId: number | null, +): T | null { + return matches.find((v) => v.id === stickyId) ?? matches[0] ?? null; +}