rafraa/TX
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Task #312: auto-sort executive-meetings schedule by start_time

Browse Source 
User report (AR): "اريدها ترتيبها تلقائيا" — after a non-drag write
(typing 13:00 into a row sitting above a 12:00 row, creating a meeting
with an explicit start time, etc.) the schedule's daily_number stayed
frozen at the row's prior position, so the visible list was no longer
chronological. Fix: extend the existing `renumberDayByStartTime` helper
to every write path that touches start_time / meeting_date / status /
daily_number, so the day is always renumbered 1..N by start_time
(NULLS LAST, cancelled rows at the tail).

Server (artifacts/api-server/src/routes/executive-meetings.ts):
  - POST  /executive-meetings           — renumber after insert.
  - PATCH /executive-meetings/:id       — renumber when an order- or
    visibility-affecting field (startTime/endTime/meetingDate/status/
    dailyNumber) is in the payload. Cross-day moves renumber BOTH the
    source and destination day. Pre-allocates a fresh dailyNumber on
    the destination via nextDailyNumber(tx, newDate) before the UPDATE
    so the (meeting_date, daily_number) unique index does not collide
    when the row arrives on a day with existing meetings.
  - DELETE /executive-meetings/:id      — renumber so the day's `#`
    sequence stays gap-free.
  - POST  /executive-meetings/:id/duplicate — renumber the target day.
Existing postpone-minutes / reschedule / cancel paths already called
renumber and were left alone. Reorder POST is also untouched (its
slot-swap is the explicit user-driven order, not auto-sort).

Tests (artifacts/api-server/tests/executive-meetings.test.mjs, +9):
  - assertDayChronological() helper asserts 1..N + non-decreasing
    start_time + no duplicate dailyNumbers.
  - PATCH startTime later → row demoted (the user's exact scenario).
  - PATCH startTime earlier → row promoted to the top.
  - POST create with middle startTime slots between existing rows.
  - PATCH startTime=null sinks to the tail of visible rows.
  - Cancel pushes out / uncancel re-slots chronologically.
  - Cross-day PATCH meetingDate renumbers BOTH days.
  - DELETE leaves no `#` gap.
  - POST /duplicate slots clone at chronological position.
All 55 tests in executive-meetings.test.mjs pass.

Architect review (advisory, scope-bounded):
  - Concurrency: PATCH/DELETE read `existing` outside the transaction.
    Pre-existing convention in this file — only the cascade-bearing
    postpone-minutes/reschedule paths use SELECT FOR UPDATE. Matching
    existing pattern; tightening locking is a separate refactor.
  - Audit: renumber side-effect not echoed back into the audit row.
    Per task plan (`.local/tasks/auto-sort-schedule-by-time.md`),
    audit shape was intentionally kept minimal — the user-intent
    fields already capture the change.
This commit is contained in:
riyadhafraa
2026-05-02 09:47:12 +00:00
parent f7b93aa4e7
commit 0a7acd683f
2 changed files with 400 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
artifacts/api-server/src/routes/executive-meetings.ts
+72
View File
@@ -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);
artifacts/api-server/tests/executive-meetings.test.mjs
+328
View File
@@ -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,