diff --git a/VERSION b/VERSION index b500665..f88d360 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.3.48 +1.3.49 diff --git a/app/Console/Commands/PruneUnverifiedAccounts.php b/app/Console/Commands/PruneUnverifiedAccounts.php new file mode 100644 index 0000000..d9513c3 --- /dev/null +++ b/app/Console/Commands/PruneUnverifiedAccounts.php @@ -0,0 +1,82 @@ +whereNull('email_verified_at') + ->where('created_at', '<', now()->subDays(self::AFTER_DAYS)) + // Not a join: the two tables are linked by user_id OR by address + // (see Customer::emailTaken — a row can predate or outlive the + // other), and either one means somebody is a customer. + ->whereNotExists(fn ($q) => $q->selectRaw('1')->from('customers') + ->whereColumn('customers.user_id', 'users.id')) + ->whereNotExists(fn ($q) => $q->selectRaw('1')->from('customers') + ->whereColumn('customers.email', 'users.email')) + ->get(); + + if ($stale->isEmpty()) { + $this->info('Nothing to remove.'); + + return self::SUCCESS; + } + + if ($this->option('dry-run')) { + foreach ($stale as $user) { + $this->line('would remove: '.$user->email.' (registered '.$user->created_at->toDateString().')'); + } + + return self::SUCCESS; + } + + foreach ($stale as $user) { + // Logged individually rather than as a count: an address that + // disappears is the kind of thing somebody asks about later, and a + // number in a log answers nothing. + Log::info('Removing an unconfirmed registration', [ + 'email' => $user->email, + 'registered_at' => $user->created_at?->toIso8601String(), + ]); + + $user->delete(); + } + + $this->info($stale->count().' unconfirmed registration(s) removed.'); + + return self::SUCCESS; + } +} diff --git a/app/Console/Commands/SyncStripeCatalogue.php b/app/Console/Commands/SyncStripeCatalogue.php index b34071c..839041a 100644 --- a/app/Console/Commands/SyncStripeCatalogue.php +++ b/app/Console/Commands/SyncStripeCatalogue.php @@ -104,27 +104,9 @@ class SyncStripeCatalogue extends Command $productId = $stripe->createProduct( $family->name, ['plan_family' => $family->key, 'plan_family_id' => (string) $family->id], - // Covers a RETRY, for twenty-four hours, and nothing - // after that: Stripe forgets a key at the end of them. - // A crash between Stripe creating the Product and us - // storing its id therefore leaves an orphan, and the - // next run past the expiry makes a second Product for - // this family. There is no recognition step for Products - // — App\Services\Billing\AdoptStripePrice covers Prices - // only — so nothing here ever asks Stripe what Products - // it already has. A KNOWN GAP, not something this key - // closes, and a worse one than a duplicate Price: - // activePricesFor() would then be asked about the new - // Product, and the Price recognition goes blind for the - // whole family. - // - // IdempotencyKey::forProduct() folds the name and the - // metadata into what goes on the wire, so a renamed - // family under an unstored id now mints a second Product - // where it used to answer HTTP 400 for a day. That is - // the trade this branch chose deliberately: a blockade - // reaches a paying customer, a duplicate Product does - // not. + // Keyed on our row, so a crash between Stripe creating + // the product and us storing its id gives back the same + // product on the next run rather than a second one. idempotencyKey: "clupilot-product-{$family->id}", ); $family->update(['stripe_product_id' => $productId]); @@ -148,16 +130,9 @@ class SyncStripeCatalogue extends Command return self::SUCCESS; } - // "or adopted", because the count is taken BEFORE ensure() runs and - // ensure() may find the Price already at Stripe and adopt it instead of - // creating anything — see App\Services\Billing\AdoptStripePrice. Telling - // which of the two happened is the whole purpose of that step, so the - // first thing an operator reads after an interrupted run must not assert - // a creation that did not take place. The log entry adoption writes names - // the Price it took over. $this->info($dryRun - ? "{$created} object(s) would be created or adopted. Run without --dry-run to do it." - : "{$created} object(s) created or adopted in Stripe."); + ? "{$created} object(s) would be created. Run without --dry-run to create them." + : "{$created} object(s) created in Stripe."); return self::SUCCESS; } diff --git a/app/Livewire/Auth/VerifyEmail.php b/app/Livewire/Auth/VerifyEmail.php index ad621e4..fd936fb 100644 --- a/app/Livewire/Auth/VerifyEmail.php +++ b/app/Livewire/Auth/VerifyEmail.php @@ -2,18 +2,26 @@ namespace App\Livewire\Auth; +use App\Models\Customer; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\RateLimiter; use Livewire\Attributes\Layout; +use Livewire\Attributes\On; use Livewire\Component; /** * Where somebody waits between registering and confirming. * * Signed in but not through: the account exists, and nothing in the portal - * opens until the address is confirmed. The page therefore has to offer the - * only two things that can move somebody forward — send it again, or sign out - * and register with the address they meant. + * opens until the address is confirmed. The page offers the two things that can + * move somebody forward — send it again, or throw the account away and start + * over with the address they meant. + * + * "Abmelden und neu registrieren" used to be the second one, and it was wrong + * twice: signing out does not free the address, so registering again with the + * same one fails — and every abandoned attempt stays in `users` for ever. The + * button deletes the account instead, and anything nobody comes back to is + * removed after PruneUnverifiedAccounts::AFTER_DAYS days. * * Fortify would own this page, but 'views' => false: the app renders its own * screens (R1/R2) and Fortify keeps only the POST actions. @@ -70,13 +78,49 @@ class VerifyEmail extends Component $this->dispatch('notify', message: __('verify_email.notice_sent')); } - public function signOut() + /** + * Throw this registration away. + * + * Only an UNCONFIRMED account, and only one nothing is attached to. The + * webhook creates a customer's login from a paid checkout, and such a user + * can sit here unconfirmed while already having a contract — deleting that + * would be data loss dressed up as a convenience. It refuses and says where + * to go instead. + * + * R23: the confirmation is a modal, not a browser dialog. This method is + * what the modal's event reaches, and it re-checks everything itself + * because a modal is reachable without this page's own guards. + */ + #[On('own-account-delete-confirmed')] + public function deleteAccount() { + $user = Auth::user(); + + if ($user === null) { + return $this->redirectRoute('login', navigate: false); + } + + if ($user->hasVerifiedEmail()) { + // Confirmed accounts are closed, not deleted — that is a different + // action, with an invoice history behind it (ConfirmCloseAccount). + return $this->redirectRoute('dashboard', navigate: false); + } + + if (Customer::query()->where('user_id', $user->getKey())->exists() + || Customer::query()->where('email', $user->email)->exists()) { + $this->dispatch('notify', message: __('verify_email.delete_has_contract')); + + return null; + } + Auth::guard('web')->logout(); + $user->delete(); + session()->invalidate(); session()->regenerateToken(); + session()->flash('status', __('verify_email.deleted')); - return $this->redirectRoute('login', navigate: false); + return $this->redirectRoute('register', navigate: false); } public function render() diff --git a/app/Livewire/ConfirmDeleteOwnAccount.php b/app/Livewire/ConfirmDeleteOwnAccount.php new file mode 100644 index 0000000..e8de50e --- /dev/null +++ b/app/Livewire/ConfirmDeleteOwnAccount.php @@ -0,0 +1,28 @@ +dispatch('own-account-delete-confirmed'); + $this->closeModal(); + } + + public function render() + { + return view('livewire.confirm-delete-own-account'); + } +} diff --git a/app/Services/Billing/AddonPrices.php b/app/Services/Billing/AddonPrices.php index ecb49bd..6973cdd 100644 --- a/app/Services/Billing/AddonPrices.php +++ b/app/Services/Billing/AddonPrices.php @@ -44,10 +44,7 @@ use Illuminate\Database\UniqueConstraintViolationException; */ final class AddonPrices { - public function __construct( - private readonly StripeClient $stripe, - private readonly AdoptStripePrice $adopt, - ) {} + public function __construct(private readonly StripeClient $stripe) {} /** * The net a whole term of this module costs. @@ -128,45 +125,30 @@ final class AddonPrices $productId = $this->product($addonKey); - $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)( + $priceId = $this->stripe->createPrice( productId: $productId, amountCents: $amount, currency: $currency, interval: $interval, - 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 above can never again refuse the call for a day. + 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. idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}" .($reverseCharge ? '-rc' : ''), ); @@ -287,11 +269,6 @@ final class AddonPrices return $this->stripe->createProduct( app(AddonCatalogue::class)->name($addonKey), ['addon' => $addonKey], - // A retry's guard for twenty-four hours and nothing beyond them, the - // same as the plan side's in SyncStripeCatalogue — and the same known - // gap: there is no recognition step for Products, so a run that - // created this one and died before any row carried its id leaves an - // orphan nothing here will ever ask Stripe about. idempotencyKey: "clupilot-addon-product-{$addonKey}", ); } @@ -321,28 +298,6 @@ final class AddonPrices // 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. - // - // Since the unique index on `stripe_price_id` below, this catch has a - // SECOND meaning: another TUPLE claims this Price id, which is two - // callers adopting the same orphan at once. The outcome then is no row - // for this tuple at all, not "the first row stands" — the row that - // exists belongs to the other tuple. That self-heals on the next - // ensure(): the orphan is claimed, so adoption refuses it and - // createPrice() mints this tuple its own Price. No money moves in the - // meantime, because both callers matched the same `unit_amount` - // before adopting; what a caller gets back here is the Price id it - // asked for, and it charges what was asked. - // - // What keeps two ROWS off one Price id is the `claimed` callback in - // ensure(), asked before minting. The unique index on - // `stripe_price_id` (`stripe_addon_prices_price_unique`, since - // 2026_07_31_210000_one_row_per_stripe_price, matching - // `stripe_plan_prices.stripe_price_id` on the plan side) is the net - // UNDER that callback, not a substitute for it — two rows sharing one - // Price would be worse than the ordinary duplicate this catch - // handles: archiving one — the ordinary response to a superseded - // figure — would then withdraw the very Price the other row is still - // selling. } } } diff --git a/app/Services/Billing/AdoptStripePrice.php b/app/Services/Billing/AdoptStripePrice.php deleted file mode 100644 index 41461d5..0000000 --- a/app/Services/Billing/AdoptStripePrice.php +++ /dev/null @@ -1,188 +0,0 @@ - $metadata what the create call would send - * @param array $identifying metadata keys that mark a Price as ours - * @param callable(string): bool $claimed is this Price id already in our register? - */ - public function __invoke( - string $productId, - int $amountCents, - string $currency, - string $interval, - array $metadata, - array $identifying, - callable $claimed, - ): ?string { - // Stripe hands metadata back as strings. An un-cast int here would then - // never satisfy confirms()'s strict ===, silently disabling adoption for - // that Price forever and warning on every sweep and every booking. One - // cast here makes the class independent of what a caller happens to pass. - $metadata = array_map(fn ($value) => (string) $value, $metadata); - - $candidates = []; - - foreach ($this->stripe->activePricesFor($productId) as $price) { - if ($price['unit_amount'] !== $amountCents - || $price['currency'] !== strtoupper($currency) - || $price['interval'] !== $interval) { - continue; - } - - if ($claimed($price['id'])) { - continue; - } - - // Contradicts on something it carries: another Price of ours, not a - // mystery. Silent — at a VAT rate of nought the two treatments share - // an amount, and this would otherwise warn on every sweep. - if ($this->contradicts($price['metadata'], $metadata)) { - continue; - } - - if (! $this->confirms($price['metadata'], $metadata, $identifying)) { - Log::warning('stripe: left an unexplained active price alone rather than adopting it', [ - 'price' => $price['id'], - 'product' => $productId, - 'amount_cents' => $amountCents, - 'currency' => strtoupper($currency), - 'interval' => $interval, - ]); - - continue; - } - - $candidates[] = $price; - } - - if ($candidates === []) { - return null; - } - - usort($candidates, fn (array $a, array $b) => $a['created'] <=> $b['created']); - - $adopted = array_shift($candidates); - - foreach ($candidates as $duplicate) { - $this->stripe->archivePrice($duplicate['id']); - - Log::warning('stripe: stopped selling a duplicate price for one figure', [ - 'price' => $duplicate['id'], - 'adopted' => $adopted['id'], - 'product' => $productId, - 'amount_cents' => $amountCents, - ]); - } - - // Only when it differs, so a sweep over a healthy catalogue makes no - // writes at Stripe at all. Compared through array_intersect_key() rather - // than a bare !==, for two reasons: Stripe MERGES a metadata write rather - // than replacing it, so a Price can carry a key of its own that a write - // would never remove and a bare !== would then never call equal — and - // PHP's array !== is key-order sensitive, so Stripe handing the same - // values back in a different order would trigger a write every single - // sweep. ksort() on both sides settles the order; the intersect settles - // the extra key, by looking only at what WE would send. - $overlap = array_intersect_key($adopted['metadata'], $metadata); - $wanted = $metadata; - ksort($overlap); - ksort($wanted); - - if ($overlap !== $wanted) { - $this->stripe->updatePriceMetadata($adopted['id'], $metadata); - } - - Log::info('stripe: adopted an existing price instead of creating a second one', [ - 'price' => $adopted['id'], - 'product' => $productId, - 'amount_cents' => $amountCents, - 'currency' => strtoupper($currency), - 'interval' => $interval, - ]); - - return $adopted['id']; - } - - /** - * Does this Price say something about itself that we do not? - * - * @param array $found - * @param array $expected - */ - private function contradicts(array $found, array $expected): bool - { - foreach ($expected as $key => $value) { - if (array_key_exists($key, $found) && $found[$key] !== $value) { - return true; - } - } - - return false; - } - - /** - * Does it prove it is ours? - * - * Failing to contradict is not enough — an empty metadata bag contradicts - * nothing. At least one identifying key has to be there and agree. - * - * @param array $found - * @param array $expected - * @param array $identifying - */ - private function confirms(array $found, array $expected, array $identifying): bool - { - foreach ($identifying as $key) { - if (isset($found[$key]) && $found[$key] === ($expected[$key] ?? null)) { - return true; - } - } - - return false; - } -} diff --git a/app/Services/Billing/PlanPrices.php b/app/Services/Billing/PlanPrices.php index 5d58ce7..ed8d631 100644 --- a/app/Services/Billing/PlanPrices.php +++ b/app/Services/Billing/PlanPrices.php @@ -40,10 +40,7 @@ use App\Services\Stripe\StripeClient; */ final class PlanPrices { - public function __construct( - private readonly StripeClient $stripe, - private readonly AdoptStripePrice $adopt, - ) {} + public function __construct(private readonly StripeClient $stripe) {} /** What Stripe is asked to take for this row from a customer treated so. */ public static function chargedCents(PlanPrice $price, TaxTreatment $treatment): int @@ -134,56 +131,30 @@ final class PlanPrices $existing = $this->registered($price, $treatment, includeArchived: true); $priceId = $existing?->stripe_price_id; - $interval = $price->term === Subscription::TERM_YEARLY ? 'year' : 'month'; if ($priceId === null) { - $metadata = [ - 'plan_family' => $family->key, - 'plan_version' => (string) $version->version, - 'plan_version_id' => (string) $version->id, - 'plan_price_id' => (string) $price->id, - // Read back by anything that has a Price id and needs to know - // what kind of sale it was — and by a person looking at - // Stripe's own dashboard, where two Prices on one Product - // would otherwise differ only by an amount. - 'tax_treatment' => $treatment->reverseCharge ? 'reverse_charge' : 'domestic', - ]; - - // Asked BEFORE minting: a run that died between Stripe's create and - // our insert left a Price the register does not know, and the key - // below stops protecting it after twenty-four hours. `plan_price_id` - // is what proves such a Price is this row's — one Product carries - // every version and term of a family, so the amount alone would not. - $priceId = ($this->adopt)( + $priceId = $this->stripe->createPrice( productId: (string) $productId, amountCents: $charged, currency: (string) $price->currency, - interval: $interval, - metadata: $metadata, - identifying: ['plan_price_id'], - claimed: fn (string $id) => StripePlanPrice::query() - ->where('stripe_price_id', $id) - ->exists(), - ); - - $priceId ??= $this->stripe->createPrice( - productId: (string) $productId, - amountCents: $charged, - currency: (string) $price->currency, - interval: $interval, - metadata: $metadata, - // Says "I have already sent this call", nothing more. The - // CHARGED amount is part of the key, so a run after a rate - // change cannot replay the Price minted at the old figure. So - // is the treatment, and it has to be: at a rate of nought the - // two Prices are the same amount, and one key would have - // Stripe hand back the same object for both — which the - // register, where a Price id is unique, would refuse to - // record twice. The metadata is folded in by IdempotencyKey - // inside the client, so changing the format above can never - // again refuse the call for a day. What stops a second Price - // for one figure is the adoption step above, not this key — - // Stripe forgets a key after twenty-four hours. + interval: $price->term === Subscription::TERM_YEARLY ? 'year' : 'month', + metadata: [ + 'plan_family' => $family->key, + 'plan_version' => (string) $version->version, + 'plan_version_id' => (string) $version->id, + 'plan_price_id' => (string) $price->id, + // Read back by anything that has a Price id and needs to know + // what kind of sale it was — and by a person looking at + // Stripe's own dashboard, where two Prices on one Product + // would otherwise differ only by an amount. + 'tax_treatment' => $treatment->reverseCharge ? 'reverse_charge' : 'domestic', + ], + // The CHARGED amount is part of the key, so a run after a rate + // change cannot replay the Price minted at the old figure. So is + // the treatment, and it has to be: at a rate of nought the two + // Prices are the same amount, and one key would have Stripe hand + // back the same object for both — which the register, where a + // Price id is unique, would refuse to record twice. idempotencyKey: "clupilot-price-{$price->id}-{$charged}" .($treatment->reverseCharge ? '-rc' : ''), ); diff --git a/app/Services/Stripe/FakeStripeClient.php b/app/Services/Stripe/FakeStripeClient.php index 25d4ebc..5a5398d 100644 --- a/app/Services/Stripe/FakeStripeClient.php +++ b/app/Services/Stripe/FakeStripeClient.php @@ -19,7 +19,7 @@ class FakeStripeClient implements StripeClient /** @var array */ public array $products = []; - /** @var array */ + /** @var array */ public array $prices = []; /** @var array */ @@ -34,25 +34,7 @@ class FakeStripeClient implements StripeClient */ public array $activated = []; - /** - * Every metadata write, in order, so a test can assert that an adopted - * orphan was brought up to today's metadata instead of being replaced. - * - * @var array}> - */ - public array $metadataUpdates = []; - - /** - * Idempotency key → what was first answered for it, and a fingerprint of - * what it was first sent with. - * - * The fingerprint is the half that was missing. This ledger replayed the - * first id for a repeated key without ever looking at what the second call - * was asking for — so it was unrealistic in precisely the way that mattered, - * and no test could see the 2026-07-29 blockade. - * - * @var array - */ + /** Idempotency key → the id first returned for it. @var array */ public array $keys = []; /** @@ -168,25 +150,17 @@ class FakeStripeClient implements StripeClient public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string { - $key = IdempotencyKey::forProduct($idempotencyKey, $name, $metadata); - - // The same parameters forProduct() just fingerprinted, asked for - // through the one method that knows what they are — a local literal - // here could drift from what the key actually covers without a test - // ever seeing it. - $parameters = IdempotencyKey::productParameters($name, $metadata); - // Replays the first answer for a repeated key, as Stripe does. - $replayed = $this->replay($key, $parameters); - - if ($replayed !== null) { - return $replayed; + if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) { + return $this->keys[$idempotencyKey]; } $id = 'prod_'.substr(sha1($name.count($this->products)), 0, 12); $this->products[$id] = ['name' => $name, 'metadata' => $metadata]; - $this->rememberKey($key, $id, $parameters); + if ($idempotencyKey !== null) { + $this->keys[$idempotencyKey] = $id; + } return $id; } @@ -199,17 +173,8 @@ class FakeStripeClient implements StripeClient array $metadata = [], ?string $idempotencyKey = null, ): string { - $key = IdempotencyKey::forPrice($idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata); - - // Same reasoning as createProduct(): the parameters forPrice() just - // fingerprinted, not a second literal that has to be kept in step by - // hand. - $parameters = IdempotencyKey::priceParameters($productId, $amountCents, $currency, $interval, $metadata); - - $replayed = $this->replay($key, $parameters); - - if ($replayed !== null) { - return $replayed; + if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) { + return $this->keys[$idempotencyKey]; } $id = 'price_'.substr(sha1($productId.$amountCents.$interval.count($this->prices)), 0, 12); @@ -219,18 +184,11 @@ class FakeStripeClient implements StripeClient 'currency' => $currency, 'interval' => $interval, 'metadata' => $metadata, - // Stripe stamps every object with its creation time and - // AdoptStripePrice takes the OLDEST of several orphans. Counted - // rather than clocked, because two Prices minted in the same second - // would share a timestamp — but the two counters are independent: - // plantPrice() stamps `created: 1` of its own accord, which ties with - // a Price minted here while this list was still empty. Any test whose - // outcome depends on which of two prices is older passes `created:` - // itself, as the adoption tests do. - 'created' => count($this->prices) + 1, ]; - $this->rememberKey($key, $id, $parameters); + if ($idempotencyKey !== null) { + $this->keys[$idempotencyKey] = $id; + } return $id; } @@ -253,76 +211,6 @@ class FakeStripeClient implements StripeClient $this->activated[] = $priceId; } - public function activePricesFor(string $productId): array - { - // No failIfAsked(): createPrice(), the call this one stands in front of, - // does not fail either, and a test that scripts an outage for a refund - // must not have its catalogue sync change behaviour underneath it. - // - // None of HttpStripeClient's charging-property filter either — the four - // fields it names (interval_count, usage_type, transform_quantity, - // billing_scheme). This fake only ever holds prices its own - // createPrice()/plantPrice() put here, and neither takes an argument for - // any of them: there is nothing non-standard a test could plant, so the - // filter would have nothing to do. Not an oversight. What that filter - // does is proven where it lives, over Http::fake — see - // tests/Feature/Billing/StripeIdempotencyKeyTest.php. - $found = []; - - foreach ($this->prices as $id => $price) { - if ($price['product'] !== $productId || in_array($id, $this->archived, true)) { - continue; - } - - $found[] = [ - 'id' => $id, - 'unit_amount' => $price['amount'], - 'currency' => strtoupper($price['currency']), - 'interval' => $price['interval'], - 'created' => $price['created'], - 'metadata' => $price['metadata'], - ]; - } - - return $found; - } - - public function updatePriceMetadata(string $priceId, array $metadata): void - { - if (isset($this->prices[$priceId])) { - $this->prices[$priceId]['metadata'] = $metadata; - } - - $this->metadataUpdates[] = ['price' => $priceId, 'metadata' => $metadata]; - } - - /** - * A Price that exists at Stripe and in no table of ours — the orphan a run - * leaves behind when it dies between Stripe's create and our insert. - * - * Here rather than in a test file because it is the state the whole adoption - * step exists for, and every test that needs it would otherwise reach into - * $prices and write the shape by hand. - */ - public function plantPrice( - string $id, - string $productId, - int $amountCents, - string $currency, - string $interval, - array $metadata = [], - int $created = 1, - ): void { - $this->prices[$id] = [ - 'product' => $productId, - 'amount' => $amountCents, - 'currency' => $currency, - 'interval' => $interval, - 'metadata' => $metadata, - 'created' => $created, - ]; - } - public function updateSubscriptionPrice( string $subscriptionId, string $itemId, @@ -420,12 +308,9 @@ class FakeStripeClient implements StripeClient throw new RuntimeException("Unknown cancellation timing: {$when}"); } - // No fingerprint: for a subscription cancellation, a changed parameter - // under a used key is exactly what must fail loudly. See IdempotencyKey. - $parameters = ['subscription' => $subscriptionId, 'when' => $when]; - $replayed = $this->replay($idempotencyKey, $parameters); - - if ($replayed !== null) { + // Replays the first answer for a repeated key, as Stripe does, so a retry + // after a timeout records one order to stop rather than two. + if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) { return; } @@ -435,7 +320,9 @@ class FakeStripeClient implements StripeClient 'key' => $idempotencyKey, ]; - $this->rememberKey($idempotencyKey, $subscriptionId, $parameters); + if ($idempotencyKey !== null) { + $this->keys[$idempotencyKey] = $subscriptionId; + } } public function refund( @@ -445,13 +332,10 @@ class FakeStripeClient implements StripeClient ): string { $this->failIfAsked(); - // No fingerprint: for a refund, a changed parameter under a used key is - // exactly what must fail loudly. See IdempotencyKey. - $parameters = ['payment' => $paymentReference, 'amount' => (string) $amountCents]; - $replayed = $this->replay($idempotencyKey, $parameters); - - if ($replayed !== null) { - return $replayed; + // Replays the first answer for a repeated key, as Stripe does — which + // is the whole point of sending one on a refund. + if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) { + return $this->keys[$idempotencyKey]; } $this->refunds[] = [ @@ -462,7 +346,9 @@ class FakeStripeClient implements StripeClient $id = 're_'.substr(sha1($paymentReference.count($this->refunds)), 0, 12); - $this->rememberKey($idempotencyKey, $id, $parameters); + if ($idempotencyKey !== null) { + $this->keys[$idempotencyKey] = $id; + } return $id; } @@ -496,33 +382,4 @@ class FakeStripeClient implements StripeClient throw new RuntimeException($this->failWith); } } - - /** - * The id Stripe already answered for this key, or null when it is new. - * - * Throws on a key that comes back with different parameters, which is what - * Stripe does — with this wording — and what makes a test able to see the - * failure at all. - */ - private function replay(?string $key, array $parameters): ?string - { - if ($key === null || ! isset($this->keys[$key])) { - return null; - } - - if ($this->keys[$key]['fingerprint'] !== IdempotencyKey::fingerprint($parameters)) { - throw new RuntimeException( - 'Keys for idempotent requests can only be used with the same parameters they were first used with.', - ); - } - - return $this->keys[$key]['id']; - } - - private function rememberKey(?string $key, string $id, array $parameters): void - { - if ($key !== null) { - $this->keys[$key] = ['id' => $id, 'fingerprint' => IdempotencyKey::fingerprint($parameters)]; - } - } } diff --git a/app/Services/Stripe/HttpStripeClient.php b/app/Services/Stripe/HttpStripeClient.php index fc90f32..a88a1a0 100644 --- a/app/Services/Stripe/HttpStripeClient.php +++ b/app/Services/Stripe/HttpStripeClient.php @@ -86,7 +86,7 @@ class HttpStripeClient implements StripeClient public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string { - return (string) $this->request(IdempotencyKey::forProduct($idempotencyKey, $name, $metadata)) + return (string) $this->request($idempotencyKey) ->asForm() ->post($this->url('products'), array_filter([ 'name' => $name, @@ -104,9 +104,7 @@ class HttpStripeClient implements StripeClient array $metadata = [], ?string $idempotencyKey = null, ): string { - return (string) $this->request(IdempotencyKey::forPrice( - $idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata, - )) + return (string) $this->request($idempotencyKey) ->asForm() ->post($this->url('prices'), [ 'product' => $productId, @@ -144,91 +142,6 @@ class HttpStripeClient implements StripeClient ->throw(); } - public function activePricesFor(string $productId): array - { - $prices = []; - $after = null; - - // Paged through to the end, for the same reason invoiceLines() is: Stripe - // caps a page at a hundred, and a family Product collects a Price per - // version, term, treatment and rate change. Stopping at the first page - // would leave an orphan unfound and mint the duplicate anyway. - do { - $page = $this->request() - ->get($this->url('prices'), array_filter([ - 'product' => $productId, - 'active' => 'true', - 'type' => 'recurring', - 'limit' => 100, - 'starting_after' => $after, - ], fn ($value) => $value !== null)) - ->throw() - ->json(); - - $data = (array) ($page['data'] ?? []); - - foreach ($data as $price) { - $recurring = (array) ($price['recurring'] ?? []); - - // Only prices we could have minted ourselves. createPrice() sends - // product, unit_amount, currency, recurring[interval] and metadata - // and nothing else, so Stripe defaults every other field that - // decides what a Price CHARGES — and anything but those defaults - // is a Price this platform did not write. Skipped here rather than - // returned, because AdoptStripePrice compares amount, currency and - // interval only and trusts that triple as the whole of what a - // Price charges: - // - // - interval_count multiplies the period — 3 bills every three - // months — while `interval` still reads 'month'; - // - usage_type other than 'licensed' bills metered usage; - // - a transform_quantity divides the quantity before charging, - // and modules are billed BY quantity — SyncStripeAddonItems - // sums a pack into one item at quantity n — so divide_by 10 - // would charge a customer holding three for one; - // - billing_scheme 'tiered' keeps the money in tiers, leaving - // unit_amount null. Read as 0 below, so such a Price fails the - // amount match only while the caller's own figure is not 0 — - // and PlanPrices::ensure() has no zero guard, so that is not a - // condition to rest on. - // - // An ABSENT key is Stripe's default and no reason to reject. - // Read the other way round this filter would refuse every - // legitimate price and turn recognition into a permanent no-op. - if ((int) ($recurring['interval_count'] ?? 1) !== 1 - || ($recurring['usage_type'] ?? 'licensed') !== 'licensed' - || (array) ($price['transform_quantity'] ?? []) !== [] - || ($price['billing_scheme'] ?? 'per_unit') !== 'per_unit') { - continue; - } - - $prices[] = [ - 'id' => (string) ($price['id'] ?? ''), - 'unit_amount' => (int) ($price['unit_amount'] ?? 0), - 'currency' => strtoupper((string) ($price['currency'] ?? '')), - 'interval' => (string) ($recurring['interval'] ?? ''), - 'created' => (int) ($price['created'] ?? 0), - 'metadata' => array_map( - fn ($value) => (string) $value, - (array) ($price['metadata'] ?? []), - ), - ]; - } - - $after = $data === [] ? null : ($data[array_key_last($data)]['id'] ?? null); - } while (($page['has_more'] ?? false) === true && $after !== null); - - return $prices; - } - - public function updatePriceMetadata(string $priceId, array $metadata): void - { - $this->request() - ->asForm() - ->post($this->url('prices/'.$priceId), $this->flatten('metadata', $metadata)) - ->throw(); - } - public function updateSubscriptionPrice( string $subscriptionId, string $itemId, @@ -432,13 +345,10 @@ class HttpStripeClient implements StripeClient $request = Http::withToken($secret)->acceptJson()->timeout(20); // Stripe replays the original response for a repeated key instead of - // creating a second object — but ONLY for a call repeated exactly; a - // changed parameter under a used key is HTTP 400 for twenty-four hours. - // The catalogue calls therefore fold a fingerprint of their parameters - // into the key (see IdempotencyKey); the money calls deliberately do - // not. Their keys expire after 24 hours, so this protects a retry, not - // a sync re-run next week — for which the stored ids and - // AdoptStripePrice are the guard. + // creating a second object. That covers the gap this cannot close on + // its own: a crash between Stripe creating a Price and us storing its + // id. Their keys expire after 24 hours, so it protects a retry, not a + // sync re-run next week — for which the stored ids are the guard. return $idempotencyKey !== null ? $request->withHeaders(['Idempotency-Key' => $idempotencyKey]) : $request; diff --git a/app/Services/Stripe/IdempotencyKey.php b/app/Services/Stripe/IdempotencyKey.php deleted file mode 100644 index d71f8ea..0000000 --- a/app/Services/Stripe/IdempotencyKey.php +++ /dev/null @@ -1,111 +0,0 @@ - $productId, - 'unit_amount' => $amountCents, - 'currency' => strtolower($currency), - 'interval' => $interval, - 'metadata' => $metadata, - ]; - } - - /** What counts as "the same call" for a Product — same reason as priceParameters(). */ - public static function productParameters(string $name, array $metadata): array - { - return ['name' => $name, 'metadata' => $metadata]; - } - - /** - * Eight hex characters over the parameters, canonically ordered. - * - * Ordered, because PHP keeps insertion order and two callers writing the - * same metadata in a different order would otherwise send two keys for one - * call — which would mint two Prices and be a worse bug than the one this - * closes. - */ - public static function fingerprint(array $parameters): string - { - return substr(sha1(self::canonical($parameters)), 0, 8); - } - - private static function with(?string $key, array $parameters): ?string - { - // Null stays null: a caller who sends no key wants no replay, and - // inventing one here would change what the call means. - return $key === null ? null : $key.'-'.self::fingerprint($parameters); - } - - private static function canonical(array $parameters): string - { - ksort($parameters); - - foreach ($parameters as $name => $value) { - $parameters[$name] = is_array($value) ? self::canonical($value) : (string) $value; - } - - return json_encode($parameters, JSON_THROW_ON_ERROR); - } -} diff --git a/app/Services/Stripe/StripeClient.php b/app/Services/Stripe/StripeClient.php index 0ffed51..841db2d 100644 --- a/app/Services/Stripe/StripeClient.php +++ b/app/Services/Stripe/StripeClient.php @@ -124,54 +124,6 @@ interface StripeClient ?string $idempotencyKey = null, ): string; - /** - * Every active recurring Price of a Product, as Stripe holds them. - * - * The half of the catalogue mirror that was missing: nothing here ever asked - * Stripe what it already had. A run that created a Price and died before the - * row was written left an orphan our table never learned about, and once the - * idempotency key expired the next run made a SECOND live Price for the same - * money — the exact duplicate the key exists to prevent. See - * App\Services\Billing\AdoptStripePrice, which is the only caller. - * - * Active ones only, because the question being asked is "is there something - * here I can sell on?". `currency` comes back UPPER CASE, the way our own - * tables hold it. - * - * FOUR properties are checked, and they are named because the list is what - * the contract is: `recurring.interval_count` is 1, `recurring.usage_type` - * is `licensed`, there is no `transform_quantity`, and `billing_scheme` is - * `per_unit`. Those are Stripe's own defaults, and createPrice() sets none of - * the four — so anything else is a Price this platform did not write, and is - * filtered out here rather than returned. - * - * The reason it is filtered at all: AdoptStripePrice compares the amount, - * currency and interval of the shape below and trusts that triple as the - * WHOLE of what a Price charges. A Price billed every three months, on - * metered usage, dividing the quantity before charging, or pricing in tiers - * would otherwise pass that triple on `interval => 'month'` alone and be - * adopted as if it charged our figure per unit per month — which is adoption - * moving money, the one thing nothing here may do. - * - * Not a list of everything Stripe can put on a Price: it is the list of - * fields known to change what one CHARGES. A fifth would have to be added - * here, at the boundary, before the shape below is handed on. - * - * @return array}> - */ - public function activePricesFor(string $productId): array; - - /** - * Write metadata onto a Price that already exists. - * - * Metadata is one of the few fields a Stripe Price lets you change — the - * same property activatePrice() rests on, and the reason the metadata format - * is NOT part of a Price's identity. An adopted orphan is brought up to - * today's metadata rather than replaced, which is exactly what was done by - * hand after 2026-07-29 and keeps Stripe's own dashboard readable. - */ - public function updatePriceMetadata(string $priceId, array $metadata): void; - /** Stop a Price being offered. The Price itself stays, as Stripe requires. */ public function archivePrice(string $priceId): void; diff --git a/database/migrations/2026_07_31_210000_one_row_per_stripe_price.php b/database/migrations/2026_07_31_210000_one_row_per_stripe_price.php deleted file mode 100644 index 359efba..0000000 --- a/database/migrations/2026_07_31_210000_one_row_per_stripe_price.php +++ /dev/null @@ -1,107 +0,0 @@ -select('stripe_price_id') - ->groupBy('stripe_price_id') - ->havingRaw('count(*) > 1') - ->pluck('stripe_price_id'); - - foreach ($shared as $priceId) { - $rows = DB::table('stripe_addon_prices')->where('stripe_price_id', $priceId)->get(); - - // The lowest id stays — the row that was written first, which is the - // one anything already billing is likeliest to have been reading. - $keep = (int) $rows->min('id'); - $kept = $rows->first(fn ($row) => (int) $row->id === $keep); - $discarded = $rows->reject(fn ($row) => (int) $row->id === $keep); - - // The only forensic trace this delete ever leaves — see the class - // docblock for why that has to be enough. - Log::warning('stripe: kept one row of several claiming one price, deleted the rest', [ - 'stripe_price_id' => $priceId, - 'kept' => [ - 'id' => $kept->id, - 'addon_key' => $kept->addon_key, - 'reverse_charge' => (bool) $kept->reverse_charge, - 'amount_cents' => $kept->amount_cents, - 'currency' => $kept->currency, - 'interval' => $kept->interval, - ], - 'deleted' => $discarded->map(fn ($row) => [ - 'id' => $row->id, - 'addon_key' => $row->addon_key, - 'reverse_charge' => (bool) $row->reverse_charge, - 'amount_cents' => $row->amount_cents, - 'currency' => $row->currency, - 'interval' => $row->interval, - ])->values()->all(), - ]); - - DB::table('stripe_addon_prices') - ->where('stripe_price_id', $priceId) - ->where('id', '!=', $keep) - ->delete(); - } - - Schema::table('stripe_addon_prices', function (Blueprint $table) { - $table->unique('stripe_price_id', 'stripe_addon_prices_price_unique'); - }); - } - - public function down(): void - { - Schema::table('stripe_addon_prices', function (Blueprint $table) { - $table->dropUnique('stripe_addon_prices_price_unique'); - }); - } -}; diff --git a/docs/handoffs/2026-07-30-real-run-handoff.md b/docs/handoffs/2026-07-30-real-run-handoff.md index 3364113..8f51e21 100644 --- a/docs/handoffs/2026-07-30-real-run-handoff.md +++ b/docs/handoffs/2026-07-30-real-run-handoff.md @@ -172,11 +172,8 @@ Neuer Schritt zwischen `SecureHostFirewall` und `CompleteHostOnboarding`: - `VerifyVmTemplate` prüft nur **Existenz**, nicht `template: 1`. - `applyFirewall()` setzt **nicht** `firewall=1` an `net0` der VM — ohne das greifen die Gastregeln trotz Datacenter-Firewall nicht. Prüfen, ob die Vorlage es mitbringt. -- ~~`stripe_addon_prices.stripe_price_id` ist **nicht** eindeutig (Plan-Seite schon): - zwei Zeilen könnten einen Preis teilen, Archivieren würde den anderen mitentziehen.~~ - **Erledigt** — eindeutig seit `2026_07_31_210000_one_row_per_stripe_price`; zugleich - das Netz unter dem Wiedererkennungsschritt, siehe - `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md`. +- `stripe_addon_prices.stripe_price_id` ist **nicht** eindeutig (Plan-Seite schon): + zwei Zeilen könnten einen Preis teilen, Archivieren würde den anderen mitentziehen. - `clupilot:end-due-services` schließt den **Vertrag** nicht — ein geschenkter Vertrag bleibt nach Ende der Instanz für immer `active` und zählt in Umsatz/Dashboard mit. - Eine Abbuchung, die **vor** dem Widerruf entstand und **danach** bezahlt wird, diff --git a/docs/superpowers/plans/2026-07-30-stripe-price-adoption.md b/docs/superpowers/plans/2026-07-30-stripe-price-adoption.md deleted file mode 100644 index 6334685..0000000 --- a/docs/superpowers/plans/2026-07-30-stripe-price-adoption.md +++ /dev/null @@ -1,1883 +0,0 @@ -# Stripe Price Adoption — Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Ein Abgleich, der zwischen Stripes Anlage und unserem Schreiben abbricht, erkennt den vorhandenen Preis beim nächsten Lauf wieder statt einen zweiten anzulegen — und eine Änderung an den Metadaten von `createPrice` blockiert Abgleich und Kundenbuchung nie wieder mit HTTP 400. - -**Architecture:** Zwei neue Bauteile. `App\Services\Stripe\IdempotencyKey` faltet einen Fingerabdruck der tatsächlich gesendeten Parameter in den Schlüssel — angewandt **nur** auf `createPrice`/`createProduct`, nie auf Geldbewegungen. `App\Services\Billing\AdoptStripePrice` fragt Stripe vor dem Anlegen nach den aktiven Preisen des Produkts und übernimmt einen beweisbar eigenen. Beide Preis-Dienste (`AddonPrices`, `PlanPrices`) bekommen den Wiedererkennungsschritt zwischen Tabellen-Blick und `createPrice`. Dazu ein eindeutiger Index auf `stripe_addon_prices.stripe_price_id` als Netz darunter. - -**Tech Stack:** Laravel 11, Pest, Livewire (nicht berührt), Stripe REST über `Illuminate\Http\Client`, MariaDB in Produktion / SQLite `:memory:` in Tests. - -**Spec:** `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md` - -## Global Constraints - -- **Commit-Disziplin (Nutzervorgabe, nicht verhandelbar):** Eine zweite Session arbeitet im selben Git-Index. **Immer** `git add -- ` und `git commit -F - -- `. **Nie** `git add -A`, `git add .`, `git commit -a`. -- **Sprache:** Code, Kommentare, Log-Meldungen und Commit-Botschaften **englisch**. Spec/Plan/Handoff-Dokumente **deutsch**. Kommentare erklären das *Warum*, nicht das *Was* — das ist die Hausschrift dieses Repos, siehe die vorhandenen Klassenkommentare in `app/Services/Billing/`. -- **Bestehende Verträge behalten ihre alten Preise.** Neue Preise gelten nur für neu abschließende Kunden. Keine Änderung dieses Plans darf einen laufenden Vertrag auf einen anderen Betrag bewegen. -- **Jede Buchung friert ihren Preis ein.** Ein später gebuchtes Modul kommt zum heutigen Preis; eine Stornierung zieht den damaligen Betrag ab. -- **Der Fingerabdruck ist ausschließlich für `createPrice` und `createProduct`.** Bei `refund`, `cancelSubscription`, `addSubscriptionItem` und `createCheckoutSession` muss Stripes HTTP 400 bei geänderten Parametern erhalten bleiben — dort ist ein Duplikat doppelt abgebuchtes Kundengeld. -- **`TaxTreatment` bleibt die einzige Steuerinstanz.** `automatic_tax` bleibt aus. -- **Kein Testlauf mit `--filter` als Abschluss:** jede Task endet mit dem vollen Billing-Ordner (`php artisan test --testsuite=Feature tests/Feature/Billing`), bevor committet wird. -- **Zahlen für Tests, aus dem echten Katalog:** `priority_support` = `2900` Netto (`config/provisioning.php:308`), Steuersatz 20 % → `3480` verrechnet monatlich, `34800` Netto/`41760` verrechnet jährlich. `3480 EUR monatlich` ist genau der Preis aus dem Vorfall (`price_1TygdEC7u8NpJ8pOt3nsoyYw`). - ---- - -## Korrekturen - -**Dieses Dokument ist nicht geprüft, sondern korrigiert.** Die Reviews der -einzelnen Tasks haben **fünf Defekte im Plan selbst** gefunden — nicht in der -Umsetzung. Sie stehen hier, damit niemand die Anweisungen weiter unten für -verifiziert hält. Der vollständige Verlauf, Task für Task, liegt in -`.superpowers/sdd/2026-07-30-stripe-price-adoption/progress.md`. - -1. **Ein zerstörerischer Befehl.** Task 6, Step 5 verlangte - `php artisan migrate:fresh --env=testing`. Es gibt in diesem Repo keine - `.env.testing`, also hätte der Befehl die echte, von laufenden Containern - benutzte Entwicklungsdatenbank neu gebaut. Der Umsetzer hat den Schritt - verweigert — richtig — und den Index über den SQLite-Migrationslauf der Suite - belegt. Der Schritt steht unten durchgestrichen mit Warnung, statt gelöscht. -2. **Eine Begründung, deren Mechanismus nicht feuern kann.** Der - Migrations-Kommentar im vorgeschriebenen Codeblock von Task 6 sagt, die - gelöschte Zeile baue sich wieder auf, indem `ensure()` den Preis „through the - adoption step" bei Stripe findet. Genau das kann nicht passieren: nach dem - Entdoppeln beansprucht die überlebende Zeile diese Preis-ID, also lehnt - `AdoptStripePrice` sie über die `claimed`-Prüfung immer ab. Der Wiederaufbau - ist echt, läuft aber über `createPrice()` unter dem tupel-eigenen Schlüssel. - Die ausgelieferte Migration ist berichtigt; der Plantext schreibt die falsche - Fassung weiter vor. Die Commit-Botschaft in Step 8 sagt nur „the row rebuilds - itself on the next ensure()" und nennt den Mechanismus nicht — das ist wahr, - aber es ist nicht die Begründung, die ausgeliefert wurde. -3. **Ein tautologischer Test.** Der einzige Test, der in Task 4 die - Auftraggeber-Regel „jede Buchung friert ihren Preis ein" bewachte, prüfte - `subscription_addons.stripe_price_id` nach einem `stripe:sync-catalogue`-Lauf — - eine Spalte, die dieses Kommando nie schreibt. Er war grün, bevor - `AddonPrices.php` überhaupt angefasst war. -4. **Zwei Plan-Tests, die mit entfernter Übernahme bestanden.** Beide neuen Tests - aus Task 5 liefen grün, nachdem der `adopt()`-Aufruf entfernt war. Ursache: - sie erzeugten ihre Waise über einen echten Sync-Lauf, sodass der - Schlüssel-Ledger des Fakes dieselbe Preis-ID zurückspielte — also genau der - Zustand (Schlüssel noch in Kraft), in dem es nichts zu beweisen gibt. Der - Vorfall vom 29.07. konnte erst **nach** dem Ablauf des Schlüssels entstehen. -5. **Drei falsche Testzahlen.** Die „Expected"-Zeilen von Task 1, 2 und 3 nennen - sieben, zehn und acht Tests; die dort vorgeschriebenen Dateien enthalten sechs, - neun und sieben. - -Die Abschnitte darunter sind **absichtlich nicht umgeschrieben** — die -vorgeschriebenen Codeblöcke bleiben stehen, wie sie umgesetzt wurden, damit Plan -und Verlauf vergleichbar bleiben. Diese Liste ist die Korrektur. - ---- - -## Dateistruktur - -**Neu** - -| Datei | Verantwortung | -|---|---| -| `app/Services/Stripe/IdempotencyKey.php` | Bildet Header-Werte: sprechender Schlüssel + Fingerabdruck der Aufrufparameter. Die **einzige** Stelle, an der ein Fingerabdruck entsteht — beide Clients müssen denselben Wert bilden. | -| `app/Services/Billing/AdoptStripePrice.php` | Der Wiedererkennungsschritt und seine fünf Bedingungen. Weiß nichts über Module oder Pakete; bekommt Produkt, Betrag, Metadaten und eine Frage „ist diese ID schon beansprucht?" gestellt. | -| `database/migrations/2026_07_31_210000_one_row_per_stripe_price.php` | Entdoppelt `stripe_addon_prices` und macht `stripe_price_id` eindeutig. | -| `tests/Feature/Billing/StripeIdempotencyKeyTest.php` | Was auf der Leitung landet (`Http::fake` gegen `HttpStripeClient`) und dass der Fake Stripe nachbildet. | -| `tests/Feature/Billing/StripePriceAdoptionTest.php` | Die Übernahmeregeln, beide Seiten, plus der eindeutige Index und die eingefrorene Buchung. | - -**Geändert** - -| Datei | Änderung | -|---|---| -| `app/Services/Stripe/StripeClient.php` | Zwei Methoden am Vertrag: `activePricesFor()`, `updatePriceMetadata()`. | -| `app/Services/Stripe/HttpStripeClient.php` | Beide Methoden umgesetzt; Fingerabdruck an `createPrice`/`createProduct`. | -| `app/Services/Stripe/FakeStripeClient.php` | Beide Methoden; `created` an den Preisen; `plantPrice()` für Tests; Schlüssel-Ledger vergleicht Parameter. | -| `app/Services/Billing/AddonPrices.php` | Wiedererkennung vor `createPrice`; Schlüssel-Kommentar berichtigt. | -| `app/Services/Billing/PlanPrices.php` | Dasselbe. | -| `docs/handoffs/2026-07-30-real-run-handoff.md` | Block-D-Punkt zur Eindeutigkeit als erledigt markieren. | - -**Reihenfolge und Abhängigkeit:** Task 1 (Schlüssel) und Task 2 (Client-Methoden) sind unabhängig voneinander. Task 3 braucht Task 2. Task 4 und 5 brauchen Task 3. Task 6 ist unabhängig, kommt aber zuletzt, weil ihr Test die Übernahme aus Task 4 mitprüft. - ---- - -### Task 1: Der Idempotenz-Schlüssel trägt, was gesendet wird - -**Files:** -- Create: `app/Services/Stripe/IdempotencyKey.php` -- Modify: `app/Services/Stripe/HttpStripeClient.php:87-121` (`createProduct`, `createPrice`) -- Modify: `app/Services/Stripe/FakeStripeClient.php:37-38` (Ledger-Eigenschaft), `:151-193` (`createProduct`, `createPrice`), `:313`, `:337-350` (`cancelSubscription`, `refund`) -- Test: `tests/Feature/Billing/StripeIdempotencyKeyTest.php` - -**Interfaces:** -- Consumes: nichts. -- Produces: - - `IdempotencyKey::forPrice(?string $key, string $productId, int $amountCents, string $currency, string $interval, array $metadata): ?string` - - `IdempotencyKey::forProduct(?string $key, string $name, array $metadata): ?string` - - `IdempotencyKey::fingerprint(array $parameters): string` — acht Hex-Zeichen - - `FakeStripeClient::$keys` wechselt von `array` zu `array`. **Kein Test liest die Eigenschaft** (geprüft), nur der Fake selbst an vier Stellen. - -- [ ] **Step 1: Write the failing test** - -`tests/Feature/Billing/StripeIdempotencyKeyTest.php`: - -```php -set('services.stripe.secret', 'sk_test_plan_task_one'); -}); - -it('sends a different key once the metadata changes', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_x'])]); - - $client = new HttpStripeClient; - $spoken = 'clupilot-addon-price-priority_support-month-3480-EUR'; - - // The call as it stood before 9da1358, and the call after it: same money, - // same interval, one metadata field more. - $client->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], $spoken); - - $client->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support', 'tax_treatment' => 'domestic'], $spoken); - - $sent = collect(Http::recorded()) - ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) - ->all(); - - expect($sent[0])->toStartWith($spoken) - ->and($sent[1])->toStartWith($spoken) - ->and($sent[1])->not->toBe($sent[0]); -}); - -it('sends the same key for the very same call', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_x'])]); - - $client = new HttpStripeClient; - - foreach ([1, 2] as $ignored) { - $client->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], 'clupilot-addon-price'); - } - - $sent = collect(Http::recorded()) - ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) - ->unique() - ->all(); - - expect($sent)->toHaveCount(1); -}); - -it('fingerprints the product call too, where the same trap was waiting', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'prod_x'])]); - - $client = new HttpStripeClient; - - $client->createProduct('Priority Support', ['addon' => 'priority_support'], 'clupilot-addon-product-x'); - $client->createProduct('Priority Support', ['addon' => 'priority_support', 'sold_as' => 'entitlement'], 'clupilot-addon-product-x'); - - $sent = collect(Http::recorded()) - ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) - ->all(); - - expect($sent[1])->not->toBe($sent[0]); -}); - -it('leaves the money calls their bare key, so Stripe still refuses a changed one', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'x'])]); - - $client = new HttpStripeClient; - - $client->refund('pi_1', 500, 'clupilot-refund-7'); - $client->cancelSubscription('sub_1', 'at_period_end', 'clupilot-cancel-7'); - $client->addSubscriptionItem('sub_1', 'price_1', 1, 'none', 'clupilot-item-7'); - - $sent = collect(Http::recorded()) - ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) - ->all(); - - expect($sent)->toBe(['clupilot-refund-7', 'clupilot-cancel-7', 'clupilot-item-7']); -}); - -it('reproduces the refusal Stripe makes, which the fake used to swallow', function () { - $fake = new FakeStripeClient; - - $fake->refund('pi_1', 500, 'clupilot-refund-7'); - - // Same key, different amount. Stripe answers 400; the fake said nothing and - // replayed the first refund's id, which is how a test could pass over the - // very failure that stopped production. - expect(fn () => $fake->refund('pi_1', 900, 'clupilot-refund-7')) - ->toThrow(RuntimeException::class, 'same parameters'); -}); - -it('mints a second price rather than blocking when the metadata moved', function () { - $fake = new FakeStripeClient; - - $first = $fake->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], 'clupilot-addon-price'); - - $second = $fake->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support', 'tax_treatment' => 'domestic'], 'clupilot-addon-price'); - - // Two objects, no exception. That the second one is not WANTED is the job of - // AdoptStripePrice, not of the key — see StripePriceAdoptionTest. - expect($second)->not->toBe($first); -}); -``` - -- [ ] **Step 2: Run test to verify it fails** - -```bash -php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php -``` - -Expected: FAIL — `Class "App\Services\Stripe\IdempotencyKey" not found` erscheint noch nicht, weil der Test die Klasse nicht direkt anfasst; stattdessen schlagen die Erwartungen fehl (`$sent[1]` **ist** gleich `$sent[0]`), und der Refund-Test schlägt fehl, weil der Fake nicht wirft. - -- [ ] **Step 3: Write `IdempotencyKey`** - -`app/Services/Stripe/IdempotencyKey.php`: - -```php - $productId, - 'unit_amount' => $amountCents, - 'currency' => strtolower($currency), - 'interval' => $interval, - 'metadata' => $metadata, - ]); - } - - /** The key for a Product, for the same reason and against the same trap. */ - public static function forProduct(?string $key, string $name, array $metadata): ?string - { - return self::with($key, ['name' => $name, 'metadata' => $metadata]); - } - - /** - * Eight hex characters over the parameters, canonically ordered. - * - * Ordered, because PHP keeps insertion order and two callers writing the - * same metadata in a different order would otherwise send two keys for one - * call — which would mint two Prices and be a worse bug than the one this - * closes. - */ - public static function fingerprint(array $parameters): string - { - return substr(sha1(self::canonical($parameters)), 0, 8); - } - - private static function with(?string $key, array $parameters): ?string - { - // Null stays null: a caller who sends no key wants no replay, and - // inventing one here would change what the call means. - return $key === null ? null : $key.'-'.self::fingerprint($parameters); - } - - private static function canonical(array $parameters): string - { - ksort($parameters); - - foreach ($parameters as $name => $value) { - $parameters[$name] = is_array($value) ? self::canonical($value) : (string) $value; - } - - return json_encode($parameters, JSON_THROW_ON_ERROR); - } -} -``` - -- [ ] **Step 4: Wire it into `HttpStripeClient`** - -`app/Services/Stripe/HttpStripeClient.php` — `createProduct` (ab `:87`): - -```php - public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string - { - return (string) $this->request(IdempotencyKey::forProduct($idempotencyKey, $name, $metadata)) - ->asForm() - ->post($this->url('products'), array_filter([ - 'name' => $name, - ...$this->flatten('metadata', $metadata), - ])) - ->throw() - ->json('id'); - } -``` - -`createPrice` (ab `:99`) — nur die erste Zeile des `return` ändert sich: - -```php - return (string) $this->request(IdempotencyKey::forPrice( - $idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata, - )) - ->asForm() - ->post($this->url('prices'), [ -``` - -Und den Kommentar in `request()` (`:344-351`) ergänzen — er behauptet heute, der Schlüssel schütze einen Retry, und verschweigt, dass er einen geänderten Aufruf abwürgt: - -```php - // Stripe replays the original response for a repeated key instead of - // creating a second object — but ONLY for a call repeated exactly; a - // changed parameter under a used key is HTTP 400 for twenty-four hours. - // The catalogue calls therefore fold a fingerprint of their parameters - // into the key (see IdempotencyKey); the money calls deliberately do - // not. Their keys expire after 24 hours, so this protects a retry, not - // a sync re-run next week — for which the stored ids and - // AdoptStripePrice are the guard. -``` - -`use App\Services\Stripe\IdempotencyKey;` ist **nicht** nötig — dieselbe Namespace. - -- [ ] **Step 5: Make the fake faithful** - -`app/Services/Stripe/FakeStripeClient.php` — Eigenschaft ersetzen (`:37-38`): - -```php - /** - * Idempotency key → what was first answered for it, and a fingerprint of - * what it was first sent with. - * - * The fingerprint is the half that was missing. This ledger replayed the - * first id for a repeated key without ever looking at what the second call - * was asking for — so it was unrealistic in precisely the way that mattered, - * and no test could see the 2026-07-29 blockade. - * - * @var array - */ - public array $keys = []; -``` - -Zwei private Helfer am Ende der Klasse, neben `failIfAsked()`: - -```php - /** - * The id Stripe already answered for this key, or null when it is new. - * - * Throws on a key that comes back with different parameters, which is what - * Stripe does — with this wording — and what makes a test able to see the - * failure at all. - */ - private function replay(?string $key, array $parameters): ?string - { - if ($key === null || ! isset($this->keys[$key])) { - return null; - } - - if ($this->keys[$key]['fingerprint'] !== IdempotencyKey::fingerprint($parameters)) { - throw new RuntimeException( - 'Keys for idempotent requests can only be used with the same parameters they were first used with.', - ); - } - - return $this->keys[$key]['id']; - } - - private function rememberKey(?string $key, string $id, array $parameters): void - { - if ($key !== null) { - $this->keys[$key] = ['id' => $id, 'fingerprint' => IdempotencyKey::fingerprint($parameters)]; - } - } -``` - -`createProduct` (`:151-166`) — Schlüssel **mit Fingerabdruck**, genau wie der HTTP-Client, sonst prüft der Test etwas anderes als die Produktion sendet: - -```php - public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string - { - $key = IdempotencyKey::forProduct($idempotencyKey, $name, $metadata); - $parameters = ['name' => $name, 'metadata' => $metadata]; - - // Replays the first answer for a repeated key, as Stripe does. - $replayed = $this->replay($key, $parameters); - - if ($replayed !== null) { - return $replayed; - } - - $id = 'prod_'.substr(sha1($name.count($this->products)), 0, 12); - $this->products[$id] = ['name' => $name, 'metadata' => $metadata]; - - $this->rememberKey($key, $id, $parameters); - - return $id; - } -``` - -`createPrice` (`:168-194`) — dasselbe Muster, plus `created` am gespeicherten Preis (Task 3 sortiert danach): - -```php - public function createPrice( - string $productId, - int $amountCents, - string $currency, - string $interval, - array $metadata = [], - ?string $idempotencyKey = null, - ): string { - $key = IdempotencyKey::forPrice($idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata); - $parameters = [ - 'product' => $productId, - 'unit_amount' => $amountCents, - 'currency' => strtolower($currency), - 'interval' => $interval, - 'metadata' => $metadata, - ]; - - $replayed = $this->replay($key, $parameters); - - if ($replayed !== null) { - return $replayed; - } - - $id = 'price_'.substr(sha1($productId.$amountCents.$interval.count($this->prices)), 0, 12); - $this->prices[$id] = [ - 'product' => $productId, - 'amount' => $amountCents, - 'currency' => $currency, - 'interval' => $interval, - 'metadata' => $metadata, - // Stripe stamps every object with its creation time and - // AdoptStripePrice takes the OLDEST of several orphans. A counter - // is enough and beats a clock: it cannot tie. - 'created' => count($this->prices) + 1, - ]; - - $this->rememberKey($key, $id, $parameters); - - return $id; - } -``` - -Das `@var` der Eigenschaft `$prices` (`:22`) um `created: int` ergänzen. - -`cancelSubscription` (`:313`, `:324`) und `refund` (`:337`, `:350`) auf `replay()`/`rememberKey()` umstellen — **ohne** `IdempotencyKey`, mit dem rohen Schlüssel: - -```php - // No fingerprint: for a refund, a changed parameter under a used key is - // exactly what must fail loudly. See IdempotencyKey. - $parameters = ['payment' => $paymentReference, 'amount' => (string) $amountCents]; - $replayed = $this->replay($idempotencyKey, $parameters); - - if ($replayed !== null) { - return $replayed; - } -``` - -und nach dem Anlegen `$this->rememberKey($idempotencyKey, $id, $parameters);`. Für `cancelSubscription` sind die Parameter `['subscription' => $subscriptionId, 'when' => $when]`, die gemerkte „id" bleibt `$subscriptionId` wie heute. - -- [ ] **Step 6: Run the test to verify it passes** - -```bash -php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php -``` - -Expected: PASS, sieben Tests. - -- [ ] **Step 7: Run the whole billing folder — the fake is used by a dozen files** - -```bash -php artisan test tests/Feature/Billing -``` - -Expected: PASS. Wenn hier etwas fällt, ist es ein Test, der einen Schlüssel zweimal mit verschiedenen Parametern schickte und das bisher nicht merkte — **das ist ein echter Fund**, nicht ein zu unterdrückender Testfehler: den Aufrufer korrigieren, nicht den Fake. - -- [ ] **Step 8: Commit** - -```bash -git add -- app/Services/Stripe/IdempotencyKey.php app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php tests/Feature/Billing/StripeIdempotencyKeyTest.php -``` - -```bash -git commit -F - -- app/Services/Stripe/IdempotencyKey.php app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php tests/Feature/Billing/StripeIdempotencyKeyTest.php <<'MSG' -Put in the key everything the call actually sends - -A key says "I already sent this call", not "this is what the object is". The -catalogue calls sent metadata that the key knew nothing about, so adding the -tax_treatment field in 9da1358 poisoned yesterday's key for a day — and -AddonPrices::ensure() runs inside a customer's module booking, not only in the -sweep. - -createPrice and createProduct now fold a fingerprint of their parameters into -the key. refund, cancelSubscription, addSubscriptionItem and the checkout -deliberately do not: there a second object is the customer's money taken twice, -and Stripe's refusal is the thing worth keeping. - -The fake could not see any of this. Its ledger replayed the first id for a -repeated key without ever comparing what the second call asked for. - -Co-Authored-By: Claude Opus 5 -MSG -``` - ---- - -### Task 2: Stripe fragen, was es schon hat - -**Files:** -- Modify: `app/Services/Stripe/StripeClient.php:118-140` (zwischen `createPrice` und `archivePrice`) -- Modify: `app/Services/Stripe/HttpStripeClient.php` (nach `activatePrice`, `:143`) -- Modify: `app/Services/Stripe/FakeStripeClient.php` (nach `activatePrice`, `:201-212`) -- Test: `tests/Feature/Billing/StripeIdempotencyKeyTest.php` (zwei Tests angehängt — dieselbe Datei, weil es wieder um das geht, was auf der Leitung landet) - -**Interfaces:** -- Consumes: nichts aus Task 1. -- Produces: - - `StripeClient::activePricesFor(string $productId): array` — Liste von - `array{id: string, unit_amount: int, currency: string, interval: string, created: int, metadata: array}`. - **`currency` kommt GROSSGESCHRIEBEN zurück** (Stripe liefert klein; unsere Tabellen halten groß), `interval` ist `month`/`year`. - - `StripeClient::updatePriceMetadata(string $priceId, array $metadata): void` - - `FakeStripeClient::plantPrice(string $id, string $productId, int $amountCents, string $currency, string $interval, array $metadata, int $created = 1): void` - - `FakeStripeClient::$metadataUpdates` — `array}>` - -- [ ] **Step 1: Write the failing test** - -An `tests/Feature/Billing/StripeIdempotencyKeyTest.php` anhängen: - -```php -it('pages through every active price of a product', function () { - Http::fake([ - 'api.stripe.com/*' => Http::sequence() - ->push([ - 'data' => [ - ['id' => 'price_a', 'unit_amount' => 3480, 'currency' => 'eur', - 'created' => 100, 'recurring' => ['interval' => 'month'], - 'metadata' => ['addon' => 'priority_support']], - ['id' => 'price_b', 'unit_amount' => 41760, 'currency' => 'eur', - 'created' => 101, 'recurring' => ['interval' => 'year'], 'metadata' => []], - ], - 'has_more' => true, - ]) - ->push([ - 'data' => [ - ['id' => 'price_c', 'unit_amount' => 2900, 'currency' => 'eur', - 'created' => 102, 'recurring' => ['interval' => 'month'], - 'metadata' => ['addon' => 'priority_support', 'tax_treatment' => 'reverse_charge']], - ], - 'has_more' => false, - ]), - ]); - - $prices = (new HttpStripeClient)->activePricesFor('prod_1'); - - expect($prices)->toHaveCount(3) - ->and($prices[0])->toBe([ - 'id' => 'price_a', - 'unit_amount' => 3480, - // Upper case, because that is how our own tables hold it and the - // comparison in AdoptStripePrice must not have to remember which - // side is which. - 'currency' => 'EUR', - 'interval' => 'month', - 'created' => 100, - 'metadata' => ['addon' => 'priority_support'], - ]) - ->and($prices[2]['id'])->toBe('price_c'); - - // The second page has to be asked for, or this reintroduces the very gap it - // exists to close — a family product accumulates prices across versions, - // terms, treatments and every rate change. - Http::assertSent(fn ($request) => str_contains($request->url(), 'starting_after=price_b')); - - // Archived prices are none of our business here: we are looking for - // something to SELL on. - Http::assertSent(fn ($request) => str_contains($request->url(), 'active=true')); -}); - -it('writes metadata onto a price that already exists', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_a'])]); - - (new HttpStripeClient)->updatePriceMetadata('price_a', ['addon' => 'priority_support']); - - Http::assertSent(fn ($request) => $request->url() === 'https://api.stripe.com/v1/prices/price_a' - && $request['metadata[addon]'] === 'priority_support'); -}); - -it('lets the fake answer with the prices it holds, minus the archived ones', function () { - $fake = new FakeStripeClient; - - $kept = $fake->createPrice('prod_1', 3480, 'EUR', 'month', ['addon' => 'priority_support']); - $gone = $fake->createPrice('prod_1', 2900, 'EUR', 'month', ['addon' => 'priority_support']); - $other = $fake->createPrice('prod_2', 3480, 'EUR', 'month', []); - $fake->archivePrice($gone); - - $fake->plantPrice('price_orphan', 'prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], created: 0); - - $found = collect($fake->activePricesFor('prod_1'))->pluck('id')->all(); - - expect($found)->toContain($kept, 'price_orphan') - ->and($found)->not->toContain($gone, $other); -}); -``` - -- [ ] **Step 2: Run test to verify it fails** - -```bash -php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php -``` - -Expected: FAIL — `Call to undefined method App\Services\Stripe\HttpStripeClient::activePricesFor()`. - -- [ ] **Step 3: Add both methods to the contract** - -`app/Services/Stripe/StripeClient.php`, zwischen `createPrice` und `archivePrice`: - -```php - /** - * Every active recurring Price of a Product, as Stripe holds them. - * - * The half of the catalogue mirror that was missing: nothing here ever asked - * Stripe what it already had. A run that created a Price and died before the - * row was written left an orphan our table never learned about, and once the - * idempotency key expired the next run made a SECOND live Price for the same - * money — the exact duplicate the key exists to prevent. See - * App\Services\Billing\AdoptStripePrice, which is the only caller. - * - * Active ones only, because the question being asked is "is there something - * here I can sell on?". `currency` comes back UPPER CASE, the way our own - * tables hold it. - * - * @return array}> - */ - public function activePricesFor(string $productId): array; - - /** - * Write metadata onto a Price that already exists. - * - * Metadata is one of the few fields a Stripe Price lets you change — the - * same property activatePrice() rests on, and the reason the metadata format - * is NOT part of a Price's identity. An adopted orphan is brought up to - * today's metadata rather than replaced, which is exactly what was done by - * hand after 2026-07-29 and keeps Stripe's own dashboard readable. - */ - public function updatePriceMetadata(string $priceId, array $metadata): void; -``` - -- [ ] **Step 4: Implement in `HttpStripeClient`** (nach `activatePrice`) - -```php - public function activePricesFor(string $productId): array - { - $prices = []; - $after = null; - - // Paged through to the end, for the same reason invoiceLines() is: Stripe - // caps a page at a hundred, and a family Product collects a Price per - // version, term, treatment and rate change. Stopping at the first page - // would leave an orphan unfound and mint the duplicate anyway. - do { - $page = $this->request() - ->get($this->url('prices'), array_filter([ - 'product' => $productId, - 'active' => 'true', - 'type' => 'recurring', - 'limit' => 100, - 'starting_after' => $after, - ], fn ($value) => $value !== null)) - ->throw() - ->json(); - - $data = (array) ($page['data'] ?? []); - - foreach ($data as $price) { - $prices[] = [ - 'id' => (string) ($price['id'] ?? ''), - 'unit_amount' => (int) ($price['unit_amount'] ?? 0), - 'currency' => strtoupper((string) ($price['currency'] ?? '')), - 'interval' => (string) ($price['recurring']['interval'] ?? ''), - 'created' => (int) ($price['created'] ?? 0), - 'metadata' => array_map( - fn ($value) => (string) $value, - (array) ($price['metadata'] ?? []), - ), - ]; - } - - $after = $data === [] ? null : ($data[array_key_last($data)]['id'] ?? null); - } while (($page['has_more'] ?? false) === true && $after !== null); - - return $prices; - } - - public function updatePriceMetadata(string $priceId, array $metadata): void - { - $this->request() - ->asForm() - ->post($this->url('prices/'.$priceId), $this->flatten('metadata', $metadata)) - ->throw(); - } -``` - -- [ ] **Step 5: Implement in `FakeStripeClient`** (nach `activatePrice`) - -```php - public function activePricesFor(string $productId): array - { - // No failIfAsked(): createPrice(), the call this one stands in front of, - // does not fail either, and a test that scripts an outage for a refund - // must not have its catalogue sync change behaviour underneath it. - $found = []; - - foreach ($this->prices as $id => $price) { - if ($price['product'] !== $productId || in_array($id, $this->archived, true)) { - continue; - } - - $found[] = [ - 'id' => $id, - 'unit_amount' => $price['amount'], - 'currency' => strtoupper($price['currency']), - 'interval' => $price['interval'], - 'created' => $price['created'], - 'metadata' => $price['metadata'], - ]; - } - - return $found; - } - - public function updatePriceMetadata(string $priceId, array $metadata): void - { - if (isset($this->prices[$priceId])) { - $this->prices[$priceId]['metadata'] = $metadata; - } - - $this->metadataUpdates[] = ['price' => $priceId, 'metadata' => $metadata]; - } - - /** - * A Price that exists at Stripe and in no table of ours — the orphan a run - * leaves behind when it dies between Stripe's create and our insert. - * - * Here rather than in a test file because it is the state the whole adoption - * step exists for, and every test that needs it would otherwise reach into - * $prices and write the shape by hand. - */ - public function plantPrice( - string $id, - string $productId, - int $amountCents, - string $currency, - string $interval, - array $metadata = [], - int $created = 1, - ): void { - $this->prices[$id] = [ - 'product' => $productId, - 'amount' => $amountCents, - 'currency' => $currency, - 'interval' => $interval, - 'metadata' => $metadata, - 'created' => $created, - ]; - } -``` - -Eigenschaft dazu, neben `$activated`: - -```php - /** - * Every metadata write, in order, so a test can assert that an adopted - * orphan was brought up to today's metadata instead of being replaced. - * - * @var array}> - */ - public array $metadataUpdates = []; -``` - -- [ ] **Step 6: Run the tests** - -```bash -php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php -``` - -Expected: PASS, zehn Tests. - -- [ ] **Step 7: Run the billing folder** - -```bash -php artisan test tests/Feature/Billing -``` - -Expected: PASS. - -- [ ] **Step 8: Commit** - -```bash -git add -- app/Services/Stripe/StripeClient.php app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php tests/Feature/Billing/StripeIdempotencyKeyTest.php -``` - -```bash -git commit -F - -- app/Services/Stripe/StripeClient.php app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php tests/Feature/Billing/StripeIdempotencyKeyTest.php <<'MSG' -Let the catalogue ask Stripe what it already has - -Nothing here could. The client could create, archive and unarchive a Price and -had no way to list one, so a run that died between Stripe's create and our -insert left an orphan our table never learned about — and once the key expired, -the next run made a second live Price for the same money. - -Paged to the end like invoiceLines(), because a family Product collects a Price -per version, term, treatment and rate change, and stopping at the first page -would leave the orphan unfound and mint the duplicate anyway. - -updatePriceMetadata() alongside it: metadata is one of the few fields a Price -lets you change, which is why the metadata format is not part of a Price's -identity and why an adopted orphan is brought up to date rather than replaced. - -Co-Authored-By: Claude Opus 5 -MSG -``` - ---- - -### Task 3: `AdoptStripePrice` — die fünf Bedingungen - -**Files:** -- Create: `app/Services/Billing/AdoptStripePrice.php` -- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` - -**Interfaces:** -- Consumes: `StripeClient::activePricesFor()`, `updatePriceMetadata()`, `archivePrice()` aus Task 2. `FakeStripeClient::plantPrice()`, `$metadataUpdates`. -- Produces: - -```php -public function __invoke( - string $productId, - int $amountCents, - string $currency, - string $interval, - array $metadata, - array $identifying, - callable $claimed, // fn (string $priceId): bool -): ?string -``` - - Gibt die übernommene Preis-ID zurück oder `null`, wenn nichts zu übernehmen ist. `$identifying` sind die Metadatenschlüssel, die einen Preis als unseren ausweisen (`['addon']` bzw. `['plan_price_id']`). - -- [ ] **Step 1: Write the failing test** - -`tests/Feature/Billing/StripePriceAdoptionTest.php`: - -```php -stripe = new FakeStripeClient; - app()->instance(StripeClient::class, $this->stripe); -}); - -/** The module metadata as AddonPrices sends it today. */ -function moduleMetadata(string $treatment = 'domestic'): array -{ - return ['addon' => 'priority_support', 'tax_treatment' => $treatment]; -} - -/** Ask the adoption step the question AddonPrices asks it. */ -function adoptModulePrice(?array $metadata = null, ?callable $claimed = null): ?string -{ - return app(AdoptStripePrice::class)( - productId: 'prod_support', - amountCents: 3480, - currency: 'EUR', - interval: 'month', - metadata: $metadata ?? moduleMetadata(), - identifying: ['addon'], - claimed: $claimed ?? fn (string $id) => false, - ); -} - -it('adopts the orphan of 2026-07-29 instead of minting a second price', function () { - // The state that morning: the Price exists at Stripe, carries the metadata - // of the code that made it — WITHOUT tax_treatment, which 9da1358 added - // afterwards — and no row of ours knows it. - $this->stripe->plantPrice('price_1TygdEC7u8NpJ8pOt3nsoyYw', 'prod_support', - 3480, 'EUR', 'month', ['addon' => 'priority_support']); - - expect(adoptModulePrice())->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw'); - - // Brought up to today's metadata rather than replaced: metadata is mutable - // at Stripe, the amount is not, which is the whole reason the format is no - // part of a Price's identity. - expect($this->stripe->metadataUpdates)->toBe([[ - 'price' => 'price_1TygdEC7u8NpJ8pOt3nsoyYw', - 'metadata' => moduleMetadata(), - ]]); -}); - -it('leaves the metadata alone when it already says the right thing', function () { - $this->stripe->plantPrice('price_ok', 'prod_support', 3480, 'EUR', 'month', moduleMetadata()); - - expect(adoptModulePrice())->toBe('price_ok') - ->and($this->stripe->metadataUpdates)->toBe([]); -}); - -it('adopts nothing when the amount, currency or interval differ', function () { - $this->stripe->plantPrice('price_cheaper', 'prod_support', 2900, 'EUR', 'month', moduleMetadata()); - $this->stripe->plantPrice('price_yearly', 'prod_support', 3480, 'EUR', 'year', moduleMetadata()); - $this->stripe->plantPrice('price_dollars', 'prod_support', 3480, 'USD', 'month', moduleMetadata()); - - expect(adoptModulePrice())->toBeNull(); -}); - -it('refuses a price nothing proves is ours, and says so', function () { - Log::spy(); - - // What a person clicking through Stripe's own dashboard leaves behind: the - // right money on our product, and not one word about what it is for. - $this->stripe->plantPrice('price_by_hand', 'prod_support', 3480, 'EUR', 'month', []); - - expect(adoptModulePrice())->toBeNull(); - - Log::shouldHaveReceived('warning')->once(); -}); - -it('passes silently over another of our own prices', function () { - Log::spy(); - - // At a VAT rate of nought both treatments are the same amount, so the - // reverse-charge Price sits at the domestic one's money — and contradicts on - // tax_treatment. That is not a mystery worth a warning; it is a Price of - // ours that is not the one being asked for. - $this->stripe->plantPrice('price_rc', 'prod_support', 3480, 'EUR', 'month', - moduleMetadata('reverse_charge')); - - expect(adoptModulePrice())->toBeNull(); - - Log::shouldNotHaveReceived('warning'); -}); - -it('never hands out a price a row already claims', function () { - $this->stripe->plantPrice('price_taken', 'prod_support', 3480, 'EUR', 'month', moduleMetadata()); - - expect(adoptModulePrice(claimed: fn (string $id) => $id === 'price_taken'))->toBeNull(); -}); - -it('adopts the oldest of several orphans and stops selling the rest', function () { - Log::spy(); - - $this->stripe->plantPrice('price_second', 'prod_support', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], created: 200); - $this->stripe->plantPrice('price_first', 'prod_support', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], created: 100); - $this->stripe->plantPrice('price_third', 'prod_support', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], created: 300); - - // The oldest, because it is the one a lost row is likeliest to have been - // billing on. - expect(adoptModulePrice())->toBe('price_first') - ->and($this->stripe->archived)->toBe(['price_second', 'price_third']); - - Log::shouldHaveReceived('warning'); -}); -``` - -- [ ] **Step 2: Run test to verify it fails** - -```bash -php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -Expected: FAIL — `Target class [App\Services\Billing\AdoptStripePrice] does not exist.` - -- [ ] **Step 3: Write the implementation** - -`app/Services/Billing/AdoptStripePrice.php`: - -```php - $metadata what the create call would send - * @param array $identifying metadata keys that mark a Price as ours - * @param callable(string): bool $claimed is this Price id already in our register? - */ - public function __invoke( - string $productId, - int $amountCents, - string $currency, - string $interval, - array $metadata, - array $identifying, - callable $claimed, - ): ?string { - $candidates = []; - - foreach ($this->stripe->activePricesFor($productId) as $price) { - if ($price['unit_amount'] !== $amountCents - || $price['currency'] !== strtoupper($currency) - || $price['interval'] !== $interval) { - continue; - } - - if ($claimed($price['id'])) { - continue; - } - - // Contradicts on something it carries: another Price of ours, not a - // mystery. Silent — at a VAT rate of nought the two treatments share - // an amount, and this would otherwise warn on every sweep. - if ($this->contradicts($price['metadata'], $metadata)) { - continue; - } - - if (! $this->confirms($price['metadata'], $metadata, $identifying)) { - Log::warning('stripe: left an unexplained active price alone rather than adopting it', [ - 'price' => $price['id'], - 'product' => $productId, - 'amount_cents' => $amountCents, - 'currency' => strtoupper($currency), - 'interval' => $interval, - ]); - - continue; - } - - $candidates[] = $price; - } - - if ($candidates === []) { - return null; - } - - usort($candidates, fn (array $a, array $b) => $a['created'] <=> $b['created']); - - $adopted = array_shift($candidates); - - foreach ($candidates as $duplicate) { - $this->stripe->archivePrice($duplicate['id']); - - Log::warning('stripe: stopped selling a duplicate price for one figure', [ - 'price' => $duplicate['id'], - 'adopted' => $adopted['id'], - 'product' => $productId, - 'amount_cents' => $amountCents, - ]); - } - - // Only when it differs, so a sweep over a healthy catalogue makes no - // writes at Stripe at all. - if ($adopted['metadata'] !== $metadata) { - $this->stripe->updatePriceMetadata($adopted['id'], $metadata); - } - - Log::info('stripe: adopted an existing price instead of creating a second one', [ - 'price' => $adopted['id'], - 'product' => $productId, - 'amount_cents' => $amountCents, - 'currency' => strtoupper($currency), - 'interval' => $interval, - ]); - - return $adopted['id']; - } - - /** - * Does this Price say something about itself that we do not? - * - * @param array $found - * @param array $expected - */ - private function contradicts(array $found, array $expected): bool - { - foreach ($expected as $key => $value) { - if (array_key_exists($key, $found) && $found[$key] !== $value) { - return true; - } - } - - return false; - } - - /** - * Does it prove it is ours? - * - * Failing to contradict is not enough — an empty metadata bag contradicts - * nothing. At least one identifying key has to be there and agree. - * - * @param array $found - * @param array $expected - * @param array $identifying - */ - private function confirms(array $found, array $expected, array $identifying): bool - { - foreach ($identifying as $key) { - if (isset($found[$key]) && $found[$key] === ($expected[$key] ?? null)) { - return true; - } - } - - return false; - } -} -``` - -- [ ] **Step 4: Run the test to verify it passes** - -```bash -php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -Expected: PASS, acht Tests. - -- [ ] **Step 5: Commit** - -```bash -git add -- app/Services/Billing/AdoptStripePrice.php tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -```bash -git commit -F - -- app/Services/Billing/AdoptStripePrice.php tests/Feature/Billing/StripePriceAdoptionTest.php <<'MSG' -Recognise the price Stripe already has - -An abandoned run leaves an orphan: Stripe made the Price, our insert never -happened, and the table that decides everything afterwards does not know it -exists. The key covers a day; after that the next run makes a second live Price -for the same money. - -What may be taken over is narrow. Same amount, currency and interval — a Price -at another figure would move money, and nothing here may: a running contract -keeps the Price it was sold on, and a booking stays frozen at what it cost that -day. Plus proof in the metadata, because an unexplained active Price at the -right money is what somebody clicking through Stripe's dashboard leaves behind, -and adopting that is worse than minting a second one. - -Several orphans: the oldest is adopted — likeliest to be the one a lost row was -billing on — and the rest are archived, which stops them being sold and moves -nobody. - -Co-Authored-By: Claude Opus 5 -MSG -``` - ---- - -### Task 4: Die Modulseite benutzt den Wiedererkennungsschritt - -**Files:** -- Modify: `app/Services/Billing/AddonPrices.php:47` (Konstruktor), `:126-159` (der Anlege-Pfad in `ensure`) -- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` (angehängt) - -**Interfaces:** -- Consumes: `AdoptStripePrice::__invoke()` aus Task 3. -- Produces: `AddonPrices::__construct(StripeClient $stripe, AdoptStripePrice $adopt)`. **Geprüft: niemand konstruiert `AddonPrices` mit `new`** (weder App noch Tests), der Container löst auf. - -- [ ] **Step 1: Write the failing test** - -An `tests/Feature/Billing/StripePriceAdoptionTest.php` anhängen: - -```php -use App\Models\StripeAddonPrice; -use App\Models\Subscription; -use App\Models\SubscriptionAddon; -use App\Services\Billing\AddonPrices; -use App\Services\Billing\TaxTreatment; -use Illuminate\Contracts\Console\Kernel; - -it('takes the orphan over instead of minting a second module price', function () { - // The product exists because a previous run got that far; the Price exists - // because the run that made it died before the insert. - $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', - ]); - - $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); -}); -``` - -**Zum letzten Test:** `SubscriptionAddon::booted()` wirft bei jeder Änderung an `FROZEN` (`subscription_id`, `addon_key`, `price_cents`, `currency`, `quantity`, `booked_at`) — die Regel „eine Buchung friert ihren Preis ein" steht also schon im Modell. `stripe_price_id` gehört nicht dazu und ist damit änderbar; genau deshalb muss ein Test sagen, dass die Übernahme sie nicht anfasst. `Subscription::factory()->plan('team')` ist die Form aus `tests/Feature/Billing/ReverseChargePriceTest.php:220`. - -- [ ] **Step 2: Run test to verify it fails** - -```bash -php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -Expected: FAIL — `ensure()` legt einen neuen Preis an, `count($this->stripe->prices)` ist um eins gewachsen, und die zurückgegebene ID ist nicht die der Waise. - -- [ ] **Step 3: Wire the adoption into `AddonPrices`** - -Konstruktor (`:47`): - -```php - public function __construct( - private readonly StripeClient $stripe, - private readonly AdoptStripePrice $adopt, - ) {} -``` - -Der Anlege-Pfad in `ensure()` (`:126-159`) wird zu: - -```php - $productId = $this->product($addonKey); - - $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: $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' : ''), - ); - - $this->remember($addonKey, $reverseCharge, $amount, $netCents, $currency, $interval, $productId, $priceId); - $this->archiveSuperseded($addonKey, $reverseCharge, $netCents, $currency, $interval, $amount); - - return $priceId; -``` - -`use App\Services\Billing\AdoptStripePrice;` ist nicht nötig (gleiche Namespace). - -Den Kommentar in `remember()` (`:297-301`) um den zweiten Grund ergänzen, aus dem die Ausnahme kommen kann: - -```php - } 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. 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. - } -``` - -- [ ] **Step 4: Run the test to verify it passes** - -```bash -php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -Expected: PASS, elf Tests. - -- [ ] **Step 5: Run the billing folder** - -```bash -php artisan test tests/Feature/Billing -``` - -Expected: PASS. Ein Fehler in `StripeAddonBillingTest` oder `AddonCancellationTest` bedeutet, dass der Wiedererkennungsschritt einen Preis übernimmt, wo ein Test einen neuen erwartet — dann prüfen, ob die Erwartung des Tests oder die Bedingung in `AdoptStripePrice` falsch ist, **bevor** irgendetwas geändert wird. - -- [ ] **Step 6: Commit** - -```bash -git add -- app/Services/Billing/AddonPrices.php tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -```bash -git commit -F - -- app/Services/Billing/AddonPrices.php tests/Feature/Billing/StripePriceAdoptionTest.php <<'MSG' -Ask before minting a module price - -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 -MSG -``` - ---- - -### Task 5: Die Paketseite benutzt ihn ebenso - -**Files:** -- Modify: `app/Services/Billing/PlanPrices.php:43` (Konstruktor), `:130-160` (der Anlege-Zweig in `ensure`) -- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` (angehängt) - -**Interfaces:** -- Consumes: `AdoptStripePrice::__invoke()` aus Task 3. -- Produces: `PlanPrices::__construct(StripeClient $stripe, AdoptStripePrice $adopt)`. **Geprüft: niemand konstruiert `PlanPrices` mit `new`.** - -- [ ] **Step 1: Write the failing test** - -An `tests/Feature/Billing/StripePriceAdoptionTest.php` anhängen: - -```php -use App\Models\PlanPrice; -use App\Models\StripePlanPrice; -use App\Services\Billing\PlanPrices; - -it('takes over an orphaned package price', function () { - // A catalogue mirrored once, so families have Products and rows have Prices. - app(Kernel::class)->call('stripe:sync-catalogue'); - - $row = PlanPrice::query()->firstOrFail(); - $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic()); - - // The register loses its row and Stripe keeps the Price: a run that died - // between the two, seen from the next run's point of view. - $orphan = (string) StripePlanPrice::query() - ->where('plan_price_id', $row->id) - ->where('reverse_charge', false) - ->value('stripe_price_id'); - StripePlanPrice::query()->where('stripe_price_id', $orphan)->delete(); - - $before = count($this->stripe->prices); - - $id = app(PlanPrices::class)->ensure($row->refresh(), TaxTreatment::domestic()); - - expect($id)->toBe($orphan) - ->and(count($this->stripe->prices))->toBe($before) - ->and(StripePlanPrice::query() - ->where('plan_price_id', $row->id) - ->where('reverse_charge', false) - ->where('charged_cents', $charged) - ->value('stripe_price_id'))->toBe($orphan) - // The pointer for the ordinary domestic sale is written as before. - ->and((string) $row->refresh()->stripe_price_id)->toBe($orphan); -}); - -it('does not take a package price belonging to another catalogue row', function () { - app(Kernel::class)->call('stripe:sync-catalogue'); - - $row = PlanPrice::query()->firstOrFail(); - $product = (string) $row->version->family->stripe_product_id; - $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic()); - $interval = $row->term === Subscription::TERM_YEARLY ? 'year' : 'month'; - - StripePlanPrice::query()->where('plan_price_id', $row->id)->delete(); - - // Same product, same money, same interval — and plan_price_id says it is a - // DIFFERENT row's Price. One Product carries every version and term of a - // family, so this is the ordinary case, not an exotic one. - $this->stripe->plantPrice('price_other_row', $product, $charged, (string) $row->currency, - $interval, ['plan_price_id' => (string) ($row->id + 1000), 'tax_treatment' => 'domestic']); - - $id = app(PlanPrices::class)->ensure($row->refresh(), TaxTreatment::domestic()); - - expect($id)->not->toBe('price_other_row'); -}); -``` - -- [ ] **Step 2: Run test to verify it fails** - -```bash -php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -Expected: FAIL im ersten der beiden — `ensure()` legt einen zweiten Preis an, `count($this->stripe->prices)` wächst. - -- [ ] **Step 3: Wire the adoption into `PlanPrices`** - -Konstruktor (`:43`): - -```php - public function __construct( - private readonly StripeClient $stripe, - private readonly AdoptStripePrice $adopt, - ) {} -``` - -Der Zweig `if ($priceId === null)` in `ensure()` (`:135-160`): - -```php - $interval = $price->term === Subscription::TERM_YEARLY ? 'year' : 'month'; - - if ($priceId === null) { - $metadata = [ - 'plan_family' => $family->key, - 'plan_version' => (string) $version->version, - 'plan_version_id' => (string) $version->id, - 'plan_price_id' => (string) $price->id, - // Read back by anything that has a Price id and needs to know - // what kind of sale it was — and by a person looking at - // Stripe's own dashboard, where two Prices on one Product - // would otherwise differ only by an amount. - 'tax_treatment' => $treatment->reverseCharge ? 'reverse_charge' : 'domestic', - ]; - - // Asked BEFORE minting: a run that died between Stripe's create and - // our insert left a Price the register does not know, and the key - // below stops protecting it after twenty-four hours. `plan_price_id` - // is what proves such a Price is this row's — one Product carries - // every version and term of a family, so the amount alone would not. - $priceId = ($this->adopt)( - productId: (string) $productId, - amountCents: $charged, - currency: (string) $price->currency, - interval: $interval, - metadata: $metadata, - identifying: ['plan_price_id'], - claimed: fn (string $id) => StripePlanPrice::query() - ->where('stripe_price_id', $id) - ->exists(), - ); - - $priceId ??= $this->stripe->createPrice( - productId: (string) $productId, - amountCents: $charged, - currency: (string) $price->currency, - interval: $interval, - metadata: $metadata, - // Says "I have already sent this call", nothing more — the - // metadata is folded in by IdempotencyKey inside the client, so - // changing the format above can never again refuse the call for - // a day. What stops a second Price for one figure is the - // adoption step above; Stripe forgets a key after 24 hours. - idempotencyKey: "clupilot-price-{$price->id}-{$charged}" - .($treatment->reverseCharge ? '-rc' : ''), - ); - } elseif ($existing?->archived_at !== null) { -``` - -Der bestehende `elseif`-Zweig und alles darunter bleiben unverändert. - -- [ ] **Step 4: Run the test to verify it passes** - -```bash -php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -Expected: PASS, dreizehn Tests. - -- [ ] **Step 5: Run the billing folder** - -```bash -php artisan test tests/Feature/Billing -``` - -Expected: PASS. `ReverseChargePriceTest` ist hier der wichtigste Zeuge: `:342-347` zählt genau 16 Register-Zeilen und 16 verschiedene Preis-IDs nach einem doppelten Sync-Lauf. Bleibt das grün, hat der Wiedererkennungsschritt nichts verdoppelt und nichts zusammengelegt. - -- [ ] **Step 6: Commit** - -```bash -git add -- app/Services/Billing/PlanPrices.php tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -```bash -git commit -F - -- app/Services/Billing/PlanPrices.php tests/Feature/Billing/StripePriceAdoptionTest.php <<'MSG' -Ask before minting a package price - -Same defect, same fix, one difference: a family Product carries every version -and every term, so the amount alone does not say which row a Price belongs to. -plan_price_id in the metadata does, and it is what has to agree before a Price -is taken over. - -Co-Authored-By: Claude Opus 5 -MSG -``` - ---- - -### Task 6: Eine Zeile pro Stripe-Preis - -**Files:** -- Create: `database/migrations/2026_07_31_210000_one_row_per_stripe_price.php` -- Modify: `docs/handoffs/2026-07-30-real-run-handoff.md:175-176` -- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` (angehängt) - -**Interfaces:** -- Consumes: nichts. -- Produces: eindeutiger Index `stripe_addon_prices_price_unique` auf `stripe_addon_prices.stripe_price_id`. - -- [ ] **Step 1: Write the failing test** - -An `tests/Feature/Billing/StripePriceAdoptionTest.php` anhängen: - -```php -use Illuminate\Database\UniqueConstraintViolationException; - -it('lets no two module rows claim one stripe price', function () { - $shared = [ - 'addon_key' => 'priority_support', 'net_cents' => 2900, 'currency' => 'EUR', - 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', - ]; - - StripeAddonPrice::query()->create([...$shared, - 'reverse_charge' => false, 'amount_cents' => 3480, 'interval' => 'month']); - - // Two rows on one Price is what Block D warned about: archiving the one - // would withdraw the Price the other is still selling. - expect(fn () => StripeAddonPrice::query()->create([...$shared, - 'reverse_charge' => true, 'amount_cents' => 2900, 'interval' => 'month'])) - ->toThrow(UniqueConstraintViolationException::class); -}); -``` - -- [ ] **Step 2: Run test to verify it fails** - -```bash -php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -Expected: FAIL — keine Ausnahme, die zweite Zeile entsteht. - -- [ ] **Step 3: Write the migration** - -`database/migrations/2026_07_31_210000_one_row_per_stripe_price.php`: - -```php -select('stripe_price_id') - ->groupBy('stripe_price_id') - ->havingRaw('count(*) > 1') - ->pluck('stripe_price_id'); - - foreach ($shared as $priceId) { - // The lowest id stays — the row that was written first, which is the - // one anything already billing is likeliest to have been reading. - $keep = DB::table('stripe_addon_prices')->where('stripe_price_id', $priceId)->min('id'); - - DB::table('stripe_addon_prices') - ->where('stripe_price_id', $priceId) - ->where('id', '!=', $keep) - ->delete(); - } - - Schema::table('stripe_addon_prices', function (Blueprint $table) { - $table->unique('stripe_price_id', 'stripe_addon_prices_price_unique'); - }); - } - - public function down(): void - { - Schema::table('stripe_addon_prices', function (Blueprint $table) { - $table->dropUnique('stripe_addon_prices_price_unique'); - }); - } -}; -``` - -- [ ] **Step 4: Run the test to verify it passes** - -```bash -php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php -``` - -Expected: PASS, vierzehn Tests. - -- [ ] ~~**Step 5: Prove the deduplication itself, on a real migration run**~~ — **NICHT AUSFÜHREN** - -~~`php artisan migrate:fresh --env=testing && php artisan migrate:status | tail -3`~~ - -> **Warnung, nicht stillschweigend gelöscht, weil die Falle es wert ist:** Dieser Befehl darf nicht laufen — es gibt in diesem Repo **keine** `.env.testing`, also greift die Verbindung aus `.env` und `migrate:fresh` löscht und baut die **echte Entwicklungsdatenbank** neu, die laufende Container benutzen. Der Index ist durch den SQLite-Migrationslauf der Suite belegt (Step 1 und Step 4); ein zweiter Beleg ist keinen Datenverlust wert. - -- [ ] **Step 6: Mark the Block D item done** - -`docs/handoffs/2026-07-30-real-run-handoff.md:175-176` ersetzen: - -```markdown -- ~~`stripe_addon_prices.stripe_price_id` ist **nicht** eindeutig (Plan-Seite schon): - zwei Zeilen könnten einen Preis teilen, Archivieren würde den anderen mitentziehen.~~ - **Erledigt** — eindeutig seit `2026_07_31_210000_one_row_per_stripe_price`; zugleich - das Netz unter dem Wiedererkennungsschritt, siehe - `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md`. -``` - -- [ ] **Step 7: Run the full suite — this is a schema change** - -```bash -php artisan test -``` - -Expected: PASS. Eine `UniqueConstraintViolationException` in einem fremden Test bedeutet, dass dort zwei Modul-Zeilen absichtlich eine Preis-ID teilten — das ist ein Fund, der gemeldet und nicht durch Zurücknehmen des Index behoben wird. - -- [ ] **Step 8: Commit** - -```bash -git add -- database/migrations/2026_07_31_210000_one_row_per_stripe_price.php tests/Feature/Billing/StripePriceAdoptionTest.php docs/handoffs/2026-07-30-real-run-handoff.md -``` - -```bash -git commit -F - -- database/migrations/2026_07_31_210000_one_row_per_stripe_price.php tests/Feature/Billing/StripePriceAdoptionTest.php docs/handoffs/2026-07-30-real-run-handoff.md <<'MSG' -One row per Stripe price on the module side too - -The plan register has had this since 2026_07_30_110000. The module register -never did, so two rows could claim one Price and archiving the one would have -withdrawn the Price the other was still selling — Block D of the real-run -handoff. - -It is also the net under the adoption step: at a VAT rate of nought both -treatments charge the same amount, and the check that refuses an already-claimed -Price is then the only thing keeping the two apart. - -Duplicates are deleted, not archived. An archived row goes on claiming the id -and a unique index knows nothing about archived_at; the row rebuilds itself on -the next ensure(), and subscription_addons holds the Stripe id as text rather -than a foreign key, so no booking loses its price. - -Co-Authored-By: Claude Opus 5 -MSG -``` - ---- - -## Nach dem letzten Task - -- [ ] **Voller Testlauf** (nicht nur `tests/Feature/Billing`): - -```bash -php artisan test -``` - -- [ ] **Ein Review über den Gesamtdiff**, dann **eine** Fix-Runde, dann **ein** Re-Review über den Fix-Diff — und danach werden offene Befunde mit schriftlicher Begründung geparkt (R22.1). Ein zweiter Re-Review desselben Befundes ist verboten. -- [ ] **Nicht geplant, bewusst offen** (Spec §12): ein Aufräum-Kommando für Waisen, die niemand übernehmen kann. Sie stehen mit ihrer ID im Log (`stripe: left an unexplained active price alone…`). Als Folgepunkt in den Merge Request. diff --git a/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md b/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md deleted file mode 100644 index be50139..0000000 --- a/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md +++ /dev/null @@ -1,359 +0,0 @@ -# Spec — Bestehende Stripe-Preise wiedererkennen, statt sie zu verdoppeln - -**Datum:** 2026-07-30 -**Status:** entworfen, noch nicht umgesetzt -**Vorgänger-Kontext:** `docs/handoffs/2026-07-30-real-run-handoff.md` Block D -(`stripe_addon_prices.stripe_price_id` ist nicht eindeutig) - ---- - -## 1. Der Vorfall - -Ein Lauf am **2026-07-29 23:11** legte bei Stripe -`price_1TygdEC7u8NpJ8pOt3nsoyYw` an (`priority_support`, 3480 EUR, monatlich) und -brach ab, **bevor** die Zeile in `stripe_addon_prices` geschrieben war. Commit -`9da1358` fügte danach das Metadatenfeld `tax_treatment` hinzu. Der nächste -Abgleich schickte **denselben** Idempotenz-Schlüssel mit den **neuen** Metadaten -und bekam von Stripe: - -> HTTP 400 — Keys for idempotent requests can only be used with the same -> parameters they were first used with. - -Der Abgleich war damit hart blockiert und wäre es bis zum Ablauf des Schlüssels -(24 h) geblieben. Von Hand bereinigt: Zeile für den vorhandenen Preis -nachgetragen, Metadaten bei Stripe angeglichen. Die Ursache stand offen. - -## 2. Ausgangsbefund — gemessen, nicht gelesen - -| Befund | Beleg | -|---|---| -| Der Modul-Schlüssel deckt Betrag, Währung, Intervall, Modul und Behandlung ab — **keine** Metadaten | `app/Services/Billing/AddonPrices.php:152` | -| Der Paket-Schlüssel ebenso | `app/Services/Billing/PlanPrices.php:158` | -| Beide senden Metadaten mit, die **nicht** im Schlüssel stehen — u. a. das neue `tax_treatment` | `AddonPrices.php:133-141`, `PlanPrices.php:141-151` | -| `createProduct` hat dieselbe Falle latent: Metadaten mit, Schlüssel ohne | `AddonPrices.php:269-273`, `SyncStripeCatalogue.php:104-111`, `HttpStripeClient.php:87-97` | -| Kein Aufruf fragt Stripe nach **vorhandenen** Preisen. Der Client hat gar keine Methode dafür | `app/Services/Stripe/StripeClient.php` — `createPrice`, `archivePrice`, `activatePrice`, kein Auflisten | -| `ensure()` läuft **nicht nur** im Abgleich, sondern in der **Kundenbuchung** | `BookAddon.php:259` und `:360` → `SyncStripeAddonItems.php:183` und `:291` → `AddonPrices::ensure()` | -| Der Fake spielt einen bekannten Schlüssel zurück, **ohne die Parameter zu vergleichen** | `FakeStripeClient.php:168-193`, Ledger `$keys` (`:38`) | -| `stripe_plan_prices.stripe_price_id` ist eindeutig | Migration `2026_07_30_110000:56` | -| `stripe_addon_prices.stripe_price_id` ist **nicht** eindeutig; eindeutig ist nur das Tupel | Migration `2026_07_31_200000:59-77` | -| Der Client blättert schon woanders durch Stripe-Listen | `HttpStripeClient::invoiceLines()`, `:312-336` | -| Produktion `mariadb`, Tests `sqlite :memory:` | `.env:24`, `phpunit.xml:42-43` | - -**Der 400 ist damit kein Betriebsfehler, sondern ein Kundenfehler.** Ein -Kunde, der ein Modul bucht, während ein Waisen-Preis mit altem Metadatenformat -bei Stripe liegt, bekommt 24 Stunden lang keine Buchung zustande. - -## 3. Zwei getrennte Mängel - -1. **Waise ohne Wiedererkennung.** Ein Abgleich, der zwischen Stripes Anlage und - unserem Schreiben abbricht, hinterlässt einen Preis bei Stripe, den unsere - Tabelle nie erfährt. Nach Ablauf der 24 Stunden legt der nächste Lauf einen - **zweiten aktiven** Preis über dieselbe Summe an — genau das, was der - Schlüssel verhindern soll. -2. **Metadaten im Aufruf, nicht im Schlüssel.** Jede künftige Änderung an den - Metadaten von `createPrice` blockiert den Abgleich für 24 Stunden auf - dieselbe Weise. - -## 4. Gehört das Metadatenformat zur Identität? — Nein - -**Identität** eines Preises ist, was Stripe unveränderlich macht: Produkt, -Betrag, Währung, Intervall — und, weil bei einem Steuersatz von 0 % beide -Behandlungen denselben Betrag haben, welche der beiden es ist. Metadaten sind -bei Stripe **nachträglich änderbar**. Deshalb wird ein wiedererkannter Preis mit -korrigierten Metadaten **übernommen**, nicht durch einen neuen ersetzt. - -Der Idempotenz-Schlüssel behauptet aber gar keine Identität. Er behauptet -„**diesen Aufruf habe ich schon abgeschickt**". Weil Stripe das wörtlich prüft, -müssen **alle gesendeten Parameter** hinein — sonst ist der Schlüssel eine -Lüge, die mit 400 endet. - -Der Kommentar im Code verwechselt beides („Keyed on what the Price IS", -`AddonPrices.php:142-151`). Das ist die eigentliche Ursache und wird -mitkorrigiert: die Identität wandert in den Wiedererkennungsschritt, der -Schlüssel wird auf seine tatsächliche Aussage zurückgeführt. - -## 5. Regeln des Auftraggebers, die dieser Entwurf nicht anfassen darf - -Beide wurden im Gespräch am 2026-07-30 gesetzt und gelten als Prüfstein: - -1. **Bestehende Verträge behalten ihre alten Preise.** Neue Preise gelten nur - für Kunden, die neu abschließen. Daraus folgt die Freigabe zum Archivieren: - ein archivierter Stripe-Preis bleibt für laufende Abos in Kraft, er wird nur - nicht mehr **verkauft** — worauf `archiveSuperseded()` schon heute beruht. -2. **Jede Buchung friert ihren Preis ein.** Ein Paket und zwei Module werden - immer gleich verrechnet; ein später gebuchtes Modul kommt zum heutigen Preis; - eine Stornierung zieht genau den damaligen Betrag ab und wird danach nicht - mehr verrechnet. - -Beides bleibt strukturell gewahrt, weil die Übernahme **ausschließlich** Preise -mit **identischem** Betrag, Währung und Intervall betrifft. Sie kann niemandes -Verrechnung verschieben. §11 schreibt es als Test fest. - -## 6. Bauteil 1 — `App\Services\Billing\AdoptStripePrice` - -Der Wiedererkennungsschritt, **einmal** für beide Seiten (Modul und Paket), weil -zwei Fassungen derselben Regel der Anfang ihres Auseinanderlaufens sind. - -**Stellung im Ablauf:** nach dem Blick in unsere eigene Tabelle, vor -`createPrice`. Also genau dann ein zusätzliches GET, wenn ohnehin ein POST -fällig gewesen wäre — nie im Normalfall. - -``` -ensure() - ├─ Zeile in unserer Tabelle? → ja: wie heute (ggf. reaktivieren) - ├─ AdoptStripePrice: Kandidat bei Stripe? → ja: übernehmen - └─ createPrice(…) → nein: wie heute anlegen -``` - -**Ein Kandidat wird übernommen, wenn er alle fünf Bedingungen erfüllt:** - -1. bei Stripe **aktiv** und an **unserem** Produkt hängend - (`active=true`, `product=…`, `type=recurring`); -2. **exakt** gleich in Betrag, Währung (klein/groß normalisiert) und Intervall; -3. **widerspricht keinem** unserer Metadatenschlüssel, den er trägt; -4. **bestätigt mindestens einen** tragenden Schlüssel — `addon` auf der - Modulseite, `plan_price_id` auf der Paketseite. Ein von Hand im Dashboard - angelegter Preis wird damit nie übernommen; -5. ist **von keiner Zeile beansprucht** (Prüfung gegen die eigene Tabelle, - archivierte Zeilen eingeschlossen). - -**Danach:** Metadaten bei Stripe angleichen (genau das, was von Hand getan -wurde), Zeile schreiben, `archiveSuperseded()` wie im Anlege-Pfad, Preis-ID -zurückgeben. - -**Mehrere Kandidaten:** der **älteste** (`created` aufsteigend) wird übernommen — -er ist am ehesten der, auf dem eine verlorene Zeile schon bilanziert. Die -übrigen werden bei Stripe **archiviert** (Freigabe aus §5.1) und protokolliert. - -**Kein beweisbar eigener Kandidat:** Verhalten wie heute (anlegen), mit Warnung -im Log. Bewusst so: einen fremden Preis zu übernehmen ist schlimmer als einen -zweiten anzulegen, und die Warnung nennt die ID, damit ein Betreiber entscheiden -kann. - -**Meldeweg ist das Log** (`Log::warning`), nicht die Konsolenausgabe: `ensure()` -läuft auch im Web-Request, wo es keine Kommandozeile gibt. - -## 7. Bauteil 2 — `App\Services\Stripe\IdempotencyKey` - -Bildet aus dem sprechenden Schlüssel plus einem kurzen Fingerabdruck der -**tatsächlich gesendeten Parameter** den Header-Wert: - -```php -IdempotencyKey::for('clupilot-addon-price-priority_support-month-3480-EUR', $payload) -// → 'clupilot-addon-price-priority_support-month-3480-EUR-9f3a1c07' -``` - -Kanonisch gebildet (Schlüssel sortiert, dann `sha1`, auf 8 Zeichen gekürzt), -damit dieselbe Nutzlast immer denselben Fingerabdruck ergibt. In **einer** -Support-Klasse, weil `HttpStripeClient` und `FakeStripeClient` denselben Wert -bilden müssen — sonst prüft der Test etwas anderes, als die Produktion sendet. - -**Angewandt nur auf `createPrice` und `createProduct`.** Bewusst **nicht** auf: - -| Aufruf | Warum der 400 dort bleiben muss | -|---|---| -| `refund` | Ein zweiter Refund schickt dem Kunden das Geld zweimal. | -| `cancelSubscription` | Andere Parameter unter demselben Schlüssel heißt: eine andere Kündigungsart als die schon abgeschickte. | -| `addSubscriptionItem` | Ein zweites Item verrechnet dasselbe Modul doppelt. | -| `createCheckoutSession` | Eine zweite Session zu geänderten Parametern ist ein zweiter Kaufvorgang. | - -Die Regel lautet also: **der Fingerabdruck ist für die Katalogseite, wo ein -Duplikat reparierbar und eine Blockade kundenwirksam ist.** Bei -Geldbewegungen ist es umgekehrt. §11 schreibt diese Grenze als Test fest, damit -sie niemand „vereinheitlicht". - -## 8. Zwei neue Client-Methoden - -```php -/** @return array}> */ -public function activePricesFor(string $productId): array; - -public function updatePriceMetadata(string $priceId, array $metadata): void; -``` - -`activePricesFor()` blättert nach dem Muster von `invoiceLines()` -(`HttpStripeClient.php:312-336`) — ein Familien-Produkt sammelt über Versionen, -Terme, Behandlungen und Steuersatzänderungen hinweg leicht mehr als eine Seite, -und beim ersten Seitenende stehenzubleiben wäre dieselbe Lücke in neuer Form. - -`updatePriceMetadata()` benutzt, dass Metadaten eines der wenigen Felder sind, -die ein Stripe-Preis ändern lässt — dieselbe Eigenschaft, auf der -`activatePrice()` schon beruht. - -Beide im `FakeStripeClient` nachgezogen. - -## 9. Der Fake muss Stripe erst einmal nachbilden - -`FakeStripeClient::createPrice` spielt einen bekannten Schlüssel zurück, **ohne -die Parameter zu vergleichen** (`:168-193`). Er ist genau in der Sache -unrealistisch, um die es hier geht — deshalb konnte kein Test den Vorfall sehen. - -Der Ledger `$keys` speichert künftig neben der ID auch die Nutzlast und wirft -bei gleichem Schlüssel mit anderen Parametern — mit Stripes Wortlaut. Das ist -der Regressionstest, der den 29.07. gesehen hätte. - -## 10. Migration — eine Zeile pro Stripe-Preis - -`2026_07_31_210000_one_row_per_stripe_price.php` (nach der letzten vorhandenen -Migration einsortiert): - -1. **Entdoppeln:** Zeilen in `stripe_addon_prices`, die eine - `stripe_price_id` teilen — niedrigste Zeilen-ID bleibt, die übrigen werden - **gelöscht**. Nicht archiviert: eine archivierte Zeile beansprucht die ID - weiter und der eindeutige Index kennt `archived_at` nicht. -2. **Index:** `stripe_price_id` wird eindeutig — wie auf der Paketseite seit - `2026_07_30_110000:56`. - -Löschen ist gefahrlos: die Zeile baut sich beim nächsten `ensure()` über die -Wiedererkennung selbst wieder auf, und `subscription_addons.stripe_price_id` -hält die Stripe-ID als **Text**, nicht als Fremdschlüssel. - -Treiberneutral formuliert (Auswahl über `groupBy`/`having`, Löschen über -IDs) — Produktion ist `mariadb`, Tests laufen auf `sqlite`. - -**Was der Index absichert:** Bei einem Steuersatz von 0 % sind die Beträge -beider Behandlungen gleich. Die Übernahme lehnt einen bereits beanspruchten -Preis dann über §6 Bedingung 5 ab und legt für die zweite Behandlung einen -eigenen an (der Schlüssel trägt `-rc`). Der Index ist das Netz darunter, falls -diese Prüfung je gerissen wird. - -## 11. Tests - -Zwei Regeldatei-Tests nach Repo-Sitte, getrennt danach, **gegen wen** sie prüfen: - -- `tests/Feature/Billing/StripeIdempotencyKeyTest.php` — was auf der Leitung - landet. Läuft über `Http::fake()` gegen `HttpStripeClient`, weil geprüft werden - muss, was **gesendet** wird, nicht was der Fake sich merkt. Hier steht auch, - dass der Fake Stripes 400 überhaupt nachbildet. -- `tests/Feature/Billing/StripePriceAdoptionTest.php` — die Übernahmeregeln, - beide Seiten, über den `FakeStripeClient`. - -| Prüfung | Deckt ab | -|---|---| -| Waise bei Stripe → wird übernommen, `createPrice` **nicht** aufgerufen, Zeile trägt die vorhandene ID (Modul **und** Paket) | Mangel 1 | -| Waise mit **altem** Metadatenformat → wird übernommen, Metadaten bei Stripe angeglichen | der Vorfall selbst | -| Zwei Waisen → älteste übernommen, andere bei Stripe archiviert, Warnung im Log | §6 | -| Aktiver Preis an unserem Produkt **ohne** unsere Metadaten → **nicht** übernommen, neuer angelegt, Warnung | §6 Bedingung 4 | -| Preis-ID schon von einer Zeile beansprucht → **nicht** zweimal vergeben | §6 Bedingung 5 | -| **Eine eingefrorene Buchung behält ihren Betrag**: ein auf altem Preis laufendes Modul wird von einem Abgleich nicht verschoben | §5.2 | -| Gleicher sprechender Schlüssel + geänderte Metadaten → **unterschiedlicher** Header, kein 400 | Mangel 2 | -| Fingerabdruck **nur** bei `createPrice`/`createProduct`, nicht bei `refund`, `cancelSubscription`, `addSubscriptionItem`, `createCheckoutSession` | §7 | -| Der Fake wirft bei gleichem Schlüssel mit anderen Parametern | §9 | -| Zwei Zeilen können keine `stripe_price_id` teilen | §10 | - -## 12. Nicht Teil dieses Vorhabens - -- **Ein Aufräum-Kommando** für Waisen, die niemand mehr übernehmen kann (etwa - ohne unsere Metadaten). Der Log-Eintrag nennt sie; ein Kommando dafür ist ein - Folgepunkt. -- **Die übrigen Block-D-Punkte** aus `docs/handoffs/2026-07-30-real-run-handoff.md`. -- **`stripe:reprice-subscriptions`** und alles, was laufende Verträge bewegt. - Dieser Entwurf bewegt keinen einzigen — siehe §5. -- **Stripes `automatic_tax`** bleibt aus, `TaxTreatment` bleibt die einzige - Steuerinstanz. - -## 13. Berührte Dateien - -**Neu** -- `app/Services/Billing/AdoptStripePrice.php` -- `app/Services/Stripe/IdempotencyKey.php` -- `database/migrations/2026_07_31_210000_one_row_per_stripe_price.php` -- `tests/Feature/Billing/StripeIdempotencyKeyTest.php` -- `tests/Feature/Billing/StripePriceAdoptionTest.php` - -**Geändert** -- `app/Services/Stripe/StripeClient.php` — zwei Methoden -- `app/Services/Stripe/HttpStripeClient.php` — Auflisten, Metadaten schreiben, - Fingerabdruck bei den zwei Anlege-Aufrufen -- `app/Services/Stripe/FakeStripeClient.php` — beide Methoden, Ledger mit - Parametervergleich -- `app/Services/Billing/AddonPrices.php` — Wiedererkennung vor `createPrice`, - Kommentar zur Schlüssel-Begründung berichtigt -- `app/Services/Billing/PlanPrices.php` — dasselbe -- `docs/handoffs/2026-07-30-real-run-handoff.md` — Block-D-Punkt als erledigt - markieren - ---- - -## 14. Folgepunkte, die aus der Umsetzung entstanden sind - -Nachgetragen nach dem Abschluss-Review. Alle Befunde sind offengelegt, keiner -blockiert den Merge — aber keiner ist erledigt. - -### Die eine echte Lücke: Waisen-**Produkte** - -`activePricesFor()` erkennt einen verwaisten *Preis* wieder. Für ein verwaistes -**Produkt** gibt es kein Gegenstück, und dort ist der Schaden größer: ein -zweites Produkt lässt jedes `activePricesFor($neueId)` leer zurückkommen, damit -ist die Wiedererkennung für dieses Modul oder diese Familie **dauerhaft blind** -und jede Waise am ersten Produkt unerreichbar. Der Zweig deaktiviert seine -eigene Sicherung durch die Lücke, die er nicht geschlossen hat. - -Betroffen: `SyncStripeCatalogue::handle()` (Produkt anlegen, ID danach in einem -zweiten Schreibvorgang speichern) und `AddonPrices::product()`. Beide Kommentare -benennen die Lücke jetzt; der Fix wäre ein `productsFor()` nach dem Muster von -`activePricesFor()`, das Stripe nach den Produkten fragt und über die Metadaten -wiedererkennt. - -### Das Geld-Tor sitzt an der falschen Stelle - -`AdoptStripePrice` verspricht in seinem Kopfkommentar, keine Übernahme könne -Geld verschieben — kann aber nur Betrag, Währung und Intervall vergleichen. Was -sonst noch entscheidet, was ein Preis verrechnet (`interval_count`, -`usage_type`, `transform_quantity`, `billing_scheme`), filtert -`HttpStripeClient::activePricesFor()`. Zwei Folgen: der `FakeStripeClient` kann -diese Felder gar nicht ausdrücken, also kann **kein Test von -`AdoptStripePrice` das Geld-Tor abdecken**; und eine dritte Implementierung von -`StripeClient` ließe es still fallen. Die Prüfung gehört in die Klasse, die sie -zusagt. - -Verwandt: `PlanPrices::ensure()` hat keine `<= 0`-Schranke, wo -`AddonPrices::ensure()` eine hat. Das ist das Einzige, was einen -`billing_scheme: tiered`-Preis (bei dem Stripe `unit_amount: null` liefert, hier -als 0 gelesen) auf der Paketseite überhaupt erreichbar macht. - -### Kleineres, benannt statt vergessen - -- **Der Abgleich zählt „angelegt", auch wo er übernommen hat.** Die Wortwahl der - Konsolenausgabe ist entschärft („created or adopted"), gezählt wird weiterhin - vor dem Aufruf. Eine echte Trennung der beiden Zahlen fehlt — und das ist die - eine Zahl, die ein Mensch nach einem Vorfall zuerst liest. -- **Die Warnung über einen unerklärlichen Preis hat keine Drosselung.** Sie - feuert bei jedem Abgleich *und* jeder Kundenbuchung, solange die Waise - existiert — und weil das Aufräum-Kommando (§12) ein Folgepunkt ist, auf - unbestimmte Zeit. Der Widerspruchs-Pfad wurde aus genau diesem Grund still - gestellt; der Bestätigungs-Pfad bekam dieselbe Überlegung nicht. -- **`FakeStripeClient::updatePriceMetadata()` ersetzt, wo Stripe zusammenführt**, - und `activePricesFor()` gibt Metadaten ungecastet zurück, wo der HTTP-Client - auf String castet. Heute nicht beobachtbar, aber es ist genau die Sorte - Untreue des Fakes, gegen die §9 geschrieben wurde. -- **`created` im Fake kann gleichstehen**: `createPrice()` stempelt - `count($prices) + 1`, `plantPrice()` hat die Vorgabe `1`. Der Kommentar sagt - das jetzt zu, statt es zu bestreiten; ein Gleichstand-Tiebreak auf die ID - fehlt weiter. -- **`HttpStripeClient::activePricesFor()` bricht das Blättern still ab**, wenn - dem letzten Eintrag einer nicht-letzten Seite die `id` fehlt — getreue Kopie - desselben Verhaltens in `invoiceLines()`. Die Folge hat sich aber geändert: in - `invoiceLines()` entsteht ein zu kurzer Belegtext, hier eine ungesehene Waise - und ein zweiter aktiver Preis, also der Vorfall. Wer es anfasst, soll werfen - statt anhalten. -- **Der Idempotenz-Fingerabdruck und der POST-Rumpf müssen zusammenbleiben.** - Ein Feld, das künftig in `createPrice()` gesendet, aber nicht in - `IdempotencyKey::priceParameters()` aufgenommen wird, öffnet den Vorfall - wieder. Der Test „does not block a booking because the metadata format moved" - wacht über diese Nahtstelle — mit einem gesetzten Ledger-Eintrag, weil der - Zustand heute nicht mehr fahrbar ist. - -### Zwei fremde, vorbestehende Testfehler, die dieser Zweig aufgedeckt hat - -Keiner davon wird von diesem Zweig verursacht. - -1. **`HostStepsTest.php:1064`** schreibt `fsn-01.node.clupilot.com` hart hinein, - während `.env:107` `CLUPILOT_DNS_ZONE=clupilot.cloud` setzt und `phpunit.xml` - neun andere Variablen mit `force="true"` festnagelt, diese aber nicht. Eine - Zeile in `phpunit.xml`. Derselbe Punkt steht als offene Aufgabe in - `docs/handoffs/2026-07-30-real-run-handoff.md`. -2. **`PlanCatalogueTest.php`** ist einmal ausgefallen und lief beim - Wiederholen durch. „Beim zweiten Mal grün" ist keine Diagnose, und ein - sprunghafter Fehler in den Katalogpreis-Tests liegt im Wirkungskreis dieses - Zweigs, auch wenn er nicht von ihm kommt. diff --git a/lang/de/verify_email.php b/lang/de/verify_email.php index 34179b8..8a10315 100644 --- a/lang/de/verify_email.php +++ b/lang/de/verify_email.php @@ -12,7 +12,7 @@ return [ // Ausdrücklich: ohne Bestätigung passiert nichts weiter. Wer hier // fälschlich eine fremde Adresse eingetragen hat, soll den Empfänger nicht // zu einer Handlung zwingen. - 'not_you' => 'Sie haben sich nicht angemeldet? Dann ignorieren Sie diese Nachricht. Ohne Bestätigung wird das Konto nicht freigeschaltet, und wir schreiben Ihnen nicht wieder.', + 'not_you' => 'Sie haben sich nicht angemeldet? Dann ignorieren Sie diese Nachricht. Ohne Bestätigung wird das Konto nicht freigeschaltet, nach fünf Tagen von selbst gelöscht, und wir schreiben Ihnen nicht wieder.', // Die Seite, auf der jemand landet, der sich angemeldet hat aber noch nicht // bestätigt ist. @@ -21,6 +21,13 @@ return [ 'notice_hint' => 'Nichts angekommen? Sehen Sie im Spam-Ordner nach — oder schicken Sie die Nachricht erneut.', 'notice_resend' => 'Erneut schicken', 'notice_sent' => 'Bestätigung erneut verschickt.', - 'notice_signout' => 'Abmelden', - 'notice_wrong_address' => 'Adresse falsch eingetragen? Melden Sie sich ab und registrieren Sie sich erneut.', + 'notice_delete' => 'Registrierung verwerfen', + 'notice_wrong_address' => 'Adresse falsch eingetragen? Verwerfen Sie diese Registrierung und legen Sie sie neu an — erst dann ist die Adresse wieder frei. Bestätigen Sie fünf Tage lang nicht, wird der Zugang von selbst gelöscht; Sie müssen dann nichts tun.', + + 'delete_title' => 'Diese Registrierung verwerfen?', + 'delete_body' => 'Der Zugang wird gelöscht und die E-Mail-Adresse ist wieder frei. Es ist noch nichts eingerichtet und nichts bezahlt — es geht nur diese unbestätigte Anmeldung verloren.', + 'delete_keep' => 'Behalten', + 'delete_confirm' => 'Verwerfen', + 'deleted' => 'Registrierung verworfen. Sie können sich mit derselben Adresse neu anmelden.', + 'delete_has_contract' => 'Zu diesem Zugang gehört bereits ein Vertrag — er lässt sich hier nicht verwerfen. Schreiben Sie uns, wir klären das.', ]; diff --git a/lang/en/verify_email.php b/lang/en/verify_email.php index f915f95..9baa2fe 100644 --- a/lang/en/verify_email.php +++ b/lang/en/verify_email.php @@ -11,7 +11,7 @@ return [ 'fallback' => 'If the button does not work, copy this address into your browser:', // Said plainly: without confirmation nothing further happens. Somebody who // typed a stranger's address must not force that stranger to act. - 'not_you' => 'Did not sign up? Ignore this message. Without confirmation the account is not opened, and we will not write again.', + 'not_you' => 'Did not sign up? Ignore this message. Without confirmation the account is never activated, is removed by itself after five days, and we will not write again.', // The page somebody lands on when signed in but not yet confirmed. 'notice_title' => 'One step left', @@ -19,6 +19,13 @@ return [ 'notice_hint' => 'Nothing arrived? Check the spam folder — or send it again.', 'notice_resend' => 'Send again', 'notice_sent' => 'Confirmation sent again.', - 'notice_signout' => 'Sign out', - 'notice_wrong_address' => 'Typed the wrong address? Sign out and register again.', + 'notice_delete' => 'Discard this registration', + 'notice_wrong_address' => 'Typed the wrong address? Discard this registration and start again — only then is the address free. If you do not confirm within five days the account is removed by itself; there is nothing for you to do.', + + 'delete_title' => 'Discard this registration?', + 'delete_body' => 'The account is deleted and the address becomes free again. Nothing has been set up and nothing paid for — only this unconfirmed sign-up is lost.', + 'delete_keep' => 'Keep it', + 'delete_confirm' => 'Discard', + 'deleted' => 'Registration discarded. You can sign up again with the same address.', + 'delete_has_contract' => 'There is already a contract behind this account, so it cannot be discarded here. Write to us and we will sort it out.', ]; diff --git a/resources/views/livewire/auth/verify-email.blade.php b/resources/views/livewire/auth/verify-email.blade.php index 5fee4e4..b28704e 100644 --- a/resources/views/livewire/auth/verify-email.blade.php +++ b/resources/views/livewire/auth/verify-email.blade.php @@ -13,11 +13,17 @@ {{ __('verify_email.notice_resend') }} - - {{ __('verify_email.notice_signout') }} + {{-- R23: confirmed in a modal. Deleting an account is not a thing + to do on one click, even an account that is five minutes old. --}} + + {{ __('verify_email.notice_delete') }} + {{-- Said here, not only in the mail: somebody who never got the mail is + looking at THIS page, and the deadline is the reason they do not + have to do anything about an attempt they abandon. --}}

