diff --git a/app/Actions/ApplyStripeBillingEvent.php b/app/Actions/ApplyStripeBillingEvent.php index ddbbb20..19e44a6 100644 --- a/app/Actions/ApplyStripeBillingEvent.php +++ b/app/Actions/ApplyStripeBillingEvent.php @@ -25,7 +25,10 @@ use Throwable; * theirs and is not a lawful Austrian invoice series; ours is, and it is gapless * 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. + * and invoicePaid() below for the two exceptions: the checkout's own invoice, + * which the purchase already has a document for, and a charge for a term that + * begins after the contract ended, which owes nobody anything — see + * owesADocument(). * * 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" @@ -86,7 +89,8 @@ class ApplyStripeBillingEvent // A contract that has ended stays ended (mutateInOrder refuses one), so // a final invoice arriving after the deletion cannot hand a departed // customer their service back. The payment is still entered in the - // register below; it did happen. + // register below; it did happen. Whether it also owes a DOCUMENT is a + // separate question with a separate answer — see owesADocument(). if ($isRenewal && $start !== null && $end !== null) { $this->mutateInOrder($subscription, $eventAt, self::RANK_PAID, fn (Subscription $fresh) => $end // Never backwards: a renewal delayed behind the next one would @@ -128,11 +132,85 @@ class ApplyStripeBillingEvent ) ); - $this->invoicePayment($subscription, $invoice, $start, $end); + if ($this->owesADocument($subscription->refresh(), $invoice, $start)) { + $this->invoicePayment($subscription, $invoice, $start, $end); + } return $record; } + /** + * Whether this charge is one the customer is still owed a document for. + * + * Nothing in this application could cancel a Stripe subscription, so a + * customer who cancelled and a customer who withdrew both stayed subscribed — + * and every month Stripe took another payment, `invoice.paid` arrived here, + * and a real invoice was drawn out of the gapless Austrian series and mailed, + * for a service that no longer existed. Cancelling the subscription is the + * first half of the fix; this is the second, because a subscription somebody + * failed to cancel, or one cancelled by hand in Stripe's dashboard, must not + * be able to mint numbers here either. + * + * ## The rule, and why it is not simply "refuse a cancelled contract" + * + * A perfectly legitimate FINAL invoice can arrive after the contract has + * ended, and refusing every one of them would lose a document the customer is + * legally owed. The commonest case: the cycle invoice for the term they were + * in went unpaid, Stripe's dunning ran for three weeks, the contract ended in + * the middle of it, and then the customer paid. That money is for a term they + * really did have a running cloud in. + * + * So the question is not WHEN the payment landed — Stripe's dunning makes that + * arbitrary — but WHAT it is for: + * + * the period this invoice bills STARTED before the contract ended + * → a term the customer was actually in. Document it. + * the period begins at or after the moment the contract ended + * → a term that does not exist. No document, no number, and an error in + * the log, because a charge for a service that has ended means Stripe + * was never stopped and somebody has to send that money back. + * + * The boundary case — a period starting exactly at `cancelled_at` — is the + * first month the customer does NOT have, so it is refused. + * + * A contract marked cancelled with no date on it, or an invoice carrying no + * period, cannot be judged by that rule at all, and both are refused. The + * asymmetry is deliberate and it is the same one IssueInvoice is built on: a + * missing document is recoverable — an operator issues it from the console — + * while a number handed out in error can never be withdrawn, only corrected by + * a cancellation document plus a new one. + * + * The register entry is written either way, above. The money moved; refusing + * to record that would hide it, and the register is where an operator finds + * the payments that need sending back. + * + * @param array $invoice + */ + private function owesADocument(Subscription $subscription, array $invoice, ?Carbon $periodStart): bool + { + if (! $subscription->hasEnded()) { + return true; + } + + $endedAt = $subscription->cancelled_at; + + if ($endedAt !== null && $periodStart !== null && $periodStart->lessThan($endedAt)) { + return true; + } + + Log::error('Stripe charged for a contract that has ended, so no invoice was issued and no number was used. This money has to go back.', [ + 'subscription' => $subscription->uuid, + 'stripe_subscription' => $subscription->stripe_subscription_id, + 'stripe_invoice' => $invoice['id'] ?? null, + 'billing_reason' => $invoice['billing_reason'] ?? null, + 'amount_paid' => $invoice['amount_paid'] ?? null, + 'contract_ended_at' => $endedAt?->toIso8601String(), + 'billed_period_start' => $periodStart?->toIso8601String(), + ]); + + return false; + } + /** * The document this payment owes the customer, and the one mail that carries * it. diff --git a/app/Actions/StartCustomerProvisioning.php b/app/Actions/StartCustomerProvisioning.php index 2cd2b10..31ff9ee 100644 --- a/app/Actions/StartCustomerProvisioning.php +++ b/app/Actions/StartCustomerProvisioning.php @@ -6,6 +6,7 @@ use App\Exceptions\IdentityCollisionException; use App\Mail\InvoiceMail; use App\Mail\OrderConfirmationMail; use App\Models\Customer; +use App\Models\Invoice; use App\Models\Order; use App\Models\ProvisioningRun; use App\Models\Subscription; @@ -128,12 +129,10 @@ class StartCustomerProvisioning /** * Invoice the payment, and tell the customer once. * - * ONE mail, not two. The invoice is issued at the moment the money lands, - * so an order confirmation and an invoice mail would otherwise go out in - * the same second for the same event — which is a defect, not a preference. - * The invoice mail carries the document and says everything the - * confirmation said, so it replaces it whenever there is an invoice; the - * confirmation remains for the case where there cannot be one. + * Both halves are idempotent and live in invoiceOnce() and mailInvoiceOnce() + * below, because this is not the only caller any more: resume() has to be able + * to finish exactly this work when a crash cut it off, and two copies of "have + * we invoiced this yet" would be two answers. * * Sent here rather than when provisioning finishes: those are minutes apart * at best and can be much longer, and somebody who has just been charged @@ -150,13 +149,64 @@ class StartCustomerProvisioning */ private function confirmByMail(Order $order, Customer $customer): void { + $invoice = $this->invoiceOnce($order, $customer); + + if ($invoice !== null) { + $this->mailInvoiceOnce($invoice, $customer); + + return; + } + $address = $customer->email; if (! is_string($address) || $address === '') { return; } - $invoice = null; + // No invoice, and there could not be one: an incomplete company profile, + // or a purchase that was a full gift. The confirmation says what it can. + // + // Sent from HERE only, never from resume(). It has nothing to stamp — an + // order carries no `sent_at` — so a redelivery could not tell that it had + // already gone out, and Stripe redelivers until it gets a 2xx. The invoice + // mail is the one a crash must not lose, and that one is stamped. + try { + Mail::to($address)->queue(new OrderConfirmationMail($order, (string) $customer->name)); + } catch (Throwable $e) { + Log::warning('Could not queue the purchase confirmation', [ + 'order' => $order->id, + 'exception' => $e->getMessage(), + ]); + } + } + + /** + * The document this purchase owes, issued exactly once however often we are + * asked. + * + * The guard is the invoice already filed against this order, because an + * invoice number comes out of a gapless series and can never be handed out + * twice. Cancellations are excluded from the search: a Storno carries the same + * `order_id` as the document it takes back, so counting one would leave a + * withdrawn purchase looking as though it had been invoiced by its own + * cancellation. + * + * Not a database-level guard, and it cannot be one — `invoices.order_id` is + * legitimately shared by an invoice and its Storno, so it can never be unique. + * What this closes is the case that actually happens: Stripe redelivering the + * same event, one delivery after another. + */ + private function invoiceOnce(Order $order, Customer $customer): ?Invoice + { + $existing = Invoice::query() + ->where('order_id', $order->id) + ->whereNull('cancels_invoice_id') + ->orderBy('id') + ->first(); + + if ($existing !== null) { + return $existing; + } try { // Refuses while the company details are incomplete, which is the @@ -165,21 +215,46 @@ class StartCustomerProvisioning // that can never be handed out again. The purchase is unaffected: // the order stands, the machine gets built, and the invoice can be // issued from the console once the details are there. - $invoice = app(IssueInvoice::class)->forOrders($customer, collect([$order])); + return app(IssueInvoice::class)->forOrders($customer, collect([$order])); } catch (Throwable $e) { Log::warning('Could not issue an invoice for this order', [ 'order' => $order->id, 'exception' => $e->getMessage(), ]); + + return null; + } + } + + /** + * Hand the invoice to the mailer, once. + * + * `sent_at` is the stamp, the same one App\Actions\IssueStripeInvoice uses, + * and it is written only after the mailer has accepted the message — so a + * redelivery arriving after the document was written but before the mail went + * out finishes the job, and one arriving afterwards does neither twice. + * + * ONE mail, not two. The invoice is issued at the moment the money lands, so + * an order confirmation and an invoice mail would otherwise go out in the same + * second for the same event. The invoice mail says everything the confirmation + * said and carries the document, so it replaces it whenever there is one. + */ + private function mailInvoiceOnce(Invoice $invoice, Customer $customer): void + { + $address = $customer->email; + + if (! is_string($address) || $address === '' || $invoice->sent_at !== null) { + return; } try { - Mail::to($address)->queue($invoice !== null - ? new InvoiceMail($invoice, (string) $customer->name) - : new OrderConfirmationMail($order, (string) $customer->name)); + Mail::to($address)->queue(new InvoiceMail($invoice, (string) $customer->name)); + + $invoice->update(['sent_at' => now()]); } catch (Throwable $e) { - Log::warning('Could not queue the purchase confirmation', [ - 'order' => $order->id, + Log::warning('Could not queue the invoice mail for this purchase', [ + 'order' => $invoice->order_id, + 'invoice' => $invoice->number, 'exception' => $e->getMessage(), ]); } @@ -199,17 +274,39 @@ class StartCustomerProvisioning * that was merely never dispatched needs no nudge from here — the scheduler * tick sweeps pending runs. A run that already ran and FAILED for want of * the contract does, because nothing sweeps failed runs. + * + * ## The invoice is the other half, and it used to be the half that was lost + * + * This returned as soon as a contract existed, and the only production call to + * IssueInvoice::forOrders() is in confirmByMail() — which runs AFTER the + * order commits. A worker killed in between left a paid purchase with no + * invoice, for good and in silence: the next redelivery found the contract + * already open, returned here, and nothing anywhere sweeps for an uninvoiced + * order. So the invoice and its mail are finished here too, on every + * redelivery, and the guards in invoiceOnce()/mailInvoiceOnce() are what make + * "however many times Stripe asks" come out as one invoice, one number and + * one mail. */ private function resume(Order $order): void { - if ($order->subscription()->exists()) { + if (! $order->subscription()->exists()) { + $this->openContract($order); + + if ($order->subscription()->exists()) { + $this->reviveRunStrandedWithoutAContract($order); + } + } + + $customer = $order->customer; + + if ($customer === null) { return; } - $this->openContract($order); + $invoice = $this->invoiceOnce($order, $customer); - if ($order->subscription()->exists()) { - $this->reviveRunStrandedWithoutAContract($order); + if ($invoice !== null) { + $this->mailInvoiceOnce($invoice, $customer); } } diff --git a/app/Actions/WithdrawContract.php b/app/Actions/WithdrawContract.php index 7a5f17e..08a2687 100644 --- a/app/Actions/WithdrawContract.php +++ b/app/Actions/WithdrawContract.php @@ -12,6 +12,7 @@ use App\Services\Billing\IssueInvoice; use App\Services\Billing\WithdrawalRight; use App\Services\Stripe\StripeClient; use Illuminate\Support\Carbon; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; use RuntimeException; use Throwable; @@ -33,20 +34,26 @@ use Throwable; * 2. **The withdrawal is claimed**, conditionally, on the contract. Two clicks * in flight write one withdrawal between them; the loser is told it has * already happened rather than sending a second refund. - * 3. **The paperwork**: the invoice that was issued is CANCELLED by a Storno - * with its own number — never edited, never deleted. That is the established - * rule for correcting an issued document, and a withdrawal is the clearest - * case there is for it. + * 3. **The paperwork**: EVERY invoice raised inside the window is CANCELLED by a + * Storno with its own number — never edited, never deleted. That is the + * established rule for correcting an issued document, and a withdrawal is the + * clearest case there is for it. Every one of them, and not just the opening + * purchase: a module booked on day three and a renewal that fell inside the + * fourteen days are charges too, and they used to be left standing because + * they carry no `order_id` — see chargesInsideTheWindow(). * 4. **The money**, which falls out of the paperwork rather than being - * computed beside it: the whole of what the cancelled document said. FAGG - * §16 would let us keep the pro-rata value of the days the cloud ran, and - * the owner has decided not to — a withdrawing consumer gets everything - * back. Two sums that must agree cannot disagree if only one of them is - * ever calculated, and here there is only the one: the Storno's. + * computed beside it: the whole of what each cancelled document said, sent + * back against the payment that actually took it. FAGG §16 would let us keep + * the pro-rata value of the days the cloud ran, and the owner has decided not + * to — a withdrawing consumer gets everything back. Two sums that must agree + * cannot disagree if only one of them is ever calculated, and here there is + * only the one: the Stornos'. * 5. **The service ends**, through App\Actions\EndInstanceService and the * `cancellation_scheduled` machinery it already reads — the address comes * down, the DNS record goes, the instance is marked ended. No second way - * for an instance to stop being served. + * for an instance to stop being served. Stripe is told to stop billing in + * the same breath, immediately: a withdrawal unwinds the contract, so there + * is no paid-up term to run out and no next cycle to raise. * * ## What happens when the money side fails * @@ -124,56 +131,175 @@ class WithdrawContract } /** - * The paperwork and the money. + * The paperwork and the money, for every charge inside the window. + * + * One Storno per document and one refund per payment, because that is what + * each of the two things IS: a Storno mirrors exactly one invoice and carries + * its number, and a Stripe refund goes against exactly one payment. A single + * refund for the whole sum, sent against the opening payment, would be + * refused by Stripe for exceeding it — which is the trap the old one-invoice + * version was hiding behind. + * + * Each charge is settled in its own try, so a module invoice whose payment + * cannot be reached does not stop the opening purchase from being refunded. + * What actually went back is totalled from the refunds that succeeded, never + * from what was intended — a contract claiming money moved that did not is + * the one state nobody may discover three weeks later. * * @return int what actually went back to the customer, gross, in cents */ private function settle(Subscription $subscription, Carbon $at): int { - try { - $original = $this->openingInvoice($subscription); + $charges = $this->chargesInsideTheWindow($subscription, $at); + $refunded = 0; + $references = []; + $failed = false; + if ($charges->isEmpty()) { // No document was ever issued for this purchase — the company // details were incomplete when it was made, or it was a grant. There // is nothing to cancel, so the refund is worked out from what was // charged instead. Stated here rather than silently skipped: a // withdrawal without paperwork is a withdrawal an accountant will // ask about. - if ($original === null) { - return $this->sendBack($subscription, max(0, $subscription->order?->chargedCents() ?? 0), $at); + try { + $gross = max(0, $subscription->order?->chargedCents() ?? 0); + + $reference = $this->refund( + $subscription, + $gross, + $this->paymentBehind(null, $subscription), + 'clupilot-withdrawal-'.$subscription->uuid, + $at, + ); + + if ($reference !== null) { + $refunded += $gross; + $references[] = $reference; + } + } catch (Throwable $e) { + $this->parkMoneyFailure($subscription, $e); + $failed = true; } - - // Taken back in full, with its own gapless number, pointing at the - // one it cancels. The original keeps its number for ever. - $storno = $this->invoices->cancelling($original); - - // The refund IS what the cancellation took back — never a second - // figure computed beside it, which would have to agree with the - // document and one day would not. The Storno mirrors the original, - // so this is the whole amount the customer paid. - return $this->sendBack($subscription, max(0, -(int) $storno->gross_cents), $at); - } catch (Throwable $e) { - $this->parkMoneyFailure($subscription, $e); - - return 0; } + + foreach ($charges as $invoice) { + try { + // Taken back in full, with its own gapless number, pointing at + // the one it cancels. The original keeps its number for ever. + $storno = $this->invoices->cancelling($invoice); + + // The refund IS what the cancellation took back — never a second + // figure computed beside it, which would have to agree with the + // document and one day would not. + $gross = max(0, -(int) $storno->gross_cents); + + $reference = $this->refund( + $subscription, + $gross, + $this->paymentBehind($invoice, $subscription), + // Keyed per DOCUMENT and not per contract. One key for the + // whole withdrawal would have Stripe replay the first + // refund's answer for the second, and the second charge + // would silently never be sent back at all. + 'clupilot-withdrawal-'.$subscription->uuid.'-invoice-'.$invoice->id, + $at, + ); + + if ($reference !== null) { + $refunded += $gross; + $references[] = $reference; + } + } catch (Throwable $e) { + $this->parkMoneyFailure($subscription, $e); + $failed = true; + } + } + + $subscription->update([ + 'withdrawal_refund_cents' => $refunded, + // Every reference, because there is a refund per charge and an + // operator reconciling this against Stripe needs all of them. + 'withdrawal_refund_reference' => $references === [] + ? null + : mb_substr(implode(',', $references), 0, 250), + // Cleared only when nothing went wrong. parkMoneyFailure() has + // already written the reason otherwise, and clearing it here would + // erase the one note saying this withdrawal needs a hand. + ...($failed ? [] : ['withdrawal_refund_error' => null]), + ]); + + return $refunded; } /** - * Put the money back where it came from. + * Every charge this contract raised inside the withdrawal window. * - * Stripe refunds a PAYMENT, not a subscription and not an invoice, so the - * handle is whatever the checkout reported — a PaymentIntent if there was - * one, otherwise the invoice that charged the card, resolved through Stripe - * once. A contract with neither was never paid for through Stripe at all (a - * grant, a contract opened by hand), and there is nothing to send back. + * `invoices.order_id` alone was the defect: only the opening purchase carries + * one. A renewal and a Stripe-invoice document are written against the + * CONTRACT and leave `order_id` null on purpose — see + * IssueInvoice::forBilledPeriod() and forStripeInvoice(), which say why — so a + * customer who booked a storage pack on day three and withdrew on day ten had + * that pack neither cancelled nor refunded. The owner's rule is that a + * withdrawing consumer gets EVERYTHING back, so every document has to be in + * this list. + * + * Both links, therefore, and both are needed: the opening invoice has an + * `order_id` and no `subscription_id`, and everything after it is the other + * way round. + * + * Cancellations are excluded by construction — one is not an invoice that can + * be taken back — and so is anything that already carries a Storno. + * IssueInvoice::cancelling() refuses the latter in its own right; excluding it + * here as well means a second withdrawal attempt finds nothing to do rather + * than raising an exception that would be parked as a money failure. + * + * Bounded at the moment of withdrawal rather than at fourteen days. It is the + * same set — a withdrawal can only be declared inside the window, so every + * document this contract has is inside it — and the bound that matters is + * "before the customer changed their mind", which is what `$at` is. + * + * @return Collection */ - private function sendBack(Subscription $subscription, int $grossCents, Carbon $at): int + private function chargesInsideTheWindow(Subscription $subscription, Carbon $at): Collection { - $subscription->update(['withdrawal_refund_cents' => $grossCents]); + return Invoice::query() + ->where(function ($query) use ($subscription) { + $query->where('subscription_id', $subscription->id); - if ($grossCents <= 0) { - return 0; + if ($subscription->order_id !== null) { + $query->orWhere('order_id', $subscription->order_id); + } + }) + ->whereNull('cancels_invoice_id') + ->whereNotIn( + 'id', + Invoice::query()->whereNotNull('cancels_invoice_id')->select('cancels_invoice_id') + ) + ->where('created_at', '<=', $at) + ->orderBy('id') + ->get(); + } + + /** + * The Stripe payment a charge was settled by. + * + * Stripe refunds a PAYMENT, not a subscription and not an invoice, and each + * charge inside the window was a different payment: the checkout, a renewal, + * a module prorated in the middle of a term. So every document is sent back + * against the money that actually paid for it. + * + * The opening purchase is the one document with no Stripe invoice of its own — + * it is written from the ORDER, which is where the checkout's PaymentIntent + * was recorded, falling back to the invoice that charged the card, resolved + * through Stripe once. A contract with neither was never paid for through + * Stripe at all (a grant, a contract opened by hand), and there is nothing to + * send back. + */ + private function paymentBehind(?Invoice $invoice, Subscription $subscription): ?string + { + if ($invoice?->stripe_invoice_id !== null) { + return $this->stripe->invoicePaymentReference((string) $invoice->stripe_invoice_id); } $order = $subscription->order; @@ -183,26 +309,36 @@ class WithdrawContract $reference = $this->stripe->invoicePaymentReference((string) $order->stripe_invoice_id); } + return $reference; + } + + /** + * Put one charge's money back where it came from. + * + * @return string|null Stripe's reference for the refund, or null when there + * was nothing to send back + */ + private function refund( + Subscription $subscription, + int $grossCents, + ?string $reference, + string $idempotencyKey, + Carbon $at, + ): ?string { + if ($grossCents <= 0) { + return null; + } + if ($reference === null) { throw new RuntimeException( - 'No Stripe payment is recorded for this contract, so the refund has to be sent by hand.' + 'No Stripe payment is recorded for this charge, so the refund has to be sent by hand.' ); } - $refundId = $this->stripe->refund( - $reference, - $grossCents, - // Keyed on the contract, because a contract is withdrawn from once. - // A retry after a timeout that in fact went through replays Stripe's - // first answer rather than sending the money a second time — which - // is the single most expensive mistake this file could make. - idempotencyKey: 'clupilot-withdrawal-'.$subscription->uuid, - ); - - $subscription->update([ - 'withdrawal_refund_reference' => $refundId, - 'withdrawal_refund_error' => null, - ]); + // A retry after a timeout that in fact went through replays Stripe's + // first answer rather than sending the money a second time — which is + // the single most expensive mistake this file could make. + $refundId = $this->stripe->refund($reference, $grossCents, idempotencyKey: $idempotencyKey); Log::info('Refunded a withdrawal.', [ 'subscription' => $subscription->uuid, @@ -211,7 +347,7 @@ class WithdrawContract 'withdrawn_at' => $at->toIso8601String(), ]); - return $grossCents; + return $refundId; } /** @@ -228,6 +364,13 @@ class WithdrawContract * reason: there is no term left to keep. The customer has withdrawn from the * contract the modules hang off, and BookAddon::cancel() takes them off * Stripe and out of the register. + * + * The order of the two Stripe calls is the reason the subscription is stopped + * LAST and not first. BookAddon::cancel() takes each module's item off the + * subscription, and an item cannot be removed from a subscription that no + * longer exists — so cancelling the whole thing first would make every one of + * those removals fail and park a sync error on a contract that is already + * over. */ private function endEverything(Subscription $subscription, Carbon $at): void { @@ -235,6 +378,8 @@ class WithdrawContract $this->bookAddon->cancel($addon); } + $this->stopBilling($subscription); + $instance = $this->instanceOf($subscription); if ($instance !== null && $instance->status !== 'ended') { @@ -255,6 +400,53 @@ class WithdrawContract ]); } + /** + * Tell Stripe to stop, now. + * + * The call nothing in this application used to make. A withdrawal marked the + * contract cancelled, ended the service and sent the money back, and the + * subscription at Stripe went on charging the card every month — for a + * machine that had been switched off, with `invoice.paid` arriving here each + * time and drawing a real invoice number for it. + * + * Immediately and not at the period end, which is the whole difference + * between this and a customer's own cancellation: a withdrawal unwinds the + * contract from the beginning, the entire amount has just gone back, and + * there is no paid-up term left for the customer to use up. + * + * A failure does not un-declare the withdrawal — same principle as the refund, + * for the same reason — but it is the most urgent thing on this contract, so + * it is said on the contract as well as in the log. Written after settle(), + * so the note cannot be cleared by a refund that succeeded. + */ + private function stopBilling(Subscription $subscription): void + { + // Nothing to stop, and nothing has gone wrong. A granted package was + // never sold through Stripe — GrantSubscription leaves the id null on + // purpose — and neither was a contract opened by hand. + if ($subscription->stripe_subscription_id === null) { + return; + } + + try { + $this->stripe->cancelSubscription( + (string) $subscription->stripe_subscription_id, + StripeClient::CANCEL_IMMEDIATELY, + // Keyed on the contract, because a contract is withdrawn from + // once. A retry after a timeout replays Stripe's first answer. + idempotencyKey: 'clupilot-withdrawal-stop-'.$subscription->uuid, + ); + } catch (Throwable $e) { + Log::error('A withdrawal was declared and Stripe was not told to stop charging for it.', [ + 'subscription' => $subscription->uuid, + 'stripe_subscription' => $subscription->stripe_subscription_id, + 'error' => $e->getMessage(), + ]); + + $this->note($subscription, 'Stripe was not told to stop: '.$e->getMessage()); + } + } + /** * The machine this contract pays for. * @@ -285,26 +477,6 @@ class WithdrawContract ->first(); } - /** - * The document issued for the purchase that opened this contract. - * - * Cancellations are excluded by construction — one is not an invoice that - * can be taken back — and so is anything that already has a Storno against - * it, which IssueInvoice::cancelling() refuses in its own right. - */ - private function openingInvoice(Subscription $subscription): ?Invoice - { - if ($subscription->order_id === null) { - return null; - } - - return Invoice::query() - ->where('order_id', $subscription->order_id) - ->whereNull('cancels_invoice_id') - ->orderBy('id') - ->first(); - } - /** * The refund did not go out. Said on the contract, not only in a log. * @@ -320,8 +492,28 @@ class WithdrawContract 'error' => $e->getMessage(), ]); + $this->note($subscription, $e->getMessage()); + } + + /** + * Write a sentence an operator has to read onto the contract itself. + * + * Appended rather than overwritten, because a withdrawal can go wrong in more + * than one way at once — a module invoice whose payment could not be reached + * AND an order to stop billing that Stripe refused — and an operator who read + * only the last of them would send the money back and leave the customer + * subscribed. + */ + private function note(Subscription $subscription, string $sentence): void + { + $existing = trim((string) $subscription->withdrawal_refund_error); + $subscription->update([ - 'withdrawal_refund_error' => mb_substr($e->getMessage(), 0, 250), + 'withdrawal_refund_error' => mb_substr( + $existing === '' ? $sentence : $existing.' | '.$sentence, + 0, + 250, + ), ]); } diff --git a/app/Console/Commands/VerifyVatIds.php b/app/Console/Commands/VerifyVatIds.php index 3baaba6..40ac932 100644 --- a/app/Console/Commands/VerifyVatIds.php +++ b/app/Console/Commands/VerifyVatIds.php @@ -19,10 +19,24 @@ use Illuminate\Console\Command; * before this check existed is unverified, so those customers are on the domestic * rate today whether or not they are entitled to reverse charge. * - * Deliberately NOT scheduled by default. It talks to a public service with a - * concurrency limit, on behalf of somebody else's tax position, and an owner - * should decide the cadence rather than inherit one — see the note in - * routes/console.php. Run it monthly, or before a VAT return. + * ## Scheduled monthly, and why that is not the owner's decision to inherit + * + * This used to say it was deliberately NOT scheduled, and to point at a note in + * routes/console.php that did not exist — so an owner reading the schedule could + * not learn the command was there at all, and reverse charge went on resting on a + * one-off answer for ever. + * + * It is scheduled now, because the exposure is the SELLER's. A registration that + * is withdrawn keeps earning rate 0 on every invoice we issue afterwards, and the + * unpaid VAT on those is ours to make good, not the customer's — so the cadence is + * not a preference but the width of the window in which that can happen unnoticed. + * Monthly, on the first, because that is the rhythm the VAT return is filed on: a + * number that lapsed in March is caught before the March return is prepared. + * + * The public service's concurrency limit is answered by `--limit` rather than by + * not running: the query takes the longest-unchecked first, so a customer base + * larger than one run rotates through it instead of starving. `--dry-run` and + * `--customer` are for the by-hand run before a return. */ class VerifyVatIds extends Command { diff --git a/app/Livewire/ConfirmCancelPackage.php b/app/Livewire/ConfirmCancelPackage.php index 18f01af..f0919e1 100644 --- a/app/Livewire/ConfirmCancelPackage.php +++ b/app/Livewire/ConfirmCancelPackage.php @@ -3,14 +3,42 @@ namespace App\Livewire; use App\Livewire\Concerns\ResolvesCustomer; +use App\Models\Customer; use App\Models\Instance; +use App\Models\Subscription; +use App\Services\Stripe\StripeClient; +use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Log; use LivewireUI\Modal\ModalComponent; +use Throwable; /** * Cancel the customer's package (R5). Per the founder's decision: effective at * the end of the billing term, irreversible once confirmed; at term end the * customer receives a finished data export, then the instance is deprovisioned * (both mocked for now). Requires typing the instance subdomain to confirm. + * + * ## Stripe is told first, and everything else follows from its answer + * + * This used to write three columns on the instance and stop there — the contract + * was untouched and Stripe was never told anything at all, so the card went on + * being charged every month for a package that had ended, for ever. The order of + * the two writes is therefore the design: + * + * 1. **Stripe is asked to stop at the period end.** If it refuses or cannot be + * reached, NOTHING here is recorded and the customer is asked to try again. + * Recording a cancellation we could not make effective is the defect this + * class was built out of: the customer sees "gekündigt" on their settings page + * and keeps paying. A cancellation has no deadline — unlike the statutory + * withdrawal in App\Actions\WithdrawContract, which stands whatever Stripe + * says — so a retry a minute later costs nobody anything. + * 2. **Then our own rows.** The reverse failure — Stripe stopped, our write lost + * — is the harmless direction: the customer is not charged again, and + * Stripe's `customer.subscription.deleted` at the period end closes the + * contract here through the machinery that already reads it. + * + * The order to stop carries an idempotency key on the contract, so the retry + * after a timeout that in fact went through is provably a no-op. */ class ConfirmCancelPackage extends ModalComponent { @@ -40,21 +68,124 @@ class ConfirmCancelPackage extends ModalComponent return; } + $contract = $this->contractFor($customer, $instance); + + if (! $this->stopBilling($contract)) { + $this->addError('confirmName', __('settings.cancel_billing_unreachable')); + + return; + } + $instance->update([ 'status' => 'cancellation_scheduled', 'cancel_requested_at' => now(), - 'service_ends_at' => $this->currentPeriodEnd($instance), + 'service_ends_at' => $this->currentPeriodEnd($instance, $contract), ]); + // The contract carries the request too. Without it nothing in the billing + // half of the application can tell a cancelled contract from an untouched + // one — and the status deliberately stays `active`, because until the term + // runs out this is a paying customer. + $contract?->update(['cancel_requested_at' => now()]); + return $this->redirectRoute('settings', navigate: true); } /** - * End of the current monthly billing period, anchored on the subscription - * start (the order date) rather than assuming calendar-month billing. + * Ask Stripe to raise no further cycle for this contract. + * + * @return bool whether the cancellation may now be recorded */ - private function currentPeriodEnd(Instance $instance): \Illuminate\Support\Carbon + private function stopBilling(?Subscription $contract): bool { + // Nothing to stop, and nothing has gone wrong. A granted package was + // never sold through Stripe — GrantSubscription leaves the id null on + // purpose — and neither was a machine built by hand with no contract + // behind it at all. + if ($contract?->stripe_subscription_id === null) { + return true; + } + + try { + app(StripeClient::class)->cancelSubscription( + (string) $contract->stripe_subscription_id, + // At the period end, because that is what the customer is + // promised on this very modal: everything stays available to the + // end of the term they have already paid for. + StripeClient::CANCEL_AT_PERIOD_END, + // Keyed on the contract, because a contract is cancelled once. + idempotencyKey: 'clupilot-cancel-'.$contract->uuid, + ); + + return true; + } catch (Throwable $e) { + Log::error('A customer asked to cancel and Stripe could not be told to stop, so nothing was recorded.', [ + 'subscription' => $contract->uuid, + 'stripe_subscription' => $contract->stripe_subscription_id, + 'error' => $e->getMessage(), + ]); + + return false; + } + } + + /** + * The contract this machine is billed through. + * + * `subscriptions.instance_id` is the link and is written when resources are + * reserved. The order is the fallback because an order belongs to exactly one + * purchase; the customer is the last resort, and only when they have exactly + * one contract — guessing between two would cancel the wrong one, which is + * worse than cancelling nothing. + */ + private function contractFor(?Customer $customer, Instance $instance): ?Subscription + { + if ($instance->subscription !== null) { + return $instance->subscription; + } + + if ($instance->order_id !== null) { + $byOrder = Subscription::query()->where('order_id', $instance->order_id)->latest('id')->first(); + + if ($byOrder !== null) { + return $byOrder; + } + } + + if ($customer === null) { + return null; + } + + $contracts = $customer->subscriptions()->where('status', 'active')->get(); + + return $contracts->count() === 1 ? $contracts->first() : null; + } + + /** + * The end of the term the customer has already paid for. + * + * The contract knows it, and it is the only thing that does: `term` is + * monthly or yearly, and `current_period_end` is the boundary Stripe pushes + * forward on every renewal — so it is right for both, and right for a + * customer who is three months into a yearly contract. Walking months forward + * from the order date, which is what this used to do, took eleven months of + * paid service off every yearly customer who cancelled. + * + * The fallback below is only for a machine with NO contract behind it — one + * built by hand, or one whose contract failed to open. There is genuinely + * nothing else to read there: no term is recorded anywhere, so a month is the + * only assumption available, and it is the shorter of the two. + */ + private function currentPeriodEnd(Instance $instance, ?Subscription $contract): Carbon + { + if ($contract?->current_period_end !== null) { + // Taken as it stands, even when it is already past. A term that has + // run out has run out — the customer is in dunning, not in a paid + // period — and inventing another month for them would give away + // service nobody decided to give. + return $contract->current_period_end->copy(); + } + $start = $instance->order?->created_at ?? $instance->created_at ?? now(); // Always add from the immutable start so the billing-day anchor is kept diff --git a/app/Models/Subscription.php b/app/Models/Subscription.php index 1ba5cec..aeefa36 100644 --- a/app/Models/Subscription.php +++ b/app/Models/Subscription.php @@ -62,6 +62,12 @@ class Subscription extends Model 'current_period_start' => 'datetime', 'current_period_end' => 'datetime', 'pending_effective_at' => 'datetime', + // When the customer asked for the contract to end, and when it + // actually did. Two facts, not one: a cancelled package is still a + // paying customer until the term they bought runs out, so the first + // is stamped the moment they confirm and the second only when Stripe + // reports the subscription gone. + 'cancel_requested_at' => 'datetime', 'cancelled_at' => 'datetime', // The fourteen days a consumer may change their mind in, and what // happened inside them. See App\Services\Billing\WithdrawalRight, @@ -316,6 +322,21 @@ class Subscription extends Model return $this->withdrawn_at !== null; } + /** + * Is this contract over — cancelled at the end of its term, withdrawn from, + * or deleted at Stripe? + * + * `status` and not `cancel_requested_at`: a customer who has cancelled keeps + * everything they paid for until the term runs out, and treating them as + * gone the moment they clicked would take away service they are owed. Only + * `cancelled` is the end, and it is written by whichever of the three got + * there — see ApplyStripeBillingEvent::subscriptionDeleted(). + */ + public function hasEnded(): bool + { + return $this->status === 'cancelled'; + } + public function isYearly(): bool { return $this->term === self::TERM_YEARLY; diff --git a/app/Services/Stripe/FakeStripeClient.php b/app/Services/Stripe/FakeStripeClient.php index 0a9a948..2c5e657 100644 --- a/app/Services/Stripe/FakeStripeClient.php +++ b/app/Services/Stripe/FakeStripeClient.php @@ -69,6 +69,15 @@ class FakeStripeClient implements StripeClient */ public array $invoiceLines = []; + /** + * Every order to stop billing, in order, so a test can assert WHEN a + * contract was cancelled as well as that it was — the difference between a + * customer keeping the term they paid for and losing it. + * + * @var array + */ + public array $cancellations = []; + /** * Every refund that was sent, in order, so a test can assert the amount as * well as the fact of it. @@ -258,6 +267,36 @@ class FakeStripeClient implements StripeClient ]; } + public function cancelSubscription( + string $subscriptionId, + string $when, + ?string $idempotencyKey = null, + ): void { + $this->failIfAsked(); + + // Refused here as well as in HttpStripeClient, so a caller that invents a + // third timing fails in the tests rather than in production. + if (! in_array($when, [self::CANCEL_AT_PERIOD_END, self::CANCEL_IMMEDIATELY], true)) { + throw new RuntimeException("Unknown cancellation timing: {$when}"); + } + + // Replays the first answer for a repeated key, as Stripe does, so a retry + // after a timeout records one order to stop rather than two. + if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) { + return; + } + + $this->cancellations[] = [ + 'subscription' => $subscriptionId, + 'when' => $when, + 'key' => $idempotencyKey, + ]; + + if ($idempotencyKey !== null) { + $this->keys[$idempotencyKey] = $subscriptionId; + } + } + public function refund( string $paymentReference, ?int $amountCents = null, diff --git a/app/Services/Stripe/HttpStripeClient.php b/app/Services/Stripe/HttpStripeClient.php index 797ace6..68afdf6 100644 --- a/app/Services/Stripe/HttpStripeClient.php +++ b/app/Services/Stripe/HttpStripeClient.php @@ -171,6 +171,48 @@ class HttpStripeClient implements StripeClient ->throw(); } + public function cancelSubscription( + string $subscriptionId, + string $when, + ?string $idempotencyKey = null, + ): void { + $atPeriodEnd = match ($when) { + self::CANCEL_AT_PERIOD_END => true, + self::CANCEL_IMMEDIATELY => false, + // A typo would take a paying customer's service away this afternoon, + // or leave a withdrawn one subscribed for ever. Neither is a thing to + // fall back to a default about. + default => throw new RuntimeException("Unknown cancellation timing: {$when}"), + }; + + if ($atPeriodEnd) { + // An UPDATE and not a delete. The subscription stays alive to the + // boundary the customer has paid to, raises no further cycle, and + // Stripe ends it there of its own accord; deleting it here would take + // the service away the moment somebody clicked "cancel". + $this->request($idempotencyKey) + ->asForm() + ->post($this->url('subscriptions/'.$subscriptionId), ['cancel_at_period_end' => 'true']) + ->throw(); + + return; + } + + // Both stated rather than left to Stripe's defaults, because both decide + // money. `prorate` would raise a credit for the unused days and + // `invoice_now` would charge whatever is unbilled — and the caller that + // asks for an immediate stop is a withdrawal, which sends the WHOLE + // amount back from a cancellation document of its own. A credit note of + // Stripe's beside that would be the same money twice. + $this->request($idempotencyKey) + ->asForm() + ->delete($this->url('subscriptions/'.$subscriptionId), [ + 'prorate' => 'false', + 'invoice_now' => 'false', + ]) + ->throw(); + } + public function refund( string $paymentReference, ?int $amountCents = null, diff --git a/app/Services/Stripe/StripeClient.php b/app/Services/Stripe/StripeClient.php index 4aa461a..5a1c8d7 100644 --- a/app/Services/Stripe/StripeClient.php +++ b/app/Services/Stripe/StripeClient.php @@ -30,6 +30,26 @@ interface StripeClient */ public const PRORATE_NONE = 'none'; + /** + * Stop at the end of the term the customer has already paid for. + * + * What a CANCELLATION means. They keep the service to the boundary they have + * been billed to, no further cycle is ever raised, and Stripe ends the + * subscription itself at that moment — which arrives here as + * `customer.subscription.deleted` and is what ApplyStripeBillingEvent reads + * as the end of the contract. + */ + public const CANCEL_AT_PERIOD_END = 'at_period_end'; + + /** + * Stop now. + * + * What a WITHDRAWAL means. The contract is unwound rather than ended, the + * whole amount goes back, and there is no paid-up term left to run out — see + * App\Actions\WithdrawContract. + */ + public const CANCEL_IMMEDIATELY = 'immediately'; + /** Whether an account is actually configured. */ public function isConfigured(): bool; @@ -149,6 +169,31 @@ interface StripeClient */ public function removeSubscriptionItem(string $itemId, string $prorationBehaviour): void; + /** + * Tell Stripe to stop charging for a contract that is over. + * + * The one call this platform could not make. Nothing here cancelled a Stripe + * subscription at all: a customer's cancellation wrote a date on the + * instance, a withdrawal marked the contract cancelled and sent the money + * back, and in both cases Stripe went on taking the next month, and the next, + * for a service that had ended — and every one of those payments arrived as + * `invoice.paid` and drew a number out of the gapless Austrian series. + * + * `$when` is one of the CANCEL_* constants and is always passed explicitly, + * for the same reason `$prorationBehaviour` is above: the two are different + * promises to the customer, and which one a caller means has to be legible at + * the call site rather than inherited from a default written down here. + * + * `$idempotencyKey` because a retry after a timeout cannot tell whether the + * first attempt landed. Cancelling twice is harmless at Stripe, but the same + * key makes the retry provably a no-op instead of merely a likely one. + */ + public function cancelSubscription( + string $subscriptionId, + string $when, + ?string $idempotencyKey = null, + ): void; + /** * Send money back, against the payment that took it. * diff --git a/database/migrations/2026_07_31_140000_record_a_cancellation_on_the_contract_too.php b/database/migrations/2026_07_31_140000_record_a_cancellation_on_the_contract_too.php new file mode 100644 index 0000000..f97f279 --- /dev/null +++ b/database/migrations/2026_07_31_140000_record_a_cancellation_on_the_contract_too.php @@ -0,0 +1,45 @@ +timestamp('cancel_requested_at')->nullable()->after('stripe_event_rank'); + }); + } + + public function down(): void + { + Schema::table('subscriptions', function (Blueprint $table) { + $table->dropColumn('cancel_requested_at'); + }); + } +}; diff --git a/lang/de/settings.php b/lang/de/settings.php index aeb6bc3..8b14fdb 100644 --- a/lang/de/settings.php +++ b/lang/de/settings.php @@ -48,6 +48,7 @@ return [ 'cancel_confirm_label' => 'Zum Bestätigen „:name" eingeben:', 'cancel_confirm' => 'Verbindlich kündigen', 'cancel_mismatch' => 'Die Eingabe stimmt nicht mit Ihrer Cloud-Adresse überein.', + 'cancel_billing_unreachable' => 'Die Kündigung konnte gerade nicht bei unserem Zahlungsdienstleister eingetragen werden. Es wurde nichts geändert — bitte versuchen Sie es in ein paar Minuten erneut.', 'keep' => 'Abbrechen', 'close_account_title' => 'Konto schließen', diff --git a/lang/en/settings.php b/lang/en/settings.php index 4427fd1..8f33501 100644 --- a/lang/en/settings.php +++ b/lang/en/settings.php @@ -48,6 +48,7 @@ return [ 'cancel_confirm_label' => 'Type “:name” to confirm:', 'cancel_confirm' => 'Cancel for good', 'cancel_mismatch' => 'That does not match your cloud address.', + 'cancel_billing_unreachable' => 'The cancellation could not be registered with our payment provider just now. Nothing has been changed — please try again in a few minutes.', 'keep' => 'Keep', 'close_account_title' => 'Close account', diff --git a/routes/console.php b/routes/console.php index 25326c9..0137430 100644 --- a/routes/console.php +++ b/routes/console.php @@ -160,6 +160,24 @@ Schedule::command('clupilot:end-cancelled-addons') ->everyFifteenMinutes() ->withoutOverlapping(); +// Ask the VIES register again about the VAT numbers reverse charge rests on. +// +// A number is verified once, at the moment somebody types it in, and never again +// — and a registration that is wound up or withdrawn keeps earning rate 0 on +// every invoice issued after it lapsed. The unpaid VAT on those is the SELLER's +// liability, not the customer's, so the cadence here is the width of the window +// in which that can happen without anyone noticing. +// +// Monthly, on the first, because that is the rhythm a VAT return is filed on: a +// registration that lapsed in March is caught before the March return is +// prepared. Not nightly — it is somebody else's public service, it answers about +// a tax position rather than about our own machines, and a number does not lapse +// twice in a month. The command takes the longest-unchecked first, so a customer +// base larger than one run's --limit rotates through instead of starving. +Schedule::command('clupilot:verify-vat-ids') + ->monthlyOn(1, '04:10') + ->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/PackageCancellationTest.php b/tests/Feature/Billing/PackageCancellationTest.php new file mode 100644 index 0000000..1d75a3e --- /dev/null +++ b/tests/Feature/Billing/PackageCancellationTest.php @@ -0,0 +1,291 @@ + 'CluPilot Cloud e.U.', + 'address' => 'Dreherstraße 66/1/8', + 'postcode' => '1110', + 'city' => 'Wien', + 'vat_id' => 'ATU00000000', + ]); +}); + +afterEach(function () { + Carbon::setTestNow(); +}); + +/** A Stripe that records what it was asked to stop. */ +function cancellationStripe(): FakeStripeClient +{ + $fake = new FakeStripeClient; + app()->instance(StripeClient::class, $fake); + + return $fake; +} + +/** + * A running package its owner can reach the cancel modal for. + * + * @return array{0: User, 1: Instance, 2: Subscription} + */ +function cancellablePackage(string $term = Subscription::TERM_MONTHLY, array $contract = []): array +{ + $user = User::factory()->create(['email' => 'berger@example.test', 'email_verified_at' => now()]); + + $customer = Customer::factory()->create([ + 'email' => 'berger@example.test', + 'user_id' => $user->id, + 'status' => 'active', + ]); + + // Two and a half months ago, which is what makes the old month-walk visible: + // it would land on 1 April whatever the term is. + $order = Order::factory()->create([ + 'customer_id' => $customer->id, + 'plan' => 'team', + 'created_at' => '2026-01-01 09:00:00', + ]); + + $instance = Instance::factory()->create([ + 'customer_id' => $customer->id, + 'order_id' => $order->id, + 'plan' => 'team', + 'status' => 'active', + 'subdomain' => 'berger', + ]); + + $subscription = Subscription::factory()->plan('team', $term)->create(array_merge([ + 'customer_id' => $customer->id, + 'order_id' => $order->id, + 'instance_id' => $instance->id, + 'stripe_subscription_id' => 'sub_cancel', + 'started_at' => '2026-01-01 09:00:00', + 'current_period_start' => '2026-01-01 09:00:00', + 'current_period_end' => $term === Subscription::TERM_YEARLY + ? '2027-01-01 09:00:00' + : '2026-04-01 09:00:00', + ], $contract)); + + return [$user, $instance->refresh(), $subscription->refresh()]; +} + +it('tells Stripe to stop at the end of the term and keeps a yearly customer their year', function () { + $stripe = cancellationStripe(); + [$user, $instance, $contract] = cancellablePackage(Subscription::TERM_YEARLY); + + Livewire::actingAs($user)->test(ConfirmCancelPackage::class) + ->set('confirmName', 'berger') + ->call('cancelPackage') + ->assertHasNoErrors(); + + // Stripe is told, and told WHICH cancellation this is. At the period end, + // because the customer keeps everything they have paid for; an immediate stop + // would be a withdrawal, which is a different promise and a refund. + expect($stripe->cancellations)->toHaveCount(1) + ->and($stripe->cancellations[0]['subscription'])->toBe('sub_cancel') + ->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_AT_PERIOD_END); + + $instance->refresh(); + + // The end of the YEAR that was bought, written out rather than recomputed — + // and explicitly not the date months-from-the-order-date used to produce. + expect($instance->status)->toBe('cancellation_scheduled') + ->and($instance->service_ends_at->toDateTimeString())->toBe('2027-01-01 09:00:00') + ->and($instance->service_ends_at->toDateString())->not->toBe('2026-04-01'); + + $contract->refresh(); + + // The contract carries the request — and is still ACTIVE, because until the + // first of January this is a paying customer with a running cloud. Only + // Stripe reporting the subscription gone ends it. + expect($contract->cancel_requested_at?->toDateTimeString())->toBe('2026-03-15 09:00:00') + ->and($contract->status)->toBe('active') + ->and($contract->cancelled_at)->toBeNull(); +}); + +it('keeps a monthly customer their month and no longer', function () { + $stripe = cancellationStripe(); + [$user, $instance] = cancellablePackage(Subscription::TERM_MONTHLY); + + Livewire::actingAs($user)->test(ConfirmCancelPackage::class) + ->set('confirmName', 'berger') + ->call('cancelPackage') + ->assertHasNoErrors(); + + // The other half of the pair: a monthly contract really does end a month + // after its period started, so the yearly assertion above is about the TERM + // and not about this code being generous to everybody. + expect($instance->fresh()->service_ends_at->toDateTimeString())->toBe('2026-04-01 09:00:00') + ->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_AT_PERIOD_END); +}); + +it('cancels a granted package cleanly and asks Stripe nothing at all', function () { + $stripe = cancellationStripe(); + + // A grant was never sold through Stripe — GrantSubscription leaves the id + // null on purpose — so there is no subscription to stop and nothing has gone + // wrong. + [$user, $instance, $contract] = cancellablePackage(Subscription::TERM_MONTHLY, [ + 'stripe_subscription_id' => null, + 'granted_at' => '2026-01-01 09:00:00', + 'granted_by' => admin()->id, + ]); + + Livewire::actingAs($user)->test(ConfirmCancelPackage::class) + ->set('confirmName', 'berger') + ->call('cancelPackage') + ->assertHasNoErrors(); + + expect($stripe->cancellations)->toBeEmpty() + ->and($instance->fresh()->status)->toBe('cancellation_scheduled') + ->and($instance->fresh()->service_ends_at->toDateTimeString())->toBe('2026-04-01 09:00:00') + ->and($contract->fresh()->cancel_requested_at)->not->toBeNull(); +}); + +it('records nothing at all when Stripe cannot be told to stop', function () { + $stripe = cancellationStripe(); + $stripe->failWith = 'connection refused'; + + [$user, $instance, $contract] = cancellablePackage(); + + Livewire::actingAs($user)->test(ConfirmCancelPackage::class) + ->set('confirmName', 'berger') + ->call('cancelPackage') + ->assertHasErrors(['confirmName']); + + // A cancellation we could not make effective is the defect this whole file + // is about: the customer would read "gekündigt" on their settings page and + // keep paying. Nothing is written, and they are asked to try again — which + // costs nobody anything, because a cancellation has no deadline. + expect($instance->fresh()->status)->toBe('active') + ->and($instance->fresh()->service_ends_at)->toBeNull() + ->and($contract->fresh()->cancel_requested_at)->toBeNull(); +}); + +/** + * A contract that has ended, and a Stripe that has not noticed. + */ +function endedContract(string $endedAt = '2026-04-01 09:00:00'): Subscription +{ + $customer = Customer::factory()->create(['name' => 'Berger GmbH', 'email' => 'kundin@example.test']); + + return Subscription::factory()->plan('team')->create([ + 'customer_id' => $customer->id, + 'stripe_subscription_id' => 'sub_ended', + 'current_period_start' => '2026-03-01 09:00:00', + 'current_period_end' => $endedAt, + 'status' => 'cancelled', + 'cancelled_at' => $endedAt, + ]); +} + +/** The event Stripe raises when a term rolls over and is paid. */ +function cyclePaid(string $invoiceId, string $periodStart, string $periodEnd): array +{ + return [ + 'id' => 'evt_'.$invoiceId, + 'type' => 'invoice.paid', + 'data' => ['object' => [ + 'id' => $invoiceId, + 'subscription' => 'sub_ended', + 'amount_paid' => 21480, + 'total' => 21480, + 'billing_reason' => 'subscription_cycle', + 'period_start' => Carbon::parse($periodStart)->timestamp, + 'period_end' => Carbon::parse($periodEnd)->timestamp, + ]], + ]; +} + +it('issues no invoice and uses no number for a term that begins after the contract ended', function () { + Mail::fake(); + $contract = endedContract(); + + // Stripe charged for April, and the contract ended on the first of April. + // That is a month the customer does not have. + $this->postJson(route('webhooks.stripe'), cyclePaid('in_after', '2026-04-01 09:00:00', '2026-05-01 09:00:00')) + ->assertOk(); + + // No document, and no mail carrying one. + expect(Invoice::query()->count())->toBe(0); + Mail::assertNothingQueued(); + + // The money moved all the same, and the register is where an operator finds + // the payments that have to go back — so it IS recorded. What must not + // happen is a lawful invoice being written for a service that no longer + // exists. + expect(SubscriptionRecord::query() + ->where('subscription_id', $contract->id) + ->where('event', SubscriptionRecord::EVENT_RENEWAL) + ->count())->toBe(1); + + // And the number was not merely unused, it was never drawn: the next + // legitimate document is still the first of the year. A gapless series + // cannot afford a number that went nowhere. + $final = cyclePaid('in_final', '2026-03-01 09:00:00', '2026-04-01 09:00:00'); + $this->postJson(route('webhooks.stripe'), $final)->assertOk(); + + expect(Invoice::query()->sole()->number)->toBe('RE-2026-0001'); +}); + +it('still issues the final invoice for a term the customer really did use', function () { + Mail::fake(); + $contract = endedContract(); + + // The one case refusing everything would lose. The cycle invoice for March + // went unpaid, Stripe's dunning ran, the contract ended on the first of + // April in the middle of it — and then the customer paid. That money is for + // a month they had a running cloud in, and they are owed the document for it. + $this->travelTo('2026-04-05 11:00:00'); + + $this->postJson(route('webhooks.stripe'), cyclePaid('in_march', '2026-03-01 09:00:00', '2026-04-01 09:00:00')) + ->assertOk(); + + $invoice = Invoice::query()->sole(); + + expect($invoice->stripe_invoice_id)->toBe('in_march') + ->and($invoice->subscription_id)->toBe($contract->id) + ->and($invoice->number)->toBe('RE-2026-0001') + // Issued AND sent: the customer gets the paper, not just a row. + ->and($invoice->sent_at)->not->toBeNull(); + + Mail::assertQueued(InvoiceMail::class); +}); diff --git a/tests/Feature/Billing/WithdrawalTest.php b/tests/Feature/Billing/WithdrawalTest.php index 833236d..3026313 100644 --- a/tests/Feature/Billing/WithdrawalTest.php +++ b/tests/Feature/Billing/WithdrawalTest.php @@ -333,6 +333,98 @@ it('cancels the invoice with its own document and never reuses a number', functi ->toThrow(RuntimeException::class); }); +it('tells Stripe to stop charging immediately', function () { + $stripe = withdrawalStripe(); + [$contract] = withdrawableContract(['customer_type' => Customer::TYPE_CONSUMER]); + + // The call nothing in this application used to be able to make. Without it a + // withdrawal ended the service, sent the money back — and left the card being + // charged every month for a machine that had been switched off. + $contract->update(['stripe_subscription_id' => 'sub_withdraw']); + + Carbon::setTestNow('2026-03-08 09:00:00'); + + app(WithdrawContract::class)($contract->refresh()); + + expect($stripe->cancellations)->toHaveCount(1) + ->and($stripe->cancellations[0]['subscription'])->toBe('sub_withdraw') + // IMMEDIATELY, which is the whole difference from a customer's own + // cancellation: a withdrawal unwinds the contract and the entire amount + // has just gone back, so there is no paid-up term left to use up. + ->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_IMMEDIATELY) + ->and($contract->refresh()->withdrawal_refund_error)->toBeNull(); +}); + +it('refunds and cancels every charge inside the window, not only the opening one', function () { + $stripe = withdrawalStripe(); + [$contract, $opening] = withdrawableContract(['customer_type' => Customer::TYPE_CONSUMER]); + + $contract->update(['stripe_subscription_id' => 'sub_withdraw']); + + // Day three: the customer books a storage pack, Stripe prorates it and takes + // 12,00 € there and then. Its document points at the CONTRACT and carries no + // `order_id` — which is exactly why the old code, which looked the document up + // by `invoices.order_id`, could not see it at all. It was neither cancelled + // nor refunded, and the owner's rule is that a withdrawing consumer gets + // everything back. + Carbon::setTestNow('2026-03-04 09:00:00'); + + $module = app(IssueInvoice::class)->forStripeInvoice( + subscription: $contract->refresh(), + stripeInvoiceId: 'in_module', + lines: [[ + 'id' => 'il_module', + 'amount' => 1200, + 'quantity' => 1, + 'price' => ['id' => 'price_storage', 'metadata' => ['addon' => 'storage']], + ]], + ); + + // A different charge is a different payment, and Stripe refunds a PAYMENT. + $stripe->invoicePayments['in_module'] = 'pi_module'; + + expect($module->order_id)->toBeNull() + ->and($module->subscription_id)->toBe($contract->id) + ->and($module->gross_cents)->toBe(1200); + + Carbon::setTestNow('2026-03-08 09:00:00'); + + app(WithdrawContract::class)($contract->refresh()); + + // A Storno per document, because that is what a Storno is: it mirrors ONE + // invoice and carries its number. Both originals keep theirs for ever. + $stornos = Invoice::query()->whereNotNull('cancels_invoice_id')->orderBy('id')->get(); + + expect($stornos)->toHaveCount(2) + ->and($stornos->pluck('cancels_invoice_id')->all())->toBe([$opening->id, $module->id]) + ->and($stornos->pluck('gross_cents')->all())->toBe([-5880, -1200]); + + // And the money: one refund per payment, each against the payment that took + // it. One refund of 70,80 € against the opening PaymentIntent is what Stripe + // would have refused for exceeding the charge. + expect($stripe->refunds)->toHaveCount(2) + ->and($stripe->refunds[0]['payment'])->toBe('pi_withdraw') + ->and($stripe->refunds[0]['amount'])->toBe(5880) + ->and($stripe->refunds[1]['payment'])->toBe('pi_module') + ->and($stripe->refunds[1]['amount'])->toBe(1200) + // Distinct idempotency keys. One key for the whole withdrawal would have + // Stripe replay the first refund's answer for the second, and the module + // charge would silently never go back. + ->and($stripe->refunds[0]['key'])->not->toBe($stripe->refunds[1]['key']); + + expect($contract->refresh()->withdrawal_refund_cents)->toBe(7080) + ->and($contract->withdrawal_refund_error)->toBeNull(); + + // The register states what actually went back, so it agrees with the two + // cancellation documents rather than with the opening one alone. + $entry = SubscriptionRecord::query() + ->where('subscription_id', $contract->id) + ->where('event', SubscriptionRecord::EVENT_WITHDRAWAL) + ->firstOrFail(); + + expect($entry->snapshot['withdrawal']['refunded_gross_cents'])->toBe(7080); +}); + it('records the withdrawal in the proof register', function () { withdrawalStripe(); [$contract] = withdrawableContract(['customer_type' => Customer::TYPE_CONSUMER]); diff --git a/tests/Feature/Provisioning/EndInstanceServiceTest.php b/tests/Feature/Provisioning/EndInstanceServiceTest.php index 4b54289..c8b2e42 100644 --- a/tests/Feature/Provisioning/EndInstanceServiceTest.php +++ b/tests/Feature/Provisioning/EndInstanceServiceTest.php @@ -6,7 +6,10 @@ use App\Models\Customer; use App\Models\Host; use App\Models\Instance; use App\Models\Order; +use App\Models\Subscription; use App\Models\User; +use App\Services\Stripe\FakeStripeClient; +use App\Services\Stripe\StripeClient; use App\Support\ProvisioningSettings; use Livewire\Livewire; @@ -158,7 +161,25 @@ it('is safe to run twice', function () { }); it('carries the whole way from the customer cancelling to the address going away', function () { - ['services' => $services, 'instance' => $instance] = routedInstance(); + $this->travelTo('2026-05-10 09:00:00'); + + ['services' => $services, 'instance' => $instance, 'order' => $order] = routedInstance(); + + $stripe = new FakeStripeClient; + app()->instance(StripeClient::class, $stripe); + + $contract = Subscription::query()->where('order_id', $order->id)->sole(); + + $contract->update([ + 'stripe_subscription_id' => 'sub_journey', + // The boundary the customer has paid to, and the one Stripe pushes + // forward on every renewal. Written as a literal date on purpose: this + // test used to travel to whatever `service_ends_at` the implementation + // had just computed, which made it recompute the code instead of + // checking it — a monthly answer on a yearly contract passed as happily + // as the right one. + 'current_period_end' => '2026-08-01 09:00:00', + ]); $user = User::factory()->create(['email_verified_at' => now()]); Customer::query()->whereKey($instance->customer_id)->update(['user_id' => $user->id, 'email' => $user->email]); @@ -170,14 +191,29 @@ it('carries the whole way from the customer cancelling to the address going away $instance->refresh(); expect($instance->status)->toBe('cancellation_scheduled') - ->and($instance->service_ends_at)->not->toBeNull(); + ->and($instance->service_ends_at->toDateTimeString())->toBe('2026-08-01 09:00:00'); - // Still theirs, still served, for the whole of the term they bought. + // The billing half, which was the broken half: the money stops too. Nothing + // in this application could cancel a Stripe subscription at all, so a + // customer whose address came down here went on being charged every month. + expect($stripe->cancellations)->toHaveCount(1) + ->and($stripe->cancellations[0]['subscription'])->toBe('sub_journey') + ->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_AT_PERIOD_END) + ->and($contract->fresh()->cancel_requested_at?->toDateTimeString())->toBe('2026-05-10 09:00:00') + // Still active: until the first of August this is a paying customer. + ->and($contract->fresh()->status)->toBe('active'); + + // Still theirs, still served, for the whole of the term they bought — an hour + // before it runs out as much as on the day they cancelled. + $this->artisan('clupilot:end-due-services')->assertSuccessful(); + expect($services['traefik']->routes)->toHaveKey('berger'); + + $this->travelTo('2026-08-01 08:00:00'); $this->artisan('clupilot:end-due-services')->assertSuccessful(); expect($services['traefik']->routes)->toHaveKey('berger'); // And gone the moment it is genuinely over. - $this->travelTo($instance->service_ends_at->copy()->addMinute()); + $this->travelTo('2026-08-01 09:01:00'); $this->artisan('clupilot:end-due-services')->assertSuccessful(); expect($services['traefik']->routes)->toBe([]) diff --git a/tests/Feature/Provisioning/StripeWebhookTest.php b/tests/Feature/Provisioning/StripeWebhookTest.php index 5cb452d..db02b52 100644 --- a/tests/Feature/Provisioning/StripeWebhookTest.php +++ b/tests/Feature/Provisioning/StripeWebhookTest.php @@ -1,10 +1,16 @@ 'CluPilot Cloud e.U.', + 'address' => 'Dreherstraße 66/1/8', + 'postcode' => '1110', + 'city' => 'Wien', + 'vat_id' => 'ATU00000000', + ]); + + $customer = Customer::factory()->create(['email' => 'kunde@example.com', 'name' => 'Kanzlei Berger']); + + $factory = Order::factory(); + + return ($withContract ? $factory->withSubscription() : $factory)->create([ + 'customer_id' => $customer->id, + 'plan' => 'team', + 'amount_cents' => 21480, + 'currency' => 'EUR', + 'status' => 'paid', + 'stripe_event_id' => $sessionId, + ]); +} + +it('finishes the invoice a crash interrupted, once, however often Stripe asks', function () { + Mail::fake(); + Queue::fake(); + + // A paid order with its contract open and NO invoice: the state a worker + // killed between the order committing and confirmByMail() leaves behind. + // Nothing anywhere sweeps for that, so Stripe's own retry is the only chance + // this customer has of ever getting the invoice they paid for — and resume() + // used to see the contract, decide there was nothing to do, and return. + $order = interruptedPurchase('cs_evt_crash'); + + expect(Invoice::query()->count())->toBe(0); + + $this->postJson(route('webhooks.stripe'), stripeEvent('evt_crash'))->assertOk(); + + $invoice = Invoice::query()->sole(); + + expect($invoice->order_id)->toBe($order->id) + ->and($invoice->gross_cents)->toBe(21480) + ->and($invoice->number)->toBe('RE-'.now()->year.'-0001') + // Stamped, which is what makes the mail idempotent as well as the number. + ->and($invoice->sent_at)->not->toBeNull(); + + Mail::assertQueued(InvoiceMail::class, 1); + // The invoice mail says everything the confirmation said and carries the + // document, so it replaces it. Two mails for one purchase is the defect this + // whole path was written around. + Mail::assertNotQueued(OrderConfirmationMail::class); + + // Two more redeliveries change nothing. An invoice number can never be handed + // out twice, and nobody may receive the same invoice three times. + $this->postJson(route('webhooks.stripe'), stripeEvent('evt_crash'))->assertOk(); + $this->postJson(route('webhooks.stripe'), stripeEvent('evt_crash'))->assertOk(); + + expect(Invoice::query()->count())->toBe(1) + ->and(Invoice::query()->sole()->number)->toBe($invoice->number); + + Mail::assertQueued(InvoiceMail::class, 1); +}); + +it('opens the contract and invoices when the crash came even earlier', function () { + Mail::fake(); + Queue::fake(); + + // Killed before openContract() this time: a paid order with no contract at + // all. Both halves of the repair have to happen on the redelivery. + $order = interruptedPurchase('cs_evt_crash_early', withContract: false); + + expect($order->subscription()->exists())->toBeFalse(); + + $this->postJson(route('webhooks.stripe'), stripeEvent('evt_crash_early'))->assertOk(); + + expect($order->fresh()->subscription()->exists())->toBeTrue() + ->and(Invoice::query()->sole()->order_id)->toBe($order->id); + + Mail::assertQueued(InvoiceMail::class, 1); +}); + it('rejects an invalid signature when a secret is configured', function () { config()->set('services.stripe.webhook_secret', 'whsec_test'); diff --git a/tests/Feature/SettingsTest.php b/tests/Feature/SettingsTest.php index a96d285..130b510 100644 --- a/tests/Feature/SettingsTest.php +++ b/tests/Feature/SettingsTest.php @@ -6,7 +6,10 @@ use App\Livewire\Settings; use App\Models\Customer; use App\Models\Instance; use App\Models\Order; +use App\Models\Subscription; use App\Models\User; +use App\Services\Stripe\FakeStripeClient; +use App\Services\Stripe\StripeClient; use Livewire\Livewire; function settingsSetup(bool $withInstance = true): array @@ -97,23 +100,54 @@ it('rejects an invalid brand colour', function () { }); it('schedules package cancellation only with the correct typed confirmation', function () { + $stripe = new FakeStripeClient; + app()->instance(StripeClient::class, $stripe); + ['user' => $user, 'customer' => $customer] = settingsSetup(); $instance = $customer->instances()->first(); - // Wrong confirmation → no change. + // A YEARLY contract, because the assertion below is a date and a monthly + // assumption would land eleven months early. This test used to assert only + // that `service_ends_at` was not null — a column the code had just written, + // which the broken monthly arithmetic satisfied exactly as well as the right + // answer does. + $contract = Subscription::factory()->plan('team', Subscription::TERM_YEARLY)->create([ + 'customer_id' => $customer->id, + 'order_id' => $instance->order_id, + 'instance_id' => $instance->id, + 'stripe_subscription_id' => 'sub_settings', + 'current_period_start' => '2026-01-01 00:00:00', + 'current_period_end' => '2027-01-01 00:00:00', + ]); + + // Wrong confirmation → no change, and Stripe is not told anything either. Livewire::actingAs($user)->test(ConfirmCancelPackage::class) ->set('confirmName', 'wrong') ->call('cancelPackage') ->assertHasErrors(['confirmName']); - expect($instance->fresh()->status)->toBe('active'); + expect($instance->fresh()->status)->toBe('active') + ->and($stripe->cancellations)->toBeEmpty(); // Correct confirmation → scheduled. Livewire::actingAs($user)->test(ConfirmCancelPackage::class) ->set('confirmName', 'acme') ->call('cancelPackage'); + $instance->refresh(); + expect($instance->status)->toBe('cancellation_scheduled') - ->and($instance->service_ends_at)->not->toBeNull(); + // The end of the term that was PAID FOR, stated as a date somebody chose. + ->and($instance->service_ends_at->toDateTimeString())->toBe('2027-01-01 00:00:00'); + + // The contract knows it was cancelled — and is still active, because until + // the first of January this is a paying customer with a running cloud. + expect($contract->fresh()->cancel_requested_at)->not->toBeNull() + ->and($contract->fresh()->status)->toBe('active'); + + // And Stripe was told to stop, at the period end and not now. + expect($stripe->cancellations)->toHaveCount(1) + ->and($stripe->cancellations[0]['subscription'])->toBe('sub_settings') + ->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_AT_PERIOD_END); }); it('blocks closing the account while a package is active', function () {