fix: do not set plan_id on checkout creation; drop redundant guards
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.
This commit is contained in:
parent
f72a97f676
commit
aadfe7d6e8
3 changed files with 4 additions and 49 deletions
|
|
@ -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]));
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue