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); });