Fix two duplicate-charge bugs in scheduled billing #201

Merged

thatguygriff merged 2 commits from fix/duplicate-payment-due-notice into main 2026-09-17 18:25:19 +00:00

Conversation 1 Commits 2 Files Changed 21

+641 -12

Kydoimos commented 2026-09-17 16:19:51 +00:00

Member

Copy Link

Copy Source

Two independent defects, both surfacing as families being "charged twice." Confirmed against the live kayleighjamesmusic.ca database (Ker dc1).

Fix 1 — duplicate "Payment due" emails under concurrent cron

The daily billing scan (us_generate_due_payments) runs on request via WP-Cron; the live pod shows POST /wp-cron.php firing every few minutes on visitor traffic, with no DISABLE_WP_CRON and no serializing CronJob. WordPress's doing_cron transient is best-effort, so two scans can overlap. Each run emailed the payments it created with no record that a notice had already gone out — so a payer could receive two identical "Payment due" emails for a single charge (one row on the books).

Made the notice idempotent:

• us_payments.notice_sent_at column.
• PaymentRepository::markNoticed() — UPDATE ... SET notice_sent_at = now WHERE id = ? AND notice_sent_at IS NULL, returns whether this call won. The conditional WHERE is the guard; no check-then-act race.
• sendNotices() claims each payment before including it; already-claimed rows are dropped from crediting, batching and the email.
• PaymentService::markNoticed() passthrough; notice_sent_at on the value object and insert map.
• One-time backfillNoticeSent() in Plugin::boot() (guarded by us_payments_notice_sent_backfilled) so already-noticed charges are not re-emailed on upgrade.

Fix 2 — double charge when a class switches to monthly billing

Root cause found by widening the duplicate query to ignore period_key (the first pass filtered period_key IS NOT NULL and missed these). The live duplicates are group enrollments where one row has period_key = NULL and an earlier created_at, the other period_key = 2026-09:

• A student enrolls while the class is pay-now -> charged once at enrollment (EnrollmentEndpoint, period_key = NULL).
• The class is later switched to monthly.
• From the 1st, the daily scan bills the active enrollment (period_key = 2026-09). Its dedup (scheduledPaymentExists by exact period_key) never matches the NULL-key up-front charge, so it bills again.

The differing payer (student vs guardian) across the two rows was a side effect of guardian links created between the two charge dates, not the cause.

Reconcile on the transition into monthly:

• BillingModeReconciler — when a group class is switched into monthly, adopts each active enrollment's up-front charge into the current month (stamps period_key + due_date) so the scan treats the month as billed and charges from the next month on. Skips enrollments with no up-front charge or already billed for the month; ignores weekly and non-group offerings.
• PaymentRepository::hasUnscheduledCharge() / claimPeriodForUnscheduled(), EnrollmentRepository::findActiveByOffering().
• Wired into both offering-update paths (OfferingController admin form, OfferingEndpoint REST).

The reconciler's claim UPDATE was verified against the live MySQL primary in a rolled-back transaction: it stamps exactly the orphan row and leaves the scan row untouched.

Existing data (handled out-of-band)

5 enrollments already duplicated on the live site. A reviewed, transactional cleanup script voids the 4 pending orphans and flags enrollment 9 (student 21), which was paid twice ($340) and needs a refund/credit decision. Run by the studio, not in this PR.

Non-goals

• No recurring "you still owe" reminder exists; not added here.
• Defense-in-depth (DISABLE_WP_CRON + a single Kubernetes CronJob) is a follow-up in the infra repo.
• Version left at 1.5.8; final version resolved at merge.

Tests

composer test (1014 tests, 2927 assertions), composer lint (PHPStan level 10), composer cs all pass. Added coverage for markNoticed/backfillNoticeSent, BillingModeReconciler (transition detection, per-enrollment adoption, skip conditions), hasUnscheduledCharge/claimPeriodForUnscheduled, findActiveByOffering, and Payment::fromRow mapping notice_sent_at.

