fix(billing): hold only what cannot be matched, and let migrations roll back
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 <noreply@anthropic.com>feat/portal-design
parent
0560ae743d
commit
8c3a79e258
|
|
@ -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']],
|
||||
[
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue