RHL-012 feat(api): implement admin user management endpoints for listing, creating, updating, and deleting users

lib/frontend/apiClient.js

@@ -16,13 +16,6 @@
  * @property {{ message: string, code: string, details?: any }} error
  */
 
-/**
- * A standardized client-side error type for API failures.
- * - status: HTTP status code (e.g. 401, 403, 404, 500)
- * - code: machine-readable error code (e.g. AUTH_UNAUTHENTICATED)
- * - message: human-readable message (safe to show in UI)
- * - details: optional structured payload (validation params etc.)
- */
 export class ApiClientError extends Error {
 	/**
 	 * @param {{
@@ -36,88 +29,34 @@ export class ApiClientError extends Error {
 	 * }} input
 	 */
 	constructor({ status, code, message, details, url, method, cause }) {
-		// We attach `cause` to preserve error chains (supported in modern Node).
 		super(message, cause ? { cause } : undefined);
 
 		this.name = "ApiClientError";
 		this.status = status;
 		this.code = code;
 
-		// Only attach optional properties when provided (keeps error objects clean).
 		if (details !== undefined) this.details = details;
 		if (url) this.url = url;
 		if (method) this.method = method;
 	}
 }
 
-const DEFAULT_HEADERS = {
-	Accept: "application/json",
-};
+const DEFAULT_HEADERS = { Accept: "application/json" };
 
