From 9da13588023e506d1f7032c1589529941949dc69 Mon Sep 17 00:00:00 2001 From: nexxo Date: Thu, 30 Jul 2026 02:15:41 +0200 Subject: [PATCH] Stop charging VAT to the customers who owe us none MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A verified EU business outside Austria gets reverse charge — rate 0, no VAT line, the note on the invoice — and was still charged 214,80 € for a 179,00 € package, because one Stripe Price carried the domestic gross for everybody. There is no VAT line on their document to reclaim, so it was a flat 20 % surcharge on exactly the customers who read their invoices, and the document then stated the whole 214,80 € as net at 0 %, so they self-accounted their own VAT on a base a fifth too large. A Stripe Price can ask who is buying after all: there are two per sellable thing now, the domestic gross and the bare net, both live on one Product, and the checkout picks by TaxTreatment. The rule is Austria B2B 20 %, other EU B2B without VAT — a domestic business still pays the gross, reclaims it as input tax, and the price on the website is still the price charged for them. - stripe:sync-catalogue mints and archives both halves of every pair, per plan and per module; `reverse_charge` on stripe_plan_prices/stripe_addon_prices says which is which, and joining it on subscriptions.stripe_price_id says which one a running contract is billed on. A rate change moves the gross Price and leaves the net one alone, because the net owes nothing to the rate. - A status that changes after the sale converges: a verification (or a lapse) makes stripe:reprice-subscriptions move the contract onto the other Price with PRORATE_NONE, because the term is already paid for. The module items follow the same way. - Downstream follows: the invoice total equals what was taken, the proof register expects the figure this customer is actually charged (so a correct reverse-charge sale is no longer flagged as a mismatch, and an overcharged one is), the setup fee obeys the same rule, and the booking page quotes what it will charge. - StripeClient gained activatePrice(). Archived Prices were brought back in our own table and left inactive at Stripe, so a rate that moved and moved back pointed the catalogue at a Price no checkout could be opened for. An unverified number is still charged the gross — an unchecked string must never be a discount — and where the net Price is missing the checkout refuses rather than reaching for the domestic one. Co-Authored-By: Claude Opus 5 --- app/Actions/ApplyPlanChange.php | 16 +- app/Actions/MoveStripeSubscriptionPrice.php | 30 +- app/Actions/OpenSubscription.php | 17 +- app/Actions/RecordCommercialEvent.php | 22 +- app/Actions/SyncStripeAddonItems.php | 27 +- .../Commands/RepriceStripeSubscriptions.php | 47 +- app/Console/Commands/SyncStripeCatalogue.php | 211 +++++---- app/Http/Controllers/CheckoutController.php | 67 ++- app/Http/Controllers/LandingController.php | 24 +- app/Livewire/Order.php | 51 ++- app/Models/Order.php | 16 +- app/Models/StripeAddonPrice.php | 16 +- app/Models/StripePlanPrice.php | 26 +- app/Services/Billing/AddonPrices.php | 111 +++-- app/Services/Billing/IssueInvoice.php | 35 +- app/Services/Billing/PlanPrices.php | 259 ++++++++++++ app/Services/Billing/SetupFee.php | 36 +- app/Services/Billing/StripeInvoiceLines.php | 16 +- app/Services/Billing/TaxTreatment.php | 87 +++- app/Services/Mail/MailTemplateRenderer.php | 26 +- app/Services/Stripe/FakeStripeClient.php | 22 + app/Services/Stripe/HttpStripeClient.php | 11 + app/Services/Stripe/StripeClient.php | 26 +- ...net_price_to_a_reverse_charge_business.php | 96 +++++ lang/de/order.php | 3 + lang/en/order.php | 3 + resources/views/livewire/order.blade.php | 18 +- tests/Feature/Billing/ProofRegisterTest.php | 39 +- .../Billing/ReverseChargePriceTest.php | 400 ++++++++++++++++++ tests/Feature/Billing/SetupFeeTest.php | 6 +- .../Billing/StripeAddonBillingTest.php | 33 +- tests/Feature/Billing/StripeBillingTest.php | 70 ++- 32 files changed, 1481 insertions(+), 386 deletions(-) create mode 100644 app/Services/Billing/PlanPrices.php create mode 100644 database/migrations/2026_07_31_200000_sell_the_net_price_to_a_reverse_charge_business.php create mode 100644 tests/Feature/Billing/ReverseChargePriceTest.php diff --git a/app/Actions/ApplyPlanChange.php b/app/Actions/ApplyPlanChange.php index b8697af..6dca884 100644 --- a/app/Actions/ApplyPlanChange.php +++ b/app/Actions/ApplyPlanChange.php @@ -175,13 +175,17 @@ class ApplyPlanChange 'remaining_days' => $change->remainingDays, 'term_days' => $change->termDays, // The preview's pro-rata difference, and what the till would - // take for it. Both, because PlanChange prorates the NET - // catalogue figures while the Stripe Prices it prorates - // against are gross — so the net figure alone reads as a fifth - // less than the proration invoice that follows, and somebody - // reconciling the two needs the number to compare. + // take from THIS customer for it. Both, because PlanChange + // prorates the NET catalogue figures while the Stripe Price it + // prorates against is the one that customer is billed on — so + // for anybody who is charged VAT the net figure alone reads as + // a fifth less than the proration invoice that follows, and + // somebody reconciling the two needs the number to compare. + // For a reverse-charge business the two are equal, and saying + // so is what stops a reader looking for a missing fifth. 'prorated_net_cents' => $change->chargeCents, - 'prorated_charge_cents' => TaxTreatment::chargedCents($change->chargeCents), + 'prorated_charge_cents' => TaxTreatment::for($subscription->customer) + ->chargeCents($change->chargeCents), ]], // Nothing has been charged AT THIS EVENT. Left null rather than // reported as zero, which the register would read as a free sale. diff --git a/app/Actions/MoveStripeSubscriptionPrice.php b/app/Actions/MoveStripeSubscriptionPrice.php index dc99d97..dfa2463 100644 --- a/app/Actions/MoveStripeSubscriptionPrice.php +++ b/app/Actions/MoveStripeSubscriptionPrice.php @@ -2,8 +2,8 @@ namespace App\Actions; -use App\Models\PlanPrice; use App\Models\Subscription; +use App\Services\Billing\PlanPrices; use App\Services\Stripe\StripeClient; use Illuminate\Support\Facades\Log; use RuntimeException; @@ -47,7 +47,10 @@ use Throwable; */ class MoveStripeSubscriptionPrice { - public function __construct(private StripeClient $stripe) {} + public function __construct( + private StripeClient $stripe, + private PlanPrices $prices, + ) {} /** * @param string $behaviour one of StripeClient::PRORATE_* @@ -135,21 +138,24 @@ class MoveStripeSubscriptionPrice } /** - * The Stripe Price for the package the contract is on now. + * The Stripe Price for the package the contract is on now, for the customer + * who is paying for it. * * By plan VERSION and term, which is what a Stripe Price is: one per priced - * row, written by stripe:sync-catalogue. Resolving it by plan name would - * hand back today's terms for a contract that is grandfathered on older - * ones. + * row and per tax treatment, written by stripe:sync-catalogue. Resolving it by + * plan name would hand back today's terms for a contract that is + * grandfathered on older ones. + * + * The treatment is part of the answer, which is what makes a verification + * converge: a business whose VAT id is confirmed the week after they bought + * belongs on the net Price from that moment, and one whose registration has + * lapsed belongs back on the gross one. Neither is a change of package, so + * nothing here looks like one — the move is made with PRORATE_NONE by the + * sweep that finds it, because the term is already paid for. */ private function targetPrice(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; + return $this->prices->liveForSubscription($subscription); } /** diff --git a/app/Actions/OpenSubscription.php b/app/Actions/OpenSubscription.php index 57ae48c..124d093 100644 --- a/app/Actions/OpenSubscription.php +++ b/app/Actions/OpenSubscription.php @@ -19,9 +19,11 @@ use Illuminate\Support\Facades\DB; * cannot reach a customer who has already paid. * * `price_cents` is the catalogue's NET price, which is what PlanChange prorates - * against — deliberately not `Order::amount_cents`, which holds the GROSS total - * Stripe actually charged. The two can legitimately differ (VAT, and later - * coupons), and reconciling them is not this action's job: the proof register + * against — deliberately not `Order::amount_cents`, which holds the total Stripe + * actually charged, whichever of a package's two Prices this customer was sold on. + * The two can legitimately differ (VAT, and later coupons) — and for a + * reverse-charge business they legitimately agree, because no VAT is owed to us at + * all — and reconciling them is not this action's job: the proof register * records what was charged per event, and Stripe's invoice is the authority for * the amount. Copying a gross total into this net field would silently corrupt * every pro-rata calculation that reads it. @@ -107,10 +109,11 @@ class OpenSubscription netCents: $subscription->price_cents, at: $start, stripe: ['event' => $order->stripe_event_id], - // The order carries what Stripe actually took — gross, including - // whatever tax or discount applied on the day. The contract price - // is what was agreed; this is what was paid, and the register has - // to be able to state both. + // The order carries what Stripe actually took, including whatever tax + // or discount applied on the day. The contract price is what was + // agreed; this is what was paid, and the register has to be able to + // state both — which for a reverse-charge business is the same figure, + // because their Price carries the bare net. // // Keyed on the Stripe id, not on the amount being non-zero: a fully // discounted checkout charges zero, and reading that as "no amount diff --git a/app/Actions/RecordCommercialEvent.php b/app/Actions/RecordCommercialEvent.php index 2444c1f..763fa71 100644 --- a/app/Actions/RecordCommercialEvent.php +++ b/app/Actions/RecordCommercialEvent.php @@ -44,18 +44,18 @@ class RecordCommercialEvent $agreedNet = $netCents; - // What the till would take for this catalogue figure — the DOMESTIC - // gross, for everybody. Not $tax->grossCents(), which is how this - // customer's DOCUMENT is split: at a rate of zero that returns the net - // figure, and comparing it against the money Stripe actually took - // reported every reverse-charge sale charged at exactly the advertised - // amount as a mismatch. The one flag whose job is "somebody must look at - // this" was false for the whole healthy majority of EU business sales. + // What the till would take from THIS customer for this catalogue figure: + // the domestic gross for everybody who is charged VAT, the bare net for a + // reverse-charge business, because that is the Stripe Price their checkout + // used. Asked of the treatment rather than of a single domestic figure, + // which is what it used to be — and every reverse-charge sale, charged + // exactly the amount it should have been, was then reported as a mismatch + // by the one flag whose job is "somebody must look at this". // - // TaxTreatment::chargedCents() is the single place a gross is formed, and - // it is what the Stripe Price carries — so this is the figure the charge - // can honestly be held against. - $expectedGross = TaxTreatment::chargedCents($agreedNet); + // chargeCents() is the single place the amount taken is formed, and it is + // what the Stripe Price carries — so this is the figure the charge can + // honestly be held against. + $expectedGross = $tax->chargeCents($agreedNet); // The flat columns describe the TRANSACTION, not the contract: gross is // what was taken from the customer, and net and tax are that gross diff --git a/app/Actions/SyncStripeAddonItems.php b/app/Actions/SyncStripeAddonItems.php index f696c85..8a69a3b 100644 --- a/app/Actions/SyncStripeAddonItems.php +++ b/app/Actions/SyncStripeAddonItems.php @@ -5,6 +5,7 @@ namespace App\Actions; use App\Models\Subscription; use App\Models\SubscriptionAddon; use App\Services\Billing\AddonPrices; +use App\Services\Billing\TaxTreatment; use App\Services\Stripe\StripeClient; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; @@ -108,13 +109,19 @@ class SyncStripeAddonItems /** * Move every module item this contract carries onto the Price that charges - * today's figure for it. + * today's figure for it, to the customer who is paying 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. + * Two reasons an item is on the wrong Price, and __invoke() above finds + * neither of 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. + * + * The first is a change of figure — the catalogue moved from pushing NET + * amounts to Stripe to pushing the amount actually charged, and a Stripe Price + * cannot be edited, so every running module was on a Price that takes too + * little. The second is a change of CUSTOMER: a business whose VAT id is + * verified after they booked owes no VAT on the module either, and belongs on + * its net Price from then on — the reverse too, when a registration lapses. * * PRORATE_NONE throughout. The customer has already paid for the term they * are in, at whatever was taken then; charging the difference for days @@ -154,6 +161,7 @@ class SyncStripeAddonItems (int) $first->price_cents, (string) $first->currency, (string) $subscription->term, + TaxTreatment::for($subscription->customer), ); $current = $stamped->first()?->stripe_price_id; @@ -177,6 +185,7 @@ class SyncStripeAddonItems (int) $first->price_cents, (string) $first->currency, (string) $subscription->term, + TaxTreatment::for($subscription->customer), ); if ($target === null) { @@ -284,6 +293,12 @@ class SyncStripeAddonItems (int) $first->price_cents, (string) $first->currency, (string) $subscription->term, + // Whoever holds the contract decides which of the module's Prices its + // item bills through: the domestic gross, or the bare net where the + // customer is a verified business in another member state. A module is + // a supply like the package beside it and cannot be taxed differently + // from it on the same invoice. + TaxTreatment::for($subscription->customer), ); if ($priceId === null) { diff --git a/app/Console/Commands/RepriceStripeSubscriptions.php b/app/Console/Commands/RepriceStripeSubscriptions.php index 2031963..6942308 100644 --- a/app/Console/Commands/RepriceStripeSubscriptions.php +++ b/app/Console/Commands/RepriceStripeSubscriptions.php @@ -4,8 +4,8 @@ namespace App\Console\Commands; use App\Actions\MoveStripeSubscriptionPrice; use App\Actions\SyncStripeAddonItems; -use App\Models\PlanPrice; use App\Models\Subscription; +use App\Services\Billing\PlanPrices; use App\Services\Stripe\StripeClient; use Illuminate\Console\Command; @@ -37,6 +37,16 @@ use Illuminate\Console\Command; * throws — MoveStripeSubscriptionPrice parks its failures on the contract and * clupilot:sync-stripe-subscriptions retries them hourly. * + * **Two reasons a contract moves, not one.** The first is a change of FIGURE, as + * above. The second is a change of CUSTOMER: a business whose VAT id is verified + * the week after they bought must be billed the net Price from then on, because + * reverse charge means they owe no VAT to us at all — and one whose registration + * has lapsed must go back onto the gross one. Neither is a change of package, so + * nothing else in the system would ever notice, and both converge here on the next + * run. PlanPrices::liveForSubscription() answers "which Price does this contract + * belong on?" for both, which is why this command has no resolver of its own: two + * places working that out is how they come to disagree. + * * The contract's own `price_cents` is not touched by any of this. It is the * catalogue's NET figure, it is frozen, and PlanChange prorates against it. */ @@ -52,6 +62,7 @@ class RepriceStripeSubscriptions extends Command StripeClient $stripe, MoveStripeSubscriptionPrice $move, SyncStripeAddonItems $modules, + PlanPrices $prices, ): int { $dryRun = (bool) $this->option('dry-run'); @@ -76,14 +87,24 @@ class RepriceStripeSubscriptions extends Command $failed = 0; foreach ($contracts as $contract) { - $target = $this->packagePrice($contract); + // Whether the catalogue prices this contract's package at all, asked + // separately from WHICH Price it should be on. A version that was never + // priced is a contract this command has nothing to say about; one that + // is priced but has no Stripe Price for this customer's treatment is a + // contract that must move, and the move is what mints it. + $priced = $prices->rowFor($contract) !== null; + $target = $priced ? $prices->liveForSubscription($contract) : null; - if ($target !== null && $contract->stripe_price_id !== $target) { + if ($priced && $contract->stripe_price_id !== $target) { $this->line(sprintf( ' package %s %s → %s', $contract->uuid, $contract->stripe_price_id ?? '(unknown)', - $target, + // Null where the catalogue has no Price for what this customer + // should be charged. Said out loud rather than skipped: the move + // below parks the reason on the contract and the run warns about + // it, and the cure is one command. + $target ?? '(not synced)', )); $packages++; @@ -128,22 +149,4 @@ class RepriceStripeSubscriptions extends Command 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 09e5e8d..839041a 100644 --- a/app/Console/Commands/SyncStripeCatalogue.php +++ b/app/Console/Commands/SyncStripeCatalogue.php @@ -5,10 +5,10 @@ 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\PlanPrices; use App\Services\Billing\TaxTreatment; use App\Services\Stripe\StripeClient; use Illuminate\Console\Command; @@ -21,12 +21,22 @@ 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. * - * **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. + * **It pushes TWO Prices for everything it sells.** The catalogue's `amount_cents` + * is net and stays net — it is frozen onto every contract and PlanChange prorates + * against it — and what Stripe is asked to take from it depends on who is buying: + * + * - the DOMESTIC GROSS, which is the figure on the website and what everybody + * who is charged VAT pays, private and business alike. Pushing the net was the + * original defect: a customer quoted 214,80 € paid 179,00 € and was then + * invoiced 179,00 € plus 35,80 € VAT that nobody had collected; + * - the BARE NET, for a business in another member state whose VAT id is + * verified. Reverse charge means no VAT is owed to us at all, so the net is + * the whole of what they owe. Charging them the gross was a flat 20 % surcharge + * with no VAT line on the document for them to reclaim. + * + * Which of the two a checkout uses is TaxTreatment's decision — see + * App\Services\Billing\PlanPrices, which owns the plan half of the mirror, and + * App\Services\Billing\AddonPrices, which owns the module half. * * 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 @@ -34,10 +44,11 @@ use Illuminate\Console\Command; * * 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. + * does not gets a NEW Price with the old one archived. A rate change moves the + * gross Price of each pair and leaves the net one untouched, because the net does + * not depend on the rate. 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 at one figure 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 @@ -104,7 +115,7 @@ class SyncStripeCatalogue extends Command foreach ($published as $version) { foreach ($version->prices as $price) { - $created += $this->syncPrice($stripe, $family, $version, $price, $productId, $dryRun); + $created += $this->syncPrice($family, $version, $price, $productId, $dryRun); } } } @@ -127,20 +138,19 @@ class SyncStripeCatalogue extends Command } /** - * The Stripe Price one priced catalogue row is sold on, brought into step. + * The Stripe Prices 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: + * Both treatments, always: the domestic gross AND the bare net a + * reverse-charge business is charged. Unconditionally, rather than only when + * such a customer exists — the Price has to be there BEFORE the checkout that + * needs it, and a verified business meeting a refused checkout is worse than + * an unused Price sitting in Stripe. * - * - 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. + * What "in step" means, and the find-or-mint-or-replace that follows from it, + * belongs to PlanPrices: it is the same question the checkout and the plan + * swap ask, and asking it in two places is how they come to disagree. All + * this adds is the reporting, because an operator running a sync has to be + * able to see which half of which pair moved. * * Archiving stops a Price being offered and leaves every subscription on it * untouched, which is exactly what is wanted: those are moved deliberately, @@ -149,87 +159,48 @@ class SyncStripeCatalogue extends Command * @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); + $prices = app(PlanPrices::class); + $changed = 0; - $existing = StripePlanPrice::query() - ->where('plan_price_id', $price->id) - ->where('charged_cents', $charged) - ->first(); + foreach ($this->treatments() as $label => $treatment) { + if ($prices->inStep($price, $treatment)) { + continue; + } - if ($existing !== null && $existing->isLive() && $price->stripe_price_id === $existing->stripe_price_id) { - return 0; + $this->line(sprintf( + ' price %s v%d %s %s %d %s net → %d %s charged', + $family->key, $version->version, $price->term, $label, + $price->amount_cents, $price->currency, + PlanPrices::chargedCents($price, $treatment), $price->currency, + )); + $changed++; + + // Nothing to create against: the Product for this family is minted + // by the caller, and on a dry run it does not exist yet either. + if ($dryRun || $productId === null) { + continue; + } + + $prices->ensure($price, $treatment); } - $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; + return $changed; } /** - * A Product and two Prices — monthly and yearly — for every module on sale. + * A Product and four Prices for every module on sale: monthly and yearly, + * each at the domestic gross and at the bare net. * - * Both terms, unconditionally, rather than only the ones somebody has bought - * a contract on: the Price has to exist BEFORE the booking that needs it, and - * a customer on a yearly package booking their first module would otherwise - * discover the gap at the moment their money was due. + * Both terms and both treatments, unconditionally, rather than only the ones + * somebody has bought a contract on: the Price has to exist BEFORE the booking + * that needs it, and a customer on a yearly package booking their first module + * would otherwise discover the gap at the moment their money was due. * * @return int how many objects were created, for the run's own count */ @@ -253,32 +224,52 @@ class SyncStripeCatalogue extends Command } 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; + foreach ($this->treatments() as $label => $treatment) { + // 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, $treatment) !== null) { + continue; + } + + $this->line(sprintf( + ' module %s %s %s %d %s net → %d %s charged', + $key, $term, $label, + AddonPrices::termNetCents($monthly, $term), $currency, + AddonPrices::chargedCents($monthly, $term, $treatment), $currency, + )); + $created++; + + if ($dryRun) { + continue; + } + + $prices->ensure($key, $monthly, $currency, $term, $treatment); } - - $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) { - continue; - } - - $prices->ensure($key, $monthly, $currency, $term); } } return $created; } + /** + * The two customers everything on sale is mirrored for, labelled for the + * output so an operator reading a run can see which of a pair moved. + * + * Built from TaxTreatment rather than from a rate written down here: it is the + * single tax authority, and a second list of treatments is how the catalogue + * would come to sell a figure no invoice agrees with. + * + * @return array + */ + private function treatments(): array + { + return [ + 'domestic' => TaxTreatment::domestic(), + 'reverse-charge' => TaxTreatment::reverseCharge(), + ]; + } + /** Versions whose prices are live in Stripe, for the status line. */ public static function syncedVersions(): int { diff --git a/app/Http/Controllers/CheckoutController.php b/app/Http/Controllers/CheckoutController.php index bf97df6..38c1b68 100644 --- a/app/Http/Controllers/CheckoutController.php +++ b/app/Http/Controllers/CheckoutController.php @@ -5,7 +5,9 @@ namespace App\Http\Controllers; use App\Models\Customer; use App\Models\Subscription; use App\Services\Billing\PlanCatalogue; +use App\Services\Billing\PlanPrices; use App\Services\Billing\SetupFee; +use App\Services\Billing\TaxTreatment; use App\Services\Provisioning\HostCapacity; use App\Services\Stripe\StripeClient; use Illuminate\Http\RedirectResponse; @@ -42,8 +44,12 @@ class CheckoutController extends Controller * charged is the one the catalogue holds for the version on sale right now, * never a figure carried in the request. */ - public function start(Request $request, StripeClient $stripe, PlanCatalogue $catalogue): RedirectResponse - { + public function start( + Request $request, + StripeClient $stripe, + PlanCatalogue $catalogue, + PlanPrices $planPrices, + ): RedirectResponse { $data = $request->validate([ 'plan' => ['required', 'string', 'max:64'], 'term' => ['nullable', 'in:'.Subscription::TERM_MONTHLY.','.Subscription::TERM_YEARLY], @@ -62,11 +68,19 @@ class CheckoutController extends Controller $term = $data['term'] ?? Subscription::TERM_MONTHLY; $user = $request->user(); + // Read once and handed to everything below that has to know who is + // buying. Which of a package's two Stripe Prices this session uses is + // decided from it, and a second lookup is a second chance to decide it + // differently. + $customer = $user === null + ? null + : Customer::query()->where('email', $user->email)->first(); + // Somebody who already has a running contract is not buying a second // one by accident: changing package is a plan change, which prorates // and keeps their data where it is. A second checkout would build them // a second, empty cloud and bill for both. - if ($this->hasLiveContract($user?->email)) { + if ($this->hasLiveContract($customer)) { return redirect()->route('billing')->with('status', __('checkout.already_customer')); } @@ -89,26 +103,43 @@ class CheckoutController extends Controller return back()->withErrors(['plan' => __('checkout.plan_gone')]); } - // A price with no Stripe id was never synced. Sending the customer on - // would open a checkout for nothing. - if ($price === null || blank($price->stripe_price_id)) { - Log::error('Checkout for a plan that is not on sale in Stripe', [ + // WHO is buying decides WHICH of the package's two Stripe Prices this + // session uses: the domestic gross for everybody who is charged VAT, the + // bare net for a business in another member state whose VAT id is + // verified, because reverse charge means no VAT is owed to us at all. + // Decided by TaxTreatment and read from PlanPrices — nothing about the + // customer is examined here, or there would be two answers to one + // question and no way to tell which an invoice was written against. + $treatment = TaxTreatment::for($customer); + $priceId = $price === null ? null : $planPrices->liveFor($price, $treatment); + + // A Price Stripe has never been told about. Sending the customer on would + // open a checkout for nothing — and for a reverse-charge business, refusing + // is the only safe answer even though a Price does exist: the domestic one + // would take a fifth more than they owe, which is the overcharge this whole + // rule exists to end. A refused checkout is loud, one command cures it, and + // the log line names the command. + if ($priceId === null) { + Log::error('Checkout for a plan that is not on sale in Stripe. Run stripe:sync-catalogue.', [ 'plan' => $data['plan'], 'term' => $term, + 'reverse_charge' => $treatment->reverseCharge, ]); return back()->withErrors(['plan' => __('checkout.plan_gone')]); } - // The one-off setup fee, gross, or null where the operator has set none. - // Advertised on the price sheet and on the booking page for months and - // charged to nobody; it rides along as a second, non-recurring line on - // this session, which Stripe puts on the initial invoice only. - $setup = SetupFee::checkoutLine((string) $price->currency); + // The one-off setup fee as this customer is charged it, or null where the + // operator has set none. Advertised on the price sheet and on the booking + // page for months and charged to nobody; it rides along as a second, + // non-recurring line on this session, which Stripe puts on the initial + // invoice only. Subject to the same rule as the package: a reverse-charge + // business pays the bare net of it. + $setup = SetupFee::checkoutLine((string) $price->currency, $treatment); try { $url = $stripe->createCheckoutSession( - priceId: (string) $price->stripe_price_id, + priceId: $priceId, successUrl: route('checkout.done'), cancelUrl: route('order'), // Exactly the keys StripeWebhookController reads back. The @@ -157,15 +188,9 @@ class CheckoutController extends Controller return redirect()->route('dashboard')->with('status', __('checkout.thanks')); } - /** Does this address already have a contract that is being billed? */ - private function hasLiveContract(?string $email): bool + /** Does this customer already have a contract that is being billed? */ + private function hasLiveContract(?Customer $customer): bool { - if ($email === null) { - return false; - } - - $customer = Customer::query()->where('email', $email)->first(); - return $customer !== null && Subscription::query() ->where('customer_id', $customer->id) ->whereIn('status', ['active', 'past_due']) diff --git a/app/Http/Controllers/LandingController.php b/app/Http/Controllers/LandingController.php index f4deab8..5e0af28 100644 --- a/app/Http/Controllers/LandingController.php +++ b/app/Http/Controllers/LandingController.php @@ -217,7 +217,7 @@ class LandingController extends Controller 'label' => rtrim(rtrim(number_format($tax, 1, ',', '.'), '0'), ','), ], 'setup' => $setupNet === 0 ? null : [ - 'gross' => $this->money(SetupFee::chargedCents(), Subscription::catalogueCurrency()), + 'gross' => $this->money(SetupFee::chargedCents(TaxTreatment::domestic()), Subscription::catalogueCurrency()), 'net' => $this->money($setupNet, Subscription::catalogueCurrency()), ], 'baseline' => $this->baseline($plans), @@ -562,23 +562,23 @@ class LandingController extends Controller * "179,00". */ /** - * A net figure as it is actually charged. + * A net figure as the ordinary customer is charged it. * * 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. + * call the domestic Stripe Price is formed with, so the figure on this page and + * the figure on the card statement 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, 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. + * This page is read by visitors nobody has met, so the domestic gross is the + * only honest figure to put on it. A business in another member state with a + * verified VAT id is charged less — the bare net, no VAT owed to us at all — + * and learns that on the pages that know who they are, not here, where + * advertising it would quote every private visitor a price they cannot have. */ private function gross(int $netCents): int { - return TaxTreatment::chargedCents($netCents); + return TaxTreatment::advertisedCents($netCents); } private function money(int $cents, string $currency): string diff --git a/app/Livewire/Order.php b/app/Livewire/Order.php index 6a554ae..d8f309d 100644 --- a/app/Livewire/Order.php +++ b/app/Livewire/Order.php @@ -6,6 +6,7 @@ use App\Models\Customer; use App\Models\Subscription; use App\Services\Billing\PlanCatalogue; use App\Services\Billing\SetupFee; +use App\Services\Billing\TaxTreatment; use App\Services\Provisioning\HostCapacity; use App\Support\CompanyProfile; use Illuminate\Support\Facades\Auth; @@ -20,11 +21,14 @@ use Throwable; * the same working day. That is an afternoon of somebody's time per customer, * for a product whose whole point is that the machine does the work. * - * The prices here are the catalogue's, gross, exactly as the public sheet shows - * them: a customer who was quoted 58,80 € must not meet 49 € on the page where - * they press the button, nor the other way round. The delivery promise is the - * estate's own answer (HostCapacity), so nobody is told "sofort" while a server - * still has to be bought. + * The prices here are the catalogue's at the till, exactly as the public sheet + * shows them: a customer who was quoted 58,80 € must not meet 49 € on the page + * where they press the button, nor the other way round. This page differs from the + * public sheet in one respect, and it is allowed to because it knows who is + * reading it: a business in another member state with a verified VAT id is quoted + * — and charged — the bare net, because reverse charge means no VAT is owed to us + * at all. The delivery promise is the estate's own answer (HostCapacity), so + * nobody is told "sofort" while a server still has to be bought. * * Nothing is charged here. The button opens a Stripe hosted checkout; the * purchase becomes real on the webhook, which is the only place allowed to @@ -46,26 +50,40 @@ class Order extends Component ->whereIn('status', ['active', 'past_due']) ->first(); + // Resolved once and used for every figure on the page, so the package + // price, the setup fee and the sentence about VAT underneath them cannot + // state three different treatments of one sale. It is the same call the + // checkout makes to pick the Stripe Price, so what is shown here is what + // will be taken. + $tax = TaxTreatment::for($customer); + return view('livewire.order', [ - 'plans' => $this->plans(), + 'plans' => $this->plans($tax), + 'tax' => $tax, 'contract' => $contract, 'vat' => rtrim(rtrim(number_format(CompanyProfile::taxRate(), 1, ',', '.'), '0'), ','), - // Gross, like the package price above it and like the public sheet, - // which quotes "Einrichtung kostet einmalig 118,80 €". This page was - // quoting the NET figure under the same sentence, so the site said - // 118,80 and the page with the button on it said 99,00 — and the - // checkout now charges the gross of it, which is what a visitor was - // shown out there. - 'setup' => SetupFee::chargedCents(), + // At the till, like the package price above it and like the public + // sheet, which quotes "Einrichtung kostet einmalig 118,80 €". This page + // was quoting the NET figure under the same sentence, so the site said + // 118,80 and the page with the button on it said 99,00. Through the + // same treatment as the package, because a fee is a supply like any + // other and cannot be taxed differently from the thing it sets up. + 'setup' => SetupFee::chargedCents($tax), ]); } /** - * The packages on sale, in the words and figures the public sheet uses. + * The packages on sale, in the words and figures the public sheet uses — + * priced for whoever is reading them. + * + * `gross` is what this customer's card would be charged, formed by + * TaxTreatment and not by a multiplication of its own. It used to be the + * latter, which is a second source for a number the checkout also forms, and + * therefore a second answer waiting to happen. * * @return array> */ - private function plans(): array + private function plans(TaxTreatment $tax): array { try { $sellable = app(PlanCatalogue::class)->sellable(); @@ -78,7 +96,6 @@ class Order extends Component } $capacity = app(HostCapacity::class); - $rate = CompanyProfile::taxRate(); $plans = []; foreach ($sellable as $key => $plan) { @@ -88,7 +105,7 @@ class Order extends Component 'key' => $key, 'name' => (string) $plan['name'], 'audience' => (string) ($plan['audience'] ?? ''), - 'gross' => (int) round($net * (1 + $rate / 100)), + 'gross' => $tax->chargeCents($net), 'net' => $net, 'currency' => (string) $plan['currency'], 'quota_gb' => (int) $plan['quota_gb'], diff --git a/app/Models/Order.php b/app/Models/Order.php index 0224581..84a8320 100644 --- a/app/Models/Order.php +++ b/app/Models/Order.php @@ -89,15 +89,17 @@ class Order extends Model implements ProvisioningSubject } /** - * What this order actually cost the customer, tax included. + * What this order actually cost the customer, whatever tax was in it. * * `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. + * `amount_total` — what was actually taken from the card, whichever of a + * package's two Prices this customer was sold on, so it needs nothing added to + * it. 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 through this customer's own treatment: the + * domestic gross where VAT is charged, the bare net for a reverse-charge + * business. * * 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 @@ -107,7 +109,7 @@ class Order extends Model implements ProvisioningSubject { return $this->stripe_event_id !== null ? (int) $this->amount_cents - : TaxTreatment::chargedCents((int) $this->amount_cents); + : $this->taxTreatment()->chargeCents((int) $this->amount_cents); } /** diff --git a/app/Models/StripeAddonPrice.php b/app/Models/StripeAddonPrice.php index 242ceb0..da49cb1 100644 --- a/app/Models/StripeAddonPrice.php +++ b/app/Models/StripeAddonPrice.php @@ -13,10 +13,17 @@ use Illuminate\Database\Eloquent\Model; * 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. + * `amount_cents` is what the Price CHARGES; `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. + * + * `reverse_charge` is the other half of a module's identity, and it is why there + * are four Prices per module and not two. A customer who is charged VAT pays the + * gross of the catalogue figure; a business in another member state with a + * verified VAT id owes the bare net and is charged exactly that, so for them + * `amount_cents` and `net_cents` are the same number. Which of the two a booking + * bills on is TaxTreatment's decision — see App\Services\Billing\AddonPrices. */ class StripeAddonPrice extends Model { @@ -27,6 +34,7 @@ class StripeAddonPrice extends Model return [ 'amount_cents' => 'integer', 'net_cents' => 'integer', + 'reverse_charge' => 'boolean', 'archived_at' => 'datetime', ]; } diff --git a/app/Models/StripePlanPrice.php b/app/Models/StripePlanPrice.php index 6a5854f..4906d70 100644 --- a/app/Models/StripePlanPrice.php +++ b/app/Models/StripePlanPrice.php @@ -8,12 +8,25 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; /** * One Stripe Price a priced catalogue row has had, live or superseded. * - * `plan_prices.stripe_price_id` points at the one that is on sale; this is the - * history behind it, and it exists because a Stripe Price cannot be edited. When - * the amount a row is charged at changes — the move from net to gross, or a - * change to the VAT rate — a new Price is minted, this row is archived, and the - * old Price stays live in Stripe for as long as a contract is still billing on - * it. See the migration for why that history has to be readable. + * Two of them are live at any moment, not one: `reverse_charge` says whether a + * row is the DOMESTIC GROSS Price, which is what the website quotes and what + * everybody who is charged VAT pays, or the BARE NET one, which is the whole of + * what a business in another member state with a verified VAT id owes. Which of + * the two a checkout uses is TaxTreatment's decision — see + * App\Services\Billing\PlanPrices. + * + * `plan_prices.stripe_price_id` points at the domestic one, because that is the + * ordinary sale and it must have a single source; the net one is only ever read + * from here. And because a Price id is unique in this table, joining it on + * `subscriptions.stripe_price_id` is what says which of the two a running + * contract is actually billed on. + * + * The rest is history, and it exists because a Stripe Price cannot be edited. + * When the amount a row is charged at changes — a change to the VAT rate moves + * the gross Price and leaves the net one alone — a new Price is minted, the old + * row is archived, and the old Price stays in Stripe for as long as a contract is + * still billing on it. See the migrations for why that history has to be + * readable. */ class StripePlanPrice extends Model { @@ -23,6 +36,7 @@ class StripePlanPrice extends Model { return [ 'charged_cents' => 'integer', + 'reverse_charge' => 'boolean', 'archived_at' => 'datetime', ]; } diff --git a/app/Services/Billing/AddonPrices.php b/app/Services/Billing/AddonPrices.php index 62f1312..6973cdd 100644 --- a/app/Services/Billing/AddonPrices.php +++ b/app/Services/Billing/AddonPrices.php @@ -29,13 +29,18 @@ use Illuminate\Database\UniqueConstraintViolationException; * 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. + * **What the Price charges depends on who is billed.** `amount_cents` on the row + * is what Stripe takes: the catalogue's net figure with the domestic rate on it + * for everybody who is charged VAT, because the figure on the price sheet is the + * figure charged — and the bare net for a business in another member state whose + * VAT id is verified, because reverse charge means no VAT is owed to us at all + * and the net is the whole of it. So a module has TWO live Prices per interval, + * not one, and `reverse_charge` on the row says which is which. + * + * `net_cents` beside it is the catalogue figure the Price 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 { @@ -55,10 +60,17 @@ final class AddonPrices 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 + /** + * What Stripe is asked to take for a term of this module from a customer + * treated so. + * + * The treatment is a parameter rather than the domestic one assumed, because + * a reverse-charge business owes the bare net and there is a Price of their + * own for it. Grossed ONCE, after the term is formed, for the reason above. + */ + public static function chargedCents(int $monthlyNetCents, string $term, TaxTreatment $treatment): int { - return TaxTreatment::chargedCents(self::termNetCents($monthlyNetCents, $term)); + return $treatment->chargeCents(self::termNetCents($monthlyNetCents, $term)); } /** @@ -69,20 +81,27 @@ final class AddonPrices * a zero-amount item on a subscription is a line on every invoice that says * the customer owes nothing for it. */ - public function ensure(string $addonKey, int $unitNetCents, string $currency, string $term): ?string - { + public function ensure( + string $addonKey, + int $unitNetCents, + string $currency, + string $term, + TaxTreatment $treatment, + ): ?string { if ($unitNetCents <= 0) { return null; } $interval = $term === Subscription::TERM_YEARLY ? 'year' : 'month'; $currency = strtoupper($currency); + $reverseCharge = $treatment->reverseCharge; $netCents = self::termNetCents($unitNetCents, $term); - $amount = TaxTreatment::chargedCents($netCents); + $amount = $treatment->chargeCents($netCents); $existing = StripeAddonPrice::query() ->where('addon_key', $addonKey) + ->where('reverse_charge', $reverseCharge) ->where('amount_cents', $amount) ->where('currency', $currency) ->where('interval', $interval) @@ -91,9 +110,15 @@ final class AddonPrices if ($existing !== null) { // 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. + // leave a second Stripe Price for one figure. Brought back at Stripe + // too where it had been archived there: a Price that is only live in + // our own table is one a checkout is refused for. + if ($existing->archived_at !== null) { + $this->stripe->activatePrice((string) $existing->stripe_price_id); + } + $existing->update(['archived_at' => null, 'net_cents' => $netCents]); - $this->archiveSuperseded($addonKey, $netCents, $currency, $interval, $amount); + $this->archiveSuperseded($addonKey, $reverseCharge, $netCents, $currency, $interval, $amount); return (string) $existing->stripe_price_id; } @@ -109,17 +134,27 @@ final class AddonPrices // Read back when a Stripe invoice line has to be turned into // wording a customer can read — see StripeInvoiceLines. 'addon' => $addonKey, + // Which of the module's two Prices this is, for anyone reading + // Stripe's own dashboard, where they would otherwise differ only + // by an amount. + 'tax_treatment' => $reverseCharge ? 'reverse_charge' : 'domestic', ], // Keyed on what the Price IS, so a crash between Stripe creating it // and us storing its id gives back the same Price on the next // attempt rather than a second one at the same money. The amount is // the CHARGED one, so the move from net to gross does not replay the - // net Price the old key was minted under. - idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}", + // net Price the old key was minted under — and the treatment is in + // there because at a rate of nought the two Prices are the same + // amount: one key would have Stripe hand the same object back for + // both, and the two rows would then share a Price, so archiving the + // gross one at the next rate change would withdraw the very Price the + // net side is still selling. + idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}" + .($reverseCharge ? '-rc' : ''), ); - $this->remember($addonKey, $amount, $netCents, $currency, $interval, $productId, $priceId); - $this->archiveSuperseded($addonKey, $netCents, $currency, $interval, $amount); + $this->remember($addonKey, $reverseCharge, $amount, $netCents, $currency, $interval, $productId, $priceId); + $this->archiveSuperseded($addonKey, $reverseCharge, $netCents, $currency, $interval, $amount); return $priceId; } @@ -129,27 +164,40 @@ final class AddonPrices * * 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. + * that knows the rate — and from the treatment, because a reverse-charge + * business is billed on a Price of their own at the bare net. */ - public function liveFor(string $addonKey, int $unitNetCents, string $currency, string $term): ?string - { + public function liveFor( + string $addonKey, + int $unitNetCents, + string $currency, + string $term, + TaxTreatment $treatment, + ): ?string { if ($unitNetCents <= 0) { return null; } return $this->find( $addonKey, - self::chargedCents($unitNetCents, $term), + self::chargedCents($unitNetCents, $term, $treatment), $currency, $term === Subscription::TERM_YEARLY ? 'year' : 'month', + $treatment->reverseCharge, ); } /** The live Price we already have for this charged figure, or null. */ - public function find(string $addonKey, int $amountCents, string $currency, string $interval): ?string - { + public function find( + string $addonKey, + int $amountCents, + string $currency, + string $interval, + bool $reverseCharge, + ): ?string { $id = StripeAddonPrice::query() ->where('addon_key', $addonKey) + ->where('reverse_charge', $reverseCharge) ->where('amount_cents', $amountCents) ->where('currency', strtoupper($currency)) ->where('interval', $interval) @@ -165,8 +213,13 @@ final class AddonPrices * 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. + * Price for today's figure at yesterday's rate. + * + * And only ones for the same TREATMENT. A change of rate moves the gross + * Price and leaves the net one exactly where it was, so a sweep that did not + * know the difference would archive the half it had not just replaced — and + * every reverse-charge business would find their module Price withdrawn the + * next time the VAT rate moved. * * Archived in Stripe as well as here. The Price object stays, as Stripe * requires, because a contract may still be billing on it until @@ -174,6 +227,7 @@ final class AddonPrices */ private function archiveSuperseded( string $addonKey, + bool $reverseCharge, int $netCents, string $currency, string $interval, @@ -181,6 +235,7 @@ final class AddonPrices ): void { $superseded = StripeAddonPrice::query() ->where('addon_key', $addonKey) + ->where('reverse_charge', $reverseCharge) ->where('net_cents', $netCents) ->where('currency', $currency) ->where('interval', $interval) @@ -220,6 +275,7 @@ final class AddonPrices private function remember( string $addonKey, + bool $reverseCharge, int $amountCents, int $netCents, string $currency, @@ -230,6 +286,7 @@ final class AddonPrices try { StripeAddonPrice::create([ 'addon_key' => $addonKey, + 'reverse_charge' => $reverseCharge, 'amount_cents' => $amountCents, 'net_cents' => $netCents, 'currency' => $currency, diff --git a/app/Services/Billing/IssueInvoice.php b/app/Services/Billing/IssueInvoice.php index dd96702..2af1f45 100644 --- a/app/Services/Billing/IssueInvoice.php +++ b/app/Services/Billing/IssueInvoice.php @@ -30,12 +30,14 @@ use RuntimeException; * * ## 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. + * Everything sold from the catalogue is priced at the till: the price sheet, the + * Stripe Price and the card charge are one figure — the domestic gross for + * everybody who is charged VAT, the bare net for a reverse-charge business, whose + * Stripe Price carries exactly that. 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 @@ -135,10 +137,11 @@ final class IssueInvoice * * 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. + * net price put through this customer's treatment, which is the amount the + * Stripe Price this contract is billed on carries — the domestic gross where + * VAT is charged, the bare net for a reverse-charge business. `price_cents` + * itself stays the catalogue's net figure and is not by itself what a document + * totals to. * * **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 @@ -180,7 +183,8 @@ final class IssueInvoice ])], 'quantity_milli' => 1000, 'unit' => '', - 'unit_net_cents' => $chargedCents ?? TaxTreatment::chargedCents((int) $subscription->price_cents), + 'unit_net_cents' => $chargedCents + ?? TaxTreatment::for($customer)->chargeCents((int) $subscription->price_cents), ]]; return $this->issue( @@ -443,10 +447,11 @@ final class IssueInvoice $rate = $treatment->basisPoints(); 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. + // The charged figure is the fixed point, whatever it was: at a rate of + // zero the whole of it becomes net rather than the document shrinking + // by a VAT the price never contained. That is what keeps the total + // equal to the money for a reverse-charge business, who is charged the + // bare net and whose document therefore states that same net at 0 %. ['lines' => $lines, 'totals' => $totals] = InvoiceMath::fromCharged($lines, $rate); $totals += ['rate_basis_points' => $rate]; } else { diff --git a/app/Services/Billing/PlanPrices.php b/app/Services/Billing/PlanPrices.php new file mode 100644 index 0000000..ed8d631 --- /dev/null +++ b/app/Services/Billing/PlanPrices.php @@ -0,0 +1,259 @@ +chargeCents((int) $price->amount_cents); + } + + /** + * The Price a customer with this treatment is sold on, or null where the + * catalogue has never been mirrored. + * + * The domestic answer falls back to the catalogue row's own column, which is + * where it lived before this register existed and where an install that has + * not run the sync since still has it. There is deliberately NO such fallback + * for a reverse-charge business: handing them the domestic Price is the + * overcharge this whole file exists to end, and refusing the sale until the + * catalogue is synced is the one outcome that cannot take money nobody owes. + */ + public function liveFor(PlanPrice $price, TaxTreatment $treatment): ?string + { + $registered = $this->registered($price, $treatment); + + if ($registered !== null) { + return (string) $registered->stripe_price_id; + } + + if ($treatment->reverseCharge) { + return null; + } + + return blank($price->stripe_price_id) ? null : (string) $price->stripe_price_id; + } + + /** + * Is Stripe selling this row at the right figure for this treatment? + * + * Asked of the CHARGED figure and of the pointer together, which is what + * makes a second sync run free and a run that died halfway recoverable: a + * rate change leaves the register live at yesterday's amount, and a crash + * between Stripe minting the Price and us storing its id leaves the register + * right and the catalogue row empty. + */ + public function inStep(PlanPrice $price, TaxTreatment $treatment): bool + { + $registered = $this->registered($price, $treatment); + + if ($registered === null) { + return false; + } + + // The net Price has no pointer to be out of step with — see the class + // comment on where each of the two is written down. + return $treatment->reverseCharge + || $price->stripe_price_id === $registered->stripe_price_id; + } + + /** + * The Price this row is sold on for this treatment, created at Stripe if it + * has never been told about it. + * + * Three outcomes, decided by the charged figure and never by whether an id + * happens to be stored: + * + * - the register is live at that figure and the catalogue row points at it: + * nothing to do; + * - 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, at Stripe + * as well as here, rather than minted a second time; + * - nothing charges it: a new Price, the pointer moved, and whatever it was + * pointing at archived — after the replacement exists, never before, so a + * failure in between leaves the old Price selling rather than nothing. + * + * Null only where the family has no Product yet, which means the run that + * creates Products has not reached it. + */ + public function ensure(PlanPrice $price, TaxTreatment $treatment): ?string + { + $version = $price->version; + $family = $version?->family; + $productId = $family?->stripe_product_id; + + if ($version === null || $family === null || blank($productId)) { + return null; + } + + $charged = self::chargedCents($price, $treatment); + $existing = $this->registered($price, $treatment, includeArchived: true); + + $priceId = $existing?->stripe_price_id; + + if ($priceId === null) { + $priceId = $this->stripe->createPrice( + productId: (string) $productId, + amountCents: $charged, + currency: (string) $price->currency, + interval: $price->term === Subscription::TERM_YEARLY ? 'year' : 'month', + metadata: [ + 'plan_family' => $family->key, + 'plan_version' => (string) $version->version, + 'plan_version_id' => (string) $version->id, + 'plan_price_id' => (string) $price->id, + // Read back by anything that has a Price id and needs to know + // what kind of sale it was — and by a person looking at + // Stripe's own dashboard, where two Prices on one Product + // would otherwise differ only by an amount. + 'tax_treatment' => $treatment->reverseCharge ? 'reverse_charge' : 'domestic', + ], + // The CHARGED amount is part of the key, so a run after a rate + // change cannot replay the Price minted at the old figure. So is + // the treatment, and it has to be: at a rate of nought the two + // Prices are the same amount, and one key would have Stripe hand + // back the same object for both — which the register, where a + // Price id is unique, would refuse to record twice. + idempotencyKey: "clupilot-price-{$price->id}-{$charged}" + .($treatment->reverseCharge ? '-rc' : ''), + ); + } elseif ($existing?->archived_at !== null) { + // Unarchived at Stripe too. Bringing it back only in our own table + // would leave the catalogue pointing at a Price Stripe refuses to + // open a checkout for, which is the failure a customer meets and + // nobody else does. + $this->stripe->activatePrice($priceId); + } + + StripePlanPrice::query()->updateOrCreate( + [ + 'plan_price_id' => $price->id, + 'reverse_charge' => $treatment->reverseCharge, + 'charged_cents' => $charged, + ], + ['stripe_price_id' => $priceId, 'archived_at' => null], + ); + + $this->archiveSuperseded($price, $treatment, $charged); + + // The pointer is the domestic Price and only ever that. 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. + if (! $treatment->reverseCharge) { + PlanPrice::query()->whereKey($price->id)->update(['stripe_price_id' => $priceId]); + } + + return $priceId; + } + + /** The priced catalogue row a contract is billed through, or null. */ + public function rowFor(Subscription $subscription): ?PlanPrice + { + return PlanPrice::query() + ->where('plan_version_id', $subscription->plan_version_id) + ->where('term', $subscription->term) + ->first(); + } + + /** + * The Price this contract should be billed on, given who is paying for it. + * + * By plan VERSION and term, because that pair is what a Stripe Price is — + * resolving it by plan name would hand a grandfathered contract today's + * terms — and then by the customer's own treatment, because a business whose + * registration was verified after they bought belongs on the net Price from + * that moment, and one whose registration has lapsed belongs back on the + * gross one. + */ + public function liveForSubscription(Subscription $subscription): ?string + { + $row = $this->rowFor($subscription); + + return $row === null + ? null + : $this->liveFor($row, TaxTreatment::for($subscription->customer)); + } + + /** The register's row for this figure, live unless asked otherwise. */ + private function registered( + PlanPrice $price, + TaxTreatment $treatment, + bool $includeArchived = false, + ): ?StripePlanPrice { + return StripePlanPrice::query() + ->where('plan_price_id', $price->id) + ->where('reverse_charge', $treatment->reverseCharge) + ->where('charged_cents', self::chargedCents($price, $treatment)) + ->when(! $includeArchived, fn ($query) => $query->whereNull('archived_at')) + ->first(); + } + + /** + * Stop offering every other Price this row was sold on to customers treated + * the same way. + * + * Within ONE treatment, which is the whole reason `reverse_charge` is part of + * the register's key: a change of rate moves the gross Price and leaves the + * net one exactly where it was, and a sweep that did not know the difference + * would archive the half it had not just replaced. + * + * 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(PlanPrice $price, TaxTreatment $treatment, int $keepChargedCents): void + { + $superseded = StripePlanPrice::query() + ->where('plan_price_id', $price->id) + ->where('reverse_charge', $treatment->reverseCharge) + ->where('charged_cents', '!=', $keepChargedCents) + ->whereNull('archived_at') + ->get(); + + foreach ($superseded as $old) { + $this->stripe->archivePrice((string) $old->stripe_price_id); + $old->update(['archived_at' => now()]); + } + } +} diff --git a/app/Services/Billing/SetupFee.php b/app/Services/Billing/SetupFee.php index a40f3d3..680024f 100644 --- a/app/Services/Billing/SetupFee.php +++ b/app/Services/Billing/SetupFee.php @@ -19,13 +19,19 @@ use App\Support\CompanyProfile; * this class, so a change of rate or of fee cannot move one without moving the * others. * - * **The figure is GROSS, like every other price this platform charges.** The - * console asks the operator for the net amount, because that is the commercial - * number and the field says so; what a customer pays is that with the domestic - * rate on it, formed by TaxTreatment::chargedCents(), which is the single place a - * gross is formed. That is also why the fee is the same for a consumer in Vienna - * and for a reverse-charge business in Rotterdam: the price on the page is the - * price at the till, for everybody, and the invoice is where the split differs. + * **What is charged for it follows the same rule as a package.** The console asks + * the operator for the NET amount, because that is the commercial number and the + * field says so. What a customer pays is that amount put through their own + * TaxTreatment: the domestic gross for everybody who is charged VAT, so the + * figure at the till is the figure the price sheet quoted — and the bare net for + * a business in another member state whose VAT id is verified, because reverse + * charge means no VAT is owed to us on the fee either. A fee is a supply like any + * other; taxing it differently from the package on the same invoice would be a + * document at two rates. + * + * The public price sheet quotes the domestic gross, because it is read by + * visitors nobody has met — TaxTreatment::advertisedCents(), the same figure the + * ordinary checkout takes. * * Zero means there is no such fee. Every reader has to treat that as "no line at * all" rather than "a line of nought" — a checkout that itemises nothing owed is @@ -39,12 +45,18 @@ final class SetupFee return CompanyProfile::setupFeeCents(); } - /** What a customer is actually charged for it, tax included. */ - public static function chargedCents(): int + /** + * What a customer treated so is actually charged for it. + * + * The treatment is a required argument rather than the domestic one assumed, + * so that every call site has to say whose fee it is quoting. The page that + * quotes it to a stranger passes TaxTreatment::domestic(), and says so. + */ + public static function chargedCents(TaxTreatment $treatment): int { $net = self::netCents(); - return $net === 0 ? 0 : TaxTreatment::chargedCents($net); + return $net === 0 ? 0 : $treatment->chargeCents($net); } /** @@ -58,9 +70,9 @@ final class SetupFee * * @return array{amount_cents: int, label: string, currency: string}|null */ - public static function checkoutLine(string $currency): ?array + public static function checkoutLine(string $currency, TaxTreatment $treatment): ?array { - $charged = self::chargedCents(); + $charged = self::chargedCents($treatment); if ($charged === 0) { return null; diff --git a/app/Services/Billing/StripeInvoiceLines.php b/app/Services/Billing/StripeInvoiceLines.php index 47fb6da..e8d8195 100644 --- a/app/Services/Billing/StripeInvoiceLines.php +++ b/app/Services/Billing/StripeInvoiceLines.php @@ -29,11 +29,17 @@ 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 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. + * Amounts are what was CHARGED, which is not the same figure for everybody: our + * catalogue is pushed to Stripe twice, at the domestic gross and at the bare net, + * and the Price a contract is billed on decides which of the two its lines carry. + * IssueInvoice divides the sum according to the customer's own treatment rather + * than adding a rate to it — so a domestic line is split into net and VAT, and a + * reverse-charge line, which contained no VAT to begin with, becomes net at 0 %. + * Either way the document totals to the money. See TaxTreatment. + * + * Naming a line is unaffected by there being two Prices per sellable thing: a + * Price id is unique in both registers, so it resolves to one catalogue row + * whichever of the pair it is. */ final class StripeInvoiceLines { diff --git a/app/Services/Billing/TaxTreatment.php b/app/Services/Billing/TaxTreatment.php index 20d5073..5d5b35e 100644 --- a/app/Services/Billing/TaxTreatment.php +++ b/app/Services/Billing/TaxTreatment.php @@ -39,18 +39,27 @@ use App\Support\CompanyProfile; * 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 price at the till is the domestic gross — except where no VAT is charged * - * 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. + * The treatment above decides how a DOCUMENT is split, and it also decides the + * amount taken from the card: chargeCents() below. For everybody who is charged + * VAT that amount is the domestic gross, and the owner's rule holds exactly — + * the figure on the website is the figure charged, for a private person and for + * an Austrian business alike, because the VAT is a pass-through that a business + * reclaims as input tax. * - * 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. + * A reverse-charge sale carries no VAT at all, and there the same rule points + * the other way: the bare net figure is the whole of what is owed, so the bare + * net figure is what is charged. Charging them the gross was a flat 20 % + * surcharge with no VAT line anywhere for them to reclaim, and their document + * then stated the whole of it as net at 0 % — so they self-accounted their own + * VAT on a base a fifth too large. The rule is: Austria B2B 20 %, other EU B2B + * without VAT. + * + * That is why a Stripe Price CAN ask who is buying. There are two per sellable + * thing — the domestic gross and the bare net — both live, and the checkout + * picks by this class. See App\Services\Billing\PlanPrices and + * App\Services\Billing\AddonPrices. * * Stripe's own `automatic_tax` is deliberately NOT enabled anywhere. This class * is the single tax authority here, knowingly over-collecting on cross-border @@ -81,7 +90,7 @@ final readonly class TaxTreatment * * 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 + * goes through chargeCents() below rather than multiplying by a percentage * of its own. */ public static function domestic(): self @@ -90,17 +99,34 @@ final readonly class TaxTreatment } /** - * What a net catalogue figure is actually charged as. + * The intra-EU business treatment itself, with nobody in particular in mind. * - * 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. + * For the catalogue mirror, which has to create the Price a reverse-charge + * business will be sold on BEFORE any such customer has reached the checkout — + * so it needs the treatment without having anybody to derive it from. Who + * actually GETS it is for() below and is never decided here: this is a rate + * and a flag, not a judgement about a customer. */ - public static function chargedCents(int $netCents): int + public static function reverseCharge(): self { - return self::domestic()->grossCents($netCents); + return new self(0.0, true); + } + + /** + * The figure the website quotes, with nobody in particular in mind. + * + * The public price sheet is read by visitors nobody has met, so it can only + * state the domestic gross — and it must state the same number the ordinary + * checkout takes, which is what makes this the domestic treatment's own + * chargeCents() rather than a second multiplication. + * + * A reverse-charge business is charged less than this and is shown so on the + * pages that know who they are. Advertising the net out here instead is not + * an option: it would quote every private visitor a price they cannot have. + */ + public static function advertisedCents(int $netCents): int + { + return self::domestic()->chargeCents($netCents); } public static function for(?Customer $customer): self @@ -134,7 +160,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) : $domestic; + return $eligible ? self::reverseCharge() : $domestic; } public function grossCents(int $netCents): int @@ -142,6 +168,27 @@ final readonly class TaxTreatment return (int) round($netCents * (1 + $this->rate)); } + /** + * What THIS customer's card is charged for a net catalogue figure. + * + * The one place the amount taken is formed, and therefore the one place that + * decides which of a sellable thing's two Stripe Prices a checkout uses, what + * the proof register expects to have been taken, and what a document has to + * total to. A second reader forming the figure from a rate of its own is how + * the charge, the Price and the invoice come to disagree. + * + * Written on the reverse-charge flag rather than on the rate, although at a + * rate of nought grossCents() would return the same number. The two are + * different questions that happen to share an answer here: grossCents() says + * how a document is SPLIT, and this says what is TAKEN — and if cross-border + * B2C is ever taxed at the customer's own rate under OSS, the two part + * company and this must go on charging what was quoted. + */ + public function chargeCents(int $netCents): int + { + return $this->reverseCharge ? $netCents : $this->grossCents($netCents); + } + /** * The rate in the form the invoice arithmetic works in — 2000 for 20 %. * diff --git a/app/Services/Mail/MailTemplateRenderer.php b/app/Services/Mail/MailTemplateRenderer.php index 8ca7b39..ba5e563 100644 --- a/app/Services/Mail/MailTemplateRenderer.php +++ b/app/Services/Mail/MailTemplateRenderer.php @@ -7,6 +7,7 @@ use App\Models\Instance; use App\Models\MailTemplate; use App\Models\Operator; use App\Models\Subscription; +use App\Services\Billing\TaxTreatment; use App\Support\CompanyProfile; use Illuminate\Support\Number; @@ -96,10 +97,10 @@ final class MailTemplateRenderer 'contact' => (string) ($customer->contact_name ?: $customer->name), 'email' => (string) $customer->email, 'plan' => $subscription === null ? '' : __('billing.plan.'.$subscription->plan), - // Gross, because that is the figure the customer knows: it is what - // the price sheet quoted them and what their bank statement says. + // What was actually charged, because that is the figure the customer + // knows: it is what their bank statement says. 'amount' => $subscription === null ? '' : Number::currency( - $this->grossCents($subscription) / 100, + $this->chargedCents($subscription, $customer) / 100, in: (string) ($subscription->currency ?: 'EUR'), locale: app()->getLocale(), ), @@ -125,17 +126,18 @@ final class MailTemplateRenderer } /** - * The contract's monthly figure with VAT on it. + * The contract's monthly figure as this customer is charged it. * - * The domestic rate, from the same place an invoice reads it. Reverse charge - * is deliberately not applied: this figure goes into a sentence a person - * reads, and a business quoting itself a net price in a mail is a different - * problem from a document that has to be lawful. + * Through TaxTreatment, which is the one place the amount taken is formed, and + * therefore the same figure the Stripe Price for this contract carries. It used + * to be a multiplication at the domestic rate written out here, with the + * reasoning that reverse charge was a matter for documents and not for + * sentences — and that stopped being true the moment a reverse-charge business + * began to be charged the bare net. A mail naming a fifth more than the bank + * statement is a mail the customer writes back about. */ - private function grossCents(Subscription $subscription): int + private function chargedCents(Subscription $subscription, Customer $customer): int { - $net = (int) $subscription->price_cents; - - return (int) round($net * (1 + CompanyProfile::taxRate() / 100)); + return TaxTreatment::for($customer)->chargeCents((int) $subscription->price_cents); } } diff --git a/app/Services/Stripe/FakeStripeClient.php b/app/Services/Stripe/FakeStripeClient.php index 0beaee1..5a5398d 100644 --- a/app/Services/Stripe/FakeStripeClient.php +++ b/app/Services/Stripe/FakeStripeClient.php @@ -25,6 +25,15 @@ class FakeStripeClient implements StripeClient /** @var array */ public array $archived = []; + /** + * Every Price that was put back on sale, in order — a rate that moved and + * moved back reaches for the Price it already has rather than minting a + * second one for the same amount. + * + * @var array + */ + public array $activated = []; + /** Idempotency key → the id first returned for it. @var array */ public array $keys = []; @@ -189,6 +198,19 @@ class FakeStripeClient implements StripeClient $this->archived[] = $priceId; } + public function activatePrice(string $priceId): void + { + // Taken off the archived list rather than merely appended to a second + // one, so a test asking "is this Price still being sold?" gets the + // answer Stripe would give instead of a history it has to interpret. + $this->archived = array_values(array_filter( + $this->archived, + fn (string $id) => $id !== $priceId, + )); + + $this->activated[] = $priceId; + } + public function updateSubscriptionPrice( string $subscriptionId, string $itemId, diff --git a/app/Services/Stripe/HttpStripeClient.php b/app/Services/Stripe/HttpStripeClient.php index d96a879..a88a1a0 100644 --- a/app/Services/Stripe/HttpStripeClient.php +++ b/app/Services/Stripe/HttpStripeClient.php @@ -131,6 +131,17 @@ class HttpStripeClient implements StripeClient ->throw(); } + public function activatePrice(string $priceId): void + { + // `active` is one of the very few fields a Stripe Price does let you + // change, which is what makes bringing one back possible at all — the + // amount is not, and that is why a changed figure is always a new Price. + $this->request() + ->asForm() + ->post($this->url('prices/'.$priceId), ['active' => 'true']) + ->throw(); + } + public function updateSubscriptionPrice( string $subscriptionId, string $itemId, diff --git a/app/Services/Stripe/StripeClient.php b/app/Services/Stripe/StripeClient.php index 92dc03c..841db2d 100644 --- a/app/Services/Stripe/StripeClient.php +++ b/app/Services/Stripe/StripeClient.php @@ -66,8 +66,16 @@ interface StripeClient * subscription: the session's copy is what StripeWebhookController reads, * the subscription's is what any later billing event carries. * + * `$priceId` is one of the TWO Prices the catalogue sells that package on — + * the domestic gross or the bare net a reverse-charge business owes — and which + * one it is has been decided by TaxTreatment before this is called. Nothing + * here computes tax, and `automatic_tax` is deliberately never enabled: a + * second rate worked out by Stripe would charge a German consumer 19 % while + * our own document said 20 %. + * * `$oneOff` is the setup fee: a second line item with no recurrence, whose - * `amount_cents` is GROSS like every other figure this platform charges. + * `amount_cents` is what this same customer is charged for it, formed the same + * way and by the same authority as the Price above. * Stripe allows one-time prices in a subscription-mode session and puts them * on the INITIAL invoice only, which is precisely what a setup fee is — so it * is charged once, with the first month, and never again. Null means there is @@ -103,7 +111,9 @@ interface StripeClient /** * Create a Price. Stripe Prices are immutable and carry their own interval, - * which is why one exists per plan version AND term. + * which is why one exists per plan version AND term — and, since a + * reverse-charge business is charged the bare net while everybody else pays + * the gross, two per priced row on each term. */ public function createPrice( string $productId, @@ -117,6 +127,18 @@ interface StripeClient /** Stop a Price being offered. The Price itself stays, as Stripe requires. */ public function archivePrice(string $priceId): void; + /** + * Offer an archived Price again. + * + * The other half of archivePrice(), and it was missing. A rate that moves and + * then moves back finds the Price for the figure it has come back to already + * in our register: the sync brings that row back rather than minting a second + * Price for one amount, which is right — but the Price was archived AT STRIPE, + * and only this makes it sellable again. Without it the catalogue pointed at + * an inactive Price and the failure was a customer meeting a refused checkout. + */ + public function activatePrice(string $priceId): void; + /** * Move a subscription onto another Price. * diff --git a/database/migrations/2026_07_31_200000_sell_the_net_price_to_a_reverse_charge_business.php b/database/migrations/2026_07_31_200000_sell_the_net_price_to_a_reverse_charge_business.php new file mode 100644 index 0000000..d586a63 --- /dev/null +++ b/database/migrations/2026_07_31_200000_sell_the_net_price_to_a_reverse_charge_business.php @@ -0,0 +1,96 @@ +boolean('reverse_charge')->default(false)->after('plan_price_id'); + + // The old key was (row, figure), which cannot hold both Prices at a + // rate of nought — where the gross and the net are the same number — + // and, worse, could not say which of two rows at one figure the + // checkout should pick. The treatment belongs in the identity. + $table->dropUnique('stripe_plan_prices_unique'); + $table->unique(['plan_price_id', 'reverse_charge', 'charged_cents'], 'stripe_plan_prices_unique'); + }); + + Schema::table('stripe_addon_prices', function (Blueprint $table) { + $table->boolean('reverse_charge')->default(false)->after('addon_key'); + + $table->dropUnique('stripe_addon_prices_unique'); + $table->unique( + ['addon_key', 'reverse_charge', 'amount_cents', 'currency', 'interval'], + 'stripe_addon_prices_unique', + ); + + // Read when a Price is superseded by a rate change, which happens + // within ONE treatment: the gross Price for today's figure moves and + // the net one beside it does not, so the treatment has to be part of + // the index the sweep reads or it would archive the other half. + $table->dropIndex('stripe_addon_prices_net'); + $table->index( + ['addon_key', 'reverse_charge', 'net_cents', 'currency', 'interval'], + 'stripe_addon_prices_net', + ); + }); + } + + public function down(): void + { + Schema::table('stripe_addon_prices', function (Blueprint $table) { + $table->dropIndex('stripe_addon_prices_net'); + $table->dropUnique('stripe_addon_prices_unique'); + $table->dropColumn('reverse_charge'); + $table->unique(['addon_key', 'amount_cents', 'currency', 'interval'], 'stripe_addon_prices_unique'); + $table->index(['addon_key', 'net_cents', 'currency', 'interval'], 'stripe_addon_prices_net'); + }); + + Schema::table('stripe_plan_prices', function (Blueprint $table) { + $table->dropUnique('stripe_plan_prices_unique'); + $table->dropColumn('reverse_charge'); + $table->unique(['plan_price_id', 'charged_cents'], 'stripe_plan_prices_unique'); + }); + } +}; diff --git a/lang/de/order.php b/lang/de/order.php index 419e8da..0831bd7 100644 --- a/lang/de/order.php +++ b/lang/de/order.php @@ -10,6 +10,9 @@ return [ 'recommended' => 'Empfohlen', 'per_month' => '/Monat', 'incl_vat' => 'inkl. :rate % MwSt.', + // Für ein bestätigtes EU-Unternehmen außerhalb Österreichs: keine Umsatzsteuer, + // also auch keine zweite Zahl daneben. Der Nettobetrag IST der Preis. + 'reverse_charge' => 'ohne USt. — Steuerschuldnerschaft des Leistungsempfängers (Reverse Charge)', 'net' => 'netto', 'setup' => 'zzgl. Einrichtung einmalig :amount', diff --git a/lang/en/order.php b/lang/en/order.php index feabdcd..9bf9101 100644 --- a/lang/en/order.php +++ b/lang/en/order.php @@ -10,6 +10,9 @@ return [ 'recommended' => 'Recommended', 'per_month' => '/month', 'incl_vat' => 'incl. :rate % VAT', + // For a verified EU business outside Austria: no VAT, and therefore no second + // figure beside it either. The net amount IS the price. + 'reverse_charge' => 'no VAT — reverse charge, VAT payable by the recipient', 'net' => 'net', 'setup' => 'plus one-off setup :amount', diff --git a/resources/views/livewire/order.blade.php b/resources/views/livewire/order.blade.php index 4d2f5c7..5fe395b 100644 --- a/resources/views/livewire/order.blade.php +++ b/resources/views/livewire/order.blade.php @@ -45,16 +45,24 @@

