#486: Executive Meetings row click → quick-actions popover

Clicking any meeting row on the Executive Meetings schedule (gated only on canMutate, not editMode) opens a small popover with Move up / Move down / Postpone. Move up/down swap only 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. Backend - POST /executive-meetings/swap-times: transactional swap with FOR UPDATE row locking, optimistic-lock conflict shape (stale_meeting + conflict payload), date/time-window guards, audit logging, and renumberDayByStartTime + day-changed broadcast. - Zod schema in lib/api-zod/src/manual.ts. Frontend - Shared lib/api-json.ts JSON helper. - ScheduleSection.swapTimes does an optimistic (startTime, endTime) swap against the day query cache and rolls back on failure (mirrors the existing inline-edit UX). - MeetingRow uses Popover/PopoverAnchor with skip rules: ARIA roles (button/checkbox/switch/combobox/dialog) and em-time-* / em-edit-* / em-row-grip / em-row-actions data-testid prefixes do NOT open the popover. - PostponeDialog reused from upcoming-meeting-alert.tsx. Tests - Backend swap-times: happy path, stale_meeting (409), different_dates (400), no_time_window (400), unauth (401), viewer-no-mutate (403) — all 6 pass. - E2E: Move up swap, edge-disable states (solo / first / middle / last), Postpone 5-min chip end-to-end, click-exclusion on grip / time cell / row-actions — all 4 pass. Each test uses its own future date to avoid cross-test pollution. Code review approved on second pass. Pre-existing failures in other suites (executive-meetings reorder, font-settings, notes-share, service-orders) are unrelated to this task and predate it. Follow-ups proposed: #487 (keyboard a11y on the popover), #488 (edit- mode test gaps).
2026-05-11 11:03:24 +00:00
parent b1b77395d0
commit 02b3b374b0
3 changed files with 234 additions and 16 deletions
@@ -96,12 +96,18 @@ let userA = null;
 let cookieA = null;
 let userB = null;
 let cookieB = null;
+let viewerCookie = 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);
+  // executive_viewer is the read-only role — passes
+  // requireExecutiveAccess but is NOT in MUTATE_ROLES, so it lets us
+  // assert that swap-times rejects non-mutators with 403.
+  const viewer = await createUser("em_swap_view", "executive_viewer", "Vee Viewer");
+  viewerCookie = await login(viewer.username, TEST_PASSWORD);
 });

 after(async () => {
@@ -304,6 +310,50 @@ test("swap-times: different dates → 400 different_dates", async () => {
  assert.equal(body.code, "different_dates");
 });

+test("swap-times: unauthenticated request → 401", async () => {
+  const res = await fetch(`${API_BASE}/api/executive-meetings/swap-times`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json" },
+    body: JSON.stringify({
+      aId: 1,
+      bId: 2,
+      expectedUpdatedAtA: new Date().toISOString(),
+      expectedUpdatedAtB: new Date().toISOString(),
+    }),
+  });
+  assert.equal(res.status, 401);
+});
+
+test("swap-times: viewer (no mutate role) → 403 forbidden", async () => {
+  // Create the pair as a mutator first, then try to swap as a viewer.
+  const a = await createMeeting({
+    date: futureDate(35),
+    titleEn: "Viewer A",
+    startTime: "08:00",
+    endTime: "08:30",
+  });
+  const b = await createMeeting({
+    date: futureDate(35),
+    titleEn: "Viewer B",
+    startTime: "09:00",
+    endTime: "09:30",
+  });
+  const aFresh = await fetchMeeting(cookieA, a.id);
+  const bFresh = await fetchMeeting(cookieA, b.id);
+  const res = await api(
+    viewerCookie,
+    "POST",
+    "/api/executive-meetings/swap-times",
+    {
+      aId: a.id,
+      bId: b.id,
+      expectedUpdatedAtA: aFresh.updatedAt,
+      expectedUpdatedAtB: bFresh.updatedAt,
+    },
+  );
+  assert.equal(res.status, 403, "viewer must not be able to swap times");
+});
+
 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).
