From d09eaa33d699cc19f564e740def3f874f38d6d55 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Sun, 30 Aug 2026 10:53:50 +0200 Subject: [PATCH] fix(housekeeping): address task 14 review round 1 --- src/actions/admin-banners.ts | 3 + src/actions/admin-photos.test.ts | 52 +- src/actions/content-legacy-parity.test.ts | 79 +- src/actions/save-favicon.ts | 85 +- src/actions/save-logo.ts | 42 +- src/actions/set-trade-lock.test.ts | 38 +- src/app/api/media/[...path]/route.ts | 7 + .../content-providers-production.test.ts | 79 +- .../domains/content/content-providers.test.ts | 64 +- .../domains/content/inbox-production.ts | 21 +- .../housekeeping/domains/content/inbox.ts | 7 +- .../content/pages/content-command-form.tsx | 29 +- .../content/pages/content-pages.test.tsx | 124 ++- .../domains/content/pages/engagement.tsx | 419 ++++++++- .../domains/content/pages/localization.tsx | 120 ++- .../queries/content-queries-production.ts | 154 ++-- .../content/queries/content-queries.test.ts | 30 +- .../mutation-runtime-database.test.ts | 145 ++++ .../services/mutation-runtime-database.ts | 812 ++++++++++++++---- .../mutation-runtime-external.test.ts | 134 ++- .../services/mutation-runtime-external.ts | 519 +++++++++-- .../domains/content/widgets-production.ts | 24 +- src/lib/admin-operations-contract.test.ts | 4 +- 23 files changed, 2503 insertions(+), 488 deletions(-) create mode 100644 src/actions/admin-banners.ts create mode 100644 src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts diff --git a/src/actions/admin-banners.ts b/src/actions/admin-banners.ts new file mode 100644 index 0000000000..288e99cdf7 --- /dev/null +++ b/src/actions/admin-banners.ts @@ -0,0 +1,3 @@ +"use server"; + +export { createBanner, deleteBanner, updateBanner } from "./banners"; diff --git a/src/actions/admin-photos.test.ts b/src/actions/admin-photos.test.ts index ce8ac4f4aa..048a859f24 100644 --- a/src/actions/admin-photos.test.ts +++ b/src/actions/admin-photos.test.ts @@ -1,25 +1,44 @@ // @ts-nocheck +import { readFileSync } from "node:fs"; import { revalidatePath } from "next/cache"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { requirePermission } from "@/lib/admin/guard"; +import { tryRemoveLocalPhotoFile } from "@/lib/admin/photo-files"; import { deletePhoto } from "./admin-photos"; const { execute } = vi.hoisted(() => ({ execute: vi.fn() })); -vi.mock("@/features/housekeeping/domains/content/services/mutations", () => ({ contentMutationService: { execute }, createContentMutationInvocation: (actor, correlationId) => ({ expectedActorId: actor.id, correlationId, legacy: true }) })); +vi.mock("@/features/housekeeping/domains/content/services/mutations", () => ({ + contentMutationService: { execute }, + createContentMutationInvocation: (actor, correlationId) => ({ + expectedActorId: actor.id, + correlationId, + legacy: true, + }), +})); vi.mock("@/lib/admin/guard", () => ({ requirePermission: vi.fn() })); vi.mock("@/lib/permissions", () => ({ PERMS: { PAGES_EDIT: "pages.edit" } })); vi.mock("next/cache", () => ({ revalidatePath: vi.fn() })); beforeEach(() => { vi.clearAllMocks(); - vi.mocked(requirePermission).mockResolvedValue({ id: 1, rank: 7, username: "admin" }); - execute.mockResolvedValue({ ok: true, data: { before: { id: 42 }, after: null }, correlationId: "legacy" }); + vi.mocked(requirePermission).mockResolvedValue({ + id: 1, + rank: 7, + username: "admin", + }); + execute.mockResolvedValue({ + ok: true, + data: { before: { id: 42 }, after: null }, + correlationId: "legacy", + }); }); describe("deletePhoto", () => { it("delegates deletion and preserves both revalidations", async () => { - await deletePhoto({ get: (key) => key === "id" ? "42" : null }); - expect(execute).toHaveBeenCalledWith(expect.anything(), "photo.delete", { id: 42 }); + await deletePhoto({ get: (key) => (key === "id" ? "42" : null) }); + expect(execute).toHaveBeenCalledWith(expect.anything(), "photo.delete", { + id: 42, + }); expect(revalidatePath).toHaveBeenCalledWith("/admin/photos"); expect(revalidatePath).toHaveBeenCalledWith("/photos"); }); @@ -28,3 +47,26 @@ describe("deletePhoto", () => { expect(execute).not.toHaveBeenCalled(); }); }); + +describe("admin-photos extracted runtime contract", () => { + it("keeps the wrapper and owning runtime responsible for purge and audit", () => { + const wrapper = readFileSync("src/actions/admin-photos.ts", "utf8"); + const runtime = readFileSync( + "src/features/housekeeping/domains/content/services/mutation-runtime-external.ts", + "utf8", + ); + expect(wrapper).toContain('"photo.delete"'); + expect(wrapper).toContain('revalidatePath("/photos")'); + expect(runtime).toContain("CameraWeb"); + expect(runtime).toContain("tryRemoveLocalPhotoFile"); + expect(runtime).toContain("logStaffActivity"); + }); + + it("rejects traversal and remote photo purge targets", async () => { + expect(await tryRemoveLocalPhotoFile("https://cdn.example/photo.png")).toBe( + false, + ); + expect(await tryRemoveLocalPhotoFile("/../../etc/passwd")).toBe(false); + expect(await tryRemoveLocalPhotoFile("")).toBe(false); + }); +}); diff --git a/src/actions/content-legacy-parity.test.ts b/src/actions/content-legacy-parity.test.ts index a36a576342..ea44832a42 100644 --- a/src/actions/content-legacy-parity.test.ts +++ b/src/actions/content-legacy-parity.test.ts @@ -2,11 +2,12 @@ import { readFileSync } from "node:fs"; import { redirect } from "next/navigation"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { requirePermission } from "@/lib/admin/guard"; +import { requirePermission, requireStaff } from "@/lib/admin/guard"; import { createAd } from "./admin-ads"; import { createArticle } from "./admin-articles"; import { uploadMedia } from "./admin-media"; -import { saveFavicon } from "./save-favicon"; +import { deleteFavicon, saveFavicon } from "./save-favicon"; +import { saveLogo } from "./save-logo"; const { execute } = vi.hoisted(() => ({ execute: vi.fn(async () => ({ @@ -15,6 +16,13 @@ const { execute } = vi.hoisted(() => ({ correlationId: "legacy", })), })); +const { executeLegacyBrandAssetMutation } = vi.hoisted(() => ({ + executeLegacyBrandAssetMutation: vi.fn(async () => ({ + before: null, + after: { value: "/api/media/x" }, + output: { url: "/api/media/x" }, + })), +})); vi.mock("@/features/housekeeping/domains/content/services/mutations", () => ({ contentMutationService: { execute }, @@ -24,7 +32,16 @@ vi.mock("@/features/housekeeping/domains/content/services/mutations", () => ({ legacy: true, }), })); -vi.mock("@/lib/admin/guard", () => ({ requirePermission: vi.fn() })); +vi.mock( + "@/features/housekeeping/domains/content/services/mutation-runtime-external", + () => ({ + executeLegacyBrandAssetMutation, + }), +); +vi.mock("@/lib/admin/guard", () => ({ + requirePermission: vi.fn(), + requireStaff: vi.fn(), +})); vi.mock("@/lib/safe-action", () => ({ adminAction: (_options: unknown, handler: unknown) => handler, })); @@ -40,6 +57,7 @@ vi.mock("@/lib/permissions", () => ({ NEWS_EDIT: "news.edit", PAGES_EDIT: "pages.edit", SETTINGS_EDIT: "settings.edit", + SETTINGS_VIEW: "settings.view", }, })); vi.mock("@/lib/db", () => ({ @@ -84,6 +102,7 @@ const form = (data: Record) => ({ beforeEach(() => { vi.clearAllMocks(); vi.mocked(requirePermission).mockResolvedValue(staff as never); + vi.mocked(requireStaff).mockResolvedValue(staff as never); execute.mockResolvedValue({ ok: true, data: { before: null, after: { id: "1" }, output: { url: "/api/media/x" } }, @@ -111,7 +130,9 @@ describe("Content legacy wrappers", () => { it("delegates ad creation and keeps the legacy void/redirect contract", async () => { expect( - await createAd(form({ image: "https://example.test/ad.png" }) as FormData), + await createAd( + form({ image: "https://example.test/ad.png" }) as FormData, + ), ).toBeUndefined(); expect(execute).toHaveBeenCalledWith( expect.objectContaining({ expectedActorId: 42, legacy: true }), @@ -132,17 +153,51 @@ describe("Content legacy wrappers", () => { "media.upload", expect.objectContaining({ file }), ); - expect(execute).toHaveBeenCalledWith( - expect.anything(), + expect(executeLegacyBrandAssetMutation).toHaveBeenCalledWith( "favicon.save", - expect.objectContaining({ file }), + { file }, ); }); + it("preserves the legacy favicon page gate and establishes a staff logo floor", async () => { + vi.clearAllMocks(); + const file = new File(["bytes"], "image.png", { type: "image/png" }); + await saveFavicon(form({ file }) as FormData); + await deleteFavicon(); + await saveLogo(form({ file }) as FormData); + expect(requirePermission).toHaveBeenNthCalledWith(1, "settings.view"); + expect(requirePermission).toHaveBeenNthCalledWith(2, "settings.view"); + expect(requirePermission).not.toHaveBeenCalledWith("settings.edit"); + expect(requireStaff).toHaveBeenCalledOnce(); + expect( + executeLegacyBrandAssetMutation.mock.calls.map( + ([operation]) => operation, + ), + ).toEqual(["favicon.save", "favicon.delete", "logo.save"]); + }); + + it("does not mutate brand assets when either legacy guard denies access", async () => { + const file = new File(["bytes"], "image.png", { type: "image/png" }); + vi.mocked(requirePermission).mockRejectedValueOnce( + new Error("favicon denied"), + ); + await expect(saveFavicon(form({ file }) as FormData)).rejects.toThrow( + "favicon denied", + ); + expect(executeLegacyBrandAssetMutation).not.toHaveBeenCalled(); + + vi.mocked(requireStaff).mockRejectedValueOnce(new Error("logo denied")); + await expect(saveLogo(form({ file }) as FormData)).rejects.toThrow( + "logo denied", + ); + expect(executeLegacyBrandAssetMutation).not.toHaveBeenCalled(); + }); + it("keeps every listed legacy action as a thin shared-service wrapper", () => { for (const path of [ "src/actions/admin-ads.ts", "src/actions/admin-articles.ts", + "src/actions/admin-banners.ts", "src/actions/admin-email-templates.ts", "src/actions/admin-help.ts", "src/actions/admin-media.ts", @@ -160,9 +215,13 @@ describe("Content legacy wrappers", () => { "src/actions/translations.ts", "src/actions/emulator.ts", ]) { - expect(readFileSync(path, "utf8"), path).toContain( - "contentMutationService", - ); + const source = readFileSync(path, "utf8"); + expect( + source.includes("contentMutationService") || + source.includes("executeLegacyBrandAssetMutation") || + source.includes('from "./banners"'), + path, + ).toBe(true); } }); }); diff --git a/src/actions/save-favicon.ts b/src/actions/save-favicon.ts index 64705537d4..043c2480fb 100644 --- a/src/actions/save-favicon.ts +++ b/src/actions/save-favicon.ts @@ -1,43 +1,68 @@ "use server"; import { revalidatePath } from "next/cache"; -import { - contentMutationService, - createContentMutationInvocation, -} from "@/features/housekeeping/domains/content/services/mutations"; -import { createCorrelationId } from "@/features/housekeeping/foundation/contracts"; +import { executeLegacyBrandAssetMutation } from "@/features/housekeeping/domains/content/services/mutation-runtime-external"; import { requirePermission } from "@/lib/admin/guard"; import { PERMS } from "@/lib/permissions"; const MAX_SIZE = 2 * 1024 * 1024; -const ALLOWED = ["image/png", "image/jpeg", "image/gif", "image/webp", "image/x-icon", "image/svg+xml"]; +const ALLOWED = [ + "image/png", + "image/jpeg", + "image/gif", + "image/webp", + "image/x-icon", + "image/svg+xml", +]; -export async function saveFavicon(formData: FormData): Promise<{ success: boolean; url?: string; error?: string }> { - const staff = await requirePermission(PERMS.SETTINGS_EDIT); - const file = formData.get("file") as File | null; - if (!file || file.size === 0) return { success: false, error: "No file provided" }; - if (file.size > MAX_SIZE) return { success: false, error: "File too large (max 2MB)" }; - if (!ALLOWED.includes(file.type)) return { success: false, error: "Invalid file type. Allowed: PNG, JPEG, GIF, WebP, ICO, SVG" }; - const result = await contentMutationService.execute( - createContentMutationInvocation(staff, createCorrelationId()), - "favicon.save", - { file }, - ); - if (!result.ok) return { success: false, error: result.error.messageKey }; - siteRevalidate(); - return { success: true, ...(typeof result.data.output?.url === "string" ? { url: result.data.output.url } : {}) }; +export async function saveFavicon( + formData: FormData, +): Promise<{ success: boolean; url?: string; error?: string }> { + await requirePermission(PERMS.SETTINGS_VIEW); + try { + const file = formData.get("file") as File | null; + if (!file || file.size === 0) + return { success: false, error: "No file provided" }; + if (file.size > MAX_SIZE) + return { success: false, error: "File too large (max 2MB)" }; + if (!ALLOWED.includes(file.type)) + return { + success: false, + error: "Invalid file type. Allowed: PNG, JPEG, GIF, WebP, ICO, SVG", + }; + const result = await executeLegacyBrandAssetMutation("favicon.save", { + file, + }); + siteRevalidate(); + return { + success: true, + ...(typeof result.output?.url === "string" + ? { url: result.output.url } + : {}), + }; + } catch (error) { + return { + success: false, + error: error instanceof Error ? error.message : "Unknown error", + }; + } } -export async function deleteFavicon(): Promise<{ success: boolean; error?: string }> { - const staff = await requirePermission(PERMS.SETTINGS_EDIT); - const result = await contentMutationService.execute( - createContentMutationInvocation(staff, createCorrelationId()), - "favicon.delete", - {}, - ); - if (!result.ok) return { success: false, error: result.error.messageKey }; - siteRevalidate(); - return { success: true }; +export async function deleteFavicon(): Promise<{ + success: boolean; + error?: string; +}> { + await requirePermission(PERMS.SETTINGS_VIEW); + try { + await executeLegacyBrandAssetMutation("favicon.delete", {}); + siteRevalidate(); + return { success: true }; + } catch (error) { + return { + success: false, + error: error instanceof Error ? error.message : "Unknown error", + }; + } } function siteRevalidate(): void { diff --git a/src/actions/save-logo.ts b/src/actions/save-logo.ts index 5d52025508..74dac12801 100644 --- a/src/actions/save-logo.ts +++ b/src/actions/save-logo.ts @@ -1,24 +1,28 @@ "use server"; import { revalidatePath } from "next/cache"; -import { - contentMutationService, - createContentMutationInvocation, -} from "@/features/housekeeping/domains/content/services/mutations"; -import { createCorrelationId } from "@/features/housekeeping/foundation/contracts"; -import { requirePermission } from "@/lib/admin/guard"; -import { PERMS } from "@/lib/permissions"; +import { executeLegacyBrandAssetMutation } from "@/features/housekeeping/domains/content/services/mutation-runtime-external"; +import { requireStaff } from "@/lib/admin/guard"; -export async function saveLogo(formData: FormData): Promise<{ success: boolean; url?: string; error?: string }> { - const staff = await requirePermission(PERMS.SETTINGS_EDIT); - const file = formData.get("file") as File | null; - if (!file) return { success: false, error: "No file provided" }; - const result = await contentMutationService.execute( - createContentMutationInvocation(staff, createCorrelationId()), - "logo.save", - { file }, - ); - if (!result.ok) return { success: false, error: result.error.messageKey }; - revalidatePath("/", "layout"); - return { success: true, ...(typeof result.data.output?.url === "string" ? { url: result.data.output.url } : {}) }; +export async function saveLogo( + formData: FormData, +): Promise<{ success: boolean; url?: string; error?: string }> { + 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 }); + revalidatePath("/", "layout"); + return { + success: true, + ...(typeof result.output?.url === "string" + ? { url: result.output.url } + : {}), + }; + } catch (error) { + return { + success: false, + error: error instanceof Error ? error.message : "Unknown error", + }; + } } diff --git a/src/actions/set-trade-lock.test.ts b/src/actions/set-trade-lock.test.ts index b5052ac6b8..69a07361d5 100644 --- a/src/actions/set-trade-lock.test.ts +++ b/src/actions/set-trade-lock.test.ts @@ -1,30 +1,28 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; -import { tryRemoveLocalPhotoFile } from "@/lib/admin/photo-files"; -describe("admin-photos Content service contract", () => { - const wrapper = readFileSync("src/actions/admin-photos.ts", "utf8"); - const runtime = readFileSync( - "src/features/housekeeping/domains/content/services/mutation-runtime-external.ts", +describe("setTradeLock database and live-sync contract", () => { + const wrapper = readFileSync("src/actions/bulk-users.ts", "utf8"); + const service = readFileSync( + "src/features/housekeeping/domains/people/services/mutations.ts", "utf8", ); + const rcon = readFileSync("src/lib/services/rcon.ts", "utf8"); - it("delegates while the runtime deletes CameraWeb and purges local files", () => { - expect(wrapper).toContain("contentMutationService.execute"); - expect(wrapper).toContain('"photo.delete"'); - expect(wrapper).toContain('revalidatePath("/photos")'); - expect(runtime).toContain("@/lib/db"); - expect(runtime).toContain("CameraWeb"); - expect(runtime).toContain("tryRemoveLocalPhotoFile"); + it("retains the legacy action while the owning service writes both trade-lock stores", () => { + expect(wrapper).toMatch(/export async function setTradeLock/u); + expect(wrapper).toContain('"user.trade-lock"'); + expect(service).toContain("UsersSettings"); + expect(service).toContain("Sanctions"); + expect(service).toContain("canTrade"); + expect(service).toContain("tradeLockedUntil"); }); -}); -describe("tryRemoveLocalPhotoFile", () => { - it("rejects path traversal and remote CDN urls", async () => { - expect(await tryRemoveLocalPhotoFile("https://cdn.example/photo.png")).toBe( - false, - ); - expect(await tryRemoveLocalPhotoFile("/../../etc/passwd")).toBe(false); - expect(await tryRemoveLocalPhotoFile("")).toBe(false); + it("keeps live RCON lock, alert, and disconnect behavior", () => { + expect(rcon).toContain("settradelock"); + expect(rcon).toContain("setTradeLock(userId: number, locked: boolean)"); + expect(service).toContain("rcon.setTradeLock"); + expect(service).toContain("rcon.alertUser"); + expect(service).toContain("rcon.disconnectUser"); }); }); diff --git a/src/app/api/media/[...path]/route.ts b/src/app/api/media/[...path]/route.ts index cbfc054050..b0bea0c3b3 100644 --- a/src/app/api/media/[...path]/route.ts +++ b/src/app/api/media/[...path]/route.ts @@ -47,6 +47,13 @@ export async function GET( headers: { // eslint-disable-next-line security/detect-object-injection -- ext validated against ALLOWED_EXT "Content-Type": mime[ext] ?? "application/octet-stream", + "X-Content-Type-Options": "nosniff", + ...(ext === ".svg" + ? { + "Content-Security-Policy": + "sandbox; default-src 'none'; style-src 'unsafe-inline'", + } + : {}), "Cache-Control": "public, max-age=3600, must-revalidate", }, }); diff --git a/src/features/housekeeping/domains/content/content-providers-production.test.ts b/src/features/housekeeping/domains/content/content-providers-production.test.ts index 97d41c1821..e83a0327f5 100644 --- a/src/features/housekeeping/domains/content/content-providers-production.test.ts +++ b/src/features/housekeeping/domains/content/content-providers-production.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { PERMS } from "@/lib/permission-slugs"; import type { HousekeepingCapabilityContext } from "../../foundation/contracts"; import { loadContentInboxItems } from "./inbox-production"; import { loadContentSearchCandidates } from "./search-production"; @@ -18,6 +19,16 @@ const context = { hasAll: () => true, } satisfies HousekeepingCapabilityContext; +function capability(granted: readonly string[]): HousekeepingCapabilityContext { + const permissions = new Set(granted); + return { + ...context, + has: (slug) => permissions.has(slug), + hasAny: (...slugs) => slugs.some((slug) => permissions.has(slug)), + hasAll: (...slugs) => slugs.every((slug) => permissions.has(slug)), + }; +} + function result( routeId: string, total: number, @@ -45,9 +56,9 @@ describe("Content production providers", () => { it("returns only truthful persisted counts from editorial and localization widgets", async () => { const signal = new AbortController().signal; - await expect(loadContentWidget("editorial", context, signal)).resolves.toEqual( - { articles: 7 }, - ); + await expect( + loadContentWidget("editorial", context, signal), + ).resolves.toEqual({ articles: 7 }); await expect( loadContentWidget("localization", context, signal), ).resolves.toEqual({ stores: 3 }); @@ -79,7 +90,9 @@ describe("Content production providers", () => { ]); expect(run).toHaveBeenCalledWith( context, - expect.objectContaining({ list: { search: "launch", pageSize: 25, offset: 0 } }), + expect.objectContaining({ + list: { search: "launch", pageSize: 25, offset: 0 }, + }), ); }); @@ -101,11 +114,59 @@ describe("Content production providers", () => { context, new AbortController().signal, ); - expect(items).toHaveLength(1); - expect(items[0]).toMatchObject({ - itemId: "5", - sourceId: "content.publication", - href: "/ase/content/editorial/articles/5", + expect(items).toEqual([]); + }); + + it("loads only capability-matched media counts", async () => { + const result = await loadContentWidget( + "media", + capability([PERMS.BANNERS_VIEW]), + new AbortController().signal, + ); + expect(result).toEqual({ banners: 3 }); + expect(run).toHaveBeenCalledTimes(1); + expect(run).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ routeId: "content.media.banners" }), + ); + }); + + it("does not invent a zero when a widget dependency is unavailable", async () => { + run.mockResolvedValueOnce({ + ok: false, + error: { code: "DEPENDENCY_UNAVAILABLE", messageKey: "dependency" }, + correlationId: "unavailable", }); + await expect( + loadContentWidget("editorial", context, new AbortController().signal), + ).rejects.toThrow("Content widget query unavailable"); + }); + + it("emits only actionable publication statuses", async () => { + run.mockResolvedValueOnce( + result("content.editorial.articles", 2, [ + { + id: "draft", + title: "Draft", + status: "draft", + updatedAt: new Date().toISOString(), + href: "/ase/content/editorial/articles/draft", + }, + { + id: "published", + title: "Published", + status: "published", + updatedAt: new Date().toISOString(), + href: "/ase/content/editorial/articles/published", + }, + ]), + ); + const items = await loadContentInboxItems( + "publication", + context, + new AbortController().signal, + ); + expect(items.map((item) => item.itemId)).toEqual(["draft"]); + expect(items[0]?.state).toBe("draft"); }); }); diff --git a/src/features/housekeeping/domains/content/content-providers.test.ts b/src/features/housekeeping/domains/content/content-providers.test.ts index 948c5dd54c..83b25629c7 100644 --- a/src/features/housekeeping/domains/content/content-providers.test.ts +++ b/src/features/housekeeping/domains/content/content-providers.test.ts @@ -4,10 +4,7 @@ import { anyCapability, type HousekeepingCapabilityContext, } from "../../foundation/contracts"; -import { - CONTENT_INBOX_SOURCE_IDS, - createContentInboxSources, -} from "./inbox"; +import { CONTENT_INBOX_SOURCE_IDS, createContentInboxSources } from "./inbox"; import { CONTENT_SEARCH_PROVIDER_IDS, createContentSearchProviders, @@ -74,27 +71,30 @@ describe("Content search providers", () => { "/ase/content/%255c..%255csystem", "https://example.test/ase/content/editorial", "//example.test/ase/content/editorial", - ])("rejects normalized and double-encoded traversal href %s", async (href) => { - const adapters = { - articles: async () => [ - { - id: "unsafe", - title: "Unsafe", - href, - capability: anyCapability(PERMS.NEWS_VIEW), - }, - ], - events: async () => [], - media: async () => [], - help: async () => [], - }; - const [provider] = createContentSearchProviders(adapters); - const result = await provider.search(context([PERMS.NEWS_VIEW]), { - term: "", - limit: 25, - }); - expect(result).toMatchObject({ ok: true, data: [] }); - }); + ])( + "rejects normalized and double-encoded traversal href %s", + async (href) => { + const adapters = { + articles: async () => [ + { + id: "unsafe", + title: "Unsafe", + href, + capability: anyCapability(PERMS.NEWS_VIEW), + }, + ], + events: async () => [], + media: async () => [], + help: async () => [], + }; + const [provider] = createContentSearchProviders(adapters); + const result = await provider.search(context([PERMS.NEWS_VIEW]), { + term: "", + limit: 25, + }); + expect(result).toMatchObject({ ok: true, data: [] }); + }, + ); }); describe("Content inbox and widgets", () => { @@ -119,6 +119,20 @@ describe("Content inbox and widgets", () => { }); }); + it("does not advertise media permissions for an event-and-poll attention source", async () => { + const attention = vi.fn(async () => []); + const [, source] = createContentInboxSources({ + publication: async () => [], + attention, + }); + const result = await source.getItems( + context([PERMS.PAGES_VIEW, PERMS.BANNERS_VIEW]), + new AbortController().signal, + ); + expect(result).toMatchObject({ ok: false, error: { code: "FORBIDDEN" } }); + expect(attention).not.toHaveBeenCalled(); + }); + it("keeps editorial mandatory and media/localization optional without preview DB imports", async () => { const adapters = { editorial: vi.fn(async () => ({ drafts: 2, scheduled: 1 })), diff --git a/src/features/housekeeping/domains/content/inbox-production.ts b/src/features/housekeeping/domains/content/inbox-production.ts index ab7f8923ff..b27013ad99 100644 --- a/src/features/housekeeping/domains/content/inbox-production.ts +++ b/src/features/housekeeping/domains/content/inbox-production.ts @@ -10,6 +10,11 @@ import { contentQuery } from "./queries/content-queries"; type ContentInboxKind = "publication" | "attention"; +const ACTIONABLE_STATUS = { + publication: new Set(["draft", "scheduled", "pending", "failed"]), + attention: new Set(["draft", "cancelled", "closed", "failed"]), +} as const; + function time(value: string | null | undefined) { if (!value) return null; const timestamp = Date.parse(value); @@ -31,7 +36,11 @@ export async function loadContentInboxItems( const definitions = kind === "publication" ? ([ - ["content.editorial.articles", PERMS.NEWS_VIEW, "content.publication"], + [ + "content.editorial.articles", + PERMS.NEWS_VIEW, + "content.publication", + ], ] as const) : ([ ["content.engagement.events", PERMS.EVENTS_VIEW, "content.attention"], @@ -39,12 +48,15 @@ export async function loadContentInboxItems( ] as const); const items: HousekeepingWorkItem[] = []; for (const [routeId, permission, sourceId] of definitions) { + if (!context.has(permission)) continue; const result = await contentQuery.run(context, { routeId, list: { pageSize: 25, offset: 0 }, }); if (!result.ok) continue; for (const item of result.data.items) { + const status = item.status?.toLocaleLowerCase() ?? ""; + if (!ACTIONABLE_STATUS[kind].has(status)) continue; const date = time(item.updatedAt); if (!date || !item.href) continue; items.push({ @@ -53,10 +65,11 @@ export async function loadContentInboxItems( deduplicationKey: sourceId + ":" + routeId + ":" + item.id, domain: "content", capability: anyCapability(permission), - severity: item.status === "failed" ? "warning" : "info", - priority: item.status === "failed" ? "high" : "normal", + severity: + status === "failed" || status === "cancelled" ? "warning" : "info", + priority: status === "failed" ? "high" : "normal", ...date, - state: item.status ?? "ready", + state: status, titleKey: "pages.housekeeping.items.content", context: { title: item.title }, href: item.href as `/ase/${string}`, diff --git a/src/features/housekeeping/domains/content/inbox.ts b/src/features/housekeeping/domains/content/inbox.ts index cb20b5d3d6..e99e365974 100644 --- a/src/features/housekeeping/domains/content/inbox.ts +++ b/src/features/housekeeping/domains/content/inbox.ts @@ -76,12 +76,7 @@ export function createContentInboxSources( ), createSource( "content.attention", - anyCapability( - PERMS.EVENTS_VIEW, - PERMS.POLLS_VIEW, - PERMS.PAGES_VIEW, - PERMS.BANNERS_VIEW, - ), + anyCapability(PERMS.EVENTS_VIEW, PERMS.POLLS_VIEW), adapters.attention, ), ]; diff --git a/src/features/housekeeping/domains/content/pages/content-command-form.tsx b/src/features/housekeeping/domains/content/pages/content-command-form.tsx index c49bf17f90..a4c7dfae28 100644 --- a/src/features/housekeeping/domains/content/pages/content-command-form.tsx +++ b/src/features/housekeeping/domains/content/pages/content-command-form.tsx @@ -37,12 +37,19 @@ interface ContentCommandFormProps extends ContentCommandSubmission { readonly buttonLabel: string; } +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 (field.type === "file") return rawValue instanceof File ? rawValue : null; - const raw = String(rawValue ?? "").normalize("NFC").trim(); + const raw = String(rawValue ?? "") + .normalize("NFC") + .trim(); + if (!raw && !field.required) return OMIT_FIELD; if (field.type === "number") { + if (!raw) return raw; const value = Number(raw); if (!Number.isSafeInteger(value)) return 0; return Math.min(field.max ?? value, Math.max(field.min ?? value, value)); @@ -60,7 +67,10 @@ function parseField(field: ContentCommandField, formData: FormData): unknown { return null; } } - return raw.slice(0, field.maxLength ?? (field.type === "textarea" ? 20_000 : 500)); + return raw.slice( + 0, + field.maxLength ?? (field.type === "textarea" ? 20_000 : 500), + ); } const initialState: HousekeepingResult | null = null; @@ -70,14 +80,13 @@ export async function submitContentCommandForm( _previous: HousekeepingResult | null, formData: FormData, ): Promise> { + const submittedFields = (configuration.fields ?? []).flatMap((field) => { + const value = parseField(field, formData); + return value === OMIT_FIELD ? [] : [[field.name, value] as const]; + }); const input = { ...configuration.input, - ...Object.fromEntries( - (configuration.fields ?? []).map((field) => [ - field.name, - parseField(field, formData), - ]), - ), + ...Object.fromEntries(submittedFields), }; const reason = String(formData.get("reason") ?? "") .normalize("NFC") @@ -127,7 +136,8 @@ export function ContentCommandForm({ id={`${commandId}-${field.name}`} name={field.name} type="checkbox" - /> {field.label} + />{" "} + {field.label} ) : field.type === "select" ? ( <> @@ -139,6 +149,7 @@ export function ContentCommandForm({ required={field.required} className="mt-1 block w-full" > + {field.required ? null : } {field.options?.map((option) => (