Task #108 — Executive Meetings Phase 2 (RBAC + audit + Zod + i18n)

Schedule UI / shell: - Centered cells, locked RTL column order # | الاجتماع | الحضور | الوقت, attendees column widest, tighter padding, navy/white/gray + red highlight only. Header label "م" → "#" (ar.json). Schema (lib/db/src/schema/executive-meetings.ts): - executive_meeting_requests.meetingId is now nullable so create-suggestion requests work without an existing meeting. db push applied. Backend (artifacts/api-server/src/routes/executive-meetings.ts) — full rewrite: - ALL routes (including /me) gated by requireExecutiveAccess plus fine-grained requireMutate / requireApprove / requireRequest / requireAdminAudit on the appropriate routes. - Zod schemas validate every mutating body via parseBody(). - All mutations write to executiveMeetingAuditLogsTable via logAudit(). - New nested route POST /:id/requests in addition to top-level requests POST. - Status-driven approvals: PATCH /requests/:id accepts {status: 'approved'|'rejected'|'needs_edit', reviewNotes?} or {action:'withdraw'} for the requester. - PUT /font-settings (canonical) plus PATCH alias — both wired DIRECTLY to a single shared upsertFontSettingsHandler (no Express method-rewrite hack). - GET /tasks supports date and assigneeId filters. - GET /notifications now accepts ?date=YYYY-MM-DD and joins through the meetings table to scope notifications to the selected day; returns [] immediately when no meetings exist on that date. - Removed all `as never` casts via $inferInsert/Partial typing. Auth middleware (artifacts/api-server/src/middlewares/auth.ts): - Added requireExecutiveAccess gating EXECUTIVE_READ_ROLES (admin + executive_*). Frontend (artifacts/tx-os/src/pages/executive-meetings.tsx): - Tab-level RBAC: SECTIONS.filter(isSectionVisible(key, me)) so unauthorized users never see Manage / Approvals / Audit / Tasks tabs (matrix in the in-file isSectionVisible helper). - Approvals: review() sends {status, reviewNotes}; new "Send back for edits" (needs_edit) button beside Approve / Reject. - Requests default scope is "mine" for non-approvers and "all" for approvers (no over-exposure to plain requesters). - Notifications visibility uses canRead AND the query is scoped by the same selected `date` as the schedule (queryKey + ?date=… included). - PdfSection: primary "Print + Archive" button (testid em-print) does archive then print in one click; em-print-only and em-archive remain as separate buttons. - AuditSection: 6th column renders the new in-file AuditDiffSummary component (compact "field: old → new" diff with 6-row truncation and create/remove fallback). - New tWithFallback(t, key, fallback) helper used in audit + notifications cells (avoids strict TFunction overload errors); apiJson rewritten to extract body/headers separately and stringify rawBody, eliminating the "Record<string, unknown> not assignable to BodyInit" TS errors. tsc --noEmit on executive-meetings.tsx is now clean. - Print page (executive-meetings-print.tsx) intentionally keeps its inline bilingual T table — it loads in a standalone print window before the i18n provider mounts, so an inline dictionary is the right pattern. i18n (en.json + ar.json): - New keys executiveMeetings.approvals.{needsEdit, needsEditDone}, .audit.col.changes, .audit.{created, removed, moreChanges}, .audit.entity.pdf_archive, .audit.action.{approved, rejected, needs_edit, withdraw, apply, duplicate, replace_attendees, done}, .pdf.{print = "Print + archive" / "طباعة وأرشفة", printOnly, archivedAndPrinted}. Validation: - runTest e2e passed: login, schedule loads with "#" header, Manage tab creates "اجتماع تجريبي E2E", Approvals/PDF/Audit tabs render with expected testids and the new Changes column. - curl smoke: login 200, /me 200, dates 200, create meeting 201, nested POST /:id/requests 201, PATCH /requests/:id status approval 200, audit-logs 200, DELETE meeting 204, PUT and PATCH /font-settings both 200 with persisted data, /notifications?date=YYYY-MM-DD filters correctly. - tsc --noEmit on artifacts/tx-os/src/pages/executive-meetings.tsx is clean (pre-existing admin.tsx errors are unrelated to this task). - Architect review issued VERDICT: APPROVED on the previous round; only the date wiring on NotificationsSection had to be added afterwards (now done — NotificationsSection({date}) and queryKey/url include date). Validation-round-6 fixes (this commit): - Bilingual title is now mandatory across the API + UI. Backend Zod schema (meetingBaseFields.titleEn) requires .min(1); the manage form's client-side `save()` validator now rejects empty titleEn alongside empty titleAr (single i18n error key reused). - Tasks: full reassign + edit flow. Coordinator-lead/admin actions now expose a per-row "Reassign / edit" button (em-task-reassign-{id}) opening a dialog with assignedTo + notes inputs that PATCHes /executive-meetings/tasks/{id}. New i18n keys executiveMeetings.tasks.{reassign,reassigned} (AR + EN). - Audit: filter row now includes From + To date pickers (em-audit-date-from / -to), an actor ID filter (em-audit-actor) and an action filter (em-audit-action) on top of the existing entity filter. Backend already accepts dateFrom/dateTo/action/actorId so only the UI needed wiring + i18n keys executiveMeetings.audit.filter.{from,to,actorId} (AR + EN). - PDF archives: each archive row in the snapshots list now has an "Open" button (em-archive-open-{id}) that opens the print view for that snapshot's archiveDate + version in a new tab. New i18n key executiveMeetings.pdf.openArchive (AR + EN). - Requests: status filter now exposes the full enum the backend accepts: new, needs_edit, approved, rejected, withdrawn, done (status labels were already translated). - AI-slop cleanup: trimmed verbose explanatory comments in artifacts/api-server/tests/executive-meetings.test.mjs and the artifacts/tx-os/tests/executive-meetings-manage-create.spec.mjs. - Test coverage extended: API test count went from 8 → 14, adding bilingual-title 400 path, attendees PUT replace, /duplicate roundtrip, full review-and-apply pipeline (highlight request applied), task reassign PATCH, audit filter combination (dateFrom/dateTo/action/ actorId/entityType), and pdf-archive POST→GET. All 14 pass against the live API. The Manage create E2E (1 spec) still passes. Validation-round-5 fixes: - Coordinator task scoping (server-side, blocking RBAC fix). Added TASK_BROAD_VIEW_ROLES (admin + office_manager + coord_lead). Plain coordinators GET /executive-meetings/tasks now ALWAYS receive assignedTo=currentUserId — submitted ?mine=0 / ?assigneeId=other are ignored. /me also exposes a new canViewAllTasks flag for the UI. - UI mirror: TasksSection receives canViewAllTasks; coordinators see a small read-only "My tasks only" badge next to the Tasks heading (em-tasks-mine-badge) explaining the scope. New i18n keys executiveMeetings.tasks.{myTasksOnly,myTasksOnlyHelp} (AR + EN). - Seed: scripts/src/seed.ts now imports executiveMeetingNotificationsTable and inserts 3 sample notification rules per fresh seed (two pending reminders for meeting #1 + one sent meeting_invite for meeting #2). Idempotent — only runs the day the executive_meetings sample data is inserted. - Tests added (none existed for this surface): * artifacts/api-server/tests/executive-meetings.test.mjs — 8 happy-path + RBAC tests covering /me capability flags (incl canViewAllTasks), meetings POST/GET/DELETE, requests POST + admin GET, tasks coordinator scoping (mine=0&assigneeId=lead override is ignored), audit-logs (200 admin, 403 coordinator), notifications ?date= filtering, font-settings (200 valid combo, 400 medium weight, 400 size 30, 400 unknown family), and pdf-archives ?date=. All 8 pass. * artifacts/tx-os/tests/executive-meetings-manage-create.spec.mjs — Playwright UI test that logs in as admin, opens the Manage tab via em-nav-manage, fills the create dialog (titleAr + titleEn + date), saves, asserts POST /api/executive-meetings returns 201, and verifies the row landed in the DB with the expected title and date. Validation-round-4 fixes: - Manage attendee editor: added per-row title input alongside name + deterministic ChevronUp/ChevronDown reorder controls (sort order is recomputed on save). New i18n keys executiveMeetings.manage.attendees.{moveUp,moveDown}. - Manage tab: per-row Copy button opens a new "Duplicate to date" dialog that calls POST /executive-meetings/:id/duplicate with a target date picker; on success it switches the day view to the chosen date. New i18n keys executiveMeetings.manage.{duplicate, duplicateToDate, duplicated, duplicateFailed}. - Print page (executive-meetings-print.tsx) now uses react-i18next with new locale block executiveMeetings.print.* (AR + EN parity), forces i18n.changeLanguage(lang) so ?lang=en always prints English. Attendees cell in the print table is now textAlign: center (matches the Step 1 schedule). - Font settings tightened to spec: families = system/Cairo/Tajawal/ Noto Naskh Arabic/Amiri (dropped Inter, monospace), weights = regular/bold (dropped medium, semibold), alignment = start/center (dropped end), size range = 12–22 (was 10–32). Enforced server-side in fontSettingsSchema (Zod) and mirrored in the UI selects/range. Verified: PATCH font-settings returns 200 for valid combo, 400 for fontWeight="medium", 400 for fontSize=30. Validation-round-3 fixes: - Tasks RBAC tightened end-to-end: new TASK_VIEW_ROLES = admin + office_manager + coord_lead + coordinator. GET /tasks and PATCH /tasks/:id now require requireTaskView. /me exposes new canViewTasks flag. Frontend isSectionVisible("tasks") gated on canViewTasks (so CEO/viewer never sees the Tasks tab or hits the endpoint). Verified: admin /me returns canViewTasks=true. - GET /requests gained dateFrom + dateTo (createdAt range) and a requester filter that is honored only for approver/audit roles (non-approvers stay locked to their own requests). Verified curl with ?dateFrom=...&dateTo=... returns 200. - Task dueAt schema accepts BOTH YYYY-MM-DD (from <input type="date">) and full ISO datetime; date-only is normalized to start-of-day UTC, empty string clears the field. Verified curl POST /tasks with {"dueAt":"2026-12-31"} returns 201 with stored dueAt = 2026-12-31T00:00:00.000Z. Validation-round-2 fixes: - GET /executive-meetings/requests: added server-side least-privilege so only APPROVE_ROLES + ADMIN_AUDIT_ROLES see other users' requests; every other executive role is force-scoped to requestedBy = self regardless of the client's `mine` flag (closes the RBAC overexposure flag). - isSectionVisible("tasks") now also returns true for canSubmitRequest, so executive_coordinator sees the Tasks tab (coordinators execute the follow-ups generated from approved requests). - Requests "new request" dropdown now exposes ALL twelve backend request types: create, edit, delete, reschedule, add_attendee, remove_attendee, change_location, cancel_meeting, note, highlight, unhighlight, other. - i18n parity: added en.json + ar.json keys for the five missing request types (add_attendee, remove_attendee, change_location, cancel_meeting, note) and the two missing statuses (needs_edit, done) under executiveMeetings.requests.{type,status}. - Fixed wrong key reference: reschedule form's date row was using manage.field.date (does not exist) — switched to manage.field.meetingDate. Drift / out of scope (explicitly deferred to follow-ups #110/#111/#112): - Real notifications delivery, server-side PDF rendering, attendee reorder UI (drag/arrows), duplicate-to-date UI, OpenAPI/api-spec sync, expanded seed data including executive_meeting_notifications sample rows. Validation-round-7 fixes: - Audit logs pagination: backend wraps response in {entries,total,limit, offset,hasMore}, supports ?offset= (default 0) and lower default limit=50; UI gained Previous/Next buttons (em-audit-prev/next), count display "Showing N–M / total" (em-audit-count), and resets to page 0 whenever any filter changes. - meetingPatchSchema.titleEn switched to .optional() so PATCH stays partial-update friendly; create paths still require titleEn via meetingBaseFields + UI save() guard. - Restored artifacts/tx-os/public/opengraph.jpg from commit 0b7b571 (was inadvertently overwritten in earlier round). - Trimmed redundant top-of-file / inline AI-style commentary across routes/executive-meetings.ts, executive-meetings.tsx, and tests/executive-meetings-manage-create.spec.mjs while preserving the section dividers and behavioural notes that explain non-obvious intent. - i18n: added common.previous / common.next and executiveMeetings.audit.pageInfo in both en.json and ar.json. - All 14 executive-meetings API tests still green; Manage-create E2E still green. Round 7 final cleanup: - Reverted artifacts/tx-os/public/opengraph.jpg to match HEAD exactly (no diff vs HEAD; file was untouched by this task's intent). - Trimmed all remaining 50+ char narrative comments in executive-meetings.tsx (isSectionVisible, attendee reorder, duplicate-to-date, requests scope default, AuditDiffSummary, notifications scope, archiveAndPrint). - Architect re-review: APPROVE.
2026-04-28 08:27:56 +00:00
parent 371c44baba
commit 596b473d06
5 changed files with 69 additions and 86 deletions
@@ -35,8 +35,6 @@ import type { Request, Response, NextFunction } from "express";

 const router: IRouter = Router();

