fix(housekeeping): preserve form state and accurate access details

This commit is contained in:
Simo committed 2026-09-03 20:39:10 +02:00
1 parent 93367a239f
commit 11ddf6cf0b
8 files changed
+293 -68

No files matched your search

@@ -1,5 +1,6 @@
"use client"; "use client";
import { useRouter } from "next/navigation";
import { useActionState, useId } from "react"; import { useActionState, useId } from "react";
import { requiresHousekeepingReason } from "../../../foundation/commands/reason-policy"; import { requiresHousekeepingReason } from "../../../foundation/commands/reason-policy";
import type { HousekeepingResult } from "../../../foundation/contracts"; import type { HousekeepingResult } from "../../../foundation/contracts";
@@ -161,7 +162,22 @@ export async function submitContentCommandForm(
input, input,
...(reasonRequired ? { reason } : {}), ...(reasonRequired ? { reason } : {}),
}); });
return { result, values: result.ok ? {} : submittedValues }; return { result, values: submittedValues };
}
export async function submitContentCommandFormAndRefresh(
configuration: ContentCommandSubmission,
refresh: () => void,
previous: ContentCommandFormState | null,
formData: FormData,
): Promise<ContentCommandFormState> {
const state = await submitContentCommandForm(
configuration,
previous,
formData,
);
if (state.result?.ok) refresh();
return state;
} }
export function ContentCommandFieldError({ export function ContentCommandFieldError({
@@ -191,18 +207,23 @@ export function ContentCommandForm({
requiresReason = false, requiresReason = false,
}: ContentCommandFormProps) { }: ContentCommandFormProps) {
const formId = useId(); const formId = useId();
const router = useRouter();
const reasonRequired = requiresHousekeepingReason(requiresReason, input); const reasonRequired = requiresHousekeepingReason(requiresReason, input);
const [state, submit, pending] = useActionState( const [state, submit, pending] = useActionState(
submitContentCommandForm.bind(null, { submitContentCommandFormAndRefresh.bind(
commandId, null,
input, {
fields, commandId,
requiresReason: reasonRequired, input,
}), fields,
requiresReason: reasonRequired,
},
() => router.refresh(),
),
initialState, initialState,
); );
const result = state.result; const result = state.result;
const retainedValues = result && !result.ok ? state.values : {}; const retainedValues = result ? state.values : {};
const remountKey = `${formId}-${result?.correlationId ?? "initial"}`; const remountKey = `${formId}-${result?.correlationId ?? "initial"}`;
const reasonErrorId = `${formId}-reason-error`; const reasonErrorId = `${formId}-reason-error`;
const hasReasonError = const hasReasonError =
@@ -23,8 +23,9 @@ import { ContentHelpPage } from "./help";
import { ContentLocalizationPage } from "./localization"; import { ContentLocalizationPage } from "./localization";
import { ContentMediaPage } from "./media"; import { ContentMediaPage } from "./media";
const { actionState } = vi.hoisted(() => ({ const { actionState, routerRefresh } = vi.hoisted(() => ({
actionState: { current: null as unknown }, actionState: { current: null as unknown },
routerRefresh: vi.fn(),
})); }));
vi.mock("react", async (importOriginal) => { vi.mock("react", async (importOriginal) => {
@@ -43,6 +44,10 @@ vi.mock("@/actions/housekeeping-command", () => ({
executeHousekeepingCommand: vi.fn(), executeHousekeepingCommand: vi.fn(),
})); }));
vi.mock("next/navigation", () => ({
useRouter: () => ({ refresh: routerRefresh }),
}));
function context(granted: readonly string[]): HousekeepingCapabilityContext { function context(granted: readonly string[]): HousekeepingCapabilityContext {
const permissions = new Set(granted); const permissions = new Set(granted);
return { return {
@@ -313,7 +318,7 @@ describe("Content actionable form wiring", () => {
}); });
}); });
it("clears retained values after a successful command", async () => { it("retains submitted values after a successful command", async () => {
vi.mocked(executeHousekeepingCommand).mockResolvedValue( vi.mocked(executeHousekeepingCommand).mockResolvedValue(
ok({ before: null, after: { id: "41" } }, "poll-success"), ok({ before: null, after: { id: "41" } }, "poll-success"),
); );
@@ -330,7 +335,96 @@ describe("Content actionable form wiring", () => {
formData, formData,
); );
expect(state).toMatchObject({ result: { ok: true }, values: {} }); expect(state).toMatchObject({
result: { ok: true },
values: { title: "Published poll" },
});
});
it("keeps the first successful edit across stale SSR defaults and refreshes each success once", async () => {
const module = (await import("./content-command-form")) as Record<
string,
unknown
>;
const submitAndRefresh = module.submitContentCommandFormAndRefresh;
expect(submitAndRefresh).toBeTypeOf("function");
if (typeof submitAndRefresh !== "function") return;
vi.mocked(executeHousekeepingCommand)
.mockResolvedValueOnce(
ok(
{ before: { title: "Title A" }, after: { title: "Title B" } },
"first-success",
),
)
.mockResolvedValueOnce(
ok(
{
before: { title: "Title B", description: "Old summary" },
after: { title: "Title B", description: "New summary" },
},
"second-success",
),
);
const configuration = {
commandId: "content.editorial.article.change",
input: { action: "update", id: "41" },
fields: [
{ name: "title", label: "Title", type: "text" as const },
{
name: "description",
label: "Summary",
type: "text" as const,
},
],
};
const firstData = new FormData();
firstData.set("title", "Title B");
firstData.set("description", "Old summary");
const firstState = await Reflect.apply(submitAndRefresh, undefined, [
configuration,
routerRefresh,
null,
firstData,
]);
expect(firstState).toMatchObject({
result: { ok: true },
values: { title: "Title B", description: "Old summary" },
});
actionState.current = firstState;
const remounted = renderToStaticMarkup(
<ContentCommandForm
{...configuration}
buttonLabel="Save article"
fields={[
{ ...configuration.fields[0], defaultValue: "Title A" },
{ ...configuration.fields[1], defaultValue: "Old summary" },
]}
/>,
);
expect(remounted).toMatch(/name="title"[^>]*value="Title B"/u);
const secondData = new FormData();
secondData.set("title", "Title B");
secondData.set("description", "New summary");
await Reflect.apply(submitAndRefresh, undefined, [
configuration,
routerRefresh,
firstState,
secondData,
]);
expect(executeHousekeepingCommand).toHaveBeenNthCalledWith(2, {
commandId: "content.editorial.article.change",
input: {
action: "update",
id: "41",
title: "Title B",
description: "New summary",
},
});
expect(routerRefresh).toHaveBeenCalledTimes(2);
}); });
it("renders an accessible stable field error", () => { it("renders an accessible stable field error", () => {
@@ -1,6 +1,6 @@
"use client"; "use client";
import { useActionState } from "react"; import { useActionState, useId } from "react";
import type { HousekeepingResult } from "../../../foundation/contracts"; import type { HousekeepingResult } from "../../../foundation/contracts";
export interface PeopleCommandField { export interface PeopleCommandField {
@@ -204,6 +204,7 @@ export function PeopleCommandForm({
includeReasonInInput = false, includeReasonInInput = false,
nestFields = false, nestFields = false,
}: PeopleCommandFormProps) { }: PeopleCommandFormProps) {
const formId = useId();
const [result, submit, pending] = useActionState( const [result, submit, pending] = useActionState(
submitPeopleCommandForm.bind(null, { submitPeopleCommandForm.bind(null, {
commandId, commandId,
@@ -222,61 +223,56 @@ export function PeopleCommandForm({
data-housekeeping-command={commandId} data-housekeeping-command={commandId}
className="space-y-3 rounded border border-[var(--admin-border)] p-3" className="space-y-3 rounded border border-[var(--admin-border)] p-3"
> >
{fields.map((field) => ( {fields.map((field) => {
<label const fieldId = `${formId}-${field.name}`;
key={field.name} return (
htmlFor={`${commandId}-${field.name}`} <label key={field.name} htmlFor={fieldId} className="block text-sm">
className="block text-sm" {field.type === "checkbox" ? (
> <>
{field.type === "checkbox" ? ( <input id={fieldId} name={field.name} type="checkbox" />{" "}
<> {field.label}
<input </>
id={`${commandId}-${field.name}`} ) : field.type === "select" ? (
name={field.name} <>
type="checkbox" {field.label}
/>{" "} <select
{field.label} id={fieldId}
</> name={field.name}
) : field.type === "select" ? ( defaultValue={field.defaultValue}
<> required={field.required}
{field.label} className="mt-1 block w-full"
<select >
id={`${commandId}-${field.name}`} {field.options?.map((option) => (
name={field.name} <option key={option.value} value={option.value}>
defaultValue={field.defaultValue} {option.label}
required={field.required} </option>
className="mt-1 block w-full" ))}
> </select>
{field.options?.map((option) => ( </>
<option key={option.value} value={option.value}> ) : (
{option.label} <>
</option> {field.label}
))} <input
</select> id={fieldId}
</> name={field.name}
) : ( type={field.type === "number" ? "number" : "text"}
<> defaultValue={field.defaultValue}
{field.label} required={field.required}
<input min={field.min}
id={`${commandId}-${field.name}`} max={field.max}
name={field.name} maxLength={field.maxLength}
type={field.type === "number" ? "number" : "text"} className="mt-1 block w-full"
defaultValue={field.defaultValue} />
required={field.required} </>
min={field.min} )}
max={field.max} </label>
maxLength={field.maxLength} );
className="mt-1 block w-full" })}
/>
</>
)}
</label>
))}
{requiresReason ? ( {requiresReason ? (
<label htmlFor={`${commandId}-reason`} className="block text-sm"> <label htmlFor={`${formId}-reason`} className="block text-sm">
Reason Reason
<textarea <textarea
id={`${commandId}-reason`} id={`${formId}-reason`}
name="reason" name="reason"
required required
maxLength={1000} maxLength={1000}
@@ -244,6 +244,14 @@ describe("People support/moderation pages", () => {
expect(html).toContain('value="Hello"'); expect(html).toContain('value="Hello"');
expect(html).toContain('value="general"'); expect(html).toContain('value="general"');
expect(html).toContain('value="4"'); expect(html).toContain('value="4"');
const titleIds = [
...html.matchAll(
/<input(?=[^>]*\bname="title")(?=[^>]*\bid="([^"]+)")[^>]*>/gu,
),
].map((match) => match[1]);
expect(titleIds).toHaveLength(2);
expect(new Set(titleIds).size).toBe(titleIds.length);
for (const id of titleIds) expect(html).toContain(`for="${id}"`);
executeHousekeepingCommand.mockResolvedValue({ executeHousekeepingCommand.mockResolvedValue({
ok: true, ok: true,
@@ -8,6 +8,7 @@ import { HousekeepingPageShell } from "../../../foundation/page/housekeeping-pag
import { HousekeepingPageState } from "../../../foundation/page/housekeeping-page-state"; import { HousekeepingPageState } from "../../../foundation/page/housekeeping-page-state";
import type { HousekeepingPageInput } from "../../../route-handlers"; import type { HousekeepingPageInput } from "../../../route-handlers";
import { import {
SYSTEM_ACCESS_USER_SAMPLE_LIMIT,
type SystemAccessQueryData, type SystemAccessQueryData,
type SystemAccessQueryInput, type SystemAccessQueryInput,
systemAccessQuery, systemAccessQuery,
@@ -61,8 +62,13 @@ function accessContent(data: SystemAccessQueryData) {
<dd>{data.rank.id}</dd> <dd>{data.rank.id}</dd>
<dt>Level</dt> <dt>Level</dt>
<dd>{data.rank.level}</dd> <dd>{data.rank.level}</dd>
<dt>Users</dt> <dt>Total users</dt>
<dd>{data.rank.userCount}</dd> <dd>{data.rank.userCount}</dd>
<dt>User sample</dt>
<dd>
Showing {data.rank.users.length} of {data.rank.userCount} (maximum{" "}
{SYSTEM_ACCESS_USER_SAMPLE_LIMIT})
</dd>
</dl> </dl>
</section> </section>
<section className="rounded-lg border border-[var(--admin-border)] bg-[var(--admin-surface)] p-4"> <section className="rounded-lg border border-[var(--admin-border)] bg-[var(--admin-surface)] p-4">
@@ -204,3 +204,34 @@ it.each([
expect(html).not.toContain('data-housekeeping-state="empty"'); expect(html).not.toContain('data-housekeeping-state="empty"');
}, },
); );
it("labels the rank total separately from its bounded user sample", () => {
const html = renderToStaticMarkup(
<SystemAccessPage
result={ok(
{
kind: "permission-detail",
rank: {
id: 7,
name: "Administrator",
level: 7,
userCount: 237,
badge: "ADM",
permissions: {},
cmsRole: null,
users: [
{ id: 1, username: "Ada" },
{ id: 2, username: "Linus" },
],
},
partialDependencies: [],
},
correlationId,
)}
/>,
);
expect(html).toContain("Total users");
expect(html).toContain("User sample");
expect(html).toContain("Showing 2 of 237 (maximum 200)");
});
@@ -0,0 +1,59 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
const { execute, fetchEmulatorRankForEdit } = vi.hoisted(() => ({
execute: vi.fn(),
fetchEmulatorRankForEdit: vi.fn(),
}));
vi.mock("@/lib/db", () => ({ db: { execute } }));
vi.mock("@/lib/services/permission-ranks", () => ({
fetchEmulatorRankForEdit,
}));
import { systemAccessAdapters } from "./access";
describe("System rank-detail production adapter", () => {
beforeEach(() => {
execute.mockReset();
fetchEmulatorRankForEdit.mockReset();
});
it("separates the exact user total from the bounded 200-row sample", async () => {
fetchEmulatorRankForEdit.mockResolvedValue({
id: 7,
rank_name: "Administrator",
level: 7,
badge: "ADM",
permissions: {},
});
execute
.mockResolvedValueOnce([
[
{ id: 1n, username: "Ada" },
{ id: 2n, username: "Linus" },
],
])
.mockResolvedValueOnce([[{ total: 237n }]])
.mockResolvedValueOnce([
[
{
id: 3n,
slug: "rank_7",
title: "Administrator",
permission_slug: "admin.settings.view",
},
],
]);
const detail = await systemAccessAdapters.loadRankDetail(7);
expect(execute).toHaveBeenCalledTimes(3);
expect(detail).toMatchObject({
userCount: 237,
users: [
{ id: 1, username: "Ada" },
{ id: 2, username: "Linus" },
],
});
});
});
@@ -28,6 +28,8 @@ export interface SystemAccessRankDetail extends SystemAccessRank {
readonly users: readonly { id: number; username: string }[]; readonly users: readonly { id: number; username: string }[];
} }
export const SYSTEM_ACCESS_USER_SAMPLE_LIMIT = 200;
export interface SystemAclOverview { export interface SystemAclOverview {
readonly roles: number; readonly roles: number;
readonly permissions: number; readonly permissions: number;
@@ -178,13 +180,18 @@ export const systemAccessAdapters: SystemAccessAdapters = {
const rank = await fetchEmulatorRankForEdit(db, rankId); const rank = await fetchEmulatorRankForEdit(db, rankId);
if (!rank) return null; if (!rank) return null;
const [userResult, roleResult] = await Promise.all([ const [userResult, userCountResult, roleResult] = await Promise.all([
db.execute(sql` db.execute(sql`
SELECT id, username SELECT id, username
FROM users FROM users
WHERE \`rank\` = ${rankId} WHERE \`rank\` = ${rankId}
ORDER BY username ASC ORDER BY username ASC
LIMIT 200 LIMIT ${SYSTEM_ACCESS_USER_SAMPLE_LIMIT}
`),
db.execute(sql`
SELECT COUNT(*) AS total
FROM users
WHERE \`rank\` = ${rankId}
`), `),
db.execute(sql` db.execute(sql`
SELECT ar.id, ar.slug, ar.title, ap.slug AS permission_slug SELECT ar.id, ar.slug, ar.title, ap.slug AS permission_slug
@@ -200,6 +207,9 @@ export const systemAccessAdapters: SystemAccessAdapters = {
id: number | bigint; id: number | bigint;
username: string; username: string;
}[]; }[];
const [userCountRow] = userCountResult[0] as unknown as {
total: number | bigint;
}[];
const roles = roleResult[0] as unknown as { const roles = roleResult[0] as unknown as {
id: number | bigint; id: number | bigint;
slug: string; slug: string;
@@ -212,7 +222,7 @@ export const systemAccessAdapters: SystemAccessAdapters = {
id: rank.id, id: rank.id,
name: rank.rank_name, name: rank.rank_name,
level: rank.level, level: rank.level,
userCount: users.length, userCount: Number(userCountRow?.total ?? 0),
badge: rank.badge, badge: rank.badge,
permissions: rank.permissions, permissions: rank.permissions,
cmsRole: role cmsRole: role