From 97b655938ef6e217ae0cc8bce5c243748f75f051 Mon Sep 17 00:00:00 2001 From: nexxo Date: Thu, 30 Jul 2026 11:24:05 +0200 Subject: [PATCH] Ask before minting a module price MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sweep that died on 2026-07-29 left price_1TygdEC7u8NpJ8pOt3nsoyYw at Stripe and no row here. This is the step that finds it: same money, same interval, our metadata on it, unclaimed — taken over and brought up to date instead of duplicated. The comment on the key was the root of it. It read "keyed on what the Price IS", and a key is not that: identity is product, amount, currency and interval, and recognising those is AdoptStripePrice's job. Stripe forgets a key after a day; it never was the guard against a duplicate. A booking stays where it was sold. Adoption only ever matches an identical amount, so a frozen booking cannot be moved by it — there is a test that says so. Co-Authored-By: Claude Opus 5 --- app/Services/Billing/AddonPrices.php | 66 +++++++++----- .../Billing/StripePriceAdoptionTest.php | 87 +++++++++++++++++++ 2 files changed, 131 insertions(+), 22 deletions(-) diff --git a/app/Services/Billing/AddonPrices.php b/app/Services/Billing/AddonPrices.php index 6973cdd..e1d4610 100644 --- a/app/Services/Billing/AddonPrices.php +++ b/app/Services/Billing/AddonPrices.php @@ -44,7 +44,10 @@ use Illuminate\Database\UniqueConstraintViolationException; */ final class AddonPrices { - public function __construct(private readonly StripeClient $stripe) {} + public function __construct( + private readonly StripeClient $stripe, + private readonly AdoptStripePrice $adopt, + ) {} /** * The net a whole term of this module costs. @@ -125,30 +128,45 @@ final class AddonPrices $productId = $this->product($addonKey); - $priceId = $this->stripe->createPrice( + $metadata = [ + // Read back when a Stripe invoice line has to be turned into + // wording a customer can read — see StripeInvoiceLines. + 'addon' => $addonKey, + // Which of the module's two Prices this is, for anyone reading + // Stripe's own dashboard, where they would otherwise differ only + // by an amount. + 'tax_treatment' => $reverseCharge ? 'reverse_charge' : 'domestic', + ]; + + // Asked BEFORE minting, because a run that died between Stripe's create + // and our insert left a Price no table of ours knows — and the key below + // stops protecting it after twenty-four hours. See AdoptStripePrice for + // what may be taken over and why so narrowly. + $priceId = ($this->adopt)( productId: $productId, amountCents: $amount, currency: $currency, interval: $interval, - metadata: [ - // Read back when a Stripe invoice line has to be turned into - // wording a customer can read — see StripeInvoiceLines. - 'addon' => $addonKey, - // Which of the module's two Prices this is, for anyone reading - // Stripe's own dashboard, where they would otherwise differ only - // by an amount. - 'tax_treatment' => $reverseCharge ? 'reverse_charge' : 'domestic', - ], - // Keyed on what the Price IS, so a crash between Stripe creating it - // and us storing its id gives back the same Price on the next - // attempt rather than a second one at the same money. The amount is - // the CHARGED one, so the move from net to gross does not replay the - // net Price the old key was minted under — and the treatment is in - // there because at a rate of nought the two Prices are the same - // amount: one key would have Stripe hand the same object back for - // both, and the two rows would then share a Price, so archiving the - // gross one at the next rate change would withdraw the very Price the - // net side is still selling. + metadata: $metadata, + identifying: ['addon'], + claimed: fn (string $id) => StripeAddonPrice::query() + ->where('stripe_price_id', $id) + ->exists(), + ); + + $priceId ??= $this->stripe->createPrice( + productId: $productId, + amountCents: $amount, + currency: $currency, + interval: $interval, + metadata: $metadata, + // Says "I have already sent this call", nothing more. The amount is + // the CHARGED one and the treatment is in there because at a rate of + // nought the two Prices are the same amount — but what stops a + // second Price for one figure is AdoptStripePrice above, not this: + // Stripe forgets a key after twenty-four hours. The metadata is + // folded in by IdempotencyKey inside the client, so changing the + // format below can never again refuse the call for a day. idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}" .($reverseCharge ? '-rc' : ''), ); @@ -297,7 +315,11 @@ final class AddonPrices } catch (UniqueConstraintViolationException) { // Two bookings of the same module landed together. Stripe replayed // one Price for both — the idempotency key saw to that — so there is - // nothing to correct here beyond letting the first row stand. + // nothing to correct here beyond letting the first row stand. The + // same catch now also covers a Price id another row already claims, + // which the unique index refuses: the caller is handed the id + // regardless, because refusing would fail a customer's booking over + // a register that is one row short. } } } diff --git a/tests/Feature/Billing/StripePriceAdoptionTest.php b/tests/Feature/Billing/StripePriceAdoptionTest.php index c3d47a1..4c06251 100644 --- a/tests/Feature/Billing/StripePriceAdoptionTest.php +++ b/tests/Feature/Billing/StripePriceAdoptionTest.php @@ -1,8 +1,14 @@ stripe->plantPrice('price_1TygdEC7u8NpJ8pOt3nsoyYw', 'prod_support', + 3480, 'EUR', 'month', ['addon' => 'priority_support']); + StripeAddonPrice::query()->create([ + 'addon_key' => 'priority_support', 'reverse_charge' => false, + 'amount_cents' => 41760, 'net_cents' => 34800, 'currency' => 'EUR', + 'interval' => 'year', 'stripe_product_id' => 'prod_support', + 'stripe_price_id' => 'price_yearly_already_known', + ]); + + $before = count($this->stripe->prices); + + $id = app(AddonPrices::class)->ensure( + 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), + ); + + expect($id)->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw') + // Nothing new at Stripe: the orphan was taken over, not replaced. + ->and(count($this->stripe->prices))->toBe($before) + ->and(StripeAddonPrice::query() + ->where('addon_key', 'priority_support') + ->where('interval', 'month') + ->where('reverse_charge', false) + ->value('stripe_price_id'))->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw'); +}); + +it('does not block a booking because the metadata format moved', function () { + // 2026-07-29, exactly: orphan with the old metadata, code with the new. This + // is the call that answered HTTP 400 for twenty-four hours. + $this->stripe->plantPrice('price_1TygdEC7u8NpJ8pOt3nsoyYw', 'prod_support', + 3480, 'EUR', 'month', ['addon' => 'priority_support']); + StripeAddonPrice::query()->create([ + 'addon_key' => 'priority_support', 'reverse_charge' => false, + 'amount_cents' => 41760, 'net_cents' => 34800, 'currency' => 'EUR', + 'interval' => 'year', 'stripe_product_id' => 'prod_support', + 'stripe_price_id' => 'price_yearly_already_known', + ]); + + $id = app(AddonPrices::class)->ensure( + 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), + ); + + expect($id)->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw') + ->and($this->stripe->metadataUpdates)->toHaveCount(1); +}); + +it('leaves a frozen booking on the price it was sold at', function () { + app(Kernel::class)->call('stripe:sync-catalogue'); + + $sold = app(AddonPrices::class)->liveFor( + 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), + ); + + $subscription = Subscription::factory()->plan('team')->create(); + $booking = SubscriptionAddon::query()->create([ + 'subscription_id' => $subscription->id, + 'addon_key' => 'priority_support', + // Net, per month, per unit — frozen at booking. `currency` and + // `booked_at` are both NOT NULL (2026_07_26_060000), and `uuid` fills + // itself through the model's uniqueIds(). + 'price_cents' => 2900, + 'currency' => 'EUR', + 'quantity' => 1, + 'booked_at' => now(), + 'stripe_price_id' => $sold, + ]); + + // The catalogue moves, and somebody has already left an orphan at the new + // figure. Neither may reach a booking that is already frozen. + config()->set('provisioning.addons.priority_support.price_cents', 3900); + $this->stripe->plantPrice('price_orphan_new_figure', 'prod_support', + 4680, 'EUR', 'month', ['addon' => 'priority_support']); + + app(Kernel::class)->call('stripe:sync-catalogue'); + + expect($booking->refresh()->stripe_price_id)->toBe($sold) + ->and($this->stripe->archived)->not->toContain($sold); +});