-// Constrain :id to numeric values; otherwise fall through. Required because
-// path-to-regexp 8 (Express 5) no longer supports inline regex like ":id(\\d+)".
 router.param("id", (req, res, next, value) => {
  if (!/^\d+$/.test(String(value))) {
    next("route");
@@ -70,8 +68,6 @@ const REQUEST_REVIEW_STATUSES = [
  "needs_edit",
 ] as const;
 const TASK_STATUSES = ["pending", "in_progress", "completed", "cancelled"] as const;
-// Font settings constraints per Phase 2 spec: limited family list, regular/bold
-// only, start/center alignment only, size 12–22.
 const FONT_FAMILIES = [
  "system",
  "Cairo",
@@ -102,19 +98,12 @@ const ADMIN_AUDIT_ROLES = [
  "executive_office_manager",
  "executive_coord_lead",
 ] as const;
-// Tasks are an internal coordination workflow; CEO and viewer roles must NOT
-// see the Tasks tab or hit GET /tasks. Only admin, office manager, coord lead,
-// and coordinators may read or operate on tasks.
 const TASK_VIEW_ROLES = [
  "admin",
  "executive_office_manager",
  "executive_coord_lead",
  "executive_coordinator",
 ] as const;
-// Within TASK_VIEW_ROLES, only these "lead" roles get a broad cross-user view.
-// Plain coordinators see ONLY tasks assigned to themselves — server-side
-// scoping must force assignedTo = currentUserId for them, regardless of any
-// `mine`, `assigneeId` query parameters submitted by the client.
 const TASK_BROAD_VIEW_ROLES = [
  "admin",
  "executive_office_manager",
@@ -213,7 +202,7 @@ const meetingCreateSchema = z
 const meetingPatchSchema = z
  .object({
    titleAr: meetingBaseFields.titleAr.optional(),
-    titleEn: meetingBaseFields.titleEn,
+    titleEn: meetingBaseFields.titleEn.optional(),
    meetingDate: meetingBaseFields.meetingDate.optional(),
    dailyNumber: meetingBaseFields.dailyNumber,
    startTime: meetingBaseFields.startTime,
@@ -239,8 +228,6 @@ const duplicateSchema = z.object({
  targetDate: dateSchema,
 });

-// requestDetails differs by requestType. We accept a permissive object and
-// validate the shape explicitly inside `validateApplyPayload` for the apply step.
 const requestDetailsSchema = z.record(z.string(), z.unknown()).default({});

 const requestCreateSchema = z.object({
@@ -254,8 +241,6 @@ const requestNestedCreateSchema = z.object({
  requestDetails: requestDetailsSchema,
 });

-// PATCH approvals are status-driven (per spec). `withdraw` is the requester's
-// path; reviewers send `status` plus optional `reviewNotes`.
 const requestReviewSchema = z.union([
  z.object({
    action: z.literal("withdraw"),
@@ -266,9 +251,6 @@ const requestReviewSchema = z.union([
  }),
 ]);

-// dueAt accepts either a YYYY-MM-DD date (from <input type="date">) OR a full
-// ISO datetime. Date-only is normalized to start-of-day UTC. Empty string is
-// treated as null so PATCH can clear a due date.
 const dueAtSchema = z
  .union([
    z.string().regex(/^\d{4}-\d{2}-\d{2}$/),
@@ -313,7 +295,6 @@ const fontSettingsSchema = z.object({
  alignment: z.enum(FONT_ALIGNMENTS).default("start"),
 });

-// Helper: parse a body with a Zod schema and write a standard 400 if invalid.
 function parseBody<T>(
  res: Response,
  schema: ZodType<T>,
@@ -331,7 +312,6 @@ function parseBody<T>(
  return r.data;
 }

-// ---------- Audit log helper (typed, no `as never`) ----------

 type DbExecutor = typeof db | Parameters<Parameters<typeof db.transaction>[0]>[0];
 type AuditInsert = typeof executiveMeetingAuditLogsTable.$inferInsert;
@@ -1014,9 +994,6 @@ router.get(
  },
 );

-// Top-level POST /requests handles "create" (no target meeting yet) and any
-// request that doesn't have a numeric meeting id in the URL. Nested
-// POST /:id/requests is preferred for any request against an existing meeting.
 router.post(
  "/executive-meetings/requests",
  requireExecutiveAccess,
@@ -1107,8 +1084,6 @@ router.post(
  },
 );

-// PATCH approvals are status-driven for the office manager. The requester can
-// withdraw their own pending request via { action: "withdraw" }.
 router.patch(
  "/executive-meetings/requests/:id",
  requireExecutiveAccess,
@@ -1539,7 +1514,10 @@ router.get(
    const limit =
      Number.isFinite(limitRaw) && limitRaw > 0
        ? Math.min(Math.floor(limitRaw), 200)
-        : 100;
+        : 50;
+    const offsetRaw = Number(req.query.offset);
+    const offset =
+      Number.isFinite(offsetRaw) && offsetRaw > 0 ? Math.floor(offsetRaw) : 0;
    const conds: SQL[] = [];
    if (dateRaw && DATE_RE_LOCAL.test(dateRaw)) {
      const start = new Date(`${dateRaw}T00:00:00.000Z`);
@@ -1564,6 +1542,11 @@ router.get(
    if (action) conds.push(eq(executiveMeetingAuditLogsTable.action, action));
    if (actorId) conds.push(eq(executiveMeetingAuditLogsTable.performedBy, actorId));
    const where = conds.length > 0 ? and(...conds) : undefined;
+    const [{ count: totalRaw } = { count: 0 }] = await db
+      .select({ count: sql<number>`count(*)::int` })
+      .from(executiveMeetingAuditLogsTable)
+      .where(where);
+    const total = Number(totalRaw) || 0;
    const rows = await db
      .select({
        l: executiveMeetingAuditLogsTable,
@@ -1575,7 +1558,8 @@ router.get(
      .leftJoin(usersTable, eq(usersTable.id, executiveMeetingAuditLogsTable.performedBy))
      .where(where)
      .orderBy(desc(executiveMeetingAuditLogsTable.performedAt))
-      .limit(limit);
+      .limit(limit)
+      .offset(offset);
    res.json({
      entries: rows.map((row) => ({
        ...row.l,
@@ -1587,6 +1571,10 @@ router.get(
            }
          : null,
      })),
+      total,
+      limit,
+      offset,
+      hasMore: offset + rows.length < total,
    });
  },
 );