-/**
- * Resolve a request URL.
- * - If `path` is absolute (http/https), return as-is.
- * - If `baseUrl` is provided, resolve relative to it.
- * - Otherwise return the relative path (browser-friendly: "/api/...").
- *
- * Why this exists:
- * - The same client can be used:
- *   - in the browser (relative "/api/..." calls)
- *   - in Node scripts (absolute baseUrl like "http://127.0.0.1:3000")
- *
- * @param {string} path
- * @param {string=} baseUrl
- * @returns {string}
- */
 function resolveUrl(path, baseUrl) {
-	// If someone passes a full URL, keep it unchanged.
 	if (/^https?:\/\//i.test(path)) return path;
 
 	const base = (baseUrl || "").trim();
-
-	// Browser usage: baseUrl omitted -> use relative path.
 	if (!base) return path;
 
-	// Ensure baseUrl ends with a slash so URL() resolves correctly.
 	return new URL(path, base.endsWith("/") ? base : `${base}/`).toString();
 }
 
-/**
- * Best-effort detection if response is JSON.
- *
- * Why we need this:
- * - Our API is intended to always respond with JSON.
- * - But in error cases (misconfig, reverse proxy, 404 HTML), we might receive non-JSON.
- * - We want robust parsing behavior and a predictable client-side error.
- *
- * @param {Response} response
- * @returns {boolean}
- */
 function isJsonResponse(response) {
 	const ct = response.headers.get("content-type") || "";
 	return ct.toLowerCase().includes("application/json");
 }
 
-/**
- * Core fetch helper with:
- * - credentials: "include" (cookie-based session)
- * - cache: "no-store" (match backend freshness strategy)
- * - standardized error mapping into ApiClientError
- *
- * Clean code rule:
- * - UI code should NOT call fetch directly.
- * - Instead, it should call domain helpers that route through apiFetch().
- *
- * @param {string} path
- * @param {{
- *   method?: string,
- *   headers?: Record<string, string>,
- *   body?: any,
- *   baseUrl?: string,
- *   fetchImpl?: typeof fetch
- * }=} options
- * @returns {Promise<any>} parsed JSON payload (or null for empty responses)
- */
 export async function apiFetch(path, options = {}) {
 	const {
 		method = "GET",
@@ -129,8 +68,6 @@ export async function apiFetch(path, options = {}) {
 
 	const url = resolveUrl(path, baseUrl);
 
-	// Build request init. We always set credentials + no-store.
-	// For JSON bodies, we serialize and set Content-Type.
 	const init = {
 		method,
 		credentials: "include",
@@ -139,15 +76,10 @@ export async function apiFetch(path, options = {}) {
 	};
 
 	if (body !== undefined) {
-		// If the caller passes a string, we assume it is already serialized.
-		// Otherwise we serialize as JSON.
 		if (typeof body === "string") {
 			init.body = body;
 		} else {
 			init.body = JSON.stringify(body);
-
-			// Only set Content-Type if caller didn't provide it.
-			// This allows callers to send non-JSON payloads later if needed.
 			if (!init.headers["Content-Type"]) {
 				init.headers["Content-Type"] = "application/json";
 			}
@@ -158,8 +90,6 @@ export async function apiFetch(path, options = {}) {
 	try {
 		response = await fetchImpl(url, init);
 	} catch (err) {
-		// Network errors, DNS errors, connection refused, etc.
-		// We use status=0 to indicate "no HTTP response".
 		throw new ApiClientError({
 			status: 0,
 			code: "CLIENT_NETWORK_ERROR",
@@ -170,17 +100,14 @@ export async function apiFetch(path, options = {}) {
 		});
 	}
 
-	// Handle empty responses explicitly (e.g. some endpoints might return 204 later).
 	if (response.status === 204) return null;
 
-	// Prefer JSON when the server says it's JSON.
 	if (isJsonResponse(response)) {
 		let payload;
 
 		try {
 			payload = await response.json();
 		} catch (err) {
-			// Server said JSON but response body was not parseable JSON.
 			throw new ApiClientError({
 				status: response.status,
 				code: "CLIENT_INVALID_JSON",
@@ -191,13 +118,10 @@ export async function apiFetch(path, options = {}) {
 			});
 		}
 
-		// Happy path: return parsed JSON.
 		if (response.ok) return payload;
 
-		/** @type {ApiErrorPayload|any} */
 		const maybeError = payload;
 
-		// Map standardized backend errors into ApiClientError
 		if (maybeError?.error?.code && maybeError?.error?.message) {
 			throw new ApiClientError({
 				status: response.status,
@@ -209,7 +133,6 @@ export async function apiFetch(path, options = {}) {
 			});
 		}
 
-		// Fallback: error is JSON but not in our standardized shape.
 		throw new ApiClientError({
 			status: response.status,
 			code: "CLIENT_HTTP_ERROR",
@@ -220,9 +143,7 @@ export async function apiFetch(path, options = {}) {
 		});
 	}
 
-	// Non-JSON response fallback (should be rare for current endpoints)
 	const text = await response.text().catch(() => "");
-
 	if (response.ok) return text || null;
 
 	throw new ApiClientError({
@@ -235,17 +156,9 @@ export async function apiFetch(path, options = {}) {
 }
 
 /* -------------------------------------------------------------------------- */
-/* Domain helpers (thin wrappers)                                              */
+/* Domain helpers                                                              */
 /* -------------------------------------------------------------------------- */
 
-/**
- * Login:
- * - Sends credentials to the backend.
- * - Backend sets an HTTP-only cookie when successful.
- *
- * @param {{ username: string, password: string }} input
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function login(input, options) {
 	return apiFetch("/api/auth/login", {
 		method: "POST",
@@ -254,40 +167,14 @@ export function login(input, options) {
 	});
 }
 
-/**
- * Logout:
- * - Clears the session cookie (idempotent).
- *
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function logout(options) {
 	return apiFetch("/api/auth/logout", { method: "GET", ...options });
 }
 
-/**
- * Get current session identity (frontend-friendly):
- * - Always returns HTTP 200 with:
- *   - { user: null } when unauthenticated
- *   - { user: { userId, role, branchId } } when authenticated
- *
- * Why we want this:
- * - The UI should not use 401 as basic control-flow to determine "am I logged in?"
- * - This endpoint enables a clean "session check" UX (RHL-020 AuthGate).
- *
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function getMe(options) {
 	return apiFetch("/api/auth/me", { method: "GET", ...options });
 }
 
-/**
- * Change password (RHL-009):
- * - Requires an active session cookie.
- * - Body: { currentPassword, newPassword }
- *
- * @param {{ currentPassword: string, newPassword: string }} input
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function changePassword(input, options) {
 	return apiFetch("/api/auth/change-password", {
 		method: "POST",
@@ -296,19 +183,10 @@ export function changePassword(input, options) {
 	});
 }
 
-/**
- * List branches visible to the current session (RBAC is enforced server-side).
- *
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function getBranches(options) {
 	return apiFetch("/api/branches", { method: "GET", ...options });
 }
 
-/**
- * @param {string} branch
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function getYears(branch, options) {
 	return apiFetch(`/api/branches/${encodeURIComponent(branch)}/years`, {
 		method: "GET",
@@ -316,26 +194,13 @@ export function getYears(branch, options) {
 	});
 }
 
-/**
- * @param {string} branch
- * @param {string} year
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function getMonths(branch, year, options) {
 	return apiFetch(
-		`/api/branches/${encodeURIComponent(branch)}/${encodeURIComponent(
-			year,
-		)}/months`,
+		`/api/branches/${encodeURIComponent(branch)}/${encodeURIComponent(year)}/months`,
 		{ method: "GET", ...options },
 	);
 }
 
-/**
- * @param {string} branch
- * @param {string} year
- * @param {string} month
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function getDays(branch, year, month, options) {
 	return apiFetch(
 		`/api/branches/${encodeURIComponent(branch)}/${encodeURIComponent(
@@ -345,13 +210,6 @@ export function getDays(branch, year, month, options) {
 	);
 }
 
-/**
- * @param {string} branch
- * @param {string} year
- * @param {string} month
- * @param {string} day
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function getFiles(branch, year, month, day, options) {
 	const qs = new URLSearchParams({
 		branch: String(branch),
@@ -363,31 +221,10 @@ export function getFiles(branch, year, month, day, options) {
 	return apiFetch(`/api/files?${qs.toString()}`, { method: "GET", ...options });
 }
 
-/**
- * Search delivery notes (RHL-024).
- *
- * Notes:
- * - This endpoint is JSON and can be called via apiFetch.
- * - Cursor is intentionally not stored in shareable URLs by default; the UI can keep it in state.
- *
- * @param {{
- *   q?: string|null,
- *   branch?: string|null,
- *   scope?: "branch"|"multi"|"all"|string|null,
- *   branches?: string[]|null,
- *   from?: string|null,
- *   to?: string|null,
- *   limit?: number|null,
- *   cursor?: string|null
- * }} input
- * @param {{ baseUrl?: string, fetchImpl?: typeof fetch }=} options
- */
 export function search(input, options) {
 	const { q, branch, scope, branches, from, to, limit, cursor } = input || {};
-
 	const params = new URLSearchParams();
 
-	// Stable insertion order (helps debugging and makes URLs readable).
 	if (typeof q === "string" && q.trim()) params.set("q", q.trim());
 	if (typeof scope === "string" && scope.trim())
 		params.set("scope", scope.trim());
@@ -407,12 +244,67 @@ export function search(input, options) {
 		if (raw) params.set("limit", raw);
 	}
 
-	if (typeof cursor === "string" && cursor.trim()) {
+	if (typeof cursor === "string" && cursor.trim())
 		params.set("cursor", cursor.trim());
+
+	const qs = params.toString();
+	return apiFetch(qs ? `/api/search?${qs}` : "/api/search", {
+		method: "GET",
+		...options,
+	});
+}
+
+export function adminListUsers(input, options) {
+	const { q, role, branchId, limit, cursor } = input || {};
+	const params = new URLSearchParams();
+
+	if (typeof q === "string" && q.trim()) params.set("q", q.trim());
+	if (typeof role === "string" && role.trim()) params.set("role", role.trim());
+	if (typeof branchId === "string" && branchId.trim())
+		params.set("branchId", branchId.trim());
+
+	if (limit !== undefined && limit !== null) {
+		const raw = String(limit).trim();
+		if (raw) params.set("limit", raw);
 	}
 
+	if (typeof cursor === "string" && cursor.trim())
+		params.set("cursor", cursor.trim());
+
 	const qs = params.toString();
-	const path = qs ? `/api/search?${qs}` : "/api/search";
+	return apiFetch(qs ? `/api/admin/users?${qs}` : "/api/admin/users", {
+		method: "GET",
+		...options,
+	});
+}
+
+export function adminCreateUser(input, options) {
+	return apiFetch("/api/admin/users", {
+		method: "POST",
+		body: input,
+		...options,
+	});
+}
 
-	return apiFetch(path, { method: "GET", ...options });
+export function adminUpdateUser(userId, patch, options) {
+	if (typeof userId !== "string" || !userId.trim()) {
+		throw new Error("adminUpdateUser requires a userId string");
+	}
+
+	return apiFetch(`/api/admin/users/${encodeURIComponent(userId.trim())}`, {
+		method: "PATCH",
+		body: patch || {},
+		...options,
+	});
+}
+
+export function adminDeleteUser(userId, options) {
+	if (typeof userId !== "string" || !userId.trim()) {
+		throw new Error("adminDeleteUser requires a userId string");
+	}
+
+	return apiFetch(`/api/admin/users/${encodeURIComponent(userId.trim())}`, {
+		method: "DELETE",
+		...options,
+	});
 }

lib/frontend/apiClient.test.js

@@ -9,17 +9,14 @@ import {
 	getMe,
 	search,
 	changePassword,
+	adminListUsers,
+	adminCreateUser,
+	adminUpdateUser,
+	adminDeleteUser,
 } from "./apiClient.js";
 
 beforeEach(() => {
-	// Restore mocks between tests to avoid cross-test pollution.
 	vi.restoreAllMocks();
-
-	// In these unit tests we stub the global fetch implementation.
-	// This allows us to validate:
-	// - request defaults (credentials/cache)
-	// - URL building
-	// - error mapping
 	global.fetch = vi.fn();
 });
 
@@ -34,32 +31,13 @@ describe("lib/frontend/apiClient", () => {
 
 		await apiFetch("/api/health");
 
-		expect(fetch).toHaveBeenCalledTimes(1);
 		const [url, init] = fetch.mock.calls[0];
-
 		expect(url).toBe("/api/health");
 		expect(init.credentials).toBe("include");
 		expect(init.cache).toBe("no-store");
 	});
 
-	it("apiFetch serializes JSON bodies and sets Content-Type", async () => {
-		fetch.mockResolvedValue(
-			new Response(JSON.stringify({ ok: true }), {
-				status: 200,
-				headers: { "Content-Type": "application/json" },
-			}),
-		);
-
-		await login({ username: "u", password: "p" });
-
-		const [, init] = fetch.mock.calls[0];
-		expect(init.method).toBe("POST");
-		expect(init.headers.Accept).toBe("application/json");
-		expect(init.headers["Content-Type"]).toBe("application/json");
-		expect(init.body).toBe(JSON.stringify({ username: "u", password: "p" }));
-	});
-
-	it("apiFetch throws ApiClientError for standardized backend error payloads", async () => {
+	it("apiFetch maps standardized backend errors into ApiClientError", async () => {
 		fetch.mockResolvedValue(
 			new Response(
 				JSON.stringify({
@@ -73,7 +51,6 @@ describe("lib/frontend/apiClient", () => {
 			name: "ApiClientError",
 			status: 401,
 			code: "AUTH_UNAUTHENTICATED",
-			message: "Unauthorized",
 		});
 	});
 
@@ -92,22 +69,15 @@ describe("lib/frontend/apiClient", () => {
 
 	it("getFiles builds the expected query string", async () => {
 		fetch.mockResolvedValue(
-			new Response(
-				JSON.stringify({
-					branch: "NL01",
-					year: "2024",
-					month: "10",
-					day: "23",
-					files: [],
-				}),
-				{ status: 200, headers: { "Content-Type": "application/json" } },
-			),
+			new Response(JSON.stringify({ files: [] }), {
+				status: 200,
+				headers: { "Content-Type": "application/json" },
+			}),
 		);
 
 		await getFiles("NL01", "2024", "10", "23");
 
 		const [url] = fetch.mock.calls[0];
-		// We do not rely on param ordering beyond URLSearchParams defaults.
 		expect(url).toContain("/api/files?");
 		expect(url).toContain("branch=NL01");
 		expect(url).toContain("year=2024");
@@ -115,12 +85,7 @@ describe("lib/frontend/apiClient", () => {
 		expect(url).toContain("day=23");
 	});
 
-	it("getMe calls /api/auth/me and returns the parsed payload", async () => {
-		// /api/auth/me returns 200 with { user: null } or { user: {...} }.
-		// For this unit test we only need to ensure:
-		// - correct endpoint
-		// - correct method
-		// - response is returned as parsed JSON
+	it("getMe calls /api/auth/me", async () => {
 		fetch.mockResolvedValue(
 			new Response(JSON.stringify({ user: null }), {
 				status: 200,
@@ -129,20 +94,32 @@ describe("lib/frontend/apiClient", () => {
 		);
 
 		const res = await getMe();
-
 		expect(res).toEqual({ user: null });
 
-		expect(fetch).toHaveBeenCalledTimes(1);
 		const [url, init] = fetch.mock.calls[0];
 		expect(url).toBe("/api/auth/me");
 		expect(init.method).toBe("GET");
+	});
 
-		// Ensure our global defaults are still enforced for session-based calls.
-		expect(init.credentials).toBe("include");
-		expect(init.cache).toBe("no-store");
+	it("search builds expected query string", async () => {
+		fetch.mockResolvedValue(
+			new Response(JSON.stringify({ items: [], nextCursor: null }), {
+				status: 200,
+				headers: { "Content-Type": "application/json" },
+			}),
+		);
+
+		await search({ q: "x", branch: "NL01", limit: 100 });
+
+		const [url] = fetch.mock.calls[0];
+		const u = new URL(url, "http://localhost");
+		expect(u.pathname).toBe("/api/search");
+		expect(u.searchParams.get("q")).toBe("x");
+		expect(u.searchParams.get("branch")).toBe("NL01");
+		expect(u.searchParams.get("limit")).toBe("100");
 	});
 
-	it("changePassword calls /api/auth/change-password with POST and JSON body", async () => {
+	it("changePassword calls POST /api/auth/change-password", async () => {
 		fetch.mockResolvedValue(
 			new Response(JSON.stringify({ ok: true }), {
 				status: 200,
@@ -150,75 +127,40 @@ describe("lib/frontend/apiClient", () => {
 			}),
 		);
 
-		await changePassword({
-			currentPassword: "OldPassword123",
-			newPassword: "StrongPassword123",
-		});
+		await changePassword({ currentPassword: "a", newPassword: "b" });
 
-		expect(fetch).toHaveBeenCalledTimes(1);
 		const [url, init] = fetch.mock.calls[0];
-
 		expect(url).toBe("/api/auth/change-password");
 		expect(init.method).toBe("POST");
-		expect(init.headers.Accept).toBe("application/json");
-		expect(init.headers["Content-Type"]).toBe("application/json");
-		expect(init.body).toBe(
-			JSON.stringify({
-				currentPassword: "OldPassword123",
-				newPassword: "StrongPassword123",
-			}),
-		);
-
-		expect(init.credentials).toBe("include");
-		expect(init.cache).toBe("no-store");
 	});
 
-	it("search builds the expected query string for branch scope", async () => {
+	it("adminUpdateUser calls PATCH /api/admin/users/:id", async () => {
 		fetch.mockResolvedValue(
-			new Response(JSON.stringify({ items: [], nextCursor: null }), {
+			new Response(JSON.stringify({ ok: true }), {
 				status: 200,
 				headers: { "Content-Type": "application/json" },
 			}),
 		);
 
-		await search({ q: "bridgestone", branch: "NL01", limit: 100 });
+		await adminUpdateUser("507f1f77bcf86cd799439011", { role: "admin" });
 
-		expect(fetch).toHaveBeenCalledTimes(1);
 		const [url, init] = fetch.mock.calls[0];
-
-		const u = new URL(url, "http://localhost");
-		expect(u.pathname).toBe("/api/search");
-		expect(u.searchParams.get("q")).toBe("bridgestone");
-		expect(u.searchParams.get("branch")).toBe("NL01");
-		expect(u.searchParams.get("limit")).toBe("100");
-
-		expect(init.method).toBe("GET");
-		expect(init.credentials).toBe("include");
-		expect(init.cache).toBe("no-store");
+		expect(url).toBe("/api/admin/users/507f1f77bcf86cd799439011");
+		expect(init.method).toBe("PATCH");
 	});
 
-	it("search supports multi scope + branches + cursor", async () => {
+	it("adminDeleteUser calls DELETE /api/admin/users/:id", async () => {
 		fetch.mockResolvedValue(
-			new Response(JSON.stringify({ items: [], nextCursor: null }), {
+			new Response(JSON.stringify({ ok: true }), {
 				status: 200,
 				headers: { "Content-Type": "application/json" },
 			}),
 		);
 
-		await search({
-			q: "  reifen  ",
-			scope: "multi",
-			branches: ["NL06", "NL20"],
-			cursor: "abc",
-		});
+		await adminDeleteUser("507f1f77bcf86cd799439099");
 
-		const [url] = fetch.mock.calls[0];
-		const u = new URL(url, "http://localhost");
-
-		expect(u.pathname).toBe("/api/search");
-		expect(u.searchParams.get("q")).toBe("reifen");
-		expect(u.searchParams.get("scope")).toBe("multi");
-		expect(u.searchParams.get("branches")).toBe("NL06,NL20");
-		expect(u.searchParams.get("cursor")).toBe("abc");
+		const [url, init] = fetch.mock.calls[0];
+		expect(url).toBe("/api/admin/users/507f1f77bcf86cd799439099");
+		expect(init.method).toBe("DELETE");
 	});
 });