diff --git a/app/Console/Commands/SyncStripeCatalogue.php b/app/Console/Commands/SyncStripeCatalogue.php index 1876dd8..6f57bc8 100644 --- a/app/Console/Commands/SyncStripeCatalogue.php +++ b/app/Console/Commands/SyncStripeCatalogue.php @@ -63,6 +63,12 @@ use Illuminate\Console\Command; * Product for it would be a price list entry for something that may never * exist. * + * Nor is an INTERNAL family (`plan_families.internal`) — handed out by hand, + * never sold, never billed. A Stripe Price cannot be deleted once minted, so a + * package nobody is ever charged for stays out of the mirror entirely, on a + * dry run and for real. A family that already carries Stripe ids from before + * it was marked internal keeps them; this only stops anything new being added. + * * **A run belongs to ONE Stripe account, and it says which.** Products and * Prices are account-bound, and the operating mode picks the key — so the mode * a run worked in is recorded (App\Support\StripeCatalogueMode) and a run into @@ -151,6 +157,20 @@ class SyncStripeCatalogue extends Command $adoptedFamilyProducts = 0; foreach (PlanFamily::query()->with('versions.prices')->orderBy('tier')->get() as $family) { + // An internal family is never billed through Stripe — handed out by + // hand (the internal test package, Enterprise once a machine is + // built for it) and never bought. A Stripe Price cannot be deleted, + // only archived, so whatever a live run mints here stays in the + // operator's account for good — and a package nobody is ever + // charged for is not worth that. Skipped here, ahead of the + // published-version check below and everything after it, so a + // family that already carries Stripe ids from when it WAS sellable + // (Enterprise, precisely) is left untouched: nothing is cleared, + // nothing is replaced, and nothing new is ever added for it. + if ($family->internal) { + continue; + } + $published = $family->versions->filter(fn (PlanVersion $version) => $version->isPublished()); // A family whose versions are all drafts has promised nothing, so diff --git a/app/Support/Readiness/BillingChecks.php b/app/Support/Readiness/BillingChecks.php index ca96f8a..6684e74 100644 --- a/app/Support/Readiness/BillingChecks.php +++ b/app/Support/Readiness/BillingChecks.php @@ -44,7 +44,18 @@ final class BillingChecks // alarm that never clears on any installation that has ever drafted a // next version, which stops it being read at all. $published = PlanPrice::query() - ->whereHas('version', fn ($query) => $query->whereNotNull('published_at')); + ->whereHas('version', fn ($query) => $query->whereNotNull('published_at')) + // Eine interne Familie (`test`, `enterprise`) überspringt + // stripe:sync-catalogue absichtlich — Produkt UND Preise, im + // Trockenlauf wie im echten Lauf (Aufgabe 12: ein Stripe-Preis + // lässt sich nicht löschen, und was nie belastet wird, wird nicht + // angelegt). Ihre Preiszeilen bleiben deshalb für immer ohne + // stripe_price_id, ganz gleich wie oft der Abgleich läuft. Ohne + // diese Ausnahme wäre diese Prüfung auf jeder Installation mit dem + // internen Testpaket oder Enterprise im Katalog auf Dauer rot — + // eine Bereitschaftsseite, die niemals grün werden kann, ist so + // gut wie keine. + ->whereHas('version.family', fn ($query) => $query->where('internal', false)); $unsyncedPrices = (clone $published)->whereNull('stripe_price_id')->exists(); diff --git a/tests/Feature/Billing/ReverseChargePriceTest.php b/tests/Feature/Billing/ReverseChargePriceTest.php index 5d07aa4..59bef23 100644 --- a/tests/Feature/Billing/ReverseChargePriceTest.php +++ b/tests/Feature/Billing/ReverseChargePriceTest.php @@ -342,19 +342,21 @@ it('creates no second Price for a figure it already has, however often it is run $this->artisan('stripe:sync-catalogue')->assertSuccessful(); - // Sixteen priced rows: five families (Start, Team, Business, Enterprise and - // the internal package) on eight versions — the ladder switch left a closed - // predecessor beside each of the three it replaced — at two terms each. A - // closed version keeps its Prices because a checkout quoted on it is still - // owed them. + // Twelve priced rows: three SELLABLE families (Start, Team, Business) on + // six versions — the ladder switch left a closed predecessor beside each + // one it replaced — at two terms each. A closed version keeps its Prices + // because a checkout quoted on it is still owed them. Enterprise and the + // internal package are `internal = true` and stripe:sync-catalogue skips + // them entirely since Task 12 — a package nobody is ever charged through + // Stripe gets no Stripe Price, because one cannot be deleted once minted. expect($this->stripe->prices)->toHaveCount($before) // Two rows per priced catalogue row, both live, one per treatment. - ->and(StripePlanPrice::query()->count())->toBe(32) - ->and(StripePlanPrice::query()->whereNull('archived_at')->count())->toBe(32) - ->and(StripePlanPrice::query()->where('reverse_charge', true)->count())->toBe(16) + ->and(StripePlanPrice::query()->count())->toBe(24) + ->and(StripePlanPrice::query()->whereNull('archived_at')->count())->toBe(24) + ->and(StripePlanPrice::query()->where('reverse_charge', true)->count())->toBe(12) // 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(32); + ->and(StripePlanPrice::query()->distinct()->count('stripe_price_id'))->toBe(24); }); it('leaves the net Price alone when the VAT rate moves, and puts a Price back on sale rather than minting a second', function () { @@ -387,10 +389,11 @@ it('leaves the net Price alone when the VAT rate moves, and puts a Price back on ->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 sixteen priced rows and for each module on each interval. + // each of the twelve SELLABLE priced rows (Enterprise and the internal + // package are skipped, Task 12) 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 + 16 + $modules * 2); + ->and($this->stripe->prices)->toHaveCount($count + 12 + $modules * 2); }); it('refuses the sale rather than overcharging a business the catalogue has no net price for', function () { diff --git a/tests/Feature/Billing/StripeBillingTest.php b/tests/Feature/Billing/StripeBillingTest.php index 16c83cd..2a152d5 100644 --- a/tests/Feature/Billing/StripeBillingTest.php +++ b/tests/Feature/Billing/StripeBillingTest.php @@ -11,6 +11,7 @@ use App\Services\Billing\PlanCatalogue; use App\Services\Billing\TaxTreatment; use App\Services\Stripe\FakeStripeClient; use App\Services\Stripe\StripeClient; +use App\Support\Settings; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Queue; @@ -72,20 +73,30 @@ it('mirrors the catalogue into Stripe, once', function () { // 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. - // Fünf Familien: die drei verkauften, Enterprise (auf Anfrage, aber mit - // laufenden Verträgen) und das interne Testpaket. Sechzehn ausgepreiste - // Zeilen, weil die Umschaltung auf die neue Leiter neben jede der drei - // verkauften eine beendete Vorgängerfassung gestellt hat — und deren Preise - // bleiben stehen, denn ein Checkout, der auf ihr eröffnet wurde, hat sie - // gesehen und bezahlt. - expect(planProducts($stripe))->toHaveCount(5) - ->and(planPrices($stripe))->toHaveCount(32) - ->and(planPrices($stripe, 'domestic'))->toHaveCount(16) - ->and(planPrices($stripe, 'reverse_charge'))->toHaveCount(16) - ->and(PlanFamily::query()->whereNull('stripe_product_id')->count())->toBe(0) + // Drei Familien, nicht fünf: Enterprise und das interne Testpaket tragen + // `internal = true` und werden von stripe:sync-catalogue seit Aufgabe 12 + // übersprungen — ein Paket, das nie über Stripe abgerechnet wird, bekommt + // keinen Stripe-Preis, weil sich ein einmal angelegter nicht löschen lässt. + // Zwölf ausgepreiste Zeilen, weil die Umschaltung auf die neue Leiter neben + // jede der drei verkauften eine beendete Vorgängerfassung gestellt hat — + // und deren Preise bleiben stehen, denn ein Checkout, der auf ihr eröffnet + // wurde, hat sie gesehen und bezahlt. + expect(planProducts($stripe))->toHaveCount(3) + ->and(planPrices($stripe))->toHaveCount(24) + ->and(planPrices($stripe, 'domestic'))->toHaveCount(12) + ->and(planPrices($stripe, 'reverse_charge'))->toHaveCount(12) + ->and(PlanFamily::query()->where('internal', false)->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(PlanPrice::query() + ->whereHas('version.family', fn ($q) => $q->where('internal', false)) + ->whereNull('stripe_price_id')->count())->toBe(0) + // The two internal families get neither: no product, and no price on + // either of their rows. + ->and(PlanFamily::query()->where('internal', true)->whereNotNull('stripe_product_id')->count())->toBe(0) + ->and(PlanPrice::query() + ->whereHas('version.family', fn ($q) => $q->where('internal', true)) + ->whereNotNull('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 @@ -110,9 +121,76 @@ it('mirrors the catalogue into Stripe, once', function () { // 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(32) + expect(planPrices($stripe))->toHaveCount(24) ->and(modulePrices($stripe))->toHaveCount(count($modules) * 4) - ->and($stripe->products)->toHaveCount(5 + count($modules)); + ->and($stripe->products)->toHaveCount(3 + count($modules)); +}); + +it('does not mirror an internal family into Stripe, product or price, dry run or real', function () { + $stripe = fakeStripe(); + + // Der Befund, der zu dieser Aufgabe führte: der Trockenlauf listete vier + // Preise für "test v2", ein Paket, das nie über Stripe abgerechnet wird — + // es wird von Hand vergeben, nie gekauft. Weder das interne Testpaket noch + // Enterprise (auf Anfrage vergeben, `internal = true` seit derselben + // Migration) dürfen hier auftauchen. + $this->artisan('stripe:sync-catalogue', ['--dry-run' => true]) + ->doesntExpectOutputToContain('test v') + ->doesntExpectOutputToContain('enterprise') + ->assertSuccessful(); + + $this->artisan('stripe:sync-catalogue')->assertSuccessful(); + + $internalFamilies = PlanFamily::query()->where('internal', true)->get(); + + expect($internalFamilies)->toHaveCount(2) + ->and($internalFamilies->pluck('stripe_product_id')->filter())->toBeEmpty() + ->and(PlanPrice::query() + ->whereHas('version.family', fn ($q) => $q->where('internal', true)) + ->whereNotNull('stripe_price_id')->count())->toBe(0) + ->and(planProducts($stripe)->pluck('metadata.plan_family')) + ->not->toContain('test')->not->toContain('enterprise'); +}); + +it('keeps a family\'s existing Stripe ids once it is switched from sellable to internal, and mints nothing new for it', function () { + $stripe = fakeStripe(); + $this->artisan('stripe:sync-catalogue')->assertSuccessful(); + + $family = PlanFamily::query()->where('key', 'start')->sole(); + $productIdBefore = $family->stripe_product_id; + $priceIdsBefore = PlanPrice::query() + ->whereHas('version', fn ($q) => $q->where('plan_family_id', $family->id)) + ->orderBy('id')->pluck('stripe_price_id', 'id'); + + expect($productIdBefore)->not->toBeNull() + ->and($priceIdsBefore->filter())->not->toBeEmpty(); + + // Genau der Fall von Enterprise: verkäuflich, bereits gespiegelt, und dann + // vom Betreiber auf intern umgestellt. Was einmal angelegt wurde, bleibt + // verknüpft — es kommt nur nichts Neues mehr hinzu. + $family->update(['internal' => true]); + + // Die Steuer ändert sich noch dazu: ohne die Sperre in + // stripe:sync-catalogue wäre das genau der Auslöser, der einen + // Bruttopreis für erneuerungsbedürftig hält und einen neuen anlegt — an + // einem Paket, das niemand mehr bezahlt. + Settings::set('company.tax_rate', 10.0); + + $this->artisan('stripe:sync-catalogue')->assertSuccessful(); + + $family->refresh(); + + expect($family->stripe_product_id)->toBe($productIdBefore) + ->and(PlanPrice::query() + ->whereHas('version', fn ($q) => $q->where('plan_family_id', $family->id)) + ->orderBy('id')->pluck('stripe_price_id', 'id')->all()) + ->toBe($priceIdsBefore->all()) + // Nicht das gesamte $stripe->archived — die Rate hat sich für den + // ganzen Katalog geändert, und Team und Business (weiterhin + // verkäuflich) ARCHIVIEREN zu Recht ihre alten Bruttopreise. Was hier + // zählt, ist einzig, dass KEINER der Preise dieser (jetzt internen) + // Familie darunter ist. + ->and(array_intersect($priceIdsBefore->filter()->all(), $stripe->archived))->toBe([]); }); it('gives each price its own recurring interval', function () { @@ -123,8 +201,8 @@ it('gives each price its own recurring interval', function () { // Monthly and yearly cannot share a Price, because the interval belongs to // the Price itself. - expect($intervals['month'])->toBe(8) - ->and($intervals['year'])->toBe(8); + expect($intervals['month'])->toBe(6) + ->and($intervals['year'])->toBe(6); // Die Fassung im Verkauf, ausdrücklich benannt: seit der Umschaltung auf die // neue Leiter gibt es zwei monatliche Team-Preise bei Stripe, und „der @@ -167,7 +245,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(32) + expect(planPrices($stripe))->toHaveCount(24) ->and(planPrices($stripe)->pluck('amount'))->not->toContain(19900); }); @@ -185,9 +263,11 @@ it('does not mint a second object when a run is interrupted before the id is sto // Stripe replays the original answer for a repeated idempotency key, so the // catalogue reconnects to what is already there instead of duplicating it — // and a Price cannot be deleted afterwards to tidy up. - expect(planProducts($stripe))->toHaveCount(5) - ->and(planPrices($stripe))->toHaveCount(32) - ->and(PlanPrice::query()->whereNull('stripe_price_id')->count())->toBe(0); + expect(planProducts($stripe))->toHaveCount(3) + ->and(planPrices($stripe))->toHaveCount(24) + ->and(PlanPrice::query() + ->whereHas('version.family', fn ($q) => $q->where('internal', false)) + ->whereNull('stripe_price_id')->count())->toBe(0); }); it('does not revive a contract Stripe already ended', function () { @@ -232,7 +312,7 @@ it('does not put a plan that has never been published in Stripe at all', functio // Not even the Product: it would be a price-list entry for something that // has promised nothing to anyone. - expect(planProducts($stripe))->toHaveCount(5) + expect(planProducts($stripe))->toHaveCount(3) ->and($family->fresh()->stripe_product_id)->toBeNull(); }); diff --git a/tests/Feature/Billing/StripeProductAdoptionTest.php b/tests/Feature/Billing/StripeProductAdoptionTest.php index 9ea9275..d2b62a2 100644 --- a/tests/Feature/Billing/StripeProductAdoptionTest.php +++ b/tests/Feature/Billing/StripeProductAdoptionTest.php @@ -38,7 +38,11 @@ beforeEach(function () { it('adopts an orphaned family product instead of minting a second one', function () { // A run that got as far as creating the Product and died before storing its // id: Stripe has it, plan_families does not. - $family = PlanFamily::query()->orderBy('tier')->firstOrFail(); + // + // Not simply the lowest tier: since Task 12 that is the internal test + // package, which stripe:sync-catalogue skips outright and would never + // adopt anything for. + $family = PlanFamily::query()->where('internal', false)->orderBy('tier')->firstOrFail(); $family->update(['stripe_product_id' => null]); $this->stripe->plantProduct('prod_orphan_family', $family->name, [ @@ -136,8 +140,9 @@ it('says it adopted, not that it created', function () { it('names a duplicate product in its report, without touching it', function () { // Nothing has been synced yet, same as the orphan test above — the family // has no product of its own, and two stray ones claiming to be it are the - // whole of what Stripe has for it. - $family = PlanFamily::query()->orderBy('tier')->firstOrFail(); + // whole of what Stripe has for it. Not the internal test package, for the + // same reason as above: sync-catalogue never looks at it at all. + $family = PlanFamily::query()->where('internal', false)->orderBy('tier')->firstOrFail(); $family->update(['stripe_product_id' => null]); $metadata = ['plan_family' => $family->key, 'plan_family_id' => (string) $family->id]; @@ -165,12 +170,16 @@ it('does not fold a module Product adoption into the created/minted count', func ]); // Computed from the catalogue rather than hardcoded, the way - // ReverseChargePriceTest sizes a rate-change sweep: one Product per family, - // two treatments per priced row, and monthly+yearly at both treatments for - // every module — all of it newly minted here, nothing pre-existing to - // adopt except the one Product just planted above. + // ReverseChargePriceTest sizes a rate-change sweep: one Product per + // SELLABLE family, two treatments per priced row, and monthly+yearly at + // both treatments for every module — all of it newly minted here, nothing + // pre-existing to adopt except the one Product just planted above. + // Internal families (Task 12) are skipped by the sync entirely and count + // for neither the family nor the price half of this sum. $modules = count(array_merge(array_keys((array) config('provisioning.addons')), ['storage'])); - $expectedCreated = PlanFamily::query()->count() + PlanPrice::query()->count() * 2 + $modules * 4; + $expectedCreated = PlanFamily::query()->where('internal', false)->count() + + PlanPrice::query()->whereHas('version.family', fn ($q) => $q->where('internal', false))->count() * 2 + + $modules * 4; $this->artisan('stripe:sync-catalogue') ->expectsOutputToContain("{$expectedCreated} object(s) created, 0 adopted in Stripe.") diff --git a/tests/Feature/Readiness/BillingChecksTest.php b/tests/Feature/Readiness/BillingChecksTest.php index aee5a35..3c92f5d 100644 --- a/tests/Feature/Readiness/BillingChecksTest.php +++ b/tests/Feature/Readiness/BillingChecksTest.php @@ -217,3 +217,22 @@ it('is satisfied again once the catalogue was synced in the mode that is running expect(checkFor('billing.catalogue_synced')->satisfied)->toBeTrue(); }); + +it('does not raise a permanent alarm for an internal family stripe never mirrors', function () { + // stripe:sync-catalogue skips every family marked `internal` outright — + // Product and Prices both, dry run and real (Task 12: a Stripe Price + // cannot be deleted once minted, and a package nobody is ever billed for + // is not mirrored). Both the internal test package and Enterprise are + // published and on sale (`sales_enabled`) but `internal = true`, so their + // price rows stay unsynced for ever, however often the sync runs. Unlike + // the draft above, there is no command that would ever clear this one — a + // query that does not also filter on `internal` would keep this check + // permanently unsatisfied on every installation. + PlanPrice::query() + ->whereHas('version.family', fn ($query) => $query->where('internal', false)) + ->update(['stripe_price_id' => 'price_already_synced']); + + StripeCatalogueMode::record(); + + expect(checkFor('billing.catalogue_synced')->satisfied)->toBeTrue(); +});