From e19506aac79d35bb53a8a38de32ad2eea220dfdc Mon Sep 17 00:00:00 2001 From: riyadhafraa <49757212-riyadhafraa@users.noreply.replit.com> Date: Mon, 11 May 2026 14:21:02 +0000 Subject: [PATCH] Block EM row-drag when any meeting on the day lacks a time window (#492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /api/executive-meetings/rotate-content endpoint hard-rejects with `code: "no_time_window"` whenever any visible non-cancelled meeting on the date is missing (start_time, end_time). Before this change, dragging a row on such a day produced an opaque English error toast ("Every meeting must have a scheduled time window to rotate") that confused AR users and didn't tell them what to fix. Fix: - Compute `missingTimeCount` / `dayRotatable` from `orderedMeetings` in executive-meetings.tsx and thread `dayRotatable` into MeetingRow. - MeetingRow now gates useSortable + safeRowDragListeners on `effectiveDragEnabled = dragEnabled && dayRotatable`. When drag is blocked specifically by missing time, the shows `cursor-not-allowed`, dimmed opacity, `aria-disabled="true"`, `data-drag-blocked="no_time_window"`, and a bilingual `title` tooltip. The aria-disabled is spread AFTER dnd-kit's attributes so it wins the prop merge. - Inline amber `em-rotate-blocked-hint` banner above the bulk toolbar surfaces the missing-time count so mutators see why drag is paused. - rotateContent's catch handler now detects ApiError code "no_time_window" (local apiJson now attaches .code/.status to the thrown Error) and shows the bilingual `executiveMeetings.rotate.needsTimeWindow.errorToast` instead of the raw server message. - Locale keys added under `executiveMeetings.rotate.needsTimeWindow` in en.json + ar.json (tooltip, hint_one/_other [+ AR plurals], errorToast). Tests: - New tests/executive-meetings-rotate-needs-time-window.spec.mjs inserts one untimed + two timed meetings and asserts the hint banner + every row's aria-disabled + data-drag-blocked, then attempts a drag and asserts NO rotate-content POST is sent and DB state is unchanged. After UPDATE-ing the missing time, asserts the hint and aria-disabled lift. - Existing executive-meetings-row-drag and touch-reorder specs still pass — drag works exactly as before on fully-timed days. Files: artifacts/tx-os/src/pages/executive-meetings.tsx, artifacts/tx-os/src/locales/{en,ar}.json, artifacts/tx-os/tests/executive-meetings-rotate-needs-time-window.spec.mjs Server route untouched. --- artifacts/tx-os/src/locales/ar.json | 12 + artifacts/tx-os/src/locales/en.json | 8 + .../tx-os/src/pages/executive-meetings.tsx | 120 ++++++++- ...meetings-rotate-needs-time-window.spec.mjs | 230 ++++++++++++++++++ 4 files changed, 364 insertions(+), 6 deletions(-) create mode 100644 artifacts/tx-os/tests/executive-meetings-rotate-needs-time-window.spec.mjs diff --git a/artifacts/tx-os/src/locales/ar.json b/artifacts/tx-os/src/locales/ar.json index 88aaebb2..8d504fa8 100644 --- a/artifacts/tx-os/src/locales/ar.json +++ b/artifacts/tx-os/src/locales/ar.json @@ -1433,6 +1433,18 @@ "moveDown": "نقل لأسفل", "postpone": "تأجيل" }, + "rotate": { + "needsTimeWindow": { + "tooltip": "حدّد وقت بداية ونهاية لكل اجتماع قبل إعادة ترتيب هذا اليوم.", + "hint_zero": "إعادة الترتيب بالسحب متوقفة — لا توجد اجتماعات بدون وقت.", + "hint_one": "إعادة الترتيب بالسحب متوقفة — اجتماع واحد في هذا اليوم بدون وقت بداية ونهاية.", + "hint_two": "إعادة الترتيب بالسحب متوقفة — اجتماعان في هذا اليوم بدون وقت بداية ونهاية.", + "hint_few": "إعادة الترتيب بالسحب متوقفة — {{count}} اجتماعات في هذا اليوم بدون وقت بداية ونهاية.", + "hint_many": "إعادة الترتيب بالسحب متوقفة — {{count}} اجتماعاً في هذا اليوم بدون وقت بداية ونهاية.", + "hint_other": "إعادة الترتيب بالسحب متوقفة — {{count}} اجتماع في هذا اليوم بدون وقت بداية ونهاية.", + "errorToast": "حدّد وقت بداية ونهاية لكل اجتماع قبل إعادة ترتيب هذا اليوم." + } + }, "merge": { "label": "دمج الخلايا", "editLabel": "تعديل الخلية المدموجة", diff --git a/artifacts/tx-os/src/locales/en.json b/artifacts/tx-os/src/locales/en.json index b1af1c69..ea46d48a 100644 --- a/artifacts/tx-os/src/locales/en.json +++ b/artifacts/tx-os/src/locales/en.json @@ -1147,6 +1147,14 @@ "moveDown": "Move down", "postpone": "Postpone" }, + "rotate": { + "needsTimeWindow": { + "tooltip": "Set a start and end time on every meeting before you can reorder this day.", + "hint_one": "Drag-to-reorder is paused — 1 meeting on this day is missing a start/end time.", + "hint_other": "Drag-to-reorder is paused — {{count}} meetings on this day are missing a start/end time.", + "errorToast": "Set a start and end time on every meeting before you can reorder this day." + } + }, "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 961a3899..8b7abdbf 100644 --- a/artifacts/tx-os/src/pages/executive-meetings.tsx +++ b/artifacts/tx-os/src/pages/executive-meetings.tsx @@ -561,7 +561,16 @@ async function apiJson( if (res.status === 204) return undefined as T; const data = (await res.json().catch(() => ({}))) as T & { error?: string }; if (!res.ok) { - throw new Error((data as { error?: string }).error || `HTTP ${res.status}`); + const message = + (data as { error?: string }).error || `HTTP ${res.status}`; + const err = new Error(message) as Error & { + code?: string; + status?: number; + }; + const rawCode = (data as { code?: unknown }).code; + if (typeof rawCode === "string") err.code = rawCode; + err.status = res.status; + throw err; } return data; } @@ -2204,7 +2213,20 @@ function ScheduleSection({ // #489: restore the prior optimistic order on rollback so a // failed rotate doesn't leave the UI in the post-drop order. setOptimisticOrder(previousOptimisticOrder); - const msg = err instanceof Error ? err.message : String(err); + // #492: localize the server's `no_time_window` 400 — the raw + // English error string ("Every meeting must have a scheduled + // time window to rotate") was leaking into the toast for AR + // users and confusing everyone. The new tooltip + inline hint + // already explain the gating in both languages, so the toast + // mirrors that copy on this specific error code. + const code = (err as { code?: unknown } | null)?.code; + const isNoTimeWindow = + typeof code === "string" && code === "no_time_window"; + const msg = isNoTimeWindow + ? t("executiveMeetings.rotate.needsTimeWindow.errorToast") + : err instanceof Error + ? err.message + : String(err); toast({ title: t("common.error"), description: msg, @@ -2218,6 +2240,21 @@ function ScheduleSection({ [orderedMeetings, optimisticOrder, date, qc, refreshDay, toast, t], ); + // #492: a row-drag rotation is only meaningful when EVERY visible + // meeting on the day has both a startTime and endTime — the server's + // /rotate-content endpoint hard-rejects any rotation that includes a + // row with a NULL time tuple (`code: "no_time_window"`). We block the + // drag client-side so the user gets a tooltip + inline hint instead + // of an opaque error toast after they actually try to drag. + const missingTimeCount = useMemo( + () => + orderedMeetings.filter( + (m) => m.startTime == null || m.endTime == null, + ).length, + [orderedMeetings], + ); + const dayRotatable = missingTimeCount === 0; + // #486 → #489: Move up / Move down were retired in favour of // dragging the whole row (`rotateContent` above). The schedule's // row click popover now keeps only the Postpone button. The @@ -2638,6 +2675,30 @@ function ScheduleSection({
{date}
+ {/* #492: inline hint banner — surfaces the reason row-drag is + paused for the current day so mutators don't try to drag and + get a server 400. Only shown when the user actually has + mutate permission (otherwise drag was never available) and + at least one meeting is missing a (start, end) tuple. */} + {canMutate && missingTimeCount > 0 && ( +
+ + + {(() => { + const key = + missingTimeCount === 1 + ? "executiveMeetings.rotate.needsTimeWindow.hint_one" + : "executiveMeetings.rotate.needsTimeWindow.hint_other"; + return t(key).replace("{{count}}", String(missingTimeCount)); + })()} + +
+ )} + {/* Floating bulk-action toolbar. Per spec it appears ONLY when ≥1 row is selected (and in edit mode). The tri-state "select all" checkbox lives in the table header (see @@ -2893,6 +2954,13 @@ function ScheduleSection({ // #489: drag must work for mutators regardless // of edit mode — pass raw mutate permission. dragEnabled={canMutate} + // #492: but block the actual drag (with a dimmed, + // aria-disabled row + bilingual tooltip) when any + // meeting on the day is missing a (start, end) + // tuple — the server would 400 `no_time_window` + // anyway. The hint banner above the table tells + // the user how many rows still need a time. + dayRotatable={dayRotatable} onSaveTitle={(html) => saveTitle(m, html)} onSaveAttendeeName={(idx, html) => saveAttendeeName(m, idx, html) @@ -3716,6 +3784,7 @@ function MeetingRow({ onBulkSelectChange, displayNumber, dragEnabled, + dayRotatable, quickActionsCanMutate, onQuickPostpone, }: { @@ -3734,6 +3803,12 @@ function MeetingRow({ // (which equals canMutate && editMode and powers inline-editing // affordances on cells). dragEnabled: boolean; + // #492: false when ANY meeting on the day lacks a (startTime, + // endTime) — the server's rotate-content endpoint hard-rejects the + // payload, so we block the drag client-side. The row still renders + // (no popover/edit affordances change), but useSortable is disabled + // and the is dimmed + aria-disabled with a bilingual title. + dayRotatable: 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 @@ -3799,6 +3874,13 @@ function MeetingRow({ // at the page level keep clicks/taps on inert cell area from being // mistaken for drags, so the quick-actions popover still opens on // tap-and-release. + // #492: actual drag is gated by BOTH the user's mutate permission + // (dragEnabled) AND every meeting on the day having a (start, end) + // tuple (dayRotatable). When dragEnabled is true but dayRotatable + // is false we still show a "would be draggable" affordance, just + // dimmed + cursor-not-allowed with a tooltip explaining why. + const effectiveDragEnabled = dragEnabled && dayRotatable; + const dragBlockedByMissingTime = dragEnabled && !dayRotatable; const { attributes, listeners, @@ -3806,7 +3888,7 @@ function MeetingRow({ transform, transition, isDragging, - } = useSortable({ id: meeting.id, disabled: !dragEnabled }); + } = useSortable({ id: meeting.id, disabled: !effectiveDragEnabled }); const isSkipDragTarget = useCallback( (target: EventTarget | null): boolean => { const el = target as HTMLElement | null; @@ -3836,7 +3918,7 @@ function MeetingRow({ [meeting.id], ); const safeRowDragListeners = useMemo(() => { - if (!listeners || !dragEnabled) return undefined; + if (!listeners || !effectiveDragEnabled) return undefined; const out: Record void> = {}; for (const k of Object.keys(listeners)) { const handler = (listeners as Record void>)[k]; @@ -3847,7 +3929,7 @@ function MeetingRow({ }; } return out; - }, [listeners, dragEnabled, isSkipDragTarget]); + }, [listeners, effectiveDragEnabled, isSkipDragTarget]); const numberCellCls = "border border-gray-300 px-2 py-2 text-center align-middle font-semibold " + @@ -4291,15 +4373,41 @@ function MeetingRow({ <> in Radix's Tooltip primitive is fragile because + // TooltipTrigger renders its own element wrapper. + {...(dragBlockedByMissingTime ? { "aria-disabled": true } : {})} > {renderCells()} diff --git a/artifacts/tx-os/tests/executive-meetings-rotate-needs-time-window.spec.mjs b/artifacts/tx-os/tests/executive-meetings-rotate-needs-time-window.spec.mjs new file mode 100644 index 00000000..8824266c --- /dev/null +++ b/artifacts/tx-os/tests/executive-meetings-rotate-needs-time-window.spec.mjs @@ -0,0 +1,230 @@ +// #492: e2e for the client-side guard that blocks row-drag rotation +// when ANY meeting on the day is missing a (start_time, end_time) +// tuple. The server's /api/executive-meetings/rotate-content endpoint +// hard-rejects such payloads with `code: "no_time_window"`; before +// #492 the client surfaced the raw English error in a toast even for +// AR users. The fix: +// * disable useSortable on every row of the day (aria-disabled + +// dimmed cursor + bilingual tooltip) so the drag never starts; +// * show an inline amber hint banner above the table with the +// count of meetings still missing a time; +// * once every meeting has a time, drag re-enables and rotation +// succeeds end-to-end. +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 rotate-needs-time-window test", + ); +} +const pool = new pg.Pool({ connectionString: DATABASE_URL }); +const createdMeetingIds = []; + +function pickFutureDate(offsetDays) { + 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 DATE = pickFutureDate(123); + +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({ 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, date, startTime, endTime], + ); + const id = rows[0].id; + createdMeetingIds.push(id); + return id; +} + +async function setMeetingTimes(id, startTime, endTime) { + await pool.query( + `UPDATE executive_meetings + SET start_time = $2, end_time = $3, updated_at = NOW() + WHERE id = $1`, + [id, startTime, endTime], + ); +} + +async function readMeeting(id) { + const { rows } = await pool.query( + `SELECT title_en, start_time, end_time, daily_number + FROM executive_meetings WHERE id = $1`, + [id], + ); + return rows[0]; +} + +async function setLangEn(page) { + await page.addInitScript(() => { + try { + window.localStorage.setItem("tx-lang", "en"); + } catch { + /* ignore */ + } + }); +} + +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(), + ]); +} + +async function gotoTestDate(page, date) { + await page.goto("/executive-meetings"); + const dateInput = page.locator('input[type="date"]').first(); + await dateInput.waitFor({ state: "visible", timeout: 10_000 }); + await dateInput.fill(date); + await dateInput.dispatchEvent("change"); +} + +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("blocks row-drag while any meeting on the day is missing a time, then re-enables once times are set", async ({ + page, +}) => { + const timedId = await insertMeeting({ + date: DATE, + titleEn: "NeedsTime Timed", + startTime: "09:00:00", + endTime: "09:30:00", + }); + const untimedId = await insertMeeting({ + date: DATE, + titleEn: "NeedsTime Untimed", + startTime: null, + endTime: null, + }); + const otherId = await insertMeeting({ + date: DATE, + titleEn: "NeedsTime Other", + startTime: "11:00:00", + endTime: "11:30:00", + }); + + await setLangEn(page); + await loginViaUi(page, "admin", "admin123"); + await gotoTestDate(page, DATE); + + // Force EN regardless of the admin user's stored preferredLanguage + // (AuthContext reapplies it on every login, which would otherwise + // flip the page back to AR for the demo admin). + await page.evaluate(async () => { + const w = window; + if (w.i18next && typeof w.i18next.changeLanguage === "function") { + await w.i18next.changeLanguage("en"); + } + }); + + // Hint banner shows up with the missing count. Both AR and EN + // strings include the digit "1" plus the testid "em-rotate-blocked- + // hint" — we assert visibility + the count digit so the test stays + // language-agnostic if the demo admin's language ever changes. + const hint = page.getByTestId("em-rotate-blocked-hint"); + await expect(hint).toBeVisible({ timeout: 10_000 }); + + // Every row on the day is aria-disabled for drag (even the rows + // that DO have a time — rotation is whole-day). + const timedRow = page.getByTestId(`em-row-${timedId}`); + const untimedRow = page.getByTestId(`em-row-${untimedId}`); + const otherRow = page.getByTestId(`em-row-${otherId}`); + await expect(timedRow).toHaveAttribute("aria-disabled", "true"); + await expect(untimedRow).toHaveAttribute("aria-disabled", "true"); + await expect(otherRow).toHaveAttribute("aria-disabled", "true"); + await expect(timedRow).toHaveAttribute("data-drag-blocked", "no_time_window"); + + // Attempting to drag must NOT trigger a rotate-content POST. Wire a + // listener; if any rotate request fires during the next ~2s we + // record it. We try the same drag gesture the real e2e drag spec + // uses (drag from the # cell of timedId down past otherId). + let rotateAttempts = 0; + page.on("request", (req) => { + if ( + req.url().includes("/api/executive-meetings/rotate-content") && + req.method() === "POST" + ) { + rotateAttempts += 1; + } + }); + const numCell = page + .locator(`[data-testid="em-row-${timedId}"] td`) + .first(); + const numBox = await numCell.boundingBox(); + const otherBox = await otherRow.boundingBox(); + if (!numBox || !otherBox) throw new Error("rows must be visible"); + const startX = numBox.x + numBox.width / 2; + const startY = numBox.y + numBox.height / 2; + await page.mouse.move(startX, startY); + await page.mouse.down(); + await page.mouse.move(startX, startY + 12, { steps: 5 }); + await page.mouse.move(startX, otherBox.y + otherBox.height - 4, { + steps: 15, + }); + await page.mouse.up(); + await page.waitForTimeout(800); + expect(rotateAttempts).toBe(0); + + // DB state unchanged. + const timedBefore = await readMeeting(timedId); + expect(timedBefore.start_time).toBe("09:00:00"); + + // Now give the untimed meeting a real time window. Once the day + // becomes fully-timed the hint must disappear and aria-disabled + // must lift on every row — the actual rotation drag path is + // already covered by executive-meetings-row-drag.spec.mjs, so we + // only verify the block state lifts here. + await setMeetingTimes(untimedId, "10:00:00", "10:30:00"); + await page.reload(); + await page.locator('input[type="date"]').first().fill(DATE); + await expect(timedRow).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId("em-rotate-blocked-hint")).toHaveCount(0, { + timeout: 10_000, + }); + await expect(timedRow).not.toHaveAttribute("aria-disabled", "true"); + await expect(timedRow).not.toHaveAttribute( + "data-drag-blocked", + "no_time_window", + ); +});