From 2c9e9ea3984e945c8691c180796d6d81a719d1d4 Mon Sep 17 00:00:00 2001 From: nexxo Date: Thu, 30 Jul 2026 12:52:27 +0200 Subject: [PATCH] Watch the deletion branch delete the right row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 2026_07_31_210000_one_row_per_stripe_price deletes rows from a live table — the one place in this branch where a mistake loses data, not merely a place where "read the migration" is enough. Nothing exercised that branch: every other test in this file starts from an empty, already-migrated table, so the delete never had anything to find. RefreshDatabase has already run the migration by the time any test starts, so its own unique index refuses the very duplicate this needs to insert. The new test drops the index first, puts two rows on one Price back the way a pre-migration table actually looked, and re-runs up() (require, not require_once, the way the other migration-replay tests in this suite already do) to reach the branch directly. It asserts the row with the LOWER id survives, not merely that a row survives: that ordering is a decision made on the operator's behalf, and it is the row anything already billing is likeliest to have been reading. Checked by temporarily inverting min('id') to max('id') during this work — the test failed exactly there, then passed again once reverted. Co-Authored-By: Claude Opus 5 --- .../Billing/StripePriceAdoptionTest.php | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/Feature/Billing/StripePriceAdoptionTest.php b/tests/Feature/Billing/StripePriceAdoptionTest.php index 54336c1..aaf86ef 100644 --- a/tests/Feature/Billing/StripePriceAdoptionTest.php +++ b/tests/Feature/Billing/StripePriceAdoptionTest.php @@ -12,8 +12,11 @@ use App\Services\Billing\TaxTreatment; use App\Services\Stripe\FakeStripeClient; use App\Services\Stripe\StripeClient; use Illuminate\Contracts\Console\Kernel; +use Illuminate\Database\Schema\Blueprint; use Illuminate\Database\UniqueConstraintViolationException; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Schema; /** * A run that dies between Stripe's create and our insert leaves an orphan — and @@ -467,3 +470,65 @@ it('lets no two module rows claim one stripe price', function () { 'reverse_charge' => true, 'amount_cents' => 2900, 'interval' => 'month'])) ->toThrow(UniqueConstraintViolationException::class); }); + +it('deletes the newer of two rows already sharing one stripe price when the migration runs', function () { + // This is the one branch of 2026_07_31_210000_one_row_per_stripe_price that + // deletes rows from a live table, and it cannot be reached through the + // ordinary migrated schema: RefreshDatabase already ran this migration, so + // its OWN unique index would refuse the very duplicate this test needs to + // insert. Dropping the index first puts the table back into the shape the + // migration was written to find — the shape a real, never-yet-migrated + // production table was in — so up() can be run again and actually take its + // delete branch instead of finding nothing to do. + Schema::table('stripe_addon_prices', fn (Blueprint $table) => $table->dropUnique('stripe_addon_prices_price_unique')); + + // Two rows sharing one Price, differing in `reverse_charge` so the + // pre-existing COMPOSITE unique index (addon_key, reverse_charge, + // amount_cents, currency, interval) does not itself refuse the insert — + // the VAT-rate-zero case Block D warned about, where only the shared + // stripe_price_id gives the duplicate away. + $kept = DB::table('stripe_addon_prices')->insertGetId([ + 'addon_key' => 'priority_support', 'reverse_charge' => false, + 'amount_cents' => 2900, 'net_cents' => 2900, 'currency' => 'EUR', 'interval' => 'month', + 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', + 'created_at' => now(), 'updated_at' => now(), + ]); + + $duplicate = DB::table('stripe_addon_prices')->insertGetId([ + 'addon_key' => 'priority_support', 'reverse_charge' => true, + 'amount_cents' => 2900, 'net_cents' => 2900, 'currency' => 'EUR', 'interval' => 'month', + 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', + 'created_at' => now(), 'updated_at' => now(), + ]); + + // On its own Price entirely, and must survive untouched. + $unrelated = DB::table('stripe_addon_prices')->insertGetId([ + 'addon_key' => 'extra_storage', 'reverse_charge' => false, + 'amount_cents' => 1000, 'net_cents' => 1000, 'currency' => 'EUR', 'interval' => 'month', + 'stripe_product_id' => 'prod_storage', 'stripe_price_id' => 'price_untouched', + 'created_at' => now(), 'updated_at' => now(), + ]); + + // require, not require_once: the file `return`s a fresh anonymous-class + // instance every time it is evaluated, which is what lets this run the + // migration's up() a second time in this same process. + (require database_path('migrations/2026_07_31_210000_one_row_per_stripe_price.php'))->up(); + + // The LOWER id survives — the row that was written first, which is the one + // anything already billing is likeliest to have been reading. Were that + // ordering silently inverted, the surviving row could be one nothing ever + // pointed at. + expect(DB::table('stripe_addon_prices')->where('id', $kept)->exists())->toBeTrue() + ->and(DB::table('stripe_addon_prices')->where('id', $duplicate)->exists())->toBeFalse() + ->and(DB::table('stripe_addon_prices')->where('id', $unrelated)->exists())->toBeTrue(); + + // And the index is back in force: a further row on a third, otherwise + // distinct tuple, but the SAME Price id, is refused by stripe_price_id + // alone rather than being silently accepted. + expect(fn () => DB::table('stripe_addon_prices')->insert([ + 'addon_key' => 'priority_support', 'reverse_charge' => false, + 'amount_cents' => 4900, 'net_cents' => 4900, 'currency' => 'EUR', 'interval' => 'year', + 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', + 'created_at' => now(), 'updated_at' => now(), + ]))->toThrow(UniqueConstraintViolationException::class); +});