From aadfe7d6e80d22118c35c540f68920563795f3ef Mon Sep 17 00:00:00 2001
From: Paulo Castellano <paulo@castellanos.llc>
Date: Thu, 7 May 2026 11:08:30 -0300
Subject: [PATCH] fix: do not set plan_id on checkout creation; drop redundant
 guards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two issues from review:

1. BillingController::checkout was setting plan_id immediately after
   creating the Stripe Checkout session, before the user actually paid.
   If the user abandoned checkout, the account ended up with a plan it
   never paid for. Plan activation is now driven exclusively by the
   customer.subscription.created webhook, which fires only after a
   successful payment.

2. The 'if (\$account->wasChanged('plan_id')) { ... }' guards around
   forgetPlanFeatureCache() were tautological — Eloquent's update()
   already short-circuits when nothing changed, and Pennant forget()
   is idempotent, so an extra cache clear when the plan didn't move
   is harmless. Removing the guards keeps the listener and swap path
   readable.
---
 .../Controllers/App/BillingController.php     | 11 +-------
 app/Listeners/StripeEventListener.php         | 15 +++--------
 .../Listeners/StripeEventListenerTest.php     | 27 -------------------
 3 files changed, 4 insertions(+), 49 deletions(-)

diff --git a/app/Http/Controllers/App/BillingController.php b/app/Http/Controllers/App/BillingController.php
index 3338e4bb..67836c76 100644
--- a/app/Http/Controllers/App/BillingController.php
+++ b/app/Http/Controllers/App/BillingController.php
@@ -70,12 +70,6 @@ public function checkout(Request $request, Plan $plan): SymfonyResponse|Redirect
             'cancel_url' => route('app.subscribe'),
         ]);
 
-        $account->update(['plan_id' => $plan->id]);
-
-        if ($account->wasChanged('plan_id')) {
-            $account->forgetPlanFeatureCache();
-        }
-
         return Inertia::location($checkoutSession->url);
     }
 
@@ -167,10 +161,7 @@ public function swap(Request $request, Plan $plan): RedirectResponse
 
         $subscription->swap($priceId);
         $account->update(['plan_id' => $plan->id]);
-
-        if ($account->wasChanged('plan_id')) {
-            $account->forgetPlanFeatureCache();
-        }
+        $account->forgetPlanFeatureCache();
 
         return redirect()->route('app.billing.index')
             ->with('flash.success', __('billing.flash.plan_changed', ['plan' => $plan->name]));
diff --git a/app/Listeners/StripeEventListener.php b/app/Listeners/StripeEventListener.php
index 02f361f8..e2634b90 100644
--- a/app/Listeners/StripeEventListener.php
+++ b/app/Listeners/StripeEventListener.php
@@ -53,10 +53,7 @@ protected function handleSubscriptionCreated(Account $account, array $payload):
 
         if ($plan = $this->resolvePlanFromSubscriptionItems($payload, $account)) {
             $account->update(['plan_id' => $plan->id]);
-
-            if ($account->wasChanged('plan_id')) {
-                $account->forgetPlanFeatureCache();
-            }
+            $account->forgetPlanFeatureCache();
         }
 
         SubscriptionCreated::dispatch($account);
@@ -78,10 +75,7 @@ protected function handleSubscriptionUpdated(Account $account, array $payload):
 
         if ($plan = $this->resolvePlanFromSubscriptionItems($payload, $account)) {
             $account->update(['plan_id' => $plan->id]);
-
-            if ($account->wasChanged('plan_id')) {
-                $account->forgetPlanFeatureCache();
-            }
+            $account->forgetPlanFeatureCache();
         }
 
         $this->trackPlanChange($account, BillingEvent::Updated, $previousPlan, $payload);
@@ -100,10 +94,7 @@ protected function handleSubscriptionDeleted(Account $account, array $payload):
         $previousPlan = $account->plan?->name;
 
         $account->update(['plan_id' => null]);
-
-        if ($account->wasChanged('plan_id')) {
-            $account->forgetPlanFeatureCache();
-        }
+        $account->forgetPlanFeatureCache();
 
         $this->trackPlanChange($account, BillingEvent::Cancelled, $previousPlan, $payload);
     }
diff --git a/tests/Feature/Listeners/StripeEventListenerTest.php b/tests/Feature/Listeners/StripeEventListenerTest.php
index fba305d0..92b4c7cb 100644
--- a/tests/Feature/Listeners/StripeEventListenerTest.php
+++ b/tests/Feature/Listeners/StripeEventListenerTest.php
@@ -447,30 +447,3 @@
     expect(DB::table('features')->where('scope', 'account|'.$this->account->id)->count())
         ->toBe(0);
 });
-
-test('subscription updated with same plan does not flush the pennant cache', function () {
-    $starter = Plan::query()->where('slug', 'starter')->firstOrFail();
-    $this->account->update(['plan_id' => $starter->id]);
-
-    Feature::for($this->account)->value(WorkspaceLimit::class);
-
-    $cachedRow = DB::table('features')
-        ->where('scope', 'account|'.$this->account->id)
-        ->first();
-
-    expect($cachedRow)->not->toBeNull();
-
-    $this->listener->handle(new WebhookReceived([
-        'type' => 'customer.subscription.updated',
-        'data' => ['object' => [
-            'customer' => 'cus_test123',
-            'items' => ['data' => [['price' => ['id' => 'price_starter_monthly']]]],
-        ]],
-    ]));
-
-    $stillCachedRow = DB::table('features')
-        ->where('scope', 'account|'.$this->account->id)
-        ->first();
-
-    expect($stillCachedRow->id)->toBe($cachedRow->id);
-});