fix(feed): a filter on a deleted tag matches nothing and its chip says so
A saved view or the stored filter outlives its tag. feedquery dropped the unknown id, so the view showed the whole feed under its old name. The id now lands in a MISSING_TAG_CATEGORY that filters to nothing in the SQL and in feed_summary's mirror alike; the feed chip reads "Deleted tag" (a placeholder while the tag list loads, the id if it failed). Channels' persisted tag filter gets the same chip, first in the row so the +N overflow cannot fold it away.
This commit is contained in:
@@ -106,15 +106,26 @@ def _tag_visible(user: User):
|
||||
return or_(ChannelTag.user_id.is_(None), ChannelTag.user_id == user.id)
|
||||
|
||||
|
||||
# The pseudo-category of a selected tag id no Tag row answers (a saved view or the feed state outlives
|
||||
# a deleted tag). It filters like a category whose tags no channel carries — to nothing, in the SQL and
|
||||
# in feed_summary's mirror alike — instead of the id being dropped, which showed the WHOLE feed under
|
||||
# the view's old name (E32 S2). No real category is called this (language/topic/other).
|
||||
MISSING_TAG_CATEGORY = "missing"
|
||||
|
||||
|
||||
def tags_by_category(db: Session, tag_ids: list[int]) -> dict[str, list[int]]:
|
||||
"""Group tag ids by their category (language/topic/other). The single source for the category split
|
||||
shared by the feed's tag WHERE (filtered_query) and the facet/count summary (feed_summary), so the
|
||||
two can't disagree on which selected tag belongs to which category."""
|
||||
two can't disagree on which selected tag belongs to which category. An unknown id lands in
|
||||
MISSING_TAG_CATEGORY."""
|
||||
if not tag_ids:
|
||||
return {} # skip the pointless `WHERE id IN ()` round-trip on the no-tag-filter hot path
|
||||
out: dict[str, list[int]] = {}
|
||||
for tid, cat in db.execute(select(Tag.id, Tag.category).where(Tag.id.in_(tag_ids))).all():
|
||||
out.setdefault(cat, []).append(tid)
|
||||
known = {tid for ids in out.values() for tid in ids}
|
||||
if missing := [tid for tid in tag_ids if tid not in known]:
|
||||
out[MISSING_TAG_CATEGORY] = missing
|
||||
return out
|
||||
|
||||
|
||||
|
||||
@@ -168,9 +168,30 @@ _SUMMARY_CASES = [
|
||||
"show=all&untagged=true&tags={L1}",
|
||||
"show=all&q=clip",
|
||||
"show=unwatched",
|
||||
# A deleted tag's id (a saved view or the feed state still holds it) matches nothing.
|
||||
"show=all&tags=987654",
|
||||
"show=all&tags={L1}&tags=987654",
|
||||
]
|
||||
|
||||
|
||||
def test_a_deleted_tags_id_filters_to_nothing_not_to_everything(db, as_user, make_user):
|
||||
"""A saved view or the feed's filter state keeps a tag id after the tag is deleted. The feed used
|
||||
to drop an unknown id, so that view silently showed the WHOLE feed under its old name (E32 S2).
|
||||
A filter on a tag that no longer exists matches nothing; the chip says the tag is gone."""
|
||||
user, tag_ids = _seed_tagged(db, make_user)
|
||||
client = as_user(user)
|
||||
everything = client.get("/api/feed/count?show=all")
|
||||
assert everything.status_code == 200
|
||||
assert everything.json()["count"] == 8
|
||||
for qs in ("show=all&tags=987654", f"show=all&tags={tag_ids['L1']}&tags=987654"):
|
||||
r = client.get(f"/api/feed/count?{qs}")
|
||||
assert r.status_code == 200
|
||||
assert r.json()["count"] == 0, qs
|
||||
r = client.get(f"/api/feed?{qs}")
|
||||
assert r.status_code == 200
|
||||
assert r.json()["items"] == [], qs
|
||||
|
||||
|
||||
def test_untagged_filter_keeps_only_channels_the_user_has_not_tagged(db, as_user, make_user):
|
||||
"""The "No tag" filter (item B): `untagged=true` keeps only channels with NO tag the USER personally
|
||||
attached (no user-owned ChannelTag). System/auto tags (user_id NULL) don't count, and another user's
|
||||
|
||||
@@ -16,7 +16,7 @@ import { api, NO_TAG_SENTINEL, type ManagedChannel } from "../lib/api";
|
||||
import { Sparkles } from "lucide-react";
|
||||
import { notifyYouTubeActionError } from "../lib/youtubeErrors";
|
||||
import { accountKey, LS } from "../lib/storage";
|
||||
import { buildChannelColumns, applySyncFilter } from "./channelColumns";
|
||||
import { buildChannelColumns, applySyncFilter, tagFilterChips } from "./channelColumns";
|
||||
import { notify } from "../lib/notifications";
|
||||
import { Badge } from "./ui/Badge";
|
||||
import { Button } from "./ui/Button";
|
||||
@@ -39,7 +39,7 @@ import { focusNavItem } from "../lib/listNav";
|
||||
import ViewSwitcher, { type ViewOption } from "./ViewSwitcher";
|
||||
import ColumnSortControl from "./ui/ColumnSortControl";
|
||||
import FilterChipGroup from "./ui/FilterChipGroup";
|
||||
import TagFilterChips, { type FilterChip } from "./TagFilterChips";
|
||||
import TagFilterChips from "./TagFilterChips";
|
||||
import ActionMenu, { type ActionMenuItem } from "./ui/ActionMenu";
|
||||
import ChannelStatsPopover from "./ChannelStatsPopover";
|
||||
import Pager from "./ui/Pager";
|
||||
@@ -345,22 +345,17 @@ export default function Channels() {
|
||||
const noTagOn = tagFilter.includes(NO_TAG_SENTINEL);
|
||||
const toggleNoTagFilter = () =>
|
||||
setFilters({ ...filters, tags: noTagOn ? [] : [NO_TAG_SENTINEL] });
|
||||
// The tag-filter chips (user tags + the "No tag" sentinel), fed to TagFilterChips which folds any
|
||||
// overflow into a "+N" pill so the row stays one line and the trailing "…" actions can't wrap.
|
||||
const tagFilterChips: FilterChip[] = [
|
||||
...userTags.map((tg) => ({
|
||||
key: `t${tg.id}`,
|
||||
label: tg.name,
|
||||
active: tagFilter.includes(String(tg.id)),
|
||||
onToggle: () => toggleTagFilter(tg.id),
|
||||
})),
|
||||
{
|
||||
key: NO_TAG_SENTINEL,
|
||||
label: t("channels.tags.noTag"),
|
||||
active: noTagOn,
|
||||
onToggle: toggleNoTagFilter,
|
||||
},
|
||||
];
|
||||
// The tag-filter chips, fed to TagFilterChips which folds any overflow into a "+N" pill so the row
|
||||
// stays one line and the trailing "…" actions can't wrap.
|
||||
const tagChips = tagFilterChips({
|
||||
userTags,
|
||||
allTags,
|
||||
picked: tagFilter,
|
||||
noTagOn,
|
||||
toggle: toggleTagFilter,
|
||||
toggleNoTag: toggleNoTagFilter,
|
||||
t,
|
||||
});
|
||||
// A focus-channel intent (header "without full history" / tag manager) seeds the search box.
|
||||
// The TOKEN is the intent (it advances even when the same channel is re-focused); the name is the
|
||||
// payload, so it is read through an effect event. Naming it would re-seed the box on any render that
|
||||
@@ -735,7 +730,7 @@ export default function Channels() {
|
||||
</Button>
|
||||
</Tooltip>
|
||||
<FilterFoldButton fold={fold} label={t("sidebar.filters")} count={foldCount} />
|
||||
{!fold.folded && <TagFilterChips chips={tagFilterChips} className="flex-1" />}
|
||||
{!fold.folded && <TagFilterChips chips={tagChips} className="flex-1" />}
|
||||
{!fold.folded && tagFilter.length > 0 && (
|
||||
<Button
|
||||
variant="link-muted"
|
||||
|
||||
@@ -5,8 +5,14 @@
|
||||
// "fully" | "needsFull" | "" vocab + predicate — asserted here so a regression can't silently change
|
||||
// which channels the top chips show.
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { applySyncFilter, buildChannelColumns, tagsColumnTest } from "./channelColumns";
|
||||
import { NO_TAG_SENTINEL, type ManagedChannel } from "../lib/api";
|
||||
import {
|
||||
applySyncFilter,
|
||||
buildChannelColumns,
|
||||
tagFilterChips,
|
||||
tagsColumnTest,
|
||||
} from "./channelColumns";
|
||||
import { NO_TAG_SENTINEL, type ManagedChannel, type Tag } from "../lib/api";
|
||||
import type { TFunction } from "i18next";
|
||||
|
||||
const ch = (over: Partial<ManagedChannel>): ManagedChannel => ({
|
||||
id: "c",
|
||||
@@ -72,6 +78,33 @@ describe("tagsColumnTest (item B — 'No tag')", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("tagFilterChips — a pick whose tag is gone (E32 S2)", () => {
|
||||
const mine = { id: 1, name: "Music", system: false } as Tag;
|
||||
const t = ((k: string) => k) as unknown as TFunction;
|
||||
const chips = (allTags: Tag[] | undefined, picked: string[]) =>
|
||||
tagFilterChips({
|
||||
userTags: allTags ? [mine] : [],
|
||||
allTags,
|
||||
picked,
|
||||
noTagOn: false,
|
||||
toggle: () => {},
|
||||
toggleNoTag: () => {},
|
||||
t,
|
||||
});
|
||||
|
||||
it("stays a visible, active chip instead of an empty table with no reason", () => {
|
||||
expect(chips([mine], ["1", "77"]).map((c) => [c.key, c.label, c.active])).toEqual([
|
||||
["t77", "channels.tags.deleted", true], // first: the "+N" overflow must not fold it away
|
||||
["t1", "Music", true],
|
||||
[NO_TAG_SENTINEL, "channels.tags.noTag", false],
|
||||
]);
|
||||
});
|
||||
|
||||
it("is not called deleted before the tag list has loaded", () => {
|
||||
expect(chips(undefined, ["77"]).map((c) => c.key)).toEqual([NO_TAG_SENTINEL]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildChannelColumns — the row's end", () => {
|
||||
it('ends every row in the channel\'s "…", with no action buttons before it', () => {
|
||||
const more = (c: ManagedChannel) => `more:${c.id}`;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { ComponentProps, ReactNode } from "react";
|
||||
import type { TFunction } from "i18next";
|
||||
import { NO_TAG_SENTINEL, type ManagedChannel } from "../lib/api";
|
||||
import { NO_TAG_SENTINEL, type ManagedChannel, type Tag } from "../lib/api";
|
||||
import { formatCountOrDash, formatTotalHours } from "../lib/format";
|
||||
import type { Column } from "./ui/DataTable";
|
||||
import { PriorityStepper } from "./ui/PriorityStepper";
|
||||
@@ -10,6 +10,7 @@ import { ChannelTagsControl, type SuggestedTag } from "./ChannelTags";
|
||||
import { StoredCell } from "./channelCells";
|
||||
import { isFullySynced } from "./channelSync";
|
||||
import { RelativeTime } from "./ui/RelativeTime";
|
||||
import type { FilterChip } from "./TagFilterChips";
|
||||
|
||||
/** The shared "subscribers" DataTable column (right-aligned, sortable, em-dash for null) — identical
|
||||
* in the Channels manager and the ChannelDiscovery table, which show subscriber counts on different
|
||||
@@ -49,6 +50,43 @@ export function tagsColumnTest(c: ManagedChannel, values: string[]): boolean {
|
||||
return values.some((v) => c.tag_ids.includes(Number(v)));
|
||||
}
|
||||
|
||||
/** The manager's tag-filter chip row (the Tags column filter's values): any picked id whose tag no
|
||||
* longer exists, then each user tag, then the "No tag" sentinel. A stored pick can outlive its tag
|
||||
* (deleted here or on another device) and still narrows the table to nothing, so it stays a visible,
|
||||
* removable chip — FIRST, so the row's "+N" overflow never folds the reason away (E32 S2). */
|
||||
export function tagFilterChips(o: {
|
||||
userTags: Tag[];
|
||||
allTags: Tag[] | undefined;
|
||||
picked: string[];
|
||||
noTagOn: boolean;
|
||||
toggle: (tagId: number) => void;
|
||||
toggleNoTag: () => void;
|
||||
t: TFunction;
|
||||
}): FilterChip[] {
|
||||
const known = new Set((o.allTags ?? []).map((tg) => String(tg.id)));
|
||||
const gone = o.allTags ? o.picked.filter((id) => id !== NO_TAG_SENTINEL && !known.has(id)) : [];
|
||||
return [
|
||||
...gone.map((id) => ({
|
||||
key: `t${id}`,
|
||||
label: o.t("channels.tags.deleted"),
|
||||
active: true,
|
||||
onToggle: () => o.toggle(Number(id)),
|
||||
})),
|
||||
...o.userTags.map((tg) => ({
|
||||
key: `t${tg.id}`,
|
||||
label: tg.name,
|
||||
active: o.picked.includes(String(tg.id)),
|
||||
onToggle: () => o.toggle(tg.id),
|
||||
})),
|
||||
{
|
||||
key: NO_TAG_SENTINEL,
|
||||
label: o.t("channels.tags.noTag"),
|
||||
active: o.noTagOn,
|
||||
onToggle: o.toggleNoTag,
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
/** Everything the Channels-manager column set closes over. The manager owns the state + mutations;
|
||||
* this factory just wires them into `Column<ManagedChannel>` render/filter/sort defs so the ~150-line
|
||||
* column block lives outside the page component (its cell components are in channelCells). */
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// A saved view or the stored feed filter can outlive its tag. The feed then matches nothing (E32 S2),
|
||||
// so the chip must say the tag is gone, not read as a bare id; while the tag list loads it is a
|
||||
// placeholder, and if the list fails it falls back to the id rather than claiming "deleted".
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, renderHook, waitFor } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { createElement, type ReactNode } from "react";
|
||||
import { api, type FeedFilters, type Tag } from "../lib/api";
|
||||
import { clearedFilters, useActiveFilters } from "../lib/useActiveFilters";
|
||||
|
||||
vi.mock("react-i18next", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("react-i18next")>();
|
||||
return {
|
||||
...actual,
|
||||
useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
|
||||
};
|
||||
});
|
||||
|
||||
const filters: FeedFilters = { ...clearedFilters({ q: "" } as FeedFilters), tags: [1, 99] };
|
||||
let qc: QueryClient;
|
||||
const wrapper = ({ children }: { children: ReactNode }) =>
|
||||
createElement(QueryClientProvider, { client: qc }, children);
|
||||
const chips = () => renderHook(() => useActiveFilters(filters, () => {}), { wrapper }).result;
|
||||
|
||||
beforeEach(() => {
|
||||
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
});
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("a tag filter's chip names", () => {
|
||||
it("name a live tag, and say a deleted one is gone", async () => {
|
||||
vi.spyOn(api, "tags").mockResolvedValue([{ id: 1, name: "Music", category: "topic" }] as Tag[]);
|
||||
const result = chips();
|
||||
const chip = (id: number) => result.current.find((f) => f.key === `tag:${id}`)!;
|
||||
expect(chip(99).pending).toBe(true);
|
||||
await waitFor(() => expect(chip(1)).toMatchObject({ label: "Music", pending: false }));
|
||||
expect(chip(99)).toMatchObject({ label: "feed.chips.deletedTag", pending: false });
|
||||
});
|
||||
|
||||
it("fall back to the id when the tag list fails, never claiming the tag was deleted", async () => {
|
||||
vi.spyOn(api, "tags").mockRejectedValue(new Error("down"));
|
||||
const result = chips();
|
||||
const chip = (id: number) => result.current.find((f) => f.key === `tag:${id}`)!;
|
||||
await waitFor(() => expect(chip(99).pending).toBe(false));
|
||||
expect(chip(99).label).toBe("99");
|
||||
});
|
||||
});
|
||||
@@ -57,6 +57,7 @@
|
||||
"createFailed": "Couldn't create the tag",
|
||||
"updateFailed": "Couldn't update the tag",
|
||||
"noTag": "No tag",
|
||||
"deleted": "Deleted tag",
|
||||
"moreCount_one": "{{count}} more tag",
|
||||
"moreCount_other": "{{count}} more tags",
|
||||
"suggestedHint": "Suggested from your tagging pattern ({{pct}}% match) — click to add",
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
"remove": "Remove filter: {{name}}",
|
||||
"contentNone": "No content types",
|
||||
"untagged": "Untagged",
|
||||
"deletedTag": "Deleted tag",
|
||||
"count_one": "{{count}} filter",
|
||||
"count_other": "{{count}} filters",
|
||||
"moreCount_one": "{{count}} more filter",
|
||||
|
||||
@@ -57,6 +57,7 @@
|
||||
"createFailed": "Nem sikerült létrehozni a címkét",
|
||||
"updateFailed": "Nem sikerült frissíteni a címkét",
|
||||
"noTag": "Nincs címke",
|
||||
"deleted": "Törölt címke",
|
||||
"moreCount_one": "még {{count}} címke",
|
||||
"moreCount_other": "még {{count}} címke",
|
||||
"suggestedHint": "A címkézési mintád alapján javasolt ({{pct}}% egyezés) — kattints a hozzáadáshoz",
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
"remove": "Szűrő eltávolítása: {{name}}",
|
||||
"contentNone": "Nincs tartalomtípus",
|
||||
"untagged": "Címkézetlen",
|
||||
"deletedTag": "Törölt címke",
|
||||
"count_one": "{{count}} szűrő",
|
||||
"count_other": "{{count}} szűrő",
|
||||
"moreCount_one": "még {{count}} szűrő",
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { TFunction } from "i18next";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { DEFAULT_TAG_MODES, type FeedFilters } from "./api";
|
||||
import { DEFAULT_TAG_MODES, type FeedFilters, type Tag } from "./api";
|
||||
import { useChannelNames } from "./useChannelNames";
|
||||
import { parseSort } from "./feedSort";
|
||||
import { useTags } from "./useTags";
|
||||
@@ -35,6 +36,15 @@ export function clearedFilters(current: FeedFilters): FeedFilters {
|
||||
return { ...DEFAULT_SIDEBAR_FILTERS, q: current.q, scope: "my" };
|
||||
}
|
||||
|
||||
// A tag chip's name. A saved view or the stored filter can outlive its tag: the feed then matches
|
||||
// nothing (E32 S2), and the chip says why instead of reading as a bare id — but only once the tag
|
||||
// list has loaded; while it loads (a placeholder) or if it failed, the id, never a guessed "deleted".
|
||||
function tagLabel(id: number, tags: { data?: Tag[]; isSuccess: boolean }, t: TFunction): string {
|
||||
const tag = tags.data?.find((x) => x.id === id);
|
||||
if (tag) return tag.name;
|
||||
return tags.isSuccess ? t("feed.chips.deletedTag") : String(id);
|
||||
}
|
||||
|
||||
/**
|
||||
* The single source of truth for "what is currently narrowing the feed" — it backs BOTH the side
|
||||
* panel's active-count badge and the removable chip row above the feed, so the two can never
|
||||
@@ -56,10 +66,10 @@ export function useActiveFilters(
|
||||
|
||||
// Tags: one chip each — removing one shouldn't drop the rest.
|
||||
for (const id of filters.tags) {
|
||||
const tag = (tagsQuery.data ?? []).find((x) => x.id === id);
|
||||
out.push({
|
||||
key: `tag:${id}`,
|
||||
label: tag?.name ?? String(id),
|
||||
label: tagLabel(id, tagsQuery, t),
|
||||
pending: tagsQuery.isPending,
|
||||
remove: () => setFilters({ ...filters, tags: filters.tags.filter((x) => x !== id) }),
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user