fix(search): address request search review feedback
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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(<RequestToAddSection variant="dialog" query="dune" libraryHadHits />);
|
||||
|
||||
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(<RequestToAddSection variant="dialog" query="dune" libraryHadHits />);
|
||||
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(<RequestToAddSection variant="dialog" query="dune" libraryHadHits />);
|
||||
|
||||
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)", () => {
|
||||
|
||||
@@ -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<RequestMediaResult, "media_type" | "tmdb_id">): 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 (
|
||||
<Link
|
||||
@@ -128,9 +139,9 @@ function DialogRow({ item }: { item: RequestMediaResult }) {
|
||||
) : (
|
||||
<span
|
||||
className="rounded-full border border-white/10 bg-white/5 px-2 py-0.5 text-[9px] font-medium tracking-[0.5px] text-white/60 uppercase"
|
||||
title={reasonLabel ?? "Blocked"}
|
||||
title={unavailableLabel ?? "Blocked"}
|
||||
>
|
||||
{reasonLabel}
|
||||
{unavailableLabel}
|
||||
</span>
|
||||
)}
|
||||
</Link>
|
||||
|
||||
@@ -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(<CallHook mediaType="all" q="dune" />);
|
||||
|
||||
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(<CallHookWithStaleTime />);
|
||||
|
||||
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(<CallHook mediaType="all" q="dune" />);
|
||||
|
||||
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(<CallHookRequiringProfile />);
|
||||
|
||||
const options = mocks.useQuery.mock.calls[0]![0] as { enabled: boolean };
|
||||
expect(options.enabled).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("requestKeys.all invalidation", () => {
|
||||
|
||||
@@ -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<RequestMediaPage>(`/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,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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)", () => {
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user