Add conflict detection and resolution for meeting postponements
Implement optimistic locking for meeting postponements to prevent concurrent edits. The server now requires an `updatedAt` token for postpone requests, returning a 409 conflict error if the meeting has been modified since the token was issued. The client displays a user-friendly prompt allowing users to reapply changes with the latest token.
This commit is contained in:
@@ -794,6 +794,15 @@ const alertActionSchema = z.object({
|
|||||||
});
|
});
|
||||||
const postponeMinutesSchema = z.object({
|
const postponeMinutesSchema = z.object({
|
||||||
minutes: z.number().int().min(1).max(24 * 60),
|
minutes: z.number().int().min(1).max(24 * 60),
|
||||||
|
// #283: optimistic-locking token. The client must send the
|
||||||
|
// `updatedAt` it last observed for this meeting; if another user
|
||||||
|
// (or the same user from another tab) mutated the meeting in
|
||||||
|
// between, we reject with HTTP 409 + stale_meeting so the UI can
|
||||||
|
// surface "already postponed by X" instead of double-shifting.
|
||||||
|
// Required by design — there is no opt-out path. To "apply anyway"
|
||||||
|
// after a conflict, the client refetches (or reuses
|
||||||
|
// conflict.lastModifiedAt) and resubmits with the fresher token.
|
||||||
|
expectedUpdatedAt: z.string().datetime(),
|
||||||
});
|
});
|
||||||
const rescheduleSchema = z.object({
|
const rescheduleSchema = z.object({
|
||||||
meetingDate: dateSchema,
|
meetingDate: dateSchema,
|
||||||
@@ -997,6 +1006,24 @@ router.post(
|
|||||||
// #273: lock-then-mutate inside a single transaction so concurrent
|
// #273: lock-then-mutate inside a single transaction so concurrent
|
||||||
// postpone/reschedule/cancel requests serialize and audit logs reflect
|
// postpone/reschedule/cancel requests serialize and audit logs reflect
|
||||||
// the actual committed transition.
|
// the actual committed transition.
|
||||||
|
type StaleConflict = {
|
||||||
|
ok: false;
|
||||||
|
status: 409;
|
||||||
|
error: string;
|
||||||
|
code: "stale_meeting";
|
||||||
|
conflict: {
|
||||||
|
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 =
|
type TxResult =
|
||||||
| {
|
| {
|
||||||
ok: true;
|
ok: true;
|
||||||
@@ -1005,7 +1032,8 @@ router.post(
|
|||||||
newEnd: string;
|
newEnd: string;
|
||||||
conflicts: boolean;
|
conflicts: boolean;
|
||||||
}
|
}
|
||||||
| { ok: false; status: number; error: string; code: string };
|
| { ok: false; status: number; error: string; code: string }
|
||||||
|
| StaleConflict;
|
||||||
const txResult = await db.transaction(async (tx): Promise<TxResult> => {
|
const txResult = await db.transaction(async (tx): Promise<TxResult> => {
|
||||||
const [locked] = await tx
|
const [locked] = await tx
|
||||||
.select()
|
.select()
|
||||||
@@ -1015,6 +1043,48 @@ router.post(
|
|||||||
if (!locked) {
|
if (!locked) {
|
||||||
return { ok: false, status: 404, error: "Meeting not found", code: "not_found" };
|
return { ok: false, status: 404, error: "Meeting not found", code: "not_found" };
|
||||||
}
|
}
|
||||||
|
// #283: Optimistic-locking guard. The client always sends the
|
||||||
|
// `updatedAt` it last observed; if the row has been modified
|
||||||
|
// since, refuse to mutate and return the current state + the
|
||||||
|
// last actor's display info so the UI can show
|
||||||
|
// "Already postponed by Ahmed at 10:05 — apply N more minutes?"
|
||||||
|
// We do this *inside* the FOR UPDATE lock so concurrent
|
||||||
|
// postpone-minutes calls are serialized: the second one sees the
|
||||||
|
// first one's committed update and returns 409. There is no
|
||||||
|
// opt-out — to apply after a conflict, the client must resubmit
|
||||||
|
// with the freshly-seen `lastModifiedAt`, so a third concurrent
|
||||||
|
// mutation between conflict-display and apply-anyway will trigger
|
||||||
|
// another 409 instead of silently stacking.
|
||||||
|
const observedIso = new Date(body.expectedUpdatedAt).toISOString();
|
||||||
|
const currentIso = locked.updatedAt.toISOString();
|
||||||
|
if (observedIso !== currentIso) {
|
||||||
|
let lastActor: StaleConflict["conflict"]["lastActor"] = null;
|
||||||
|
if (locked.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, locked.updatedBy));
|
||||||
|
lastActor = u ?? null;
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
status: 409,
|
||||||
|
error: "Meeting was modified by someone else",
|
||||||
|
code: "stale_meeting",
|
||||||
|
conflict: {
|
||||||
|
currentStartTime: locked.startTime,
|
||||||
|
currentEndTime: locked.endTime,
|
||||||
|
currentStatus: locked.status,
|
||||||
|
lastModifiedAt: currentIso,
|
||||||
|
lastActor,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
const startMin = parseTimeToMinutes(locked.startTime);
|
const startMin = parseTimeToMinutes(locked.startTime);
|
||||||
const endMin = parseTimeToMinutes(locked.endTime);
|
const endMin = parseTimeToMinutes(locked.endTime);
|
||||||
if (startMin == null || endMin == null) {
|
if (startMin == null || endMin == null) {
|
||||||
@@ -1043,7 +1113,14 @@ router.post(
|
|||||||
const newEnd = formatMinutesAsTime(endMin + body.minutes);
|
const newEnd = formatMinutesAsTime(endMin + body.minutes);
|
||||||
await tx
|
await tx
|
||||||
.update(executiveMeetingsTable)
|
.update(executiveMeetingsTable)
|
||||||
.set({ startTime: newStart, endTime: newEnd, status: "postponed" })
|
.set({
|
||||||
|
startTime: newStart,
|
||||||
|
endTime: newEnd,
|
||||||
|
status: "postponed",
|
||||||
|
// #283: stamp the actor so the next stale_meeting reply can
|
||||||
|
// attribute "postponed by X" to a real user.
|
||||||
|
updatedBy: userId,
|
||||||
|
})
|
||||||
.where(eq(executiveMeetingsTable.id, id));
|
.where(eq(executiveMeetingsTable.id, id));
|
||||||
await logAudit(tx, {
|
await logAudit(tx, {
|
||||||
action: "meeting_postponed_minutes",
|
action: "meeting_postponed_minutes",
|
||||||
@@ -1080,7 +1157,18 @@ router.post(
|
|||||||
};
|
};
|
||||||
});
|
});
|
||||||
if (!txResult.ok) {
|
if (!txResult.ok) {
|
||||||
res.status(txResult.status).json({ error: txResult.error, code: txResult.code });
|
// #283: Stale-conflict responses carry an extra `conflict` payload
|
||||||
|
// (current state + last actor display info) so the UI can render
|
||||||
|
// a "tried to postpone but X already postponed it to 10:05" prompt
|
||||||
|
// instead of the generic toast.
|
||||||
|
const payload: Record<string, unknown> = {
|
||||||
|
error: txResult.error,
|
||||||
|
code: txResult.code,
|
||||||
|
};
|
||||||
|
if (txResult.code === "stale_meeting" && "conflict" in txResult) {
|
||||||
|
payload.conflict = txResult.conflict;
|
||||||
|
}
|
||||||
|
res.status(txResult.status).json(payload);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
void emitExecutiveMeetingsDaysChanged([txResult.meetingDate]);
|
void emitExecutiveMeetingsDaysChanged([txResult.meetingDate]);
|
||||||
|
|||||||
@@ -0,0 +1,295 @@
|
|||||||
|
// #283: Concurrent-postpone race test.
|
||||||
|
// When two users postpone the same meeting at roughly the same moment,
|
||||||
|
// the second writer must NOT silently stack their N minutes on top of
|
||||||
|
// the first writer's already-shifted time. Instead, the server should
|
||||||
|
// return 409 stale_meeting with a `conflict` payload identifying who
|
||||||
|
// won the race so the UI can surface a confirmation prompt.
|
||||||
|
//
|
||||||
|
// We exercise three scenarios:
|
||||||
|
// 1. A → 200 (first wins).
|
||||||
|
// 2. B with the *original* updatedAt → 409 stale_meeting + conflict
|
||||||
|
// payload that names A as the lastActor.
|
||||||
|
// 3. B with the *fresh* updatedAt (re-fetched after losing the race)
|
||||||
|
// → 200; the meeting now reflects A's + B's shifts in order, and
|
||||||
|
// we can also confirm the "force apply anyway" path by sending
|
||||||
|
// no expectedUpdatedAt at all.
|
||||||
|
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 () => {
|
||||||
|
// executive_coord_lead carries the mutate role required by the
|
||||||
|
// postpone/reschedule endpoints — see MUTATE_ROLES in the route.
|
||||||
|
userA = await createUser("em_race_a", "executive_coord_lead", "Alice");
|
||||||
|
cookieA = await login(userA.username, TEST_PASSWORD);
|
||||||
|
userB = await createUser("em_race_b", "executive_coord_lead", "Bob");
|
||||||
|
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();
|
||||||
|
});
|
||||||
|
|
||||||
|
const today = new Date().toISOString().slice(0, 10);
|
||||||
|
|
||||||
|
async function createMeeting() {
|
||||||
|
// Create through the API so updatedAt and updatedBy are set the same
|
||||||
|
// way real meetings are. Use a fixed afternoon window so we don't
|
||||||
|
// race the alert loop or the noTimeWindow guard.
|
||||||
|
const res = await api(cookieA, "POST", "/api/executive-meetings", {
|
||||||
|
titleAr: "اجتماع التزامن",
|
||||||
|
titleEn: "Concurrency meeting",
|
||||||
|
meetingDate: today,
|
||||||
|
startTime: "14:00",
|
||||||
|
endTime: "14:30",
|
||||||
|
status: "scheduled",
|
||||||
|
});
|
||||||
|
assert.ok(
|
||||||
|
res.status === 200 || res.status === 201,
|
||||||
|
`meeting create should succeed (got ${res.status})`,
|
||||||
|
);
|
||||||
|
const body = await res.json();
|
||||||
|
const meeting = body.meeting ?? body;
|
||||||
|
created.meetingIds.push(meeting.id);
|
||||||
|
return meeting;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function fetchMeeting(cookie, id) {
|
||||||
|
// /api/executive-meetings/days serves the same shape the alert UI
|
||||||
|
// sees, but the simpler GET-by-id endpoint is enough here.
|
||||||
|
const res = await api(cookie, "GET", `/api/executive-meetings/${id}`);
|
||||||
|
assert.equal(res.status, 200, "meeting fetch should succeed");
|
||||||
|
return res.json();
|
||||||
|
}
|
||||||
|
|
||||||
|
test("postpone-minutes: second concurrent writer hits 409 stale_meeting", async () => {
|
||||||
|
const meeting = await createMeeting();
|
||||||
|
// Both clients capture the same updatedAt — this is the race window.
|
||||||
|
const initial = await fetchMeeting(cookieA, meeting.id);
|
||||||
|
const staleUpdatedAt = initial.updatedAt;
|
||||||
|
assert.ok(staleUpdatedAt, "meeting payload must include updatedAt");
|
||||||
|
|
||||||
|
// A wins.
|
||||||
|
const aRes = await api(
|
||||||
|
cookieA,
|
||||||
|
"POST",
|
||||||
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
|
{ minutes: 5, expectedUpdatedAt: staleUpdatedAt },
|
||||||
|
);
|
||||||
|
assert.equal(aRes.status, 200, "first postpone should succeed");
|
||||||
|
const aBody = await aRes.json();
|
||||||
|
assert.equal(aBody.meeting.startTime.slice(0, 5), "14:05");
|
||||||
|
|
||||||
|
// B loses — sends the now-stale token.
|
||||||
|
const bRes = await api(
|
||||||
|
cookieB,
|
||||||
|
"POST",
|
||||||
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
|
{ minutes: 5, expectedUpdatedAt: staleUpdatedAt },
|
||||||
|
);
|
||||||
|
assert.equal(bRes.status, 409, "second postpone should be rejected as stale");
|
||||||
|
const bBody = await bRes.json();
|
||||||
|
assert.equal(bBody.code, "stale_meeting");
|
||||||
|
assert.ok(bBody.conflict, "stale response must include conflict payload");
|
||||||
|
assert.equal(bBody.conflict.currentStartTime?.slice(0, 5), "14:05");
|
||||||
|
assert.ok(bBody.conflict.lastActor, "conflict must name the last actor");
|
||||||
|
assert.equal(bBody.conflict.lastActor.username, userA.username);
|
||||||
|
assert.equal(bBody.conflict.lastActor.displayNameEn, "Alice");
|
||||||
|
|
||||||
|
// Confirm the start time did NOT silently shift again.
|
||||||
|
const afterB = await fetchMeeting(cookieA, meeting.id);
|
||||||
|
assert.equal(
|
||||||
|
afterB.startTime.slice(0, 5),
|
||||||
|
"14:05",
|
||||||
|
"B's stale call must not have stacked another shift",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("postpone-minutes: 'apply anyway' uses freshly-seen token from conflict", async () => {
|
||||||
|
const meeting = await createMeeting();
|
||||||
|
|
||||||
|
// A wins.
|
||||||
|
const aRes = await api(
|
||||||
|
cookieA,
|
||||||
|
"POST",
|
||||||
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
|
{ minutes: 10, expectedUpdatedAt: meeting.updatedAt },
|
||||||
|
);
|
||||||
|
assert.equal(aRes.status, 200);
|
||||||
|
const aBody = await aRes.json();
|
||||||
|
assert.equal(aBody.meeting.startTime.slice(0, 5), "14:10");
|
||||||
|
|
||||||
|
// B sees a 409 with conflict.lastModifiedAt naming the new state…
|
||||||
|
const conflictRes = await api(
|
||||||
|
cookieB,
|
||||||
|
"POST",
|
||||||
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
|
{ minutes: 5, expectedUpdatedAt: meeting.updatedAt },
|
||||||
|
);
|
||||||
|
assert.equal(conflictRes.status, 409);
|
||||||
|
const conflictBody = await conflictRes.json();
|
||||||
|
const freshToken = conflictBody.conflict.lastModifiedAt;
|
||||||
|
assert.ok(freshToken, "conflict must include lastModifiedAt for retry");
|
||||||
|
|
||||||
|
// …and "apply anyway" mirrors the client by resubmitting with that
|
||||||
|
// freshly-seen token. A third concurrent mutation between conflict
|
||||||
|
// display and click would return 409 again — proving no silent stack.
|
||||||
|
const retryRes = await api(
|
||||||
|
cookieB,
|
||||||
|
"POST",
|
||||||
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
|
{ minutes: 5, expectedUpdatedAt: freshToken },
|
||||||
|
);
|
||||||
|
assert.equal(retryRes.status, 200, "apply-anyway with fresh token succeeds");
|
||||||
|
const retryBody = await retryRes.json();
|
||||||
|
assert.equal(
|
||||||
|
retryBody.meeting.startTime.slice(0, 5),
|
||||||
|
"14:15",
|
||||||
|
"apply-anyway shifts on top of A's already-shifted state (10 + 5)",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("postpone-minutes: missing expectedUpdatedAt is a 400 (required by design)", async () => {
|
||||||
|
const meeting = await createMeeting();
|
||||||
|
const res = await api(
|
||||||
|
cookieA,
|
||||||
|
"POST",
|
||||||
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
|
{ minutes: 5 },
|
||||||
|
);
|
||||||
|
assert.equal(
|
||||||
|
res.status,
|
||||||
|
400,
|
||||||
|
"the optimistic-locking token is mandatory — no opt-out path",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("postpone-minutes: B refetches and retries cleanly with fresh updatedAt", async () => {
|
||||||
|
const meeting = await createMeeting();
|
||||||
|
|
||||||
|
// A shifts by 5.
|
||||||
|
await api(
|
||||||
|
cookieA,
|
||||||
|
"POST",
|
||||||
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
|
{ minutes: 5, expectedUpdatedAt: meeting.updatedAt },
|
||||||
|
);
|
||||||
|
|
||||||
|
// B refetches → gets the new updatedAt → retries → 200.
|
||||||
|
const refreshed = await fetchMeeting(cookieB, meeting.id);
|
||||||
|
const retryRes = await api(
|
||||||
|
cookieB,
|
||||||
|
"POST",
|
||||||
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
|
{ minutes: 5, expectedUpdatedAt: refreshed.updatedAt },
|
||||||
|
);
|
||||||
|
assert.equal(retryRes.status, 200);
|
||||||
|
const retryBody = await retryRes.json();
|
||||||
|
assert.equal(retryBody.meeting.startTime.slice(0, 5), "14:10");
|
||||||
|
});
|
||||||
@@ -45,6 +45,10 @@ type Meeting = {
|
|||||||
location: string | null;
|
location: string | null;
|
||||||
meetingUrl: string | null;
|
meetingUrl: string | null;
|
||||||
status: string;
|
status: string;
|
||||||
|
// #283: optimistic-locking token used by the postpone flow to detect
|
||||||
|
// when another user mutated this meeting after it was loaded. Server
|
||||||
|
// sets it on every update; client echoes it back as expectedUpdatedAt.
|
||||||
|
updatedAt: string;
|
||||||
};
|
};
|
||||||
type DayResponse = { date: string; meetings: Meeting[] };
|
type DayResponse = { date: string; meetings: Meeting[] };
|
||||||
type AlertState = {
|
type AlertState = {
|
||||||
@@ -84,6 +88,23 @@ function formatTime(t: string | null): string {
|
|||||||
return `${m[1].padStart(2, "0")}:${m[2]}`;
|
return `${m[1].padStart(2, "0")}:${m[2]}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// #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<string, unknown>;
|
||||||
|
constructor(message: string, status: number, body: Record<string, unknown>) {
|
||||||
|
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<T>(input: string, init?: RequestInit): Promise<T> {
|
async function apiJson<T>(input: string, init?: RequestInit): Promise<T> {
|
||||||
const res = await fetch(input, {
|
const res = await fetch(input, {
|
||||||
credentials: "include",
|
credentials: "include",
|
||||||
@@ -94,9 +115,10 @@ async function apiJson<T>(input: string, init?: RequestInit): Promise<T> {
|
|||||||
const text = await res.text();
|
const text = await res.text();
|
||||||
const data = text ? (JSON.parse(text) as unknown) : ({} as unknown);
|
const data = text ? (JSON.parse(text) as unknown) : ({} as unknown);
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
throw new Error(
|
const body = (data ?? {}) as Record<string, unknown>;
|
||||||
(data as { error?: string }).error || `HTTP ${res.status}`,
|
const message =
|
||||||
);
|
(body.error as string | undefined) || `HTTP ${res.status}`;
|
||||||
|
throw new ApiError(message, res.status, body);
|
||||||
}
|
}
|
||||||
return data as T;
|
return data as T;
|
||||||
}
|
}
|
||||||
@@ -593,6 +615,22 @@ function PostponeDialog({
|
|||||||
// before the API call fires. While it's non-null, the dialog shows
|
// before the API call fires. While it's non-null, the dialog shows
|
||||||
// an "Are you sure? — Confirm / Back" prompt instead of executing.
|
// an "Are you sure? — Confirm / Back" prompt instead of executing.
|
||||||
const [pendingMinutes, setPendingMinutes] = useState<number | null>(null);
|
const [pendingMinutes, setPendingMinutes] = useState<number | null>(null);
|
||||||
|
// #283: When the server returns 409 stale_meeting (someone else
|
||||||
|
// postponed/rescheduled this meeting between load and click), we
|
||||||
|
// capture the conflict payload here and render an explicit
|
||||||
|
// "Already postponed by X — apply N more minutes anyway?" prompt
|
||||||
|
// instead of silently double-shifting the meeting. `latestUpdatedAt`
|
||||||
|
// is the freshest server-observed token; the apply-anyway retry
|
||||||
|
// resubmits with it so a *third* concurrent mutation between
|
||||||
|
// conflict-display and click triggers another 409 instead of a
|
||||||
|
// silent stack.
|
||||||
|
const [staleConflict, setStaleConflict] = useState<{
|
||||||
|
minutes: number;
|
||||||
|
actorName: string | null;
|
||||||
|
currentStart: string | null;
|
||||||
|
currentEnd: string | null;
|
||||||
|
latestUpdatedAt: string;
|
||||||
|
} | null>(null);
|
||||||
|
|
||||||
// Reset local form state whenever the dialog opens against a new meeting
|
// Reset local form state whenever the dialog opens against a new meeting
|
||||||
// so stale values don't leak between alerts.
|
// so stale values don't leak between alerts.
|
||||||
@@ -607,6 +645,7 @@ function PostponeDialog({
|
|||||||
setConfirmCancel(false);
|
setConfirmCancel(false);
|
||||||
setActiveTab("minutes");
|
setActiveTab("minutes");
|
||||||
setPendingMinutes(null);
|
setPendingMinutes(null);
|
||||||
|
setStaleConflict(null);
|
||||||
}, [open, meeting.id, meeting.meetingDate, meeting.startTime, meeting.endTime]);
|
}, [open, meeting.id, meeting.meetingDate, meeting.startTime, meeting.endTime]);
|
||||||
|
|
||||||
const title = isRtl
|
const title = isRtl
|
||||||
@@ -639,21 +678,82 @@ function PostponeDialog({
|
|||||||
|
|
||||||
// #273: shared mutation used by both the manual "Apply" button and
|
// #273: shared mutation used by both the manual "Apply" button and
|
||||||
// each minute chip click so the chips can shift start/end immediately.
|
// each minute chip click so the chips can shift start/end immediately.
|
||||||
const postponeBy = async (delta: number) => {
|
// #283: every call sends `expectedUpdatedAt`. The first attempt uses
|
||||||
|
// the token from the loaded meeting; the "apply anyway" retry passes
|
||||||
|
// the freshly-seen token from the conflict payload so a third
|
||||||
|
// concurrent mutation between conflict-display and click will trigger
|
||||||
|
// another 409 instead of silently stacking.
|
||||||
|
const postponeBy = async (delta: number, expectedUpdatedAt?: string) => {
|
||||||
if (!Number.isFinite(delta) || delta <= 0) return;
|
if (!Number.isFinite(delta) || delta <= 0) return;
|
||||||
|
const token = expectedUpdatedAt ?? meeting.updatedAt;
|
||||||
|
if (!token) {
|
||||||
|
// Defensive: should never happen — meeting payload always includes
|
||||||
|
// updatedAt. If it ever is missing, fail loudly rather than send
|
||||||
|
// a request the server will reject as malformed.
|
||||||
|
toast({
|
||||||
|
title: t("executiveMeetings.alert.saveFailed"),
|
||||||
|
description: "Missing updatedAt token",
|
||||||
|
variant: "destructive",
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
setBusy("minutes");
|
setBusy("minutes");
|
||||||
try {
|
try {
|
||||||
const res = await apiJson<{ conflicts?: boolean }>(
|
const res = await apiJson<{ conflicts?: boolean }>(
|
||||||
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
`/api/executive-meetings/${meeting.id}/postpone-minutes`,
|
||||||
{
|
{
|
||||||
method: "POST",
|
method: "POST",
|
||||||
body: JSON.stringify({ minutes: Math.floor(delta) }),
|
body: JSON.stringify({
|
||||||
|
minutes: Math.floor(delta),
|
||||||
|
expectedUpdatedAt: token,
|
||||||
|
}),
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
handleResult(res, "executiveMeetings.alert.saved");
|
handleResult(res, "executiveMeetings.alert.saved");
|
||||||
|
setStaleConflict(null);
|
||||||
await onSaved();
|
await onSaved();
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
handleErr(err);
|
// #283: 409 stale_meeting → don't error-toast; capture the
|
||||||
|
// conflict so the dialog can show the "already postponed by X"
|
||||||
|
// prompt with the option to apply N minutes anyway. We carry
|
||||||
|
// `lastModifiedAt` forward so the retry uses the freshly-seen
|
||||||
|
// token, not the original one.
|
||||||
|
if (err instanceof ApiError && err.code === "stale_meeting") {
|
||||||
|
const conflict = (err.body as {
|
||||||
|
conflict?: {
|
||||||
|
currentStartTime: string | null;
|
||||||
|
currentEndTime: string | null;
|
||||||
|
lastModifiedAt: string;
|
||||||
|
lastActor: {
|
||||||
|
displayNameAr: string | null;
|
||||||
|
displayNameEn: string | null;
|
||||||
|
username: string | null;
|
||||||
|
} | null;
|
||||||
|
};
|
||||||
|
}).conflict;
|
||||||
|
if (!conflict) {
|
||||||
|
handleErr(err);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const actor = conflict.lastActor ?? null;
|
||||||
|
const actorName = actor
|
||||||
|
? (isRtl
|
||||||
|
? actor.displayNameAr || actor.displayNameEn || actor.username
|
||||||
|
: actor.displayNameEn || actor.displayNameAr || actor.username)
|
||||||
|
: null;
|
||||||
|
setStaleConflict({
|
||||||
|
minutes: Math.floor(delta),
|
||||||
|
actorName: actorName ?? null,
|
||||||
|
currentStart: conflict.currentStartTime ?? null,
|
||||||
|
currentEnd: conflict.currentEndTime ?? null,
|
||||||
|
latestUpdatedAt: conflict.lastModifiedAt,
|
||||||
|
});
|
||||||
|
// Clear the "are you sure?" prompt — we're replacing it with
|
||||||
|
// the conflict-aware prompt rendered below.
|
||||||
|
setPendingMinutes(null);
|
||||||
|
} else {
|
||||||
|
handleErr(err);
|
||||||
|
}
|
||||||
} finally {
|
} finally {
|
||||||
setBusy(null);
|
setBusy(null);
|
||||||
}
|
}
|
||||||
@@ -815,7 +915,64 @@ function PostponeDialog({
|
|||||||
? t("executiveMeetings.alert.noTimeWindow")
|
? t("executiveMeetings.alert.noTimeWindow")
|
||||||
: t("executiveMeetings.alert.postponeMinutesHint")}
|
: t("executiveMeetings.alert.postponeMinutesHint")}
|
||||||
</p>
|
</p>
|
||||||
{pendingMinutes == null ? (
|
{staleConflict ? (
|
||||||
|
<div
|
||||||
|
className="flex flex-col gap-2 rounded border border-rose-300 bg-rose-50 p-3 dark:border-rose-700 dark:bg-rose-950"
|
||||||
|
data-testid="postpone-stale-block"
|
||||||
|
>
|
||||||
|
<p className="text-sm font-medium">
|
||||||
|
{staleConflict.actorName
|
||||||
|
? t("executiveMeetings.alert.staleMeetingByUser", {
|
||||||
|
name: staleConflict.actorName,
|
||||||
|
})
|
||||||
|
: t("executiveMeetings.alert.staleMeetingTitle")}
|
||||||
|
</p>
|
||||||
|
{staleConflict.currentStart ? (
|
||||||
|
<p className="text-xs text-muted-foreground">
|
||||||
|
{t("executiveMeetings.alert.staleMeetingCurrentTime", {
|
||||||
|
start: formatTime(staleConflict.currentStart),
|
||||||
|
end: staleConflict.currentEnd
|
||||||
|
? formatTime(staleConflict.currentEnd)
|
||||||
|
: "—",
|
||||||
|
})}
|
||||||
|
</p>
|
||||||
|
) : null}
|
||||||
|
<div className="flex items-center gap-2">
|
||||||
|
<Button
|
||||||
|
type="button"
|
||||||
|
size="sm"
|
||||||
|
variant="destructive"
|
||||||
|
onClick={() => {
|
||||||
|
const n = staleConflict.minutes;
|
||||||
|
const fresh = staleConflict.latestUpdatedAt;
|
||||||
|
setStaleConflict(null);
|
||||||
|
// #283: pass the freshly-seen token; if the
|
||||||
|
// meeting was changed *again* between conflict
|
||||||
|
// display and click, the server will return 409
|
||||||
|
// a second time so the user re-confirms against
|
||||||
|
// the newer state.
|
||||||
|
void postponeBy(n, fresh);
|
||||||
|
}}
|
||||||
|
disabled={busy !== null}
|
||||||
|
data-testid="postpone-stale-apply-anyway"
|
||||||
|
>
|
||||||
|
{t("executiveMeetings.alert.staleMeetingApplyAnyway", {
|
||||||
|
n: staleConflict.minutes,
|
||||||
|
})}
|
||||||
|
</Button>
|
||||||
|
<Button
|
||||||
|
type="button"
|
||||||
|
size="sm"
|
||||||
|
variant="outline"
|
||||||
|
onClick={() => setStaleConflict(null)}
|
||||||
|
disabled={busy !== null}
|
||||||
|
data-testid="postpone-stale-cancel"
|
||||||
|
>
|
||||||
|
{t("executiveMeetings.alert.back")}
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
) : pendingMinutes == null ? (
|
||||||
<>
|
<>
|
||||||
<div className="flex flex-wrap items-center gap-1">
|
<div className="flex flex-wrap items-center gap-1">
|
||||||
{[5, 10, 15, 30, 45, 60].map((n) => (
|
{[5, 10, 15, 30, 45, 60].map((n) => (
|
||||||
|
|||||||
@@ -1060,7 +1060,11 @@
|
|||||||
"saved": "تم الحفظ",
|
"saved": "تم الحفظ",
|
||||||
"saveFailed": "تعذّر حفظ التغييرات",
|
"saveFailed": "تعذّر حفظ التغييرات",
|
||||||
"conflictWarning": "تنبيه: الوقت الجديد يتداخل مع اجتماع آخر في نفس اليوم.",
|
"conflictWarning": "تنبيه: الوقت الجديد يتداخل مع اجتماع آخر في نفس اليوم.",
|
||||||
"noTimeWindow": "هذا الاجتماع بدون وقت محدد، لا يمكن تأجيله بدقائق."
|
"noTimeWindow": "هذا الاجتماع بدون وقت محدد، لا يمكن تأجيله بدقائق.",
|
||||||
|
"staleMeetingTitle": "تم تعديل هذا الاجتماع للتو من قبل مستخدم آخر.",
|
||||||
|
"staleMeetingByUser": "{{name}} عدّل هذا الاجتماع للتو.",
|
||||||
|
"staleMeetingCurrentTime": "الوقت الحالي: {{start}} – {{end}}.",
|
||||||
|
"staleMeetingApplyAnyway": "أضف {{n}} دقيقة إضافية على أي حال"
|
||||||
},
|
},
|
||||||
"schedule": {
|
"schedule": {
|
||||||
"addRow": "أضف اجتماع",
|
"addRow": "أضف اجتماع",
|
||||||
|
|||||||
@@ -978,7 +978,11 @@
|
|||||||
"saved": "Saved",
|
"saved": "Saved",
|
||||||
"saveFailed": "Could not save changes",
|
"saveFailed": "Could not save changes",
|
||||||
"conflictWarning": "Heads up: the new time overlaps another meeting on the same day.",
|
"conflictWarning": "Heads up: the new time overlaps another meeting on the same day.",
|
||||||
"noTimeWindow": "This meeting has no scheduled time, so it can’t be postponed by minutes."
|
"noTimeWindow": "This meeting has no scheduled time, so it can’t be postponed by minutes.",
|
||||||
|
"staleMeetingTitle": "This meeting was just changed by another user.",
|
||||||
|
"staleMeetingByUser": "{{name}} just changed this meeting.",
|
||||||
|
"staleMeetingCurrentTime": "Current time: {{start}} – {{end}}.",
|
||||||
|
"staleMeetingApplyAnyway": "Add {{n}} more minutes anyway"
|
||||||
},
|
},
|
||||||
"schedule": {
|
"schedule": {
|
||||||
"addRow": "Add meeting",
|
"addRow": "Add meeting",
|
||||||
|
|||||||
Reference in New Issue
Block a user