672 feat auto update opportunity and match status when appointment date passes#758
Conversation
nadavosa
left a comment
There was a problem hiding this comment.
What it does: Adds a new daily cron (scheduler-daily.ts) that scans accompanying/event-type opportunities and marks the opportunity INACTIVE once their date is in the past. Renames the existing cron to scheduler-hourly.ts for clarity.
🔴 Doesn't fully close #672 — the match-status half is missing
#672 has two explicit requirements:
- Opportunity status →
inactive— implemented - "If a volunteer was matched, set the matching status to
paston the deal record" — not implemented at all
scanExpiredOnetimers only ever touches opportunity.status; it never looks at or updates the opportunity-volunteer match-status entity (src/data/entity/m2m/opportunity-volunteer.ts), even though the query already joins deal/dealTimeslot. Since this PR is tagged "Closes #672", merging it will auto-close an issue that's only half-satisfied — a volunteer matched to an expired accompanying/event opportunity will keep showing as active/matched indefinitely. Existing sibling jobs (scan-post-match-checkup.ts, scan-stale-pending.ts) already update this kind of match status — worth following their pattern.
🟠 Cutoff uses a rolling 24h window, not a Berlin calendar-day boundary
export function daysAgo(n: number): Date {
const date = new Date();
date.setDate(date.getDate() - n);
return date;
}The issue spec is explicit: "date is strictly before today (today is not considered past)" — a calendar-day boundary. daysAgo(1) instead computes "exactly 24 hours before the instant the cron fires," using server-local time arithmetic. The codebase already has berlinToday() and berlinDayBoundaries() in services/jobs/german-holidays.ts, built specifically to avoid this — their own comment says: "Node.js new Date(y, m, d) uses the process-local timezone (UTC on AWS), so it would not correctly represent Berlin midnight." This new job re-introduces exactly that pitfall instead of reusing the existing helper. Practical effect: an accompanying appointment late in the day can miss getting flagged for up to an extra day depending on what time the cron fires relative to the appointment time — not the clean "before today" cutoff the issue asks for.
🟠 Both cron plugins share the same advisory lock ID
SCHEDULER_LOCK_ID = 20240701 is identical in both scheduler-daily.ts and scheduler-hourly.ts. Postgres advisory locks are keyed globally by that integer, not per-job — so these two independent jobs can block each other (one skipping because it thinks another instance of itself is running, when it's actually the unrelated other job holding the lock) instead of the lock only preventing duplicate concurrent runs of the same job across ECS replicas. Each job needs its own lock ID.
🟡 runWithAdvisoryLock duplicated verbatim
The entire lock-acquisition helper (including its comments) is copy-pasted byte-for-byte between the two scheduler files. Now that there are two call sites, this is the moment to extract it into a shared module rather than maintaining two copies.
🟡 Both plugins registered under the same name
Despite renaming the files, both still do fp(schedulerPlugin, { name: "scheduler", ... }) — so Fastify's plugin introspection/logging can't distinguish the hourly job from the daily one. Should be "scheduler-hourly" / "scheduler-daily".
Minor:
- The comment above the new daily cron ("Hourly on the hour, 08:00–19:00 Berlin time, weekdays only") is a stale copy-paste from the hourly scheduler — describes the wrong schedule.
- No test for
scanExpiredOnetimers, but consistent with the rest of this codebase'sscan-*jobs (none of them have dedicated tests either) — not a real ding.
Bottom line: the opportunity-status half works, but I'd hold this back from merge until the match-status update is added (or split into a fast-follow) and the timezone/lock-ID issues are fixed — as-is, closing #672 with this would be premature.
…h-status-when-appointment-date-passes
Covers the no-op empty-result case, correct INACTIVE/PAST status transitions, and the per-item try/catch isolation added for opportunity and opportunity-volunteer saves. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
need4deed
left a comment
There was a problem hiding this comment.
Re-reviewed after fixes: date-arithmetic type bug fixed, per-item error isolation added, plugin naming collision resolved, dead code removed, and unit tests added covering the status-transition logic and error isolation.
Description
Describe the change and why it matters (optional).
Related Issues
Closes #672
Changes
Screenshots / Demos
If UI-related, add before/after or GIFs.
Checklist