From cd42610c957666e1fcc8de4fb058b92536d00a32 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Tue, 26 May 2026 15:11:10 -0400 Subject: [PATCH] fix(search): address request search review feedback --- .../2026-05-25-search-request-section.md | 2 + web/src/components/GlobalSearch.test.tsx | 6 ++- web/src/components/GlobalSearch.tsx | 2 + .../components/RequestToAddSection.test.tsx | 48 ++++++++++++++++++- web/src/components/RequestToAddSection.tsx | 29 +++++++---- web/src/hooks/queries/useRequests.test.tsx | 36 +++++++++++++- web/src/hooks/queries/useRequests.ts | 11 +++-- web/src/pages/Catalog.test.tsx | 6 ++- web/src/pages/Catalog.tsx | 2 + 9 files changed, 125 insertions(+), 17 deletions(-) diff --git a/docs/superpowers/plans/2026-05-25-search-request-section.md b/docs/superpowers/plans/2026-05-25-search-request-section.md index 269666b9..b276015c 100644 --- a/docs/superpowers/plans/2026-05-25-search-request-section.md +++ b/docs/superpowers/plans/2026-05-25-search-request-section.md @@ -1,5 +1,7 @@ # Search Request Section Implementation Plan +Commands assume the repository root is the cwd. + > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Add a TMDB-backed "Request to Add" section beneath library results in both the Cmd+K search dialog (`GlobalSearch`) and the Catalog search results page, so users can discover and request items missing from the library without leaving the search flow. diff --git a/web/src/components/GlobalSearch.test.tsx b/web/src/components/GlobalSearch.test.tsx index ca67ae5d..1dccbf86 100644 --- a/web/src/components/GlobalSearch.test.tsx +++ b/web/src/components/GlobalSearch.test.tsx @@ -240,7 +240,11 @@ describe("GlobalSearch + RequestToAddSection wiring", () => { renderSearchMarkup({ defaultOpen: true, initialQuery: "Dune" }); const call = mocks.useRequestSearch.mock.calls[mocks.useRequestSearch.mock.calls.length - 1]; - expect(call?.[3]).toEqual({ enabled: false }); + expect(call?.[3]).toEqual({ + enabled: false, + requireProfile: true, + staleTime: 5 * 60 * 1000, + }); }); it("does not mount RequestToAddSection when discovery is disabled", () => { diff --git a/web/src/components/GlobalSearch.tsx b/web/src/components/GlobalSearch.tsx index 5eaf3f80..f9bcc8b1 100644 --- a/web/src/components/GlobalSearch.tsx +++ b/web/src/components/GlobalSearch.tsx @@ -108,6 +108,8 @@ export function GlobalSearch({ const canRequest = useCanRequest(); const tmdbQuery = useRequestSearch("all", tmdbDebouncedQuery, 1, { enabled: canRequest.discoveryEnabled, + requireProfile: true, + staleTime: 5 * 60 * 1000, }); const tmdbMissingCount = tmdbQuery.data?.results?.filter((result) => result.availability !== "available").length ?? 0; diff --git a/web/src/components/RequestToAddSection.test.tsx b/web/src/components/RequestToAddSection.test.tsx index 14404e5f..0d041bae 100644 --- a/web/src/components/RequestToAddSection.test.tsx +++ b/web/src/components/RequestToAddSection.test.tsx @@ -95,7 +95,11 @@ describe("RequestToAddSection (dialog variant)", () => { expect(call?.[0]).toBe("all"); expect(call?.[1]).toBe("dune"); expect(call?.[2]).toBe(1); - expect(call?.[3]).toEqual({ enabled: false }); + expect(call?.[3]).toEqual({ + enabled: false, + requireProfile: true, + staleTime: 5 * 60 * 1000, + }); }); it("passes enabled=true to useRequestSearch when discovery is enabled", () => { @@ -113,7 +117,11 @@ describe("RequestToAddSection (dialog variant)", () => { render(); const call = mocks.useRequestSearch.mock.calls[mocks.useRequestSearch.mock.calls.length - 1]; - expect(call?.[3]).toEqual({ enabled: true }); + expect(call?.[3]).toEqual({ + enabled: true, + requireProfile: true, + staleTime: 5 * 60 * 1000, + }); }); it("renders 'Request to Add' header when library had hits", () => { @@ -166,6 +174,16 @@ describe("RequestToAddSection (dialog variant)", () => { expect(markup).toBe(""); }); + it("keeps rendering cached TMDB results when a refetch errors", () => { + mocks.useRequestSearch.mockReturnValue({ + data: { page: 1, total_pages: 1, total_results: 1, results: [missingResult()] }, + isLoading: false, + isError: true, + }); + const markup = render(); + expect(markup).toContain("Dune: Prophecy"); + }); + it("renders nothing when all TMDB results are already in the library", () => { mocks.useRequestSearch.mockReturnValue({ data: { page: 1, total_pages: 1, total_results: 1, results: [availableResult()] }, @@ -218,6 +236,32 @@ describe("RequestToAddSection (dialog variant)", () => { expect(markup).toContain("Limit reached"); expect(markup).toContain('title="Limit reached"'); }); + + it("prefers request status over reason when a row is already requested", () => { + mocks.useRequestSearch.mockReturnValue({ + data: { + page: 1, + total_pages: 1, + total_results: 1, + results: [ + missingResult({ + tmdb_id: 8, + title: "Already Pending Movie", + request: { requestable: false, reason: "blocked", status: "pending" }, + }), + ], + }, + isLoading: false, + isError: false, + }); + + const markup = render(); + + expect(markup).toContain("Already Pending Movie"); + expect(markup).toContain("Pending"); + expect(markup).toContain('title="Pending"'); + expect(markup).not.toContain('title="Blocked"'); + }); }); describe("RequestToAddSection (grid variant)", () => { diff --git a/web/src/components/RequestToAddSection.tsx b/web/src/components/RequestToAddSection.tsx index ffcd9dd4..5f3aba5b 100644 --- a/web/src/components/RequestToAddSection.tsx +++ b/web/src/components/RequestToAddSection.tsx @@ -6,6 +6,7 @@ import { useCreateMediaRequest, useRequestSearch } from "@/hooks/queries/useRequ import type { RequestMediaResult } from "@/api/types"; import { formatRequestReason, + formatRequestStatus, requestInputFromMediaResult, tmdbImageURL, } from "@/lib/mediaRequests"; @@ -16,6 +17,16 @@ function cardKey(item: Pick): stri return `${item.media_type}-${item.tmdb_id}`; } +function nonRequestableLabel(item: RequestMediaResult): string { + if (item.request.status) { + return formatRequestStatus(item.request.status); + } + if (item.request.reason) { + return formatRequestReason(item.request.reason); + } + return "Blocked"; +} + const DIALOG_LIMIT = 4; const GRID_LIMIT = 20; @@ -28,10 +39,14 @@ export type RequestToAddSectionProps = { export function RequestToAddSection({ variant, query, libraryHadHits }: RequestToAddSectionProps) { const { discoveryEnabled } = useCanRequest(); - const search = useRequestSearch("all", query, 1, { enabled: discoveryEnabled }); + const search = useRequestSearch("all", query, 1, { + enabled: discoveryEnabled, + requireProfile: true, + staleTime: 5 * 60 * 1000, + }); if (!discoveryEnabled) return null; - if (search.isError) return null; + if (search.isError && !search.data) return null; const filtered = (search.data?.results ?? []).filter((item) => item.availability !== "available"); if (filtered.length === 0) return null; @@ -89,11 +104,7 @@ function DialogRow({ item }: { item: RequestMediaResult }) { const poster = tmdbImageURL(item.poster_path); const Icon = item.media_type === "series" ? Tv : Film; const requestable = item.request.requestable; - const reasonLabel = !requestable - ? item.request.reason - ? formatRequestReason(item.request.reason) - : "Blocked" - : null; + const unavailableLabel = requestable ? null : nonRequestableLabel(item); return ( - {reasonLabel} + {unavailableLabel} )} diff --git a/web/src/hooks/queries/useRequests.test.tsx b/web/src/hooks/queries/useRequests.test.tsx index a59213b6..4ea70af2 100644 --- a/web/src/hooks/queries/useRequests.test.tsx +++ b/web/src/hooks/queries/useRequests.test.tsx @@ -80,10 +80,23 @@ describe("useRequestSearch", () => { expect(init.signal).toBe(controller.signal); }); - it("uses a 5-minute staleTime", () => { + it("keeps the existing Requests page staleTime by default", () => { mocks.useCurrentProfile.mockReturnValue({ profile: { id: "p" } }); render(); + const options = mocks.useQuery.mock.calls[0]![0] as { staleTime: number }; + expect(options.staleTime).toBe(30 * 1000); + }); + + it("allows callers to opt into a longer staleTime", () => { + mocks.useCurrentProfile.mockReturnValue({ profile: { id: "p" } }); + + function CallHookWithStaleTime() { + useRequestSearch("all", "dune", 1, { staleTime: 5 * 60 * 1000 }); + return null; + } + render(); + const options = mocks.useQuery.mock.calls[0]![0] as { staleTime: number }; expect(options.staleTime).toBe(5 * 60 * 1000); }); @@ -108,6 +121,27 @@ describe("useRequestSearch", () => { const options = mocks.useQuery.mock.calls[0]![0] as { enabled: boolean }; expect(options.enabled).toBe(true); }); + + it("does not require profile by default", () => { + mocks.useCurrentProfile.mockReturnValue({ profile: null }); + render(); + + const options = mocks.useQuery.mock.calls[0]![0] as { enabled: boolean }; + expect(options.enabled).toBe(true); + }); + + it("can require a profile before fetching", () => { + mocks.useCurrentProfile.mockReturnValue({ profile: null }); + + function CallHookRequiringProfile() { + useRequestSearch("all", "dune", 1, { requireProfile: true }); + return null; + } + render(); + + const options = mocks.useQuery.mock.calls[0]![0] as { enabled: boolean }; + expect(options.enabled).toBe(false); + }); }); describe("requestKeys.all invalidation", () => { diff --git a/web/src/hooks/queries/useRequests.ts b/web/src/hooks/queries/useRequests.ts index 7ffe2959..f9423dda 100644 --- a/web/src/hooks/queries/useRequests.ts +++ b/web/src/hooks/queries/useRequests.ts @@ -29,7 +29,6 @@ import type { import { adminKeys, requestKeys } from "./keys"; const REQUESTS_STALE_TIME = 30_000; -const REQUEST_SEARCH_STALE_TIME = 5 * 60 * 1000; const DISCOVER_BRAND_STALE_TIME = 24 * 60 * 60 * 1000; const BROWSE_STALE_TIME = 60 * 1000; @@ -156,6 +155,10 @@ export function useRequestMediaDetail(mediaType: RequestMediaType, tmdbID: numbe export interface UseRequestSearchOptions { /** When false, suppresses the query regardless of the query string. Default: true. */ enabled?: boolean; + /** When true, suppresses the query until the active profile is loaded. Default: false. */ + requireProfile?: boolean; + /** Cache freshness window for this search surface. Default: existing Requests page timing. */ + staleTime?: number; } export function useRequestSearch( @@ -172,6 +175,7 @@ export function useRequestSearch( // later be read by a different viewer. const viewerKey = profile?.id ?? "anon"; const enabledOverride = options.enabled ?? true; + const requireProfile = options.requireProfile ?? false; return useQuery({ queryKey: requestKeys.search(mediaType, normalizedQuery, page, viewerKey), @@ -183,8 +187,9 @@ export function useRequestSearch( }); return api(`/requests/search?${params}`, { signal }); }, - enabled: enabledOverride && normalizedQuery.length > 1 && Boolean(profile?.id), - staleTime: REQUEST_SEARCH_STALE_TIME, + enabled: + enabledOverride && normalizedQuery.length > 1 && (!requireProfile || Boolean(profile?.id)), + staleTime: options.staleTime ?? REQUESTS_STALE_TIME, }); } diff --git a/web/src/pages/Catalog.test.tsx b/web/src/pages/Catalog.test.tsx index da662804..bfcdd5cf 100644 --- a/web/src/pages/Catalog.test.tsx +++ b/web/src/pages/Catalog.test.tsx @@ -441,7 +441,11 @@ describe("Catalog page", () => { ); const call = mockUseRequestSearch.mock.calls[mockUseRequestSearch.mock.calls.length - 1]; - expect(call?.[3]).toEqual({ enabled: false }); + expect(call?.[3]).toEqual({ + enabled: false, + requireProfile: true, + staleTime: 5 * 60 * 1000, + }); }); it("hides ItemGrid when library is empty and TMDB is still loading (request section will rescue)", () => { diff --git a/web/src/pages/Catalog.tsx b/web/src/pages/Catalog.tsx index 4be0434b..688f8d1e 100644 --- a/web/src/pages/Catalog.tsx +++ b/web/src/pages/Catalog.tsx @@ -112,6 +112,8 @@ function CatalogResults({ const tmdbDebouncedQ = useDebounce(state.q ?? "", 200); const tmdbQuery = useRequestSearch("all", tmdbDebouncedQ, 1, { enabled: canRequest.discoveryEnabled && isQuerySource, + requireProfile: true, + staleTime: 5 * 60 * 1000, }); const tmdbMissingCount = tmdbQuery.data?.results?.filter((result) => result.availability !== "available").length ?? 0;