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(), ]);