Fix round 1, correction: name the assertion that actually bites

The comment from the last commit had it backwards. It called the two
original assertions "not the guard" and claimed the three DB-state
assertions fail when adoption reaches the wrong figure — both false.
AdoptStripePrice performs no database writes at all, so nothing it does can
fail an assertion that reads the 2900-net row's own table state, singly or
in combination. The `archived` assertion — the only one that reads what
Stripe was actually told — is the one adoption can fail, by archiving the
frozen booking's own Price as a false duplicate of the new figure's orphan.

Left as a trusted comment, this would have read as an invitation to delete
the real guard as redundant scaffolding around the "real" three. Reworded to
say what the DB-state assertions actually defend instead — archiveSuperseded()
losing its net_cents scoping, or remember() rewritten as an updateOrCreate()
keyed without amount_cents — and to name where each of adoption's two guards
(the amount match, the claimed callback) is tested in isolation elsewhere in
this file, since neither is isolated here.

Also closed the `archived_at` assertion: it read null for a row that is
still live and for one that is gone or rewritten onto the wrong net_cents
alike. An `exists()` check now stands beside it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feature/betriebsmodus^2
nexxo 2026-07-30 11:59:01 +02:00
parent 40f7818757
commit dca74f0d78
1 changed files with 36 additions and 11 deletions

View File

@ -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)