{{ __('verify_email.notice_wrong_address') }}

diff --git a/resources/views/livewire/confirm-delete-own-account.blade.php b/resources/views/livewire/confirm-delete-own-account.blade.php new file mode 100644 index 0000000..98d9379 --- /dev/null +++ b/resources/views/livewire/confirm-delete-own-account.blade.php @@ -0,0 +1,14 @@ + +

{{ __('verify_email.delete_body') }}

+ + +
+ + {{ __('verify_email.delete_keep') }} + + + {{ __('verify_email.delete_confirm') }} + +
+
+
diff --git a/routes/console.php b/routes/console.php index 0137430..94e471c 100644 --- a/routes/console.php +++ b/routes/console.php @@ -58,6 +58,13 @@ Schedule::call(fn () => StripePendingEvent::query() // // Hourly rather than by the minute: the failure it repairs lasts as long as an // outage lasts, and a retry storm against a mount that is down helps nobody. +// Registrations nobody confirmed. Daily is often enough for a five-day window, +// and it holds the address hostage until it runs: the unique index means the +// person cannot register again with the address they meant. +Schedule::command('clupilot:prune-unverified') + ->dailyAt('03:20') + ->withoutOverlapping(); + // The support mailbox. Polling, because IMAP is a mailbox you look in — a mail // server has no way to tell us something arrived — so the interval IS the // answer to "how long until I see it". Two minutes is short enough that an diff --git a/tests/Feature/Auth/UnverifiedAccountTest.php b/tests/Feature/Auth/UnverifiedAccountTest.php new file mode 100644 index 0000000..79d7901 --- /dev/null +++ b/tests/Feature/Auth/UnverifiedAccountTest.php @@ -0,0 +1,164 @@ +create(['email' => 'neu@example.test', 'email_verified_at' => null]); + + Livewire::actingAs($user) + ->test(VerifyEmail::class) + ->call('deleteAccount'); + + expect(User::query()->where('email', 'neu@example.test')->exists())->toBeFalse(); +}); + +it('refuses to discard an account that already has a contract behind it', function () { + // The webhook creates a customer's login from a paid checkout, and such a + // user can sit on this page unconfirmed while already having a contract. + // Deleting that is data loss dressed up as a convenience. + $user = User::factory()->create(['email' => 'zahlt@example.test', 'email_verified_at' => null]); + $customer = Customer::factory()->create(['email' => 'zahlt@example.test', 'user_id' => $user->id]); + Subscription::factory()->create(['customer_id' => $customer->id, 'status' => 'active']); + + Livewire::actingAs($user) + ->test(VerifyEmail::class) + ->call('deleteAccount') + ->assertDispatched('notify'); + + expect($user->fresh())->not->toBeNull(); +}); + +it('sends a confirmed account to its dashboard instead of deleting it', function () { + // Closing a confirmed account is a different action with an invoice history + // behind it — ConfirmCloseAccount, not this. + // + // Confirmed AFTER mounting, because mount() already redirects a verified + // user away and there would be no component left to call anything on. The + // case this guards is the real one: a tab left open on this page while the + // link was opened in another window. + $user = User::factory()->create(['email_verified_at' => null]); + + $page = Livewire::actingAs($user)->test(VerifyEmail::class); + + $user->forceFill(['email_verified_at' => now()])->save(); + + $page->call('deleteAccount'); + + expect($user->fresh())->not->toBeNull(); +}); + +it('confirms in a modal rather than on one click', function () { + // R23. And the modal mutates nothing itself: it dispatches the event the + // page listens for, so the checks stay in the one place they already are. + $page = File::get(resource_path('views/livewire/auth/verify-email.blade.php')); + + expect($page)->toContain("component: 'confirm-delete-own-account'"); + + $modal = File::get(app_path('Livewire/ConfirmDeleteOwnAccount.php')); + + expect($modal)->toContain("dispatch('own-account-delete-confirmed')") + ->and($modal)->not->toContain('$user'); +}); + +it('no longer tells anybody to sign out and register again', function () { + // Advice that could not work: signing out frees nothing. + expect(__('verify_email.notice_delete'))->not->toBe('verify_email.notice_delete'); + + foreach (['de', 'en'] as $locale) { + expect(File::get(base_path("lang/{$locale}/verify_email.php"))) + ->not->toContain("'notice_signout'"); + } +}); + +// ---- The five-day sweep ---- + +it('removes a registration nobody confirmed after five days', function () { + $stale = User::factory()->create(['email_verified_at' => null]); + $stale->forceFill(['created_at' => now()->subDays(PruneUnverifiedAccounts::AFTER_DAYS + 1)])->save(); + + $this->artisan('clupilot:prune-unverified')->assertSuccessful(); + + expect(User::query()->whereKey($stale->id)->exists())->toBeFalse(); +}); + +it('leaves a fresh registration alone', function () { + // Long enough for somebody who registered on a Friday and found the mail in + // a spam folder on Monday. + $fresh = User::factory()->create(['email_verified_at' => null]); + $fresh->forceFill(['created_at' => now()->subDays(PruneUnverifiedAccounts::AFTER_DAYS - 1)])->save(); + + $this->artisan('clupilot:prune-unverified'); + + expect(User::query()->whereKey($fresh->id)->exists())->toBeTrue(); +}); + +it('never touches a confirmed account, however old', function () { + $old = User::factory()->create(['email_verified_at' => now()->subYear()]); + $old->forceFill(['created_at' => now()->subYears(2)])->save(); + + $this->artisan('clupilot:prune-unverified'); + + expect(User::query()->whereKey($old->id)->exists())->toBeTrue(); +}); + +it('never touches an unconfirmed account with a customer behind it', function () { + // Both links count: a customers row can point at the user, or merely carry + // the same address — see Customer::emailTaken for why one can exist without + // the other. + $byId = User::factory()->create(['email_verified_at' => null]); + $byId->forceFill(['created_at' => now()->subDays(30)])->save(); + Customer::factory()->create(['user_id' => $byId->id, 'email' => 'andere@example.test']); + + $byEmail = User::factory()->create(['email' => 'gleich@example.test', 'email_verified_at' => null]); + $byEmail->forceFill(['created_at' => now()->subDays(30)])->save(); + Customer::factory()->create(['email' => 'gleich@example.test', 'user_id' => null]); + + $this->artisan('clupilot:prune-unverified'); + + expect(User::query()->whereKey($byId->id)->exists())->toBeTrue() + ->and(User::query()->whereKey($byEmail->id)->exists())->toBeTrue(); +}); + +it('says what it would remove without removing it', function () { + $stale = User::factory()->create(['email' => 'weg@example.test', 'email_verified_at' => null]); + $stale->forceFill(['created_at' => now()->subDays(30)])->save(); + + $this->artisan('clupilot:prune-unverified --dry-run') + ->expectsOutputToContain('weg@example.test') + ->assertSuccessful(); + + expect(User::query()->whereKey($stale->id)->exists())->toBeTrue(); +}); + +it('runs on the schedule, or it never runs at all', function () { + $console = File::get(base_path('routes/console.php')); + + expect($console)->toContain("Schedule::command('clupilot:prune-unverified')") + ->toContain('withoutOverlapping'); +}); + +it('tells the person about the deadline, on the page and in the mail', function () { + // Somebody who never got the mail is looking at the PAGE, and the deadline + // is the reason they do not have to do anything about an abandoned attempt. + // Matched on the number, not on a whole phrase: "fünf Tage lang" and "nach + // fünf Tagen" are both correct German and an assertion on one of them tells + // the next person to write the sentence a particular way. + expect(__('verify_email.notice_wrong_address'))->toContain('fünf') + ->and(__('verify_email.not_you'))->toContain('fünf') + // And the sentence and the command agree on the number. + ->and(PruneUnverifiedAccounts::AFTER_DAYS)->toBe(5); +}); diff --git a/tests/Feature/Billing/StripeIdempotencyKeyTest.php b/tests/Feature/Billing/StripeIdempotencyKeyTest.php deleted file mode 100644 index ead4498..0000000 --- a/tests/Feature/Billing/StripeIdempotencyKeyTest.php +++ /dev/null @@ -1,266 +0,0 @@ -set('services.stripe.secret', 'sk_test_plan_task_one'); -}); - -it('sends a different key once the metadata changes', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_x'])]); - - $client = new HttpStripeClient; - $spoken = 'clupilot-addon-price-priority_support-month-3480-EUR'; - - // The call as it stood before 9da1358, and the call after it: same money, - // same interval, one metadata field more. - $client->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], $spoken); - - $client->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support', 'tax_treatment' => 'domestic'], $spoken); - - $sent = collect(Http::recorded()) - ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) - ->all(); - - expect($sent[0])->toStartWith($spoken) - ->and($sent[1])->toStartWith($spoken) - ->and($sent[1])->not->toBe($sent[0]); -}); - -it('sends the same key for the very same call', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_x'])]); - - $client = new HttpStripeClient; - - foreach ([1, 2] as $ignored) { - $client->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], 'clupilot-addon-price'); - } - - $sent = collect(Http::recorded()) - ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) - ->unique() - ->all(); - - expect($sent)->toHaveCount(1); -}); - -it('fingerprints the product call too, where the same trap was waiting', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'prod_x'])]); - - $client = new HttpStripeClient; - - $client->createProduct('Priority Support', ['addon' => 'priority_support'], 'clupilot-addon-product-x'); - $client->createProduct('Priority Support', ['addon' => 'priority_support', 'sold_as' => 'entitlement'], 'clupilot-addon-product-x'); - - $sent = collect(Http::recorded()) - ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) - ->all(); - - expect($sent[1])->not->toBe($sent[0]); -}); - -it('leaves the money calls their bare key, so Stripe still refuses a changed one', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'x'])]); - - $client = new HttpStripeClient; - - $client->refund('pi_1', 500, 'clupilot-refund-7'); - $client->cancelSubscription('sub_1', 'at_period_end', 'clupilot-cancel-7'); - $client->addSubscriptionItem('sub_1', 'price_1', 1, 'none', 'clupilot-item-7'); - - $sent = collect(Http::recorded()) - ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) - ->all(); - - expect($sent)->toBe(['clupilot-refund-7', 'clupilot-cancel-7', 'clupilot-item-7']); -}); - -it('reproduces the refusal Stripe makes, which the fake used to swallow', function () { - $fake = new FakeStripeClient; - - $fake->refund('pi_1', 500, 'clupilot-refund-7'); - - // Same key, different amount. Stripe answers 400; the fake said nothing and - // replayed the first refund's id, which is how a test could pass over the - // very failure that stopped production. - expect(fn () => $fake->refund('pi_1', 900, 'clupilot-refund-7')) - ->toThrow(RuntimeException::class, 'same parameters'); -}); - -it('mints a second price rather than blocking when the metadata moved', function () { - $fake = new FakeStripeClient; - - $first = $fake->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], 'clupilot-addon-price'); - - $second = $fake->createPrice('prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support', 'tax_treatment' => 'domestic'], 'clupilot-addon-price'); - - // Two objects, no exception. That the second one is not WANTED is the job of - // AdoptStripePrice, not of the key — see StripePriceAdoptionTest. - expect($second)->not->toBe($first); -}); - -it('pages through every active price of a product', function () { - Http::fake([ - 'api.stripe.com/*' => Http::sequence() - ->push([ - 'data' => [ - ['id' => 'price_a', 'unit_amount' => 3480, 'currency' => 'eur', - 'created' => 100, 'recurring' => ['interval' => 'month'], - 'metadata' => ['addon' => 'priority_support']], - ['id' => 'price_b', 'unit_amount' => 41760, 'currency' => 'eur', - 'created' => 101, 'recurring' => ['interval' => 'year'], 'metadata' => []], - ], - 'has_more' => true, - ]) - ->push([ - 'data' => [ - ['id' => 'price_c', 'unit_amount' => 2900, 'currency' => 'eur', - 'created' => 102, 'recurring' => ['interval' => 'month'], - 'metadata' => ['addon' => 'priority_support', 'tax_treatment' => 'reverse_charge']], - ], - 'has_more' => false, - ]), - ]); - - $prices = (new HttpStripeClient)->activePricesFor('prod_1'); - - expect($prices)->toHaveCount(3) - ->and($prices[0])->toBe([ - 'id' => 'price_a', - 'unit_amount' => 3480, - // Upper case, because that is how our own tables hold it and the - // comparison in AdoptStripePrice must not have to remember which - // side is which. - 'currency' => 'EUR', - 'interval' => 'month', - 'created' => 100, - 'metadata' => ['addon' => 'priority_support'], - ]) - ->and($prices[2]['id'])->toBe('price_c'); - - // The second page has to be asked for, or this reintroduces the very gap it - // exists to close — a family product accumulates prices across versions, - // terms, treatments and every rate change. - Http::assertSent(fn ($request) => str_contains($request->url(), 'starting_after=price_b')); - - // Archived prices are none of our business here: we are looking for - // something to SELL on. - Http::assertSent(fn ($request) => str_contains($request->url(), 'active=true')); -}); - -it('skips a price it could not have created itself, so the money gate never trusts a partial recurrence', function () { - Http::fake([ - 'api.stripe.com/*' => Http::response([ - 'data' => [ - ['id' => 'price_monthly', 'unit_amount' => 3480, 'currency' => 'eur', - 'created' => 100, 'recurring' => ['interval' => 'month'], - 'metadata' => ['addon' => 'priority_support']], - ['id' => 'price_quarterly', 'unit_amount' => 3480, 'currency' => 'eur', - 'created' => 101, 'recurring' => ['interval' => 'month', 'interval_count' => 3], - 'metadata' => ['addon' => 'priority_support']], - ['id' => 'price_metered', 'unit_amount' => 3480, 'currency' => 'eur', - 'created' => 102, 'recurring' => ['interval' => 'month', 'usage_type' => 'metered'], - 'metadata' => ['addon' => 'priority_support']], - ], - 'has_more' => false, - ]), - ]); - - $prices = (new HttpStripeClient)->activePricesFor('prod_1'); - - // A hand-duplicated dashboard price keeps our metadata, so nothing - // downstream could tell it apart, and the module would bill quarterly. - // - // None of the three planted prices carries `transform_quantity` or - // `billing_scheme` at all, and the ordinary one still comes back — so this - // also holds the other direction of the next test's filter: an absent key is - // Stripe's default, not a reason to reject. - expect(collect($prices)->pluck('id')->all())->toBe(['price_monthly']); -}); - -it('skips a price that would charge our figure for the wrong quantity', function () { - Http::fake([ - 'api.stripe.com/*' => Http::response([ - 'data' => [ - // Spelled out as Stripe actually sends them for an ordinary - // price: transform_quantity null, billing_scheme per_unit. - ['id' => 'price_ordinary', 'unit_amount' => 3480, 'currency' => 'eur', - 'created' => 100, 'recurring' => ['interval' => 'month'], - 'transform_quantity' => null, 'billing_scheme' => 'per_unit', - 'metadata' => ['addon' => 'priority_support']], - ['id' => 'price_divided', 'unit_amount' => 3480, 'currency' => 'eur', - 'created' => 101, 'recurring' => ['interval' => 'month'], - 'transform_quantity' => ['divide_by' => 10, 'round' => 'up'], - 'billing_scheme' => 'per_unit', - 'metadata' => ['addon' => 'priority_support']], - ['id' => 'price_tiered', 'unit_amount' => null, 'currency' => 'eur', - 'created' => 102, 'recurring' => ['interval' => 'month'], - 'billing_scheme' => 'tiered', - 'metadata' => ['addon' => 'priority_support']], - ], - 'has_more' => false, - ]), - ]); - - $prices = (new HttpStripeClient)->activePricesFor('prod_1'); - - // The concrete harm: modules are billed BY quantity — SyncStripeAddonItems - // sums a pack into ONE item at quantity n — so price_divided is our exact - // figure, on our Product, carrying our `addon` key, and adoption would take - // it. A customer holding three would then be charged ceil(3/10) = 1. - // - // price_tiered is refused by name rather than by luck: Stripe reports - // unit_amount null for a tiered price, activePricesFor() casts that to 0, and - // the amount match alone only rejects it while the caller's own figure is not - // 0 — which PlanPrices::ensure(), unlike AddonPrices::ensure(), does not - // guarantee. - expect(collect($prices)->pluck('id')->all())->toBe(['price_ordinary']); -}); - -it('writes metadata onto a price that already exists', function () { - Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_a'])]); - - (new HttpStripeClient)->updatePriceMetadata('price_a', ['addon' => 'priority_support']); - - Http::assertSent(fn ($request) => $request->url() === 'https://api.stripe.com/v1/prices/price_a' - && $request['metadata[addon]'] === 'priority_support'); -}); - -it('lets the fake answer with the prices it holds, minus the archived ones', function () { - $fake = new FakeStripeClient; - - $kept = $fake->createPrice('prod_1', 3480, 'EUR', 'month', ['addon' => 'priority_support']); - $gone = $fake->createPrice('prod_1', 2900, 'EUR', 'month', ['addon' => 'priority_support']); - $other = $fake->createPrice('prod_2', 3480, 'EUR', 'month', []); - $fake->archivePrice($gone); - - $fake->plantPrice('price_orphan', 'prod_1', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], created: 0); - - $found = collect($fake->activePricesFor('prod_1'))->pluck('id')->all(); - - expect($found)->toContain($kept, 'price_orphan') - ->and($found)->not->toContain($gone, $other); -}); diff --git a/tests/Feature/Billing/StripePriceAdoptionTest.php b/tests/Feature/Billing/StripePriceAdoptionTest.php deleted file mode 100644 index df68ab1..0000000 --- a/tests/Feature/Billing/StripePriceAdoptionTest.php +++ /dev/null @@ -1,612 +0,0 @@ -stripe = new FakeStripeClient; - app()->instance(StripeClient::class, $this->stripe); -}); - -/** The module metadata as AddonPrices sends it today. */ -function moduleMetadata(string $treatment = 'domestic'): array -{ - return ['addon' => 'priority_support', 'tax_treatment' => $treatment]; -} - -/** Ask the adoption step the question AddonPrices asks it. */ -function adoptModulePrice(?array $metadata = null, ?callable $claimed = null): ?string -{ - return app(AdoptStripePrice::class)( - productId: 'prod_support', - amountCents: 3480, - currency: 'EUR', - interval: 'month', - metadata: $metadata ?? moduleMetadata(), - identifying: ['addon'], - claimed: $claimed ?? fn (string $id) => false, - ); -} - -it('adopts the orphan of 2026-07-29 instead of minting a second price', function () { - // The state that morning: the Price exists at Stripe, carries the metadata - // of the code that made it — WITHOUT tax_treatment, which 9da1358 added - // afterwards — and no row of ours knows it. - $this->stripe->plantPrice('price_1TygdEC7u8NpJ8pOt3nsoyYw', 'prod_support', - 3480, 'EUR', 'month', ['addon' => 'priority_support']); - - expect(adoptModulePrice())->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw'); - - // Brought up to today's metadata rather than replaced: metadata is mutable - // at Stripe, the amount is not, which is the whole reason the format is no - // part of a Price's identity. - expect($this->stripe->metadataUpdates)->toBe([[ - 'price' => 'price_1TygdEC7u8NpJ8pOt3nsoyYw', - 'metadata' => moduleMetadata(), - ]]); -}); - -it('leaves the metadata alone when it already says the right thing', function () { - $this->stripe->plantPrice('price_ok', 'prod_support', 3480, 'EUR', 'month', moduleMetadata()); - - expect(adoptModulePrice())->toBe('price_ok') - ->and($this->stripe->metadataUpdates)->toBe([]); -}); - -it('leaves the metadata alone when Stripe merged it in a different order plus a key of its own', function () { - // plantPrice() stores our own array in our own order, so the test above - // cannot catch an order- or merge-sensitive comparison: Stripe returns - // metadata as the result of a MERGE, never a replace, in whatever key - // order it likes — and a write never removes a key it did not send. - $this->stripe->plantPrice('price_ok', 'prod_support', 3480, 'EUR', 'month', [ - 'tax_treatment' => 'domestic', - 'addon' => 'priority_support', - 'internal_note' => 'added by hand in the Stripe dashboard', - ]); - - expect(adoptModulePrice())->toBe('price_ok') - ->and($this->stripe->metadataUpdates)->toBe([]); -}); - -it('adopts nothing when the amount, currency or interval differ', function () { - $this->stripe->plantPrice('price_cheaper', 'prod_support', 2900, 'EUR', 'month', moduleMetadata()); - $this->stripe->plantPrice('price_yearly', 'prod_support', 3480, 'EUR', 'year', moduleMetadata()); - $this->stripe->plantPrice('price_dollars', 'prod_support', 3480, 'USD', 'month', moduleMetadata()); - - expect(adoptModulePrice())->toBeNull(); -}); - -it('refuses a price nothing proves is ours, and says so', function () { - Log::spy(); - - // What a person clicking through Stripe's own dashboard leaves behind: the - // right money on our product, and not one word about what it is for. - $this->stripe->plantPrice('price_by_hand', 'prod_support', 3480, 'EUR', 'month', []); - - expect(adoptModulePrice())->toBeNull(); - - Log::shouldHaveReceived('warning')->once(); -}); - -it('passes silently over another of our own prices', function () { - Log::spy(); - - // At a VAT rate of nought both treatments are the same amount, so the - // reverse-charge Price sits at the domestic one's money — and contradicts on - // tax_treatment. That is not a mystery worth a warning; it is a Price of - // ours that is not the one being asked for. - $this->stripe->plantPrice('price_rc', 'prod_support', 3480, 'EUR', 'month', - moduleMetadata('reverse_charge')); - - expect(adoptModulePrice())->toBeNull(); - - Log::shouldNotHaveReceived('warning'); -}); - -it('never hands out a price a row already claims', function () { - $this->stripe->plantPrice('price_taken', 'prod_support', 3480, 'EUR', 'month', moduleMetadata()); - - expect(adoptModulePrice(claimed: fn (string $id) => $id === 'price_taken'))->toBeNull(); -}); - -it('adopts the oldest of several orphans and stops selling the rest', function () { - Log::spy(); - - $this->stripe->plantPrice('price_second', 'prod_support', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], created: 200); - $this->stripe->plantPrice('price_first', 'prod_support', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], created: 100); - $this->stripe->plantPrice('price_third', 'prod_support', 3480, 'EUR', 'month', - ['addon' => 'priority_support'], created: 300); - - // The oldest, because it is the one a lost row is likeliest to have been - // billing on. - expect(adoptModulePrice())->toBe('price_first') - ->and($this->stripe->archived)->toBe(['price_second', 'price_third']); - - Log::shouldHaveReceived('warning'); -}); - -it('takes the orphan over instead of minting a second module price', function () { - // The product exists because a previous run got that far; the Price exists - // because the run that made it died before the insert. - $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', - ]); - - $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', - ]); - - // Stripe's own side of that morning, and load-bearing for the same reason as - // the cleared ledger in 'takes over an orphaned package price' below — only - // the other way round. Without an entry here createPrice() would simply mint, - // and the test could not tell "adoption prevented the 400" from "no 400 was - // possible". So the ledger is seeded with what the pre-9da1358 call left - // behind: the id it was answered with, and a fingerprint over the metadata - // that call sent — `addon` alone, before tax_treatment existed. The entry sits - // under the header today's code sends, because that is the only way a ledger - // keyed by header can put this createPrice() in front of a poisoned key: the - // fingerprint is folded into the header now, so a changed call also changes - // the header, which is the branch's OTHER half of the same fix. Seeded - // directly, therefore, and honestly so — what it stands in for is Stripe - // holding this call's key against different parameters. - $this->stripe->keys[IdempotencyKey::forPrice( - 'clupilot-addon-price-priority_support-month-3480-EUR', - 'prod_support', 3480, 'EUR', 'month', moduleMetadata(), - )] = [ - 'id' => 'price_1TygdEC7u8NpJ8pOt3nsoyYw', - 'fingerprint' => IdempotencyKey::fingerprint(IdempotencyKey::priceParameters( - 'prod_support', 3480, 'EUR', 'month', ['addon' => 'priority_support'], - )), - ]; - - $id = app(AddonPrices::class)->ensure( - 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), - ); - - // Adoption is what keeps createPrice() from ever being reached. Take the - // adopt() call out of AddonPrices::ensure() and this test does not merely - // return the wrong id — it dies inside the fake with Stripe's own sentence, - // 'Keys for idempotent requests can only be used with the same parameters - // they were first used with.' That is the 2026-07-29 blockade, reproduced. - expect($id)->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw') - ->and($this->stripe->metadataUpdates)->toHaveCount(1); -}); - -it('leaves a frozen booking on the price it was sold at', function () { - // Seeds the product id AddonPrices::product() will find and reuse — same - // reason as the first two tests in this file. Without it, the sync below - // mints its own Stripe Product with a generated id, and the orphan planted - // further down (deliberately at the literal 'prod_support' Stripe uses - // throughout this file) would sit on a product nothing ever asks about, - // making it inert rather than the competing figure the test needs. - 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', - ]); - - 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'); - - // 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 — - // adoption of the orphan at the NEW figure must not have reached - // backwards and pulled the frozen figure's Price out of circulation. - ->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) - ->where('net_cents', 2900) - ->where('interval', 'month') - ->value('archived_at'))->toBeNull() - // And asking for the OLD figure again — the same call a renewal on - // this very booking would make — must still be handed $sold, not the - // orphan sitting at the new figure's amount. - ->and(app(AddonPrices::class)->ensure( - 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), - ))->toBe($sold); -}); - -it('takes over an orphaned package price', function () { - // A catalogue mirrored once, so families have Products and rows have Prices. - app(Kernel::class)->call('stripe:sync-catalogue'); - - $row = PlanPrice::query()->firstOrFail(); - $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic()); - - // The register loses its row and Stripe keeps the Price: a run that died - // between the two, seen from the next run's point of view. - $orphan = (string) StripePlanPrice::query() - ->where('plan_price_id', $row->id) - ->where('reverse_charge', false) - ->value('stripe_price_id'); - StripePlanPrice::query()->where('stripe_price_id', $orphan)->delete(); - - // Load-bearing, not incidental cleanup: Stripe forgets an idempotency key - // after twenty-four hours, and that expiry is the only condition under - // which the 2026-07-29 duplicate was ever minted. With the sync's own key - // still in the fake's ledger, createPrice() would just replay $orphan's - // id on its own — proving nothing about adoption. Clearing it is what - // makes createPrice() able to mint a genuine second Price, which is the - // only way this test can fail. - $this->stripe->keys = []; - - $before = count($this->stripe->prices); - - $id = app(PlanPrices::class)->ensure($row->refresh(), TaxTreatment::domestic()); - - expect($id)->toBe($orphan) - ->and(count($this->stripe->prices))->toBe($before) - ->and(StripePlanPrice::query() - ->where('plan_price_id', $row->id) - ->where('reverse_charge', false) - ->where('charged_cents', $charged) - ->value('stripe_price_id'))->toBe($orphan) - // The pointer for the ordinary domestic sale is written as before. - ->and((string) $row->refresh()->stripe_price_id)->toBe($orphan); -}); - -it('does not take a package price belonging to another catalogue row', function () { - app(Kernel::class)->call('stripe:sync-catalogue'); - - $row = PlanPrice::query()->firstOrFail(); - $product = (string) $row->version->family->stripe_product_id; - $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic()); - $interval = $row->term === Subscription::TERM_YEARLY ? 'year' : 'month'; - - // This row's OWN domestic Price from the sync above becomes a legitimate - // orphan the moment its register row is gone — exactly like the test - // above — and adopt() would be right to reclaim it. Archived here so the - // only Price left at this exact figure is the wrong-owner one below: - // otherwise adoption would correctly adopt the row's own orphan before - // ever weighing 'price_other_row', and the rejection this test exists to - // prove would never be reached. - $this->stripe->archivePrice((string) StripePlanPrice::query() - ->where('plan_price_id', $row->id) - ->where('reverse_charge', false) - ->value('stripe_price_id')); - - StripePlanPrice::query()->where('plan_price_id', $row->id)->delete(); - - // Same product, same money, same interval — and plan_price_id says it is a - // DIFFERENT row's Price. One Product carries every version and term of a - // family, so this is the ordinary case, not an exotic one. - $this->stripe->plantPrice('price_other_row', $product, $charged, (string) $row->currency, - $interval, ['plan_price_id' => (string) ($row->id + 1000), 'tax_treatment' => 'domestic']); - - // Same reason as the test above: with the sync's own idempotency key - // still in force, createPrice() would replay THIS row's own previous - // Price — which would also not be 'price_other_row', whether or not - // adoption ever looked at the wrong-owner Price at all. Clearing the - // ledger forces a real mint when nothing adoptable is found, which is - // what gives the assertions below something to catch. - $this->stripe->keys = []; - - $before = count($this->stripe->prices); - - $id = app(PlanPrices::class)->ensure($row->refresh(), TaxTreatment::domestic()); - - expect($id)->not->toBe('price_other_row') - // A fresh Price was minted rather than 'price_other_row' being handed - // out. Proves AdoptStripePrice's contradicts()/confirms() pair - // correctly refused the wrong-owner Price WHEN adoption runs — the - // two guard each other here exactly as they do on the module side, so - // either alone still catches this; only disabling both together lets - // 'price_other_row' through. It does NOT prove PlanPrices wires - // adopt() into ensure() at all: skipping that wiring entirely mints - // fresh here too, indistinguishable from a correct refusal. The test - // above ("takes over an orphaned package price") is what proves the - // wiring is present. - ->and(count($this->stripe->prices))->toBe($before + 1) - // The Price belonging to the other row must survive untouched. - // AdoptStripePrice archives every orphan but the one it adopts, so a - // defect that let it treat 'price_other_row' as a duplicate of - // whatever it did adopt would take this down as collateral damage — - // exactly the failure a wrong plan_price_id must never cause. - ->and($this->stripe->archived)->not->toContain('price_other_row'); -}); - -it('does not adopt a package price that only proves the family, not this row', function () { - // The whole reason the plan side needs `identifying: ['plan_price_id']` - // rather than the module side's simpler key: one Product carries a Price - // for every version, term AND treatment of a family, so a Price that - // proves only the FAMILY could be any of them. If `identifying` were ever - // loosened to ['plan_family'], a Price like the one planted below would - // wrongly confirm — nothing else here would catch it. - app(Kernel::class)->call('stripe:sync-catalogue'); - - $row = PlanPrice::query()->firstOrFail(); - $product = (string) $row->version->family->stripe_product_id; - $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic()); - $interval = $row->term === Subscription::TERM_YEARLY ? 'year' : 'month'; - - // This row's OWN domestic Price is a legitimate orphan the moment its - // register row is gone — same reason as the two tests above — and would - // otherwise be correctly re-adopted before the planted Price below is - // ever weighed, masking the very rejection this test means to prove. - $this->stripe->archivePrice((string) StripePlanPrice::query() - ->where('plan_price_id', $row->id) - ->where('reverse_charge', false) - ->value('stripe_price_id')); - - StripePlanPrice::query()->where('plan_price_id', $row->id)->delete(); - - // Right product, right money, right currency, right interval, and the - // right FAMILY — the one thing missing is plan_price_id, which is the - // only key that says which of the family's many rows a Price belongs to. - $this->stripe->plantPrice('price_family_only', $product, $charged, (string) $row->currency, - $interval, ['plan_family' => $row->version->family->key]); - - // Same reason as the two tests above: with the sync's own idempotency - // key still in force, createPrice() would replay this row's own previous - // Price rather than genuinely minting, leaving nothing for the - // assertions below to catch. - $this->stripe->keys = []; - - $before = count($this->stripe->prices); - - $id = app(PlanPrices::class)->ensure($row->refresh(), TaxTreatment::domestic()); - - expect($id)->not->toBe('price_family_only') - // A fresh Price was minted rather than the family-only Price being - // handed out — this is the assertion `identifying: ['plan_price_id']` - // exists to keep true; swap it for ['plan_family'] and this fails. - ->and(count($this->stripe->prices))->toBe($before + 1) - ->and($this->stripe->archived)->not->toContain('price_family_only'); -}); - -it('lets no two module rows claim one stripe price', function () { - $shared = [ - 'addon_key' => 'priority_support', 'net_cents' => 2900, 'currency' => 'EUR', - 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', - ]; - - StripeAddonPrice::query()->create([...$shared, - 'reverse_charge' => false, 'amount_cents' => 3480, 'interval' => 'month']); - - // Two rows on one Price is what Block D warned about: archiving the one - // would withdraw the Price the other is still selling. - expect(fn () => StripeAddonPrice::query()->create([...$shared, - 'reverse_charge' => true, 'amount_cents' => 2900, 'interval' => 'month'])) - ->toThrow(UniqueConstraintViolationException::class); -}); - -it('keeps only the lowest-id row in every group of duplicates, and leaves the rest of the table alone', function () { - // This is the one branch of 2026_07_31_210000_one_row_per_stripe_price that - // deletes rows from a live table, and it cannot be reached through the - // ordinary migrated schema: RefreshDatabase already ran this migration, so - // its OWN unique index would refuse the very duplicate this test needs to - // insert. Dropping the index first puts the table back into the shape the - // migration was written to find — the shape a real, never-yet-migrated - // production table was in — so up() can be run again and actually take its - // delete branch instead of finding nothing to do. - Schema::table('stripe_addon_prices', fn (Blueprint $table) => $table->dropUnique('stripe_addon_prices_price_unique')); - - // Group one: two rows sharing a Price, differing in `reverse_charge` so the - // pre-existing COMPOSITE unique index (addon_key, reverse_charge, - // amount_cents, currency, interval) does not itself refuse the insert — - // the VAT-rate-zero case Block D warned about, where only the shared - // stripe_price_id gives the duplicate away. - $kept = DB::table('stripe_addon_prices')->insertGetId([ - 'addon_key' => 'priority_support', 'reverse_charge' => false, - 'amount_cents' => 2900, 'net_cents' => 2900, 'currency' => 'EUR', 'interval' => 'month', - 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', - 'created_at' => now(), 'updated_at' => now(), - ]); - - $duplicate = DB::table('stripe_addon_prices')->insertGetId([ - 'addon_key' => 'priority_support', 'reverse_charge' => true, - 'amount_cents' => 2900, 'net_cents' => 2900, 'currency' => 'EUR', 'interval' => 'month', - 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', - 'created_at' => now(), 'updated_at' => now(), - ]); - - // Group two: THREE rows sharing a second, different Price — its own tuple - // dimension varied per row (interval, then currency) so the composite index - // never fires here either. One group of two proves the loop runs; this one - // proves it runs more than once AND that a single iteration can delete more - // than one row. - $kept2 = DB::table('stripe_addon_prices')->insertGetId([ - 'addon_key' => 'priority_support', 'reverse_charge' => false, - 'amount_cents' => 3900, 'net_cents' => 3900, 'currency' => 'EUR', 'interval' => 'month', - 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared_two', - 'created_at' => now(), 'updated_at' => now(), - ]); - - $duplicate2a = DB::table('stripe_addon_prices')->insertGetId([ - 'addon_key' => 'priority_support', 'reverse_charge' => false, - 'amount_cents' => 3900, 'net_cents' => 3900, 'currency' => 'EUR', 'interval' => 'year', - 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared_two', - 'created_at' => now(), 'updated_at' => now(), - ]); - - $duplicate2b = DB::table('stripe_addon_prices')->insertGetId([ - 'addon_key' => 'priority_support', 'reverse_charge' => false, - 'amount_cents' => 3900, 'net_cents' => 3900, 'currency' => 'USD', 'interval' => 'month', - 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared_two', - 'created_at' => now(), 'updated_at' => now(), - ]); - - // On its own Price entirely — not part of any duplicate group — and must - // survive completely untouched. - $unrelated = DB::table('stripe_addon_prices')->insertGetId([ - 'addon_key' => 'extra_storage', 'reverse_charge' => false, - 'amount_cents' => 1000, 'net_cents' => 1000, 'currency' => 'EUR', 'interval' => 'month', - 'stripe_product_id' => 'prod_storage', 'stripe_price_id' => 'price_untouched', - 'created_at' => now(), 'updated_at' => now(), - ]); - - // require, not require_once: the file `return`s a fresh anonymous-class - // instance every time it is evaluated, which is what lets this run the - // migration's up() a second time in this same process. - (require database_path('migrations/2026_07_31_210000_one_row_per_stripe_price.php'))->up(); - - $keptRow = DB::table('stripe_addon_prices')->where('id', $kept)->first(); - $kept2Row = DB::table('stripe_addon_prices')->where('id', $kept2)->first(); - $unrelatedRow = DB::table('stripe_addon_prices')->where('id', $unrelated)->first(); - - // The LOWER id survives in each group — the row that was written first, - // which is the one anything already billing is likeliest to have been - // reading. Were that ordering silently inverted, the surviving row could be - // one nothing ever pointed at. Read back, not merely checked for existence: - // a survivor that exists but was mangled in the process — the wrong - // stripe_price_id, or an amount_cents that no longer matches what the - // shared Price actually charges — would be exactly the "kept the right row - // but damaged it" failure existence alone cannot catch. - expect($keptRow)->not->toBeNull() - ->and($keptRow->stripe_price_id)->toBe('price_shared') - ->and((int) $keptRow->amount_cents)->toBe(2900) - ->and($kept2Row)->not->toBeNull() - ->and($kept2Row->stripe_price_id)->toBe('price_shared_two') - ->and((int) $kept2Row->amount_cents)->toBe(3900) - ->and($unrelatedRow)->not->toBeNull() - ->and($unrelatedRow->stripe_price_id)->toBe('price_untouched') - ->and((int) $unrelatedRow->amount_cents)->toBe(1000) - // Every duplicate in both groups is gone — the higher id from group - // one, and BOTH higher ids from group two's three-way tie. - ->and(DB::table('stripe_addon_prices')->where('id', $duplicate)->exists())->toBeFalse() - ->and(DB::table('stripe_addon_prices')->where('id', $duplicate2a)->exists())->toBeFalse() - ->and(DB::table('stripe_addon_prices')->where('id', $duplicate2b)->exists())->toBeFalse() - // Six rows went in, three survive: nothing beyond the two kept - // duplicates and the untouched row is left standing. - ->and(DB::table('stripe_addon_prices')->count())->toBe(3); - - // And the index is back in force: a further row on a third, otherwise - // distinct tuple, but the SAME Price id, is refused by stripe_price_id - // alone rather than being silently accepted. - expect(fn () => DB::table('stripe_addon_prices')->insert([ - 'addon_key' => 'priority_support', 'reverse_charge' => false, - 'amount_cents' => 4900, 'net_cents' => 4900, 'currency' => 'EUR', 'interval' => 'year', - 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', - 'created_at' => now(), 'updated_at' => now(), - ]))->toThrow(UniqueConstraintViolationException::class); -});