diff --git a/tests/Feature/Billing/StripePriceAdoptionTest.php b/tests/Feature/Billing/StripePriceAdoptionTest.php index 9fe847c..cfeabf3 100644 --- a/tests/Feature/Billing/StripePriceAdoptionTest.php +++ b/tests/Feature/Billing/StripePriceAdoptionTest.php @@ -242,17 +242,32 @@ it('leaves a frozen booking on the price it was sold at', function () { app(Kernel::class)->call('stripe:sync-catalogue'); - // The two assertions below only say the booking ROW was not rewritten and - // that ITS OWN Price was not archived — true in this exact path even with - // adoption removed altogether, because stripe:sync-catalogue never writes - // subscription_addons.stripe_price_id (that is SyncStripeAddonItems, a - // separate write path) and the orphan planted above sits at a different - // amount from $sold, so archiveSuperseded's net_cents scoping was already - // never going to reach it. They stay because the untouched row is still - // worth stating, but they are not the guard. The three assertions after - // them are: they fail the moment adoption reaches for a figure other than - // the one it was asked for — the exact way a defect here would move money - // on an already-frozen booking. + // The `archived` assertion right below — the second of this chain — is + // this test's only adoption tooth. AdoptStripePrice archives a Price at + // Stripe when it treats one as a duplicate of another it adopted, and + // that is the one way it could reach past the figure it was asked for + // (4680, the new one) and take the frozen booking's own Price (3480) down + // with it as a false "duplicate". + // + // The three assertions after it read the SWEEP's own row-writing code, + // not adoption — AdoptStripePrice performs no database writes at all, so + // nothing it does, singly or in combination, can fail them. They earn + // their place anyway, because they are exactly what a defect in the + // sweep's OTHER two writers would fail: archiveSuperseded() losing its + // `->where('net_cents', $netCents)` scoping (its own docblock is about + // precisely why that scoping has to hold) would sweep the 2900-net row up + // alongside the 3900-net one it is meant to supersede, and remember() + // rewritten as an updateOrCreate() keyed without `amount_cents` — the + // plausible-looking fix for the UniqueConstraintViolationException catch + // a few lines below it — would silently overwrite the very row these + // assertions read. + // + // Neither of adoption's two guards is isolated by this test — the amount + // match and the `claimed` callback cover for each other in this scenario, + // which is why sabotaging either alone during this fix round left the + // booking untouched. Each is isolated on its own elsewhere in this file: + // it('adopts nothing when the amount, currency or interval differ') and + // it('never hands out a price a row already claims'). expect($booking->refresh()->stripe_price_id)->toBe($sold) ->and($this->stripe->archived)->not->toContain($sold) // The OLD figure's Price is still what a checkout for it would use — @@ -261,6 +276,16 @@ it('leaves a frozen booking on the price it was sold at', function () { ->and(app(AddonPrices::class)->liveFor( 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), ))->toBe($sold) + // The row itself, not only whether it is archived — `archived_at` + // reads null for a row that is still live AND for one that is gone + // or rewritten onto a different net_cents, so `exists()` has to stand + // beside it or the row's disappearance would pass silently. + ->and(StripeAddonPrice::query() + ->where('addon_key', 'priority_support') + ->where('reverse_charge', false) + ->where('net_cents', 2900) + ->where('interval', 'month') + ->exists())->toBeTrue() ->and(StripeAddonPrice::query() ->where('addon_key', 'priority_support') ->where('reverse_charge', false)