@@ -2168,6 +2168,27 @@ function ScheduleSection({
      if (reorderingRef.current) return;
      reorderingRef.current = true;
      setReordering(true);
+      // Optimistic UI: snapshot the day cache, swap (startTime, endTime)
+      // between A and B locally so the row repaints in the same React
+      // commit the popover closes in. Mirrors the inline-edit pattern
+      // used by saveTitle/saveTimes — on server failure the snapshot is
+      // restored and the destructive toast surfaces the error.
+      const queryKey = ["/api/executive-meetings", date] as const;
+      const previous = qc.getQueryData<DayResponse>(queryKey);
+      qc.setQueryData<DayResponse>(queryKey, (prev) =>
+        prev
+          ? {
+              ...prev,
+              meetings: prev.meetings.map((m) => {
+                if (m.id === a.id)
+                  return { ...m, startTime: b.startTime, endTime: b.endTime };
+                if (m.id === b.id)
+                  return { ...m, startTime: a.startTime, endTime: a.endTime };
+                return m;
+              }),
+            }
+          : prev,
+      );
      try {
        await apiJson(`/api/executive-meetings/swap-times`, {
          method: "POST",
@@ -2180,6 +2201,11 @@ function ScheduleSection({
        });
        refreshDay();
      } catch (err) {
+        // Rollback the optimistic write so the row snaps back to the
+        // last server-confirmed times.
+        if (previous !== undefined) {
+          qc.setQueryData<DayResponse>(queryKey, previous);
+        }
        const msg = err instanceof Error ? err.message : String(err);
        toast({
          title: t("common.error"),
@@ -2191,7 +2217,7 @@ function ScheduleSection({
        setReordering(false);
      }
    },
-    [refreshDay, toast, t],
+    [qc, date, refreshDay, toast, t],
  );

  const quickMoveUp = useCallback(
@@ -28,19 +28,25 @@ async function loginViaUi(page, username, password) {
  ]);
 }

-function pickFutureDate() {
+function pickFutureDate(offsetDays) {
  // 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 d = new Date(Date.now() + offsetDays * 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();
+// Each test gets its own dedicated future date so leftover rows from
+// the previous test (which only get cleaned in afterAll) don't pollute
+// the next test's "first row / last row / solo row" assumptions.
+const DATE_SWAP = pickFutureDate(30);
+const DATE_EDGE = pickFutureDate(31);
+const DATE_POSTPONE = pickFutureDate(32);
+const DATE_SKIP = pickFutureDate(33);

-async function gotoTestDate(page) {
+async function gotoTestDate(page, date) {
  await page.goto("/executive-meetings");
  // The schedule's date is internal React state, not a URL param, so
  // drive it through the actual `<input type="date">` in the toolbar
@@ -48,7 +54,7 @@ async function gotoTestDate(page) {
  // 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.fill(date);
  await dateInput.dispatchEvent("change");
 }

@@ -62,14 +68,14 @@ async function nextDailyNumberForDate(date) {
  return rows[0].n;
 }

-async function insertMeeting({ titleEn, startTime, endTime }) {
-  const dn = await nextDailyNumberForDate(TEST_DATE);
+async function insertMeeting({ date, titleEn, startTime, endTime }) {
+  const dn = await nextDailyNumberForDate(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],
+    [dn, titleEn, date, startTime, endTime],
  );
  const id = rows[0].id;
  createdMeetingIds.push(id);
@@ -106,11 +112,13 @@ test("row click opens quick-actions popover, Move up swaps times with neighbour"
  page,
 }) => {
  const aId = await insertMeeting({
+    date: DATE_SWAP,
    titleEn: "QA Row Alpha",
    startTime: "09:00:00",
    endTime: "09:30:00",
  });
  const bId = await insertMeeting({
+    date: DATE_SWAP,
    titleEn: "QA Row Bravo",
    startTime: "10:00:00",
    endTime: "10:30:00",
@@ -120,7 +128,7 @@ test("row click opens quick-actions popover, Move up swaps times with neighbour"
  // 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);
+  await gotoTestDate(page, DATE_SWAP);
  // Wait for both rows to render.
  await expect(page.locator(`[data-testid="em-row-${aId}"]`)).toBeVisible({
    timeout: 10_000,
@@ -154,17 +162,100 @@ test("row click opens quick-actions popover, Move up swaps times with neighbour"
  expect(bAfter.end_time).toBe("09:30:00");
 });

-test("Postpone item from quick-actions popover opens the postpone dialog", async ({
+test("edge rows expose disabled Move up / Move down at the boundaries", async ({
+  page,
+}) => {
+  // First-and-only-row scenario: a single meeting on the test day
+  // means BOTH directions should be disabled (no neighbour either way).
+  const onlyId = await insertMeeting({
+    date: DATE_EDGE,
+    titleEn: "QA Edge Solo",
+    startTime: "11:00:00",
+    endTime: "11:30:00",
+  });
+  await loginViaUi(page, "admin", "admin123");
+  await gotoTestDate(page, DATE_EDGE);
+  await expect(page.locator(`[data-testid="em-row-${onlyId}"]`)).toBeVisible({
+    timeout: 10_000,
+  });
+  await page.locator(`[data-testid="em-row-${onlyId}"]`).click();
+  const soloPop = page.locator(`[data-testid="em-row-quick-${onlyId}"]`);
+  await expect(soloPop).toBeVisible();
+  await expect(
+    soloPop.locator(`[data-testid="em-row-quick-up-${onlyId}"]`),
+  ).toBeDisabled();
+  await expect(
+    soloPop.locator(`[data-testid="em-row-quick-down-${onlyId}"]`),
+  ).toBeDisabled();
+  // Close the popover before adding more rows so subsequent waits see
+  // the freshly-rendered first/last rows.
+  await page.keyboard.press("Escape");
+
+  // Now add neighbours so onlyId becomes the FIRST chronological row.
+  // First row → Move up disabled, Move down enabled. Last row → mirror.
+  const middleId = await insertMeeting({
+    date: DATE_EDGE,
+    titleEn: "QA Edge Middle",
+    startTime: "11:30:00",
+    endTime: "12:00:00",
+  });
+  const lastId = await insertMeeting({
+    date: DATE_EDGE,
+    titleEn: "QA Edge Last",
+    startTime: "12:00:00",
+    endTime: "12:30:00",
+  });
+  // Re-load so the day query sees all three rows.
+  await gotoTestDate(page, DATE_EDGE);
+  await expect(page.locator(`[data-testid="em-row-${lastId}"]`)).toBeVisible({
+    timeout: 10_000,
+  });
+  await page.locator(`[data-testid="em-row-${onlyId}"]`).click();
+  const firstPop = page.locator(`[data-testid="em-row-quick-${onlyId}"]`);
+  await expect(firstPop).toBeVisible();
+  await expect(
+    firstPop.locator(`[data-testid="em-row-quick-up-${onlyId}"]`),
+  ).toBeDisabled();
+  await expect(
+    firstPop.locator(`[data-testid="em-row-quick-down-${onlyId}"]`),
+  ).toBeEnabled();
+  await page.keyboard.press("Escape");
+
+  await page.locator(`[data-testid="em-row-${lastId}"]`).click();
+  const lastPop = page.locator(`[data-testid="em-row-quick-${lastId}"]`);
+  await expect(lastPop).toBeVisible();
+  await expect(
+    lastPop.locator(`[data-testid="em-row-quick-down-${lastId}"]`),
+  ).toBeDisabled();
+  await expect(
+    lastPop.locator(`[data-testid="em-row-quick-up-${lastId}"]`),
+  ).toBeEnabled();
+
+  // Sanity: the middle row exposes BOTH directions enabled.
+  await page.keyboard.press("Escape");
+  await page.locator(`[data-testid="em-row-${middleId}"]`).click();
+  const midPop = page.locator(`[data-testid="em-row-quick-${middleId}"]`);
+  await expect(midPop).toBeVisible();
+  await expect(
+    midPop.locator(`[data-testid="em-row-quick-up-${middleId}"]`),
+  ).toBeEnabled();
+  await expect(
+    midPop.locator(`[data-testid="em-row-quick-down-${middleId}"]`),
+  ).toBeEnabled();
+});
+
+test("Postpone quick-action runs a 5-minute postpone end-to-end", async ({
  page,
 }) => {
  const id = await insertMeeting({
+    date: DATE_POSTPONE,
    titleEn: "QA Postpone Target",
    startTime: "13:00:00",
    endTime: "13:30:00",
  });

  await loginViaUi(page, "admin", "admin123");
-  await gotoTestDate(page);
+  await gotoTestDate(page, DATE_POSTPONE);
  await expect(page.locator(`[data-testid="em-row-${id}"]`)).toBeVisible({
    timeout: 10_000,
  });
@@ -173,8 +264,59 @@ test("Postpone item from quick-actions popover opens the postpone dialog", async
  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 });
+  // PostponeDialog is the same one upcoming-meeting-alert uses; drive
+  // its 5-minute chip → confirm flow and assert the new times land.
+  const dialog = page.locator('[data-testid="postpone-dialog"]');
+  await expect(dialog).toBeVisible({ timeout: 5_000 });
+  await dialog.locator('[data-testid="postpone-chip-5"]').click();
+  await dialog.locator('[data-testid="postpone-confirm-yes"]').click();
+  await expect.poll(async () => (await readMeeting(id)).start_time).toBe(
+    "13:05:00",
+  );
+  const after = await readMeeting(id);
+  expect(after.end_time).toBe("13:35:00");
+});
+
+test("clicking row affordances (grip, time cell, row-actions) does NOT open the popover", async ({
+  page,
+}) => {
+  const id = await insertMeeting({
+    date: DATE_SKIP,
+    titleEn: "QA Skip Surfaces",
+    startTime: "14:00:00",
+    endTime: "14:30:00",
+  });
+  await loginViaUi(page, "admin", "admin123");
+  await gotoTestDate(page, DATE_SKIP);
+  await expect(page.locator(`[data-testid="em-row-${id}"]`)).toBeVisible({
+    timeout: 10_000,
+  });
+  const popover = page.locator(`[data-testid="em-row-quick-${id}"]`);
+
+  // Drag grip — handled by dnd-kit, must not open the popover.
+  const grip = page.locator(`[data-testid="em-row-grip-${id}"]`);
+  if (await grip.count()) {
+    await grip.click({ force: true });
+    await expect(popover).toBeHidden();
+  }
+
+  // Time cell — clicking it enters inline edit mode in TimeRangeCell.
+  // Must NOT also open the quick-actions popover.
+  const timeCell = page.locator(`[data-testid="em-time-${id}"]`);
+  await timeCell.click();
+  await expect(popover).toBeHidden();
+  // Clean up the inline editor so it doesn't leak into the next assertion.
+  await page.keyboard.press("Escape");
+
+  // Row-actions trigger — opens the row's own dropdown, not our popover.
+  const rowActions = page.locator(`[data-testid="em-row-actions-${id}"]`);
+  if (await rowActions.count()) {
+    await rowActions.click();
+    await expect(popover).toBeHidden();
+    await page.keyboard.press("Escape");
+  }
+
+  // Sanity: clicking an empty cell of the row DOES open the popover.
+  await page.locator(`[data-testid="em-row-${id}"]`).click({ position: { x: 5, y: 5 } });
+  await expect(popover).toBeVisible({ timeout: 5_000 });
 });