{{ $plan['audience'] }}

@endif - {{-- Gross, like the public sheet. A customer quoted 58,80 € - out there must not meet 49 € on the page with the - button on it. --}} + {{-- What this customer's card will be charged, like the public + sheet. A customer quoted 58,80 € out there must not meet + 49 € on the page with the button on it. --}}

{{ Number::currency($plan['gross'] / 100, in: $plan['currency'], locale: app()->getLocale()) }} {{ __('order.per_month') }}

- {{ __('order.incl_vat', ['rate' => $vat]) }} · - {{ __('order.net') }} {{ Number::currency($plan['net'] / 100, in: $plan['currency'], locale: app()->getLocale()) }} + {{-- A reverse-charge business is charged the bare net, so + there is no VAT to state and no second figure to show: + saying "inkl. 20 %" over a net price would describe a + tax nothing collects. --}} + @if ($tax->reverseCharge) + {{ __('order.reverse_charge') }} + @else + {{ __('order.incl_vat', ['rate' => $vat]) }} · + {{ __('order.net') }} {{ Number::currency($plan['net'] / 100, in: $plan['currency'], locale: app()->getLocale()) }} + @endif

@if ($setup > 0)

diff --git a/tests/Feature/Billing/ProofRegisterTest.php b/tests/Feature/Billing/ProofRegisterTest.php index 20326d6..5085bae 100644 --- a/tests/Feature/Billing/ProofRegisterTest.php +++ b/tests/Feature/Billing/ProofRegisterTest.php @@ -307,34 +307,41 @@ it('states net, tax and gross rather than leaving them to be recomputed', functi $subscription = Subscription::factory()->plan('team')->create(['customer_id' => reverseChargeCustomer()->id]); // What a real reverse-charge purchase records: OpenSubscription always passes - // the charged figure, and the charged figure is the domestic gross, because a - // Stripe Price carries one amount and everybody pays the one they were shown. - $record = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 21480); + // the charged figure, and for this customer that figure is the BARE NET — + // their checkout used the net Stripe Price, because reverse charge means no + // VAT is owed to us at all. + $record = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 17900); // The whole of it is net at 0 %. Recorded as it stood on the day — a rate // that changes later must not change the answer for this sale. - expect($record->net_cents)->toBe(21480) + expect($record->net_cents)->toBe(17900) ->and($record->tax_cents)->toBe(0) - ->and($record->gross_cents)->toBe(21480) + ->and($record->gross_cents)->toBe(17900) ->and($record->reverse_charge)->toBeTrue(); }); -it('does not call a reverse-charge sale charged at the advertised amount a mismatch', function () { +it('calls a correctly charged reverse-charge sale a match, and an overcharged one a mismatch', function () { $subscription = Subscription::factory()->plan('team')->create(['customer_id' => reverseChargeCustomer()->id]); - // 179,00 net at the till is 214,80 for everybody — the figure on the website, - // the figure on the Stripe Price, the figure taken from the card. The flag was - // holding it against the customer's OWN rate, which is zero, and so reported - // every single EU business sale as charged at the wrong amount. A flag that - // means "somebody must look at this" cannot be false for the healthy majority. - $record = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 21480); + // 179,00 net is the WHOLE of what this customer owes: no VAT is charged to + // them, so there is none in the price either, and their checkout used the net + // Stripe Price. The register expects that figure, and finding it is a match. + $record = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 17900); - expect($record->snapshot['amounts']['expected_gross_cents'])->toBe(21480) - ->and($record->snapshot['amounts']['charged_gross_cents'])->toBe(21480) + expect($record->snapshot['amounts']['expected_gross_cents'])->toBe(17900) + ->and($record->snapshot['amounts']['charged_gross_cents'])->toBe(17900) ->and($record->snapshot['amounts']['matches_catalogue'])->toBeTrue(); - // And it still catches the thing it is for: a reverse-charge customer who was - // charged something other than the advertised amount. + // And now it catches the thing it is for, in the direction that actually cost + // somebody money: the domestic gross taken from a reverse-charge business is a + // flat 20 % surcharge with no VAT line anywhere for them to reclaim. That is + // the defect this whole change ends, and the flag whose job is "somebody must + // look at this" has to be the thing that would have found it. + $overcharged = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 21480); + + expect($overcharged->snapshot['amounts']['matches_catalogue'])->toBeFalse(); + + // A short charge is still a mismatch, in the other direction. $short = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 14900); expect($short->snapshot['amounts']['matches_catalogue'])->toBeFalse(); diff --git a/tests/Feature/Billing/ReverseChargePriceTest.php b/tests/Feature/Billing/ReverseChargePriceTest.php new file mode 100644 index 0000000..adc29f6 --- /dev/null +++ b/tests/Feature/Billing/ReverseChargePriceTest.php @@ -0,0 +1,400 @@ +stripe = new FakeStripeClient; + app()->instance(StripeClient::class, $this->stripe); + + // The catalogue mirrored into Stripe: two Prices per priced row and per + // module interval. Nothing here mints a Price on demand, deliberately — see + // App\Services\Billing\PlanPrices — so a checkout has to find one waiting. + app(Kernel::class)->call('stripe:sync-catalogue'); + + CompanyProfile::put([ + 'name' => 'CluPilot Cloud e.U.', 'address' => 'Dreherstraße 66/1/8', + 'postcode' => '1110', 'city' => 'Wien', 'vat_id' => 'ATU00000000', + ]); +}); + +/** + * Somebody who can reach the checkout, and the customer record the treatment is + * read from. + * + * @param array $attributes + * @return array{0: User, 1: Customer} + */ +function buyerWith(array $attributes): array +{ + $customer = Customer::factory()->create($attributes); + $user = User::factory()->create([ + 'email' => $customer->email, + 'email_verified_at' => now(), + ]); + + return [$user, $customer]; +} + +/** A business in another member state whose number the register knows. */ +function verifiedEuBusiness(): array +{ + $verifier = new FakeVatIdVerifier; + $verifier->registered('NL123456789B01', 'Berger B.V.'); + app()->instance(VatIdVerifier::class, $verifier); + + return buyerWith([ + 'customer_type' => Customer::TYPE_BUSINESS, + 'vat_id' => 'NL123456789B01', + 'vat_id_verified_at' => now(), + 'vat_id_verified_value' => 'NL123456789B01', + ]); +} + +/** What the Price a checkout was opened on actually charges. */ +function chargedByCheckout(FakeStripeClient $stripe, int $index = 0): int +{ + return (int) $stripe->prices[$stripe->checkouts[$index]['price']]['amount']; +} + +/** The Stripe Price the catalogue sells a package on to a customer treated so. */ +function packagePrice(string $plan, TaxTreatment $treatment, string $term = Subscription::TERM_MONTHLY): string +{ + $row = PlanPrice::query() + ->whereHas('version.family', fn ($query) => $query->where('key', $plan)) + ->where('term', $term) + ->sole(); + + return (string) app(PlanPrices::class)->liveFor($row, $treatment); +} + +it('charges a domestic consumer and a domestic business the same gross, to the cent', function () { + // The owner's rule, and it is right for both of them: the business pays the + // gross, the VAT is on the invoice, they reclaim it as input tax. + [$consumer] = buyerWith(['customer_type' => Customer::TYPE_CONSUMER]); + [$business] = buyerWith([ + 'customer_type' => Customer::TYPE_BUSINESS, + // Austrian, and verified. Reverse charge is an INTRA-EU rule and this is + // our own member state, so it changes nothing at all: Austria B2B 20 %. + 'vat_id' => 'ATU12345678', + 'vat_id_verified_at' => now(), + 'vat_id_verified_value' => 'ATU12345678', + ]); + + $this->actingAs($consumer)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']) + ->assertRedirect(); + $this->actingAs($business)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']) + ->assertRedirect(); + + // 179,00 € plus 20 %, which is the figure on the website. + expect(chargedByCheckout($this->stripe, 0))->toBe(21480) + ->and(chargedByCheckout($this->stripe, 1))->toBe(21480) + ->and($this->stripe->checkouts[0]['price']) + ->toBe($this->stripe->checkouts[1]['price']) + ->and($this->stripe->checkouts[0]['price']) + ->toBe(packagePrice('team', TaxTreatment::domestic())); +}); + +it('charges a verified business in another member state the bare net, and invoices exactly that', function () { + Queue::fake(); + [$user] = verifiedEuBusiness(); + + $this->actingAs($user)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']) + ->assertRedirect(); + + // The bare net: no VAT is owed to us, so there is none in the price either. + // A different Stripe Price from the domestic one, on the same Product. + expect(chargedByCheckout($this->stripe))->toBe(17900) + ->and($this->stripe->checkouts[0]['price']) + ->toBe(packagePrice('team', TaxTreatment::reverseCharge())) + ->and($this->stripe->checkouts[0]['price']) + ->not->toBe(packagePrice('team', TaxTreatment::domestic())); + + // And what Stripe took is what the document says. This is the half that made + // the overcharge unlawful as well as expensive: 214,80 € taken, and a document + // stating all of it as net at 0 %, so the customer self-accounted their VAT on + // a fifth too much. + $this->postJson(route('webhooks.stripe'), [ + 'id' => 'evt_rc', 'type' => 'checkout.session.completed', + 'data' => ['object' => [ + 'id' => 'cs_rc', 'payment_status' => 'paid', 'subscription' => 'sub_rc', + 'customer_details' => ['email' => $user->email, 'name' => 'Berger B.V.'], + 'amount_total' => 17900, 'currency' => 'eur', + 'metadata' => ['plan' => 'team', 'datacenter' => 'fsn'], + ]], + ])->assertOk(); + + $invoice = Invoice::query()->sole(); + + expect($invoice->gross_cents)->toBe(17900) + ->and($invoice->net_cents)->toBe(17900) + ->and($invoice->tax_cents)->toBe(0) + // The note that makes a zero-rated document lawful. + ->and($invoice->snapshot['meta']['closing'])->toBe(__('invoice.reverse_charge')); + + // The register holds the charge against what this customer should have been + // charged, which is the net — so a correct reverse-charge sale is no longer + // reported as a mismatch, and an overcharged one would be. + $record = SubscriptionRecord::query()->where('event', SubscriptionRecord::EVENT_PURCHASE)->sole(); + + expect($record->snapshot['amounts']['expected_gross_cents'])->toBe(17900) + ->and($record->snapshot['amounts']['charged_gross_cents'])->toBe(17900) + ->and($record->snapshot['amounts']['matches_catalogue'])->toBeTrue() + ->and($record->reverse_charge)->toBeTrue(); +}); + +it('charges an unverified EU business the gross, because an unchecked number is no discount', function () { + // The number looks exactly like the verified one. Nobody has asked the + // register about it, and a self-declared string must never be able to zero a + // tax — otherwise typing "NL…" is a twenty percent discount. + [$user] = buyerWith([ + 'customer_type' => Customer::TYPE_BUSINESS, + 'vat_id' => 'NL123456789B01', + ]); + + $this->actingAs($user)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']) + ->assertRedirect(); + + expect(chargedByCheckout($this->stripe))->toBe(21480) + ->and($this->stripe->checkouts[0]['price'])->toBe(packagePrice('team', TaxTreatment::domestic())); +}); + +it('charges the setup fee under the same rule as the package', function () { + // 99,00 € net, which is the 118,80 € the public price sheet quotes at 20 %. + Settings::set('company.setup_fee_cents', 9900); + + [$domestic] = buyerWith(['customer_type' => Customer::TYPE_CONSUMER]); + [$business] = verifiedEuBusiness(); + + $this->actingAs($domestic)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']); + $this->actingAs($business)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']); + + // A fee is a supply like the package it sets up, so it cannot be taxed + // differently from it on the same invoice — and the webhook is told how much + // of the total was the fee, so the figure has to be this customer's own. + expect($this->stripe->checkouts[0]['one_off']['amount_cents'])->toBe(11880) + ->and($this->stripe->checkouts[0]['metadata']['setup_fee_cents'])->toBe('11880') + ->and($this->stripe->checkouts[1]['one_off']['amount_cents'])->toBe(9900) + ->and($this->stripe->checkouts[1]['metadata']['setup_fee_cents'])->toBe('9900'); +}); + +it('bills a booked module on the net price too, for the customer who owes no VAT', function () { + [, $customer] = verifiedEuBusiness(); + + $contract = Subscription::factory()->plan('team')->create([ + 'customer_id' => $customer->id, + 'stripe_subscription_id' => 'sub_rc_mod', + 'stripe_item_id' => 'si_rc_plan', + ]); + + app(BookAddon::class)($contract, 'priority_support'); // 29,00 net / month + + $item = collect($this->stripe->itemsOn('sub_rc_mod'))->sole(); + + // 29,00 € and not 34,80 €. A module taxed differently from the package beside + // it would be one invoice at two rates. + expect((int) $this->stripe->prices[$item['price']]['amount'])->toBe(2900) + ->and(StripeAddonPrice::query() + ->where('stripe_price_id', $item['price']) + ->value('reverse_charge'))->toBeTruthy(); +}); + +it('moves a contract onto the net price when the VAT id is verified after the sale, without prorating', function () { + // Bought as an ordinary business: the number was on the record, nobody had + // checked it, so the gross was charged and that was correct at the time. + $verifier = new FakeVatIdVerifier; + $verifier->registered('NL123456789B01', 'Berger B.V.'); + app()->instance(VatIdVerifier::class, $verifier); + + [, $customer] = buyerWith([ + 'customer_type' => Customer::TYPE_BUSINESS, + 'vat_id' => 'NL123456789B01', + ]); + + $contract = Subscription::factory()->plan('team')->create([ + 'customer_id' => $customer->id, + 'stripe_subscription_id' => 'sub_late', + 'stripe_item_id' => 'si_late', + 'stripe_price_id' => packagePrice('team', TaxTreatment::domestic()), + ]); + + // Nothing to do while the number is unchecked, and a run that moved them here + // would be a run that trusted a string somebody typed. + $this->artisan('stripe:reprice-subscriptions')->assertSuccessful(); + + expect($this->stripe->priceChanges)->toBeEmpty(); + + // A week later the register answers, and from that moment they owe no VAT. + $this->artisan('clupilot:verify-vat-ids')->assertSuccessful(); + + expect($customer->fresh()->hasVerifiedVatId())->toBeTrue(); + + $this->artisan('stripe:reprice-subscriptions')->assertSuccessful(); + + expect($this->stripe->priceChanges)->toHaveCount(1) + ->and($this->stripe->priceChanges[0]['price'])->toBe(packagePrice('team', TaxTreatment::reverseCharge())) + // NEVER prorated. The term they are in is paid for, and charging or + // crediting the difference for days already served would be a bill + // nobody agreed to. + ->and($this->stripe->priceChanges[0]['proration'])->toBe(StripeClient::PRORATE_NONE) + // And the contract records which of the two Prices it is now billed on, + // which is what makes a second run free. + ->and($contract->fresh()->stripe_price_id) + ->toBe(packagePrice('team', TaxTreatment::reverseCharge())) + ->and(StripePlanPrice::query() + ->where('stripe_price_id', $contract->fresh()->stripe_price_id) + ->value('reverse_charge'))->toBeTruthy(); + + // Converged: the second run finds nothing left to move. + $this->artisan('stripe:reprice-subscriptions')->assertSuccessful(); + + expect($this->stripe->priceChanges)->toHaveCount(1); + + // The contract's own frozen figure is untouched by all of it. It is the + // catalogue's NET amount, every pro-rata sum reads it, and reinterpreting it + // would corrupt every plan change ever computed. + expect($contract->fresh()->price_cents)->toBe(17900); +}); + +it('moves a contract back onto the gross price when a registration lapses', function () { + [, $customer] = verifiedEuBusiness(); + + $contract = Subscription::factory()->plan('team')->create([ + 'customer_id' => $customer->id, + 'stripe_subscription_id' => 'sub_lapse', + 'stripe_item_id' => 'si_lapse', + 'stripe_price_id' => packagePrice('team', TaxTreatment::reverseCharge()), + ]); + + $this->artisan('stripe:reprice-subscriptions')->assertSuccessful(); + + expect($this->stripe->priceChanges)->toBeEmpty(); + + // The company is wound up, or the registration is withdrawn. From that moment + // we are the ones liable for the VAT nobody collected, so the customer goes + // back onto the domestic Price. + $verifier = new FakeVatIdVerifier; + $verifier->notRegistered('NL123456789B01'); + app()->instance(VatIdVerifier::class, $verifier); + + $this->artisan('clupilot:verify-vat-ids')->assertSuccessful(); + + expect($customer->fresh()->hasVerifiedVatId())->toBeFalse(); + + $this->artisan('stripe:reprice-subscriptions')->assertSuccessful(); + + expect($this->stripe->priceChanges)->toHaveCount(1) + ->and($this->stripe->priceChanges[0]['price'])->toBe(packagePrice('team', TaxTreatment::domestic())) + ->and($this->stripe->priceChanges[0]['proration'])->toBe(StripeClient::PRORATE_NONE) + ->and($contract->fresh()->stripe_price_id)->toBe(packagePrice('team', TaxTreatment::domestic())); +}); + +it('creates no second Price for a figure it already has, however often it is run', function () { + $before = count($this->stripe->prices); + + // A Stripe Price cannot be edited or deleted, so a run that minted duplicates + // would leave two live Prices for one row at one figure for ever — and nothing + // could tell afterwards which of them a customer is billed on. + $this->artisan('stripe:sync-catalogue') + ->expectsOutputToContain('already in step') + ->assertSuccessful(); + + $this->artisan('stripe:sync-catalogue')->assertSuccessful(); + + expect($this->stripe->prices)->toHaveCount($before) + // Two rows per priced catalogue row, both live, one per treatment. + ->and(StripePlanPrice::query()->count())->toBe(16) + ->and(StripePlanPrice::query()->whereNull('archived_at')->count())->toBe(16) + ->and(StripePlanPrice::query()->where('reverse_charge', true)->count())->toBe(8) + // And no Price at one figure for one row twice, which is the thing the + // register's unique key exists to make impossible. + ->and(StripePlanPrice::query()->distinct()->count('stripe_price_id'))->toBe(16); +}); + +it('leaves the net Price alone when the VAT rate moves, and puts a Price back on sale rather than minting a second', function () { + $domesticBefore = packagePrice('team', TaxTreatment::domestic()); + $netPrice = packagePrice('team', TaxTreatment::reverseCharge()); + $count = count($this->stripe->prices); + $modules = count(array_merge(array_keys((array) config('provisioning.addons')), ['storage'])); + + // The rate moves. Only the GROSS half of each pair is wrong: the net owes + // nothing to the rate, so a sweep that did not know the difference would + // withdraw every reverse-charge Price the first time an operator touched the + // Finance page. + Settings::set('company.tax_rate', 10.0); + $this->artisan('stripe:sync-catalogue')->assertSuccessful(); + + expect(packagePrice('team', TaxTreatment::domestic()))->not->toBe($domesticBefore) + ->and($this->stripe->archived)->toContain($domesticBefore) + ->and(packagePrice('team', TaxTreatment::reverseCharge()))->toBe($netPrice) + ->and($this->stripe->archived)->not->toContain($netPrice); + + // And back again. A Stripe Price cannot be edited, so the one for the figure + // we have returned to is still there — brought back on sale rather than + // duplicated, AT STRIPE as well as here. Only in our own table it would leave + // the catalogue pointing at an inactive Price, and the failure would be a + // customer meeting a refused checkout. + Settings::set('company.tax_rate', 20.0); + $this->artisan('stripe:sync-catalogue')->assertSuccessful(); + + expect(packagePrice('team', TaxTreatment::domestic()))->toBe($domesticBefore) + ->and($this->stripe->activated)->toContain($domesticBefore) + ->and($this->stripe->archived)->not->toContain($domesticBefore) + // What the rate change minted and nothing more: one 10 % gross Price for + // each of the eight priced rows and for each module on each interval. No + // duplicate of anything that existed before, and nothing new on the net + // side at all. + ->and($this->stripe->prices)->toHaveCount($count + 8 + $modules * 2); +}); + +it('refuses the sale rather than overcharging a business the catalogue has no net price for', function () { + // The state a deploy leaves behind until stripe:sync-catalogue has run: the + // domestic Price exists and the net one does not. There IS a fallback here and + // it must not be taken — putting them on the domestic Price takes a fifth more + // than they owe, which is the whole defect. A refused checkout is loud, and one + // command cures it. + StripePlanPrice::query()->where('reverse_charge', true)->delete(); + + [$user] = verifiedEuBusiness(); + + $this->actingAs($user)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']) + ->assertSessionHasErrors('plan'); + + expect($this->stripe->checkouts)->toBeEmpty(); +}); diff --git a/tests/Feature/Billing/SetupFeeTest.php b/tests/Feature/Billing/SetupFeeTest.php index d2d4b29..a5e756d 100644 --- a/tests/Feature/Billing/SetupFeeTest.php +++ b/tests/Feature/Billing/SetupFeeTest.php @@ -53,8 +53,10 @@ it('charges the fee it advertises, as a one-off line beside the monthly one', fu $checkout = $this->stripe->checkouts[0]; - // Gross, like every other figure this platform charges: the price on the page - // is the price at the till, for everybody. + // Gross, like every other figure a domestic customer is charged: the price on + // the page is the price at the till. A verified business in another member + // state is charged the bare net of the fee, exactly as of the package — see + // ReverseChargePriceTest. expect($checkout['one_off'])->not->toBeNull() ->and($checkout['one_off']['amount_cents'])->toBe(11880) ->and($checkout['one_off']['currency'])->toBe('EUR') diff --git a/tests/Feature/Billing/StripeAddonBillingTest.php b/tests/Feature/Billing/StripeAddonBillingTest.php index 646e74f..0b55846 100644 --- a/tests/Feature/Billing/StripeAddonBillingTest.php +++ b/tests/Feature/Billing/StripeAddonBillingTest.php @@ -88,24 +88,39 @@ function addonPlanPrice(Subscription $contract): string ->value('stripe_price_id'); } -/** The Stripe Price a module is sold at, monthly, at today's catalogue figure. */ +/** + * The Stripe Price a module is sold at, monthly, at today's catalogue figure — + * to a customer who is charged VAT. + * + * There are two Prices per module and interval now: this one and the bare net a + * verified business in another member state is charged. The contracts in this + * file are domestic, so the domestic treatment is the one their items bill on. + */ function addonModulePrice(string $key, int $monthlyCents): string { - return (string) app(AddonPrices::class)->liveFor($key, $monthlyCents, 'EUR', Subscription::TERM_MONTHLY); + return (string) app(AddonPrices::class)->liveFor( + $key, + $monthlyCents, + 'EUR', + Subscription::TERM_MONTHLY, + TaxTreatment::domestic(), + ); } /** - * What Stripe actually takes for a net catalogue figure. + * What Stripe actually takes for a net catalogue figure from an ordinary, + * domestic customer. * - * 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. + * Their Price carries the GROSS, so every amount on a Stripe invoice line for them + * 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. A reverse-charge business is charged the bare net instead, and that is + * tested where it belongs, in ReverseChargePriceTest. */ function atTheTill(int $netCents): int { - return TaxTreatment::chargedCents($netCents); + return TaxTreatment::advertisedCents($netCents); } /** diff --git a/tests/Feature/Billing/StripeBillingTest.php b/tests/Feature/Billing/StripeBillingTest.php index 7783d70..cebe736 100644 --- a/tests/Feature/Billing/StripeBillingTest.php +++ b/tests/Feature/Billing/StripeBillingTest.php @@ -37,10 +37,19 @@ function stripeContract(string $stripeId = 'sub_1'): Subscription ]); } -/** The plan half of the mirror — modules live in the same lists. */ -function planPrices(FakeStripeClient $stripe): Collection +/** + * The plan half of the mirror — modules live in the same lists. + * + * Two Prices per priced row, not one: the domestic gross everybody who is charged + * VAT pays, and the bare net a verified business in another member state owes. + * `$treatment` picks one of the two where a test is about a figure rather than + * about how many objects there are. + */ +function planPrices(FakeStripeClient $stripe, ?string $treatment = null): Collection { - return collect($stripe->prices)->filter(fn ($price) => isset($price['metadata']['plan_family'])); + return collect($stripe->prices) + ->filter(fn ($price) => isset($price['metadata']['plan_family'])) + ->filter(fn ($price) => $treatment === null || $price['metadata']['tax_treatment'] === $treatment); } function planProducts(FakeStripeClient $stripe): Collection @@ -48,7 +57,7 @@ function planProducts(FakeStripeClient $stripe): Collection return collect($stripe->products)->filter(fn ($product) => isset($product['metadata']['plan_family'])); } -/** The module half: a Price per module and term, so a booking has one to bill on. */ +/** The module half: a Price per module, term and treatment, so a booking has one to bill on. */ function modulePrices(FakeStripeClient $stripe): Collection { return collect($stripe->prices)->filter(fn ($price) => isset($price['metadata']['addon'])); @@ -59,32 +68,44 @@ it('mirrors the catalogue into Stripe, once', function () { $this->artisan('stripe:sync-catalogue')->assertSuccessful(); - // A product per family, a price per priced row of a published version. + // A product per family, and TWO prices per priced row of a published + // version: the domestic gross and the bare net a reverse-charge business is + // charged. Both are minted up front, because the Price has to exist before + // the checkout that needs it. expect(planProducts($stripe))->toHaveCount(4) - ->and(planPrices($stripe))->toHaveCount(8) + ->and(planPrices($stripe))->toHaveCount(16) + ->and(planPrices($stripe, 'domestic'))->toHaveCount(8) + ->and(planPrices($stripe, 'reverse_charge'))->toHaveCount(8) ->and(PlanFamily::query()->whereNull('stripe_product_id')->count())->toBe(0) + // The catalogue row still points at the DOMESTIC Price, which is the + // ordinary sale; the net one is only ever read out of the register. ->and(PlanPrice::query()->whereNull('stripe_price_id')->count())->toBe(0); // And every module we sell, on both terms: a Stripe Price carries its own // interval, so a module on a yearly contract needs a yearly one. Without // these a booked module could never become an item on the subscription, // which is why it was charged in the month it was booked and never again. + // Times two again, for the same reason as the packages. $modules = array_merge(array_keys((array) config('provisioning.addons')), ['storage']); + $storageYear = StripeAddonPrice::query()->where('addon_key', 'storage')->where('interval', 'year'); // 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')) + // which is what a yearly customer is actually charged for it. The + // reverse-charge Price beside it charges that net figure and nothing on top. + expect(modulePrices($stripe))->toHaveCount(count($modules) * 4) + ->and((clone $storageYear)->where('reverse_charge', false)->value('amount_cents')) + ->toBe(TaxTreatment::advertisedCents(12 * (int) config('provisioning.storage_addon.price_cents'))) + ->and((clone $storageYear)->where('reverse_charge', true)->value('amount_cents')) + ->toBe(12 * (int) config('provisioning.storage_addon.price_cents')) + ->and((clone $storageYear)->where('reverse_charge', false)->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 // would leave two live prices for one plan and no way to tell them apart. $this->artisan('stripe:sync-catalogue')->assertSuccessful(); - expect(planPrices($stripe))->toHaveCount(8) - ->and(modulePrices($stripe))->toHaveCount(count($modules) * 2) + expect(planPrices($stripe))->toHaveCount(16) + ->and(modulePrices($stripe))->toHaveCount(count($modules) * 4) ->and($stripe->products)->toHaveCount(4 + count($modules)); }); @@ -92,19 +113,28 @@ it('gives each price its own recurring interval', function () { $stripe = fakeStripe(); $this->artisan('stripe:sync-catalogue'); - $intervals = planPrices($stripe)->groupBy('interval')->map->count(); + $intervals = planPrices($stripe, 'domestic')->groupBy('interval')->map->count(); // Monthly and yearly cannot share a Price, because the interval belongs to // the Price itself. expect($intervals['month'])->toBe(4) ->and($intervals['year'])->toBe(4); - // 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'); + // The DOMESTIC GROSS, which is the figure on the website and the one taken + // from the card of everybody who is charged VAT. The catalogue row stays net. + $team = planPrices($stripe, 'domestic') + ->first(fn ($p) => $p['metadata']['plan_family'] === 'team' && $p['interval'] === 'month'); expect($team['amount'])->toBe(21480) - ->and($team['amount'])->toBe(TaxTreatment::chargedCents(17900)) + ->and($team['amount'])->toBe(TaxTreatment::advertisedCents(17900)) ->and($team['metadata']['plan_version'])->toBe('1'); + + // And the bare net beside it, on the same Product, for the business that owes + // no VAT at all. One Price for everybody was the overcharge: 214,80 € taken + // for a 179,00 € package from exactly the customers who read their invoices. + $reverse = planPrices($stripe, 'reverse_charge') + ->first(fn ($p) => $p['metadata']['plan_family'] === 'team' && $p['interval'] === 'month'); + expect($reverse['amount'])->toBe(17900) + ->and($reverse['product'])->toBe($team['product']); }); it('does not put an unpublished draft in the price list', function () { @@ -121,7 +151,7 @@ it('does not put an unpublished draft in the price list', function () { // A draft has promised nothing; a Price for it would be a price list entry // for something that may never exist. - expect(planPrices($stripe))->toHaveCount(8) + expect(planPrices($stripe))->toHaveCount(16) ->and(planPrices($stripe)->pluck('amount'))->not->toContain(19900); }); @@ -140,7 +170,7 @@ it('does not mint a second object when a run is interrupted before the id is sto // catalogue reconnects to what is already there instead of duplicating it — // and a Price cannot be deleted afterwards to tidy up. expect(planProducts($stripe))->toHaveCount(4) - ->and(planPrices($stripe))->toHaveCount(8) + ->and(planPrices($stripe))->toHaveCount(16) ->and(PlanPrice::query()->whereNull('stripe_price_id')->count())->toBe(0); });