diff --git a/artifacts/api-server/src/lib/pdf-renderer.ts b/artifacts/api-server/src/lib/pdf-renderer.ts index b7c150a8..a2d2e894 100644 --- a/artifacts/api-server/src/lib/pdf-renderer.ts +++ b/artifacts/api-server/src/lib/pdf-renderer.ts @@ -13,16 +13,11 @@ export type PdfFontPrefs = { fontSize: number; fontWeight: "regular" | "bold"; alignment: "start" | "center"; - // Hex color applied to body cell text. Header cells (white-on-navy) - // and the brand logo intentionally ignore this so the brand styling - // survives even with extreme user color picks. + // Body-text fill; header chrome ignores this. fontColor: string; }; -// Mapping from a row-color palette key (`red`/`amber`/...) to the exact -// fill the on-screen schedule uses for that tint. Keep in lockstep with -// `ROW_COLOR_OPTIONS` in artifacts/tx-os/src/pages/executive-meetings.tsx -// so a meeting tinted "amber" in the UI prints with the same swatch. +// Keep in sync with ROW_COLOR_OPTIONS in tx-os executive-meetings.tsx. const ROW_COLOR_FILL: Record = { red: "#fee2e2", amber: "#fef3c7", @@ -51,17 +46,9 @@ export type PdfMeeting = { startTime: string | null; endTime: string | null; location: string | null; - // Legacy "current meeting" highlight overlay (#288). Kept on the - // type for source-compat with older callers, but the renderer now - // intentionally ignores it: per-row tint comes from `rowColor` - // instead, so transient on-screen highlights never get baked into - // the printed archive. + // Legacy field; ignored by renderer (use rowColor instead). isHighlighted?: number; - // Optional per-meeting palette key from the daily schedule (red / - // amber / green / blue / violet / gray). NULL/undefined = no tint. - // Looked up against ROW_COLOR_FILL above; unknown keys are treated - // as "no tint" rather than throwing so a future palette addition on - // the frontend can roll out without breaking server PDFs. + // Palette key from ROW_COLOR_FILL; unknown/null = no tint. rowColor?: string | null; attendees: PdfMeetingAttendee[]; }; @@ -79,10 +66,7 @@ export type RenderPdfInput = { time: string; none: string; }; - // Optional brand logo bytes (PNG/JPEG) drawn in the top-left of the - // PDF header. The renderer scales it to fit a small box at the page - // margin and lays the title/date out next to it. Pass `undefined` - // to render the legacy header-without-logo layout. + // PNG/JPEG bytes for the top-left header logo; undefined = no logo. logo?: Buffer | null; }; @@ -370,10 +354,7 @@ type DrawOpts = { align: "left" | "center" | "right"; baseDirection: "ltr" | "rtl"; mapping: FamilyMapping; - // Hex fill for the text run. Optional so existing internal callers - // (e.g. the title/date header passes that need to stay black or - // navy regardless of the user's fontColor) can keep working without - // a code change. + // Optional hex fill; header callers omit it to keep brand colors. color?: string; }; @@ -486,13 +467,7 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise doc.on("error", reject); }); - // Header — show the schedule title (and the brand logo when present) - // on a single horizontal band, then the date on its own line below. - // The logo is anchored at the page's left margin regardless of the - // RTL flag, matching the printed sample the user supplied (logo on - // the left even for the Arabic deck). Title text stays centered in - // the remaining horizontal space so an RTL/LTR mix still feels - // balanced. + // Header: logo (if any) on the left, centered title, date below. const titleSize = Math.min(28, input.font.fontSize + 10); const headerLeftEdge = doc.page.margins.left; const headerWidth = doc.page.width - doc.page.margins.left - doc.page.margins.right; @@ -502,30 +477,22 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise let titleWidth = headerWidth; if (input.logo && input.logo.length > 0) { try { - // PDFKit's image() places the top-left at (x, y) by default — no - // valign needed for top-anchored placement. doc.image(input.logo, headerLeftEdge, headerTopY, { fit: [logoBoxSize, logoBoxSize], }); - // Reserve the logo footprint plus a small gap so the centered - // title doesn't overlap the artwork. We also reserve the same - // footprint on the trailing edge so the title's geometric center - // matches the header's center, not the post-logo center. + // Symmetric reservation keeps the title geometrically centered. const gap = 12; titleX = headerLeftEdge + logoBoxSize + gap; titleWidth = headerWidth - 2 * (logoBoxSize + gap); if (titleWidth < headerWidth * 0.4) { - // Logo + symmetric reservation ate too much space — fall back - // to anchoring the title to the right of the logo only. + // Fallback: too narrow for symmetric reservation. titleX = headerLeftEdge + logoBoxSize + gap; titleWidth = headerWidth - logoBoxSize - gap; } - // PDFKit moves doc.y after image() in some paths; pin it back so - // the title sits at the same vertical baseline. + // Reset doc.y; image() may have advanced it. doc.y = headerTopY; } catch (err) { - // Bad image bytes shouldn't kill the whole PDF — log and proceed - // without the logo so the user still gets their schedule. + // Bad image bytes: log and render without the logo. console.warn("[pdf-renderer] failed to embed logo, continuing without", err); } } @@ -539,7 +506,6 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise baseDirection: baseDir, mapping, }); - // Advance past the taller of (title line, logo box) before the date. doc.y = headerTopY + Math.max(titleSize * 1.25, input.logo ? logoBoxSize : 0); doc.moveDown(0.4); drawMixedLine(doc, input.date, { @@ -718,13 +684,7 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise } const rowTop = doc.y; - // Per-meeting saved tint (#288). The transient on-screen - // "isHighlighted" overlay is intentionally NOT painted here so the - // archived PDF reflects the meeting's editorial state, not the - // viewer's current-meeting cursor. Unknown palette keys silently - // map to "no tint" so a future palette addition rolls out without - // breaking server PDFs that were generated before the palette - // grew. + // Saved per-row tint (#288); isHighlighted is not painted. if (meeting.rowColor) { const fill = ROW_COLOR_FILL[meeting.rowColor]; if (fill) { diff --git a/artifacts/api-server/src/routes/executive-meetings.ts b/artifacts/api-server/src/routes/executive-meetings.ts index 6295b136..ee4a81b9 100644 --- a/artifacts/api-server/src/routes/executive-meetings.ts +++ b/artifacts/api-server/src/routes/executive-meetings.ts @@ -305,12 +305,9 @@ const pdfArchiveCreateSchema = z.object({ filePath: z.string().trim().max(500).optional(), }); -// Hex color in #RRGGBB form. Stored verbatim and forwarded to the PDF -// renderer / inline-style attribute, so reject anything that wouldn't -// round-trip safely through CSS or PDFKit's `fillColor`. +// #RRGGBB hex color. const HEX_COLOR_RE = /^#[0-9a-fA-F]{6}$/; -// `/objects/` paths are produced by `ObjectStorageService.normalizeObjectEntityPath`. -// We allow null/empty to mean "no logo" (clear the slot). +// `/objects/` path; null clears the slot. const OBJECT_PATH_RE = /^\/objects\/[A-Za-z0-9_\-./]+$/; const fontSettingsSchema = z.object({ scope: z.enum(["user", "global"]).default("user"), @@ -322,10 +319,7 @@ const fontSettingsSchema = z.object({ .string() .regex(HEX_COLOR_RE, "expected #RRGGBB hex color") .default("#000000"), - // null = explicit clear (UI's "remove logo" gesture). Undefined = - // not touching the field on this PATCH. Only honored when scope is - // "global"; ignored for per-user rows so users can't override the - // org logo with their own. + // null = clear, undefined = leave alone. Global scope only. logoObjectPath: z .string() .regex(OBJECT_PATH_RE, "expected /objects/ path") @@ -2591,17 +2585,12 @@ async function resolveFontPrefsForUser( alignment: (eff?.alignment as PdfFontPrefs["alignment"]) ?? "start", fontColor: eff?.fontColor ?? "#000000", }, - // Logo lives at the global scope only — per-user rows ignore the - // field. This keeps the brand asset shared across all viewers and - // avoids each user having to re-upload it. + // Logo is global-scope only. logoObjectPath: globalRow?.logoObjectPath ?? null, }; } -// Download a `/objects/` path back to a Buffer so PDFKit can embed -// it as a header image. Returns null on any failure (missing object, -// network blip, sidecar offline) — the renderer falls back to the -// header-without-logo layout in that case so we never block the PDF. +// Download `/objects/` as a Buffer for PDFKit. Null on failure. async function loadLogoBytes(objectPath: string | null): Promise { if (!objectPath) return null; try { @@ -2680,10 +2669,6 @@ router.get( const { font, logoObjectPath } = await resolveFontPrefsForUser(userId); const logo = await loadLogoBytes(logoObjectPath); - // Pull the bilingual PDF labels (title + column headings) from the - // shared module instead of inlining literals here. The same keys - // are mirrored under `executiveMeetings.pdf.*` in the tx-os - // locales (ar.json / en.json) so the frontend stays in sync. const labels = getPdfLabels(lang); let pdf: Buffer; @@ -2701,11 +2686,7 @@ router.get( startTime: m.startTime, endTime: m.endTime, location: m.location, - // Forward the saved per-meeting tint (red/amber/green/...) - // so the printed PDF matches the on-screen schedule. The - // legacy `isHighlighted` overlay is intentionally NOT - // forwarded — it represents the viewer's transient - // "current meeting" cursor, not editorial state. + // Forward saved tint; isHighlighted is intentionally dropped. rowColor: m.rowColor, attendees: (attendeesByMeeting.get(m.id) ?? []).map((a) => ({ name: a.name, @@ -2908,10 +2889,7 @@ const upsertFontSettingsHandler = async ( } } const targetUserId = data.scope === "global" ? null : userId; - // Logo is a brand asset and only lives on the global row. Reject - // user-scope writes that try to set a non-null logo so callers get - // a clear error instead of a silent no-op. `null` (explicit clear) - // and `undefined` (field not provided) are both fine on user rows. + // Reject user-scope logo writes loudly; null/undefined are fine. if (data.scope === "user" && data.logoObjectPath != null) { res.status(400).json({ error: "logoObjectPath is only allowed on the global font-settings row", @@ -2941,9 +2919,7 @@ const upsertFontSettingsHandler = async ( alignment: data.alignment, fontColor: data.fontColor, }; - // Only touch logo when the field was provided AND we're on the - // global row. `null` is a real value (user clicked "remove - // logo") so we have to distinguish from `undefined`. + // Distinguish null (clear) from undefined (skip). if (logoForWrite !== undefined) { updateSet.logoObjectPath = logoForWrite; } diff --git a/artifacts/api-server/tests/executive-meetings.test.mjs b/artifacts/api-server/tests/executive-meetings.test.mjs index 7abca605..81f4f78c 100644 --- a/artifacts/api-server/tests/executive-meetings.test.mjs +++ b/artifacts/api-server/tests/executive-meetings.test.mjs @@ -697,10 +697,6 @@ test("Font settings: valid combo 200; invalid weight/size/family 400", async () }); test("Font settings: user-scope rejects logoObjectPath with 400; null is allowed", async () => { - // User-scope writes that try to set a non-null logo path must be - // rejected loudly. The brand logo is a global asset; silently - // dropping the field would mislead callers into thinking their - // upload was saved. const rejected = await api( adminCookie, "PUT", @@ -719,9 +715,7 @@ test("Font settings: user-scope rejects logoObjectPath with 400; null is allowed const body = await rejected.json(); assert.equal(body.code, "logo_user_scope_forbidden"); - // Explicit null (i.e. "I'm not setting a logo") must still work - // on the user row so a generic prefs UI doesn't have to omit the - // field. + // null is allowed on user rows. const okNull = await api( adminCookie, "PUT", @@ -915,12 +909,7 @@ test("PDF GET /executive-meetings/pdf returns a real PDF and archives it", async }); test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor", async () => { - // This guards Task #349's visual contract directly against the - // bytes the renderer emits, complementing the higher-level test - // above which only checks status + magic. We render a PDF for a - // throw-away date with two meetings — one tinted via the saved - // `rowColor`, one only flagged isHighlighted — and verify by - // inflating PDFKit's FlateDecode'd content streams. + // Inflate FlateDecode streams and assert title/tint/fontColor ops. const zlib = await import("node:zlib"); const decodeStreams = (buf) => { const out = []; @@ -944,7 +933,6 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor" return out.join("\n"); }; - // Use a unique future date so we can't collide with other tests. const pdfDate = "2099-05-03"; const ambered = await api(adminCookie, "POST", "/api/executive-meetings", { @@ -956,7 +944,6 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor" assert.equal(ambered.status, 201); const amberedId = (await ambered.json()).id; created.meetingIds.push(amberedId); - // rowColor is set via PATCH (not accepted by POST today). const tint = await api( adminCookie, "PATCH", @@ -975,8 +962,7 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor" assert.equal(highlighted.status, 201); created.meetingIds.push((await highlighted.json()).id); - // Pick a non-default tint that's distinguishable from header chrome. - // #336699 → 0.2, 0.4, 0.6 in PDFKit's `rg` op. + // #336699 → 0.2/0.4/0.6 in PDFKit's rg op. const setColor = await api( adminCookie, "PUT", @@ -1002,13 +988,7 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor" const ascii = buf.toString("latin1"); const decoded = decodeStreams(buf); - // (1) Title — "قائمة بأسماء حضور الاجتماعات" survives into the - // PDF metadata `/Title` field. PDFKit serializes non-ASCII PDF - // strings as parenthesised literals: BOM `\xFE\xFF` + UTF-16BE, - // with `(`, `)` and `\` byte values backslash-escaped. To dodge - // the escape problem we only assert on the UTF-16BE prefix of - // the first four glyphs ("قائم"), which contains no metacharacters. - // ق=0x0642 ا=0x0627 ئ=0x0626 م=0x0645 + // (1) /Title prefix as UTF-16BE for "قائم" (0x0642 0x0627 0x0626 0x0645). const titlePrefix = Buffer.from([ 0xfe, 0xff, 0x06, 0x42, 0x06, 0x27, 0x06, 0x26, 0x06, 0x45, ]); @@ -1017,10 +997,7 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor" "PDF /Title metadata must start with the new Arabic title (قائم…)", ); - // PDFKit emits full-precision floats and uses the `scn` operator - // (set non-stroke color in the current color space) once a color - // space has been selected with `cs`. Older PDFKit emits `rg` for - // pure-DeviceRGB writes — accept either form. + // Accept rg or scn (PDFKit varies by version). const op = "(?:rg|scn)"; // (2) rowColor=amber → #fef3c7 = 254/255, 243/255, 199/255. @@ -1030,42 +1007,25 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor" "amber rowColor must paint the saved palette fill", ); - // (3) The legacy isHighlighted overlay (#fecaca = 254/255, 202/255, - // 202/255) must NOT appear anymore — archival PDFs reflect - // editorial state only. + // (3) Legacy isHighlighted fill (#fecaca) must NOT appear. assert.doesNotMatch( decoded, new RegExp(`0\\.9960\\d* 0\\.7921\\d* 0\\.7921\\d* ${op}`), "isHighlighted must no longer be baked into the PDF", ); - // (4) The user's fontColor must reach body text as a fill op. - // #336699 = 51/255 = 0.2 (exact), 102/255 = 0.4 (exact), 153/255 = - // 0.6 (exact) — PDFKit prints them without trailing decimals. + // (4) fontColor #336699 must appear as a fill op. assert.match( decoded, new RegExp(`(?:^|\\s)0\\.2 0\\.4 0\\.6 ${op}`), "user fontColor #336699 must appear as a fill operator", ); - void ascii; // kept for future ad-hoc debugging + void ascii; }); test("PDF embeds the brand logo when set in global font settings", async () => { - // Task #349 done-criterion: when an admin uploads a brand logo via - // the global font-settings, the rendered PDF must actually carry - // the bytes as an embedded Image XObject (not just remember the - // path). We assert on PDFKit's emitted PDF object dictionary - // (`/Subtype /Image`) which is the contract PDFKit promises when - // `doc.image()` succeeds. - // - // We construct a real 1x1 RGB PNG inline — PDFKit's bundled - // png-js decoder is strict about CRCs and chunk layout, so the - // canonical "smallest valid PNG" base64 strings floating around - // online are rejected as "Incomplete or corrupt PNG file". - // Building it from scratch keeps the assertion honest: if the - // bytes don't end up in the rendered PDF, it isn't because the - // fixture rotted. + // Build a real 1x1 RGB PNG (png-js rejects most base64 fixtures). const u32 = (n) => { const b = Buffer.alloc(4); b.writeUInt32BE(n, 0); @@ -1089,9 +1049,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => { const crc = crc32(Buffer.concat([typeBuf, data])); return Buffer.concat([u32(data.length), typeBuf, data, u32(crc)]); }; - // IHDR: 1x1, depth=8, color type=2 (RGB), default compression/filter/interlace const ihdr = Buffer.concat([u32(1), u32(1), Buffer.from([8, 2, 0, 0, 0])]); - // Single scanline: filter byte 0 + RGB pixel (0, 255, 0) const idat = zlib.deflateSync(Buffer.from([0, 0, 255, 0])); const tinyPng = Buffer.concat([ Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), @@ -1100,8 +1058,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => { mkChunk("IEND", Buffer.alloc(0)), ]); - // 1) Sign an upload URL via the server's storage endpoint. We use - // the admin cookie so the requireAuth gate passes. + // 1) Sign upload URL. const signRes = await api( adminCookie, "POST", @@ -1116,15 +1073,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => { const { uploadURL, objectPath } = await signRes.json(); assert.ok(/^\/objects\//.test(objectPath), `expected /objects/, got ${objectPath}`); - // 2) PUT the PNG bytes through the signed URL. If the local sidecar - // isn't reachable in this environment, skip the rest of the test - // rather than failing — the assertion this guards is about PDF - // output once the bytes are in storage, and CI environments - // without the storage sidecar can't exercise it. - // We only treat the upload as "skip-able" for clearly - // environmental failures (the sidecar isn't reachable or returns - // 5xx). Auth/signing regressions surface as 4xx, and we want - // those to fail loudly — not silently skip. + // 2) PUT bytes; skip on network/5xx, fail on 4xx. let putRes; try { putRes = await fetch(uploadURL, { @@ -1152,10 +1101,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => { `signed PUT must succeed (got ${putRes.status} ${putRes.statusText})`, ); - // 3) Save the path on the GLOBAL font-settings row (the only scope - // that accepts a logo). Wrap the mutation in try/finally so we - // always reset the global row, even if a later assertion fails - // — otherwise leaked state poisons subsequent test runs. + // 3) Save logo on global row (try/finally resets it). const setLogo = await api( adminCookie, "PUT", @@ -1173,8 +1119,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => { assert.equal(setLogo.status, 200, "admin must be able to save the global logo"); try { - // 4) Render a PDF. Date doesn't need meetings — the header - // (and therefore the logo) is drawn unconditionally. + // 4) Render PDF (header draws unconditionally). const pdfDate = "2099-05-04"; const res = await api( adminCookie, @@ -1189,19 +1134,13 @@ test("PDF embeds the brand logo when set in global font settings", async () => { const buf = Buffer.from(await res.arrayBuffer()); const ascii = buf.toString("latin1"); - // 5) Assert PDFKit emitted an Image XObject. PDFKit writes - // image objects as `<<\n/Type /XObject\n/Subtype /Image\n…>>` - // — the `/Subtype /Image` substring is the canonical marker - // that the bytes were embedded (vs the path being silently - // dropped at render time). + // 5) Assert PDFKit emitted an Image XObject. assert.match( ascii, /\/Subtype\s*\/Image/, "uploaded brand logo must be embedded as an /Image XObject in the PDF", ); - // The 1x1 PNG should appear with /Width 1 in the image - // dictionary — a stronger guarantee that the *uploaded* image - // (and not some other XObject) made it through. + // /Width 1 confirms our 1x1 fixture was embedded. assert.match( ascii, /\/Width\s+1[^0-9]/, diff --git a/artifacts/tx-os/src/pages/executive-meetings.tsx b/artifacts/tx-os/src/pages/executive-meetings.tsx index cb0e3885..485d722d 100644 --- a/artifacts/tx-os/src/pages/executive-meetings.tsx +++ b/artifacts/tx-os/src/pages/executive-meetings.tsx @@ -207,8 +207,7 @@ type FontPrefs = { fontSize: number; fontWeight: "regular" | "bold"; alignment: "start" | "center"; - // Hex color (#RRGGBB) the API now persists alongside the other font - // prefs. Default "#000000" matches the historical hard-coded black. + // #RRGGBB persisted with font prefs. fontColor: string; }; @@ -7360,9 +7359,7 @@ function FontSettingsSection({ const { toast } = useToast(); const [scope, setScope] = useState<"user" | "global">("user"); const [prefs, setPrefs] = useState(font); - // Local mirror of the org-wide logo path. Tracks server state but - // also reflects in-flight upload/clear gestures so the preview - // updates immediately without waiting for the next refetch. + // Local mirror of the global logo path for instant preview. const [logoPath, setLogoPath] = useState(globalLogoObjectPath); const [saving, setSaving] = useState(false); @@ -7373,10 +7370,7 @@ function FontSettingsSection({ setLogoPath(globalLogoObjectPath); }, [globalLogoObjectPath]); - // Logo uploads always target the global brand asset, regardless of - // the scope toggle, because per-user logos aren't a thing. The - // upload completes against object storage; we only persist the - // resulting `/objects/` path when the user clicks "Save" below. + // Logo is global-only; persisted on Save. const { uploadFile: uploadLogo, isUploading: isUploadingLogo } = useUpload({ onSuccess: (resp: UploadResponse) => setLogoPath(resp.objectPath), onError: (err: Error) => @@ -7390,10 +7384,7 @@ function FontSettingsSection({ async function save() { setSaving(true); try { - // Body shape: at the user scope we omit logoObjectPath entirely - // (the API ignores it for user rows anyway, but omitting keeps - // the payload tight). At the global scope we always send the - // current path so "remove logo" persists as `null`. + // Omit logoObjectPath on user scope; send it (incl. null) on global. const body: Record = { scope, ...prefs }; if (scope === "global") { body.logoObjectPath = logoPath; @@ -7418,9 +7409,7 @@ function FontSettingsSection({ function onLogoFileChange(e: React.ChangeEvent) { const file = e.target.files?.[0]; - // Reset the input so picking the same file twice in a row still - // re-fires onChange — otherwise users hit a confusing dead state - // after a failed upload. + // Reset input so re-picking the same file fires onChange. e.target.value = ""; if (!file) return; void uploadLogo(file);