RHL-037 feat(search): enhance search input handling and update tests for multi-branch UX

lib/frontend/search/searchApiInput.js

@@ -1,4 +1,3 @@
-import { ApiClientError } from "@/lib/frontend/apiClient";
 import { SEARCH_SCOPE } from "@/lib/frontend/search/urlState";
 
 function isNonEmptyString(value) {
@@ -14,9 +13,13 @@ function isNonEmptyString(value) {
  * - Role/scoping rules must be enforced consistently (branch users are always single-branch).
  *
  * Return shape:
- * - input: object for apiClient.search(...) or null (no search yet)
+ * - input: object for apiClient.search(...) or null (no search yet / not ready)
  * - error: ApiClientError or null (local validation / fast-fail)
  *
+ * UX policy for MULTI without branches:
+ * - Treat it as "not ready" (input=null, error=null) instead of an error.
+ *   The UI should show a friendly hint (select at least one branch).
+ *
  * @param {{
  *   urlState: {
  *     q: string|null,
@@ -31,7 +34,7 @@ function isNonEmptyString(value) {
  *   cursor?: string|null,
  *   limit?: number
  * }} args
- * @returns {{ input: any|null, error: ApiClientError|null }}
+ * @returns {{ input: any|null, error: any|null }}
  */
 export function buildSearchApiInput({
 	urlState,
@@ -73,15 +76,9 @@ export function buildSearchApiInput({
 				? urlState.branches
 				: [];
 
+			// UX: missing branches is not an error, it's "not ready".
 			if (branches.length === 0) {
-				return {
-					input: null,
-					error: new ApiClientError({
-						status: 400,
-						code: "VALIDATION_SEARCH_BRANCHES",
-						message: "Missing branches parameter for multi scope",
-					}),
-				};
+				return { input: null, error: null };
 			}
 
 			input.scope = "multi";

lib/frontend/search/searchApiInput.test.js

@@ -84,7 +84,7 @@ describe("lib/frontend/search/searchApiInput", () => {
 		});
 	});
 
-	it("admin/dev: MULTI without branches returns a validation ApiClientError", () => {
+	it("admin/dev: MULTI without branches is treated as not-ready (no error)", () => {
 		const { input, error } = buildSearchApiInput({
 			urlState: {
 				q: "x",
@@ -99,11 +99,7 @@ describe("lib/frontend/search/searchApiInput", () => {
 		});
 
 		expect(input).toBe(null);
-		expect(error).toMatchObject({
-			name: "ApiClientError",
-			code: "VALIDATION_SEARCH_BRANCHES",
-			status: 400,
-		});
+		expect(error).toBe(null);
 	});
 
 	it("admin/dev: SINGLE uses routeBranch as branch param", () => {

lib/frontend/search/urlState.js

@@ -43,19 +43,56 @@ function normalizeTrimmedOrNull(value) {
 	return s ? s : null;
 }
 
-function uniqueStable(items) {
-	const out = [];
-	const seen = new Set();
-
-	for (const it of items) {
-		const s = String(it);
-		if (!s) continue;
-		if (seen.has(s)) continue;
-		seen.add(s);
-		out.push(s);
+function normalizeBranchId(value) {
+	if (typeof value !== "string") return null;
+
+	const trimmed = value.trim();
+	if (!trimmed) return null;
+
+	// Keep it lenient (fail-open): uppercase for consistency.
+	// Validation is enforced later (backend + error mapping).
+	return trimmed.toUpperCase();
+}
+
+function toBranchNumber(branchId) {
+	const m = /^NL(\d+)$/i.exec(String(branchId || "").trim());
+	if (!m) return null;
+
+	const n = Number(m[1]);
+	return Number.isInteger(n) ? n : null;
+}
+
+function compareBranchIds(a, b) {
+	const aa = String(a || "");
+	const bb = String(b || "");
+
+	const na = toBranchNumber(aa);
+	const nb = toBranchNumber(bb);
+
+	// Prefer numeric ordering for NLxx patterns when possible.
+	if (na !== null && nb !== null) return na - nb;
+
+	// Stable fallback:
+	// - valid NL<num> come before unknown shapes
+	// - otherwise lexicographic
+	if (na !== null && nb === null) return -1;
+	if (na === null && nb !== null) return 1;
+
+	return aa.localeCompare(bb, "en");
+}
+
+function uniqueSorted(items) {
+	const cleaned = [];
+
+	for (const it of Array.isArray(items) ? items : []) {
+		const id = normalizeBranchId(String(it));
+		if (!id) continue;
+		cleaned.push(id);
 	}
 
-	return out;
+	const unique = Array.from(new Set(cleaned));
+	unique.sort(compareBranchIds);
+	return unique;
 }
 
 /**
@@ -68,12 +105,12 @@ export function parseBranchesCsv(raw) {
 	const s = typeof raw === "string" ? raw.trim() : "";
 	if (!s) return [];
 
-	const parts = s
-		.split(",")
-		.map((x) => String(x).trim())
-		.filter(Boolean);
-
-	return uniqueStable(parts);
+	return uniqueSorted(
+		s
+			.split(",")
+			.map((x) => String(x).trim())
+			.filter(Boolean)
+	);
 }
 
 /**
@@ -85,10 +122,7 @@ export function parseBranchesCsv(raw) {
 export function serializeBranchesCsv(branches) {
 	if (!Array.isArray(branches) || branches.length === 0) return null;
 
-	const cleaned = uniqueStable(
-		branches.map((b) => String(b).trim()).filter(Boolean)
-	);
-
+	const cleaned = uniqueSorted(branches);
 	return cleaned.length > 0 ? cleaned.join(",") : null;
 }
 
@@ -107,15 +141,10 @@ function normalizeLimit(raw) {
 /**
  * Parse search URL state from query params.
  *
- * Precedence (robust + shareable):
+ * Precedence:
  * 1) scope=all        -> ALL
  * 2) scope=multi OR branches=... -> MULTI
- * 3) branch=... or fallback routeBranch -> SINGLE
- *
- * Notes:
- * - For SINGLE we default to the route branch if branch param is missing.
- * - For MULTI/ALL we intentionally return branch=null (UI is route-context but API scope is not single).
- * - limit is restricted to SEARCH_LIMITS for predictable UX and to avoid backend 400s.
+ * 3) otherwise -> SINGLE (routeBranch is the source of truth)
  *
  * @param {URLSearchParams|Record<string, any>|any} searchParams
  * @param {{ routeBranch?: string|null }=} options
@@ -133,7 +162,7 @@ export function parseSearchUrlState(searchParams, { routeBranch = null } = {}) {
 	const q = normalizeTrimmedOrNull(readParam(searchParams, "q"));
 
 	const scopeRaw = normalizeTrimmedOrNull(readParam(searchParams, "scope"));
-	const branchRaw = normalizeTrimmedOrNull(readParam(searchParams, "branch"));
+	const branchRaw = normalizeBranchId(readParam(searchParams, "branch"));
 
 	const branches = parseBranchesCsv(readParam(searchParams, "branches"));
 
@@ -151,7 +180,7 @@ export function parseSearchUrlState(searchParams, { routeBranch = null } = {}) {
 	}
 
 	if (scope === SEARCH_SCOPE.SINGLE) {
-		const fallbackRouteBranch = normalizeTrimmedOrNull(routeBranch);
+		const fallbackRouteBranch = normalizeBranchId(routeBranch);
 		const branch = branchRaw || fallbackRouteBranch || null;
 
 		return {
@@ -195,12 +224,14 @@ export function parseSearchUrlState(searchParams, { routeBranch = null } = {}) {
  * Stable param ordering:
  * - q
  * - scope
- * - branch
  * - branches
  * - limit
  * - from
  * - to
  *
+ * SINGLE policy:
+ * - We do NOT emit `branch=` because the path segment `/:branch/search` is the source of truth.
+ *
  * @param {{
  *   q?: string|null,
  *   scope?: "single"|"multi"|"all"|string|null,
@@ -236,9 +267,7 @@ export function serializeSearchUrlState(state) {
 		const csv = serializeBranchesCsv(s.branches);
 		if (csv) params.set("branches", csv);
 	} else {
-		// SINGLE
-		const branch = normalizeTrimmedOrNull(s.branch);
-		if (branch) params.set("branch", branch);
+		// SINGLE: no `branch=` in URL (path is SoT)
 	}
 
 	// Only include non-default limit to keep URLs shorter.

lib/frontend/search/urlState.test.js

@@ -13,16 +13,20 @@ import {
 
 describe("lib/frontend/search/urlState", () => {
 	describe("parseBranchesCsv / serializeBranchesCsv", () => {
-		it("parses CSV into a unique, trimmed list", () => {
-			expect(parseBranchesCsv(" NL06, NL20 ,NL06,, ")).toEqual([
+		it("parses CSV into a unique, trimmed, sorted list", () => {
+			expect(parseBranchesCsv(" NL20, NL06 ,NL06,, ")).toEqual([
 				"NL06",
 				"NL20",
 			]);
 		});
 
-		it("serializes branches into CSV with stable order and dedupe", () => {
+		it("normalizes to uppercase (fail-open) and sorts deterministically", () => {
+			expect(parseBranchesCsv("nl20, NL06")).toEqual(["NL06", "NL20"]);
+		});
+
+		it("serializes branches into CSV (sorted + deduped)", () => {
 			expect(serializeBranchesCsv(["NL20", " NL06 ", "NL20"])).toBe(
-				"NL20,NL06"
+				"NL06,NL20"
 			);
 		});
 
@@ -48,8 +52,8 @@ describe("lib/frontend/search/urlState", () => {
 			});
 		});
 
-		it("parses SINGLE with explicit branch param", () => {
-			const sp = new URLSearchParams({ q: " test ", branch: "NL02" });
+		it("parses SINGLE with explicit branch param (legacy URL)", () => {
+			const sp = new URLSearchParams({ q: " test ", branch: "nl02" });
 			const state = parseSearchUrlState(sp, { routeBranch: "NL01" });
 
 			expect(state.scope).toBe(SEARCH_SCOPE.SINGLE);
@@ -101,7 +105,7 @@ describe("lib/frontend/search/urlState", () => {
 		it("parses MULTI when branches=... is present even without scope", () => {
 			const sp = new URLSearchParams({
 				q: "x",
-				branches: "NL06,NL20",
+				branches: "NL20,NL06",
 			});
 
 			const state = parseSearchUrlState(sp, { routeBranch: "NL01" });
@@ -135,7 +139,7 @@ describe("lib/frontend/search/urlState", () => {
 	});
 
 	describe("serializeSearchUrlState", () => {
-		it("serializes SINGLE as q + branch (no scope param)", () => {
+		it("serializes SINGLE as q only (path branch is SoT)", () => {
 			const qs = serializeSearchUrlState({
 				q: "bridgestone",
 				scope: SEARCH_SCOPE.SINGLE,
@@ -143,14 +147,14 @@ describe("lib/frontend/search/urlState", () => {
 				limit: DEFAULT_SEARCH_LIMIT,
 			});
 
-			expect(qs).toBe("q=bridgestone&branch=NL01");
+			expect(qs).toBe("q=bridgestone");
 		});
 
 		it("serializes MULTI as q + scope=multi + branches", () => {
 			const qs = serializeSearchUrlState({
 				q: "reifen",
 				scope: SEARCH_SCOPE.MULTI,
-				branches: ["NL06", "NL20"],
+				branches: ["NL20", "NL06"],
 				limit: DEFAULT_SEARCH_LIMIT,
 			});
 
@@ -175,7 +179,7 @@ describe("lib/frontend/search/urlState", () => {
 				limit: 200,
 			});
 
-			expect(qs).toBe("q=x&branch=NL01&limit=200");
+			expect(qs).toBe("q=x&limit=200");
 		});
 
 		it("includes from/to when present (future-proof for RHL-025)", () => {
@@ -188,9 +192,7 @@ describe("lib/frontend/search/urlState", () => {
 				to: "2025-12-31",
 			});
 
-			expect(qs).toBe(
-				"q=x&branch=NL01&limit=200&from=2025-12-01&to=2025-12-31"
-			);
+			expect(qs).toBe("q=x&limit=200&from=2025-12-01&to=2025-12-31");
 		});
 	});
 });