From 8f1630ba918a680bf0c30f1f9cb6b1deb627f6ba Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 20:22:02 +0200 Subject: [PATCH] Bill a booked module every month, and put it on the invoice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A module was never an item on the Stripe subscription, so it was charged in the month it was booked and never again. And the renewal document was written from our own contract snapshot, so it would not have said so even if the money had been taken. Two halves of one fault, closed together. Modules are items now. stripe:sync-catalogue mirrors a Product and a Price per module on both terms — a Stripe Price carries its own interval, and a booking frozen at an older figure needs its own Price, so they are keyed by module, amount, currency and interval in stripe_addon_prices. Booking adds an item with always_invoice: prorated by days AND charged there and then, which is the invoice the owner wants for a module booked mid-term. A second storage pack is a quantity on one item, not a second item. Cancelling takes the item off with no proration — no credit, and the next cycle is simply smaller — while the module itself is kept until the period end on subscription_addons.cancels_at and ended by clupilot:end-cancelled-addons. Documents are built from Stripe's own invoice lines. Stripe is what charged the customer; a document assembled from our figures would state a sum that is not the one taken. So every paid Stripe invoice gets one document with one number, idempotent on stripe_invoice_id — a cycle carrying package and every module together, a mid-period booking carrying just its prorated line, an upgrade carrying its proration. Only the wording is ours: each line is named from the catalogue rather than printed as a Price id, and a line nobody can name stays on the document under Stripe's own description and is logged. The checkout's own invoice is still the one exception; that purchase already has a document. Stripe being away never undoes a booking that has already reached the machine. The failure is parked on the contract in stripe_addon_sync and swept by the same clupilot:sync-stripe-subscriptions that retries a plan change, which finds its work from the bookings themselves rather than from the marker. A granted contract has no Stripe subscription and is not touched at all. Co-Authored-By: Claude Opus 5 --- app/Actions/ApplyStripeBillingEvent.php | 62 +-- app/Actions/BookAddon.php | 88 +++- app/Actions/IssueRenewalInvoice.php | 127 ----- app/Actions/IssueStripeInvoice.php | 297 +++++++++++ app/Actions/SyncStripeAddonItems.php | 299 +++++++++++ app/Console/Commands/EndCancelledAddons.php | 75 +++ app/Console/Commands/SyncStripeCatalogue.php | 65 ++- .../Commands/SyncStripeSubscriptions.php | 81 ++- app/Models/StripeAddonPrice.php | 24 + app/Models/Subscription.php | 4 + app/Models/SubscriptionAddon.php | 11 + app/Services/Billing/AddonPrices.php | 147 ++++++ app/Services/Billing/IssueInvoice.php | 78 ++- app/Services/Billing/StripeInvoiceLines.php | 183 +++++++ app/Services/Stripe/FakeStripeClient.php | 102 ++++ app/Services/Stripe/HttpStripeClient.php | 71 +++ app/Services/Stripe/StripeClient.php | 62 ++- ..._29_290000_bill_modules_through_stripe.php | 101 ++++ lang/de/invoice.php | 6 + lang/en/invoice.php | 2 + routes/console.php | 14 + tests/Feature/Billing/RenewalInvoiceTest.php | 12 +- .../Billing/StripeAddonBillingTest.php | 490 ++++++++++++++++++ tests/Feature/Billing/StripeBillingTest.php | 71 ++- 24 files changed, 2263 insertions(+), 209 deletions(-) delete mode 100644 app/Actions/IssueRenewalInvoice.php create mode 100644 app/Actions/IssueStripeInvoice.php create mode 100644 app/Actions/SyncStripeAddonItems.php create mode 100644 app/Console/Commands/EndCancelledAddons.php create mode 100644 app/Models/StripeAddonPrice.php create mode 100644 app/Services/Billing/AddonPrices.php create mode 100644 app/Services/Billing/StripeInvoiceLines.php create mode 100644 database/migrations/2026_07_29_290000_bill_modules_through_stripe.php create mode 100644 tests/Feature/Billing/StripeAddonBillingTest.php diff --git a/app/Actions/ApplyStripeBillingEvent.php b/app/Actions/ApplyStripeBillingEvent.php index 0ad839e..ddbbb20 100644 --- a/app/Actions/ApplyStripeBillingEvent.php +++ b/app/Actions/ApplyStripeBillingEvent.php @@ -23,9 +23,9 @@ use Throwable; * * The one thing Stripe does NOT own is the document. Their invoice numbering is * theirs and is not a lawful Austrian invoice series; ours is, and it is gapless - * because it has to be. So a renewal Stripe reports as paid produces a document - * of our own and one mail carrying it — see App\Actions\IssueRenewalInvoice, and - * invoicePaid() below for which paid invoices get one and which do not. + * because it has to be. So EVERY invoice Stripe reports as paid produces a + * document of our own and one mail carrying it — see App\Actions\IssueStripeInvoice, + * and invoicePaid() below for the single exception. * * Every handler is idempotent. Stripe retries a webhook until it gets a 2xx, * and it also sends the same event to several endpoints — so "already applied" @@ -49,7 +49,7 @@ class ApplyStripeBillingEvent public function __construct( private RecordCommercialEvent $record, - private IssueRenewalInvoice $issueRenewalInvoice, + private IssueStripeInvoice $issueStripeInvoice, ) {} /** @@ -128,44 +128,22 @@ class ApplyStripeBillingEvent ) ); - if ($isRenewal && $start !== null && $end !== null) { - $this->invoiceRenewal($subscription, $invoiceId, $start, $end); - } elseif ($record !== null) { - // Money that gets no document from us, said out loud once — when it - // is first recorded, not on every redelivery. - // - // Which paid invoices those are, and why none of them is invoiced - // from here: - // - // - `subscription_create` is the checkout's own invoice. The - // purchase already has a document, issued the moment the money - // landed; a second one would put two invoices on one sale. (It - // never reaches this line — the handler returns above.) - // - `subscription_update` is an upgrade's proration. Stripe worked - // the amount out against ITS period boundaries and its own view - // of what is already on the account, and a document written from - // our contract snapshot would state a sum that is not the one - // charged. The move itself is in the register as an `upgrade`, - // carrying the pro-rata the customer was quoted. - // - a charge raised by hand in Stripe's dashboard is for something - // this catalogue knows nothing about. We cannot describe what was - // sold, and an invoice line that is a guess is worse than none. - // - // All three are in the register with the amount, which is what the - // console's payments panel reads. This is the trail for whoever has - // to raise the paper. - Log::warning('A payment was received that no document of ours covers.', [ - 'subscription' => $subscription->uuid, - 'stripe_invoice' => $invoiceId ?: null, - 'billing_reason' => $reason ?: null, - ]); - } + $this->invoicePayment($subscription, $invoice, $start, $end); return $record; } /** - * The renewal's document and the one mail that carries it. + * The document this payment owes the customer, and the one mail that carries + * it. + * + * **Every paid invoice, not only a renewal.** The rule is one document per + * charge, so a proration, a module booked in the middle of a term and a + * charge raised by hand in Stripe's dashboard each get one — built from + * Stripe's own lines, which is what actually took the money. The single + * exception is the checkout's own invoice, which returns before this line: + * that purchase already has a document, and a second would put two invoices + * on one sale. * * Outside recordOnce() on purpose. The register entry and the invoice are * two different obligations, and tying the second to the first succeeding @@ -179,15 +157,17 @@ class ApplyStripeBillingEvent * this guards the rest, because the rule is that no paperwork of ours may * ever cost Stripe a 2xx — a retried webhook is a second attempt at * everything, and one of those things moves the customer's term. + * + * @param array $invoice */ - private function invoiceRenewal(Subscription $subscription, string $invoiceId, Carbon $start, Carbon $end): void + private function invoicePayment(Subscription $subscription, array $invoice, ?Carbon $start, ?Carbon $end): void { try { - ($this->issueRenewalInvoice)($subscription->refresh(), $invoiceId, $start, $end); + ($this->issueStripeInvoice)($subscription->refresh(), $invoice, $start, $end); } catch (Throwable $e) { - Log::error('Failed to invoice a renewal that was paid.', [ + Log::error('Failed to invoice a Stripe payment that went through.', [ 'subscription' => $subscription->uuid, - 'stripe_invoice' => $invoiceId ?: null, + 'stripe_invoice' => $invoice['id'] ?? null, 'error' => $e->getMessage(), ]); } diff --git a/app/Actions/BookAddon.php b/app/Actions/BookAddon.php index 366bb38..91d3c65 100644 --- a/app/Actions/BookAddon.php +++ b/app/Actions/BookAddon.php @@ -40,6 +40,13 @@ use RuntimeException; * its provenance stamped on the row. Empty for every ordinary booking. A grant * is a booking like any other, so it is refused on the same terms — giving away * a module the customer is already paying for would bill them for both. + * + * It is also where a booking becomes MONEY, every month, and not only in the + * month it was made. A module is an item on the Stripe subscription, so booking + * one adds it and cancelling one takes it off — see + * App\Actions\SyncStripeAddonItems, which is asked to bring the two into step + * after every change here. Like the storage delivery beside it, it never fails a + * booking: what it could not tell Stripe is parked on the contract and retried. */ class BookAddon { @@ -87,6 +94,7 @@ class BookAddon } $this->deliverStorage($subscription, $addonKey); + $this->billThroughStripe($subscription); return $addon; } @@ -163,10 +171,65 @@ class BookAddon } /** - * Stop charging for a module, without losing what it cost. + * Book the end of a module for the end of the term it is paid up to. + * + * The owner's rule for a module is the same as for a package: what has been + * paid for is kept. A customer cancelling on the tenth keeps the module + * until the cycle turns and gets no credit for the days they did not use — + * so the row stays active, `cancels_at` holds the appointment, and + * clupilot:end-cancelled-addons keeps it. + * + * Stripe is told at once all the same. Its item is only ever about what the + * NEXT cycle bills — the term in progress was paid in advance — and taking + * the item off with no proration moves no money while making sure the next + * invoice has no such line. Waiting for the boundary instead would be a race + * against Stripe raising that very invoice; see SyncStripeAddonItems. + * + * Ends the module outright when there is no term to wait for — a contract + * with no period end, or one that is already past. There is nothing to keep. + */ + public function cancelAtPeriodEnd(SubscriptionAddon $addon): SubscriptionAddon + { + $subscription = $addon->subscription; + $endsOn = $subscription?->current_period_end; + + if ($endsOn === null || $endsOn->isPast()) { + return $this->cancel($addon); + } + + // Claimed conditionally, like the cancellation below: two requests + // arriving together would otherwise both write an appointment, and the + // second would move a date the customer had already been told. + $claimed = SubscriptionAddon::query() + ->whereKey($addon->getKey()) + ->whereNull('cancelled_at') + ->whereNull('cancels_at') + ->update(['cancels_at' => $endsOn, 'updated_at' => now()]); + + $addon->refresh(); + + if ($claimed === 0) { + return $addon; + } + + $this->billThroughStripe($subscription); + + return $addon; + } + + /** + * Stop charging for a module now, without losing what it cost. * * Cancelled, not deleted: what a customer was paying, and until when, is * part of the same record as what they bought. + * + * This is the immediate end, and it is not what a customer's own + * cancellation does — see cancelAtPeriodEnd() above. It is what happens when + * a module stops being POSSIBLE rather than wanted: a downgrade onto a + * package that cannot have an own domain takes the module with it, and + * leaving the customer billed for a feature that has already gone is a + * complaint we would deserve. It is also the appointment being kept, once + * cancelAtPeriodEnd()'s date has passed. */ public function cancel(SubscriptionAddon $addon): SubscriptionAddon { @@ -198,6 +261,7 @@ class BookAddon }); $this->deliverStorage($cancelled->subscription, (string) $cancelled->addon_key); + $this->billThroughStripe($cancelled->subscription); return $cancelled; } @@ -228,4 +292,26 @@ class BookAddon app(ApplyStorageAllowance::class)($subscription); } + + /** + * Put what the contract now holds in front of Stripe. + * + * After the transaction for the same reason the storage delivery is: the + * reconciler reads the bookings back out of the database, and one running + * inside the transaction that wrote them would settle on the state as it was + * a moment ago. + * + * Nothing here fails a booking either. The module has been booked and, if it + * was storage, already delivered to the machine; Stripe being unreachable + * parks the difference on the contract and clupilot:sync-stripe-subscriptions + * finishes it. + */ + private function billThroughStripe(?Subscription $subscription): void + { + if ($subscription === null) { + return; + } + + app(SyncStripeAddonItems::class)($subscription->refresh()); + } } diff --git a/app/Actions/IssueRenewalInvoice.php b/app/Actions/IssueRenewalInvoice.php deleted file mode 100644 index 978f02a..0000000 --- a/app/Actions/IssueRenewalInvoice.php +++ /dev/null @@ -1,127 +0,0 @@ - $subscription->uuid, - ]); - - return null; - } - - $invoice = $this->issue($subscription, $stripeInvoiceId, $from, $to); - - if ($invoice === null || $invoice->sent_at !== null) { - return $invoice; - } - - $this->send($subscription, $invoice); - - return $invoice; - } - - private function issue(Subscription $subscription, string $stripeInvoiceId, Carbon $from, Carbon $to): ?Invoice - { - $existing = Invoice::query()->where('stripe_invoice_id', $stripeInvoiceId)->first(); - - if ($existing !== null) { - return $existing; - } - - try { - return app(IssueInvoice::class)->forBilledPeriod($subscription, $from, $to, $stripeInvoiceId); - } catch (UniqueConstraintViolationException) { - // A concurrent delivery won the race. Its document is the one to - // send; this one's invoice number went back into the series when the - // transaction rolled back. - return Invoice::query()->where('stripe_invoice_id', $stripeInvoiceId)->first(); - } catch (Throwable $e) { - // Refuses while the company details are incomplete, which is the - // correct outcome — see IssueInvoice. The renewal itself stands: the - // term has moved and the payment is in the register, and the - // document can be issued once the details are there. - Log::warning('Could not issue an invoice for this renewal.', [ - 'subscription' => $subscription->uuid, - 'stripe_invoice' => $stripeInvoiceId, - 'exception' => $e->getMessage(), - ]); - - return null; - } - } - - private function send(Subscription $subscription, Invoice $invoice): void - { - $customer = $subscription->customer; - $address = $customer?->email; - - if (! is_string($address) || $address === '') { - return; - } - - try { - Mail::to($address)->queue(new InvoiceMail($invoice, (string) $customer->name)); - - // Only once it is actually with the mailer. Stamped so a redelivery - // does not send the same invoice a second time, and left unstamped - // on a failure so that one still can. - $invoice->update(['sent_at' => now()]); - } catch (Throwable $e) { - Log::warning('Could not queue the invoice mail for this renewal.', [ - 'invoice' => $invoice->number, - 'exception' => $e->getMessage(), - ]); - } - } -} diff --git a/app/Actions/IssueStripeInvoice.php b/app/Actions/IssueStripeInvoice.php new file mode 100644 index 0000000..b8ae547 --- /dev/null +++ b/app/Actions/IssueStripeInvoice.php @@ -0,0 +1,297 @@ + $stripeInvoice the invoice as Stripe sent it + * @param Carbon|null $from the term this invoice billed, for the fallback below + * @return Invoice|null the document, or null when there is not one and could not be one + */ + public function __invoke( + Subscription $subscription, + array $stripeInvoice, + ?Carbon $from = null, + ?Carbon $to = null, + ): ?Invoice { + $stripeInvoiceId = (string) ($stripeInvoice['id'] ?? ''); + + if ($stripeInvoiceId === '') { + // Without it there is no way to tell a redelivery from a second + // charge, and guessing wrong costs an invoice number. + Log::warning('Not issuing a document: the Stripe invoice carries no id, so issuing it could not be made idempotent.', [ + 'subscription' => $subscription->uuid, + ]); + + return null; + } + + $invoice = $this->issue($subscription, $stripeInvoiceId, $stripeInvoice, $from, $to); + + if ($invoice === null || $invoice->sent_at !== null) { + return $invoice; + } + + $this->send($subscription, $invoice); + + return $invoice; + } + + /** @param array $stripeInvoice */ + private function issue( + Subscription $subscription, + string $stripeInvoiceId, + array $stripeInvoice, + ?Carbon $from, + ?Carbon $to, + ): ?Invoice { + $existing = Invoice::query()->where('stripe_invoice_id', $stripeInvoiceId)->first(); + + if ($existing !== null) { + return $existing; + } + + try { + $lines = $this->lines($stripeInvoiceId, $stripeInvoice); + } catch (Throwable $e) { + // Deliberately no document at all rather than a partial one. The + // total would still be right and the reader would have no way to see + // what they had paid for, which is the kind of invoice that gets + // rejected at an audit. A later redelivery or a replay can still + // issue it. + Log::error('Could not read the lines of a paid Stripe invoice, so no document was issued for it.', [ + 'subscription' => $subscription->uuid, + 'stripe_invoice' => $stripeInvoiceId, + 'exception' => $e->getMessage(), + ]); + + return null; + } + + try { + if ($lines === []) { + return $this->fromContract($subscription, $stripeInvoiceId, $stripeInvoice, $from, $to); + } + + $unnamed = []; + + $invoice = app(IssueInvoice::class)->forStripeInvoice( + subscription: $subscription, + stripeInvoiceId: $stripeInvoiceId, + lines: $lines, + currency: is_string($stripeInvoice['currency'] ?? null) ? $stripeInvoice['currency'] : null, + unnamed: $unnamed, + ); + + $this->reportUnnamed($subscription, $stripeInvoiceId, $unnamed); + $this->reportMismatch($invoice, $stripeInvoice); + + return $invoice; + } catch (UniqueConstraintViolationException) { + // A concurrent delivery won the race. Its document is the one to + // send; this one's invoice number went back into the series when the + // transaction rolled back. + return Invoice::query()->where('stripe_invoice_id', $stripeInvoiceId)->first(); + } catch (Throwable $e) { + // Refuses while the company details are incomplete, which is the + // correct outcome — see IssueInvoice. The payment itself stands: it + // is in the register, and the document can be issued once the + // details are there. + Log::warning('Could not issue a document for this paid Stripe invoice.', [ + 'subscription' => $subscription->uuid, + 'stripe_invoice' => $stripeInvoiceId, + 'exception' => $e->getMessage(), + ]); + + return null; + } + } + + /** + * Every line Stripe charged for, complete. + * + * The event carries them already. It does not always carry ALL of them — a + * contract with enough modules on it passes the number Stripe puts in a + * webhook — and `has_more` is how it says so. A document missing a charged + * line is worse than an extra API call, so the complete list is fetched + * whenever Stripe admits to holding one. + * + * @param array $stripeInvoice + * @return array> + */ + private function lines(string $stripeInvoiceId, array $stripeInvoice): array + { + $lines = (array) ($stripeInvoice['lines']['data'] ?? []); + + if (($stripeInvoice['lines']['has_more'] ?? false) !== true) { + return $lines; + } + + return app(StripeClient::class)->invoiceLines($stripeInvoiceId); + } + + /** + * The document for a cycle invoice that arrived without any line detail. + * + * The contract's own frozen price is an honest description of the term that + * was billed, and better than leaving a paid renewal with no paper at all. + * Only for a cycle: a proration or a charge raised by hand describes + * something the contract cannot answer for, and a line invented for those + * would be a guess printed on a document nobody can correct afterwards. + * + * @param array $stripeInvoice + */ + private function fromContract( + Subscription $subscription, + string $stripeInvoiceId, + array $stripeInvoice, + ?Carbon $from, + ?Carbon $to, + ): ?Invoice { + $isCycle = (string) ($stripeInvoice['billing_reason'] ?? '') === 'subscription_cycle'; + + if (! $isCycle || $from === null || $to === null) { + Log::warning('A paid Stripe invoice carried no lines, so no document could be built from it.', [ + 'subscription' => $subscription->uuid, + 'stripe_invoice' => $stripeInvoiceId, + 'billing_reason' => $stripeInvoice['billing_reason'] ?? null, + ]); + + return null; + } + + return app(IssueInvoice::class)->forBilledPeriod($subscription, $from, $to, $stripeInvoiceId); + } + + /** + * Say out loud that a line was charged which the catalogue could not name. + * + * The line is on the document either way, carrying whatever description + * Stripe sent. This is the trail for whoever has to work out what was sold — + * a Price created by hand in Stripe's dashboard, or a module removed from + * the catalogue while somebody was still paying for it. + * + * @param array $unnamed + */ + private function reportUnnamed(Subscription $subscription, string $stripeInvoiceId, array $unnamed): void + { + if ($unnamed === []) { + return; + } + + Log::warning('A charged line was put on an invoice under Stripe’s own description, because this catalogue could not name it.', [ + 'subscription' => $subscription->uuid, + 'stripe_invoice' => $stripeInvoiceId, + 'prices' => $unnamed, + ]); + } + + /** + * Say out loud when the document and the money do not agree. + * + * Our catalogue is pushed to Stripe as NET, so the line amounts that come + * back are net and the document adds VAT on top: Stripe's total should be + * our gross. It may legitimately be our NET instead, on an account that does + * not collect tax on our behalf. A figure that is neither is one nobody can + * explain, and it is exactly what an operator needs to be told about. + * + * Never an exception. The document is issued and the payment is recorded; + * raising here would only cost Stripe its 2xx and change nothing. + * + * @param array $stripeInvoice + */ + private function reportMismatch(Invoice $invoice, array $stripeInvoice): void + { + $total = $stripeInvoice['total'] ?? $stripeInvoice['amount_paid'] ?? null; + + if (! is_numeric($total)) { + return; + } + + $total = (int) $total; + + if ($total === $invoice->gross_cents || $total === $invoice->net_cents) { + return; + } + + Log::warning('An invoice was issued for a sum that is neither the net nor the gross Stripe reported.', [ + 'invoice' => $invoice->number, + 'stripe_invoice' => $invoice->stripe_invoice_id, + 'stripe_total' => $total, + 'net' => $invoice->net_cents, + 'gross' => $invoice->gross_cents, + ]); + } + + private function send(Subscription $subscription, Invoice $invoice): void + { + $customer = $subscription->customer; + $address = $customer?->email; + + if (! is_string($address) || $address === '') { + return; + } + + try { + Mail::to($address)->queue(new InvoiceMail($invoice, (string) $customer->name)); + + // Only once it is actually with the mailer. Stamped so a redelivery + // does not send the same invoice a second time, and left unstamped + // on a failure so that one still can. + $invoice->update(['sent_at' => now()]); + } catch (Throwable $e) { + Log::warning('Could not queue the invoice mail for this payment.', [ + 'invoice' => $invoice->number, + 'exception' => $e->getMessage(), + ]); + } + } +} diff --git a/app/Actions/SyncStripeAddonItems.php b/app/Actions/SyncStripeAddonItems.php new file mode 100644 index 0000000..0fbc542 --- /dev/null +++ b/app/Actions/SyncStripeAddonItems.php @@ -0,0 +1,299 @@ +stripe_subscription_id === null) { + $this->settled($subscription); + + return true; + } + + try { + foreach ($this->groups($subscription) as $group) { + $this->reconcile($subscription, $group); + } + + $this->settled($subscription); + + return true; + } catch (Throwable $e) { + $this->park($subscription, $e); + + return false; + } + } + + /** + * The bookings that matter, grouped into the items they should be. + * + * By module AND by frozen price, because that pair is what a Stripe Price + * is: two packs booked at the same money are one item with a quantity of + * two, and a third booked after the catalogue moved is a second item at the + * price that customer agreed to. Grouping by module alone would have to pick + * one of two prices and charge everybody that. + * + * Cancelled bookings are in here as long as they still carry an item id — + * that is the only record of what Stripe was last told, and without it the + * quantity could never be brought back down. + * + * @return Collection> + */ + private function groups(Subscription $subscription): Collection + { + return SubscriptionAddon::query() + ->where('subscription_id', $subscription->id) + ->where(fn ($q) => $q->whereNull('cancelled_at')->orWhereNotNull('stripe_item_id')) + ->orderBy('id') + ->get() + ->groupBy(fn (SubscriptionAddon $addon) => $addon->addon_key.'@'.$addon->price_cents); + } + + /** + * Bring one module's item into step with what the contract holds. + * + * @param Collection $group + */ + private function reconcile(Subscription $subscription, Collection $group): void + { + // What Stripe should bill: every booking still running and not already + // on its way out. A module with `cancels_at` set is deliberately absent + // — the customer keeps it, nobody is billed for it again. + $billable = $group->filter( + fn (SubscriptionAddon $addon) => $addon->cancelled_at === null && $addon->cancels_at === null + ); + + // What Stripe was last told, read off the rows that carry the item id. + $stamped = $group->filter(fn (SubscriptionAddon $addon) => $addon->stripe_item_id !== null); + + $desired = (int) $billable->sum('quantity'); + $known = (int) $stamped->sum('quantity'); + $itemId = $stamped->first()?->stripe_item_id; + + $first = $group->first(); + + // A module given away costs nothing, so there is nothing for Stripe to + // bill — and a zero-amount item would put a line saying so on every + // invoice the customer ever gets. Treated exactly like a module that is + // no longer held: if one was ever billed, it comes off. + if ($desired === 0 || (int) $first->price_cents <= 0) { + if ($itemId !== null) { + $this->guardConfigured(); + $this->stripe->removeSubscriptionItem($itemId, StripeClient::PRORATE_NONE); + } + + $this->stamp($group, null, null); + + return; + } + + // Stripe already bills exactly this. Only the bookings are restamped — + // one may have been cancelled while another was booked in the same + // breath, which leaves the quantity right and the rows out of date. + if ($itemId !== null && $known === $desired) { + $this->stamp($group, $itemId, $stamped->first()?->stripe_price_id); + + return; + } + + // Asked for only now that something is going to be said to Stripe: an + // unconfigured account must fail here, where it is parked and retried, + // rather than while resolving a Price nobody is going to use. + $this->guardConfigured(); + + $priceId = $this->prices->ensure( + (string) $first->addon_key, + (int) $first->price_cents, + (string) $first->currency, + (string) $subscription->term, + ); + + if ($priceId === null) { + $this->stamp($group, null, null); + + return; + } + + if ($itemId === null) { + $itemId = $this->stripe->addSubscriptionItem( + (string) $subscription->stripe_subscription_id, + $priceId, + $desired, + StripeClient::PRORATE_IMMEDIATELY, + // Keyed on the contract, the module and how many of it: a retry + // after a timeout that in fact went through replays Stripe's + // first answer instead of adding the module a second time. + idempotencyKey: sprintf( + 'clupilot-addon-item-%d-%s-%d-%d', + $subscription->id, + $first->addon_key, + $first->price_cents, + $desired, + ), + ); + } else { + $this->stripe->updateSubscriptionItemQuantity( + $itemId, + $desired, + // More of a pack is bought now and paid for the days that are + // left; fewer is a cancellation, which earns no credit. + $desired > $known ? StripeClient::PRORATE_IMMEDIATELY : StripeClient::PRORATE_NONE, + ); + } + + $this->stamp($group, $itemId, $priceId); + } + + /** + * Write down what Stripe now bills, on the bookings it bills. + * + * Every running booking in the group carries the item id, so a second pack + * booked tomorrow can see that it is joining an item rather than starting + * one. A booking that has stopped gives its id back, because the quantity + * has already been taken down by it and counting it again would take the + * same pack off twice. + * + * Through the query builder: none of these columns is part of what the + * booking froze, and the model would otherwise have to be re-read first. + * + * @param Collection $group + */ + private function stamp(Collection $group, ?string $itemId, ?string $priceId): void + { + foreach ($group as $addon) { + $billed = $itemId !== null && $addon->cancelled_at === null && $addon->cancels_at === null; + + $values = [ + 'stripe_item_id' => $billed ? $itemId : null, + 'stripe_price_id' => $billed ? $priceId : null, + ]; + + if ($addon->stripe_item_id === $values['stripe_item_id'] + && $addon->stripe_price_id === $values['stripe_price_id']) { + continue; + } + + SubscriptionAddon::query()->whereKey($addon->getKey())->update($values + ['updated_at' => now()]); + + $addon->forceFill($values)->syncOriginal(); + } + } + + private function guardConfigured(): void + { + if (! $this->stripe->isConfigured()) { + throw new RuntimeException('Stripe is not configured (STRIPE_SECRET is empty).'); + } + } + + /** Stripe bills exactly what the contract holds; nothing is outstanding. */ + private function settled(Subscription $subscription): void + { + if ($subscription->stripe_addon_sync === null) { + return; + } + + $subscription->update(['stripe_addon_sync' => null]); + } + + /** + * A module was booked or cancelled here and Stripe was not told. + * + * On the contract rather than only in a log line, for the same reason as + * `stripe_price_sync` beside it: what this describes is a customer holding a + * module nobody is billing them for — or being billed for one they gave up — + * and a log line scrolls away. + */ + private function park(Subscription $subscription, Throwable $e): void + { + $parked = (array) ($subscription->stripe_addon_sync ?? []); + $attempts = (int) ($parked['attempts'] ?? 0) + 1; + + Log::error('A module booking landed but did not reach Stripe: this contract is not billed for what it holds.', [ + 'subscription' => $subscription->uuid, + 'stripe_subscription' => $subscription->stripe_subscription_id, + 'attempts' => $attempts, + 'error' => $e->getMessage(), + ]); + + $subscription->update(['stripe_addon_sync' => [ + 'error' => mb_substr($e->getMessage(), 0, 250), + 'failed_at' => now()->toIso8601String(), + 'attempts' => $attempts, + ]]); + } +} diff --git a/app/Console/Commands/EndCancelledAddons.php b/app/Console/Commands/EndCancelledAddons.php new file mode 100644 index 0000000..e91069a --- /dev/null +++ b/app/Console/Commands/EndCancelledAddons.php @@ -0,0 +1,75 @@ +option('dry-run'); + $ended = 0; + + $due = SubscriptionAddon::query() + ->whereNull('cancelled_at') + ->whereNotNull('cancels_at') + ->where('cancels_at', '<=', now()) + ->with('subscription') + ->orderBy('id') + ->get(); + + foreach ($due as $addon) { + $this->line(sprintf( + ' %s on contract %s', + (string) $addon->addon_key, + (string) ($addon->subscription?->uuid ?? '—'), + )); + + if ($dryRun) { + continue; + } + + try { + $bookAddon->cancel($addon); + $ended++; + } catch (Throwable $e) { + // One customer's module must not stop everybody else's from + // ending. It stays due and the next tick tries again. + $this->warn(" {$addon->uuid}: {$e->getMessage()}"); + } + } + + $this->info($dryRun + ? "{$due->count()} module(s) due to end. Run without --dry-run to end them." + : "{$due->count()} module(s) due, {$ended} ended."); + + return self::SUCCESS; + } +} diff --git a/app/Console/Commands/SyncStripeCatalogue.php b/app/Console/Commands/SyncStripeCatalogue.php index 16efa0e..3d4f8de 100644 --- a/app/Console/Commands/SyncStripeCatalogue.php +++ b/app/Console/Commands/SyncStripeCatalogue.php @@ -5,12 +5,15 @@ namespace App\Console\Commands; use App\Models\PlanFamily; use App\Models\PlanPrice; use App\Models\PlanVersion; +use App\Models\Subscription; +use App\Services\Billing\AddonCatalogue; +use App\Services\Billing\AddonPrices; use App\Services\Stripe\StripeClient; use Illuminate\Console\Command; /** * Mirrors our catalogue into Stripe: a Product per plan family, a Price per - * priced row. + * priced row — and the same for every module we sell. * * Stripe owns the recurring billing — retries, dunning, off-session SCA, * invoice numbering — so it needs to know what it is billing for. It does not @@ -24,6 +27,14 @@ use Illuminate\Console\Command; * Only PUBLISHED versions are synced. A draft has promised nothing, and a * Product for it would be a price list entry for something that may never * exist. + * + * **Modules were missing from this entirely**, which is why a booked module was + * charged in the month it was booked and never again: nothing in Stripe existed + * to put on the subscription. They are pushed at today's catalogue price on both + * terms — a Stripe Price carries its own interval, so a module on a yearly + * contract needs a yearly one. What this run cannot cover is a customer + * grandfathered on an older figure or holding a discounted grant; those mint + * their own Price at the moment they are billed. See App\Services\Billing\AddonPrices. */ class SyncStripeCatalogue extends Command { @@ -109,6 +120,8 @@ class SyncStripeCatalogue extends Command } } + $created += $this->syncModules($dryRun); + $this->newLine(); if ($created === 0) { @@ -124,6 +137,56 @@ class SyncStripeCatalogue extends Command return self::SUCCESS; } + /** + * A Product and two Prices — monthly and yearly — for every module on sale. + * + * Both terms, unconditionally, rather than only the ones somebody has bought + * a contract on: the Price has to exist BEFORE the booking that needs it, and + * a customer on a yearly package booking their first module would otherwise + * discover the gap at the moment their money was due. + * + * @return int how many objects were created, for the run's own count + */ + private function syncModules(bool $dryRun): int + { + $catalogue = app(AddonCatalogue::class); + $prices = app(AddonPrices::class); + $currency = Subscription::catalogueCurrency(); + $created = 0; + + $keys = array_merge(array_keys((array) config('provisioning.addons')), [AddonCatalogue::STORAGE]); + + foreach ($keys as $key) { + $monthly = $catalogue->priceCents($key); + + // Nothing to bill and nothing to mirror. A module priced at zero is + // either a placeholder or included, and an item at no money would + // put a line saying so on every invoice a customer ever gets. + if ($monthly === null || $monthly <= 0) { + continue; + } + + foreach ([Subscription::TERM_MONTHLY => $monthly, Subscription::TERM_YEARLY => $monthly * 12] as $term => $amount) { + $interval = $term === Subscription::TERM_YEARLY ? 'year' : 'month'; + + if ($prices->find($key, $amount, $currency, $interval) !== null) { + continue; + } + + $this->line(sprintf(' module %s %s %d %s', $key, $term, $amount, $currency)); + $created++; + + if ($dryRun) { + continue; + } + + $prices->ensure($key, $monthly, $currency, $term); + } + } + + return $created; + } + /** Versions whose prices are live in Stripe, for the status line. */ public static function syncedVersions(): int { diff --git a/app/Console/Commands/SyncStripeSubscriptions.php b/app/Console/Commands/SyncStripeSubscriptions.php index af958f3..cffe5b0 100644 --- a/app/Console/Commands/SyncStripeSubscriptions.php +++ b/app/Console/Commands/SyncStripeSubscriptions.php @@ -3,11 +3,13 @@ namespace App\Console\Commands; use App\Actions\MoveStripeSubscriptionPrice; +use App\Actions\SyncStripeAddonItems; use App\Models\Subscription; +use App\Models\SubscriptionAddon; use Illuminate\Console\Command; /** - * The sweep behind the plan change. + * The sweep behind the plan change — and behind the module booking. * * Moving Stripe onto the new price at the moment the change lands is right and * it is not enough. Stripe can be unreachable, the key can have been rotated, @@ -19,18 +21,32 @@ use Illuminate\Console\Command; * ApplyPlanChange parks exactly that on the contract. This picks it up, however * long ago it happened. * - * Deliberately hands the work back to the action rather than doing it here: the - * action owns which price, which item and which proration behaviour, and two - * code paths that both move a subscription is one more than the number that can - * be kept in agreement. + * A module booking is the same failure wearing different clothes: the pack is + * already a bigger disk, the entitlement is already switched on, and if Stripe + * never heard about it the customer holds a module nobody is billing them for. + * So it is swept from here too rather than from a second command — one sweep, + * one schedule entry, one place to look when Stripe has been away. + * + * Deliberately hands the work back to the actions rather than doing it here: + * they own which price, which item and which proration behaviour, and two code + * paths that both move a subscription is one more than the number that can be + * kept in agreement. */ class SyncStripeSubscriptions extends Command { protected $signature = 'clupilot:sync-stripe-subscriptions {--limit=100}'; - protected $description = 'Retry the plan changes that landed here but never reached Stripe'; + protected $description = 'Retry the plan changes and module bookings that landed here but never reached Stripe'; - public function handle(MoveStripeSubscriptionPrice $move): int + public function handle(MoveStripeSubscriptionPrice $move, SyncStripeAddonItems $modules): int + { + $status = $this->sweepPlans($move); + $this->sweepModules($modules); + + return $status; + } + + private function sweepPlans(MoveStripeSubscriptionPrice $move): int { // Active contracts only. A cancelled one is not billed again, so moving // it onto another price would be housekeeping on something Stripe has @@ -67,4 +83,55 @@ class SyncStripeSubscriptions extends Command return self::SUCCESS; } + + /** + * Contracts whose modules Stripe is not billing. + * + * Found from the BOOKINGS rather than from a parked marker, because the + * bookings are the authority: a module still running with no item id was + * never put on the subscription, and one that has stopped while still + * carrying an item id is being billed after it ended. `stripe_addon_sync` is + * the record of what went wrong and how often — it is not where the work + * lives, so a contract whose failure was never written down (a worker killed + * mid-booking) is still found here. + */ + private function sweepModules(SyncStripeAddonItems $modules): void + { + $outstanding = SubscriptionAddon::query() + ->where(fn ($q) => $q + ->where(fn ($active) => $active->whereNull('cancelled_at')->whereNull('cancels_at')->whereNull('stripe_item_id')) + ->orWhere(fn ($gone) => $gone->whereNotNull('stripe_item_id') + ->where(fn ($ended) => $ended->whereNotNull('cancelled_at')->orWhereNotNull('cancels_at')))) + ->select('subscription_id') + ->distinct(); + + // Active contracts that Stripe actually bills. A granted contract has no + // Stripe subscription at all and its bookings will never carry an item + // id — sweeping those would retry, and log, something that is not wrong. + $contracts = Subscription::query() + ->whereIn('id', $outstanding) + ->whereNotNull('stripe_subscription_id') + ->where('status', 'active') + ->orderBy('id') + ->limit((int) $this->option('limit')) + ->get(); + + $settled = 0; + + foreach ($contracts as $subscription) { + if ($modules($subscription)) { + $settled++; + + continue; + } + + $this->warn(sprintf( + 'Contract %s: modules not billed by Stripe — %s', + $subscription->uuid, + (string) ($subscription->fresh()?->stripe_addon_sync['error'] ?? 'unknown'), + )); + } + + $this->info("{$contracts->count()} contract(s) with modules out of step, {$settled} settled."); + } } diff --git a/app/Models/StripeAddonPrice.php b/app/Models/StripeAddonPrice.php new file mode 100644 index 0000000..40eaaa8 --- /dev/null +++ b/app/Models/StripeAddonPrice.php @@ -0,0 +1,24 @@ + 'integer']; + } +} diff --git a/app/Models/Subscription.php b/app/Models/Subscription.php index e907f97..87a9100 100644 --- a/app/Models/Subscription.php +++ b/app/Models/Subscription.php @@ -68,6 +68,10 @@ class Subscription extends Model // move to, the proration behaviour its direction chose, the error // and how often it has been tried. Null once Stripe agrees with us. 'stripe_price_sync' => 'array', + // The module items that never reached Stripe: the error, when it + // failed and how often it has been tried. Null once Stripe bills + // exactly the modules this contract holds. + 'stripe_addon_sync' => 'array', 'granted_at' => 'datetime', 'granted_until' => 'datetime', 'catalogue_price_cents' => 'integer', diff --git a/app/Models/SubscriptionAddon.php b/app/Models/SubscriptionAddon.php index fa8f0f6..34eeeae 100644 --- a/app/Models/SubscriptionAddon.php +++ b/app/Models/SubscriptionAddon.php @@ -48,6 +48,11 @@ class SubscriptionAddon extends Model 'quantity' => 'integer', 'booked_at' => 'datetime', 'cancelled_at' => 'datetime', + // The end this booking has an appointment with. A cancellation is + // monthly and lands at the end of the term already paid for, so the + // module keeps running until this date passes — see + // App\Actions\BookAddon::cancelAtPeriodEnd(). + 'cancels_at' => 'datetime', 'granted_at' => 'datetime', 'granted_until' => 'datetime', 'catalogue_price_cents' => 'integer', @@ -95,4 +100,10 @@ class SubscriptionAddon extends Model { return $this->cancelled_at === null; } + + /** Cancelled, still running, and billed for no further term. */ + public function endsAtPeriodEnd(): bool + { + return $this->cancelled_at === null && $this->cancels_at !== null; + } } diff --git a/app/Services/Billing/AddonPrices.php b/app/Services/Billing/AddonPrices.php new file mode 100644 index 0000000..c397151 --- /dev/null +++ b/app/Services/Billing/AddonPrices.php @@ -0,0 +1,147 @@ +find($addonKey, $amount, $currency, $interval); + + if ($existing !== null) { + return $existing; + } + + $productId = $this->product($addonKey); + + $priceId = $this->stripe->createPrice( + productId: $productId, + amountCents: $amount, + currency: $currency, + interval: $interval, + metadata: [ + // Read back when a Stripe invoice line has to be turned into + // wording a customer can read — see StripeInvoiceLines. + 'addon' => $addonKey, + ], + // 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. + idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}", + ); + + $this->remember($addonKey, $amount, $currency, $interval, $productId, $priceId); + + return $priceId; + } + + /** The Price we already have for this combination, or null. */ + public function find(string $addonKey, int $amountCents, string $currency, string $interval): ?string + { + $id = StripeAddonPrice::query() + ->where('addon_key', $addonKey) + ->where('amount_cents', $amountCents) + ->where('currency', strtoupper($currency)) + ->where('interval', $interval) + ->value('stripe_price_id'); + + return is_string($id) && $id !== '' ? $id : null; + } + + /** + * The Stripe Product every Price for this module hangs off. + * + * One per module, however many Prices it accumulates — that is what a + * Product is for, and it is what makes the module readable in Stripe's own + * dashboard rather than a list of anonymous amounts. + */ + private function product(string $addonKey): string + { + $existing = StripeAddonPrice::query() + ->where('addon_key', $addonKey) + ->value('stripe_product_id'); + + if (is_string($existing) && $existing !== '') { + return $existing; + } + + return $this->stripe->createProduct( + app(AddonCatalogue::class)->name($addonKey), + ['addon' => $addonKey], + idempotencyKey: "clupilot-addon-product-{$addonKey}", + ); + } + + private function remember( + string $addonKey, + int $amountCents, + string $currency, + string $interval, + string $productId, + string $priceId, + ): void { + try { + StripeAddonPrice::create([ + 'addon_key' => $addonKey, + 'amount_cents' => $amountCents, + 'currency' => $currency, + 'interval' => $interval, + 'stripe_product_id' => $productId, + 'stripe_price_id' => $priceId, + ]); + } 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. + } + } +} diff --git a/app/Services/Billing/IssueInvoice.php b/app/Services/Billing/IssueInvoice.php index f78e88c..8d8b36b 100644 --- a/app/Services/Billing/IssueInvoice.php +++ b/app/Services/Billing/IssueInvoice.php @@ -91,11 +91,13 @@ final class IssueInvoice * records beside it what was actually taken, and `stripe_invoice_id` on the * row is how the two are reconciled afterwards. * - * Booked modules are NOT on it. They are not items on the Stripe - * subscription, so this renewal did not charge for them — putting them on - * the document would invoice money nobody was asked for. (That they are - * never billed after the month they were booked in is a real gap, and it is - * a gap in the booking, not in this document.) + * **The fallback, not the ordinary path.** Modules are items on the Stripe + * subscription now, so what a cycle actually charged for is on Stripe's own + * invoice — package line, every module line, any proration — and + * forStripeInvoice() below builds the document from those. This remains for + * the cycle invoice that arrives carrying no line detail at all, where the + * contract's own frozen price is the honest description of the term that was + * billed. Better a document from the contract than none. * * `$stripeInvoiceId` is what makes issuing this idempotent: the column is * unique, so a redelivered webhook collides in the database and the @@ -147,6 +149,72 @@ final class IssueInvoice ); } + /** + * One document for one paid Stripe invoice, carrying every line Stripe + * charged for. + * + * This is the ordinary path now, and it replaces a rule that had stopped + * being true. A cycle invoice used to be one line at the contract's frozen + * price, because that was all Stripe billed; modules are items on the + * subscription today, so a cycle carries the package AND every module still + * running, a mid-period booking carries that module's prorated line on its + * own, and an upgrade carries its proration. Each of those is money that was + * taken, so each of them owes the customer a document — and the owner's + * rule is one document per charge, with the package and the modules of one + * cycle together on a single one. + * + * **Built from Stripe's lines, not from our snapshot.** Stripe is what + * actually charged the customer: it worked the pro-rata out against its own + * period boundaries and its own view of what is already on the account, and + * a document assembled from the contract would state a sum that is not the + * one taken. Only the WORDING is ours — see StripeInvoiceLines, which names + * each line from the catalogue instead of printing a Price id. + * + * **Net in, tax on top.** Our catalogue is pushed to Stripe as NET, so the + * line amounts that come back are net and the document adds VAT from the + * customer's own treatment. Nothing here reads `amount_paid`, which is a + * gross total and belongs to the proof register. + * + * @param array> $lines Stripe's `lines.data` + * @param array $unnamed filled with the lines the catalogue could not name + */ + public function forStripeInvoice( + Subscription $subscription, + string $stripeInvoiceId, + array $lines, + ?string $currency = null, + ?InvoiceSeries $series = null, + array &$unnamed = [], + ): Invoice { + $customer = $subscription->customer; + + if ($customer === null) { + throw new RuntimeException('Refusing to issue an invoice for a contract with nobody on it.'); + } + + $built = app(StripeInvoiceLines::class)->build($lines); + $unnamed = $built['unnamed']; + + if ($built['lines'] === []) { + throw new RuntimeException('Refusing to issue an invoice with no lines on it.'); + } + + return $this->issue( + customer: $customer, + lines: $built['lines'], + currency: strtoupper((string) ($currency ?: $subscription->currency ?: 'EUR')), + series: $series, + attributes: [ + 'subscription_id' => $subscription->id, + // Left null for the same reason as on a renewal: the contract's + // opening order is the first purchase and already has its own + // document. + 'order_id' => null, + 'stripe_invoice_id' => $stripeInvoiceId, + ], + ); + } + /** * The part every document has in common: the number, the freeze, the copy * to the archive. diff --git a/app/Services/Billing/StripeInvoiceLines.php b/app/Services/Billing/StripeInvoiceLines.php new file mode 100644 index 0000000..a20b4da --- /dev/null +++ b/app/Services/Billing/StripeInvoiceLines.php @@ -0,0 +1,183 @@ +> $lines Stripe's `lines.data` + * @return array{lines: array>, unnamed: array} + */ + public function build(array $lines): array + { + $documentLines = []; + $unnamed = []; + + foreach ($lines as $line) { + $amount = (int) ($line['amount'] ?? 0); + $quantity = max(1, (int) ($line['quantity'] ?? 1)); + $priceId = $this->priceId($line); + $description = $this->name($priceId, $line); + + if ($description === null) { + $unnamed[] = $priceId ?? (string) ($line['id'] ?? 'unknown'); + $description = $this->fallbackName($line); + } + + // The unit price only when it divides cleanly. Stripe's line amount + // is what was charged, and it is the figure the document has to + // total to — so a quantity that does not divide it exactly is shown + // as one line at the full amount rather than as a unit price the + // reader could multiply and get a different answer. + $unit = intdiv($amount, $quantity); + $divides = $unit * $quantity === $amount; + + $documentLines[] = [ + 'description' => $description, + 'details' => $this->details($line), + 'quantity_milli' => $divides ? $quantity * 1000 : 1000, + 'unit' => '', + 'unit_net_cents' => $divides ? $unit : $amount, + ]; + } + + return ['lines' => $documentLines, 'unnamed' => $unnamed]; + } + + /** + * The Stripe Price a line billed against. + * + * Two shapes, because Stripe moved it: older invoice lines carry `price` + * inline, newer ones put it under `pricing.price_details`. Both are read + * rather than one, so an account on either API version is named correctly. + */ + private function priceId(array $line): ?string + { + $id = $line['price']['id'] + ?? $line['pricing']['price_details']['price'] + ?? null; + + return is_string($id) && $id !== '' ? $id : null; + } + + /** + * What this line is, in the words the customer knows it by. Null when the + * catalogue has never heard of it. + */ + private function name(?string $priceId, array $line): ?string + { + $metadata = (array) ($line['price']['metadata'] ?? []); + + // A module, by the Price we minted for it. The metadata is the same + // answer from the other direction and covers a Price created against + // this account before the table existed. + $addonKey = $priceId === null + ? null + : StripeAddonPrice::query()->where('stripe_price_id', $priceId)->value('addon_key'); + + $addonKey ??= is_string($metadata['addon'] ?? null) ? $metadata['addon'] : null; + + if (is_string($addonKey) && app(AddonCatalogue::class)->knows($addonKey)) { + return app(AddonCatalogue::class)->name($addonKey); + } + + // The package, by the priced row stripe:sync-catalogue pushed. Named by + // FAMILY rather than by the version number: "Paket Team" is what the + // customer bought, and the version is a fact about our catalogue. + $family = $priceId === null ? null : PlanPrice::query() + ->where('stripe_price_id', $priceId) + ->with('version.family') + ->first()?->version?->family; + + $family ??= is_string($metadata['plan_family'] ?? null) + ? PlanFamily::query()->where('key', $metadata['plan_family'])->first() + : null; + + return $family === null ? null : __('billing.cart.plan', ['plan' => $family->name]); + } + + /** + * The best we can say about a line the catalogue does not know. + * + * Stripe's own description if it sent one — it is at least true — and + * otherwise a sentence saying plainly that this is a charge from the payment + * provider. Never nothing: the line stays on the document either way. + */ + private function fallbackName(array $line): string + { + $description = $line['description'] ?? null; + + return is_string($description) && trim($description) !== '' + ? trim($description) + : __('invoice.line_unnamed'); + } + + /** + * The small print under a line: which term it covers, and whether it was + * worked out pro rata. + * + * @return array + */ + private function details(array $line): array + { + $details = []; + + $from = $line['period']['start'] ?? null; + $to = $line['period']['end'] ?? null; + + if (is_numeric($from) && is_numeric($to)) { + $start = Carbon::createFromTimestamp((int) $from); + $end = Carbon::createFromTimestamp((int) $to); + + $details[] = __('invoice.line_period', [ + 'from' => $start->local()->format('d.m.Y'), + // Stripe's period end is the EXCLUSIVE boundary — the first + // moment of the next term, which the next invoice claims in + // turn. Printed as it comes, the same day would appear on two + // consecutive documents. + 'to' => $end->greaterThan($start) + ? $end->copy()->subDay()->local()->format('d.m.Y') + : $end->local()->format('d.m.Y'), + ]); + } + + // Said out loud, because a customer looking at a figure that is not the + // monthly price they know has to be able to see why. + if (($line['proration'] ?? false) === true) { + $details[] = __('invoice.line_prorated'); + } + + return $details; + } +} diff --git a/app/Services/Stripe/FakeStripeClient.php b/app/Services/Stripe/FakeStripeClient.php index b2496c1..cc9a047 100644 --- a/app/Services/Stripe/FakeStripeClient.php +++ b/app/Services/Stripe/FakeStripeClient.php @@ -44,6 +44,31 @@ class FakeStripeClient implements StripeClient */ public array $priceChanges = []; + /** + * The items on each subscription as this Stripe sees them: item id → what + * it bills. Modules are added and removed here, so a test can assert how + * many items a contract carries and at which quantity. + * + * @var array + */ + public array $subscriptionItems = []; + + /** + * Every item call that was made, in order, so a test can assert the + * proration behaviour as well as the outcome. + * + * @var array> + */ + public array $itemCalls = []; + + /** + * The lines this Stripe would report for an invoice: invoice id → lines. + * Only for the invoice whose lines an event did not carry in full. + * + * @var array>> + */ + public array $invoiceLines = []; + /** * Set to make every call fail, the way an outage, a revoked key or a * network without a route out does. @@ -156,6 +181,83 @@ class FakeStripeClient implements StripeClient return $this->items[$subscriptionId] ?? null; } + public function addSubscriptionItem( + string $subscriptionId, + string $priceId, + int $quantity, + string $prorationBehaviour, + ?string $idempotencyKey = null, + ): string { + $this->failIfAsked(); + + $id = 'si_'.substr(sha1($subscriptionId.$priceId.count($this->subscriptionItems)), 0, 12); + + $this->subscriptionItems[$id] = [ + 'subscription' => $subscriptionId, + 'price' => $priceId, + 'quantity' => $quantity, + ]; + + $this->itemCalls[] = [ + 'call' => 'add', + 'subscription' => $subscriptionId, + 'item' => $id, + 'price' => $priceId, + 'quantity' => $quantity, + 'proration' => $prorationBehaviour, + ]; + + return $id; + } + + public function updateSubscriptionItemQuantity( + string $itemId, + int $quantity, + string $prorationBehaviour, + ): void { + $this->failIfAsked(); + + if (isset($this->subscriptionItems[$itemId])) { + $this->subscriptionItems[$itemId]['quantity'] = $quantity; + } + + $this->itemCalls[] = [ + 'call' => 'quantity', + 'item' => $itemId, + 'quantity' => $quantity, + 'proration' => $prorationBehaviour, + ]; + } + + public function removeSubscriptionItem(string $itemId, string $prorationBehaviour): void + { + $this->failIfAsked(); + + unset($this->subscriptionItems[$itemId]); + + $this->itemCalls[] = [ + 'call' => 'remove', + 'item' => $itemId, + 'proration' => $prorationBehaviour, + ]; + } + + public function invoiceLines(string $invoiceId): array + { + $this->failIfAsked(); + + return $this->invoiceLines[$invoiceId] ?? []; + } + + /** The module items this Stripe holds for one subscription. */ + public function itemsOn(string $subscriptionId): array + { + return array_filter( + $this->subscriptionItems, + fn (array $item) => $item['subscription'] === $subscriptionId, + ); + } + private function failIfAsked(): void { if ($this->failWith !== null) { diff --git a/app/Services/Stripe/HttpStripeClient.php b/app/Services/Stripe/HttpStripeClient.php index 4a95f96..588a091 100644 --- a/app/Services/Stripe/HttpStripeClient.php +++ b/app/Services/Stripe/HttpStripeClient.php @@ -125,6 +125,77 @@ class HttpStripeClient implements StripeClient return is_string($id) && $id !== '' ? $id : null; } + public function addSubscriptionItem( + string $subscriptionId, + string $priceId, + int $quantity, + string $prorationBehaviour, + ?string $idempotencyKey = null, + ): string { + // Straight onto the item collection rather than through the + // subscription: posting items[] to the subscription REPLACES the set, + // so an item added that way takes the package's off with it. + return (string) $this->request($idempotencyKey) + ->asForm() + ->post($this->url('subscription_items'), [ + 'subscription' => $subscriptionId, + 'price' => $priceId, + 'quantity' => $quantity, + 'proration_behavior' => $prorationBehaviour, + ]) + ->throw() + ->json('id'); + } + + public function updateSubscriptionItemQuantity( + string $itemId, + int $quantity, + string $prorationBehaviour, + ): void { + $this->request() + ->asForm() + ->post($this->url('subscription_items/'.$itemId), [ + 'quantity' => $quantity, + 'proration_behavior' => $prorationBehaviour, + ]) + ->throw(); + } + + public function removeSubscriptionItem(string $itemId, string $prorationBehaviour): void + { + $this->request() + ->asForm() + ->delete($this->url('subscription_items/'.$itemId), [ + 'proration_behavior' => $prorationBehaviour, + ]) + ->throw(); + } + + public function invoiceLines(string $invoiceId): array + { + $lines = []; + $after = null; + + // Paged through to the end. Stripe caps a page at a hundred, and the + // point of asking at all is to get the lines the event did not carry — + // stopping at the first page would reintroduce exactly that gap. + do { + $page = $this->request() + ->get($this->url('invoices/'.$invoiceId.'/lines'), array_filter([ + 'limit' => 100, + 'starting_after' => $after, + ])) + ->throw() + ->json(); + + $data = (array) ($page['data'] ?? []); + $lines = [...$lines, ...$data]; + $after = $data === [] ? null : ($data[array_key_last($data)]['id'] ?? null); + } while (($page['has_more'] ?? false) === true && $after !== null); + + return $lines; + } + private function request(?string $idempotencyKey = null): PendingRequest { $secret = (string) $this->secret(); diff --git a/app/Services/Stripe/StripeClient.php b/app/Services/Stripe/StripeClient.php index a54967f..6eef0f8 100644 --- a/app/Services/Stripe/StripeClient.php +++ b/app/Services/Stripe/StripeClient.php @@ -97,13 +97,67 @@ interface StripeClient ): void; /** - * The id of the single item on a subscription, or null if it has none. + * The id of the item a subscription bills its PACKAGE through, or null if it + * has none. * * For contracts opened before the item id was stored: Stripe knows it, we * did not ask, and asking once is cheaper than being unable to move them. - * One item, because one contract bills one package — a subscription with - * several would need a rule for which of them the package is, and there is - * no such thing to build one from. + * The first item is the package, because that is the one the checkout + * created; modules are added afterwards and carry their own ids, which are + * stored on the bookings themselves. */ public function subscriptionItemId(string $subscriptionId): ?string; + + /** + * Put a module on the subscription as an item of its own, and return the + * item's id. + * + * `$prorationBehaviour` is passed explicitly for the same reason as above. + * A module booked in the middle of a term is charged PRORATE_IMMEDIATELY: + * Stripe works out the days that are left, raises an invoice for exactly + * that and takes the money there and then — which is the invoice the + * customer is owed a document for. + */ + public function addSubscriptionItem( + string $subscriptionId, + string $priceId, + int $quantity, + string $prorationBehaviour, + ?string $idempotencyKey = null, + ): string; + + /** + * Change how many of a module the subscription bills for. + * + * A pack is sold by the unit, so a second one is a quantity of two on one + * item rather than a second item at the same price — two items on one price + * is a thing Stripe allows and nothing could later tell apart. + */ + public function updateSubscriptionItemQuantity( + string $itemId, + int $quantity, + string $prorationBehaviour, + ): void; + + /** + * Take a module off the subscription. + * + * Removed with PRORATE_NONE by everything that calls it: a cancelled module + * is kept until the end of the term the customer has already paid for and + * earns no credit, so nothing is to be settled — only the next cycle is + * smaller. + */ + public function removeSubscriptionItem(string $itemId, string $prorationBehaviour): void; + + /** + * Every line on a Stripe invoice, for the document we owe it. + * + * The webhook payload carries the lines already; this is for the invoice + * that has more of them than Stripe puts in an event. A document missing a + * line that was charged is worse than one nobody sent, so the complete list + * is worth an extra call. + * + * @return array> + */ + public function invoiceLines(string $invoiceId): array; } diff --git a/database/migrations/2026_07_29_290000_bill_modules_through_stripe.php b/database/migrations/2026_07_29_290000_bill_modules_through_stripe.php new file mode 100644 index 0000000..99cfadb --- /dev/null +++ b/database/migrations/2026_07_29_290000_bill_modules_through_stripe.php @@ -0,0 +1,101 @@ +id(); + $table->string('addon_key'); + $table->unsignedInteger('amount_cents'); + $table->char('currency', 3); + $table->string('interval'); + $table->string('stripe_product_id'); + $table->string('stripe_price_id'); + $table->timestamps(); + + // The identity of a Price. Two rows for one combination would leave + // two live Stripe Prices for the same module at the same money, and + // no way to tell which a customer is billed on. + $table->unique(['addon_key', 'amount_cents', 'currency', 'interval'], 'stripe_addon_prices_unique'); + }); + + Schema::table('subscription_addons', function (Blueprint $table) { + // The subscription item this booking bills through. Several + // bookings of one module at one price share a single item with a + // quantity — that is what "sold in packs" means to Stripe — so the + // id is stamped on every row of the group. + $table->string('stripe_item_id')->nullable()->after('quantity'); + $table->string('stripe_price_id')->nullable()->after('stripe_item_id'); + + // The appointment a cancellation makes, separate from + // `cancelled_at`, which says the module has actually stopped. A + // cancellation is monthly and takes effect at the end of the term + // the customer has already paid for: until this date passes they + // keep the module, and afterwards clupilot:end-cancelled-addons + // sets `cancelled_at` and the entitlement goes. + $table->timestamp('cancels_at')->nullable()->after('cancelled_at'); + + $table->index('stripe_item_id'); + // Read by the sweep that ends modules whose term has run out. + $table->index(['cancels_at', 'cancelled_at']); + }); + + Schema::table('subscriptions', function (Blueprint $table) { + // The module items that never reached Stripe, in the same shape and + // for the same reason as `stripe_price_sync` beside it: a booking is + // delivered to the machine before Stripe is told, and it is not + // rolled back when Stripe is away, so what is left is a customer + // holding a module nobody is billing them for. + // + // Its own column rather than sharing that one: a plan swap and a + // module item are two different outstanding pieces of work, and + // clearing one on success would silently drop the other. + $table->json('stripe_addon_sync')->nullable()->after('stripe_price_sync'); + }); + } + + public function down(): void + { + Schema::table('subscriptions', function (Blueprint $table) { + $table->dropColumn('stripe_addon_sync'); + }); + + Schema::table('subscription_addons', function (Blueprint $table) { + $table->dropIndex(['cancels_at', 'cancelled_at']); + $table->dropIndex(['stripe_item_id']); + $table->dropColumn(['stripe_item_id', 'stripe_price_id', 'cancels_at']); + }); + + Schema::dropIfExists('stripe_addon_prices'); + } +}; diff --git a/lang/de/invoice.php b/lang/de/invoice.php index c4a61cd..9b37239 100644 --- a/lang/de/invoice.php +++ b/lang/de/invoice.php @@ -28,6 +28,12 @@ return [ 'line_recurring' => 'monatlich', 'line_once' => 'einmalig', 'line_period' => 'Leistungszeitraum :from – :to', + // Warum eine Zeile nicht den Monatspreis zeigt, den der Kunde kennt: ein + // Modul, das mitten im Zeitraum gebucht wurde, wird tageweise berechnet. + 'line_prorated' => 'anteilig für den Restzeitraum berechnet', + // Für eine Position, die der Katalog nicht benennen kann. Sie bleibt auf der + // Rechnung — eine fehlende Position wäre schlimmer als eine unschöne. + 'line_unnamed' => 'Leistung laut Abrechnung des Zahlungsanbieters', 'salutation' => 'Sehr geehrte Damen und Herren,', 'intro' => 'wir erlauben uns, für die im Folgenden aufgeführten Leistungen in Rechnung zu stellen.', 'payment_default' => 'Zahlbar ohne Abzug innerhalb von :days Tagen auf das unten angeführte Konto.', diff --git a/lang/en/invoice.php b/lang/en/invoice.php index 161e179..5aaaa12 100644 --- a/lang/en/invoice.php +++ b/lang/en/invoice.php @@ -28,6 +28,8 @@ return [ 'line_recurring' => 'monthly', 'line_once' => 'one-off', 'line_period' => 'Service period :from – :to', + 'line_prorated' => 'charged pro rata for the remainder of the period', + 'line_unnamed' => 'Service as billed by the payment provider', 'salutation' => 'Dear Sir or Madam,', 'intro' => 'we are pleased to invoice the services listed below.', 'payment_default' => 'Payable in full within :days days to the account shown below.', diff --git a/routes/console.php b/routes/console.php index 92d5e7d..bef91a1 100644 --- a/routes/console.php +++ b/routes/console.php @@ -134,6 +134,20 @@ Schedule::command('clupilot:apply-due-plan-changes') ->everyFifteenMinutes() ->withoutOverlapping(); +// End the modules whose cancellation date has arrived. +// +// The same rule the downgrade above follows: a module cancelled on the tenth is +// kept until the term it was paid for runs out. Stripe stopped billing it at the +// moment of cancellation — its item is only ever about the NEXT cycle — so what +// waits for this sweep is the module itself: the entitlement, the space on the +// disk, and the entry in the register saying it ended. +// +// Beside the downgrades and at the same cadence, because it is the same +// appointment being kept. +Schedule::command('clupilot:end-cancelled-addons') + ->everyFifteenMinutes() + ->withoutOverlapping(); + // Keep the appointment a cancellation made. // // ConfirmCancelPackage writes a date and nothing ever went back to it, which is diff --git a/tests/Feature/Billing/RenewalInvoiceTest.php b/tests/Feature/Billing/RenewalInvoiceTest.php index c7fb4ee..8d7a326 100644 --- a/tests/Feature/Billing/RenewalInvoiceTest.php +++ b/tests/Feature/Billing/RenewalInvoiceTest.php @@ -175,7 +175,7 @@ it('sends the invoice a second delivery finds unsent, without issuing a second d Mail::assertQueuedCount(1); }); -it('invoices nothing for a proration or a charge raised by hand', function () { +it('invoices nothing for a payment that arrives with nothing to describe it', function () { Mail::fake(); renewingContract(); @@ -188,9 +188,13 @@ it('invoices nothing for a proration or a charge raised by hand', function () { ]], ])->assertOk(); - // Stripe worked that amount out against ITS boundaries and its own view of - // the account. A document written from our contract snapshot would state a - // sum that is not the one charged. The money is in the register either way. + // A proration DOES get a document now — StripeAddonBillingTest walks one — + // and it is built from Stripe's own lines, because Stripe worked the amount + // out against ITS boundaries and its own view of the account. This invoice + // carries no lines at all, so there is nothing honest to put on a document: + // the contract's own price describes a whole term, which is exactly what + // this charge is not. The money is in the register either way, which is the + // trail for whoever has to raise the paper. expect(Invoice::query()->count())->toBe(0) ->and(SubscriptionRecord::query()->where('event', SubscriptionRecord::EVENT_INVOICE_PAID)->sole()->gross_cents) ->toBe(4200); diff --git a/tests/Feature/Billing/StripeAddonBillingTest.php b/tests/Feature/Billing/StripeAddonBillingTest.php new file mode 100644 index 0000000..87eec92 --- /dev/null +++ b/tests/Feature/Billing/StripeAddonBillingTest.php @@ -0,0 +1,490 @@ + 'CluPilot Cloud e.U.', + 'address' => 'Dreherstraße 66/1/8', + 'postcode' => '1110', + 'city' => 'Wien', + 'vat_id' => 'ATU00000000', + ]); +}); + +afterEach(function () { + Carbon::setTestNow(); +}); + +/** A Stripe that answers, with the catalogue — packages and modules — mirrored into it. */ +function addonStripe(): FakeStripeClient +{ + $fake = new FakeStripeClient; + app()->instance(StripeClient::class, $fake); + + app(Kernel::class)->call('stripe:sync-catalogue'); + + return $fake; +} + +/** A paying contract with somebody on it to send a document to. */ +function addonContract(string $plan = 'team'): Subscription +{ + $customer = Customer::factory()->create([ + 'name' => 'Berger GmbH', + 'email' => 'kundin@example.com', + ]); + + return Subscription::factory()->plan($plan)->create([ + 'customer_id' => $customer->id, + 'stripe_subscription_id' => 'sub_mod', + 'stripe_item_id' => 'si_plan', + 'current_period_start' => now(), + 'current_period_end' => now()->addMonth(), + ]); +} + +/** The Stripe Price a package is sold at on this term. */ +function addonPlanPrice(Subscription $contract): string +{ + return (string) PlanPrice::query() + ->where('plan_version_id', $contract->plan_version_id) + ->where('term', $contract->term) + ->value('stripe_price_id'); +} + +/** The Stripe Price a module is sold at, monthly, at today's catalogue figure. */ +function addonModulePrice(string $key, int $monthlyCents): string +{ + return (string) app(AddonPrices::class)->find($key, $monthlyCents, 'EUR', 'month'); +} + +/** + * One line as Stripe puts it on an invoice. + * + * @return array + */ +function stripeLine(string $priceId, int $amount, Carbon $from, Carbon $to, int $quantity = 1, bool $proration = false): array +{ + return [ + 'id' => 'il_'.substr(sha1($priceId.$amount.$from->timestamp), 0, 8), + 'amount' => $amount, + 'quantity' => $quantity, + 'proration' => $proration, + 'period' => ['start' => $from->timestamp, 'end' => $to->timestamp], + 'price' => ['id' => $priceId, 'metadata' => []], + 'description' => 'Stripe’s own wording, which no customer should have to read', + ]; +} + +/** + * The `invoice.paid` event, carrying the lines Stripe charged for. + * + * @param array> $lines + * @return array + */ +function paidInvoice(string $id, array $lines, string $reason, Carbon $from, Carbon $to): array +{ + $net = array_sum(array_column($lines, 'amount')); + + return [ + 'id' => 'evt_'.$id, 'type' => 'invoice.paid', + 'data' => ['object' => [ + 'id' => $id, + 'subscription' => 'sub_mod', + 'billing_reason' => $reason, + 'currency' => 'eur', + // Our catalogue is pushed to Stripe as net, so what is taken is that + // plus the tax we work out — the document must total to it. + 'amount_paid' => (int) round($net * 1.2), + 'total' => (int) round($net * 1.2), + 'period_start' => $from->timestamp, + 'period_end' => $to->timestamp, + 'lines' => ['data' => $lines, 'has_more' => false], + ]], + ]; +} + +it('gives the owner’s example four invoices, and puts package and module on the fourth', function () { + Mail::fake(); + Queue::fake(); + + $stripe = addonStripe(); + $contract = addonContract(); + $planPrice = addonPlanPrice($contract); + + $month1 = now()->copy(); + $month2 = $month1->copy()->addMonth(); + $month3 = $month2->copy()->addMonth(); + $month4 = $month3->copy()->addMonth(); + + // Month one and month two: the package on its own, one document each. + $this->postJson(route('webhooks.stripe'), paidInvoice( + 'in_m1', [stripeLine($planPrice, 17900, $month1, $month2)], 'subscription_cycle', $month1, $month2, + ))->assertOk(); + + Carbon::setTestNow($month2); + $this->postJson(route('webhooks.stripe'), paidInvoice( + 'in_m2', [stripeLine($planPrice, 17900, $month2, $month3)], 'subscription_cycle', $month2, $month3, + ))->assertOk(); + + expect(Invoice::query()->count())->toBe(2); + + // The middle of month three: the customer books a storage pack. The item + // goes onto the subscription and Stripe settles it there and then — + // always_invoice, which prorates by days AND charges immediately, so the + // booking produces its own invoice rather than a credit line a month later. + Carbon::setTestNow($month3->copy()->addDays(14)); + Subscription::query()->whereKey($contract->id)->update([ + 'current_period_start' => $month3, 'current_period_end' => $month4, + ]); + + app(BookAddon::class)($contract->fresh(), 'storage', 1); + + expect($stripe->itemCalls)->toHaveCount(1) + ->and($stripe->itemCalls[0]['call'])->toBe('add') + ->and($stripe->itemCalls[0]['proration'])->toBe(StripeClient::PRORATE_IMMEDIATELY) + ->and($stripe->itemCalls[0]['price'])->toBe(addonModulePrice('storage', 1000)); + + // Which is the invoice Stripe then raises: half a month of the module, on + // its own, in the middle of a term. The owner's fourth document. + $storagePrice = addonModulePrice('storage', 1000); + $this->postJson(route('webhooks.stripe'), paidInvoice( + 'in_m3_storage', + [stripeLine($storagePrice, 484, now(), $month4, proration: true)], + 'subscription_update', + now(), + $month4, + ))->assertOk(); + + // From month four the package and the module are charged together, so they + // are ONE document — which is exactly what the owner asked for, and what + // separate invoices per line would not have been. + Carbon::setTestNow($month4); + $this->postJson(route('webhooks.stripe'), paidInvoice( + 'in_m4', + [ + stripeLine($planPrice, 17900, $month4, $month4->copy()->addMonth()), + stripeLine($storagePrice, 1000, $month4, $month4->copy()->addMonth()), + ], + 'subscription_cycle', + $month4, + $month4->copy()->addMonth(), + ))->assertOk(); + + $invoices = Invoice::query()->orderBy('id')->get(); + + // Four documents, four numbers, in one unbroken series. + expect($invoices)->toHaveCount(4) + ->and($invoices->pluck('number')->all())->toBe([ + 'RE-2026-0001', 'RE-2026-0002', 'RE-2026-0003', 'RE-2026-0004', + ]); + + // The first three carry one line each: package, package, module. + expect($invoices[0]->snapshot['lines'])->toHaveCount(1) + ->and($invoices[1]->snapshot['lines'])->toHaveCount(1) + ->and($invoices[2]->snapshot['lines'])->toHaveCount(1) + ->and($invoices[2]->snapshot['lines'][0]['description'])->toBe(__('billing.storage_title')) + // Prorated by days, and it says so — a figure that is not the monthly + // price the customer knows has to explain itself. + ->and($invoices[2]->net_cents)->toBe(484) + ->and($invoices[2]->snapshot['lines'][0]['details'])->toContain(__('invoice.line_prorated')); + + // And the fourth carries both, on one document with one number. + $fourth = $invoices[3]; + + expect($fourth->snapshot['lines'])->toHaveCount(2) + ->and(array_column($fourth->snapshot['lines'], 'description')) + ->toBe([__('billing.cart.plan', ['plan' => 'Team']), __('billing.storage_title')]) + // Net from Stripe's own lines, tax added by us: the catalogue is pushed + // as net and the document is what adds VAT. + ->and($fourth->net_cents)->toBe(18900) + ->and($fourth->tax_cents)->toBe(3780) + ->and($fourth->gross_cents)->toBe(22680); + + // One mail per document, never two for one. + Mail::assertQueuedCount(4); +}); + +it('issues one document and consumes one number however often Stripe redelivers a module invoice', function () { + Mail::fake(); + Queue::fake(); + + addonStripe(); + $contract = addonContract(); + + $event = paidInvoice( + 'in_twice', + [stripeLine(addonPlanPrice($contract), 17900, now(), now()->addMonth())], + 'subscription_cycle', + now(), + now()->addMonth(), + ); + + $this->postJson(route('webhooks.stripe'), $event)->assertOk(); + $this->postJson(route('webhooks.stripe'), $event)->assertOk(); + + // An invoice number comes out of a gapless series and can never be handed + // out twice — so the second delivery must not take one, not even one it + // then throws away. + expect(Invoice::query()->count())->toBe(1) + ->and(InvoiceSeries::query()->where('kind', 'invoice')->value('next_number'))->toBe(2); + + Mail::assertQueuedCount(1); +}); + +it('keeps a cancelled module until the period end and leaves it off the next cycle invoice', function () { + Mail::fake(); + Queue::fake(); + + $stripe = addonStripe(); + $contract = addonContract(); + + $addon = app(BookAddon::class)($contract, 'storage', 1); + $item = $addon->fresh()->stripe_item_id; + + expect($item)->not->toBeNull() + ->and($stripe->itemsOn('sub_mod'))->toHaveCount(1); + + // Cancelled in the middle of the term the customer has already paid for. + Carbon::setTestNow(now()->addDays(10)); + app(BookAddon::class)->cancelAtPeriodEnd($addon->fresh()); + + $addon->refresh(); + + // They keep it: the booking is still running, with an appointment for the + // end of the term. Nothing was credited — the item simply came off, so the + // NEXT cycle is smaller and this one is untouched. + expect($addon->cancelled_at)->toBeNull() + ->and($addon->cancels_at->timestamp)->toBe($contract->current_period_end->timestamp) + ->and($stripe->itemsOn('sub_mod'))->toHaveCount(0) + ->and(collect($stripe->itemCalls)->last()) + ->toBe(['call' => 'remove', 'item' => $item, 'proration' => StripeClient::PRORATE_NONE]); + + // The cycle turns. Stripe bills the package alone, and the appointment is + // kept: the module stops being held as well as being billed. + Carbon::setTestNow($contract->current_period_end->copy()->addMinute()); + $this->artisan('clupilot:end-cancelled-addons')->assertSuccessful(); + + expect($addon->fresh()->cancelled_at)->not->toBeNull(); + + $this->postJson(route('webhooks.stripe'), paidInvoice( + 'in_after_cancel', + [stripeLine(addonPlanPrice($contract), 17900, now(), now()->addMonth())], + 'subscription_cycle', + now(), + now()->addMonth(), + ))->assertOk(); + + $invoice = Invoice::query()->latest('id')->sole(); + + expect($invoice->snapshot['lines'])->toHaveCount(1) + ->and($invoice->net_cents)->toBe(17900); +}); + +it('books a second storage pack as a quantity rather than a second item', function () { + Mail::fake(); + Queue::fake(); + + $stripe = addonStripe(); + $contract = addonContract(); + + app(BookAddon::class)($contract, 'storage', 1); + app(BookAddon::class)($contract->fresh(), 'storage', 1); + + // Two packs are twice as much storage on one item, not two items at the + // same price — Stripe allows the latter and nothing could ever tell the two + // apart afterwards. + $items = $stripe->itemsOn('sub_mod'); + + expect($items)->toHaveCount(1) + ->and(reset($items)['quantity'])->toBe(2) + ->and(collect($stripe->itemCalls)->last()['call'])->toBe('quantity') + // More of a pack is had now and paid for the days that are left. + ->and(collect($stripe->itemCalls)->last()['proration'])->toBe(StripeClient::PRORATE_IMMEDIATELY); + + // Both bookings point at the one item, so a third would join it too. + expect(SubscriptionAddon::query()->pluck('stripe_item_id')->unique())->toHaveCount(1); + + // And the document says two, at the unit price, so the reader can multiply. + $this->postJson(route('webhooks.stripe'), paidInvoice( + 'in_packs', + [ + stripeLine(addonPlanPrice($contract), 17900, now(), now()->addMonth()), + stripeLine(addonModulePrice('storage', 1000), 2000, now(), now()->addMonth(), quantity: 2), + ], + 'subscription_cycle', + now(), + now()->addMonth(), + ))->assertOk(); + + $storage = collect(Invoice::query()->sole()->snapshot['lines']) + ->firstWhere('description', __('billing.storage_title')); + + expect($storage['quantity_milli'])->toBe(2000) + ->and($storage['unit_net_cents'])->toBe(1000); +}); + +it('books a module onto a granted contract without asking Stripe anything', function () { + fakeServices(); + Queue::fake(); + + $stripe = addonStripe(); + + // Any call at all would throw and be parked as a failure, so a clean + // contract afterwards is proof that none was made. + $stripe->failWith = 'Stripe must not be called for a granted contract.'; + + $customer = Customer::factory()->create(); + $granted = app(GrantSubscription::class)($customer, admin(), 'team', 'monthly', 'fsn', 0); + + $addon = app(BookAddon::class)($granted->fresh(), 'storage', 1); + + expect($addon->exists)->toBeTrue() + ->and($addon->fresh()->stripe_item_id)->toBeNull() + ->and($stripe->itemCalls)->toBeEmpty() + ->and($granted->fresh()->stripe_addon_sync)->toBeNull(); + + // And the sweep leaves it alone too: a contract Stripe never sold is not + // out of step with Stripe. + $this->artisan('clupilot:sync-stripe-subscriptions')->assertSuccessful(); + + expect($stripe->itemCalls)->toBeEmpty(); +}); + +it('does not undo a booking Stripe could not be told about, and finishes it on the sweep', function () { + Mail::fake(); + Queue::fake(); + + $stripe = addonStripe(); + $contract = addonContract(); + + $stripe->failWith = 'Connection timed out'; + + $addon = app(BookAddon::class)($contract, 'storage', 1); + + // The pack is booked and, on a running machine, already delivered. Rolling + // that back because an API was away would take storage off a customer who + // is holding it. + expect($addon->fresh()->isActive())->toBeTrue() + ->and($addon->fresh()->stripe_item_id)->toBeNull(); + + // And what did not happen is on the contract, not only in a log line. + expect($contract->fresh()->stripe_addon_sync['error'])->toContain('Connection timed out') + ->and($contract->fresh()->stripe_addon_sync['attempts'])->toBe(1); + + // The hourly sweep finishes it once Stripe answers again — the same one + // that retries a plan change that never landed. + $stripe->failWith = null; + $this->artisan('clupilot:sync-stripe-subscriptions')->assertSuccessful(); + + expect($stripe->itemsOn('sub_mod'))->toHaveCount(1) + ->and($addon->fresh()->stripe_item_id)->not->toBeNull() + ->and($contract->fresh()->stripe_addon_sync)->toBeNull(); +}); + +it('gives an upgrade’s proration a document for the amount Stripe actually charged', function () { + Mail::fake(); + Queue::fake(); + + addonStripe(); + $contract = addonContract(); + + $business = (string) PlanPrice::query() + ->where('plan_version_id', Subscription::snapshotFrom('business')['plan_version_id']) + ->where('term', 'monthly') + ->value('stripe_price_id'); + + // Stripe worked this out against ITS boundaries: the days left on the small + // package credited, the days left on the big one charged. A document written + // from our contract snapshot would state a sum nobody was charged, which is + // why this used to get no document at all — and why that was wrong. + $this->postJson(route('webhooks.stripe'), paidInvoice( + 'in_upgrade', + [ + stripeLine(addonPlanPrice($contract), -8950, now(), now()->addDays(15), proration: true), + stripeLine($business, 17450, now(), now()->addDays(15), proration: true), + ], + 'subscription_update', + now(), + now()->addDays(15), + ))->assertOk(); + + $invoice = Invoice::query()->sole(); + + expect($invoice->snapshot['lines'])->toHaveCount(2) + ->and($invoice->net_cents)->toBe(8500) + ->and($invoice->tax_cents)->toBe(1700) + ->and($invoice->gross_cents)->toBe(10200) + ->and(array_column($invoice->snapshot['lines'], 'description'))->toBe([ + __('billing.cart.plan', ['plan' => 'Team']), + __('billing.cart.plan', ['plan' => 'Business']), + ]); + + Mail::assertQueuedCount(1); +}); + +it('keeps a line it cannot name rather than dropping it from the document', function () { + Mail::fake(); + Queue::fake(); + + addonStripe(); + $contract = addonContract(); + + // A Price created by hand in Stripe's dashboard: money was taken for + // something this catalogue has never heard of. Leaving it off would give the + // customer a document whose total they cannot account for. + $this->postJson(route('webhooks.stripe'), paidInvoice( + 'in_unknown', + [ + stripeLine(addonPlanPrice($contract), 17900, now(), now()->addMonth()), + stripeLine('price_typed_by_hand', 5000, now(), now()->addMonth()), + ], + 'subscription_cycle', + now(), + now()->addMonth(), + ))->assertOk(); + + $invoice = Invoice::query()->sole(); + + expect($invoice->snapshot['lines'])->toHaveCount(2) + ->and($invoice->net_cents)->toBe(22900) + // Under Stripe's own description, which is at least true, rather than + // under a guess or not at all. + ->and($invoice->snapshot['lines'][1]['description']) + ->toBe('Stripe’s own wording, which no customer should have to read'); +}); diff --git a/tests/Feature/Billing/StripeBillingTest.php b/tests/Feature/Billing/StripeBillingTest.php index dd1f7fc..995a4ad 100644 --- a/tests/Feature/Billing/StripeBillingTest.php +++ b/tests/Feature/Billing/StripeBillingTest.php @@ -2,10 +2,15 @@ use App\Models\PlanFamily; use App\Models\PlanPrice; +use App\Models\PlanVersion; +use App\Models\StripeAddonPrice; +use App\Models\StripePendingEvent; use App\Models\Subscription; use App\Models\SubscriptionRecord; +use App\Services\Billing\PlanCatalogue; use App\Services\Stripe\FakeStripeClient; use App\Services\Stripe\StripeClient; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Queue; /** @@ -31,37 +36,65 @@ function stripeContract(string $stripeId = 'sub_1'): Subscription ]); } +/** The plan half of the mirror — modules live in the same lists. */ +function planPrices(FakeStripeClient $stripe): Collection +{ + return collect($stripe->prices)->filter(fn ($price) => isset($price['metadata']['plan_family'])); +} + +function planProducts(FakeStripeClient $stripe): Collection +{ + return collect($stripe->products)->filter(fn ($product) => isset($product['metadata']['plan_family'])); +} + +/** The module half: a Price per module and term, so a booking has one to bill on. */ +function modulePrices(FakeStripeClient $stripe): Collection +{ + return collect($stripe->prices)->filter(fn ($price) => isset($price['metadata']['addon'])); +} + it('mirrors the catalogue into Stripe, once', function () { $stripe = fakeStripe(); $this->artisan('stripe:sync-catalogue')->assertSuccessful(); // A product per family, a price per priced row of a published version. - expect($stripe->products)->toHaveCount(4) - ->and($stripe->prices)->toHaveCount(8) + expect(planProducts($stripe))->toHaveCount(4) + ->and(planPrices($stripe))->toHaveCount(8) ->and(PlanFamily::query()->whereNull('stripe_product_id')->count())->toBe(0) ->and(PlanPrice::query()->whereNull('stripe_price_id')->count())->toBe(0); + // And every module we sell, on both terms: a Stripe Price carries its own + // interval, so a module on a yearly contract needs a yearly one. Without + // these a booked module could never become an item on the subscription, + // which is why it was charged in the month it was booked and never again. + $modules = array_merge(array_keys((array) config('provisioning.addons')), ['storage']); + + expect(modulePrices($stripe))->toHaveCount(count($modules) * 2) + ->and(StripeAddonPrice::query()->where('addon_key', 'storage')->where('interval', 'year')->value('amount_cents')) + ->toBe(12 * (int) config('provisioning.storage_addon.price_cents')); + // A Stripe Price cannot be edited, so a second run that minted duplicates // would leave two live prices for one plan and no way to tell them apart. $this->artisan('stripe:sync-catalogue')->assertSuccessful(); - expect($stripe->products)->toHaveCount(4) - ->and($stripe->prices)->toHaveCount(8); + expect(planPrices($stripe))->toHaveCount(8) + ->and(modulePrices($stripe))->toHaveCount(count($modules) * 2) + ->and($stripe->products)->toHaveCount(4 + count($modules)); }); it('gives each price its own recurring interval', function () { $stripe = fakeStripe(); $this->artisan('stripe:sync-catalogue'); - $intervals = collect($stripe->prices)->groupBy('interval')->map->count(); + $intervals = planPrices($stripe)->groupBy('interval')->map->count(); // Monthly and yearly cannot share a Price, because the interval belongs to // the Price itself. expect($intervals['month'])->toBe(4) ->and($intervals['year'])->toBe(4); - $team = collect($stripe->prices)->first(fn ($p) => $p['metadata']['plan_family'] === 'team' && $p['interval'] === 'month'); + $team = planPrices($stripe)->first(fn ($p) => $p['metadata']['plan_family'] === 'team' && $p['interval'] === 'month'); expect($team['amount'])->toBe(17900) ->and($team['metadata']['plan_version'])->toBe('1'); }); @@ -70,7 +103,7 @@ it('does not put an unpublished draft in the price list', function () { $stripe = fakeStripe(); $family = PlanFamily::query()->where('key', 'team')->sole(); - app(App\Services\Billing\PlanCatalogue::class)->draft($family, [ + app(PlanCatalogue::class)->draft($family, [ 'quota_gb' => 600, 'traffic_gb' => 3000, 'seats' => 30, 'ram_mb' => 8192, 'cores' => 4, 'disk_gb' => 640, 'performance' => 'enhanced', 'template_vmid' => 9000, 'features' => [], @@ -80,8 +113,8 @@ it('does not put an unpublished draft in the price list', function () { // A draft has promised nothing; a Price for it would be a price list entry // for something that may never exist. - expect($stripe->prices)->toHaveCount(8) - ->and(collect($stripe->prices)->pluck('amount'))->not->toContain(19900); + expect(planPrices($stripe))->toHaveCount(8) + ->and(planPrices($stripe)->pluck('amount'))->not->toContain(19900); }); it('does not mint a second object when a run is interrupted before the id is stored', function () { @@ -98,8 +131,8 @@ it('does not mint a second object when a run is interrupted before the id is sto // Stripe replays the original answer for a repeated idempotency key, so the // catalogue reconnects to what is already there instead of duplicating it — // and a Price cannot be deleted afterwards to tidy up. - expect($stripe->products)->toHaveCount(4) - ->and($stripe->prices)->toHaveCount(8) + expect(planProducts($stripe))->toHaveCount(4) + ->and(planPrices($stripe))->toHaveCount(8) ->and(PlanPrice::query()->whereNull('stripe_price_id')->count())->toBe(0); }); @@ -135,7 +168,7 @@ it('does not revive a contract Stripe already ended', function () { it('does not put a plan that has never been published in Stripe at all', function () { $stripe = fakeStripe(); $family = PlanFamily::query()->create(['key' => 'agency', 'name' => 'Agentur', 'tier' => 5]); - app(App\Services\Billing\PlanCatalogue::class)->draft($family, [ + app(PlanCatalogue::class)->draft($family, [ 'quota_gb' => 600, 'traffic_gb' => 3000, 'seats' => 30, 'ram_mb' => 8192, 'cores' => 4, 'disk_gb' => 640, 'performance' => 'enhanced', 'template_vmid' => 9000, 'features' => [], @@ -145,7 +178,7 @@ it('does not put a plan that has never been published in Stripe at all', functio // Not even the Product: it would be a price-list entry for something that // has promised nothing to anyone. - expect($stripe->products)->toHaveCount(4) + expect(planProducts($stripe))->toHaveCount(4) ->and($family->fresh()->stripe_product_id)->toBeNull(); }); @@ -266,7 +299,7 @@ it('holds a billing event that arrives before the contract exists, and applies i 'data' => ['object' => ['id' => 'sub_early', 'status' => 'canceled', 'ended_at' => now()->timestamp]], ])->assertOk()->assertJson(['applied' => false]); - expect(App\Models\StripePendingEvent::query()->count())->toBe(1); + expect(StripePendingEvent::query()->count())->toBe(1); $this->postJson(route('webhooks.stripe'), [ 'id' => 'evt_checkout', 'type' => 'checkout.session.completed', @@ -285,7 +318,7 @@ it('holds a billing event that arrives before the contract exists, and applies i expect($subscription->status)->toBe('cancelled') ->and(SubscriptionRecord::query()->where('event', 'cancellation')->count())->toBe(1) // The holding area is a holding area, not a second event log. - ->and(App\Models\StripePendingEvent::query()->count())->toBe(0); + ->and(StripePendingEvent::query()->count())->toBe(0); }); it('holds nothing for a contract that is right there', function () { @@ -314,7 +347,7 @@ it('holds nothing for a contract that is right there', function () { // The holding area waits for contracts that do not exist yet. Ordinary // traffic must not silt it up with rows nothing will ever come back for. - expect(App\Models\StripePendingEvent::query()->count())->toBe(0) + expect(StripePendingEvent::query()->count())->toBe(0) ->and(SubscriptionRecord::query()->where('event', 'renewal')->count())->toBe(1); }); @@ -330,7 +363,7 @@ it('holds an event only once however often Stripe redelivers it', function () { $this->postJson(route('webhooks.stripe'), $event)->assertOk(); $this->postJson(route('webhooks.stripe'), $event)->assertOk(); - expect(App\Models\StripePendingEvent::query()->count())->toBe(1); + expect(StripePendingEvent::query()->count())->toBe(1); }); it('creates nothing when Stripe is not connected', function () { @@ -472,11 +505,11 @@ it('never rewrites the snapshot when a renewal arrives', function () { $subscription = stripeContract(); // The catalogue moves on between terms. - $catalogue = app(App\Services\Billing\PlanCatalogue::class); + $catalogue = app(PlanCatalogue::class); $current = $catalogue->currentVersion('team'); $catalogue->schedule($current, $current->available_from, now()); - $dearer = App\Models\PlanVersion::query()->create([ + $dearer = PlanVersion::query()->create([ ...$current->only(['plan_family_id', 'quota_gb', 'traffic_gb', 'seats', 'ram_mb', 'cores', 'disk_gb', 'performance', 'template_vmid']), 'version' => 2, 'features' => $current->features, 'available_from' => now(), ]);