From f02e86769bf86451fd6a07aaa512a50e0fd1a88a Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 22:42:02 +0200 Subject: [PATCH] Charge the price the website shows, and hand a withdrawal back in full Stripe was charging the catalogue's NET figure while the document added the domestic rate on top: a customer quoted 214,80 paid 179,00 and was then invoiced for VAT nobody had collected. Under para 11(12) UStG that VAT is owed to the tax office whether or not it ever arrived, so every document issued created a liability against revenue that did not contain it. The Stripe Price now carries the GROSS figure. price_cents stays net - it is frozen onto every contract and PlanChange prorates against it, so redefining it would corrupt every pro-rata sum ever computed. Only the amount at the till moved, and it is formed by the one call TaxTreatment already answers for the price sheet and for the invoice, so displayed, charged and invoiced cannot drift apart. Stripe's automatic_tax is deliberately not used. TaxTreatment is the single tax authority here, and a second rate computed by Stripe would take 19 % from a German consumer while our document said 20 %. A Stripe Price cannot be edited, so a changed figure means a new Price and the old one archived - which stops it being SOLD and leaves every subscription already on it billing the old amount for ever. stripe:reprice-subscriptions is the second half, and a command of its own because it touches live money. A withdrawing consumer is refunded in full, as the owner decided. That is more generous than FAGG para 16 requires, and it makes the express-request consent the statute hinges on irrelevant - so the gate is gone rather than left looking load-bearing. Co-Authored-By: Claude Opus 5 --- app/Actions/IssueStripeInvoice.php | 34 +++- app/Actions/OpenSubscription.php | 7 +- app/Actions/StartCustomerProvisioning.php | 10 +- app/Actions/SyncStripeAddonItems.php | 96 ++++++++++ app/Actions/WithdrawContract.php | 74 ++------ .../Commands/RepriceStripeSubscriptions.php | 149 +++++++++++++++ app/Console/Commands/SyncStripeCatalogue.php | 174 +++++++++++++----- app/Http/Controllers/LandingController.php | 22 ++- .../Controllers/StripeWebhookController.php | 12 +- app/Livewire/Admin/RecordWithdrawal.php | 11 +- app/Livewire/ConfirmWithdraw.php | 18 +- app/Models/Order.php | 38 +++- app/Models/StripeAddonPrice.php | 17 +- app/Models/StripePlanPrice.php | 40 ++++ app/Models/Subscription.php | 3 + app/Services/Billing/AddonPrices.php | 117 +++++++++++- app/Services/Billing/InvoiceMath.php | 126 +++++++++++++ app/Services/Billing/IssueInvoice.php | 91 +++++++-- app/Services/Billing/StripeInvoiceLines.php | 40 +++- app/Services/Billing/TaxTreatment.php | 78 +++++++- app/Services/Billing/WithdrawalRight.php | 83 +++------ app/Services/Stripe/StripeClient.php | 9 +- ...00_charge_the_price_that_is_advertised.php | 117 ++++++++++++ lang/de/checkout.php | 17 +- lang/de/withdrawal.php | 18 +- lang/en/checkout.php | 15 +- lang/en/withdrawal.php | 16 +- .../admin/record-withdrawal.blade.php | 10 +- .../views/livewire/confirm-withdraw.blade.php | 22 +-- .../Billing/StripeAddonBillingTest.php | 65 ++++--- tests/Feature/Billing/StripeBillingTest.php | 10 +- tests/Feature/Billing/WithdrawalTest.php | 106 +++++------ 32 files changed, 1251 insertions(+), 394 deletions(-) create mode 100644 app/Console/Commands/RepriceStripeSubscriptions.php create mode 100644 app/Models/StripePlanPrice.php create mode 100644 database/migrations/2026_07_30_110000_charge_the_price_that_is_advertised.php diff --git a/app/Actions/IssueStripeInvoice.php b/app/Actions/IssueStripeInvoice.php index b8ae547..b46dc3d 100644 --- a/app/Actions/IssueStripeInvoice.php +++ b/app/Actions/IssueStripeInvoice.php @@ -208,7 +208,18 @@ class IssueStripeInvoice return null; } - return app(IssueInvoice::class)->forBilledPeriod($subscription, $from, $to, $stripeInvoiceId); + // The total Stripe took, where it said so. A document has to add up to + // the money, and the contract's own price at today's rate is only the + // next best answer — it would miss a cycle billed before a rate change. + $total = $stripeInvoice['total'] ?? $stripeInvoice['amount_paid'] ?? null; + + return app(IssueInvoice::class)->forBilledPeriod( + $subscription, + $from, + $to, + $stripeInvoiceId, + chargedCents: is_numeric($total) ? (int) $total : null, + ); } /** @@ -237,11 +248,18 @@ class IssueStripeInvoice /** * 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. + * Our catalogue is pushed to Stripe as GROSS, so the line amounts that come + * back are what the customer paid and the document divides that sum rather + * than adding a rate to it. Stripe's total and our gross must therefore be + * the SAME NUMBER, to the cent, for every customer — including a + * reverse-charge one, whose whole charged amount is stated as net at 0 %. + * + * This used to accept our net as an alternative, on the grounds that an + * account might not collect tax for us. That tolerance is what let the + * defect run: 179,00 € taken against a document for 214,80 € matched the + * "net" branch and was never reported. Anything but equality is now worth a + * warning, and the likeliest cause is a Price that stripe:sync-catalogue has + * replaced with nobody having run stripe:reprice-subscriptions. * * Never an exception. The document is issued and the payment is recorded; * raising here would only cost Stripe its 2xx and change nothing. @@ -258,11 +276,11 @@ class IssueStripeInvoice $total = (int) $total; - if ($total === $invoice->gross_cents || $total === $invoice->net_cents) { + if ($total === $invoice->gross_cents) { return; } - Log::warning('An invoice was issued for a sum that is neither the net nor the gross Stripe reported.', [ + Log::warning('An invoice was issued for a sum Stripe did not charge.', [ 'invoice' => $invoice->number, 'stripe_invoice' => $invoice->stripe_invoice_id, 'stripe_total' => $total, diff --git a/app/Actions/OpenSubscription.php b/app/Actions/OpenSubscription.php index c7ab6ed..ca1a5f4 100644 --- a/app/Actions/OpenSubscription.php +++ b/app/Actions/OpenSubscription.php @@ -86,9 +86,10 @@ class OpenSubscription // afterwards, at which point the window has to have been running // all along rather than starting from the correction. 'withdrawal_ends_at' => $start->copy()->addDays(WithdrawalRight::WINDOW_DAYS), - // Carried from the purchase: the express request to begin at - // once is what makes a withdrawing consumer owe the pro-rata - // share, and it was given at the checkout, not here. + // Carried from the purchase, where it was given. Nothing reads + // it to decide anything any more — a withdrawing consumer gets + // the whole amount back — so this is a record of what was agreed + // at the checkout and not a gate. See Order's own cast. 'immediate_start_consent_at' => $order->immediate_start_consent_at, 'status' => 'active', ], diff --git a/app/Actions/StartCustomerProvisioning.php b/app/Actions/StartCustomerProvisioning.php index cbea15a..2cd2b10 100644 --- a/app/Actions/StartCustomerProvisioning.php +++ b/app/Actions/StartCustomerProvisioning.php @@ -78,11 +78,11 @@ class StartCustomerProvisioning // App\Actions\WithdrawContract. 'stripe_payment_intent_id' => $event['stripe_payment_intent_id'] ?? null, 'stripe_invoice_id' => $event['stripe_invoice_id'] ?? null, - // Stamped only where the consumer actually gave it. Null is - // the honest default and it is the EXPENSIVE one for us: a - // withdrawal without this consent on record refunds the - // whole amount, because we cannot charge for days the - // customer never agreed to start. + // Stamped only where the consumer actually gave it. Nothing + // turns on it any more: a withdrawal refunds the whole + // amount whether it is there or not, because the owner has + // decided not to charge for the days the cloud ran. Kept as + // a record of what the customer was shown and ticked. 'immediate_start_consent_at' => ($event['immediate_start_consent'] ?? false) === true ? now() : null, 'status' => 'paid', ]); diff --git a/app/Actions/SyncStripeAddonItems.php b/app/Actions/SyncStripeAddonItems.php index 1bc8616..f696c85 100644 --- a/app/Actions/SyncStripeAddonItems.php +++ b/app/Actions/SyncStripeAddonItems.php @@ -106,6 +106,102 @@ class SyncStripeAddonItems } } + /** + * Move every module item this contract carries onto the Price that charges + * today's figure for it. + * + * The catalogue moved from pushing NET amounts to Stripe to pushing GROSS + * ones, and a Stripe Price cannot be edited — so every running module is on a + * Price that takes too little. __invoke() above will not find them: the item + * exists and the quantity is right, so it has nothing to reconcile. This is + * the deliberate second pass, run by stripe:reprice-subscriptions. + * + * PRORATE_NONE throughout. The customer has already paid for the term they + * are in, at whatever was taken then; charging the difference for days + * already served would be a bill nobody agreed to. From the next cycle + * Stripe simply takes the right amount. + * + * The module's own frozen `price_cents` is untouched, because that is what + * the customer agreed to pay net and it is what every pro-rata sum reads. + * + * @return array what moved, for the command's output + */ + public function reprice(Subscription $subscription, bool $dryRun = false): array + { + if ($subscription->stripe_subscription_id === null) { + return []; + } + + $moved = []; + + foreach ($this->groups($subscription) as $group) { + $billable = $group->filter( + fn (SubscriptionAddon $addon) => $addon->cancelled_at === null && $addon->cancels_at === null + ); + + $stamped = $group->filter(fn (SubscriptionAddon $addon) => $addon->stripe_item_id !== null); + $itemId = $stamped->first()?->stripe_item_id; + $first = $billable->first(); + + // Nothing Stripe is billing, or nothing it should be: __invoke() + // owns both of those cases and would undo whatever this did. + if ($itemId === null || $first === null || (int) $first->price_cents <= 0) { + continue; + } + + $target = $this->prices->liveFor( + (string) $first->addon_key, + (int) $first->price_cents, + (string) $first->currency, + (string) $subscription->term, + ); + + $current = $stamped->first()?->stripe_price_id; + + if ($target !== null && $target === $current) { + continue; + } + + $moved[] = ['addon' => (string) $first->addon_key, 'from' => $current, 'to' => $target]; + + if ($dryRun) { + continue; + } + + $this->guardConfigured(); + + // Minted here where the sweep left a figure the catalogue no longer + // pre-creates — a grandfathered booking, or a discounted grant. + $target ??= $this->prices->ensure( + (string) $first->addon_key, + (int) $first->price_cents, + (string) $first->currency, + (string) $subscription->term, + ); + + if ($target === null) { + // Nothing to bill for it after all — a module given away. Taken + // back out of the report rather than announced as a move. + array_pop($moved); + + continue; + } + + $moved[count($moved) - 1]['to'] = $target; + + $this->stripe->updateSubscriptionPrice( + (string) $subscription->stripe_subscription_id, + $itemId, + $target, + StripeClient::PRORATE_NONE, + ); + + $this->stamp($group, $itemId, $target); + } + + return $moved; + } + /** * The bookings that matter, grouped into the items they should be. * diff --git a/app/Actions/WithdrawContract.php b/app/Actions/WithdrawContract.php index 59061a6..7a5f17e 100644 --- a/app/Actions/WithdrawContract.php +++ b/app/Actions/WithdrawContract.php @@ -9,7 +9,6 @@ use App\Models\Subscription; use App\Models\SubscriptionAddon; use App\Models\SubscriptionRecord; use App\Services\Billing\IssueInvoice; -use App\Services\Billing\TaxTreatment; use App\Services\Billing\WithdrawalRight; use App\Services\Stripe\StripeClient; use Illuminate\Support\Carbon; @@ -35,14 +34,15 @@ use Throwable; * 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 — and if the consumer - * owes a pro-rata share, a new invoice states it. That is the established + * 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. * 4. **The money**, which falls out of the paperwork rather than being - * computed beside it: what was invoiced, less what the new document says is - * still owed. Two sums that must agree cannot disagree if only one of them - * is ever calculated. + * 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. * 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 @@ -132,55 +132,26 @@ class WithdrawContract { try { $original = $this->openingInvoice($subscription); - $owedNet = WithdrawalRight::owedNetCents($subscription, $at); // 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 and nothing to correct, 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. + // 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) { - $paid = (int) ($subscription->order?->amount_cents ?? 0); - $owedGross = $owedNet === 0 ? 0 : $this->grossFor($subscription, $owedNet); - - return $this->sendBack($subscription, max(0, $paid - $owedGross), $at); + return $this->sendBack($subscription, max(0, $subscription->order?->chargedCents() ?? 0), $at); } // Taken back in full, with its own gapless number, pointing at the // one it cancels. The original keeps its number for ever. - $this->invoices->cancelling($original); + $storno = $this->invoices->cancelling($original); - // What the consumer owes for the days the cloud actually ran. Its - // own document, because the money is real and an amount kept with no - // invoice behind it is money we cannot account for. - $remainder = $owedNet > 0 - ? $this->invoices->forService( - customer: $original->customer, - lines: [[ - 'description' => __('withdrawal.invoice_line', [ - 'plan' => ucfirst((string) $subscription->plan), - ]), - 'details' => [__('withdrawal.invoice_detail', [ - 'days' => WithdrawalRight::deliveredDays($subscription, $at), - 'term' => WithdrawalRight::termDays($subscription), - ])], - 'quantity_milli' => 1000, - 'unit' => '', - 'unit_net_cents' => $owedNet, - ]], - currency: (string) $original->currency, - ) - : null; - - // The refund IS the difference between the two documents. Computing - // it separately would produce a second figure that has to agree with - // the first, and one day would not. - return $this->sendBack( - $subscription, - max(0, (int) $original->gross_cents - (int) ($remainder?->gross_cents ?? 0)), - $at, - ); + // 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); @@ -334,12 +305,6 @@ class WithdrawContract ->first(); } - /** Gross for a net figure, at this customer's own treatment. */ - private function grossFor(Subscription $subscription, int $netCents): int - { - return TaxTreatment::for($subscription->customer)->grossCents($netCents); - } - /** * The refund did not go out. Said on the contract, not only in a log. * @@ -377,11 +342,12 @@ class WithdrawContract at: $at, extra: ['withdrawal' => [ 'channel' => $subscription->withdrawal_channel, + // Nothing is charged for them — the refund is the whole amount — + // but how long the service actually ran is a fact about this + // contract, and the register is where facts about it are kept. 'delivered_days' => WithdrawalRight::deliveredDays($subscription, $at), 'term_days' => WithdrawalRight::termDays($subscription), - 'owed_net_cents' => WithdrawalRight::owedNetCents($subscription, $at), 'refunded_gross_cents' => $refundedGross, - 'immediate_start_consent_at' => $subscription->immediate_start_consent_at?->toIso8601String(), ]], chargedGrossCents: -$refundedGross, eventKey: 'withdrawal:'.$subscription->uuid, diff --git a/app/Console/Commands/RepriceStripeSubscriptions.php b/app/Console/Commands/RepriceStripeSubscriptions.php new file mode 100644 index 0000000..2031963 --- /dev/null +++ b/app/Console/Commands/RepriceStripeSubscriptions.php @@ -0,0 +1,149 @@ +option('dry-run'); + + if (! $dryRun && ! $stripe->isConfigured()) { + $this->error('Stripe is not configured (STRIPE_SECRET is empty). Nothing was moved.'); + + return self::FAILURE; + } + + // Active contracts Stripe actually bills. A cancelled one is not billed + // again, and a granted one has no Stripe subscription at all — moving + // either would be housekeeping on something that charges nobody. + $contracts = Subscription::query() + ->whereNotNull('stripe_subscription_id') + ->where('status', 'active') + ->orderBy('id') + ->limit((int) $this->option('limit')) + ->get(); + + $packages = 0; + $items = 0; + $failed = 0; + + foreach ($contracts as $contract) { + $target = $this->packagePrice($contract); + + if ($target !== null && $contract->stripe_price_id !== $target) { + $this->line(sprintf( + ' package %s %s → %s', + $contract->uuid, + $contract->stripe_price_id ?? '(unknown)', + $target, + )); + $packages++; + + // PRORATE_NONE: the term in progress is paid for. See the class + // comment — this must never raise a charge for days already + // served at the old figure. + if (! $dryRun && ! $move($contract, StripeClient::PRORATE_NONE)) { + $failed++; + } + } + + foreach ($modules->reprice($contract, $dryRun) as $item) { + $this->line(sprintf( + ' module %s %s %s → %s', + $contract->uuid, + $item['addon'], + $item['from'] ?? '(none)', + $item['to'] ?? '(to be created)', + )); + $items++; + } + } + + $this->newLine(); + + if ($packages === 0 && $items === 0) { + $this->info('Every live contract is already on the price the catalogue sells.'); + + return self::SUCCESS; + } + + $this->info($dryRun + ? "{$packages} package(s) and {$items} module item(s) would move. Run without --dry-run to move them." + : "{$packages} package(s) and {$items} module item(s) moved."); + + if ($failed > 0) { + // Parked on the contract by the action and retried hourly by + // clupilot:sync-stripe-subscriptions, so this is a warning and not a + // failure — but somebody watching the output has to be told. + $this->warn("{$failed} package move(s) did not reach Stripe and are parked for the hourly retry."); + } + + return self::SUCCESS; + } + + /** + * The Stripe Price the catalogue sells this contract's package on now. + * + * By plan VERSION and term, exactly as MoveStripeSubscriptionPrice resolves + * it — resolving it by plan name would hand a grandfathered contract today's + * terms. Null where the version was never synced, which is a contract this + * command has nothing to say about. + */ + private function packagePrice(Subscription $subscription): ?string + { + $priceId = PlanPrice::query() + ->where('plan_version_id', $subscription->plan_version_id) + ->where('term', $subscription->term) + ->value('stripe_price_id'); + + return is_string($priceId) && $priceId !== '' ? $priceId : null; + } +} diff --git a/app/Console/Commands/SyncStripeCatalogue.php b/app/Console/Commands/SyncStripeCatalogue.php index 3d4f8de..09e5e8d 100644 --- a/app/Console/Commands/SyncStripeCatalogue.php +++ b/app/Console/Commands/SyncStripeCatalogue.php @@ -5,9 +5,11 @@ namespace App\Console\Commands; use App\Models\PlanFamily; use App\Models\PlanPrice; use App\Models\PlanVersion; +use App\Models\StripePlanPrice; use App\Models\Subscription; use App\Services\Billing\AddonCatalogue; use App\Services\Billing\AddonPrices; +use App\Services\Billing\TaxTreatment; use App\Services\Stripe\StripeClient; use Illuminate\Console\Command; @@ -19,10 +21,28 @@ use Illuminate\Console\Command; * invoice numbering — so it needs to know what it is billing for. It does not * need to know how big the VM is, and it is not asked. * - * Idempotent by construction: a row that already carries an id is skipped. That - * matters more here than usual, because 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 which a customer is on. + * **What it pushes is the GROSS figure.** The catalogue's `amount_cents` is net + * and stays net — it is frozen onto every contract and PlanChange prorates + * against it — but the amount Stripe takes is that figure at the till, so the + * charge equals the price on the website and equals the total on the document. + * Pushing the net was the defect: a customer quoted 214,80 € paid 179,00 € and + * was then invoiced 179,00 € plus 35,80 € VAT that nobody had collected. + * + * Stripe's `automatic_tax` is deliberately NOT used. TaxTreatment is the single + * tax authority here; a second rate computed by Stripe would charge a German + * consumer 19 % while our document said 20 %. + * + * Idempotent by construction, and safe to re-run after a rate change: a row + * whose live Price already charges the right figure is skipped, and one that + * does not gets a NEW Price with the old one archived. That matters more here + * than usual, because a Stripe Price cannot be edited, so a run that minted + * duplicates would leave two live prices for one plan and no way to tell which a + * customer is on. + * + * It does not move anybody. Archiving a Price stops it being SOLD and leaves + * every subscription already on it exactly where it was — moving those is + * stripe:reprice-subscriptions, which is a separate decision because it touches + * live contracts. * * 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 @@ -84,38 +104,7 @@ class SyncStripeCatalogue extends Command foreach ($published as $version) { foreach ($version->prices as $price) { - if ($price->stripe_price_id !== null) { - continue; - } - - $this->line(sprintf( - ' price %s v%d %s %d %s', - $family->key, $version->version, $price->term, $price->amount_cents, $price->currency, - )); - $created++; - - if ($dryRun || $productId === null) { - continue; - } - - $priceId = $stripe->createPrice( - productId: $productId, - amountCents: $price->amount_cents, - currency: $price->currency, - interval: $price->term === 'yearly' ? 'year' : 'month', - metadata: [ - 'plan_family' => $family->key, - 'plan_version' => (string) $version->version, - 'plan_version_id' => (string) $version->id, - 'plan_price_id' => (string) $price->id, - ], - idempotencyKey: "clupilot-price-{$price->id}", - ); - - // Written straight through the query builder: stripe_price_id - // is not part of what publication froze, and the model would - // otherwise have to be re-read first. - PlanPrice::query()->whereKey($price->id)->update(['stripe_price_id' => $priceId]); + $created += $this->syncPrice($stripe, $family, $version, $price, $productId, $dryRun); } } } @@ -137,6 +126,103 @@ class SyncStripeCatalogue extends Command return self::SUCCESS; } + /** + * The Stripe Price one priced catalogue row is sold on, brought into step. + * + * Three outcomes, and which one happens is decided by the CHARGED figure — + * the row's net at today's rate — and never by whether an id happens to be + * stored: + * + * - the live Price already charges it, and the row points at that Price: + * nothing to do, which is what makes a second run free; + * - a Price for that figure exists but is archived or is not the one the row + * points at — a rate moved and moved back, or a run died between Stripe + * creating the Price and us storing its id: it is brought back rather than + * minted a second time; + * - nothing charges it: a new Price is created, the row is repointed, and + * whatever it was pointing at is archived in Stripe and here. + * + * Archiving stops a Price being offered and leaves every subscription on it + * untouched, which is exactly what is wanted: those are moved deliberately, + * by stripe:reprice-subscriptions. + * + * @return int how many objects this row changed, for the run's own count + */ + private function syncPrice( + StripeClient $stripe, + PlanFamily $family, + PlanVersion $version, + PlanPrice $price, + ?string $productId, + bool $dryRun, + ): int { + $charged = TaxTreatment::chargedCents((int) $price->amount_cents); + + $existing = StripePlanPrice::query() + ->where('plan_price_id', $price->id) + ->where('charged_cents', $charged) + ->first(); + + if ($existing !== null && $existing->isLive() && $price->stripe_price_id === $existing->stripe_price_id) { + return 0; + } + + $this->line(sprintf( + ' price %s v%d %s %d %s net → %d %s charged%s', + $family->key, $version->version, $price->term, + $price->amount_cents, $price->currency, + $charged, $price->currency, + $existing === null ? '' : ' (Price already exists)', + )); + + if ($dryRun || $productId === null) { + return 1; + } + + $priceId = $existing?->stripe_price_id ?? $stripe->createPrice( + productId: $productId, + amountCents: $charged, + currency: $price->currency, + interval: $price->term === 'yearly' ? 'year' : 'month', + metadata: [ + 'plan_family' => $family->key, + 'plan_version' => (string) $version->version, + 'plan_version_id' => (string) $version->id, + 'plan_price_id' => (string) $price->id, + ], + // The CHARGED amount is part of the key. Without it a run after a + // rate change would replay the Price minted at the old figure, and + // Stripe would hand back the very object this is replacing. + idempotencyKey: "clupilot-price-{$price->id}-{$charged}", + ); + + StripePlanPrice::query()->updateOrCreate( + ['plan_price_id' => $price->id, 'charged_cents' => $charged], + ['stripe_price_id' => $priceId, 'archived_at' => null], + ); + + // Everything else this row was ever sold on stops being offered — after + // the replacement exists, never before, so a failure in between leaves + // the old Price selling rather than nothing at all. + $superseded = StripePlanPrice::query() + ->where('plan_price_id', $price->id) + ->where('charged_cents', '!=', $charged) + ->whereNull('archived_at') + ->get(); + + foreach ($superseded as $old) { + $stripe->archivePrice((string) $old->stripe_price_id); + $old->update(['archived_at' => now()]); + } + + // Written straight through the query builder: stripe_price_id is not + // part of what publication froze, and the model would otherwise have to + // be re-read first. + PlanPrice::query()->whereKey($price->id)->update(['stripe_price_id' => $priceId]); + + return 1; + } + /** * A Product and two Prices — monthly and yearly — for every module on sale. * @@ -166,14 +252,20 @@ class SyncStripeCatalogue extends Command 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) { + foreach ([Subscription::TERM_MONTHLY, Subscription::TERM_YEARLY] as $term) { + // Asked of the CHARGED figure, so a rate change is seen as a + // Price that has to be replaced rather than as one that is + // already there. + if ($prices->liveFor($key, $monthly, $currency, $term) !== null) { continue; } - $this->line(sprintf(' module %s %s %d %s', $key, $term, $amount, $currency)); + $this->line(sprintf( + ' module %s %s %d %s net → %d %s charged', + $key, $term, + AddonPrices::termNetCents($monthly, $term), $currency, + AddonPrices::chargedCents($monthly, $term), $currency, + )); $created++; if ($dryRun) { diff --git a/app/Http/Controllers/LandingController.php b/app/Http/Controllers/LandingController.php index 8d9f78c..059fb48 100644 --- a/app/Http/Controllers/LandingController.php +++ b/app/Http/Controllers/LandingController.php @@ -6,6 +6,7 @@ use App\Models\Subscription; use App\Services\Billing\AddonCatalogue; use App\Services\Billing\CustomDomainAccess; use App\Services\Billing\PlanCatalogue; +use App\Services\Billing\TaxTreatment; use App\Services\Provisioning\HostCapacity; use App\Support\CompanyProfile; use App\Support\ProvisioningSettings; @@ -556,20 +557,23 @@ class LandingController extends Controller * "179,00". */ /** - * A net figure with the domestic VAT put back on. + * A net figure as it is actually charged. * - * The same rate an invoice would use, read from the same place - * (App\Services\Billing\TaxTreatment does too) — a percentage written - * into the marketing page would be the second source that makes the sheet - * and the invoice disagree. + * Handed straight to TaxTreatment rather than worked out here. It is the same + * call the Stripe catalogue makes when it decides what a Price takes and the + * same one an invoice makes when it decides what a document totals to, so the + * figure on this page, the figure on the card statement and the figure on the + * document cannot drift apart. It used to be a multiplication of its own, + * which is a second source of the same number and therefore a second answer + * waiting to happen. * - * Cross-border reverse charge is deliberately not modelled here: it depends - * on a VERIFIED VAT id, which a visitor to a public page does not have. The - * page quotes the domestic price and the invoice applies the treatment. + * Cross-border reverse charge is deliberately not modelled here, and it makes + * no difference to the price: a reverse-charge business is charged this + * figure like everybody else and their invoice states all of it as net. */ private function gross(int $netCents): int { - return (int) round($netCents * (1 + CompanyProfile::taxRate() / 100)); + return TaxTreatment::chargedCents($netCents); } private function money(int $cents, string $currency): string diff --git a/app/Http/Controllers/StripeWebhookController.php b/app/Http/Controllers/StripeWebhookController.php index e5fd3c5..5f4f87c 100644 --- a/app/Http/Controllers/StripeWebhookController.php +++ b/app/Http/Controllers/StripeWebhookController.php @@ -89,11 +89,13 @@ class StripeWebhookController extends Controller 'stripe_payment_intent_id' => is_string($object['payment_intent'] ?? null) ? $object['payment_intent'] : null, 'stripe_invoice_id' => is_string($object['invoice'] ?? null) ? $object['invoice'] : null, // The consumer's express request that the service begin inside the - // fourteen-day withdrawal window, having been told they would then - // owe the pro-rata share of whatever had been delivered (FAGG §16). - // Set on the session metadata by the checkout; anything other than - // the exact string is a no, because a consent that can be produced - // by a typo is not a consent. + // fourteen-day withdrawal window. Set on the session metadata by the + // checkout; anything other than the exact string is a no, because a + // consent that can be produced by a typo is not a consent. + // + // Nothing decides anything on it now — a withdrawal refunds the + // whole amount either way — so it is carried as a record of what was + // agreed rather than as a gate. See App\Models\Order. 'immediate_start_consent' => ($meta['immediate_start'] ?? null) === '1', 'plan' => $meta['plan'] ?? 'start', // What the customer was actually shown. Absent on a session created diff --git a/app/Livewire/Admin/RecordWithdrawal.php b/app/Livewire/Admin/RecordWithdrawal.php index 5268b09..f92c1d6 100644 --- a/app/Livewire/Admin/RecordWithdrawal.php +++ b/app/Livewire/Admin/RecordWithdrawal.php @@ -22,10 +22,9 @@ use RuntimeException; * So it goes through the SAME action as the portal, with the channel recorded * and the operator's name on it. Nothing is decided here that is not decided * there: a business customer is refused, an expired window is refused, and the - * pro-rata share is worked out by the same arithmetic. An operator cannot - * override any of it, which is the point — a withdrawal an operator could - * approve out of the window would be a refund with no legal basis and no - * document that could justify it. + * refund is the whole amount either way. An operator cannot override any of it, + * which is the point — a withdrawal an operator could approve out of the window + * would be a refund with no legal basis and no document that could justify it. * * `customers.manage` rather than `customers.grant_plan`: this is not a gift an * operator decides to make, it is a declaration the customer has already made @@ -98,7 +97,9 @@ class RecordWithdrawal extends ModalComponent return view('livewire.admin.record-withdrawal', [ 'right' => WithdrawalRight::for($contract), - 'owedNetCents' => $contract === null ? 0 : WithdrawalRight::owedNetCents($contract), + // How long the cloud has been running. Nothing is charged for it — + // the refund is the whole amount — but it is the first thing an + // operator is asked on the telephone. 'deliveredDays' => $contract === null ? 0 : WithdrawalRight::deliveredDays($contract), 'termDays' => $contract === null ? 0 : WithdrawalRight::termDays($contract), ]); diff --git a/app/Livewire/ConfirmWithdraw.php b/app/Livewire/ConfirmWithdraw.php index 7292c13..0e75397 100644 --- a/app/Livewire/ConfirmWithdraw.php +++ b/app/Livewire/ConfirmWithdraw.php @@ -35,18 +35,12 @@ class ConfirmWithdraw extends ModalComponent public function render() { - $contract = app(CustomDomainAccess::class)->contractOf($this->customer()); - $right = WithdrawalRight::for($contract); + $right = WithdrawalRight::for(app(CustomDomainAccess::class)->contractOf($this->customer())); - return view('livewire.confirm-withdraw', [ - 'right' => $right, - // What they would owe for the days the cloud has already run, and - // therefore what does NOT come back. Zero where the express request - // to start at once was never given — then the whole amount returns, - // and the dialog says so. - 'owedNetCents' => $contract === null ? 0 : WithdrawalRight::owedNetCents($contract), - 'deliveredDays' => $contract === null ? 0 : WithdrawalRight::deliveredDays($contract), - 'termDays' => $contract === null ? 0 : WithdrawalRight::termDays($contract), - ]); + // No figures to read out any more: the refund is the whole amount, so + // there is one sentence and nothing to work out from the contract. The + // right itself is still resolved here rather than handed in from the + // page, because markup is not evidence. + return view('livewire.confirm-withdraw', ['right' => $right]); } } diff --git a/app/Models/Order.php b/app/Models/Order.php index 40b870c..0c78911 100644 --- a/app/Models/Order.php +++ b/app/Models/Order.php @@ -32,9 +32,14 @@ class Order extends Model implements ProvisioningSubject 'amount_cents' => 'integer', 'plan_version_id' => 'integer', // When the consumer expressly asked for the service to begin inside - // the withdrawal window, having been told they would owe the - // pro-rata share of whatever had been delivered if they withdrew. - // Null means they were never asked, which is a refusal, not a yes. + // the withdrawal window. + // + // DEAD as a gate, kept as a record. It used to decide whether a + // withdrawing consumer owed the pro-rata value of the days already + // delivered; the owner has decided they owe nothing and get the whole + // amount back, so nothing reads this to make a decision any more. It + // stays because it is evidence of what the customer was shown and + // agreed to at the checkout. 'immediate_start_consent_at' => 'datetime', ]; } @@ -71,12 +76,39 @@ class Order extends Model implements ProvisioningSubject /** * Net is what is stored; gross is what gets charged — and how much VAT that * is depends on the customer, not on a global setting. + * + * Only ever asked of a PENDING cart line, whose `amount_cents` is a net + * catalogue figure. An order that has been through a Stripe checkout already + * holds what was taken; see chargedCents() below, which is what the invoice + * uses and which does not double the tax on such an order. */ public function grossCents(): int { return $this->taxTreatment()->grossCents($this->amount_cents); } + /** + * What this order actually cost the customer, tax included. + * + * `amount_cents` means two things, and this is the one place that resolves + * which. An order created from a Stripe checkout carries the session's + * `amount_total` — the gross that was taken from the card, and the catalogue + * is pushed to Stripe gross, so it already includes the VAT. An order that + * never went through a checkout (a grant, a line in the portal cart) carries + * the catalogue's NET figure, and what a checkout would have taken for it is + * that figure at the till. + * + * Keyed on `stripe_event_id` for exactly the reason OpenSubscription keys the + * register's charged amount on it, and not on the amount being non-zero: a + * fully discounted checkout charges nothing and is still a checkout. + */ + public function chargedCents(): int + { + return $this->stripe_event_id !== null + ? (int) $this->amount_cents + : TaxTreatment::chargedCents((int) $this->amount_cents); + } + public function taxTreatment(): TaxTreatment { return TaxTreatment::for($this->customer); diff --git a/app/Models/StripeAddonPrice.php b/app/Models/StripeAddonPrice.php index 40eaaa8..242ceb0 100644 --- a/app/Models/StripeAddonPrice.php +++ b/app/Models/StripeAddonPrice.php @@ -12,6 +12,11 @@ use Illuminate\Database\Eloquent\Model; * does for packages. See the migration for why a module needs more than one * Price: a booking is frozen at the price it was booked at, and a Stripe Price * cannot be edited. + * + * `amount_cents` is what the Price CHARGES, which is gross; `net_cents` is the + * catalogue figure it was formed from, which is what a booking is frozen at. A + * Price superseded by a rate change is archived rather than deleted, because a + * contract may still be billing on it. */ class StripeAddonPrice extends Model { @@ -19,6 +24,16 @@ class StripeAddonPrice extends Model protected function casts(): array { - return ['amount_cents' => 'integer']; + return [ + 'amount_cents' => 'integer', + 'net_cents' => 'integer', + 'archived_at' => 'datetime', + ]; + } + + /** Is this the Price a new booking of this module would be put on? */ + public function isLive(): bool + { + return $this->archived_at === null; } } diff --git a/app/Models/StripePlanPrice.php b/app/Models/StripePlanPrice.php new file mode 100644 index 0000000..6a5854f --- /dev/null +++ b/app/Models/StripePlanPrice.php @@ -0,0 +1,40 @@ + 'integer', + 'archived_at' => 'datetime', + ]; + } + + public function planPrice(): BelongsTo + { + return $this->belongsTo(PlanPrice::class); + } + + /** Is this the Price the catalogue is selling on right now? */ + public function isLive(): bool + { + return $this->archived_at === null; + } +} diff --git a/app/Models/Subscription.php b/app/Models/Subscription.php index 5a3f7b6..1ba5cec 100644 --- a/app/Models/Subscription.php +++ b/app/Models/Subscription.php @@ -67,6 +67,9 @@ class Subscription extends Model // happened inside them. See App\Services\Billing\WithdrawalRight, // which is the one place that decides whether the window is open. 'withdrawal_ends_at' => 'datetime', + // DEAD as a gate, kept as a record — see Order, which carries the + // same column for the same reason. A withdrawing consumer now gets + // the whole amount back whatever this says. 'immediate_start_consent_at' => 'datetime', 'withdrawn_at' => 'datetime', 'withdrawal_refund_cents' => 'integer', diff --git a/app/Services/Billing/AddonPrices.php b/app/Services/Billing/AddonPrices.php index c397151..62f1312 100644 --- a/app/Services/Billing/AddonPrices.php +++ b/app/Services/Billing/AddonPrices.php @@ -28,11 +28,39 @@ use Illuminate\Database\UniqueConstraintViolationException; * discounted grant, or a customer grandfathered on a figure the catalogue left * behind years ago — and refusing to bill those would be the same bug in a new * place. Stripe's idempotency key makes the on-demand path safe to repeat. + * + * **What the Price charges is GROSS.** `amount_cents` on the row is what Stripe + * takes, and it is the catalogue's net figure with the domestic rate on it — + * because the figure on the price sheet is the figure charged, for everybody. + * `net_cents` beside it is the catalogue figure it was formed from, which is what + * a booking is frozen at and what identifies the Price when the rate moves. A + * Price at the wrong figure is archived rather than edited: Stripe does not allow + * editing one, and a contract may still be billing on it. */ final class AddonPrices { public function __construct(private readonly StripeClient $stripe) {} + /** + * The net a whole term of this module costs. + * + * A module is priced per month; a yearly contract bills a year of it at + * once, in step with the package beside it. Twelve months exactly, and + * grossed ONCE afterwards rather than twelve times — twelve rounded gross + * months can differ from a rounded gross year by a cent, and the year is the + * figure the customer actually pays. + */ + public static function termNetCents(int $monthlyNetCents, string $term): int + { + return $term === Subscription::TERM_YEARLY ? $monthlyNetCents * 12 : $monthlyNetCents; + } + + /** What Stripe is asked to take for a term of this module. */ + public static function chargedCents(int $monthlyNetCents, string $term): int + { + return TaxTreatment::chargedCents(self::termNetCents($monthlyNetCents, $term)); + } + /** * The Stripe Price for this module at this money on this term, creating it * if Stripe has never been told about it. @@ -50,15 +78,24 @@ final class AddonPrices $interval = $term === Subscription::TERM_YEARLY ? 'year' : 'month'; $currency = strtoupper($currency); - // A module is priced per month; a yearly contract bills a year of it at - // once, in step with the package beside it. Twelve months exactly, so - // the figure the customer was shown times twelve is the figure charged. - $amount = $interval === 'year' ? $unitNetCents * 12 : $unitNetCents; + $netCents = self::termNetCents($unitNetCents, $term); + $amount = TaxTreatment::chargedCents($netCents); - $existing = $this->find($addonKey, $amount, $currency, $interval); + $existing = StripeAddonPrice::query() + ->where('addon_key', $addonKey) + ->where('amount_cents', $amount) + ->where('currency', $currency) + ->where('interval', $interval) + ->first(); if ($existing !== null) { - return $existing; + // Brought back rather than minted again. A rate that moves and then + // moves back — or a run interrupted between Stripe and us — must not + // leave a second Stripe Price for one figure. + $existing->update(['archived_at' => null, 'net_cents' => $netCents]); + $this->archiveSuperseded($addonKey, $netCents, $currency, $interval, $amount); + + return (string) $existing->stripe_price_id; } $productId = $this->product($addonKey); @@ -75,16 +112,40 @@ final class AddonPrices ], // 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. + // attempt rather than a second one at the same money. The amount is + // the CHARGED one, so the move from net to gross does not replay the + // net Price the old key was minted under. idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}", ); - $this->remember($addonKey, $amount, $currency, $interval, $productId, $priceId); + $this->remember($addonKey, $amount, $netCents, $currency, $interval, $productId, $priceId); + $this->archiveSuperseded($addonKey, $netCents, $currency, $interval, $amount); return $priceId; } - /** The Price we already have for this combination, or null. */ + /** + * The live Price for a module at a catalogue figure, or null. + * + * Asked with the MONTHLY net the booking is frozen at, because that is what + * a caller has: the charged figure is worked out here, from the one place + * that knows the rate. + */ + public function liveFor(string $addonKey, int $unitNetCents, string $currency, string $term): ?string + { + if ($unitNetCents <= 0) { + return null; + } + + return $this->find( + $addonKey, + self::chargedCents($unitNetCents, $term), + $currency, + $term === Subscription::TERM_YEARLY ? 'year' : 'month', + ); + } + + /** The live Price we already have for this charged figure, or null. */ public function find(string $addonKey, int $amountCents, string $currency, string $interval): ?string { $id = StripeAddonPrice::query() @@ -92,11 +153,47 @@ final class AddonPrices ->where('amount_cents', $amountCents) ->where('currency', strtoupper($currency)) ->where('interval', $interval) + ->whereNull('archived_at') ->value('stripe_price_id'); return is_string($id) && $id !== '' ? $id : null; } + /** + * Stop offering every other Price for this module at this catalogue figure. + * + * Only ones formed from the SAME net: a customer grandfathered at ten euros + * keeps their own Price, and archiving that would leave their subscription + * billing on something Stripe no longer sells. What is archived here is the + * Price for today's figure at yesterday's rate — the net one this whole + * change replaces. + * + * Archived in Stripe as well as here. The Price object stays, as Stripe + * requires, because a contract may still be billing on it until + * stripe:reprice-subscriptions has moved it. + */ + private function archiveSuperseded( + string $addonKey, + int $netCents, + string $currency, + string $interval, + int $keepAmountCents, + ): void { + $superseded = StripeAddonPrice::query() + ->where('addon_key', $addonKey) + ->where('net_cents', $netCents) + ->where('currency', $currency) + ->where('interval', $interval) + ->where('amount_cents', '!=', $keepAmountCents) + ->whereNull('archived_at') + ->get(); + + foreach ($superseded as $price) { + $this->stripe->archivePrice((string) $price->stripe_price_id); + $price->update(['archived_at' => now()]); + } + } + /** * The Stripe Product every Price for this module hangs off. * @@ -124,6 +221,7 @@ final class AddonPrices private function remember( string $addonKey, int $amountCents, + int $netCents, string $currency, string $interval, string $productId, @@ -133,6 +231,7 @@ final class AddonPrices StripeAddonPrice::create([ 'addon_key' => $addonKey, 'amount_cents' => $amountCents, + 'net_cents' => $netCents, 'currency' => $currency, 'interval' => $interval, 'stripe_product_id' => $productId, diff --git a/app/Services/Billing/InvoiceMath.php b/app/Services/Billing/InvoiceMath.php index e2d279e..d7dcc3d 100644 --- a/app/Services/Billing/InvoiceMath.php +++ b/app/Services/Billing/InvoiceMath.php @@ -62,6 +62,23 @@ final class InvoiceMath return (int) round($netCents * $rateBasisPoints / 10000); } + /** + * The VAT already contained in a figure that includes it. + * + * The exact inverse of tax() for every figure tax() can produce, and — this + * is the part that matters — it works for the figures it CANNOT produce as + * well. At 20 % one integer in six is not the gross of any whole-cent net + * (there is no net whose gross is 3 cents), and Stripe hands us exactly such + * figures whenever it prorates a term by days. Extracting the tax from the + * gross and taking the rest as net therefore always adds back up to the + * amount that was charged, which putting a rate on top of a rounded net + * would not. + */ + public static function taxInGross(int $grossCents, int $rateBasisPoints): int + { + return (int) round($grossCents * $rateBasisPoints / (10000 + $rateBasisPoints)); + } + /** * Every figure the document shows, from the lines and one adjustment. * @@ -115,6 +132,115 @@ final class InvoiceMath ]; } + /** + * A document whose lines state what was CHARGED, turned into the net lines + * it prints and the totals it adds up to. + * + * Everything this platform sells is priced gross: the figure on the price + * sheet is the figure on the Stripe Price and the figure taken from the + * card, and the document's job is to say how that one amount divides into + * net and VAT — not to rebuild it from a net figure and a rate. Rebuilding + * it is what produced an invoice for 214,80 € against a charge of 179,00 €, + * and it cannot be made to land on the cent for a prorated amount. + * + * So the split happens ONCE, on the total, exactly as totals() rounds once: + * the VAT is extracted from the sum of the lines and the rest is the net. The + * per-line net figures are then shares of that net, with the odd cent given + * to the largest line, so the column on the page still adds up to the + * subtotal underneath it. + * + * A reverse-charge customer comes through here at a rate of zero, and the + * whole charged amount becomes net — which is what their document has to + * say, since they paid the same total as everybody else. + * + * @param array> $lines `unit_net_cents` holding the amount charged + * @return array{lines: array>, totals: array} + */ + public static function fromCharged(array $lines, int $rateBasisPoints): array + { + $lines = array_values($lines); + + $charged = array_map( + fn (array $line) => self::lineNet((int) $line['unit_net_cents'], (int) $line['quantity_milli']), + $lines, + ); + + $chargedTotal = array_sum($charged); + $tax = self::taxInGross($chargedTotal, $rateBasisPoints); + $net = $chargedTotal - $tax; + + $shares = self::shareOut($charged, $chargedTotal, $net); + + foreach ($lines as $index => $line) { + $quantity = (int) $line['quantity_milli']; + $units = intdiv($quantity, 1000); + + // The quantity survives only where the net share still divides by it + // exactly. Where it does not, the line collapses to one of itself at + // the whole amount — a unit price the reader could multiply into a + // different answer than the line beside it is worse than no unit + // price at all, and it is the same rule StripeInvoiceLines applies + // to Stripe's own amounts. + $divides = $units >= 1 && $units * 1000 === $quantity && $shares[$index] % $units === 0; + + $lines[$index]['quantity_milli'] = $divides ? $quantity : 1000; + $lines[$index]['unit_net_cents'] = $divides ? intdiv($shares[$index], $units) : $shares[$index]; + } + + return [ + 'lines' => $lines, + 'totals' => [ + 'subtotal' => $net, + 'lines_net' => $net, + // No discount is expressible on a charged document: the amount + // was taken as it stands, and a discount here would describe a + // reduction that never reached the customer's card. + 'adjustment' => 0, + 'net' => $net, + 'tax' => $tax, + 'gross' => $chargedTotal, + ], + ]; + } + + /** + * `$total` divided between lines in proportion to `$parts`, to the cent. + * + * The odd cent goes to the biggest line, because it is the one where a cent + * is least visible and because it has to go somewhere: shares that are each + * rounded on their own do not add up to the sum they are shares of. + * + * @param array $parts + * @return array + */ + private static function shareOut(array $parts, int $partsTotal, int $total): array + { + if ($parts === []) { + return []; + } + + if ($partsTotal === 0) { + // Nothing to divide in proportion to. Everything on the first line + // rather than a division by zero; a zero-total document has one line + // in practice anyway. + return array_map(fn (int $index) => $index === 0 ? $total : 0, array_keys($parts)); + } + + $shares = array_map(fn (int $part) => (int) round($part * $total / $partsTotal), $parts); + + $largest = 0; + + foreach ($parts as $index => $part) { + if (abs($part) > abs($parts[$largest])) { + $largest = $index; + } + } + + $shares[$largest] += $total - array_sum($shares); + + return $shares; + } + /** "1.234,56" — Austrian formatting, done here so no view invents its own. */ public static function money(int $cents): string { diff --git a/app/Services/Billing/IssueInvoice.php b/app/Services/Billing/IssueInvoice.php index c59a107..dcb5bee 100644 --- a/app/Services/Billing/IssueInvoice.php +++ b/app/Services/Billing/IssueInvoice.php @@ -27,6 +27,22 @@ use RuntimeException; * * The number and the invoice commit together or not at all. A number taken and * then lost to a failure is a gap in a series that must not have one. + * + * ## Two kinds of line, and why + * + * Everything sold from the catalogue is priced GROSS: the price sheet, the + * Stripe Price and the card charge are one figure. A document for such a sale + * therefore starts from what was CHARGED and divides it — see + * InvoiceMath::fromCharged() — because a document that rebuilds the total from a + * net figure and a rate states a sum nobody was ever charged. That was the whole + * defect: 179,00 € taken and 214,80 € invoiced. + * + * A service invoice an operator types is the other kind. Nothing has been + * charged yet, the figures are what was agreed on the telephone, and they are + * entered net with the VAT put on top exactly as before. + * + * The two never mix on one document, and which one a method produces is stated + * on the method. */ final class IssueInvoice { @@ -41,6 +57,11 @@ final class IssueInvoice * money. Each order becomes a line, which is also what was asked for — * every add-on listed on its own. * + * Each line is what the order actually COST the customer — Order::chargedCents(), + * which is Stripe's own total where the order went through a checkout. It + * used to be `amount_cents` read as a net figure, and once the catalogue + * started charging gross that put the VAT on the document twice. + * * @param Collection $orders */ public function forOrders(Customer $customer, Collection $orders, ?InvoiceSeries $series = null): Invoice @@ -62,7 +83,7 @@ final class IssueInvoice ])), 'quantity_milli' => 1000, 'unit' => '', - 'unit_net_cents' => (int) $order->amount_cents, + 'unit_net_cents' => $order->chargedCents(), ])->all(); return $this->issue( @@ -71,6 +92,7 @@ final class IssueInvoice currency: strtoupper((string) ($orders->first()->currency ?: 'EUR')), series: $series, attributes: ['order_id' => $orders->first()->id], + linesAreCharged: true, ); } @@ -85,11 +107,12 @@ final class IssueInvoice * actually ordered. The document points at the CONTRACT instead, which is * what a renewal renews. * - * The line is the contract's own frozen price — `price_cents`, the - * catalogue's NET figure — and not Stripe's `amount_paid`, which is a gross - * total. What the customer is owed is what they agreed to; the register - * records beside it what was actually taken, and `stripe_invoice_id` on the - * row is how the two are reconciled afterwards. + * The line is what the cycle CHARGED. Where Stripe reported a total it is + * that total, to the cent; where it did not, it is the contract's own frozen + * net price at the till (TaxTreatment::chargedCents), which is the amount the + * Stripe Price for that contract carries. `price_cents` itself stays the + * catalogue's net figure and is not what a document totals to — the customer + * was charged the gross of it. * * **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 @@ -109,6 +132,7 @@ final class IssueInvoice Carbon $to, ?string $stripeInvoiceId = null, ?InvoiceSeries $series = null, + ?int $chargedCents = null, ): Invoice { $customer = $subscription->customer; @@ -130,7 +154,7 @@ final class IssueInvoice ])], 'quantity_milli' => 1000, 'unit' => '', - 'unit_net_cents' => (int) $subscription->price_cents, + 'unit_net_cents' => $chargedCents ?? TaxTreatment::chargedCents((int) $subscription->price_cents), ]]; return $this->issue( @@ -138,6 +162,7 @@ final class IssueInvoice lines: $lines, currency: strtoupper((string) ($subscription->currency ?: 'EUR')), series: $series, + linesAreCharged: true, attributes: [ 'subscription_id' => $subscription->id, // Left null on purpose. The contract's opening order is the @@ -170,10 +195,12 @@ final class IssueInvoice * 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. + * **Charged in, split here.** Our catalogue is pushed to Stripe as GROSS, so + * the line amounts that come back are what the customer actually paid, and + * the document divides that sum into net and VAT rather than adding a rate + * on top of it. That distinction is not cosmetic: Stripe prorates a term by + * days and hands back figures that are not the gross of any whole-cent net, + * and a document rebuilt from a net figure would miss those by a cent. * * @param array> $lines Stripe's `lines.data` * @param array $unnamed filled with the lines the catalogue could not name @@ -204,6 +231,7 @@ final class IssueInvoice lines: $built['lines'], currency: strtoupper((string) ($currency ?: $subscription->currency ?: 'EUR')), series: $series, + linesAreCharged: true, attributes: [ 'subscription_id' => $subscription->id, // Left null for the same reason as on a renewal: the contract's @@ -230,9 +258,12 @@ final class IssueInvoice * invoices would be a second numbering — and a series with a document * missing from it is worth nothing at an audit. * - * Lines arrive as a person typed them. Nothing here invents a price: the - * caller is the operator, and what they wrote is what the customer agreed - * to on the phone. + * Lines arrive as a person typed them, NET, with the VAT put on top — the + * one door into this class that still works that way, and deliberately. A + * catalogue sale has already been charged a gross figure that the document + * has to divide; a service invoice charges nothing yet, and the console page + * asks the operator for net amounts because that is what was agreed on the + * telephone. * * @param array, quantity_milli: int, unit?: string, unit_net_cents: int}> $lines */ @@ -324,7 +355,17 @@ final class IssueInvoice return $line; }, (array) ($snapshot['lines'] ?? [])); - $totals = InvoiceMath::totals($lines, null, $rate) + ['rate_basis_points' => $rate]; + // The original's own figures, negated — not recomputed from the lines. + // A document built from a charged amount holds a split that its own + // lines cannot be made to reproduce (the odd cent lives on one of them), + // so recomputing would take back a cent more or less than was invoiced, + // and a cancellation that does not cancel leaves that cent standing for + // ever. + $totals = array_map( + fn ($value) => -(int) $value, + array_diff_key((array) ($snapshot['totals'] ?? []), ['rate_basis_points' => null]), + ) + ['subtotal' => 0, 'lines_net' => 0, 'adjustment' => 0, 'net' => 0, 'tax' => 0, 'gross' => 0] + + ['rate_basis_points' => $rate]; $snapshot['lines'] = $lines; $snapshot['totals'] = $totals; @@ -356,19 +397,35 @@ final class IssueInvoice ); } + /** + * @param bool $linesAreCharged true when `unit_net_cents` holds what the + * customer was actually charged, tax included — + * every catalogue sale. False for the service + * invoice an operator types net. + */ private function issue( Customer $customer, array $lines, string $currency, ?InvoiceSeries $series, array $attributes, + bool $linesAreCharged = false, ): Invoice { $series ??= InvoiceSeries::query()->where('kind', 'invoice')->where('active', true)->firstOrFail(); $treatment = TaxTreatment::for($customer); - $rate = (int) round($treatment->rate * 10000); + $rate = $treatment->basisPoints(); - $totals = InvoiceMath::totals($lines, null, $rate) + ['rate_basis_points' => $rate]; + if ($linesAreCharged) { + // The charged figure is the fixed point: a reverse-charge customer + // paid the same total as everybody else, so at a rate of zero the + // whole of it becomes net rather than the document shrinking by the + // VAT that was in the price. + ['lines' => $lines, 'totals' => $totals] = InvoiceMath::fromCharged($lines, $rate); + $totals += ['rate_basis_points' => $rate]; + } else { + $totals = InvoiceMath::totals($lines, null, $rate) + ['rate_basis_points' => $rate]; + } $snapshot = [ 'issuer' => CompanyProfile::all(), diff --git a/app/Services/Billing/StripeInvoiceLines.php b/app/Services/Billing/StripeInvoiceLines.php index a20b4da..47fb6da 100644 --- a/app/Services/Billing/StripeInvoiceLines.php +++ b/app/Services/Billing/StripeInvoiceLines.php @@ -5,6 +5,7 @@ namespace App\Services\Billing; use App\Models\PlanFamily; use App\Models\PlanPrice; use App\Models\StripeAddonPrice; +use App\Models\StripePlanPrice; use Illuminate\Support\Carbon; /** @@ -28,10 +29,11 @@ use Illuminate\Support\Carbon; * line that was charged is worse than an ugly one: the total would still be * right and the reader would have no way to see what they had paid for. * - * Amounts are NET throughout, which is what our catalogue pushes to Stripe. Tax - * is added by IssueInvoice from the customer's own treatment — see TaxTreatment, - * and note that a reverse-charge customer pays no VAT at all, which no figure - * from Stripe would know. + * Amounts are what was CHARGED — our catalogue is pushed to Stripe gross, so + * every figure here already contains the VAT. IssueInvoice divides the sum + * according to the customer's own treatment rather than adding a rate to it; a + * reverse-charge customer paid the same total as everybody else and their + * document states the whole of it as net at 0 %. See TaxTreatment. */ final class StripeInvoiceLines { @@ -59,7 +61,9 @@ final class StripeInvoiceLines // 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. + // reader could multiply and get a different answer. InvoiceMath + // applies the same rule again once the amount has been split into + // net, which is the figure actually printed. $unit = intdiv($amount, $quantity); $divides = $unit * $quantity === $amount; @@ -99,9 +103,12 @@ final class StripeInvoiceLines { $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. + // A module, by the Price we minted for it. Archived Prices are in that + // table too and are looked up exactly like live ones — a contract keeps + // billing on the Price it was put on until it is deliberately moved, and + // a line nobody can name is the one failure this class exists to avoid. + // 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'); @@ -115,10 +122,23 @@ final class StripeInvoiceLines // 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() + // + // `plan_prices.stripe_price_id` only ever holds the Price on sale right + // now, so a contract still billing on a superseded one is found through + // `stripe_plan_prices`, which keeps every Price a row has ever had. + $planPrice = $priceId === null ? null : PlanPrice::query() ->where('stripe_price_id', $priceId) ->with('version.family') - ->first()?->version?->family; + ->first(); + + if ($planPrice === null && $priceId !== null) { + $planPrice = StripePlanPrice::query() + ->where('stripe_price_id', $priceId) + ->with('planPrice.version.family') + ->first()?->planPrice; + } + + $family = $planPrice?->version?->family; $family ??= is_string($metadata['plan_family'] ?? null) ? PlanFamily::query()->where('key', $metadata['plan_family'])->first() diff --git a/app/Services/Billing/TaxTreatment.php b/app/Services/Billing/TaxTreatment.php index a6341aa..20d5073 100644 --- a/app/Services/Billing/TaxTreatment.php +++ b/app/Services/Billing/TaxTreatment.php @@ -38,11 +38,29 @@ use App\Support\CompanyProfile; * that needs a maintained rate table per member state and a tax adviser's sign * off, not a guess in a config file — so those customers fall back to the * domestic rate, which over-collects rather than under-collects. + * + * ## The price at the till is the domestic gross, for everybody + * + * The treatment above decides how a DOCUMENT is split, and it decides nothing + * about the amount taken from the card. That amount is chargedCents(): the net + * catalogue figure with the domestic rate on it, and it is the same number for a + * consumer in Vienna and for a reverse-charge business in Rotterdam. The owner's + * rule is that the figure on the website is the figure charged, and a Stripe + * Price cannot ask who is buying — it carries one amount for everyone. + * + * So a reverse-charge customer pays that same total and their invoice states the + * whole of it as net at 0 %. That is a real commercial consequence and it is + * deliberate: see App\Services\Billing\IssueInvoice. + * + * Stripe's own `automatic_tax` is deliberately NOT enabled anywhere. This class + * is the single tax authority here, knowingly over-collecting on cross-border + * B2C rather than implementing OSS; letting Stripe compute a second rate would + * have it charge a German consumer 19 % while our document said 20 %. */ final readonly class TaxTreatment { /** Reverse charge applies inside the EU only. */ - private const EU_MEMBER_STATES = [ + public const EU_MEMBER_STATES = [ 'AT', 'BE', 'BG', 'CY', 'CZ', 'DE', 'DK', 'EE', 'EL', 'ES', 'FI', 'FR', 'HR', 'HU', 'IE', 'IT', 'LT', 'LU', 'LV', 'MT', 'NL', 'PL', 'PT', 'RO', 'SE', 'SI', 'SK', @@ -53,20 +71,47 @@ final readonly class TaxTreatment public bool $reverseCharge, ) {} + /** + * The seller's own rate, with nobody in particular in mind. + * + * From the Finance page, not from config: an operator can change the rate + * there and cannot change a config value, and two sources for one number is + * how an invoice ends up disagreeing with the checkout that produced it. + * CompanyProfile falls back to the config value until a rate has been saved. + * + * This is the ONE place the rate is read. Everything that has to form a + * gross figure — the price sheet, the Stripe catalogue, the module prices — + * goes through chargedCents() below rather than multiplying by a percentage + * of its own. + */ + public static function domestic(): self + { + return new self(CompanyProfile::taxRate() / 100, false); + } + + /** + * What a net catalogue figure is actually charged as. + * + * The number on the website, the amount on the Stripe Price and the total on + * the invoice are all this one, so it is computed once and here. It takes no + * customer on purpose: a Stripe Price carries a single amount and cannot ask + * who is at the checkout, and the owner's rule is that everybody pays the + * figure they were shown. + */ + public static function chargedCents(int $netCents): int + { + return self::domestic()->grossCents($netCents); + } + public static function for(?Customer $customer): self { - // From the Finance page, not from config: an operator can change the - // rate there and cannot change a config value, and two sources for one - // number is how an invoice ends up disagreeing with the checkout that - // produced it. CompanyProfile falls back to the config value until a - // rate has been saved. - $domestic = CompanyProfile::taxRate() / 100; + $domestic = self::domestic(); // Unverified is the normal state, and it must cost the customer nothing // more than the domestic rate they would pay anyway — but it must not // zero the VAT either. Otherwise typing "XX123" is a discount. if ($customer === null || ! $customer->hasVerifiedVatId()) { - return new self($domestic, false); + return $domestic; } // Asked of the recorded type, not of the number. Somebody who has said @@ -76,7 +121,7 @@ final readonly class TaxTreatment // here; an unrecorded type falls through and is treated exactly as it // was before this check existed (see the class comment). if ($customer->hasRecordedType() && ! $customer->isBusiness()) { - return new self($domestic, false); + return $domestic; } $vatId = $customer->normalisedVatId(); @@ -89,7 +134,7 @@ final readonly class TaxTreatment && $country !== $seller && preg_match('/^[A-Z]{2}[0-9A-Z]{8,12}$/', $vatId) === 1; - return $eligible ? new self(0.0, true) : new self($domestic, false); + return $eligible ? new self(0.0, true) : $domestic; } public function grossCents(int $netCents): int @@ -97,6 +142,19 @@ final readonly class TaxTreatment return (int) round($netCents * (1 + $this->rate)); } + /** + * The rate in the form the invoice arithmetic works in — 2000 for 20 %. + * + * Here rather than at each call site, because the conversion was written out + * by hand where an invoice was issued and nowhere else, and a second reader + * of `rate` doing its own multiplication is how two documents end up at two + * rates. + */ + public function basisPoints(): int + { + return (int) round($this->rate * 10000); + } + public function percentLabel(): string { return rtrim(rtrim(number_format($this->rate * 100, 1, ',', '.'), '0'), ','); diff --git a/app/Services/Billing/WithdrawalRight.php b/app/Services/Billing/WithdrawalRight.php index 8e1260d..464f897 100644 --- a/app/Services/Billing/WithdrawalRight.php +++ b/app/Services/Billing/WithdrawalRight.php @@ -16,29 +16,26 @@ use Illuminate\Support\Carbon; * customer is a business, and why `customers.customer_type` had to exist before * any of this could be written. * - * ## Why it is not simply "give it all back" + * ## It IS simply "give it all back" * - * A cloud is running from the moment it is paid for. FAGG §16 is the rule for - * exactly that case: where the consumer EXPRESSLY asked for the service to - * begin during the withdrawal period, and was told they would owe a - * proportionate amount if they withdrew, they owe the pro-rata value of what - * was actually performed up to the moment of withdrawal. Not the whole period — - * they withdrew — and not nothing either, because the machine really did run. + * FAGG §16 would let us keep the pro-rata value of the days the cloud actually + * ran, where the consumer expressly asked for the service to begin inside the + * window and was told what that would cost them. The owner has decided not to: + * a consumer who withdraws gets the WHOLE amount back, and a cancellation + * invoice is issued for the whole of it. * - * Both halves of that condition are required, and both are on record: the - * consent is a timestamp taken at the checkout, and its absence means the - * consumer owes NOTHING and the refund is the whole amount. That is the - * lawful reading and it is also the safe one: a seller who cannot prove the - * consent was given cannot charge for the days. + * That is a business decision and not a legal one, and it is more generous than + * the statute requires — which is why nothing turns on the express-start consent + * any more. It used to be the hinge the refund swung on; a full refund makes it + * irrelevant, and `immediate_start_consent_at` on the order and on the contract + * is now a record of what the customer ticked rather than a gate anything reads. * - * ## How the share is measured + * ## Days are still counted * * By whole days, over the term the customer actually paid for, and a started day - * counts as delivered. Days rather than seconds because the figure ends up on a - * document a person reads and has to be able to check with a calendar; a started - * day counts because the service was genuinely available on it. The share is - * taken of the NET price, and the VAT is put back on top by the document — never - * the other way round, for the reason InvoiceMath sets out. + * counts as delivered. Nothing is charged for them, but how long the service ran + * before a withdrawal is a fact about the contract, it goes into the proof + * register, and an operator recording a withdrawal by telephone is shown it. */ final readonly class WithdrawalRight { @@ -56,14 +53,12 @@ final readonly class WithdrawalRight public bool $open, /** Why not, in the sentence the customer is shown. Null while it is open. */ public ?string $refusal, - /** Did the consumer expressly ask for the service to start at once? */ - public bool $immediateStartConsented, ) {} public static function for(?Subscription $subscription): self { if ($subscription === null) { - return new self(false, null, null, false, __('withdrawal.refusal_no_contract'), false); + return new self(false, null, null, false, __('withdrawal.refusal_no_contract')); } $opensAt = $subscription->withdrawalOpenedAt(); @@ -72,29 +67,28 @@ final readonly class WithdrawalRight // it is derived here rather than left null so an old contract reads // as "expired" instead of "no window was ever counted". ?? $opensAt?->copy()->addDays(self::WINDOW_DAYS); - $consented = $subscription->immediate_start_consent_at !== null; // Asked of the CUSTOMER, never of the VAT number. A customer nobody has // asked is a consumer here — see Customer::isConsumer() for why the // unknown case leans this way and not the other. if ($subscription->customer?->isBusiness() === true) { - return new self(false, $opensAt, $endsAt, false, __('withdrawal.refusal_business'), $consented); + return new self(false, $opensAt, $endsAt, false, __('withdrawal.refusal_business')); } if ($subscription->isWithdrawn()) { - return new self(true, $opensAt, $endsAt, false, __('withdrawal.refusal_already'), $consented); + return new self(true, $opensAt, $endsAt, false, __('withdrawal.refusal_already')); } if ($endsAt === null) { // A contract with no beginning on record. Refused rather than // guessed: inventing the moment a statutory deadline started is the // one thing this class must never do. - return new self(true, $opensAt, null, false, __('withdrawal.refusal_no_contract'), $consented); + return new self(true, $opensAt, null, false, __('withdrawal.refusal_no_contract')); } return $endsAt->isFuture() - ? new self(true, $opensAt, $endsAt, true, null, $consented) - : new self(true, $opensAt, $endsAt, false, __('withdrawal.refusal_expired'), $consented); + ? new self(true, $opensAt, $endsAt, true, null) + : new self(true, $opensAt, $endsAt, false, __('withdrawal.refusal_expired')); } /** @@ -117,8 +111,9 @@ final readonly class WithdrawalRight * The whole days the term the customer paid for is made of. * * From the contract's own boundaries, so a month is 28, 30 or 31 days and a - * year is 365 or 366 — a fixed 30 would over- or under-charge every second - * customer by a day's worth of service. + * year is 365 or 366. Nothing is charged against it any more — it is what + * `deliveredDays()` is a share OF, in the register and on the operator's + * screen. */ public static function termDays(Subscription $subscription): int { @@ -142,8 +137,8 @@ final readonly class WithdrawalRight * * A started day counts, so this is never zero: a consumer who withdraws * three hours after buying still had a running cloud for those three hours. - * Capped at the term, because a withdrawal after the term has run out cannot - * owe more than the term was sold for. + * Capped at the term, because no more of a term can be delivered than it + * holds. */ public static function deliveredDays(Subscription $subscription, ?Carbon $at = null): int { @@ -158,30 +153,4 @@ final readonly class WithdrawalRight return max(1, min($days, self::termDays($subscription))); } - - /** - * What the consumer owes for the service they actually had, net. - * - * Zero without the express request to begin at once: the pro-rata liability - * is the price of that request, and a consumer who never made it — or who - * was never told what it would cost them — owes nothing and gets everything - * back. - */ - public static function owedNetCents(Subscription $subscription, ?Carbon $at = null): int - { - if ($subscription->immediate_start_consent_at === null) { - return 0; - } - - // The contract's own frozen NET price for the term. Never - // Order::amount_cents, which is Stripe's GROSS: mixing the two is how a - // pro-rata share ends up with VAT inside it and VAT added on top again. - $termNet = (int) $subscription->price_cents; - - if ($termNet <= 0) { - return 0; - } - - return (int) round($termNet * self::deliveredDays($subscription, $at) / self::termDays($subscription)); - } } diff --git a/app/Services/Stripe/StripeClient.php b/app/Services/Stripe/StripeClient.php index ee61fb3..4aa461a 100644 --- a/app/Services/Stripe/StripeClient.php +++ b/app/Services/Stripe/StripeClient.php @@ -154,10 +154,11 @@ interface StripeClient * * The only outgoing money movement this platform makes, and it exists * because a consumer who withdraws within fourteen days is owed one — see - * App\Actions\WithdrawContract. Partial by design: `$amountCents` is what - * goes back after the pro-rata value of the service already delivered has - * been kept, and only a withdrawal on the very first day ever equals the - * whole payment. + * App\Actions\WithdrawContract. `$amountCents` is the whole of what was + * taken: the owner has decided a withdrawing consumer gets everything back, + * not the amount less the days the cloud ran. It stays an explicit figure + * rather than "refund the payment" because a Storno states an amount and the + * money has to be the amount the Storno states. * * `$paymentReference` is a PaymentIntent (`pi_…`) or a Charge (`ch_…`) — * whichever Stripe reported for the payment. Both are accepted rather than diff --git a/database/migrations/2026_07_30_110000_charge_the_price_that_is_advertised.php b/database/migrations/2026_07_30_110000_charge_the_price_that_is_advertised.php new file mode 100644 index 0000000..e516412 --- /dev/null +++ b/database/migrations/2026_07_30_110000_charge_the_price_that_is_advertised.php @@ -0,0 +1,117 @@ +id(); + $table->foreignId('plan_price_id')->constrained()->cascadeOnDelete(); + // What the Price charges — GROSS. Deliberately not called + // `amount_cents` like the catalogue column beside it: the two are + // different numbers now, and a shared name is how they get confused. + $table->unsignedInteger('charged_cents'); + $table->string('stripe_price_id')->unique(); + // Set when the Price has been superseded and archived in Stripe. + // The row stays for ever: a contract may still be billing on it. + $table->timestamp('archived_at')->nullable(); + $table->timestamps(); + + // One Price per row per figure. A second would leave two live Stripe + // Prices for one plan at one amount and no way to tell which a + // customer is on. + $table->unique(['plan_price_id', 'charged_cents'], 'stripe_plan_prices_unique'); + $table->index(['plan_price_id', 'archived_at']); + }); + + /* + * The Prices that already exist charge the NET figure, because that is + * what they were created from. Recorded as such rather than left + * unexplained, so the sync can see they are the wrong amount and replace + * them — and so a cycle invoice for a contract still on one of them can + * still name its line. + */ + $rows = DB::table('plan_prices') + ->whereNotNull('stripe_price_id') + ->select('id', 'amount_cents', 'stripe_price_id') + ->get(); + + foreach ($rows as $row) { + DB::table('stripe_plan_prices')->insert([ + 'plan_price_id' => $row->id, + 'charged_cents' => $row->amount_cents, + 'stripe_price_id' => $row->stripe_price_id, + 'archived_at' => null, + 'created_at' => now(), + 'updated_at' => now(), + ]); + } + + Schema::table('stripe_addon_prices', function (Blueprint $table) { + // The catalogue figure this Price was formed from. `amount_cents` + // has always meant "what the Price charges" and still does — it is + // the gross now — so the net it came from needs somewhere of its + // own, or nothing could tell a repriced module from a differently + // priced one. + $table->unsignedInteger('net_cents')->nullable()->after('amount_cents'); + $table->timestamp('archived_at')->nullable()->after('stripe_price_id'); + + $table->index(['addon_key', 'net_cents', 'currency', 'interval'], 'stripe_addon_prices_net'); + }); + + // Same story as the plan side: what is there charges the net figure. + DB::table('stripe_addon_prices')->update(['net_cents' => DB::raw('amount_cents')]); + } + + public function down(): void + { + Schema::table('stripe_addon_prices', function (Blueprint $table) { + $table->dropIndex('stripe_addon_prices_net'); + $table->dropColumn(['net_cents', 'archived_at']); + }); + + Schema::dropIfExists('stripe_plan_prices'); + } +}; diff --git a/lang/de/checkout.php b/lang/de/checkout.php index f7ba793..d90e0f6 100644 --- a/lang/de/checkout.php +++ b/lang/de/checkout.php @@ -7,12 +7,15 @@ return [ 'plan_gone' => 'Dieses Paket ist derzeit nicht buchbar. Bitte wählen Sie ein anderes.', 'already_customer' => 'Sie haben bereits ein laufendes Paket. Hier wechseln Sie es, statt ein zweites zu kaufen.', - // Der ausdrückliche Wunsch, dass die Leistung sofort beginnt — die - // Voraussetzung dafür, dass ein widerrufender Verbraucher überhaupt etwas - // schuldet (FAGG §16). Ohne diese Zustimmung im Datensatz wird bei einem - // Widerruf der volle Betrag zurückgezahlt, siehe App\Actions\WithdrawContract. - // Der Satz ist der gesetzliche und keine Höflichkeitsfloskel: er muss die - // Frist UND die anteilige Zahlungspflicht nennen. - 'immediate_start' => 'Ich verlange ausdrücklich, dass Sie mit der Leistung schon vor Ablauf der 14-tägigen Widerrufsfrist beginnen. Mir ist bekannt, dass ich bei einem Widerruf einen anteiligen Betrag für die bis dahin erbrachte Leistung zu zahlen habe.', + // Der ausdrückliche Wunsch, dass die Leistung sofort beginnt. + // + // Der zweite Satz nannte früher eine anteilige Zahlungspflicht (FAGG §16). + // Die gibt es hier nicht mehr: ein widerrufender Verbraucher bekommt den + // gesamten Betrag zurück (siehe App\Actions\WithdrawContract), und ein + // Häkchen, das dem Kunden etwas Nachteiligeres zusagt als das, was wir + // tatsächlich tun, ist eine falsche Angabe — auch wenn sie zu unseren + // Lasten ginge. Von der Zustimmung hängt jetzt nichts mehr ab; sie bleibt + // als Beleg dafür, dass der Kunde den sofortigen Beginn verlangt hat. + 'immediate_start' => 'Ich verlange ausdrücklich, dass Sie mit der Leistung schon vor Ablauf der 14-tägigen Widerrufsfrist beginnen. Mein Widerrufsrecht bleibt davon unberührt: Bei einem Widerruf innerhalb der Frist erhalte ich den gesamten gezahlten Betrag zurück.', 'immediate_start_required' => 'Bitte bestätigen Sie, dass die Leistung sofort beginnen soll — sonst können wir Ihre Cloud erst nach Ablauf der Widerrufsfrist einrichten.', ]; diff --git a/lang/de/withdrawal.php b/lang/de/withdrawal.php index a32fe0e..d0daa58 100644 --- a/lang/de/withdrawal.php +++ b/lang/de/withdrawal.php @@ -10,36 +10,30 @@ return [ 'card_title' => 'Widerrufsrecht', - 'card_sub' => 'Sie können diesen Vertrag noch bis zum :date widerrufen — :days Tag(e). Der Dienst endet dann sofort und Sie erhalten das Entgelt zurück, abzüglich des Anteils für die bereits erbrachte Leistung.', + 'card_sub' => 'Sie können diesen Vertrag noch bis zum :date widerrufen — :days Tag(e). Der Dienst endet dann sofort und Sie erhalten den gesamten gezahlten Betrag zurück.', 'cta' => 'Widerruf erklären', 'keep' => 'Abbrechen', 'confirm' => 'Widerruf erklären', - 'done' => 'Ihr Widerruf ist erfasst. Der Dienst wurde beendet, die Gutschrift und die Rückzahlung sind unterwegs.', + 'done' => 'Ihr Widerruf ist erfasst. Der Dienst wurde beendet, die Stornorechnung und die Rückzahlung sind unterwegs.', 'confirm_title' => 'Vertrag widerrufen?', 'confirm_body' => 'Damit widerrufen Sie den Vertrag innerhalb der gesetzlichen Frist von 14 Tagen. Sie müssen dafür keinen Grund angeben.', 'confirm_point_now' => 'Ihre Cloud wird sofort beendet und ist danach nicht mehr erreichbar.', - 'confirm_point_documents' => 'Die Rechnung wird storniert; Sie erhalten die Stornorechnung und, falls etwas offen bleibt, eine korrigierte Rechnung.', - 'confirm_point_prorata' => 'Für die bereits genutzten :days von :term Tagen verbleiben :amount netto — dieser Anteil wird nicht zurückgezahlt.', - 'confirm_point_full' => 'Sie erhalten den gesamten gezahlten Betrag zurück.', + 'confirm_point_documents' => 'Die Rechnung wird storniert; Sie erhalten dazu eine Stornorechnung.', + 'confirm_point_full' => 'Sie erhalten den gesamten gezahlten Betrag zurück — ohne Abzug für die bereits genutzten Tage.', 'refusal_no_contract' => 'Zu diesem Konto besteht kein laufender Vertrag, der widerrufen werden könnte.', 'refusal_business' => 'Das gesetzliche Widerrufsrecht steht Verbraucherinnen und Verbrauchern zu. Für Unternehmen gilt die vertragliche Kündigungsregelung.', 'refusal_already' => 'Dieser Vertrag wurde bereits widerrufen.', 'refusal_expired' => 'Die 14-tägige Widerrufsfrist ist abgelaufen. Der Vertrag kann zum Ende der laufenden Periode gekündigt werden.', - // Die Position auf der korrigierten Rechnung: was der Kunde für die - // tatsächlich erbrachte Leistung schuldet. - 'invoice_line' => 'Anteilige Leistung :plan bis zum Widerruf', - 'invoice_detail' => ':days von :term Tagen erbracht', - 'admin_action' => 'Widerruf', 'admin_title' => 'Widerruf erfassen', 'admin_sub' => 'Widerruf von :name, telefonisch oder schriftlich erklärt.', 'admin_window' => 'Frist endet am', + // Nur zur Information: einbehalten wird nichts mehr. 'admin_delivered' => 'Erbrachte Tage', - 'admin_owed' => 'Anteil netto, der einbehalten wird', - 'admin_effect' => 'Der Dienst wird sofort beendet, die Rechnung storniert und der Restbetrag zurückgezahlt.', + 'admin_effect' => 'Der Dienst wird sofort beendet, die Rechnung storniert und der gesamte Betrag zurückgezahlt.', 'admin_confirm' => 'Widerruf erfassen', 'recorded' => 'Widerruf für :name erfasst.', ]; diff --git a/lang/en/checkout.php b/lang/en/checkout.php index 2cb28d3..58e9bcf 100644 --- a/lang/en/checkout.php +++ b/lang/en/checkout.php @@ -7,11 +7,14 @@ return [ 'plan_gone' => 'That package cannot be booked at the moment. Please choose another one.', 'already_customer' => 'You already have a running package. Change it here rather than buying a second one.', - // The express request that the service begin at once — the precondition for - // a withdrawing consumer owing anything at all (FAGG §16). Without this - // consent on record a withdrawal refunds the whole amount; see - // App\Actions\WithdrawContract. The sentence is the statutory one rather - // than a nicety: it has to name the period AND the pro-rata liability. - 'immediate_start' => 'I expressly request that you begin providing the service before the 14-day withdrawal period has ended. I understand that if I withdraw I must pay a proportionate amount for the service provided up to that point.', + // The express request that the service begin at once. + // + // The second sentence used to state a pro-rata liability (FAGG §16). There + // is none here any more: a withdrawing consumer gets the whole amount back + // (see App\Actions\WithdrawContract), and a tick-box that promises the + // customer something worse than what we actually do is a false statement, + // even when it is against our own interest. Nothing turns on the consent + // now; it stays as a record that the customer asked us to start at once. + 'immediate_start' => 'I expressly request that you begin providing the service before the 14-day withdrawal period has ended. My right of withdrawal is unaffected: if I withdraw within the period I get the full amount paid back.', 'immediate_start_required' => 'Please confirm that the service should begin at once — otherwise we can only build your cloud once the withdrawal period has ended.', ]; diff --git a/lang/en/withdrawal.php b/lang/en/withdrawal.php index 7f228ea..a73b82a 100644 --- a/lang/en/withdrawal.php +++ b/lang/en/withdrawal.php @@ -10,7 +10,7 @@ return [ 'card_title' => 'Right of withdrawal', - 'card_sub' => 'You may withdraw from this contract until :date — :days day(s) left. The service ends immediately and you get your money back, less the share for the service already provided.', + 'card_sub' => 'You may withdraw from this contract until :date — :days day(s) left. The service ends immediately and you get the full amount paid back.', 'cta' => 'Declare withdrawal', 'keep' => 'Cancel', 'confirm' => 'Declare withdrawal', @@ -19,27 +19,21 @@ return [ 'confirm_title' => 'Withdraw from this contract?', 'confirm_body' => 'This withdraws the contract within the statutory period of 14 days. You do not have to give a reason.', 'confirm_point_now' => 'Your cloud is ended immediately and will no longer be reachable.', - 'confirm_point_documents' => 'The invoice is cancelled; you receive the cancellation document and, where anything remains due, a corrected invoice.', - 'confirm_point_prorata' => 'For the :days of :term days already used, :amount net remains payable — that share is not refunded.', - 'confirm_point_full' => 'You get the full amount paid back.', + 'confirm_point_documents' => 'The invoice is cancelled; you receive a cancellation document for it.', + 'confirm_point_full' => 'You get the full amount paid back — with nothing deducted for the days already used.', 'refusal_no_contract' => 'There is no running contract on this account that could be withdrawn from.', 'refusal_business' => 'The statutory right of withdrawal is a consumer right. Business customers are covered by the contractual cancellation terms.', 'refusal_already' => 'This contract has already been withdrawn from.', 'refusal_expired' => 'The 14-day withdrawal period has ended. The contract can be cancelled with effect from the end of the current period.', - // The line on the corrected invoice: what the customer owes for the service - // that was actually provided. - 'invoice_line' => 'Pro-rata service :plan up to withdrawal', - 'invoice_detail' => ':days of :term days provided', - 'admin_action' => 'Withdrawal', 'admin_title' => 'Record a withdrawal', 'admin_sub' => 'Withdrawal declared by :name, by telephone or in writing.', 'admin_window' => 'Period ends on', + // Information only: nothing is retained any more. 'admin_delivered' => 'Days provided', - 'admin_owed' => 'Net share retained', - 'admin_effect' => 'The service ends immediately, the invoice is cancelled and the balance is refunded.', + 'admin_effect' => 'The service ends immediately, the invoice is cancelled and the full amount is refunded.', 'admin_confirm' => 'Record withdrawal', 'recorded' => 'Withdrawal recorded for :name.', ]; diff --git a/resources/views/livewire/admin/record-withdrawal.blade.php b/resources/views/livewire/admin/record-withdrawal.blade.php index 6439466..579aefe 100644 --- a/resources/views/livewire/admin/record-withdrawal.blade.php +++ b/resources/views/livewire/admin/record-withdrawal.blade.php @@ -1,6 +1,3 @@ -@php - $eur = fn (int $cents) => \Illuminate\Support\Number::currency($cents / 100, in: 'EUR', locale: app()->getLocale()); -@endphp
@@ -25,14 +22,13 @@
{{ __('withdrawal.admin_window') }}
{{ $right->endsAt?->local()->isoFormat('LL') }}
+ {{-- Nothing is charged for them any more. Still shown, because an + operator taking a withdrawal on the telephone is asked how long + the cloud has been running and should not have to work it out. --}}
{{ __('withdrawal.admin_delivered') }}
{{ $deliveredDays }} / {{ $termDays }}
-
-
{{ __('withdrawal.admin_owed') }}
-
{{ $eur($owedNetCents) }}
-

