Confirm intest feature - #321
Conversation
Documents the data model, domain actions/events, scheduled command, notification flow, and frontend/testing plan for requiring trainees to reconfirm waiting-list interest monthly, with automatic removal for those who don't. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… failures
Fix 1: gate the purge on real elapsed time, not the calendar month. The
year_month guard alone let a run landing late in a month be followed by a
run on the 1st, purging everyone who had less than a day to reconfirm (and
the same collapse after scheduler downtime across a month boundary). The
purge now requires >= 25 days since the previous run's ran_at.
Fix 3: move notification HTTP sends out of the DB transaction. The actions
now collect per-user notification payloads inside the transaction and the
sends happen after commit, so a table-level write lock is not held open for
the whole fan-out and nothing is sent for a transaction that rolls back.
Fix 4: scope the reset to WaitingListEntry::where('is_interested', true)
instead of an unscoped mass update, so correctness no longer depends on
purgeUnconfirmed() having run first in the same cycle.
Fix 6: check sendNotification's success return value and log a warning on
failure. VatgerClient swallows its own exceptions and returns success=false,
so the surrounding try/catch never fired on a real failure.
Fix 5: the "one notification" test now asserts the actual sendNotification
call count via a Mockery mock, not just event dedup. It asserts one
"Removed from Waiting List" and one "Confirm Waiting List Interest" call
(the brief suggested a single ->once(), but this run legitimately sends
both notification types to the user).
Also adds tests covering both sides of the new purge guard.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cked docker-compose.yml builds `dockerfile: Dockerfile.dev`, but only the compose file and the manual-testing doc were committed. A fresh clone of this branch could not run `docker compose up -d --build` — the exact first step the manual-testing doc prescribes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ow null Fix 7: explain why `artisan serve --no-reload` is required (its file-watcher restart drops the container env overrides and falls back to .env's mysql settings). Placed as a YAML comment above the `command:` key rather than inside it — the folded (>) scalar collapses to a single shell line, so a `#` there would comment out the rest of the command. Fix 8: `waiting_list_interest_confirmed` is `boolean | null`, not `boolean` — MentorManagementController::index() emits `$waitingEntry?->is_interested`, which is null when the user has no entry for that course. Fix 11: removed the stray untracked mentor-waiting-lists.tsx.bak backup file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… PHP npm run dev requires php on its PATH (the Wayfinder Vite plugin shells out to `php artisan wayfinder:generate` on every start), which broke `npm run dev` on machines without PHP installed natively. Extend Dockerfile.dev with Node/npm alongside PHP, give node_modules its own Docker volume (a Linux container and a non-Linux host can't safely share native npm binaries), and expose Vite's port. Update the manual-testing doc to run npm through the container instead of the host. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
docker-compose.yml previously published 8000/5173 on 0.0.0.0, exposing the dev app (fake auth, no real credentials, but still) to the rest of the local network. Bind to 127.0.0.1 instead — flagged by automated security review of the prior commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
MentorManagementController::index() resolved the concrete MoodleClient class directly (app(MoodleClient::class)) instead of the MoodleClientInterface everything else in the codebase uses. That bypassed AppServiceProvider's local/testing fake-client swap, so in local dev this made a real HTTP call to an unconfigured Moodle API (empty VATGER_API_KEY/URL), which times out, retries, and fails — always returning moodleSignedUp=false and permanently blocking the /courses page behind the "Moodle Account Required" modal. Switch to constructor-injected MoodleClientInterface, matching every other integration usage in the codebase, so local dev correctly gets FakeMoodleClient::userExists() => true. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bencodes07
left a comment
bencodes07
left a comment
There was a problem hiding this comment.
Thanks a lot for the effort. I have left some comments. If ATD does come to the conclusion, that this might be a possibility - with the requested changes this would definitely be worth implementing.
Also if you could: Integrate the new is_interested attribute into the admin sections table and form.
There was a problem hiding this comment.
As I am in the process of trying to remove console commands completely, I would like this to rather be a laravel job
There was a problem hiding this comment.
Replaced with a queued job (App\Jobs\ProcessWaitingListVerification), scheduled via $schedule->job() instead of $schedule->command() — matches your direction of moving away from console commands.
| * followed by one on the 1st, or after scheduler downtime across a month | ||
| * boundary) skip the purge so people actually get a month to reconfirm. | ||
| */ | ||
| private const MIN_DAYS_BETWEEN_PURGES = 25; |
There was a problem hiding this comment.
This might be an option, to add to the env, config or admin panel instead of hardcoding it
There was a problem hiding this comment.
Removed. The whole guard is gone now that verification is per-entry (see below) rather than a monthly batch, so there's no "did this month already run" state left to protect. The remaining grace-period value is configurable via WAITING_LIST_INTEREST_CONFIRMATION_DAYS.
There was a problem hiding this comment.
Generally I dont think all of this code belongs in a domain action tbh. Its a lot of logic for a single action. Either the individual logic parts can be split up or all be transferred into the laravel job.
There was a problem hiding this comment.
Rewritten around a per-entry removal_date. Dropping the batch/run-tracking machinery shrank the action a lot on its own — it's now close to the size of CheckUserRosterStatus.
There was a problem hiding this comment.
I think it might make more sense adding a removal_date to the waiting list entries just like already implemented for the roster entries. That logic seems simpler and doesnt complicate the database further. The data also gets removed with the waiting list entry automatically saving on database space
There was a problem hiding this comment.
Done, exactly as suggested — dropped the table/model and added removal_date directly to waiting_list_entries, mirroring RosterEntry. Simpler and avoids the extra table, as you said.
There was a problem hiding this comment.
see WaitingListVerificationRun Model
There was a problem hiding this comment.
Same change as above — table and migration removed since the model is gone.
There was a problem hiding this comment.
Dont think claudes documentation files are necessary :)
|
|
||
| {entry.is_interested ? ( | ||
| <Badge className="bg-success-100 text-success-800 dark:bg-success-900 dark:text-success-300"> | ||
| Confirmed |
There was a problem hiding this comment.
Might not be necessary to show a waiting list entry as confirmed. Should only really be important if its pending confirmation
There was a problem hiding this comment.
Removed — only "Pending confirmation" is shown now, since that's the state that actually needs attention.
There was a problem hiding this comment.
What is the reason to dockerise the test environment?
There was a problem hiding this comment.
Removed from the PR — it was just a personal convenience for testing without local PHP/Node/MySQL, not needed for review, so kept out and gitignored locally instead.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code
- Document WAITING_LIST_INTEREST_CONFIRMATION_DAYS in .env.example next to the other roster/waiting-list tuning env vars. - Restore the dropped idempotency test: running ProcessMonthlyWaitingListVerification::execute() twice back-to-back must be a no-op, since a queue worker can redeliver the job. - Lower ProcessWaitingListVerification's job timeout from 120s to 60s so it triggers before the production worker's --timeout=90 kills the job first, preserving the job's own error-logging catch block. - Revert the .gitignore change that smuggled a repo-wide policy (ignoring docker-compose.yml, Dockerfile.dev, docs/) into this feature branch; move the equivalent, properly anchored patterns into the local-only .git/info/exclude instead. - Drop stale "monthly" wording from the waiting-list form helper text now that the design isn't calendar-month-based.
|
Added |
2 participants
Monthly Waiting List Interest Verification
English
Summary
Waiting lists for courses (rating, endorsement, familiarisation, roster, guest)
have grown long over time, and a meaningful share of the people on them are no
longer actually interested in the training slot they're queued for. This PR
implements the monthly interest-verification process announced in the ATD
policy update linked above: once a month, everyone on a waiting list has to
reconfirm they're still interested in their spot, or they're automatically
removed.
This is a direct implementation of the technical approach proposed in the
linked forum thread:
("1) For each waiting-list slot, store whether interest still exists. 2)
Once a month (or every 2, 3, 6 months, whatever), e.g. always on the 1st,
send a notification asking whether interest still exists for that waiting
list. 3) Everyone who hasn't pressed a new 'confirm interest' button for
that waiting list after one month gets removed from it. That solves two
problems: staying on multiple waiting lists stays possible, and people who
are no longer interested get automatically removed.")
Mapped 1:1 onto this implementation:
waiting_list_entries.is_interested(per entry, not per user — so being on multiple lists works correctly)ProcessMonthlyWaitingListVerification, scheduled viawaitinglists:verify-interest; entries reset + notification sentWhat changed
waiting_list_entriesgainsis_interested(boolean,defaults to
true) andinterest_confirmed_at. A newwaiting_list_verification_runstable records which calendar month's cyclehas already run, so the scheduled job can safely trigger daily without
double-applying its effects within the same month.
ProcessMonthlyWaitingListVerification, run via the newwaitinglists:verify-interestartisan command, scheduled daily at 06:00):is_interested = falsefrom the previouscycle — i.e. anyone who didn't reconfirm in time.
is_interested = falseand sends anotification asking them to reconfirm before the next cycle.
than ~25 days ago, so two runs landing close together across a month
boundary (e.g. right after this feature deploys, or after scheduler
downtime) can't wipe out an entire waiting list without anyone having had
a real month to respond.
gets one notification, not one per entry.
the existing waiting-list controls whenever confirmation is pending.
"Confirmed" / "Pending confirmation" badge per entry.
WaitingListInterestConfirmed,WaitingListPurgedForInactivity,WaitingListVerificationRequested) arelogged to the activity log, consistent with every other state change in
this codebase.
Why this approach
New joiners default to "interested" so they're never purged for a cycle they
were never asked about. The purge always happens before the reset within
the same run, so someone who just joined can't be caught by the same cycle
that resets everyone else. Full design rationale and alternatives considered
are in
docs/superpowers/specs/2026-08-10-waiting-list-interest-verification-design.md.Testing
vendor/bin/pest), including dedicated coverage for thepurge/reset ordering, the double-run guard, the elapsed-time purge-skip
guard, and per-user notification dedup.
PHP/Node/MySQL required) are documented in
docs/manual-testing-waiting-list-verification.md.Deutsch
Zusammenfassung
Die Warteliste für Kurse (Rating, Endorsement, Familiarisation, Roster, Gast)
ist über die Zeit stark angewachsen, und ein nicht unerheblicher Teil der
Wartenden hat tatsächlich kein Interesse mehr an dem Trainingsplatz, auf den
sie warten. Dieser PR setzt den im oben verlinkten ATD-Policy-Update
angekündigten monatlichen Bestätigungsprozess um: Einmal im Monat muss jede
Person auf einer Warteliste ihr Interesse an ihrem Platz erneut bestätigen,
sonst wird sie automatisch von der Liste entfernt.
Dies ist eine direkte Umsetzung des technischen Vorschlags aus dem
verlinkten Forenthread:
1:1 umgesetzt in dieser PR:
waiting_list_entries.is_interested(pro Eintrag, nicht pro Nutzer — dadurch funktioniert das Stehen auf mehreren Wartelisten korrekt)ProcessMonthlyWaitingListVerification, geplant überwaitinglists:verify-interest; Einträge werden zurückgesetzt und Benachrichtigung verschicktWas sich ändert
waiting_list_entrieserhältis_interested(boolean,Standardwert
true) undinterest_confirmed_at. Eine neue Tabellewaiting_list_verification_runsspeichert, für welchen Kalendermonat derZyklus bereits gelaufen ist, damit der geplante Job täglich ausgelöst
werden kann, ohne die Wirkung innerhalb desselben Monats doppelt
anzuwenden.
ProcessMonthlyWaitingListVerification, ausgeführtüber den neuen artisan-Befehl
waitinglists:verify-interest, täglich um06:00 Uhr geplant):
is_interested = falseist — also alle, die nicht rechtzeitig bestätigthaben.
is_interestedauffalsezurück und schickt eine Benachrichtigung mit der Bitte um erneute
Bestätigung vor dem nächsten Zyklus.
wiederholt wird.
Lauf weniger als ca. 25 Tage zurückliegt — so kann nicht die gesamte
Warteliste gelöscht werden, falls zwei Läufe kurz hintereinander über
eine Monatsgrenze fallen (z. B. direkt nach dem Deployment dieses
Features oder nach einem Ausfall des Schedulers), ohne dass jemand
wirklich einen Monat Zeit zum Reagieren hatte.
Wartelisten steht, bekommt eine Benachrichtigung, nicht eine pro Eintrag.
neben den bestehenden Warteliste-Steuerelementen, sobald eine Bestätigung
aussteht.
Badge „Confirmed" / „Pending confirmation".
WaitingListInterestConfirmed,WaitingListPurgedForInactivity,WaitingListVerificationRequested)werden im Activity-Log protokolliert, konsistent mit jeder anderen
Zustandsänderung in dieser Codebase.
Warum dieser Ansatz
Neue Wartelisten-Einträge starten standardmäßig als „interessiert", damit sie
nie für einen Zyklus entfernt werden, zu dem sie noch gar nicht befragt
wurden. Die Bereinigung läuft innerhalb eines Durchlaufs immer vor dem
Zurücksetzen, damit jemand, der gerade erst beigetreten ist, nicht vom
selben Zyklus erfasst werden kann, der alle anderen zurücksetzt. Die
vollständige Design-Begründung und geprüfte Alternativen stehen in
docs/superpowers/specs/2026-08-10-waiting-list-interest-verification-design.md.Testing
vendor/bin/pest), inklusivededizierter Abdeckung für die Reihenfolge von Bereinigung/Reset, die
Doppellauf-Sperre, die zeitbasierte Bereinigungs-Sperre und die
Zusammenfassung von Benachrichtigungen pro Nutzer.
Docker-Compose-Basis, kein lokales PHP/Node/MySQL nötig) steht in
docs/manual-testing-waiting-list-verification.md.