Преглед изворни кода

RHL-037 refactor(search): refactor SearchPage and add pageHelpers for improved state management and URL handling

Code_Uwe пре 3 недеља
родитељ
комит
160daa168e

+ 26 - 77
components/search/SearchPage.jsx

@@ -22,6 +22,17 @@ import {
 	BRANCH_LIST_STATE,
 } from "@/lib/frontend/search/useSearchBranches";
 
+import {
+	buildSearchHref,
+	buildSearchKey,
+	getScopeLabel,
+	needsMultiBranchSelectionHint,
+	buildNextStateForScopeChange,
+	buildNextStateForToggleBranch,
+	buildNextStateForClearAllBranches,
+	buildHrefForSingleBranchSwitch,
+} from "@/lib/frontend/search/pageHelpers";
+
 import ExplorerPageShell from "@/components/explorer/ExplorerPageShell";
 import ExplorerSectionCard from "@/components/explorer/ExplorerSectionCard";
 import ForbiddenView from "@/components/system/ForbiddenView";
@@ -30,12 +41,6 @@ import { Button } from "@/components/ui/button";
 import SearchForm from "@/components/search/SearchForm";
 import SearchResults from "@/components/search/SearchResults";
 
-function buildSearchHref({ routeBranch, state }) {
-	const base = searchPath(routeBranch);
-	const qs = serializeSearchUrlState(state);
-	return qs ? `${base}?${qs}` : base;
-}
-
 export default function SearchPage({ branch: routeBranch }) {
 	const router = useRouter();
 	const searchParams = useSearchParams();
@@ -57,8 +62,7 @@ export default function SearchPage({ branch: routeBranch }) {
 	}, [parsedUrlState, routeBranch, user]);
 
 	const searchKey = React.useMemo(() => {
-		const qs = serializeSearchUrlState(urlState);
-		return `${routeBranch}|${qs}`;
+		return buildSearchKey({ routeBranch, urlState });
 	}, [routeBranch, urlState]);
 
 	const [qDraft, setQDraft] = React.useState(urlState.q || "");
@@ -113,25 +117,13 @@ export default function SearchPage({ branch: routeBranch }) {
 	);
 
 	const handleSubmit = React.useCallback(() => {
-		const nextState = {
-			...urlState,
-			q: qDraft,
-		};
-
-		pushStateToUrl(nextState);
-	}, [urlState, qDraft, pushStateToUrl]);
+		pushStateToUrl({ ...urlState, q: qDraft });
+	}, [pushStateToUrl, urlState, qDraft]);
 
 	const handleScopeChange = React.useCallback(
 		(nextScope) => {
 			if (!isAdminDev) return;
-
-			const nextState = {
-				...urlState,
-				scope: nextScope,
-				branches: nextScope === SEARCH_SCOPE.MULTI ? urlState.branches : [],
-			};
-
-			replaceStateToUrl(nextState);
+			replaceStateToUrl(buildNextStateForScopeChange({ urlState, nextScope }));
 		},
 		[isAdminDev, urlState, replaceStateToUrl]
 	);
