Improve meeting reminders with new scheduler and improved push notifications
Add a new background scheduler to trigger meeting reminders, implement atomic claims for push notifications to prevent duplicates, and add an `ignoreConnected` option to push notifications.
This commit is contained in:
@@ -69,5 +69,13 @@ ensureSystemSettingsBootstrap()
|
||||
.finally(() => {
|
||||
httpServer.listen(port, () => {
|
||||
logger.info({ port }, "Server listening");
|
||||
// Fire-and-forget background tick: every 60s, scan today's
|
||||
// meetings and push the 5-minute reminder to attendees whose
|
||||
// alert hasn't been pushed yet. See #558.
|
||||
import("./lib/executive-meeting-scheduler.js")
|
||||
.then((m) => m.startExecutiveMeetingScheduler())
|
||||
.catch((err) => {
|
||||
logger.error({ err }, "Failed to start executive-meeting scheduler");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,192 @@
|
||||
import { and, eq, inArray, ne, sql } from "drizzle-orm";
|
||||
import { db } from "@workspace/db";
|
||||
import {
|
||||
executiveMeetingAlertStateTable,
|
||||
executiveMeetingsTable,
|
||||
} from "@workspace/db";
|
||||
import { logger } from "./logger";
|
||||
import { sendPushToUser } from "./push";
|
||||
import { getUserIdsForRoleNames } from "./executive-meeting-notify";
|
||||
|
||||
const TICK_MS = 60_000;
|
||||
const LEAD_MINUTES = 5;
|
||||
const EM_NOTIFY_ROLES = ["admin", "executive_office_manager"] as const;
|
||||
|
||||
function pad2(n: number): string {
|
||||
return String(n).padStart(2, "0");
|
||||
}
|
||||
|
||||
function dateKey(d: Date): string {
|
||||
return `${d.getFullYear()}-${pad2(d.getMonth() + 1)}-${pad2(d.getDate())}`;
|
||||
}
|
||||
|
||||
function parseHHMM(s: string | null | undefined): { h: number; m: number } | null {
|
||||
if (!s) return null;
|
||||
const m = /^(\d{1,2}):(\d{2})/.exec(s);
|
||||
if (!m) return null;
|
||||
const h = Number(m[1]);
|
||||
const mm = Number(m[2]);
|
||||
if (!Number.isFinite(h) || !Number.isFinite(mm)) return null;
|
||||
return { h, m: mm };
|
||||
}
|
||||
|
||||
/**
|
||||
* Convert a meeting's (meetingDate, startTime) into an absolute Date in
|
||||
* the server's local timezone (matches how the client computes the
|
||||
* 5-minute window). Returns null if either field is malformed.
|
||||
*/
|
||||
function meetingStartLocal(meetingDate: string, startTime: string | null): Date | null {
|
||||
const t = parseHHMM(startTime);
|
||||
if (!t) return null;
|
||||
const d = new Date(`${meetingDate}T${pad2(t.h)}:${pad2(t.m)}:00`);
|
||||
if (Number.isNaN(d.getTime())) return null;
|
||||
return d;
|
||||
}
|
||||
|
||||
async function tickOnce(): Promise<void> {
|
||||
const now = new Date();
|
||||
// Query today AND tomorrow so a meeting at 00:02 is still picked up
|
||||
// when the previous calendar day is finishing at 23:57. The eligible
|
||||
// filter below trims to the actual 5-minute window so this is cheap.
|
||||
const todayKey = dateKey(now);
|
||||
const tomorrow = new Date(now.getTime() + 24 * 60 * 60 * 1000);
|
||||
const tomorrowKey = dateKey(tomorrow);
|
||||
|
||||
const meetings = await db
|
||||
.select({
|
||||
id: executiveMeetingsTable.id,
|
||||
titleAr: executiveMeetingsTable.titleAr,
|
||||
titleEn: executiveMeetingsTable.titleEn,
|
||||
meetingDate: executiveMeetingsTable.meetingDate,
|
||||
startTime: executiveMeetingsTable.startTime,
|
||||
status: executiveMeetingsTable.status,
|
||||
})
|
||||
.from(executiveMeetingsTable)
|
||||
.where(
|
||||
and(
|
||||
inArray(executiveMeetingsTable.meetingDate, [todayKey, tomorrowKey]),
|
||||
ne(executiveMeetingsTable.status, "cancelled"),
|
||||
ne(executiveMeetingsTable.status, "completed"),
|
||||
),
|
||||
);
|
||||
|
||||
const eligible = meetings
|
||||
.map((m) => {
|
||||
const start = meetingStartLocal(m.meetingDate, m.startTime);
|
||||
if (!start) return null;
|
||||
const deltaMs = start.getTime() - now.getTime();
|
||||
if (deltaMs <= 0) return null;
|
||||
const deltaMin = Math.ceil(deltaMs / 60_000);
|
||||
if (deltaMin > LEAD_MINUTES) return null;
|
||||
return { meeting: m, deltaMin };
|
||||
})
|
||||
.filter((x): x is { meeting: typeof meetings[number]; deltaMin: number } => !!x);
|
||||
|
||||
if (eligible.length === 0) return;
|
||||
|
||||
const recipients = await getUserIdsForRoleNames(EM_NOTIFY_ROLES);
|
||||
if (recipients.length === 0) return;
|
||||
|
||||
for (const { meeting, deltaMin } of eligible) {
|
||||
// Atomic claim: ON CONFLICT only updates pushed_at when it's still
|
||||
// NULL, and we RETURN only the rows we actually mutated. Two ticks
|
||||
// (or two instances) racing on the same (meeting,user) can never
|
||||
// both win — Postgres serialises the conflict resolution. Rows
|
||||
// that the user has already acknowledged or dismissed are excluded
|
||||
// from the update so we don't ring after the user said "Done".
|
||||
const claimed = await db
|
||||
.insert(executiveMeetingAlertStateTable)
|
||||
.values(
|
||||
recipients.map((userId) => ({
|
||||
meetingId: meeting.id,
|
||||
userId,
|
||||
pushedAt: now,
|
||||
})),
|
||||
)
|
||||
.onConflictDoUpdate({
|
||||
target: [
|
||||
executiveMeetingAlertStateTable.meetingId,
|
||||
executiveMeetingAlertStateTable.userId,
|
||||
],
|
||||
set: { pushedAt: now },
|
||||
setWhere: and(
|
||||
sql`${executiveMeetingAlertStateTable.pushedAt} IS NULL`,
|
||||
eq(executiveMeetingAlertStateTable.dismissed, false),
|
||||
eq(executiveMeetingAlertStateTable.acknowledged, false),
|
||||
),
|
||||
})
|
||||
.returning({ userId: executiveMeetingAlertStateTable.userId });
|
||||
|
||||
if (claimed.length === 0) continue;
|
||||
|
||||
const titleAr = "تذكير اجتماع";
|
||||
const titleEn = "Meeting reminder";
|
||||
const subjectAr = meeting.titleAr || meeting.titleEn || "اجتماع";
|
||||
const subjectEn = meeting.titleEn || meeting.titleAr || "Meeting";
|
||||
const bodyAr = `${subjectAr} يبدأ خلال ${deltaMin} دقيقة`;
|
||||
const bodyEn = `${subjectEn} starts in ${deltaMin} min`;
|
||||
|
||||
for (const { userId } of claimed) {
|
||||
void sendPushToUser(
|
||||
userId,
|
||||
{
|
||||
title: subjectAr,
|
||||
body: bodyAr,
|
||||
titleAr,
|
||||
titleEn,
|
||||
bodyAr,
|
||||
bodyEn,
|
||||
tag: `em-reminder-${meeting.id}`,
|
||||
url: "/meetings",
|
||||
type: "executive_meeting",
|
||||
relatedId: meeting.id,
|
||||
},
|
||||
{ ignoreConnected: true },
|
||||
).catch((err) => {
|
||||
logger.warn(
|
||||
{ err, meetingId: meeting.id, userId },
|
||||
"executive-meeting-scheduler push failed",
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
logger.info(
|
||||
{ meetingId: meeting.id, deltaMin, recipients: claimed.length },
|
||||
"executive-meeting-scheduler reminder fired",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
let started = false;
|
||||
let timer: NodeJS.Timeout | null = null;
|
||||
let running = false;
|
||||
|
||||
export function startExecutiveMeetingScheduler(): void {
|
||||
if (started) return;
|
||||
started = true;
|
||||
const run = () => {
|
||||
// In-process guard: if the previous tick is still running (slow
|
||||
// DB, long push round-trip), skip this tick so we never overlap
|
||||
// and re-claim/re-send. Multi-process deployments still rely on
|
||||
// the atomic Postgres claim above for correctness.
|
||||
if (running) return;
|
||||
running = true;
|
||||
tickOnce()
|
||||
.catch((err) => {
|
||||
logger.error({ err }, "executive-meeting-scheduler tick failed");
|
||||
})
|
||||
.finally(() => {
|
||||
running = false;
|
||||
});
|
||||
};
|
||||
setImmediate(run);
|
||||
timer = setInterval(run, TICK_MS);
|
||||
logger.info({ tickMs: TICK_MS }, "executive-meeting-scheduler started");
|
||||
}
|
||||
|
||||
export function stopExecutiveMeetingScheduler(): void {
|
||||
if (timer) clearInterval(timer);
|
||||
timer = null;
|
||||
started = false;
|
||||
running = false;
|
||||
}
|
||||
@@ -234,6 +234,7 @@ async function isUserConnected(userId: number): Promise<boolean> {
|
||||
export async function sendPushToUser(
|
||||
userId: number,
|
||||
payload: PushPayload,
|
||||
options: { ignoreConnected?: boolean } = {},
|
||||
): Promise<void> {
|
||||
if (!Number.isInteger(userId) || userId <= 0) return;
|
||||
const channel = (payload.type as ChannelType) ?? "info";
|
||||
@@ -245,7 +246,11 @@ export async function sendPushToUser(
|
||||
// they're already getting the in-app notification via Socket.IO, so
|
||||
// we skip Web Push to avoid double-alerts. Push is for the
|
||||
// backgrounded/closed PWA case.
|
||||
if (await isUserConnected(userId)) return;
|
||||
//
|
||||
// `ignoreConnected` overrides this for time-critical reminders (e.g.
|
||||
// the 5-minute upcoming-meeting scheduler) where missing the ring
|
||||
// because a stale tab is open is worse than ringing twice.
|
||||
if (!options.ignoreConnected && (await isUserConnected(userId))) return;
|
||||
|
||||
const vapid = await getVapid();
|
||||
if (!vapid) return;
|
||||
|
||||
@@ -252,6 +252,11 @@ export const executiveMeetingAlertStateTable = pgTable(
|
||||
.references(() => usersTable.id, { onDelete: "cascade" }),
|
||||
dismissed: boolean("dismissed").notNull().default(false),
|
||||
acknowledged: boolean("acknowledged").notNull().default(false),
|
||||
// #558: Set once by the server-side scheduler the moment a Web
|
||||
// Push reminder has been fired for this (meeting, user) pair, so a
|
||||
// single tick (or any later tick that re-runs the same query) does
|
||||
// not double-ring the iPad. NULL = no push has been sent yet.
|
||||
pushedAt: timestamp("pushed_at", { withTimezone: true }),
|
||||
updatedAt: timestamp("updated_at", { withTimezone: true })
|
||||
.notNull()
|
||||
.defaultNow()
|
||||
|
||||
Reference in New Issue
Block a user