Two independent defects, both surfacing as families being "charged twice." Confirmed against the live kayleighjamesmusic.ca database (Ker `dc1`). ## Fix 1 — duplicate "Payment due" emails under concurrent cron The daily billing scan (`us_generate_due_payments`) runs on request via WP-Cron; the live pod shows `POST /wp-cron.php` firing every few minutes on visitor traffic, with no `DISABLE_WP_CRON` and no serializing CronJob. WordPress's `doing_cron` transient is best-effort, so two scans can overlap. Each run emailed the payments *it* created with no record that a notice had already gone out — so a payer could receive two identical "Payment due" emails for a single charge (one row on the books). Made the notice idempotent: - `us_payments.notice_sent_at` column. - `PaymentRepository::markNoticed()` — `UPDATE ... SET notice_sent_at = now WHERE id = ? AND notice_sent_at IS NULL`, returns whether this call won. The conditional `WHERE` is the guard; no check-then-act race. - `sendNotices()` claims each payment before including it; already-claimed rows are dropped from crediting, batching and the email. - `PaymentService::markNoticed()` passthrough; `notice_sent_at` on the value object and insert map. - One-time `backfillNoticeSent()` in `Plugin::boot()` (guarded by `us_payments_notice_sent_backfilled`) so already-noticed charges are not re-emailed on upgrade. ## Fix 2 — double charge when a class switches to monthly billing Root cause found by widening the duplicate query to ignore `period_key` (the first pass filtered `period_key IS NOT NULL` and missed these). The live duplicates are group enrollments where one row has `period_key = NULL` and an earlier `created_at`, the other `period_key = 2026-09`: - A student enrolls while the class is pay-now -> charged once at enrollment (`EnrollmentEndpoint`, `period_key = NULL`). - The class is later switched to `monthly`. - From the 1st, the daily scan bills the active enrollment (`period_key = 2026-09`). Its dedup (`scheduledPaymentExists` by exact `period_key`) never matches the NULL-key up-front charge, so it bills again. The differing payer (student vs guardian) across the two rows was a side effect of guardian links created between the two charge dates, not the cause. Reconcile on the transition into monthly: - `BillingModeReconciler` — when a group class is switched into monthly, adopts each active enrollment's up-front charge into the current month (stamps `period_key` + `due_date`) so the scan treats the month as billed and charges from the next month on. Skips enrollments with no up-front charge or already billed for the month; ignores weekly and non-group offerings. - `PaymentRepository::hasUnscheduledCharge()` / `claimPeriodForUnscheduled()`, `EnrollmentRepository::findActiveByOffering()`. - Wired into both offering-update paths (`OfferingController` admin form, `OfferingEndpoint` REST). The reconciler's claim UPDATE was verified against the live MySQL primary in a rolled-back transaction: it stamps exactly the orphan row and leaves the scan row untouched. ## Existing data (handled out-of-band) 5 enrollments already duplicated on the live site. A reviewed, transactional cleanup script voids the 4 pending orphans and flags enrollment 9 (student 21), which was **paid twice** ($340) and needs a refund/credit decision. Run by the studio, not in this PR. ## Non-goals - No recurring "you still owe" reminder exists; not added here. - Defense-in-depth (`DISABLE_WP_CRON` + a single Kubernetes CronJob) is a follow-up in the infra repo. - Version left at 1.5.8; final version resolved at merge. ## Tests `composer test` (1014 tests, 2927 assertions), `composer lint` (PHPStan level 10), `composer cs` all pass. Added coverage for `markNoticed`/`backfillNoticeSent`, `BillingModeReconciler` (transition detection, per-enrollment adoption, skip conditions), `hasUnscheduledCharge`/`claimPeriodForUnscheduled`, `findActiveByOffering`, and `Payment::fromRow` mapping `notice_sent_at`.

Kydoimos added 1 commit 2026-09-17 16:19:52 +00:00

Send each scheduled payment's due notice exactly once ... e389e40843

The daily billing scan runs on request via WP-Cron and can overlap
itself under concurrent traffic. Each run emailed the payments it
created with no record that a notice had gone out, so two overlapping
runs could send a payer two identical "Payment due" emails for one
charge — read by families as being billed twice, though only one row
exists.

Stamp us_payments.notice_sent_at atomically before emailing: the scan
now claims each payment with a conditional UPDATE ... WHERE
notice_sent_at IS NULL and only notices, credits and batches the rows
it won. A competing run finds them claimed and stays quiet, so exactly
one notice is sent regardless of how the scan is triggered. A one-time
backfill stamps existing scheduled rows on upgrade so already-noticed
charges are not re-emailed.

Co-authored-by: anthropic/claude-opus-4-8

Kydoimos added 1 commit 2026-09-17 18:08:02 +00:00

Reconcile up-front charges when a class switches to monthly billing ... acda3cda0f

A student who enrols while a group class is pay-now is charged once at
enrolment, and that charge carries no period_key. When the class is
later switched to monthly, the daily scan — which dedups scheduled
charges by period_key — does not see the up-front charge and bills the
enrolment again for the current month, double-charging students who
had already paid. The differing payer between the two rows (student vs
guardian) was a side effect of guardian links created between the two
charge dates, not the cause.

Switching a group class into monthly now adopts each active enrolment's
up-front charge into the current month (stamping period_key and
due_date) so the scan treats that month as billed and charges from the
next month on. Enrolments with no up-front charge, or already billed
for the month, are left alone; weekly and non-group offerings are not
touched. Wired into both offering-update paths (admin form and REST).

Co-authored-by: anthropic/claude-opus-4-8

Kydoimos changed title from Send each scheduled payment's due notice exactly once to Fix two duplicate-charge bugs in scheduled billing 2026-09-17 18:08:25 +00:00

thatguygriff approved these changes 2026-09-17 18:25:15 +00:00

thatguygriff merged commit d8d842b1ef into main 2026-09-17 18:25:19 +00:00

thatguygriff deleted branch fix/duplicate-payment-due-notice 2026-09-17 18:25:19 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

thatguygriff

Labels

Clear labels

bug

Something isn't working

enhancement

Extends an existing feature

feature

New user-facing capability

payments

Billing, Stripe, receipts, reporting

security

Security vulnerability or hardening

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

Kydoimos (Kydoimos) thatguygriff (James Griffin-Allwood)

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Unsupervised/unsupervised-scheduler#201