@@ -139,44 +131,19 @@ export default function SearchPage({ branch: routeBranch }) {
 	const handleToggleBranch = React.useCallback(
 		(branchId) => {
 			if (!isAdminDev) return;
-
-			const current = Array.isArray(urlState.branches) ? urlState.branches : [];
-			const set = new Set(current);
-
-			if (set.has(branchId)) set.delete(branchId);
-			else set.add(branchId);
-
-			const nextState = {
-				...urlState,
-				scope: SEARCH_SCOPE.MULTI,
-				branches: Array.from(set),
-			};
-
-			replaceStateToUrl(nextState);
+			replaceStateToUrl(buildNextStateForToggleBranch({ urlState, branchId }));
 		},
 		[isAdminDev, urlState, replaceStateToUrl]
 	);
 
 	const handleClearAllBranches = React.useCallback(() => {
 		if (!isAdminDev) return;
-
-		const nextState = {
-			...urlState,
-			scope: SEARCH_SCOPE.MULTI,
-			branches: [],
-		};
-
-		replaceStateToUrl(nextState);
+		replaceStateToUrl(buildNextStateForClearAllBranches({ urlState }));
 	}, [isAdminDev, urlState, replaceStateToUrl]);
 
 	const handleLimitChange = React.useCallback(
 		(nextLimit) => {
-			const nextState = {
-				...urlState,
-				limit: nextLimit,
-			};
-
-			replaceStateToUrl(nextState);
+			replaceStateToUrl({ ...urlState, limit: nextLimit });
 		},
 		[urlState, replaceStateToUrl]
 	);
@@ -186,16 +153,10 @@ export default function SearchPage({ branch: routeBranch }) {
 			if (!isAdminDev) return;
 			if (!isValidBranchParam(nextBranch)) return;
 
-			const nextState = {
-				...urlState,
-				scope: SEARCH_SCOPE.SINGLE,
-				branches: [],
-			};
+			const href = buildHrefForSingleBranchSwitch({ nextBranch, urlState });
+			if (!href) return;
 
-			const base = searchPath(nextBranch);
-			const qs = serializeSearchUrlState(nextState);
-
-			router.push(qs ? `${base}?${qs}` : base);
+			router.push(href);
 		},
 		[isAdminDev, urlState, router]
 	);
@@ -223,28 +184,16 @@ export default function SearchPage({ branch: routeBranch }) {
 		</span>
 	) : null;
 
-	const multiCount = Array.isArray(urlState.branches)
-		? urlState.branches.length
-		: 0;
-
-	const scopeLabel =
-		urlState.scope === SEARCH_SCOPE.ALL
-			? "Alle Niederlassungen"
-			: urlState.scope === SEARCH_SCOPE.MULTI
-			? multiCount > 0
-				? `${multiCount} Niederlassung${multiCount === 1 ? "" : "en"}`
-				: "Mehrere Niederlassungen"
-			: `Niederlassung ${routeBranch}`;
+	const scopeLabel = getScopeLabel({ routeBranch, urlState });
 
 	const resultsDescription = urlState.q
 		? `Suchbereich: ${scopeLabel}`
 		: "Geben Sie einen Suchbegriff ein, um zu starten.";
 
