From 8c3a79e258fa42fb096dfca5adb9873a46d35cc1 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sun, 26 Jul 2026 13:57:11 +0200 Subject: [PATCH] fix(billing): hold only what cannot be matched, and let migrations roll back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from reviewing all five phases together rather than one at a time. The webhook held every event a handler answered `null` to — but `null` also means "already recorded" and "deliberately skipped", which is every checkout's own invoice and every redelivered renewal. Their contract is right there, so `replayHeldFor()` could never come back for them, and ordinary Stripe traffic silted up the holding area until the weekly prune. It now holds only what it genuinely cannot match — and if the contract appeared in the meantime, applies the event instead of dropping it: the creation race is narrow, but losing a cancellation to it would leave us serving someone who had left. Also fixes a rollback that predates this work and blocks `migrate:fresh` on MariaDB entirely: dropping the unique index on customers.user_id fails while the foreign key added one migration earlier still depends on it. Verified by building all 33 migrations from nothing on MariaDB, rolling every one of them back, and building them again. 458 tests green. Codex clean on the full branch diff. Co-Authored-By: Claude Opus 5 --- app/Actions/ApplyStripeBillingEvent.php | 16 ++++++++++ ...5_090004_make_customer_identity_unique.php | 8 +++++ tests/Feature/Billing/StripeBillingTest.php | 30 +++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/app/Actions/ApplyStripeBillingEvent.php b/app/Actions/ApplyStripeBillingEvent.php index d576f12..ded9ed9 100644 --- a/app/Actions/ApplyStripeBillingEvent.php +++ b/app/Actions/ApplyStripeBillingEvent.php @@ -346,6 +346,22 @@ class ApplyStripeBillingEvent return; } + // Only what we could not match. A handler returns null for several + // reasons — already recorded, or deliberately skipped, like every + // checkout's own invoice — and holding those would fill the table with + // rows that replayHeldFor() can never revisit, because their contract + // is right there. Nothing to wait for means nothing to hold. + if (Subscription::query()->where('stripe_subscription_id', $subscriptionId)->exists()) { + // Unless it appeared just now: the contract can be created between + // the handler missing it and this check, and the replay that would + // have collected it has then already been and gone. Apply it here + // instead of dropping it. Safe to repeat — the handlers are + // idempotent, so an event that was a no-op stays one. + $this->dispatch($event); + + return; + } + StripePendingEvent::query()->updateOrCreate( ['stripe_event_id' => $event['id']], [ diff --git a/database/migrations/2026_07_25_090004_make_customer_identity_unique.php b/database/migrations/2026_07_25_090004_make_customer_identity_unique.php index 8c0cc61..bce3b6a 100644 --- a/database/migrations/2026_07_25_090004_make_customer_identity_unique.php +++ b/database/migrations/2026_07_25_090004_make_customer_identity_unique.php @@ -21,7 +21,15 @@ return new class extends Migration public function down(): void { Schema::table('customers', function (Blueprint $table) { + // MariaDB will not drop the last index a foreign key depends on, + // and the constraint added one migration earlier is using this one. + // So release it, drop the index, and put it back — re-adding builds + // the index the constraint needs. Without this the whole suite of + // migrations cannot be rolled back, which is how `migrate:fresh` + // works. + $table->dropForeign(['user_id']); $table->dropUnique(['user_id']); + $table->foreign('user_id')->references('id')->on('users')->nullOnDelete(); }); } }; diff --git a/tests/Feature/Billing/StripeBillingTest.php b/tests/Feature/Billing/StripeBillingTest.php index ee9920e..dd1f7fc 100644 --- a/tests/Feature/Billing/StripeBillingTest.php +++ b/tests/Feature/Billing/StripeBillingTest.php @@ -288,6 +288,36 @@ it('holds a billing event that arrives before the contract exists, and applies i ->and(App\Models\StripePendingEvent::query()->count())->toBe(0); }); +it('holds nothing for a contract that is right there', function () { + stripeContract(); + + // Deliberately skipped: the checkout's own invoice. + $this->postJson(route('webhooks.stripe'), [ + 'id' => 'evt_c', 'type' => 'invoice.paid', 'created' => now()->timestamp, + 'data' => ['object' => [ + 'id' => 'in_c', 'subscription' => 'sub_1', 'amount_paid' => 21480, + 'billing_reason' => 'subscription_create', + ]], + ])->assertOk(); + + // Already recorded: a redelivered renewal. + $renewal = [ + 'id' => 'evt_r', 'type' => 'invoice.paid', 'created' => now()->timestamp, + 'data' => ['object' => [ + 'id' => 'in_r', 'subscription' => 'sub_1', 'amount_paid' => 21480, + 'billing_reason' => 'subscription_cycle', + 'period_start' => now()->timestamp, 'period_end' => now()->addMonth()->timestamp, + ]], + ]; + $this->postJson(route('webhooks.stripe'), $renewal)->assertOk(); + $this->postJson(route('webhooks.stripe'), $renewal)->assertOk(); + + // The holding area waits for contracts that do not exist yet. Ordinary + // traffic must not silt it up with rows nothing will ever come back for. + expect(App\Models\StripePendingEvent::query()->count())->toBe(0) + ->and(SubscriptionRecord::query()->where('event', 'renewal')->count())->toBe(1); +}); + it('holds an event only once however often Stripe redelivers it', function () { $event = [ 'id' => 'evt_repeat', 'type' => 'invoice.paid', 'created' => now()->timestamp,