fix(housekeeping): address task 14 review round 2

This commit is contained in:
Simo committed 2026-08-30 11:30:17 +02:00
1 parent d09eaa33d6
commit c524b305d7
17 files changed
+614 -130

No files matched your search

File diff suppressed because it is too large. Load diff
+31 -6
View File
@@ -1,8 +1,9 @@
"use server";
import { revalidatePath } from "next/cache";
import { executeLegacyBrandAssetMutation } from "@/features/housekeeping/domains/content/services/mutation-runtime-external";
import { requirePermission } from "@/lib/admin/guard";
import type { ContentMutationSnapshot } from "@/features/housekeeping/domains/content/services/mutations";
import { createCorrelationId } from "@/features/housekeeping/foundation/contracts";
import { requirePermission, type StaffUser } from "@/lib/admin/guard";
import { PERMS } from "@/lib/permissions";
const MAX_SIZE = 2 * 1024 * 1024;
@@ -15,10 +16,34 @@ const ALLOWED = [
"image/svg+xml",
];
async function executeAuditedFaviconMutation(
staff: StaffUser,
operation: "favicon.save" | "favicon.delete",
input: unknown,
): Promise<ContentMutationSnapshot> {
const [
{ contentProductionMutationAdapter },
{ getHousekeepingCapabilityContext },
] = await Promise.all([
import(
"@/features/housekeeping/domains/content/services/mutations-production"
),
import("@/features/housekeeping/foundation/server-capability-context"),
]);
const capability = await getHousekeepingCapabilityContext();
if (capability.actor.id !== staff.id)
throw new Error("Authenticated staff changed during favicon mutation");
return contentProductionMutationAdapter.execute(operation, input, {
capability,
correlationId: createCorrelationId(),
legacy: true,
});
}
export async function saveFavicon(
formData: FormData,
): Promise<{ success: boolean; url?: string; error?: string }> {
await requirePermission(PERMS.SETTINGS_VIEW);
const staff = await requirePermission(PERMS.SETTINGS_VIEW);
try {
const file = formData.get("file") as File | null;
if (!file || file.size === 0)
@@ -30,7 +55,7 @@ export async function saveFavicon(
success: false,
error: "Invalid file type. Allowed: PNG, JPEG, GIF, WebP, ICO, SVG",
};
const result = await executeLegacyBrandAssetMutation("favicon.save", {
const result = await executeAuditedFaviconMutation(staff, "favicon.save", {
file,
});
siteRevalidate();
@@ -52,9 +77,9 @@ export async function deleteFavicon(): Promise<{
success: boolean;
error?: string;
}> {
await requirePermission(PERMS.SETTINGS_VIEW);
const staff = await requirePermission(PERMS.SETTINGS_VIEW);
try {
await executeLegacyBrandAssetMutation("favicon.delete", {});
await executeAuditedFaviconMutation(staff, "favicon.delete", {});
siteRevalidate();
return { success: true };
} catch (error) {
+28 -4
View File
@@ -1,17 +1,41 @@
"use server";
import { revalidatePath } from "next/cache";
import { executeLegacyBrandAssetMutation } from "@/features/housekeeping/domains/content/services/mutation-runtime-external";
import { requireStaff } from "@/lib/admin/guard";
import type { ContentMutationSnapshot } from "@/features/housekeeping/domains/content/services/mutations";
import { createCorrelationId } from "@/features/housekeeping/foundation/contracts";
import { requireStaff, type StaffUser } from "@/lib/admin/guard";
async function executeAuditedLogoMutation(
staff: StaffUser,
input: unknown,
): Promise<ContentMutationSnapshot> {
const [
{ contentProductionMutationAdapter },
{ getHousekeepingCapabilityContext },
] = await Promise.all([
import(
"@/features/housekeeping/domains/content/services/mutations-production"
),
import("@/features/housekeeping/foundation/server-capability-context"),
]);
const capability = await getHousekeepingCapabilityContext();
if (capability.actor.id !== staff.id)
throw new Error("Authenticated staff changed during logo mutation");
return contentProductionMutationAdapter.execute("logo.save", input, {
capability,
correlationId: createCorrelationId(),
legacy: true,
});
}
export async function saveLogo(
formData: FormData,
): Promise<{ success: boolean; url?: string; error?: string }> {
await requireStaff();
const staff = await requireStaff();
try {
const file = formData.get("file") as File | null;
if (!file) return { success: false, error: "No file provided" };
const result = await executeLegacyBrandAssetMutation("logo.save", { file });
const result = await executeAuditedLogoMutation(staff, { file });
revalidatePath("/", "layout");
return {
success: true,
+34
View File
@@ -0,0 +1,34 @@
import { existsSync } from "node:fs";
import { readFile } from "node:fs/promises";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { GET } from "./route";
vi.mock("node:fs", () => ({ existsSync: vi.fn() }));
vi.mock("node:fs/promises", () => ({ readFile: vi.fn() }));
vi.mock("@/lib/media-storage", () => ({
MEDIA_ROOT: "C:\\media",
resolveMediaPath: vi.fn(
(name: string) => `C:\\media\\${name.replaceAll("/", "\\")}`,
),
}));
describe("GET /api/media/[...path]", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(existsSync).mockReturnValue(true);
vi.mocked(readFile).mockResolvedValue(Buffer.from([0, 0, 1, 0]));
});
it("serves a stored genuine ICO with the canonical MIME and nosniff", async () => {
const response = await GET(
new Request("http://localhost/api/media/favicon/site.ico"),
{
params: Promise.resolve({ path: ["favicon", "site.ico"] }),
},
);
expect(response.status).toBe(200);
expect(response.headers.get("content-type")).toBe("image/x-icon");
expect(response.headers.get("x-content-type-options")).toBe("nosniff");
expect(readFile).toHaveBeenCalledOnce();
});
});
+11 -1
View File
@@ -4,7 +4,16 @@ import path from "node:path";
import { NextResponse } from "next/server";
import { MEDIA_ROOT, resolveMediaPath } from "@/lib/media-storage";
const ALLOWED_EXT = [".png", ".jpg", ".jpeg", ".gif", ".webp", ".svg", ".bmp"];
const ALLOWED_EXT = [
".png",
".jpg",
".jpeg",
".gif",
".webp",
".svg",
".bmp",
".ico",
];
export async function GET(
_request: Request,
@@ -41,6 +50,7 @@ export async function GET(
".webp": "image/webp",
".svg": "image/svg+xml",
".bmp": "image/bmp",
".ico": "image/x-icon",
};
return new NextResponse(bytes, {
@@ -142,6 +142,21 @@ describe("Content production providers", () => {
).rejects.toThrow("Content widget query unavailable");
});
it("marks an inbox dependency unavailable instead of returning an available empty list", async () => {
run.mockResolvedValueOnce({
ok: false,
error: { code: "DEPENDENCY_UNAVAILABLE", messageKey: "dependency" },
correlationId: "unavailable",
});
await expect(
loadContentInboxItems(
"publication",
context,
new AbortController().signal,
),
).rejects.toThrow("Content inbox query unavailable");
});
it("emits only actionable publication statuses", async () => {
run.mockResolvedValueOnce(
result("content.editorial.articles", 2, [
@@ -53,7 +53,7 @@ export async function loadContentInboxItems(
routeId,
list: { pageSize: 25, offset: 0 },
});
if (!result.ok) continue;
if (!result.ok) throw new Error("Content inbox query unavailable");
for (const item of result.data.items) {
const status = item.status?.toLocaleLowerCase() ?? "";
if (!ACTIONABLE_STATUS[kind].has(status)) continue;
@@ -19,7 +19,7 @@ export interface ContentCommandField {
readonly min?: number;
readonly max?: number;
readonly maxLength?: number;
readonly defaultValue?: string | number;
readonly defaultValue?: string | number | boolean;
readonly options?: readonly Readonly<{
value: string | number;
label: string;
@@ -41,8 +41,8 @@ const OMIT_FIELD = Symbol("omit optional Content command field");
function parseField(field: ContentCommandField, formData: FormData): unknown {
const rawValue = formData.get(field.name);
if (rawValue === null && !field.required) return OMIT_FIELD;
if (field.type === "checkbox") return rawValue === "on";
if (rawValue === null && !field.required) return OMIT_FIELD;
if (field.type === "file") return rawValue instanceof File ? rawValue : null;
const raw = String(rawValue ?? "")
.normalize("NFC")
@@ -136,6 +136,7 @@ export function ContentCommandForm({
id={`${commandId}-${field.name}`}
name={field.name}
type="checkbox"
defaultChecked={field.defaultValue === true}
/>{" "}
{field.label}
</>
@@ -145,7 +146,11 @@ export function ContentCommandForm({
<select
id={`${commandId}-${field.name}`}
name={field.name}
defaultValue={field.defaultValue}
defaultValue={
typeof field.defaultValue === "boolean"
? undefined
: field.defaultValue
}
required={field.required}
className="mt-1 block w-full"
>
@@ -182,7 +187,10 @@ export function ContentCommandForm({
: "text"
}
defaultValue={
field.type === "file" ? undefined : field.defaultValue
field.type === "file" ||
typeof field.defaultValue === "boolean"
? undefined
: field.defaultValue
}
required={field.required}
min={field.min}
@@ -151,7 +151,7 @@ describe("Content actionable form wiring", () => {
});
});
it("omits untouched optional fields from partial-update submissions", async () => {
it("omits untouched optional text but submits a declared unchecked checkbox as false", async () => {
vi.mocked(executeHousekeepingCommand).mockResolvedValue(
ok({ before: null, after: { id: "7" } }, "partial-form"),
);
@@ -177,10 +177,29 @@ describe("Content actionable form wiring", () => {
expect(executeHousekeepingCommand).toHaveBeenCalledWith({
commandId: "content.media.banner.change",
input: { action: "update", id: "7", title: "Renamed" },
input: {
action: "update",
id: "7",
isActive: false,
title: "Renamed",
},
});
});
it("renders poll creation with show-results checked by default", () => {
const html = renderToStaticMarkup(
<ContentEngagementPage
context={context([PERMS.POLLS_EDIT])}
result={ok(
{ kind: "engagement", items: [], total: 0, partialDependencies: [] },
"poll-create-checkbox",
)}
routeId="content.engagement.poll-create"
/>,
);
expect(html).toMatch(/name="showResults"[^>]*checked=""/u);
});
it("renders mutation forms only with the exact capability", () => {
const result = ok(
{
@@ -108,7 +108,12 @@ export function ContentEngagementPage({
{ value: "closed", label: "Closed" },
],
},
{ name: "showResults", label: "Show results", type: "checkbox" },
{
name: "showResults",
label: "Show results",
type: "checkbox",
defaultValue: creatingPoll,
},
{
name: "multipleChoice",
label: "Allow multiple choices",
File diff suppressed because it is too large. Load diff
@@ -12,9 +12,23 @@ import {
loadProductionContentQuery,
} from "./content-queries-production";
const queryMocks = vi.hoisted(() => ({ execute: vi.fn() }));
const queryMocks = vi.hoisted(() => ({
execute: vi.fn(),
join: vi.fn((chunks: unknown[], separator: unknown) => ({
chunks,
separator,
})),
raw: vi.fn((statement: string) => ({ statement })),
tagged: vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => ({
strings: Array.from(strings),
values,
})),
}));
vi.mock("drizzle-orm", () => ({
sql: { raw: (statement: string) => statement },
sql: Object.assign(queryMocks.tagged, {
join: queryMocks.join,
raw: queryMocks.raw,
}),
}));
vi.mock("@/lib/db", () => ({ db: { execute: queryMocks.execute } }));
@@ -45,15 +59,15 @@ const ready: ContentQueryData = {
describe("Content query", () => {
it("reports totals beyond the former 500-row adapter cap", async () => {
const allRows = Array.from({ length: 600 }, (_, index) => ({
const pageRows = Array.from({ length: 25 }, (_, index) => ({
id: index + 1,
title: `Article ${index + 1}`,
status: "published",
updated_at: null,
}));
queryMocks.execute.mockImplementationOnce(async (statement: string) => [
statement.includes("LIMIT 500") ? allRows.slice(0, 500) : allRows,
]);
queryMocks.execute
.mockResolvedValueOnce([[{ total: 600 }]])
.mockResolvedValueOnce([pageRows]);
const result = await loadProductionContentQuery({
routeId: "content.editorial.articles",
params: {},
@@ -61,6 +75,41 @@ describe("Content query", () => {
});
expect(result.total).toBe(600);
expect(result.items).toHaveLength(25);
expect(queryMocks.execute).toHaveBeenCalledTimes(2);
const pageQuery = queryMocks.execute.mock.calls[1]?.[0];
expect(JSON.stringify(pageQuery)).toContain("25");
expect(JSON.stringify(pageQuery)).toContain("0");
});
it("binds search, limit, and offset into the bounded page query", async () => {
queryMocks.execute
.mockResolvedValueOnce([[{ total: 1 }]])
.mockResolvedValueOnce([
[{ id: 9, title: "Launch", status: "published", updated_at: null }],
]);
await loadProductionContentQuery({
routeId: "content.editorial.articles",
params: {},
list: { search: "Launch", pageSize: 7, offset: 14 },
});
const serialized = JSON.stringify(queryMocks.execute.mock.calls);
expect(serialized).toContain("%launch%");
expect(serialized).toContain("7");
expect(serialized).toContain("14");
});
it("searches only projected aliases for routes without descriptions", async () => {
queryMocks.execute
.mockResolvedValueOnce([[{ total: 0 }]])
.mockResolvedValueOnce([[]]);
await loadProductionContentQuery({
routeId: "content.engagement.events",
params: {},
list: { search: "launch", pageSize: 25, offset: 0 },
});
const serialized = JSON.stringify(queryMocks.execute.mock.calls.slice(-2));
expect(serialized).toContain("COALESCE(title");
expect(serialized).not.toContain("COALESCE(description");
});
it("never selects email template bodies into summary query results", () => {
@@ -2,6 +2,18 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
import type { HousekeepingCapabilityContext } from "../../../foundation/contracts";
import { executeContentDatabaseMutation } from "./mutation-runtime-database";
const sqlMocks = vi.hoisted(() => ({
join: vi.fn((chunks: unknown[], separator: unknown) => ({
chunks,
separator,
})),
raw: vi.fn((statement: string) => statement),
tagged: vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => ({
strings: Array.from(strings),
values,
})),
}));
const database = vi.hoisted(() => {
let selected: Record<string, unknown> = { id: 7, title: "Existing" };
const set = vi.fn((values: Record<string, unknown>) => ({
@@ -13,7 +25,9 @@ const database = vi.hoisted(() => {
const where = vi.fn(() => ({ limit }));
const from = vi.fn(() => ({ where }));
const select = vi.fn(() => ({ from }));
const execute = vi.fn(async () => undefined);
return {
execute,
set,
update,
select,
@@ -25,12 +39,16 @@ const database = vi.hoisted(() => {
vi.mock("drizzle-orm", () => ({
eq: vi.fn(),
sql: Object.assign(vi.fn(), { raw: vi.fn() }),
sql: Object.assign(sqlMocks.tagged, {
join: sqlMocks.join,
raw: sqlMocks.raw,
}),
}));
vi.mock("@/lib/db", () => {
const table = new Proxy({}, { get: (_target, key) => String(key) });
return {
db: {
execute: database.execute,
select: database.select,
update: database.update,
},
@@ -142,4 +160,38 @@ describe("Content database mutation runtime partial updates", () => {
expect(database.set.mock.calls[0]?.[0]).toHaveProperty(expectedKey);
},
);
it("updates only submitted prefix columns and preserves omitted icon, effect, and active", async () => {
const snapshot = await executeContentDatabaseMutation(
"prefix.change",
{ action: "update", id: 7, text: "Renamed" },
context,
undefined,
);
const assignments = sqlMocks.join.mock.calls.at(-1)?.[0] as
| Array<{ strings: string[]; values: unknown[] }>
| undefined;
expect(assignments).toHaveLength(1);
expect(assignments?.[0]?.strings.join(" ")).toContain("text =");
expect(assignments?.[0]?.strings.join(" ")).not.toMatch(
/icon|effect|active/u,
);
expect(snapshot?.after).toEqual({ id: 7, text: "Renamed" });
});
it("submits an explicit false prefix active patch without touching other columns", async () => {
const snapshot = await executeContentDatabaseMutation(
"prefix.change",
{ action: "update", id: 7, active: false },
context,
undefined,
);
const assignments = sqlMocks.join.mock.calls.at(-1)?.[0] as
| Array<{ strings: string[]; values: unknown[] }>
| undefined;
expect(assignments).toHaveLength(1);
expect(assignments?.[0]?.strings.join(" ")).toContain("active =");
expect(assignments?.[0]?.values).toEqual([0]);
expect(snapshot?.after).toEqual({ id: 7, active: 0 });
});
});
@@ -1,6 +1,6 @@
import "server-only";
import { eq, sql } from "drizzle-orm";
import { eq, type SQL, sql } from "drizzle-orm";
import type { ResultSetHeader } from "mysql2";
import {
db,
@@ -443,13 +443,11 @@ async function eventChange(
if (action === "create") {
const parsed = createEventSchema.safeParse(data);
if (!parsed.success) throw validation();
const [result] = await connection
.insert(WebsiteEvent)
.values({
...parsed.data,
hostUserId: context.capability.actor.id,
updatedAt: new Date(),
});
const [result] = await connection.insert(WebsiteEvent).values({
...parsed.data,
hostUserId: context.capability.actor.id,
updatedAt: new Date(),
});
const id = insertedId(result);
return {
before: null,
@@ -653,14 +651,12 @@ async function tagChange(
const name = text(data.name, 255, true);
const backgroundColor = text(data.backgroundColor, 10) || "#888888";
const now = new Date();
const [result] = (await connection
.insert(Tags)
.values({
name,
backgroundColor,
createdAt: now,
updatedAt: now,
})) as unknown as [ResultSetHeader];
const [result] = (await connection.insert(Tags).values({
name,
backgroundColor,
createdAt: now,
updatedAt: now,
})) as unknown as [ResultSetHeader];
const id = insertedId(result);
await activity(
context,
@@ -740,17 +736,35 @@ async function prefixChange(
return { before: { id }, after: null };
}
if (action !== "update") throw validation();
const prefixText = text(data.text, 255, true);
const color = text(data.color, 32, true);
const icon = text(data.icon, 255);
const effect = text(data.effect, 255);
const active = Math.min(1, nonNegativeInteger(data.active, 1));
const values: Record<string, unknown> = {};
const assignments: SQL[] = [];
if (hasOwn(data, "text")) {
values.text = text(data.text, 255, true);
assignments.push(sql`text = ${values.text}`);
}
if (hasOwn(data, "color")) {
values.color = text(data.color, 32, true);
assignments.push(sql`color = ${values.color}`);
}
if (hasOwn(data, "icon")) {
values.icon = text(data.icon, 255);
assignments.push(sql`icon = ${values.icon}`);
}
if (hasOwn(data, "effect")) {
values.effect = text(data.effect, 255);
assignments.push(sql`effect = ${values.effect}`);
}
if (hasOwn(data, "active")) {
values.active = booleanValue(data.active) ? 1 : 0;
assignments.push(sql`active = ${values.active}`);
}
requirePatch(values);
await connection.execute(
sql`UPDATE custom_prefixes SET text = ${prefixText}, color = ${color}, icon = ${icon}, effect = ${effect}, active = ${active} WHERE id = ${id}`,
sql`UPDATE custom_prefixes SET ${sql.join(assignments, sql`, `)} WHERE id = ${id}`,
);
return {
before: { id },
after: { id, text: prefixText, color, icon, effect, active },
after: { id, ...values },
output: { id: String(id) },
};
}
@@ -281,15 +281,24 @@ async function photoDelete(
.limit(1);
if (!row) throw notFound();
await db.delete(CameraWeb).where(eq(CameraWeb.id, id));
await tryRemoveLocalPhotoFile(row.url);
await logStaffActivity({
staffId: context.capability.actor.id,
action: "photo_delete",
description: "Deleted camera photo #" + id,
targetType: "camera_web",
targetId: id,
});
return { before: { id, url: row.url }, after: null };
const snapshot: ContentMutationSnapshot = {
before: { id, url: row.url },
after: null,
};
try {
const removed = await tryRemoveLocalPhotoFile(row.url);
if (!removed) throw new Error("Photo file purge failed");
await logStaffActivity({
staffId: context.capability.actor.id,
action: "photo_delete",
description: "Deleted camera photo #" + id,
targetType: "camera_web",
targetId: id,
});
} catch {
throw new ContentCommittedExternalFailure(snapshot);
}
return snapshot;
}
async function navigationUpdate(
@@ -475,14 +484,22 @@ async function themeCustomChange(
if (action !== "create" && action !== "update" && action !== "rename")
throw validation();
const name = text(data.name, 100, true);
const settings = data.values
? Object.fromEntries(
Object.entries(jsonRecord(data.values)).map(([key, value]) => [
key,
String(value),
]),
)
: await snapshotCurrentTheme();
let settings: Record<string, string>;
if (action === "rename") {
if (!id) throw validation();
const existing = await getCustomTheme(id);
if (!existing) throw notFound();
settings = existing.settings;
} else {
settings = data.values
? Object.fromEntries(
Object.entries(jsonRecord(data.values)).map(([key, value]) => [
key,
String(value),
]),
)
: await snapshotCurrentTheme();
}
const theme = await upsertCustomTheme(name, settings, id || undefined);
await logStaffActivity({
staffId: context.capability.actor.id,
@@ -799,15 +816,6 @@ const EXTERNAL_HANDLERS: Partial<
"translation.emulator.save": emulatorTranslationSave,
};
export async function executeLegacyBrandAssetMutation(
operation: "favicon.save" | "favicon.delete" | "logo.save",
input: unknown,
): Promise<ContentMutationSnapshot> {
if (operation === "favicon.save") return faviconSave(input);
if (operation === "favicon.delete") return faviconDelete();
return logoSave(input);
}
export async function executeContentExternalMutation(
operation: ContentMutationOperation,
input: unknown,
@@ -122,6 +122,32 @@ describe("Content production mutation adapter", () => {
expect(JSON.stringify(deps.writeAudit.mock.calls)).not.toContain("Hello");
});
it("routes a logo write through correlated intent and outcome audit", async () => {
const deps = dependencies();
await deps.adapter.execute(
"logo.save",
{ file: { name: "logo.png" } },
mutationContext,
);
expect(deps.executeOperation).toHaveBeenCalledWith(
"logo.save",
expect.anything(),
mutationContext,
);
expect(deps.writeAudit.mock.calls.map(([entry]) => entry)).toEqual([
expect.objectContaining({
action: "content.logo.save",
outcome: "intent",
correlationId: "production-matrix",
}),
expect.objectContaining({
action: "content.logo.save",
outcome: "success",
correlationId: "production-matrix",
}),
]);
});
it("returns typed partial when database committed but the external effect failed", async () => {
const deps = dependencies();
deps.executeOperation.mockRejectedValueOnce(