From b1b77395d05c3a6ec688e0ff9656f0d7ae3bdbde Mon Sep 17 00:00:00 2001 From: riyadhafraa <49757212-riyadhafraa@users.noreply.replit.com> Date: Mon, 11 May 2026 10:55:34 +0000 Subject: [PATCH] #486 Executive Meetings: row-click quick actions popover (Move up / Move down / Postpone) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking any meeting row on the schedule now opens a small popover with three quick actions: Move up, Move down, and Postpone. Gated only on canMutate (NOT editMode) per spec. Backend - New POST /executive-meetings/swap-times endpoint (artifacts/api-server/src/ routes/executive-meetings.ts). FOR UPDATE locks both rows by ascending id to avoid deadlocks, optimistic-lock check via expectedUpdatedAt{A,B} (returns 409 stale_meeting + conflict.lastActor — same shape PostponeDialog understands), guards different_dates and no_time_window, swaps only (startTime, endTime), audits each row as `meeting_swap_times`, calls renumberDayByStartTime so the # column matches the new chronological order, and broadcasts emitExecutiveMeetingsDayChanged. - New zod body schema ExecutiveMeetingsSwapTimesBody in lib/api-zod/src/manual.ts. Frontend - New shared lib/api-json.ts (ApiError + apiJson) extracted from upcoming- meeting-alert.tsx so the page can reuse the same fetch/error contract. - upcoming-meeting-alert.tsx exports PostponeDialog so the page can mount it for quick-action postpone without duplicating UI. - artifacts/tx-os/src/pages/executive-meetings.tsx: Schedule wires swapTimes/quickMoveUp/quickMoveDown/postpone state, computes per-day neighbours via meetingNumbersById, and mounts a single page-level PostponeDialog. MeetingRow wraps `` in a Popover/PopoverAnchor; row onClick opens the popover with skip rules for buttons/inputs/contenteditable and ARIA roles (button/checkbox/switch/combobox/dialog) plus testid prefixes (em-row-grip/-actions/-select, em-edit-*, em-merge-edit-*, em-time-*) so the time cell's inline editor and other affordances don't collide. - en/ar locales gain executiveMeetings.quickActions.{label,moveUp,moveDown,postpone}. Tests - artifacts/api-server/tests/executive-meetings-swap-times.test.mjs (new): happy path, 409 stale_meeting (with conflict actor), 400 different_dates, 400 no_time_window. Each scenario uses a distinct far-future date to avoid daily_number races with seeded data. - artifacts/tx-os/tests/executive-meetings-row-quick-actions.spec.mjs (new): drives the date input, verifies row click → popover, Move up swap reflected in DB, and Postpone item opens the dialog. Code review (architect) flagged an edit-mode conflict where the time cell (role=button div) would also bubble to the row handler; fixed by adding ARIA role and em-time-* skip rules. Architect also flagged keyboard-trigger gap and edit-mode test gaps — proposed as follow-ups. Other test failures in the repo (executive-meetings reorder, font-settings, notes-share, service-orders) pre-date this change and are unrelated. --- .../src/routes/executive-meetings.ts | 203 ++++++++++- .../executive-meetings-swap-times.test.mjs | 337 ++++++++++++++++++ .../upcoming-meeting-alert.tsx | 49 +-- artifacts/tx-os/src/lib/api-json.ts | 40 +++ artifacts/tx-os/src/locales/ar.json | 6 + artifacts/tx-os/src/locales/en.json | 6 + .../tx-os/src/pages/executive-meetings.tsx | 259 +++++++++++++- ...cutive-meetings-row-quick-actions.spec.mjs | 180 ++++++++++ lib/api-zod/src/manual.ts | 16 + 9 files changed, 1051 insertions(+), 45 deletions(-) create mode 100644 artifacts/api-server/tests/executive-meetings-swap-times.test.mjs create mode 100644 artifacts/tx-os/src/lib/api-json.ts create mode 100644 artifacts/tx-os/tests/executive-meetings-row-quick-actions.spec.mjs diff --git a/artifacts/api-server/src/routes/executive-meetings.ts b/artifacts/api-server/src/routes/executive-meetings.ts index 31cd248f..4ff384f2 100644 --- a/artifacts/api-server/src/routes/executive-meetings.ts +++ b/artifacts/api-server/src/routes/executive-meetings.ts @@ -55,7 +55,10 @@ import { getUserDisplay as getUserDisplayForNotify, EXECUTIVE_MEETING_NOTIFICATION_TYPES, } from "../lib/executive-meeting-notify"; -import { ExecutiveMeetingsReorderBody } from "@workspace/api-zod"; +import { + ExecutiveMeetingsReorderBody, + ExecutiveMeetingsSwapTimesBody, +} from "@workspace/api-zod"; import type { Request, Response, NextFunction } from "express"; const router: IRouter = Router(); @@ -2128,6 +2131,204 @@ router.post( }, ); +// ===================================================================== +// SWAP TIMES (#486 — schedule row quick-actions Move up / Move down) +// ===================================================================== +// +// Swaps the (startTime, endTime) tuple between two meetings on the +// same date so the schedule's Time column stays visually anchored +// (each row keeps its slot; the meetings rotate through the slots). +// Daily numbering is recomputed via renumberDayByStartTime so the +// `#` column matches the new chronological order. Both rows are +// FOR UPDATE-locked inside one transaction in id-order to avoid +// deadlocks when two browsers race on the same pair, and each row +// carries an optimistic-lock token (expectedUpdatedAt{A,B}) so a +// stale click returns 409 stale_meeting + a `conflict` payload that +// names the offending row + last actor — same shape the +// PostponeDialog already understands. +router.post( + "/executive-meetings/swap-times", + requireExecutiveAccess, + requireMutate, + async (req, res): Promise => { + const data = parseBody(res, ExecutiveMeetingsSwapTimesBody, req.body); + if (!data) return; + if (data.aId === data.bId) { + res + .status(400) + .json({ error: "aId and bId must differ", code: "same_id" }); + return; + } + const userId = req.session.userId!; + type StaleConflict = { + ok: false; + status: 409; + error: string; + code: "stale_meeting"; + conflict: { + id: number; + currentStartTime: string | null; + currentEndTime: string | null; + currentStatus: string; + lastModifiedAt: string; + lastActor: { + id: number | null; + username: string | null; + displayNameAr: string | null; + displayNameEn: string | null; + } | null; + }; + }; + type TxResult = + | { ok: true; meetingDate: string } + | { ok: false; status: number; error: string; code: string } + | StaleConflict; + const result = await db.transaction(async (tx): Promise => { + // Always lock by ascending id to prevent deadlocks when two + // concurrent swap-times calls touch the same pair from opposite + // sides (A↔B vs B↔A). + const ids = [data.aId, data.bId].slice().sort((x, y) => x - y); + const lockedRows = await tx + .select() + .from(executiveMeetingsTable) + .where(inArray(executiveMeetingsTable.id, ids)) + .for("update"); + const aRow = lockedRows.find((r) => r.id === data.aId); + const bRow = lockedRows.find((r) => r.id === data.bId); + if (!aRow || !bRow) { + return { + ok: false, + status: 404, + error: "Meeting not found", + code: "not_found", + }; + } + if (aRow.meetingDate !== bRow.meetingDate) { + return { + ok: false, + status: 400, + error: "Meetings are on different dates", + code: "different_dates", + }; + } + if ( + aRow.startTime == null || + aRow.endTime == null || + bRow.startTime == null || + bRow.endTime == null + ) { + return { + ok: false, + status: 400, + error: "Both meetings must have a scheduled time window to swap", + code: "no_time_window", + }; + } + // Optimistic-lock check on each row. Same shape the postpone + // and reschedule endpoints use (#283) so the popover can render + // the same "X just changed this meeting" prompt. + for (const [row, expected] of [ + [aRow, data.expectedUpdatedAtA] as const, + [bRow, data.expectedUpdatedAtB] as const, + ]) { + const observedIso = new Date(expected).toISOString(); + const currentIso = row.updatedAt.toISOString(); + if (observedIso !== currentIso) { + let lastActor: StaleConflict["conflict"]["lastActor"] = null; + if (row.updatedBy != null) { + const [u] = await tx + .select({ + id: usersTable.id, + username: usersTable.username, + displayNameAr: usersTable.displayNameAr, + displayNameEn: usersTable.displayNameEn, + }) + .from(usersTable) + .where(eq(usersTable.id, row.updatedBy)); + lastActor = u ?? null; + } + return { + ok: false, + status: 409, + error: "Meeting was modified by someone else", + code: "stale_meeting", + conflict: { + id: row.id, + currentStartTime: row.startTime, + currentEndTime: row.endTime, + currentStatus: row.status, + lastModifiedAt: currentIso, + lastActor, + }, + }; + } + } + // Snapshot the original windows so the swap is symmetric even + // if Drizzle's `set` evaluates lazily. + const aStart = aRow.startTime; + const aEnd = aRow.endTime; + const bStart = bRow.startTime; + const bEnd = bRow.endTime; + await tx + .update(executiveMeetingsTable) + .set({ startTime: bStart, endTime: bEnd, updatedBy: userId }) + .where(eq(executiveMeetingsTable.id, aRow.id)); + await tx + .update(executiveMeetingsTable) + .set({ startTime: aStart, endTime: aEnd, updatedBy: userId }) + .where(eq(executiveMeetingsTable.id, bRow.id)); + await logAudit(tx, { + action: "meeting_swap_times", + entityType: "meeting", + entityId: aRow.id, + oldValue: { startTime: aStart, endTime: aEnd }, + newValue: { + startTime: bStart, + endTime: bEnd, + swappedWith: bRow.id, + }, + performedBy: userId, + }); + await logAudit(tx, { + action: "meeting_swap_times", + entityType: "meeting", + entityId: bRow.id, + oldValue: { startTime: bStart, endTime: bEnd }, + newValue: { + startTime: aStart, + endTime: aEnd, + swappedWith: aRow.id, + }, + performedBy: userId, + }); + // Re-sort daily_number by start time so the schedule's `#` + // column matches the new chronological order. + await renumberDayByStartTime(tx, aRow.meetingDate); + return { ok: true, meetingDate: aRow.meetingDate }; + }); + if (!result.ok) { + const payload: Record = { + error: result.error, + code: result.code, + }; + if (result.code === "stale_meeting" && "conflict" in result) { + payload.conflict = result.conflict; + } + res.status(result.status).json(payload); + return; + } + void emitExecutiveMeetingsDayChanged(result.meetingDate); + const [updatedA, updatedB] = await Promise.all([ + fetchMeetingWithAttendees(data.aId), + fetchMeetingWithAttendees(data.bId), + ]); + res.json({ + ok: true, + meetings: [updatedA, updatedB].filter((m) => m != null), + }); + }, +); + // ===================================================================== // REORDER (within a single day) // ===================================================================== diff --git a/artifacts/api-server/tests/executive-meetings-swap-times.test.mjs b/artifacts/api-server/tests/executive-meetings-swap-times.test.mjs new file mode 100644 index 00000000..1894a15a --- /dev/null +++ b/artifacts/api-server/tests/executive-meetings-swap-times.test.mjs @@ -0,0 +1,337 @@ +// #486: Backend test for POST /executive-meetings/swap-times. +// +// The schedule row quick-actions popover (Move up / Move down) calls +// this endpoint to swap just the (startTime, endTime) tuple between +// the clicked meeting and its chronological neighbour. The Time +// column stays visually anchored — each row keeps its slot and the +// meetings rotate through the slots — and daily numbers are +// recomputed by start time. +// +// Cases: +// 1. Happy path: A and B on same date with valid updatedAt tokens +// → 200, times swapped, dailyNumber re-sorted, both audit rows +// written. +// 2. Stale token on either row → 409 stale_meeting + conflict +// payload that names the offending row + last actor. +// 3. Different dates → 400 different_dates. +// 4. Missing time window → 400 no_time_window. +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_HASH = + "$2b$10$Bs636ukPMyz01nKrsi.5m.JlDXSN22AVCvn8cgPWWDbo5yJRQX2vu"; +const TEST_PASSWORD = "TestPass123!"; +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, displayNameEn) { + 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, $4, 'en', true) RETURNING id`, + [username, `${username}@example.com`, TEST_PASSWORD_HASH, displayNameEn], + ); + 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 userA = null; +let cookieA = null; +let userB = null; +let cookieB = null; + +before(async () => { + userA = await createUser("em_swap_a", "executive_coord_lead", "Alice Swap"); + cookieA = await login(userA.username, TEST_PASSWORD); + userB = await createUser("em_swap_b", "executive_coord_lead", "Bob Swap"); + cookieB = await login(userB.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_id = ANY($1::int[]) AND entity_type = 'meeting'`, + [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 executive_meeting_notification_prefs WHERE user_id = ANY($1::int[])`, + [created.userIds], + ); + 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(); +}); + +// Use distinct far-future dates per test so each scenario gets a +// clean day — avoids any interference with seeded recurring meetings, +// other suites running in the same DB, and the `executive_meetings_ +// date_number_unique` constraint that nextDailyNumber races against. +function futureDate(offsetDays) { + return new Date(Date.now() + offsetDays * 86_400_000) + .toISOString() + .slice(0, 10); +} +const DATE_HAPPY = futureDate(30); +const DATE_STALE = futureDate(31); +const DATE_DIFF_A = futureDate(32); +const DATE_DIFF_B = futureDate(33); +const DATE_NO_WINDOW = futureDate(34); + +async function createMeeting({ date, titleEn, startTime, endTime }) { + const res = await api(cookieA, "POST", "/api/executive-meetings", { + titleAr: titleEn, + titleEn, + meetingDate: date, + startTime, + endTime, + status: "scheduled", + }); + if (res.status !== 200 && res.status !== 201) { + const body = await res.text(); + assert.fail( + `meeting create should succeed (got ${res.status}): ${body.slice(0, 400)}`, + ); + } + const body = await res.json(); + const meeting = body.meeting ?? body; + created.meetingIds.push(meeting.id); + return meeting; +} + +async function fetchMeeting(cookie, id) { + const res = await api(cookie, "GET", `/api/executive-meetings/${id}`); + assert.equal(res.status, 200, "meeting fetch should succeed"); + return res.json(); +} + +test("swap-times: happy path swaps (startTime, endTime) and re-sorts daily numbers", async () => { + const a = await createMeeting({ + date: DATE_HAPPY, + titleEn: "Swap A", + startTime: "09:00", + endTime: "09:30", + }); + const b = await createMeeting({ + date: DATE_HAPPY, + titleEn: "Swap B", + startTime: "10:00", + endTime: "10:30", + }); + const aFresh = await fetchMeeting(cookieA, a.id); + const bFresh = await fetchMeeting(cookieA, b.id); + const beforeNumA = aFresh.dailyNumber; + const beforeNumB = bFresh.dailyNumber; + assert.notEqual( + beforeNumA, + beforeNumB, + "two meetings on the same day should have distinct daily numbers", + ); + + const res = await api(cookieA, "POST", "/api/executive-meetings/swap-times", { + aId: a.id, + bId: b.id, + expectedUpdatedAtA: aFresh.updatedAt, + expectedUpdatedAtB: bFresh.updatedAt, + }); + assert.equal(res.status, 200, "swap should succeed"); + const body = await res.json(); + assert.equal(body.ok, true); + assert.equal(body.meetings.length, 2); + + const aAfter = await fetchMeeting(cookieA, a.id); + const bAfter = await fetchMeeting(cookieA, b.id); + // Times swapped — A now holds B's old window and vice versa. + assert.equal(aAfter.startTime.slice(0, 5), "10:00"); + assert.equal(aAfter.endTime.slice(0, 5), "10:30"); + assert.equal(bAfter.startTime.slice(0, 5), "09:00"); + assert.equal(bAfter.endTime.slice(0, 5), "09:30"); + // Daily numbers re-sorted by start time, so A and B exchanged + // positions in the `#` column too. + assert.equal(aAfter.dailyNumber, beforeNumB); + assert.equal(bAfter.dailyNumber, beforeNumA); + + // Both rows audited. + const { rows } = await pool.query( + `SELECT entity_id FROM executive_meeting_audit_logs + WHERE entity_type = 'meeting' + AND action = 'meeting_swap_times' + AND entity_id = ANY($1::int[])`, + [[a.id, b.id]], + ); + const auditedIds = rows.map((r) => r.entity_id).sort(); + assert.deepEqual(auditedIds, [a.id, b.id].sort()); +}); + +test("swap-times: stale updatedAt returns 409 stale_meeting with conflict payload", async () => { + const a = await createMeeting({ + date: DATE_STALE, + titleEn: "Stale A", + startTime: "11:00", + endTime: "11:30", + }); + const b = await createMeeting({ + date: DATE_STALE, + titleEn: "Stale B", + startTime: "12:00", + endTime: "12:30", + }); + const aFresh = await fetchMeeting(cookieA, a.id); + const bFresh = await fetchMeeting(cookieA, b.id); + // Bob mutates B in between A's read and A's swap, invalidating the + // updatedAt token Alice captured. + const patch = await api(cookieB, "PATCH", `/api/executive-meetings/${b.id}`, { + titleEn: "Stale B (renamed)", + }); + assert.ok( + patch.status === 200 || patch.status === 204, + `patch should succeed (got ${patch.status})`, + ); + const swap = await api( + cookieA, + "POST", + "/api/executive-meetings/swap-times", + { + aId: a.id, + bId: b.id, + expectedUpdatedAtA: aFresh.updatedAt, + expectedUpdatedAtB: bFresh.updatedAt, + }, + ); + assert.equal(swap.status, 409); + const body = await swap.json(); + assert.equal(body.code, "stale_meeting"); + assert.ok(body.conflict, "stale response must include conflict payload"); + assert.equal(body.conflict.id, b.id); + assert.ok(body.conflict.lastActor, "conflict must name an actor"); + assert.equal(body.conflict.lastActor.id, userB.id); +}); + +test("swap-times: different dates → 400 different_dates", async () => { + const a = await createMeeting({ + date: DATE_DIFF_A, + titleEn: "Date A", + startTime: "13:00", + endTime: "13:30", + }); + const b = await createMeeting({ + date: DATE_DIFF_B, + titleEn: "Date B", + startTime: "13:00", + endTime: "13:30", + }); + const aFresh = await fetchMeeting(cookieA, a.id); + const bFresh = await fetchMeeting(cookieA, b.id); + const res = await api(cookieA, "POST", "/api/executive-meetings/swap-times", { + aId: a.id, + bId: b.id, + expectedUpdatedAtA: aFresh.updatedAt, + expectedUpdatedAtB: bFresh.updatedAt, + }); + assert.equal(res.status, 400); + const body = await res.json(); + assert.equal(body.code, "different_dates"); +}); + +test("swap-times: meeting without a time window → 400 no_time_window", async () => { + // Create A normally, then null out B's start/end directly so we can + // exercise the guard without going through PATCH (which validates). + const a = await createMeeting({ + date: DATE_NO_WINDOW, + titleEn: "Window A", + startTime: "15:00", + endTime: "15:30", + }); + const b = await createMeeting({ + date: DATE_NO_WINDOW, + titleEn: "Window B", + startTime: "16:00", + endTime: "16:30", + }); + await pool.query( + `UPDATE executive_meetings SET start_time = NULL, end_time = NULL WHERE id = $1`, + [b.id], + ); + const aFresh = await fetchMeeting(cookieA, a.id); + const bFresh = await fetchMeeting(cookieA, b.id); + const res = await api(cookieA, "POST", "/api/executive-meetings/swap-times", { + aId: a.id, + bId: b.id, + expectedUpdatedAtA: aFresh.updatedAt, + expectedUpdatedAtB: bFresh.updatedAt, + }); + assert.equal(res.status, 400); + const body = await res.json(); + assert.equal(body.code, "no_time_window"); +}); diff --git a/artifacts/tx-os/src/components/executive-meetings/upcoming-meeting-alert.tsx b/artifacts/tx-os/src/components/executive-meetings/upcoming-meeting-alert.tsx index c42559a6..f4c0d22d 100644 --- a/artifacts/tx-os/src/components/executive-meetings/upcoming-meeting-alert.tsx +++ b/artifacts/tx-os/src/components/executive-meetings/upcoming-meeting-alert.tsx @@ -42,6 +42,11 @@ import { import { hexToRgba, useAlertPrefs } from "@/lib/upcoming-alert-prefs"; import { formatTime as formatTimeLocale } from "@/lib/i18n-format"; import { notificationPlayer } from "@/lib/notification-sounds"; +// #486: ApiError + apiJson moved to a shared module so the new +// schedule row quick-actions popover can reuse the PostponeDialog +// (exported below) and inspect the same structured 409 stale_meeting +// payload without duplicating the fetch wrapper. +import { ApiError, apiJson } from "@/lib/api-json"; const POSITION_KEY = "txos.upcomingMeetingAlert.position"; const ALERT_LEAD_MINUTES = 5; @@ -123,40 +128,9 @@ function formatTime(t: string | null, lang: string): string { }); } -// #283: Custom error so callers can inspect the structured response -// body — specifically the `code` and the `conflict` payload returned -// for 409 stale_meeting — instead of only the human-readable message. -class ApiError extends Error { - status: number; - code: string | undefined; - body: Record; - constructor(message: string, status: number, body: Record) { - super(message); - this.name = "ApiError"; - this.status = status; - this.body = body; - const code = (body as { code?: unknown }).code; - this.code = typeof code === "string" ? code : undefined; - } -} - -async function apiJson(input: string, init?: RequestInit): Promise { - const res = await fetch(input, { - credentials: "include", - headers: { "Content-Type": "application/json", ...(init?.headers ?? {}) }, - ...init, - }); - if (res.status === 204) return undefined as T; - const text = await res.text(); - const data = text ? (JSON.parse(text) as unknown) : ({} as unknown); - if (!res.ok) { - const body = (data ?? {}) as Record; - const message = - (body.error as string | undefined) || `HTTP ${res.status}`; - throw new ApiError(message, res.status, body); - } - return data as T; -} +// #283/#486: ApiError + apiJson are imported above from +// `@/lib/api-json` so this file and the schedule row quick-actions +// popover share one fetch wrapper and one error class. type DragPosition = { x: number; y: number }; function loadPosition(): DragPosition | null { @@ -1058,6 +1032,13 @@ type PostponeDialogProps = { type PostponeTab = "minutes" | "reschedule" | "cancel"; +// #486: Exported so the Executive Meetings schedule page can mount the +// same dialog from its row quick-actions popover. The Meeting type +// (also exported as PostponeDialogMeeting) is the structural minimum +// the dialog reads from — any caller passing a row with these fields +// satisfies it. +export type PostponeDialogMeeting = Meeting; +export { PostponeDialog }; function PostponeDialog({ open, onOpenChange, diff --git a/artifacts/tx-os/src/lib/api-json.ts b/artifacts/tx-os/src/lib/api-json.ts new file mode 100644 index 00000000..eca1a2c6 --- /dev/null +++ b/artifacts/tx-os/src/lib/api-json.ts @@ -0,0 +1,40 @@ +// #486: Shared apiJson + ApiError so the upcoming-meeting-alert +// PostponeDialog and the new schedule row quick-actions popover both +// inspect the same structured 409 stale_meeting payload (used by the +// postpone-minutes and swap-times endpoints) without duplicating the +// fetch wrapper. + +export class ApiError extends Error { + status: number; + code: string | undefined; + body: Record; + constructor(message: string, status: number, body: Record) { + super(message); + this.name = "ApiError"; + this.status = status; + this.body = body; + const code = (body as { code?: unknown }).code; + this.code = typeof code === "string" ? code : undefined; + } +} + +export async function apiJson( + input: string, + init?: RequestInit, +): Promise { + const res = await fetch(input, { + credentials: "include", + headers: { "Content-Type": "application/json", ...(init?.headers ?? {}) }, + ...init, + }); + if (res.status === 204) return undefined as T; + const text = await res.text(); + const data = text ? (JSON.parse(text) as unknown) : ({} as unknown); + if (!res.ok) { + const body = (data ?? {}) as Record; + const message = + (body.error as string | undefined) || `HTTP ${res.status}`; + throw new ApiError(message, res.status, body); + } + return data as T; +} diff --git a/artifacts/tx-os/src/locales/ar.json b/artifacts/tx-os/src/locales/ar.json index 09481b3a..88aaebb2 100644 --- a/artifacts/tx-os/src/locales/ar.json +++ b/artifacts/tx-os/src/locales/ar.json @@ -1427,6 +1427,12 @@ "label": "إجراءات الصف", "back": "رجوع" }, + "quickActions": { + "label": "إجراءات سريعة", + "moveUp": "نقل لأعلى", + "moveDown": "نقل لأسفل", + "postpone": "تأجيل" + }, "merge": { "label": "دمج الخلايا", "editLabel": "تعديل الخلية المدموجة", diff --git a/artifacts/tx-os/src/locales/en.json b/artifacts/tx-os/src/locales/en.json index 3517d74c..b1af1c69 100644 --- a/artifacts/tx-os/src/locales/en.json +++ b/artifacts/tx-os/src/locales/en.json @@ -1141,6 +1141,12 @@ "label": "Row actions", "back": "Back" }, + "quickActions": { + "label": "Quick actions", + "moveUp": "Move up", + "moveDown": "Move down", + "postpone": "Postpone" + }, "merge": { "label": "Merge cells", "editLabel": "Edit merged cell", diff --git a/artifacts/tx-os/src/pages/executive-meetings.tsx b/artifacts/tx-os/src/pages/executive-meetings.tsx index 5a00ed3f..e6eda9ad 100644 --- a/artifacts/tx-os/src/pages/executive-meetings.tsx +++ b/artifacts/tx-os/src/pages/executive-meetings.tsx @@ -80,6 +80,7 @@ import { import { Switch } from "@/components/ui/switch"; import { Popover, + PopoverAnchor, PopoverContent, PopoverTrigger, } from "@/components/ui/popover"; @@ -109,6 +110,9 @@ import { import { CSS } from "@dnd-kit/utilities"; import { GripVertical } from "lucide-react"; import { EditableCell } from "@/components/editable-cell"; +// #486: reuse the same dialog the upcoming-alert pops, so postponing +// from the schedule row quick-actions menu shares one implementation. +import { PostponeDialog } from "@/components/executive-meetings/upcoming-meeting-alert"; import { safeHtml, htmlToPlainText, wrapAsParagraph } from "@/lib/safe-html"; import { ALERT_COLOR_PRESETS, @@ -171,6 +175,10 @@ type Meeting = { // is the same for every viewer. NULL/missing = "default" (no tint). // Allowed values come from ROW_COLOR_OPTIONS below. rowColor?: string | null; + // #486: optimistic-lock token used by /executive-meetings/swap-times + // (the schedule row quick-actions Move up / Move down popover) and + // by the existing PostponeDialog when reused from the schedule. + updatedAt: string; }; type DayResponse = { date: string; meetings: Meeting[] }; @@ -2147,6 +2155,82 @@ function ScheduleSection({ [orderedMeetings, date, refreshDay, toast, t], ); + // #486: Quick-actions popover (Move up / Move down / Postpone) on + // every schedule row, gated only on the raw `canMutate` permission + // (NOT effectiveCanMutate / editMode) so users can reorder + postpone + // without flipping into edit mode. Move up / Move down swap just the + // (startTime, endTime) tuple between the clicked meeting and its + // chronological neighbour on the same date — see swap-times route. + // The Time column therefore stays visually anchored to its row; + // each row keeps its slot and the meetings rotate through the slots. + const swapTimes = useCallback( + async (a: Meeting, b: Meeting) => { + if (reorderingRef.current) return; + reorderingRef.current = true; + setReordering(true); + try { + await apiJson(`/api/executive-meetings/swap-times`, { + method: "POST", + body: { + aId: a.id, + bId: b.id, + expectedUpdatedAtA: a.updatedAt, + expectedUpdatedAtB: b.updatedAt, + }, + }); + refreshDay(); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + toast({ + title: t("common.error"), + description: msg, + variant: "destructive", + }); + } finally { + reorderingRef.current = false; + setReordering(false); + } + }, + [refreshDay, toast, t], + ); + + const quickMoveUp = useCallback( + (meetingId: number) => { + const idx = orderedMeetings.findIndex((m) => m.id === meetingId); + if (idx <= 0) return; + const a = orderedMeetings[idx]!; + const b = orderedMeetings[idx - 1]!; + void swapTimes(a, b); + }, + [orderedMeetings, swapTimes], + ); + const quickMoveDown = useCallback( + (meetingId: number) => { + const idx = orderedMeetings.findIndex((m) => m.id === meetingId); + if (idx < 0 || idx >= orderedMeetings.length - 1) return; + const a = orderedMeetings[idx]!; + const b = orderedMeetings[idx + 1]!; + void swapTimes(a, b); + }, + [orderedMeetings, swapTimes], + ); + + const [postponeMeetingId, setPostponeMeetingId] = useState( + null, + ); + const postponeMeeting = useMemo(() => { + if (postponeMeetingId == null) return null; + return meetings.find((m) => m.id === postponeMeetingId) ?? null; + }, [meetings, postponeMeetingId]); + // PostponeDialog renders cascade-prompt rows by daily number, so it + // needs the same id→# map the upcoming-alert builds. Mirror that + // logic from `meetings` directly (#486 reuse). + const meetingNumbersById = useMemo(() => { + const m = new Map(); + for (const x of meetings) m.set(x.id, x.dailyNumber); + return m; + }, [meetings]); + // #265: columns persistence is owned by ExecutiveMeetingsPage now that // both Schedule and Settings tabs can mutate the array. Keeping the // write effect here would silently drop edits made from the Settings @@ -2820,6 +2904,12 @@ function ScheduleSection({ bulkSelectable={effectiveCanMutate} bulkSelected={selectedMeetingIds.has(m.id)} onBulkSelectChange={(v) => toggleMeetingSelected(m.id, v)} + quickActionsCanMutate={canMutate} + canQuickMoveUp={idx > 0} + canQuickMoveDown={idx < displayedMeetings.length - 1} + onQuickMoveUp={() => quickMoveUp(m.id)} + onQuickMoveDown={() => quickMoveDown(m.id)} + onQuickPostpone={() => setPostponeMeetingId(m.id)} /> ))} @@ -2855,6 +2945,28 @@ function ScheduleSection({ + {/* #486: Page-level PostponeDialog mount, opened by clicking the + Postpone item in any row's quick-actions popover. Reuses the + same component the upcoming-meeting alert renders so the + postpone-minutes / reschedule / cancel flows + the 409 + stale_meeting handling stay in one place. */} + {postponeMeeting && canMutate ? ( + { + if (!o) setPostponeMeetingId(null); + }} + meeting={postponeMeeting} + isRtl={isRtl} + meetingNumbersById={meetingNumbersById} + accent={highlightPrefs.color} + onSaved={async () => { + setPostponeMeetingId(null); + refreshDay(); + }} + /> + ) : null} + {/* #330: Bulk-duplicate-to-date dialog. Mirrors the per-row duplicate dialog in ManageSection (single date picker, same POST endpoint per id) but operates on the visible-selected @@ -3590,6 +3702,12 @@ function MeetingRow({ bulkSelected, onBulkSelectChange, displayNumber, + quickActionsCanMutate, + canQuickMoveUp, + canQuickMoveDown, + onQuickMoveUp, + onQuickMoveDown, + onQuickPostpone, }: { meeting: Meeting; displayNumber?: number; @@ -3599,6 +3717,16 @@ function MeetingRow({ rowColorKey: string; onPickRowColor: (key: string) => void; canMutate: boolean; + // #486: raw (un-gated by editMode) mutate permission — clicking a + // row anywhere outside an interactive control opens the quick- + // actions popover. The popover itself respects the same MUTATE_ROLES + // server-side gating as the swap-times + postpone-minutes endpoints. + quickActionsCanMutate?: boolean; + canQuickMoveUp?: boolean; + canQuickMoveDown?: boolean; + onQuickMoveUp?: () => void; + onQuickMoveDown?: () => void; + onQuickPostpone?: () => void; onSaveTitle: (html: string) => Promise; onSaveAttendeeName: (idx: number, html: string) => Promise; onSaveTimes: ( @@ -4045,17 +4173,128 @@ function MeetingRow({ boxShadow: composedShadow || undefined, }; + // #486: Quick-actions popover. Clicking anywhere on the row that is + // NOT an interactive control (button, link, input, contenteditable + // cell, drag handle, row-actions menu, edit affordance, bulk-select + // checkbox, etc.) opens the popover. Anchored to the row itself so + // the menu floats next to the clicked row regardless of scroll. + // Gated on raw `quickActionsCanMutate` (not editMode) per spec. + const [quickOpen, setQuickOpen] = useState(false); + const handleRowClick = useCallback( + (e: React.MouseEvent) => { + if (!quickActionsCanMutate) return; + const target = e.target as HTMLElement | null; + if (!target) return; + // Skip clicks that landed on an interactive child — leave them to + // their own handlers. Includes ARIA roles because TimeRangeCell + // and a few other affordances render as `div role="button"` + // (keyboard-activatable) rather than native ` + + + + + ) : null} + ); } diff --git a/artifacts/tx-os/tests/executive-meetings-row-quick-actions.spec.mjs b/artifacts/tx-os/tests/executive-meetings-row-quick-actions.spec.mjs new file mode 100644 index 00000000..5f15e4fd --- /dev/null +++ b/artifacts/tx-os/tests/executive-meetings-row-quick-actions.spec.mjs @@ -0,0 +1,180 @@ +// #486: e2e for the Executive Meetings schedule row quick-actions +// popover. Clicking any row (gated only on canMutate, NOT editMode) +// opens a small menu with Move up / Move down / Postpone. Move up/ +// down swap the (startTime, endTime) tuple between the clicked +// meeting and its chronological neighbour on the same date — the +// Time column stays visually anchored to its row position. +import { test, expect } from "@playwright/test"; +import pg from "pg"; + +const DATABASE_URL = process.env.DATABASE_URL; +if (!DATABASE_URL) { + throw new Error( + "DATABASE_URL must be set to run the row-quick-actions test", + ); +} +const pool = new pg.Pool({ connectionString: DATABASE_URL }); +const createdMeetingIds = []; + +async function loginViaUi(page, username, password) { + await page.goto("/login"); + await page.locator("#username").fill(username); + await page.locator("#password").fill(password); + await Promise.all([ + page.waitForURL((url) => !url.pathname.endsWith("/login"), { + timeout: 15_000, + }), + page.locator('form button[type="submit"]').click(), + ]); +} + +function pickFutureDate() { + // Pick a date well in the future so it can't collide with the + // upcoming-meeting alert, recurring seeded data, or other suite fixtures. + const d = new Date(Date.now() + 30 * 86_400_000); + const y = d.getFullYear(); + const m = String(d.getMonth() + 1).padStart(2, "0"); + const day = String(d.getDate()).padStart(2, "0"); + return `${y}-${m}-${day}`; +} + +const TEST_DATE = pickFutureDate(); + +async function gotoTestDate(page) { + await page.goto("/executive-meetings"); + // The schedule's date is internal React state, not a URL param, so + // drive it through the actual `` in the toolbar + // (same control the user clicks). It's the only date input on the + // page until the user opens a side panel. + const dateInput = page.locator('input[type="date"]').first(); + await dateInput.waitFor({ state: "visible", timeout: 10_000 }); + await dateInput.fill(TEST_DATE); + await dateInput.dispatchEvent("change"); +} + +async function nextDailyNumberForDate(date) { + const { rows } = await pool.query( + `SELECT COALESCE(MAX(daily_number), 0) + 1 AS n + FROM executive_meetings + WHERE meeting_date = $1`, + [date], + ); + return rows[0].n; +} + +async function insertMeeting({ titleEn, startTime, endTime }) { + const dn = await nextDailyNumberForDate(TEST_DATE); + const { rows } = await pool.query( + `INSERT INTO executive_meetings + (daily_number, title_ar, title_en, meeting_date, start_time, end_time, status) + VALUES ($1, $2, $2, $3, $4, $5, 'scheduled') + RETURNING id`, + [dn, titleEn, TEST_DATE, startTime, endTime], + ); + const id = rows[0].id; + createdMeetingIds.push(id); + return id; +} + +async function readMeeting(id) { + const { rows } = await pool.query( + `SELECT start_time, end_time, daily_number FROM executive_meetings WHERE id = $1`, + [id], + ); + return rows[0]; +} + +test.afterAll(async () => { + if (createdMeetingIds.length > 0) { + await pool.query( + `DELETE FROM executive_meeting_audit_logs WHERE entity_type = 'meeting' AND entity_id = ANY($1::int[])`, + [createdMeetingIds], + ); + await pool.query( + `DELETE FROM executive_meeting_attendees WHERE meeting_id = ANY($1::int[])`, + [createdMeetingIds], + ); + await pool.query( + `DELETE FROM executive_meetings WHERE id = ANY($1::int[])`, + [createdMeetingIds], + ); + } + await pool.end(); +}); + +test("row click opens quick-actions popover, Move up swaps times with neighbour", async ({ + page, +}) => { + const aId = await insertMeeting({ + titleEn: "QA Row Alpha", + startTime: "09:00:00", + endTime: "09:30:00", + }); + const bId = await insertMeeting({ + titleEn: "QA Row Bravo", + startTime: "10:00:00", + endTime: "10:30:00", + }); + + // admin/admin123 is the canonical seeded credential the rest of the + // executive-meetings e2e suite uses, and it has the mutate role + // (canMutate=true) without needing to flip on edit mode. + await loginViaUi(page, "admin", "admin123"); + await gotoTestDate(page); + // Wait for both rows to render. + await expect(page.locator(`[data-testid="em-row-${aId}"]`)).toBeVisible({ + timeout: 10_000, + }); + await expect(page.locator(`[data-testid="em-row-${bId}"]`)).toBeVisible(); + + // Click the SECOND row (Bravo, 10:00) — it should expose Move up. + await page.locator(`[data-testid="em-row-${bId}"]`).click(); + const popover = page.locator(`[data-testid="em-row-quick-${bId}"]`); + await expect(popover).toBeVisible(); + await expect( + popover.locator(`[data-testid="em-row-quick-up-${bId}"]`), + ).toBeEnabled(); + await expect( + popover.locator(`[data-testid="em-row-quick-postpone-${bId}"]`), + ).toBeVisible(); + + await popover.locator(`[data-testid="em-row-quick-up-${bId}"]`).click(); + + // Wait for the swap to land in the DB (refetch picks it up via the + // day-changed socket but we assert the DB directly for determinism). + await expect.poll(async () => (await readMeeting(bId)).start_time).toBe( + "09:00:00", + ); + const aAfter = await readMeeting(aId); + const bAfter = await readMeeting(bId); + // Times swapped, daily numbers re-sorted by start time. + expect(aAfter.start_time).toBe("10:00:00"); + expect(aAfter.end_time).toBe("10:30:00"); + expect(bAfter.start_time).toBe("09:00:00"); + expect(bAfter.end_time).toBe("09:30:00"); +}); + +test("Postpone item from quick-actions popover opens the postpone dialog", async ({ + page, +}) => { + const id = await insertMeeting({ + titleEn: "QA Postpone Target", + startTime: "13:00:00", + endTime: "13:30:00", + }); + + await loginViaUi(page, "admin", "admin123"); + await gotoTestDate(page); + await expect(page.locator(`[data-testid="em-row-${id}"]`)).toBeVisible({ + timeout: 10_000, + }); + + await page.locator(`[data-testid="em-row-${id}"]`).click(); + const popover = page.locator(`[data-testid="em-row-quick-${id}"]`); + await expect(popover).toBeVisible(); + await popover.locator(`[data-testid="em-row-quick-postpone-${id}"]`).click(); + // PostponeDialog is the same one upcoming-meeting-alert uses, so we + // assert by role=dialog presence rather than coupling to the + // dialog's internal markup. + await expect(page.getByRole("dialog")).toBeVisible({ timeout: 5_000 }); +}); diff --git a/lib/api-zod/src/manual.ts b/lib/api-zod/src/manual.ts index c7942109..1616a61f 100644 --- a/lib/api-zod/src/manual.ts +++ b/lib/api-zod/src/manual.ts @@ -8,3 +8,19 @@ export const ExecutiveMeetingsReorderBody = z.object({ export type ExecutiveMeetingsReorderBodyT = z.infer< typeof ExecutiveMeetingsReorderBody >; + +// #486: Body for POST /executive-meetings/swap-times. Used by the +// schedule row quick-actions popover (Move up / Move down) to swap +// just the (startTime, endTime) tuple between two meetings on the +// same date without changing daily numbering, so the time column +// stays visually anchored. +export const ExecutiveMeetingsSwapTimesBody = z.object({ + aId: z.number().int().positive(), + bId: z.number().int().positive(), + expectedUpdatedAtA: z.string().min(1), + expectedUpdatedAtB: z.string().min(1), +}); + +export type ExecutiveMeetingsSwapTimesBodyT = z.infer< + typeof ExecutiveMeetingsSwapTimesBody +>;