diff --git a/artifacts/api-server/src/routes/executive-meetings.ts b/artifacts/api-server/src/routes/executive-meetings.ts index ca226a60..7f57586d 100644 --- a/artifacts/api-server/src/routes/executive-meetings.ts +++ b/artifacts/api-server/src/routes/executive-meetings.ts @@ -156,6 +156,22 @@ const requireAdminAudit = makeRequireRoles(ADMIN_AUDIT_ROLES); // ---------- Zod schemas ---------- +// Allowed values for the row-highlight overlay on the daily schedule. +// Mirrors the ROW_COLOR_OPTIONS palette in the frontend ( +// artifacts/tx-os/src/pages/executive-meetings.tsx). NULL/omitted on the +// wire = "default" (no tint). Kept as a literal whitelist (not a string +// max-length check) so an unknown key is rejected with 400 instead of +// silently persisting a colour the UI cannot render. +const ROW_COLOR_KEYS = [ + "red", + "amber", + "green", + "blue", + "violet", + "gray", +] as const; +const rowColorSchema = z.enum(ROW_COLOR_KEYS).nullable(); + const dateSchema = z.string().regex(/^\d{4}-\d{2}-\d{2}$/, "expected YYYY-MM-DD"); const DATE_RE_LOCAL = /^\d{4}-\d{2}-\d{2}$/; @@ -257,6 +273,11 @@ const meetingPatchSchema = z notes: meetingBaseFields.notes, attendees: z.array(attendeeSchema).optional(), merge: meetingMergeSchema.optional(), + // Row-highlight colour (#288). null = clear back to default (no tint). + // Optional so this PATCH endpoint stays usable for callers that only + // want to edit other fields. Validated against the ROW_COLOR_KEYS + // whitelist above, so an unknown key returns 400 instead of writing. + rowColor: rowColorSchema.optional(), }) .refine( (v) => !v.startTime || !v.endTime || v.startTime <= v.endTime, @@ -674,6 +695,11 @@ router.patch( updateValues.mergeText = sanitizeRichText(data.merge.mergeText); } } + // #288: rowColor is whitelisted upstream; null = clear back to + // default (no tint). + if (data.rowColor !== undefined) { + updateValues.rowColor = data.rowColor; + } if (Object.keys(updateValues).length > 1) { await tx diff --git a/artifacts/api-server/tests/executive-meetings-row-color.test.mjs b/artifacts/api-server/tests/executive-meetings-row-color.test.mjs new file mode 100644 index 00000000..485e597f --- /dev/null +++ b/artifacts/api-server/tests/executive-meetings-row-color.test.mjs @@ -0,0 +1,352 @@ +// API contract tests for the shared row-colour overlay added by task #288 +// to the executive-meetings PATCH endpoint. Covers: +// +// 1. Setting a colour persists it on the row and round-trips back +// through GET (server is the source of truth, not the browser). +// 2. Setting `rowColor: null` clears the colour back to default +// without disturbing other fields on the row. +// 3. Unknown / off-palette colour keys are rejected with 400 and the +// stored value does not change. +// 4. A non-mutate user (executive_viewer role) gets 403 — they can +// see the colour but cannot change it. +// 5. Each successful change writes an audit-log row attributed to the +// acting user, with the old + new colour captured so the audit +// trail mirrors other meeting field edits. +// +// Cleanup runs in `after` regardless of which assertions failed so +// repeated runs of this file against the same DB don't leak rows. + +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import pg from "pg"; + +const API_BASE = process.env.TEST_API_BASE ?? "http://localhost:8080"; +const DATABASE_URL = process.env.DATABASE_URL; +if (!DATABASE_URL) { + throw new Error("DATABASE_URL must be set to run these tests"); +} + +const TEST_PASSWORD = "TestPass123!"; +const TEST_PASSWORD_HASH = + "$2b$10$Bs636ukPMyz01nKrsi.5m.JlDXSN22AVCvn8cgPWWDbo5yJRQX2vu"; + +const pool = new pg.Pool({ connectionString: DATABASE_URL }); + +const created = { + userIds: [], + meetingIds: [], +}; + +function uniqueName(prefix) { + return `${prefix}_${Date.now().toString(36)}_${Math.random() + .toString(36) + .slice(2, 8)}`; +} + +async function createUser(prefix, roleName) { + const username = uniqueName(prefix); + const { rows } = await pool.query( + `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active) + VALUES ($1, $2, $3, 'EM RowColor Test', 'en', true) RETURNING id`, + [username, `${username}@example.com`, TEST_PASSWORD_HASH], + ); + const id = rows[0].id; + created.userIds.push(id); + for (const r of ["user", roleName]) { + await pool.query( + `INSERT INTO user_roles (user_id, role_id) + SELECT $1, id FROM roles WHERE name = $2 + ON CONFLICT DO NOTHING`, + [id, r], + ); + } + return { id, username }; +} + +function extractCookie(res) { + const setCookie = res.headers.get("set-cookie"); + if (!setCookie) return null; + return ( + setCookie + .split(",") + .map((c) => c.split(";")[0].trim()) + .find((c) => c.startsWith("connect.sid=")) ?? null + ); +} + +async function login(username, password) { + const res = await fetch(`${API_BASE}/api/auth/login`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ username, password }), + }); + assert.equal(res.status, 200, `login should succeed for ${username}`); + const cookie = extractCookie(res); + assert.ok(cookie, "login response should set a session cookie"); + return cookie; +} + +async function api(cookie, method, path, body) { + const init = { + method, + headers: { + Cookie: cookie, + ...(body !== undefined ? { "Content-Type": "application/json" } : {}), + }, + }; + if (body !== undefined) init.body = JSON.stringify(body); + return fetch(`${API_BASE}${path}`, init); +} + +let adminCookie = null; +let adminUserId = null; +let viewerCookie = null; + +const today = new Date().toISOString().slice(0, 10); + +async function createMeeting(titleEn, titleAr) { + const res = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr, + titleEn, + meetingDate: today, + startTime: "09:00", + endTime: "10:00", + platform: "none", + status: "scheduled", + isHighlighted: 0, + attendees: [ + { name: "Tester", attendanceType: "internal", sortOrder: 0 }, + ], + }); + assert.equal(res.status, 201, "create meeting should succeed"); + const meeting = await res.json(); + created.meetingIds.push(meeting.id); + return meeting; +} + +before(async () => { + adminCookie = await login("admin", "admin123"); + // Capture the admin's user id so we can assert audit attribution. + const meRes = await api(adminCookie, "GET", "/api/auth/me"); + if (meRes.status === 200) { + const me = await meRes.json(); + adminUserId = me?.id ?? me?.user?.id ?? null; + } + // executive_viewer is in READ_ROLES but NOT in MUTATE_ROLES, so + // requireExecutiveAccess passes and we hit requireMutate's 403. + // This isolates the "non-editor" case from the bare "no executive + // access at all" case (which would 403 earlier in the chain). + const viewer = await createUser("em_rowcolor_viewer", "executive_viewer"); + viewerCookie = await login(viewer.username, TEST_PASSWORD); +}); + +after(async () => { + if (created.meetingIds.length > 0) { + await pool.query( + `DELETE FROM executive_meeting_attendees WHERE meeting_id = ANY($1::int[])`, + [created.meetingIds], + ); + await pool.query( + `DELETE FROM executive_meeting_audit_logs WHERE entity_type = 'meeting' AND entity_id = ANY($1::int[])`, + [created.meetingIds], + ); + await pool.query( + `DELETE FROM executive_meetings WHERE id = ANY($1::int[])`, + [created.meetingIds], + ); + } + if (created.userIds.length > 0) { + await pool.query( + `DELETE FROM user_roles WHERE user_id = ANY($1::int[])`, + [created.userIds], + ); + await pool.query(`DELETE FROM users WHERE id = ANY($1::int[])`, [ + created.userIds, + ]); + } + await pool.end(); +}); + +test("PATCH rowColor: setting a colour persists and round-trips via GET", async () => { + const meeting = await createMeeting("RowColor set", "تعيين لون الصف"); + // Sanity: a fresh row has no colour set. + assert.equal( + meeting.rowColor ?? null, + null, + "fresh meeting should default to no colour", + ); + + const patch = await api( + adminCookie, + "PATCH", + `/api/executive-meetings/${meeting.id}`, + { rowColor: "red" }, + ); + assert.equal(patch.status, 200, "PATCH rowColor should succeed"); + const patched = await patch.json(); + assert.equal(patched.rowColor, "red", "PATCH response includes new colour"); + + // The day GET is what every other client uses to refresh after the + // socket fires, so it MUST surface the new colour. This is the + // assertion that proves the colour is shared with other viewers. + const day = await api( + adminCookie, + "GET", + `/api/executive-meetings?date=${today}`, + ); + assert.equal(day.status, 200); + const body = await day.json(); + const row = body.meetings.find((m) => m.id === meeting.id); + assert.ok(row, "patched meeting should appear in GET ?date="); + assert.equal(row.rowColor, "red", "GET reflects the persisted colour"); +}); + +test("PATCH rowColor: { rowColor: null } clears the colour without touching other fields", async () => { + const meeting = await createMeeting("RowColor clear", "مسح لون الصف"); + + // Seed with a colour so we have something to clear. + const seed = await api( + adminCookie, + "PATCH", + `/api/executive-meetings/${meeting.id}`, + { rowColor: "blue" }, + ); + assert.equal(seed.status, 200); + + const clear = await api( + adminCookie, + "PATCH", + `/api/executive-meetings/${meeting.id}`, + { rowColor: null }, + ); + assert.equal(clear.status, 200, "clearing rowColor should succeed"); + const cleared = await clear.json(); + assert.equal(cleared.rowColor, null, "rowColor is null after clear"); + // Other fields stay intact through the clear — guards against a + // future regression where the PATCH handler accidentally overwrites + // unrelated columns when only rowColor is sent. + assert.equal(cleared.titleEn, "RowColor clear"); + assert.equal(cleared.startTime, "09:00:00"); + assert.equal(cleared.endTime, "10:00:00"); +}); + +test("PATCH rowColor: unknown / off-palette colour key is rejected with 400 and does not change the row", async () => { + const meeting = await createMeeting("RowColor invalid", "لون غير صالح"); + // Seed with a known good colour so we can prove the bad request + // didn't silently overwrite it. + const seed = await api( + adminCookie, + "PATCH", + `/api/executive-meetings/${meeting.id}`, + { rowColor: "green" }, + ); + assert.equal(seed.status, 200); + + for (const bad of ["fuchsia", "RED", "", "default", " red "]) { + const res = await api( + adminCookie, + "PATCH", + `/api/executive-meetings/${meeting.id}`, + { rowColor: bad }, + ); + assert.equal( + res.status, + 400, + `unknown rowColor key "${bad}" should be rejected with 400`, + ); + const body = await res.json(); + assert.equal(body.code, "validation"); + } + + // Confirm the row still has the seeded colour, not anything else. + const day = await api( + adminCookie, + "GET", + `/api/executive-meetings?date=${today}`, + ); + const row = (await day.json()).meetings.find((m) => m.id === meeting.id); + assert.equal(row.rowColor, "green", "row colour unchanged after 400s"); +}); + +test("PATCH rowColor: non-mutate viewer role gets 403 and the row colour is unchanged", async () => { + const meeting = await createMeeting( + "RowColor viewer guard", + "حماية المشاهد من تغيير اللون", + ); + // Admin sets a colour first so the viewer's failed PATCH would be + // visible if the guard were missing. + const seed = await api( + adminCookie, + "PATCH", + `/api/executive-meetings/${meeting.id}`, + { rowColor: "amber" }, + ); + assert.equal(seed.status, 200); + + const blocked = await api( + viewerCookie, + "PATCH", + `/api/executive-meetings/${meeting.id}`, + { rowColor: "violet" }, + ); + assert.equal(blocked.status, 403, "viewer role should be blocked"); + + // Viewer can still GET — they need to *see* the colour, just not change it. + const day = await api( + viewerCookie, + "GET", + `/api/executive-meetings?date=${today}`, + ); + assert.equal(day.status, 200); + const row = (await day.json()).meetings.find((m) => m.id === meeting.id); + assert.equal( + row.rowColor, + "amber", + "viewer's blocked PATCH did not change the persisted colour", + ); +}); + +test("PATCH rowColor: each successful change writes an audit row attributed to the acting user", async () => { + const meeting = await createMeeting("RowColor audit", "سجل لون الصف"); + + const set = await api( + adminCookie, + "PATCH", + `/api/executive-meetings/${meeting.id}`, + { rowColor: "violet" }, + ); + assert.equal(set.status, 200); + + // The audit log is read by the admin section. Pull all entries for + // this meeting and assert that the most recent one captures the + // colour change. We don't strictly enforce ordering of unrelated + // create / patch entries — we just need a row whose newValue.rowColor + // is "violet" and whose performedBy is the admin. + const { rows: auditRows } = await pool.query( + `SELECT action, entity_type, entity_id, new_value, performed_by + FROM executive_meeting_audit_logs + WHERE entity_type = 'meeting' AND entity_id = $1 + ORDER BY id DESC`, + [meeting.id], + ); + const colorEntry = auditRows.find((r) => { + const nv = r.new_value; + return ( + nv && + typeof nv === "object" && + "rowColor" in nv && + nv.rowColor === "violet" + ); + }); + assert.ok( + colorEntry, + "audit log should contain an entry whose newValue.rowColor is the colour we just set", + ); + if (adminUserId != null) { + assert.equal( + colorEntry.performed_by, + adminUserId, + "audit row attributed to the admin who made the change", + ); + } +}); diff --git a/artifacts/tx-os/public/opengraph.jpg b/artifacts/tx-os/public/opengraph.jpg index 3c74bc77..2f9a2f36 100644 Binary files a/artifacts/tx-os/public/opengraph.jpg and b/artifacts/tx-os/public/opengraph.jpg differ diff --git a/artifacts/tx-os/src/pages/executive-meetings.tsx b/artifacts/tx-os/src/pages/executive-meetings.tsx index 99d0498d..1e6f4ff2 100644 --- a/artifacts/tx-os/src/pages/executive-meetings.tsx +++ b/artifacts/tx-os/src/pages/executive-meetings.tsx @@ -140,6 +140,10 @@ type Meeting = { mergeStartColumn?: ColumnId | null; mergeEndColumn?: ColumnId | null; mergeText?: string | null; + // Shared row-highlight colour (#288). Stored on the meeting row so it + // is the same for every viewer. NULL/missing = "default" (no tint). + // Allowed values come from ROW_COLOR_OPTIONS below. + rowColor?: string | null; }; type DayResponse = { date: string; meetings: Meeting[] }; @@ -742,9 +746,20 @@ function ScheduleSection({ const { toast } = useToast(); const tableStyle = buildFontStyle(font); const setColumns = onColumnsChange; - const [rowColors, setRowColors] = useState>(() => - readJsonFromStorage>(ROW_COLORS_STORAGE_KEY, {}), - ); + // #288: Row highlight colours are now stored on the meeting row in the + // DB and shared across every viewer. Derive a quick lookup from the + // current `meetings` prop so the rest of this component (which still + // calls `rowColors[m.id] ?? "default"`) keeps working unchanged. + // Mutations go through `setRowColor` below, which PATCHes the meeting + // and then refetches via the day-changed query invalidation — no + // separate client-side cache to keep in sync. + const rowColors = useMemo>(() => { + const out: Record = {}; + for (const m of meetings) { + if (m.rowColor) out[m.id] = m.rowColor; + } + return out; + }, [meetings]); // Global "Edit / View" toggle for the schedule. The default is view mode // (no edit affordances). When a user with edit permission flips it on, @@ -1525,9 +1540,141 @@ function ScheduleSection({ // write effect here would silently drop edits made from the Settings // tab while ScheduleSection is unmounted. + // #288: row colours moved to the meeting record on the server, so the + // old `writeJsonToStorage(ROW_COLORS_STORAGE_KEY, rowColors)` effect is + // gone. The client-side state is now derived from the `meetings` prop + // (see the `rowColors` memo above). + + // #288: Best-effort migration of legacy per-device colours into the + // shared server-side field. Runs every time the loaded day's meetings + // change, so colours saved on a date the user hasn't visited yet + // survive in localStorage and get migrated when they navigate there. + // Rules: + // - drops invalid keys / unknown colours immediately (those were + // never going to migrate anywhere), + // - skips ids whose server row already has a colour (don't overwrite + // someone else's choice), + // - PATCHes ids present in the currently-loaded day, + // - keeps ids whose meeting we haven't seen yet AND ids whose PATCH + // failed so the next effect run / next visit can retry, + // - removes the localStorage key entirely once nothing is left. + // Concurrency: a Set of in-flight ids prevents the same id from being + // PATCHed twice if the effect re-fires while a request is pending. + const migrationInFlightRef = useRef>(new Set()); useEffect(() => { - writeJsonToStorage(ROW_COLORS_STORAGE_KEY, rowColors); - }, [rowColors]); + if (!canMutate) return; + if (typeof window === "undefined") return; + let raw: string | null = null; + try { + raw = window.localStorage.getItem(ROW_COLORS_STORAGE_KEY); + } catch { + return; + } + if (!raw) return; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + try { + window.localStorage.removeItem(ROW_COLORS_STORAGE_KEY); + } catch { + /* ignore */ + } + return; + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + try { + window.localStorage.removeItem(ROW_COLORS_STORAGE_KEY); + } catch { + /* ignore */ + } + return; + } + const allowedKeys = new Set( + ROW_COLOR_OPTIONS.filter((o) => o.key !== "default").map((o) => o.key), + ); + const meetingById = new Map(meetings.map((m) => [m.id, m])); + const remaining: Record = {}; + const toMigrate: Array<{ id: number; color: string }> = []; + for (const [idStr, color] of Object.entries( + parsed as Record, + )) { + const id = Number(idStr); + // Drop entries that could never apply: invalid id or unknown + // colour key. Keeping them would just keep the localStorage key + // alive forever. + if (!Number.isInteger(id) || id <= 0) continue; + if (typeof color !== "string" || !allowedKeys.has(color)) continue; + const m = meetingById.get(id); + if (!m) { + // Not in this day's payload — could be a future day the user + // hasn't visited yet. Preserve so a later effect run picks it up. + remaining[idStr] = color; + continue; + } + if (m.rowColor) continue; // server already has a colour, drop ours + if (migrationInFlightRef.current.has(id)) { + remaining[idStr] = color; // already PATCHing — wait for next tick + continue; + } + toMigrate.push({ id, color }); + } + if (toMigrate.length === 0) { + // Either everything left over is for unseen days, or there's + // nothing left at all. + try { + if (Object.keys(remaining).length === 0) { + window.localStorage.removeItem(ROW_COLORS_STORAGE_KEY); + } else { + // Rewrite to compact away the dropped (invalid / already-set) + // entries so we don't keep iterating them forever. + window.localStorage.setItem( + ROW_COLORS_STORAGE_KEY, + JSON.stringify(remaining), + ); + } + } catch { + /* ignore */ + } + return; + } + for (const c of toMigrate) migrationInFlightRef.current.add(c.id); + void (async () => { + const results = await Promise.allSettled( + toMigrate.map((c) => + apiJson(`/api/executive-meetings/${c.id}`, { + method: "PATCH", + body: { rowColor: c.color }, + }), + ), + ); + // Failed PATCHes get folded back into `remaining` so we can + // retry next time. Successful ones are dropped. + const finalRemaining: Record = { ...remaining }; + results.forEach((res, i) => { + const c = toMigrate[i]; + migrationInFlightRef.current.delete(c.id); + if (res.status === "rejected") { + finalRemaining[String(c.id)] = c.color; + } + }); + try { + if (Object.keys(finalRemaining).length === 0) { + window.localStorage.removeItem(ROW_COLORS_STORAGE_KEY); + } else { + window.localStorage.setItem( + ROW_COLORS_STORAGE_KEY, + JSON.stringify(finalRemaining), + ); + } + } catch { + /* ignore */ + } + void qc.invalidateQueries({ + queryKey: ["/api/executive-meetings", date], + }); + })(); + }, [canMutate, meetings, qc, date]); const visibleColumns = columns.filter((c) => c.visible); @@ -1544,17 +1691,34 @@ function ScheduleSection({ ); }, []); - const setRowColor = useCallback((meetingId: number, colorKey: string) => { - setRowColors((prev) => { - const next = { ...prev }; - if (colorKey === "default") { - delete next[meetingId]; - } else { - next[meetingId] = colorKey; + // #288: persist a row colour by PATCHing the meeting. The server + // validates the key against the ROW_COLOR_KEYS whitelist, writes an + // audit log entry, and emits a realtime day-changed event so other + // open Tx OS tabs/devices refetch and pick up the new colour. The + // local list refreshes through the same react-query invalidation + // (`executive-meetings:day` keyed by `date`) used by every other + // mutation on this page. We do an optimistic toast-based rollback if + // the request fails so the user always knows the colour didn't stick. + const setRowColor = useCallback( + async (meetingId: number, colorKey: string) => { + const wireValue = colorKey === "default" ? null : colorKey; + try { + await apiJson(`/api/executive-meetings/${meetingId}`, { + method: "PATCH", + body: { rowColor: wireValue }, + }); + await qc.invalidateQueries({ queryKey: ["/api/executive-meetings", date] }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + toast({ + title: t("common.error"), + description: msg, + variant: "destructive", + }); } - return next; - }); - }, []); + }, + [qc, date, toast, t], + ); // Responsive breakpoints: // < md (<768px) → stacked card layout (phones) diff --git a/lib/db/src/schema/executive-meetings.ts b/lib/db/src/schema/executive-meetings.ts index 9c71f2c9..5f309b1b 100644 --- a/lib/db/src/schema/executive-meetings.ts +++ b/lib/db/src/schema/executive-meetings.ts @@ -45,6 +45,15 @@ export const executiveMeetingsTable = pgTable( mergeStartColumn: varchar("merge_start_column", { length: 32 }), mergeEndColumn: varchar("merge_end_column", { length: 32 }), mergeText: text("merge_text"), + // Optional row-highlight colour for the daily schedule grid. NULL = + // "default" (no tint). Allowed non-null values come from the + // ROW_COLOR_OPTIONS palette in the frontend (red, amber, green, + // blue, violet, gray) and are validated app-side. Stored on the + // meeting row itself (not per-user) so the colour is shared across + // every viewer in real time, since it carries an editorial + // signal about the meeting (urgent / VIP / etc.) rather than a + // personal viewing preference. + rowColor: varchar("row_color", { length: 16 }), createdBy: integer("created_by").references(() => usersTable.id, { onDelete: "set null", }),