diff --git a/artifacts/api-server/src/routes/executive-meetings.ts b/artifacts/api-server/src/routes/executive-meetings.ts index 619713ef..bd8601c5 100644 --- a/artifacts/api-server/src/routes/executive-meetings.ts +++ b/artifacts/api-server/src/routes/executive-meetings.ts @@ -600,6 +600,16 @@ router.post( newValue: { ...meeting, attendees }, performedBy: userId, }); + // #312: A new meeting with an explicit start_time arrives at the + // tail (nextDailyNumber appends), so the visible list is no + // longer chronological. Renumber 1..N by start_time so the row + // appears at its correct slot without requiring a manual drag. + // Cancelled rows go to the tail (renumberDayByStartTime handles + // that) so the per-day unique index stays satisfied. The newly + // INSERTed row has its own dailyNumber from `nextDailyNumber`, + // so the temporary positives-to-negatives sweep in step 1 picks + // it up too. + await renumberDayByStartTime(tx, data.meetingDate); const approverIds = await getUserIdsForRoleNames(EM_ADMIN_ROLES); const recipients = await recordExecutiveMeetingNotifications(tx, { recipientUserIds: approverIds, @@ -671,6 +681,23 @@ router.patch( if (data.meetingDate !== undefined) updateValues.meetingDate = data.meetingDate; if (data.dailyNumber !== undefined && data.dailyNumber !== null) updateValues.dailyNumber = data.dailyNumber; + // #312: Cross-day move via PATCH meetingDate would otherwise + // keep the row's old daily_number, which collides with an + // existing row on the destination date under the + // `(meeting_date, daily_number)` unique index. Pre-allocate a + // fresh dailyNumber on the new date (max+1) so the UPDATE + // succeeds; renumberDayByStartTime then re-sorts the destination + // day chronologically a few lines down. We only do this when the + // caller did NOT pass an explicit dailyNumber — that path keeps + // its own semantics. Safe inside the transaction because + // nextDailyNumber reads through the same `tx` executor. + if ( + data.meetingDate !== undefined && + data.meetingDate !== existing.meetingDate && + (data.dailyNumber === undefined || data.dailyNumber === null) + ) { + updateValues.dailyNumber = await nextDailyNumber(tx, data.meetingDate); + } if (data.startTime !== undefined) updateValues.startTime = data.startTime; if (data.endTime !== undefined) updateValues.endTime = data.endTime; // Plain-text fields: strip any HTML at the API boundary so a @@ -740,6 +767,40 @@ router.patch( newValue: { ...existing, ...updateValues, attendees }, performedBy: userId, }); + // #312: Auto-sort the day(s) chronologically when the PATCH + // touched a field that affects ordering or visibility. The user's + // bug report — typing 13:00 on a row that sits above a 12:00 row + // — is exactly this: a startTime PATCH leaves dailyNumber + // untouched and the row stays in its old position even though + // the time changed. Renumbering by start_time after the UPDATE + // makes the visible list strictly chronological without forcing + // the user to drag the row. We trigger on: + // - startTime / endTime: the obvious case the user reported. + // - meetingDate: the row moved to a different day; both old + // and new days need to renumber so the row leaves one + // timeline cleanly and arrives in chronological order on + // the other. + // - status: cancel/uncancel changes which rows are "visible" + // (cancelled rows go to the tail per renumberDayByStartTime), + // so the visible 1..N sequence must be recomputed. + // - dailyNumber: a manual override; we still renumber so the + // final state stays chronological (auto-sort wins over a + // stray manual number coming from non-reorder paths). + const orderAffectingChange = + data.startTime !== undefined || + data.endTime !== undefined || + data.meetingDate !== undefined || + data.status !== undefined || + data.dailyNumber !== undefined; + if (orderAffectingChange) { + const newDate = updateValues.meetingDate ?? existing.meetingDate; + await renumberDayByStartTime(tx, newDate); + if (newDate !== existing.meetingDate) { + // Cross-day move: the source day also needs to close ranks + // so its 1..N sequence stays gap-free. + await renumberDayByStartTime(tx, existing.meetingDate); + } + } }); const updated = await fetchMeetingWithAttendees(id); // Realtime: when the meeting was rescheduled to a different day we @@ -1792,6 +1853,12 @@ router.delete( oldValue: existing, performedBy: userId, }); + // #312: After deleting a row the day's `#` sequence has a gap + // (the deleted dailyNumber) and any cancelled rows that used to + // occupy slots above the deleted one are now off by one. Renumber + // 1..N by start_time so the visible list stays clean and the next + // call to nextDailyNumber() doesn't allocate the recycled slot. + await renumberDayByStartTime(tx, existing.meetingDate); }); // Realtime: every open schedule tab on the deleted meeting's day // refetches so the row disappears without a manual refresh. @@ -1927,6 +1994,11 @@ router.post( newValue: meeting, performedBy: userId, }); + // #312: A duplicate copies the source's start_time but is + // appended at the tail (nextDailyNumber). Renumber the target + // day so the duplicate slots into its correct chronological + // position — same reason POST /executive-meetings does it. + await renumberDayByStartTime(tx, data.targetDate); return meeting; }); const full = await fetchMeetingWithAttendees(inserted.id); diff --git a/artifacts/api-server/tests/executive-meetings.test.mjs b/artifacts/api-server/tests/executive-meetings.test.mjs index da750b95..35fc496e 100644 --- a/artifacts/api-server/tests/executive-meetings.test.mjs +++ b/artifacts/api-server/tests/executive-meetings.test.mjs @@ -1312,6 +1312,334 @@ test("Reorder: rejects ids that do not all belong to meetingDate", async () => { assert.equal(r.status, 400); }); +// --------------------------------------------------------------------- +// Task #312: Auto-sort the day chronologically on every write that +// affects time/order. The user reported a row showing 13:00 sitting +// above a 12:00 row after editing a single startTime — that bug exists +// because the inline-edit PATCH never re-derived `daily_number`. These +// tests lock in the new contract: any time a write touches startTime, +// endTime, meetingDate, status, or creates/duplicates/deletes a row, +// the day is renumbered 1..N by start_time so the visible list is +// strictly chronological. Cancelled rows go to the tail (preserving +// the existing renumberDayByStartTime convention) so the per-day +// unique index on (meeting_date, daily_number) stays satisfied. +// --------------------------------------------------------------------- + +// Helper: assert that the visible (non-cancelled) rows on `date` read +// 1..N by start_time order, with no chronological inversions and no +// duplicate dailyNumbers. Returns the visible rows for further checks. +async function assertDayChronological(date) { + const day = await api(adminCookie, "GET", + `/api/executive-meetings?date=${date}`); + assert.equal(day.status, 200); + const body = await day.json(); + const visible = body.meetings + .filter((m) => m.status !== "cancelled") + .sort((a, b) => a.dailyNumber - b.dailyNumber); + // dailyNumbers must be 1..N with no gaps. + for (let i = 0; i < visible.length; i++) { + assert.equal(visible[i].dailyNumber, i + 1, + `visible row index ${i} on ${date} must have dailyNumber ${i + 1}, got ${visible[i].dailyNumber}`); + } + // start_times must be non-decreasing top-to-bottom (nulls last per + // renumberDayByStartTime's NULLS LAST clause). + let lastTime = null; + let seenNull = false; + for (const m of visible) { + if (m.startTime == null) { + seenNull = true; + continue; + } + assert.ok(!seenNull, + `null-startTime row on ${date} must come after timed rows, but a timed row (${m.startTime}) followed a null one`); + const t = String(m.startTime).slice(0, 8); + if (lastTime != null) { + assert.ok(lastTime <= t, + `start_time inversion on ${date}: ${lastTime} (row N) > ${t} (row N+1)`); + } + lastTime = t; + } + // No duplicate dailyNumbers across the whole day (cancelled + visible). + const seen = new Set(); + for (const m of body.meetings) { + assert.ok(!seen.has(m.dailyNumber), + `duplicate dailyNumber ${m.dailyNumber} on ${date} after auto-sort`); + seen.add(m.dailyNumber); + } + return visible; +} + +test("Auto-sort: PATCH startTime later than next row demotes it to its new chronological slot", async () => { + const d = "2050-06-01"; + const a = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "أ", titleEn: "A", meetingDate: d, + startTime: "09:00", endTime: "09:30", + }); + const b = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ب", titleEn: "B", meetingDate: d, + startTime: "10:00", endTime: "10:30", + }); + const c = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ج", titleEn: "C", meetingDate: d, + startTime: "11:00", endTime: "11:30", + }); + const A = await a.json(); created.meetingIds.push(A.id); + const B = await b.json(); created.meetingIds.push(B.id); + const C = await c.json(); created.meetingIds.push(C.id); + await assertDayChronological(d); + + // The exact scenario from the user's screenshot: A is at the top + // (09:00). User edits A's startTime to 13:00 — without auto-sort, A + // stays in row 1 even though it now has the latest time. + const patch = await api(adminCookie, "PATCH", + `/api/executive-meetings/${A.id}`, + { startTime: "13:00", endTime: "13:30" }); + assert.equal(patch.status, 200); + + const visible = await assertDayChronological(d); + // After auto-sort the order must be B, C, A — A demoted to the tail + // because its 13:00 is now the latest start time on the day. + assert.deepEqual(visible.map((m) => m.id), [B.id, C.id, A.id], + "the row whose startTime was bumped to 13:00 must move to the bottom"); +}); + +test("Auto-sort: PATCH startTime earlier than first row promotes it to the top", async () => { + const d = "2050-06-02"; + const a = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "أ", titleEn: "A", meetingDate: d, + startTime: "09:00", endTime: "09:30", + }); + const b = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ب", titleEn: "B", meetingDate: d, + startTime: "10:00", endTime: "10:30", + }); + const c = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ج", titleEn: "C", meetingDate: d, + startTime: "11:00", endTime: "11:30", + }); + const A = await a.json(); created.meetingIds.push(A.id); + const B = await b.json(); created.meetingIds.push(B.id); + const C = await c.json(); created.meetingIds.push(C.id); + + // Move C (currently bottom @ 11:00) to 07:00 — must jump to the top. + const patch = await api(adminCookie, "PATCH", + `/api/executive-meetings/${C.id}`, + { startTime: "07:00", endTime: "07:30" }); + assert.equal(patch.status, 200); + + const visible = await assertDayChronological(d); + assert.deepEqual(visible.map((m) => m.id), [C.id, A.id, B.id], + "the row whose startTime was moved earliest must rise to the top"); +}); + +test("Auto-sort: POST create with a middle startTime slots the new row between existing ones", async () => { + const d = "2050-06-03"; + const a = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "أ", titleEn: "A", meetingDate: d, + startTime: "09:00", endTime: "09:30", + }); + const c = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ج", titleEn: "C", meetingDate: d, + startTime: "11:00", endTime: "11:30", + }); + const A = await a.json(); created.meetingIds.push(A.id); + const C = await c.json(); created.meetingIds.push(C.id); + + // Create B at 10:00 — without auto-sort it would land at the tail + // (nextDailyNumber = 3) so the visible order would be [A, C, B] even + // though chronologically it belongs in the middle. + const b = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ب", titleEn: "B", meetingDate: d, + startTime: "10:00", endTime: "10:30", + }); + assert.equal(b.status, 201); + const B = await b.json(); created.meetingIds.push(B.id); + + const visible = await assertDayChronological(d); + assert.deepEqual(visible.map((m) => m.id), [A.id, B.id, C.id], + "newly-created meeting with middle startTime must slot between existing rows"); +}); + +test("Auto-sort: clearing startTime to null pushes the row to the tail of the visible list", async () => { + const d = "2050-06-04"; + const a = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "أ", titleEn: "A", meetingDate: d, + startTime: "09:00", endTime: "09:30", + }); + const b = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ب", titleEn: "B", meetingDate: d, + startTime: "10:00", endTime: "10:30", + }); + const c = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ج", titleEn: "C", meetingDate: d, + startTime: "11:00", endTime: "11:30", + }); + const A = await a.json(); created.meetingIds.push(A.id); + const B = await b.json(); created.meetingIds.push(B.id); + const C = await c.json(); created.meetingIds.push(C.id); + + // Clear B's start/end times — renumberDayByStartTime puts NULL-time + // rows at the tail of the visible 1..N (NULLS LAST). + const patch = await api(adminCookie, "PATCH", + `/api/executive-meetings/${B.id}`, + { startTime: null, endTime: null }); + assert.equal(patch.status, 200); + + const visible = await assertDayChronological(d); + assert.deepEqual(visible.map((m) => m.id), [A.id, C.id, B.id], + "row with cleared startTime must sink below the timed rows"); + assert.equal(visible[2].startTime, null, "B's startTime must be null after clearing"); +}); + +test("Auto-sort: cancel pushes a row out of the visible list and renumbers, uncancel re-slots chronologically", async () => { + const d = "2050-06-05"; + const a = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "أ", titleEn: "A", meetingDate: d, + startTime: "09:00", endTime: "09:30", + }); + const b = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ب", titleEn: "B", meetingDate: d, + startTime: "10:00", endTime: "10:30", + }); + const c = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ج", titleEn: "C", meetingDate: d, + startTime: "11:00", endTime: "11:30", + }); + const A = await a.json(); created.meetingIds.push(A.id); + const B = await b.json(); created.meetingIds.push(B.id); + const C = await c.json(); created.meetingIds.push(C.id); + + // Cancel B via the dedicated endpoint (which already calls + // renumberDayByStartTime). The visible list shrinks to [A, C] and + // their dailyNumbers must compact to 1..2 with B sitting at the tail. + const cancel = await api(adminCookie, "POST", + `/api/executive-meetings/${B.id}/cancel`); + assert.equal(cancel.status, 200); + let visible = await assertDayChronological(d); + assert.deepEqual(visible.map((m) => m.id), [A.id, C.id], + "cancelled row must drop out of the visible list"); + + // Uncancel via PATCH status='scheduled'. B must re-enter the visible + // list at its chronological slot (between A @ 09:00 and C @ 11:00). + const uncancel = await api(adminCookie, "PATCH", + `/api/executive-meetings/${B.id}`, + { status: "scheduled" }); + assert.equal(uncancel.status, 200); + visible = await assertDayChronological(d); + assert.deepEqual(visible.map((m) => m.id), [A.id, B.id, C.id], + "uncancelled row must re-slot chronologically by its startTime"); +}); + +test("Auto-sort: PATCH meetingDate cross-day move renumbers BOTH days chronologically", async () => { + const dSrc = "2050-07-10"; + const dDst = "2050-07-11"; + const a = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "أ", titleEn: "A-src", meetingDate: dSrc, + startTime: "09:00", endTime: "09:30", + }); + const b = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ب", titleEn: "B-src", meetingDate: dSrc, + startTime: "10:00", endTime: "10:30", + }); + const c = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ج", titleEn: "C-src", meetingDate: dSrc, + startTime: "11:00", endTime: "11:30", + }); + const x = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "س", titleEn: "X-dst", meetingDate: dDst, + startTime: "08:00", endTime: "08:30", + }); + const y = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ص", titleEn: "Y-dst", meetingDate: dDst, + startTime: "13:00", endTime: "13:30", + }); + const A = await a.json(); created.meetingIds.push(A.id); + const B = await b.json(); created.meetingIds.push(B.id); + const C = await c.json(); created.meetingIds.push(C.id); + const X = await x.json(); created.meetingIds.push(X.id); + const Y = await y.json(); created.meetingIds.push(Y.id); + + // Move B from the source day to the destination day at 10:00 — it + // must arrive between X (08:00) and Y (13:00) on dDst, and the + // source day must close ranks so [A, C] reads 1..2. + const patch = await api(adminCookie, "PATCH", + `/api/executive-meetings/${B.id}`, + { meetingDate: dDst }); + assert.equal(patch.status, 200); + + const srcVisible = await assertDayChronological(dSrc); + assert.deepEqual(srcVisible.map((m) => m.id), [A.id, C.id], + "source day must compact to [A, C] after B moves away"); + const dstVisible = await assertDayChronological(dDst); + assert.deepEqual(dstVisible.map((m) => m.id), [X.id, B.id, Y.id], + "destination day must accept B at its chronological slot between X and Y"); +}); + +test("Auto-sort: DELETE leaves the day's `#` sequence gap-free and chronological", async () => { + const d = "2050-06-15"; + const a = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "أ", titleEn: "A", meetingDate: d, + startTime: "09:00", endTime: "09:30", + }); + const b = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ب", titleEn: "B", meetingDate: d, + startTime: "10:00", endTime: "10:30", + }); + const c = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ج", titleEn: "C", meetingDate: d, + startTime: "11:00", endTime: "11:30", + }); + const A = await a.json(); created.meetingIds.push(A.id); + const B = await b.json(); created.meetingIds.push(B.id); + const C = await c.json(); created.meetingIds.push(C.id); + + // Delete the middle row. Without auto-sort, dailyNumbers would be + // [1, 3] with a gap, and a future POST would reuse dailyNumber=2. + const del = await api(adminCookie, "DELETE", + `/api/executive-meetings/${B.id}`); + assert.ok(del.status === 200 || del.status === 204); + // Drop B from cleanup since it's already gone. + created.meetingIds = created.meetingIds.filter((x) => x !== B.id); + + const visible = await assertDayChronological(d); + assert.deepEqual(visible.map((m) => m.id), [A.id, C.id], + "remaining rows must compact to 1..2 with no gap after delete"); +}); + +test("Auto-sort: POST /duplicate slots the cloned meeting at its chronological position on the target day", async () => { + const dSrc = "2050-06-20"; + const dDst = "2050-06-21"; + const a = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "أ", titleEn: "A-src", meetingDate: dSrc, + startTime: "10:00", endTime: "10:30", + }); + const x = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "س", titleEn: "X-dst", meetingDate: dDst, + startTime: "08:00", endTime: "08:30", + }); + const y = await api(adminCookie, "POST", "/api/executive-meetings", { + titleAr: "ص", titleEn: "Y-dst", meetingDate: dDst, + startTime: "13:00", endTime: "13:30", + }); + const A = await a.json(); created.meetingIds.push(A.id); + const X = await x.json(); created.meetingIds.push(X.id); + const Y = await y.json(); created.meetingIds.push(Y.id); + + // Duplicate A (10:00) onto dDst — the clone must land between X + // (08:00) and Y (13:00). Without auto-sort, the duplicate would be + // appended at the tail (dailyNumber = 3) and the visible order would + // be [X, Y, clone] even though clone @ 10:00 belongs in the middle. + const dup = await api(adminCookie, "POST", + `/api/executive-meetings/${A.id}/duplicate`, + { targetDate: dDst }); + assert.equal(dup.status, 201); + const Clone = await dup.json(); created.meetingIds.push(Clone.id); + + const visible = await assertDayChronological(dDst); + assert.deepEqual(visible.map((m) => m.id), [X.id, Clone.id, Y.id], + "duplicate must slot at its chronological position on the target day"); +}); + test("Reorder: user without executive role is denied (403)", async () => { const m = await api(adminCookie, "POST", "/api/executive-meetings", { titleAr: "ا", titleEn: "A", meetingDate: today,