-	const needsBranchSelection =
-		isAdminDev &&
-		urlState.scope === SEARCH_SCOPE.MULTI &&
-		Boolean(urlState.q) &&
-		multiCount === 0;
+	const needsBranchSelection = needsMultiBranchSelectionHint({
+		isAdminDev,
+		urlState,
+	});
 
 	return (
 		<ExplorerPageShell

+ 1 - 1
components/search/form/SearchLimitSelect.jsx

@@ -47,7 +47,7 @@ export default function SearchLimitSelect({
 					</Button>
 				</DropdownMenuTrigger>
 
-				<DropdownMenuContent align="start" className="min-w-[12rem]">
+				<DropdownMenuContent align="start" className="min-w-48">
 					<DropdownMenuLabel>Treffer pro Seite</DropdownMenuLabel>
 					<DropdownMenuSeparator />
 

+ 138 - 0
lib/frontend/search/pageHelpers.js

@@ -0,0 +1,138 @@
+import { searchPath } from "@/lib/frontend/routes";
+import {
+	serializeSearchUrlState,
+	SEARCH_SCOPE,
+} from "@/lib/frontend/search/urlState";
+import { isValidBranchParam } from "@/lib/frontend/params";
+
+/**
+ * Build the full /:branch/search href for a given state.
+ *
+ * @param {{ routeBranch: string, state: any }} input
+ * @returns {string}
+ */
+export function buildSearchHref({ routeBranch, state }) {
+	const base = searchPath(routeBranch);
+	const qs = serializeSearchUrlState(state);
+	return qs ? `${base}?${qs}` : base;
+}
+
+/**
+ * Build a stable searchKey identity (cursor excluded).
+ * Includes routeBranch to avoid cross-branch race conditions.
+ *
+ * @param {{ routeBranch: string, urlState: any }} input
+ * @returns {string}
+ */
+export function buildSearchKey({ routeBranch, urlState }) {
+	const qs = serializeSearchUrlState(urlState);
+	return `${routeBranch}|${qs}`;
+}
+
+/**
+ * Derive the results description label for the current scope.
+ *
+ * @param {{ routeBranch: string, urlState: any }} input
+ * @returns {string}
+ */
+export function getScopeLabel({ routeBranch, urlState }) {
+	const multiCount = Array.isArray(urlState?.branches)
+		? urlState.branches.length
+		: 0;
+
+	if (urlState?.scope === SEARCH_SCOPE.ALL) return "Alle Niederlassungen";
+
+	if (urlState?.scope === SEARCH_SCOPE.MULTI) {
+		if (multiCount > 0) {
+			return `${multiCount} Niederlassung${multiCount === 1 ? "" : "en"}`;
+		}
+		return "Mehrere Niederlassungen";
+	}
+
+	return `Niederlassung ${routeBranch}`;
+}
+
+/**
+ * Whether the UI should show the "select at least one branch" hint.
+ *
+ * @param {{ isAdminDev: boolean, urlState: any }} input
+ * @returns {boolean}
+ */
+export function needsMultiBranchSelectionHint({ isAdminDev, urlState }) {
+	const multiCount = Array.isArray(urlState?.branches)
+		? urlState.branches.length
+		: 0;
+
+	return (
+		Boolean(isAdminDev) &&
+		urlState?.scope === SEARCH_SCOPE.MULTI &&
+		Boolean(urlState?.q) &&
+		multiCount === 0
+	);
+}
+
+/**
+ * Build the next state for scope changes.
+ *
+ * @param {{ urlState: any, nextScope: string }} input
+ * @returns {any}
+ */
+export function buildNextStateForScopeChange({ urlState, nextScope }) {
+	return {
+		...urlState,
+		scope: nextScope,
+		branches: nextScope === SEARCH_SCOPE.MULTI ? urlState.branches : [],
+	};
+}
+
+/**
+ * Toggle one branch in MULTI mode.
+ *
+ * @param {{ urlState: any, branchId: string }} input
+ * @returns {any}
+ */
+export function buildNextStateForToggleBranch({ urlState, branchId }) {
+	const current = Array.isArray(urlState?.branches) ? urlState.branches : [];
+	const set = new Set(current);
+
+	if (set.has(branchId)) set.delete(branchId);
+	else set.add(branchId);
+
+	return {
+		...urlState,
+		scope: SEARCH_SCOPE.MULTI,
+		branches: Array.from(set),
+	};
+}
+
+/**
+ * Clear all branches in MULTI mode.
+ *
+ * @param {{ urlState: any }} input
+ * @returns {any}
+ */
+export function buildNextStateForClearAllBranches({ urlState }) {
+	return {
+		...urlState,
+		scope: SEARCH_SCOPE.MULTI,
+		branches: [],
+	};
+}
+
+/**
+ * Build a navigation target for switching SINGLE branch (path segment).
+ *
+ * @param {{ nextBranch: string, urlState: any }} input
+ * @returns {string|null}
+ */
+export function buildHrefForSingleBranchSwitch({ nextBranch, urlState }) {
+	if (!isValidBranchParam(nextBranch)) return null;
+
+	const nextState = {
+		...urlState,
+		scope: SEARCH_SCOPE.SINGLE,
+		branches: [],
+	};
+
+	return buildSearchHref({ routeBranch: nextBranch, state: nextState });
+}

+ 120 - 0
lib/frontend/search/pageHelpers.test.js

@@ -0,0 +1,120 @@
+/* @vitest-environment node */
+
+import { describe, it, expect } from "vitest";
+import { SEARCH_SCOPE } from "@/lib/frontend/search/urlState";
+import {
+	buildSearchHref,
+	buildSearchKey,
+	getScopeLabel,
+	needsMultiBranchSelectionHint,
+	buildNextStateForScopeChange,
+	buildNextStateForToggleBranch,
+	buildNextStateForClearAllBranches,
+	buildHrefForSingleBranchSwitch,
+} from "./pageHelpers.js";
+
+describe("lib/frontend/search/pageHelpers", () => {
+	it("buildSearchHref builds /:branch/search with query string", () => {
+		const href = buildSearchHref({
+			routeBranch: "NL01",
+			state: { q: "x", scope: SEARCH_SCOPE.SINGLE, limit: 200 },
+		});
+		expect(href).toBe("/NL01/search?q=x&limit=200");
+	});
+
+	it("buildSearchKey includes routeBranch", () => {
+		const key = buildSearchKey({
+			routeBranch: "NL01",
+			urlState: { q: "x", scope: SEARCH_SCOPE.SINGLE, limit: 200 },
+		});
+		expect(key).toBe("NL01|q=x&limit=200");
+	});
+
+	it("getScopeLabel returns correct labels", () => {
+		expect(
+			getScopeLabel({
+				routeBranch: "NL01",
+				urlState: { scope: SEARCH_SCOPE.SINGLE },
+			})
+		).toBe("Niederlassung NL01");
+
+		expect(
+			getScopeLabel({
+				routeBranch: "NL01",
+				urlState: { scope: SEARCH_SCOPE.ALL },
+			})
+		).toBe("Alle Niederlassungen");
+
+		expect(
+			getScopeLabel({
+				routeBranch: "NL01",
+				urlState: { scope: SEARCH_SCOPE.MULTI, branches: ["NL01", "NL02"] },
+			})
+		).toBe("2 Niederlassungen");
+	});
+
+	it("needsMultiBranchSelectionHint only when q exists and branches empty", () => {
+		expect(
+			needsMultiBranchSelectionHint({
+				isAdminDev: true,
+				urlState: { scope: SEARCH_SCOPE.MULTI, q: "x", branches: [] },
+			})
+		).toBe(true);
+
+		expect(
+			needsMultiBranchSelectionHint({
+				isAdminDev: true,
+				urlState: { scope: SEARCH_SCOPE.MULTI, q: null, branches: [] },
+			})
+		).toBe(false);
+	});
+
+	it("buildNextStateForScopeChange keeps branches only for MULTI", () => {
+		const base = { q: "x", branches: ["NL01"] };
+
+		expect(
+			buildNextStateForScopeChange({
+				urlState: base,
+				nextScope: SEARCH_SCOPE.ALL,
+			})
+		).toMatchObject({ scope: SEARCH_SCOPE.ALL, branches: [] });
+
+		expect(
+			buildNextStateForScopeChange({
+				urlState: base,
+				nextScope: SEARCH_SCOPE.MULTI,
+			})
+		).toMatchObject({ scope: SEARCH_SCOPE.MULTI, branches: ["NL01"] });
+	});
+
+	it("buildNextStateForToggleBranch toggles membership", () => {
+		const s1 = buildNextStateForToggleBranch({
+			urlState: { scope: SEARCH_SCOPE.MULTI, branches: ["NL01"] },
+			branchId: "NL02",
+		});
+		expect(s1.branches.sort()).toEqual(["NL01", "NL02"]);
+
+		const s2 = buildNextStateForToggleBranch({
+			urlState: { scope: SEARCH_SCOPE.MULTI, branches: ["NL01", "NL02"] },
+			branchId: "NL02",
+		});
+		expect(s2.branches).toEqual(["NL01"]);
+	});
+
+	it("buildNextStateForClearAllBranches clears branches", () => {
+		const s = buildNextStateForClearAllBranches({
+			urlState: { scope: SEARCH_SCOPE.MULTI, branches: ["NL01"] },
+		});
+		expect(s.branches).toEqual([]);
+		expect(s.scope).toBe(SEARCH_SCOPE.MULTI);
+	});
+
+	it("buildHrefForSingleBranchSwitch returns null for invalid branch", () => {
+		expect(
+			buildHrefForSingleBranchSwitch({
+				nextBranch: "bad",
+				urlState: { q: "x", scope: SEARCH_SCOPE.SINGLE, limit: 100 },
+			})
+		).toBe(null);
+	});
+});