{{ __('withdrawal.admin_effect') }}

diff --git a/resources/views/livewire/confirm-withdraw.blade.php b/resources/views/livewire/confirm-withdraw.blade.php index 711d731..68e8d73 100644 --- a/resources/views/livewire/confirm-withdraw.blade.php +++ b/resources/views/livewire/confirm-withdraw.blade.php @@ -1,6 +1,3 @@ -@php - $eur = fn (int $cents) => \Illuminate\Support\Number::currency($cents / 100, in: 'EUR', locale: app()->getLocale()); -@endphp
@@ -19,21 +16,12 @@
  • {{ __('withdrawal.confirm_point_documents') }}
    • - {{-- The pro-rata share, stated before the button and not after it. - FAGG §16 lets us keep it only because the customer asked for the - service to start at once and was told this would follow — so this - is where they are told it again, with the actual figure. Where no - such request was recorded the amount is zero and the sentence says - the whole amount comes back. --}} + {{-- What comes back, stated before the button and not after it. FAGG §16 + would let us keep the value of the days already delivered; the owner + has decided not to, so there is one sentence here and no arithmetic + to show. --}}
  • - - {{ $owedNetCents > 0 - ? __('withdrawal.confirm_point_prorata', [ - 'amount' => $eur($owedNetCents), - 'days' => $deliveredDays, - 'term' => $termDays, - ]) - : __('withdrawal.confirm_point_full') }} + {{ __('withdrawal.confirm_point_full') }}
    • diff --git a/tests/Feature/Billing/StripeAddonBillingTest.php b/tests/Feature/Billing/StripeAddonBillingTest.php index 87eec92..646e74f 100644 --- a/tests/Feature/Billing/StripeAddonBillingTest.php +++ b/tests/Feature/Billing/StripeAddonBillingTest.php @@ -9,6 +9,7 @@ use App\Models\PlanPrice; use App\Models\Subscription; use App\Models\SubscriptionAddon; use App\Services\Billing\AddonPrices; +use App\Services\Billing\TaxTreatment; use App\Services\Stripe\FakeStripeClient; use App\Services\Stripe\StripeClient; use App\Support\CompanyProfile; @@ -90,7 +91,21 @@ function addonPlanPrice(Subscription $contract): string /** 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'); + return (string) app(AddonPrices::class)->liveFor($key, $monthlyCents, 'EUR', Subscription::TERM_MONTHLY); +} + +/** + * What Stripe actually takes for a net catalogue figure. + * + * The catalogue is pushed to Stripe GROSS, so every amount on a Stripe invoice + * line already contains the VAT. The fixtures below state the net figure a + * reader recognises from the price sheet and put it through the same call the + * sync does, rather than writing 21480 and leaving the next reader to work out + * where it came from. + */ +function atTheTill(int $netCents): int +{ + return TaxTreatment::chargedCents($netCents); } /** @@ -119,7 +134,10 @@ function stripeLine(string $priceId, int $amount, Carbon $from, Carbon $to, int */ function paidInvoice(string $id, array $lines, string $reason, Carbon $from, Carbon $to): array { - $net = array_sum(array_column($lines, 'amount')); + // What Stripe took IS the sum of its lines. The catalogue is pushed gross, + // so there is nothing to add on top — and the document has to total to this + // figure exactly, which is what these tests are checking. + $charged = array_sum(array_column($lines, 'amount')); return [ 'id' => 'evt_'.$id, 'type' => 'invoice.paid', @@ -128,10 +146,8 @@ function paidInvoice(string $id, array $lines, string $reason, Carbon $from, Car '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), + 'amount_paid' => $charged, + 'total' => $charged, 'period_start' => $from->timestamp, 'period_end' => $to->timestamp, 'lines' => ['data' => $lines, 'has_more' => false], @@ -154,12 +170,12 @@ it('gives the owner’s example four invoices, and puts package and module on th // 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, + 'in_m1', [stripeLine($planPrice, atTheTill(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, + 'in_m2', [stripeLine($planPrice, atTheTill(17900), $month2, $month3)], 'subscription_cycle', $month2, $month3, ))->assertOk(); expect(Invoice::query()->count())->toBe(2); @@ -185,7 +201,7 @@ it('gives the owner’s example four invoices, and puts package and module on th $storagePrice = addonModulePrice('storage', 1000); $this->postJson(route('webhooks.stripe'), paidInvoice( 'in_m3_storage', - [stripeLine($storagePrice, 484, now(), $month4, proration: true)], + [stripeLine($storagePrice, atTheTill(484), now(), $month4, proration: true)], 'subscription_update', now(), $month4, @@ -198,8 +214,8 @@ it('gives the owner’s example four invoices, and puts package and module on th $this->postJson(route('webhooks.stripe'), paidInvoice( 'in_m4', [ - stripeLine($planPrice, 17900, $month4, $month4->copy()->addMonth()), - stripeLine($storagePrice, 1000, $month4, $month4->copy()->addMonth()), + stripeLine($planPrice, atTheTill(17900), $month4, $month4->copy()->addMonth()), + stripeLine($storagePrice, atTheTill(1000), $month4, $month4->copy()->addMonth()), ], 'subscription_cycle', $month4, @@ -230,11 +246,14 @@ it('gives the owner’s example four invoices, and puts package and module on th 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. + // Stripe took 226,80 € and the document says 226,80 €, split into the + // net and the VAT that were inside it. The catalogue is pushed gross, so + // the total is not something the document adds up to — it is the figure + // it starts from and divides. + ->and($fourth->gross_cents)->toBe(atTheTill(17900) + atTheTill(1000)) + ->and($fourth->gross_cents)->toBe(22680) ->and($fourth->net_cents)->toBe(18900) - ->and($fourth->tax_cents)->toBe(3780) - ->and($fourth->gross_cents)->toBe(22680); + ->and($fourth->tax_cents)->toBe(3780); // One mail per document, never two for one. Mail::assertQueuedCount(4); @@ -249,7 +268,7 @@ it('issues one document and consumes one number however often Stripe redelivers $event = paidInvoice( 'in_twice', - [stripeLine(addonPlanPrice($contract), 17900, now(), now()->addMonth())], + [stripeLine(addonPlanPrice($contract), atTheTill(17900), now(), now()->addMonth())], 'subscription_cycle', now(), now()->addMonth(), @@ -304,7 +323,7 @@ it('keeps a cancelled module until the period end and leaves it off the next cyc $this->postJson(route('webhooks.stripe'), paidInvoice( 'in_after_cancel', - [stripeLine(addonPlanPrice($contract), 17900, now(), now()->addMonth())], + [stripeLine(addonPlanPrice($contract), atTheTill(17900), now(), now()->addMonth())], 'subscription_cycle', now(), now()->addMonth(), @@ -344,8 +363,8 @@ it('books a second storage pack as a quantity rather than a second item', functi $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), + stripeLine(addonPlanPrice($contract), atTheTill(17900), now(), now()->addMonth()), + stripeLine(addonModulePrice('storage', 1000), atTheTill(2000), now(), now()->addMonth(), quantity: 2), ], 'subscription_cycle', now(), @@ -436,8 +455,8 @@ it('gives an upgrade’s proration a document for the amount Stripe actually cha $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), + stripeLine(addonPlanPrice($contract), atTheTill(-8950), now(), now()->addDays(15), proration: true), + stripeLine($business, atTheTill(17450), now(), now()->addDays(15), proration: true), ], 'subscription_update', now(), @@ -471,8 +490,8 @@ it('keeps a line it cannot name rather than dropping it from the document', func $this->postJson(route('webhooks.stripe'), paidInvoice( 'in_unknown', [ - stripeLine(addonPlanPrice($contract), 17900, now(), now()->addMonth()), - stripeLine('price_typed_by_hand', 5000, now(), now()->addMonth()), + stripeLine(addonPlanPrice($contract), atTheTill(17900), now(), now()->addMonth()), + stripeLine('price_typed_by_hand', atTheTill(5000), now(), now()->addMonth()), ], 'subscription_cycle', now(), diff --git a/tests/Feature/Billing/StripeBillingTest.php b/tests/Feature/Billing/StripeBillingTest.php index 995a4ad..7783d70 100644 --- a/tests/Feature/Billing/StripeBillingTest.php +++ b/tests/Feature/Billing/StripeBillingTest.php @@ -8,6 +8,7 @@ use App\Models\StripePendingEvent; use App\Models\Subscription; use App\Models\SubscriptionRecord; use App\Services\Billing\PlanCatalogue; +use App\Services\Billing\TaxTreatment; use App\Services\Stripe\FakeStripeClient; use App\Services\Stripe\StripeClient; use Illuminate\Support\Collection; @@ -70,8 +71,12 @@ it('mirrors the catalogue into Stripe, once', function () { // 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']); + // A yearly module Price is twelve months of the NET figure, grossed once — + // which is what a yearly customer is actually charged for it. expect(modulePrices($stripe))->toHaveCount(count($modules) * 2) ->and(StripeAddonPrice::query()->where('addon_key', 'storage')->where('interval', 'year')->value('amount_cents')) + ->toBe(TaxTreatment::chargedCents(12 * (int) config('provisioning.storage_addon.price_cents'))) + ->and(StripeAddonPrice::query()->where('addon_key', 'storage')->where('interval', 'year')->value('net_cents')) ->toBe(12 * (int) config('provisioning.storage_addon.price_cents')); // A Stripe Price cannot be edited, so a second run that minted duplicates @@ -94,8 +99,11 @@ it('gives each price its own recurring interval', function () { expect($intervals['month'])->toBe(4) ->and($intervals['year'])->toBe(4); + // The GROSS figure, which is the one on the website and the one taken from + // the card. The catalogue row stays net. $team = planPrices($stripe)->first(fn ($p) => $p['metadata']['plan_family'] === 'team' && $p['interval'] === 'month'); - expect($team['amount'])->toBe(17900) + expect($team['amount'])->toBe(21480) + ->and($team['amount'])->toBe(TaxTreatment::chargedCents(17900)) ->and($team['metadata']['plan_version'])->toBe('1'); }); diff --git a/tests/Feature/Billing/WithdrawalTest.php b/tests/Feature/Billing/WithdrawalTest.php index 30aef42..833236d 100644 --- a/tests/Feature/Billing/WithdrawalTest.php +++ b/tests/Feature/Billing/WithdrawalTest.php @@ -24,20 +24,22 @@ use Illuminate\Support\Carbon; use Livewire\Livewire; /** - * The fourteen-day right of withdrawal — who has it, what it costs, and what it - * leaves behind. + * The fourteen-day right of withdrawal — who has it, what it gives back, and + * what it leaves behind. * * The concrete example these tests are built on, once, so the arithmetic can be * checked by hand rather than recomputed from the implementation: * * contract concluded 1 March 2026, 09:00 * term 1 March → 1 April = 31 days - * price 49,00 € net, 20 % VAT → 58,80 € gross, invoiced and paid + * price 49,00 € net; Stripe charges the gross, 58,80 € * withdrawal 8 March 2026, 09:00 — 7 days of service delivered * - * owed net = round(4900 × 7 / 31) = 1106 (11,06 €) - * owed gross = 1106 + round(1106 × 0,20) = 1327 (13,27 €) - * refund = 5880 − 1327 = 4553 (45,53 €) + * refund = 5880, the whole of it (58,80 €) + * + * FAGG §16 would let us keep the pro-rata value of those seven days — 13,27 € + * gross. The owner has decided not to, so there is no arithmetic left: whatever + * the customer paid comes back, and the Storno is for the whole document. */ beforeEach(function () { Carbon::setTestNow('2026-03-01 09:00:00'); @@ -65,7 +67,7 @@ afterEach(function () { * * @return array{0: Subscription, 1: Invoice, 2: Instance} */ -function withdrawableContract(array $customerState = [], bool $consented = true): array +function withdrawableContract(array $customerState = []): array { $customer = Customer::factory()->create($customerState + [ 'name' => 'Frau Berger', @@ -75,21 +77,17 @@ function withdrawableContract(array $customerState = [], bool $consented = true) $order = Order::factory()->create([ 'customer_id' => $customer->id, 'plan' => 'team', - // What the payment provider took, and what the document is written from. - 'amount_cents' => 4900, + // What the payment provider took, and what the document is written from: + // the GROSS of the 49,00 € catalogue figure, because that is what the + // Stripe Price charges. + 'amount_cents' => 5880, 'currency' => 'EUR', 'status' => 'paid', 'stripe_event_id' => 'cs_withdraw', 'stripe_payment_intent_id' => 'pi_withdraw', ]); - $factory = Subscription::factory(); - - if ($consented) { - $factory = $factory->startedImmediately(); - } - - $contract = $factory->create([ + $contract = Subscription::factory()->create([ 'customer_id' => $customer->id, 'order_id' => $order->id, 'price_cents' => 4900, @@ -217,59 +215,53 @@ it('refuses a withdrawal once the fourteen days are up', function () { ->and($stripe->refunds)->toBeEmpty(); }); -it('refunds the amount paid less the pro-rata value of the service delivered', function () { +it('refunds a consumer withdrawing on day thirteen the whole of what they paid', function () { $stripe = withdrawalStripe(); [$contract, $invoice] = withdrawableContract(['customer_type' => Customer::TYPE_CONSUMER]); - // The document the customer actually has: 49,00 € net, 20 % VAT. - expect($invoice->net_cents)->toBe(4900) - ->and($invoice->tax_cents)->toBe(980) - ->and($invoice->gross_cents)->toBe(5880); + // The document the customer actually has: 58,80 € was charged, and the + // document divides it — 49,00 € net and 9,80 € VAT. + expect($invoice->gross_cents)->toBe(5880) + ->and($invoice->net_cents)->toBe(4900) + ->and($invoice->tax_cents)->toBe(980); - Carbon::setTestNow('2026-03-08 09:00:00'); + // Day thirteen: deep inside the window, and thirteen days of cloud really + // did run. None of it is charged for. + Carbon::setTestNow('2026-03-13 09:00:00'); - expect(WithdrawalRight::deliveredDays($contract->refresh()))->toBe(7) - ->and(WithdrawalRight::termDays($contract))->toBe(31) - ->and(WithdrawalRight::owedNetCents($contract))->toBe(1106); + $right = WithdrawalRight::for($contract->refresh()); + + expect($right->open)->toBeTrue() + ->and(WithdrawalRight::deliveredDays($contract))->toBe(12) + ->and(WithdrawalRight::termDays($contract))->toBe(31); app(WithdrawContract::class)($contract); - // The corrected document states exactly what is kept… - $remainder = Invoice::query() - ->where('customer_id', $contract->customer_id) - ->whereNull('cancels_invoice_id') - ->where('id', '>', $invoice->id) - ->firstOrFail(); + // Nothing is invoiced for the days that were delivered — there is the + // original and its Storno, and no third document. + expect(Invoice::query()->where('customer_id', $contract->customer_id)->count())->toBe(2); - expect($remainder->net_cents)->toBe(1106) - ->and($remainder->tax_cents)->toBe(221) - ->and($remainder->gross_cents)->toBe(1327); - - // …and the money is the difference between the two documents, to the cent. + // And the money is the whole of what the cancelled document said. expect($stripe->refunds)->toHaveCount(1) - ->and($stripe->refunds[0]['amount'])->toBe(4553) + ->and($stripe->refunds[0]['amount'])->toBe(5880) ->and($stripe->refunds[0]['payment'])->toBe('pi_withdraw') - ->and($contract->refresh()->withdrawal_refund_cents)->toBe(4553); - - expect(5880 - 1327)->toBe(4553); + ->and($contract->refresh()->withdrawal_refund_cents)->toBe(5880); }); -it('refunds everything where no express request to start at once was recorded', function () { +it('refunds in full on day seven as well, whatever the express-start consent said', function () { $stripe = withdrawalStripe(); - // FAGG §16 lets us keep the pro-rata share only where the consumer asked - // for the service to begin inside the window AND was told what that would - // cost. Without that on record they owe nothing. - [$contract, $invoice] = withdrawableContract( - ['customer_type' => Customer::TYPE_CONSUMER], - consented: false, - ); + // FAGG §16 would let us keep the pro-rata share where the consumer asked for + // the service to begin inside the window. The owner has decided not to, so + // the consent decides nothing: the contract carries it and the refund is + // still everything. + [$contract, $invoice] = withdrawableContract(['customer_type' => Customer::TYPE_CONSUMER]); + + $contract->update(['immediate_start_consent_at' => now()]); Carbon::setTestNow('2026-03-08 09:00:00'); - expect(WithdrawalRight::owedNetCents($contract->refresh()))->toBe(0); - - app(WithdrawContract::class)($contract); + app(WithdrawContract::class)($contract->refresh()); expect($stripe->refunds[0]['amount'])->toBe($invoice->gross_cents) ->and($stripe->refunds[0]['amount'])->toBe(5880); @@ -354,13 +346,13 @@ it('records the withdrawal in the proof register', function () { ->where('event', SubscriptionRecord::EVENT_WITHDRAWAL) ->firstOrFail(); - // Revenue leaving, with what actually went back and the days it was worked - // out from beside it. - expect($entry->gross_cents)->toBe(-4553) + // Revenue leaving, with what actually went back — and the days the service + // ran beside it, which nothing is charged for but which is still a fact + // about this contract. + expect($entry->gross_cents)->toBe(-5880) ->and($entry->snapshot['withdrawal']['delivered_days'])->toBe(7) ->and($entry->snapshot['withdrawal']['term_days'])->toBe(31) - ->and($entry->snapshot['withdrawal']['owed_net_cents'])->toBe(1106) - ->and($entry->snapshot['withdrawal']['refunded_gross_cents'])->toBe(4553); + ->and($entry->snapshot['withdrawal']['refunded_gross_cents'])->toBe(5880); }); it('withdraws once however many times it is asked', function () { @@ -419,7 +411,7 @@ it('lets an operator record a withdrawal that came in by telephone', function () expect($contract->withdrawn_at)->not->toBeNull() ->and($contract->withdrawal_channel)->toBe(WithdrawContract::CHANNEL_OPERATOR) ->and($contract->withdrawal_recorded_by)->toBe($support->id) - ->and($stripe->refunds[0]['amount'])->toBe(4553); + ->and($stripe->refunds[0]['amount'])->toBe(5880); }); it('will not let an operator record a withdrawal a business customer does not have', function () {