Skip reminder habit evaluation for completed delivery slots
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { expect, test, spyOn } from "bun:test";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { fixture } from "../habits/test-fixture";
|
||||
import { reminderDeliveries, reminderSettings } from "../db/schema";
|
||||
@@ -213,3 +213,21 @@ test("messages include names only for the applicable reminders in the delivery b
|
||||
expect(JSON.parse(sent[1]!).allowed_mentions.parse).toEqual([]);
|
||||
} finally { f.close(); }
|
||||
});
|
||||
|
||||
|
||||
test('delivered slots skip habit evaluation while later slots and retries remain eligible', async () => {
|
||||
const { f } = await setup(); const sent: string[] = [];
|
||||
const { HabitService } = await import('../habits/service');
|
||||
let sync: ReturnType<typeof spyOn> | undefined;
|
||||
try {
|
||||
f.db.update(reminderSettings).set({ quietStart: '00:00', quietEnd: '00:00', reminders: [
|
||||
{ type: 'date', date: '2026-09-04', time: '10:00' }, { type: 'date', date: '2026-09-04', time: '12:00' },
|
||||
] }).run();
|
||||
const tick = (clock: string) => sendDueReminders(f.db, config, transport(sent), () => Date.parse(`2026-09-04T${clock}:00Z`));
|
||||
await tick('08:00'); expect(sent).toHaveLength(1);
|
||||
sync = spyOn(HabitService.prototype, 'sync');
|
||||
for (let i = 0; i < 5; i++) await tick('08:30');
|
||||
expect(sync).not.toHaveBeenCalled(); expect(sent).toHaveLength(1);
|
||||
await tick('10:00'); expect(sync).toHaveBeenCalled(); expect(sent).toHaveLength(2);
|
||||
} finally { sync?.mockRestore(); f.close(); }
|
||||
});
|
||||
|
||||
@@ -22,14 +22,26 @@ export async function sendDueReminders(db: AppDatabase, config: ReminderConfig,
|
||||
// Retain recent delivery history and deduplication records without unbounded growth.
|
||||
db.delete(reminderDeliveries).where(lt(reminderDeliveries.updatedAt, now() - 90 * 86400000)).run();
|
||||
for (const { user } of rows) {
|
||||
function due() {
|
||||
function due(checkDeliveries = false) {
|
||||
const currentUser = db.select().from(users).where(eq(users.id, user.id)).get();
|
||||
const prefs = db.select().from(reminderSettings).where(eq(reminderSettings.userId, user.id)).get();
|
||||
if (!currentUser || !prefs?.enabled) return null;
|
||||
const timestamp = now(); const clock = localClock(timestamp, currentUser.timezone);
|
||||
if (inQuietHours(clock, prefs.quietStart, prefs.quietEnd)) return null;
|
||||
const times = timesForDate(prefs, localDate(timestamp, currentUser.timezone)).filter(time => time <= clock);
|
||||
const date = localDate(timestamp, currentUser.timezone);
|
||||
const times = timesForDate(prefs, date).filter(time => time <= clock);
|
||||
if (!times.length) return null;
|
||||
if (checkDeliveries) {
|
||||
const previous = db.select().from(reminderDeliveries).where(and(eq(reminderDeliveries.userId, user.id), eq(reminderDeliveries.date, date))).all();
|
||||
// This is only a cheap eligibility hint. The transaction below still owns claims.
|
||||
// Let stale pending records reach its recovery step before skipping the user.
|
||||
const stalePending = previous.some(row => row.status === 'pending' && row.updatedAt < timestamp - 60000);
|
||||
if (!stalePending) {
|
||||
const legacy = previous.find(row => row.slot === '*');
|
||||
const claimable = (prior: typeof previous[number] | undefined) => !prior || (prior.status === 'deferred' && (prior.retryAt ?? 0) <= timestamp);
|
||||
if (previous.some(row => row.status === 'pending') || (legacy ? !claimable(legacy) : !times.some(slot => claimable(previous.find(row => row.slot === slot))))) return null;
|
||||
}
|
||||
}
|
||||
const service = new HabitService(db, { id: currentUser.id, discordId: currentUser.discordId, username: currentUser.username, displayName: currentUser.globalName ?? currentUser.username, avatarUrl: null, timezone: currentUser.timezone }, timestamp);
|
||||
service.sync();
|
||||
// Avoid sending an artificial "today" after a backwards timezone change.
|
||||
@@ -37,7 +49,7 @@ export async function sendDueReminders(db: AppDatabase, config: ReminderConfig,
|
||||
const remaining = service.list().filter(habit => { const day = service.day(habit.id, service.today); return day.due && !day.complete; }).length;
|
||||
return remaining ? { date: service.today, remaining, timezone: currentUser.timezone, times, entries: entriesForDate(prefs, service.today) } : null;
|
||||
}
|
||||
const initial = due(); if (!initial) continue;
|
||||
const initial = due(true); if (!initial) continue;
|
||||
const claims = db.transaction(tx => {
|
||||
// A crashed/aborted send must not block later times indefinitely.
|
||||
tx.update(reminderDeliveries).set({ status: "uncertain" }).where(and(eq(reminderDeliveries.userId, user.id), eq(reminderDeliveries.status, "pending"), lt(reminderDeliveries.updatedAt, now() - 60000))).run();
|
||||
|
||||
Reference in New Issue
Block a user