refactor(billing): single source of truth for the billed-user check

Extract the flag-independent billed check to User::isBilled() and derive
hasActiveSubscriptionOrTrial() from it, so the definition lives in one place.
Add tests locking the trialing, past_due and grace-period branches.
This commit is contained in:
Víctor Falcón 2026-07-17 10:34:39 +02:00
parent f140f5c785
commit 09076684a4
3 changed files with 73 additions and 25 deletions

View File

@ -5,7 +5,6 @@ namespace App\Console\Commands\Concerns;
use App\Models\User;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Support\Collection;
use Laravel\Cashier\Subscription;
trait FindsUsersWithLegacyEncryption
{
@ -34,32 +33,17 @@ trait FindsUsersWithLegacyEncryption
* Drop users who are still being billed. These accounts must never be emailed a
* deletion warning nor deleted while a subscription or trial is active.
*
* Unlike {@see User::hasActiveSubscriptionOrTrial()}, this check is intentionally
* independent of the `subscriptions.enabled` feature flag: that flag controls whether
* the paywall is enforced, not whether real Stripe subscriptions exist. A destructive
* command must never delete a paying customer just because the flag is off in the
* runtime that happens to execute it.
* {@see User::isBilled()} is used instead of {@see User::hasActiveSubscriptionOrTrial()}
* because it is independent of the `subscriptions.enabled` feature flag: that flag
* controls whether the paywall is enforced, not whether real Stripe subscriptions
* exist. A destructive command must never delete a paying customer just because the
* flag is off in the runtime that happens to execute it.
*
* @param Collection<int, User> $users
* @return Collection<int, User>
*/
protected function excludeBilledUsers(Collection $users): Collection
{
return $users->reject(fn (User $user): bool => $this->isBilled($user))->values();
}
/**
* Whether the user is on a generic trial or holds a subscription that is still
* valid and not merely coasting through its grace period (it will not renew).
*/
private function isBilled(User $user): bool
{
if ($user->onGenericTrial()) {
return true;
}
return $user->subscriptions->contains(
fn (Subscription $subscription): bool => $subscription->valid() && ! $subscription->onGracePeriod()
);
return $users->reject(fn (User $user): bool => $user->isBilled())->values();
}
}

View File

@ -409,10 +409,21 @@ class User extends Authenticatable implements HasLocalePreference, MustVerifyEma
*/
public function hasActiveSubscriptionOrTrial(): bool
{
if (! config('subscriptions.enabled')) {
return false;
}
return config('subscriptions.enabled') && $this->isBilled();
}
/**
* Whether the user is on a generic trial or holds a subscription that is still
* valid and not merely coasting through its grace period (it will not renew).
*
* Unlike hasActiveSubscriptionOrTrial() this is independent of the
* `subscriptions.enabled` flag: that flag controls whether the paywall is
* enforced, not whether real Stripe subscriptions exist. Destructive flows that
* must never touch a paying customer (the encryption cleanup commands) rely on
* this flag-independent check.
*/
public function isBilled(): bool
{
if ($this->onGenericTrial()) {
return true;
}

View File

@ -45,6 +45,59 @@ test('it never deletes a subscribed user even when the subscriptions flag is off
expect(User::whereKey($subscribed->id)->exists())->toBeTrue();
});
test('it never deletes a trialing subscriber', function () {
config(['subscriptions.enabled' => false]);
$trialing = User::factory()->create(['last_active_at' => now()->subDays(30)]);
Transaction::factory()->for($trialing)->create();
$trialing->subscriptions()->create([
'type' => 'default',
'stripe_id' => 'sub_trial123',
'stripe_status' => 'trialing',
'stripe_price' => 'price_test123',
'trial_ends_at' => now()->addDays(5),
]);
artisan('encryption:delete-accounts', ['--force' => true])->assertSuccessful();
expect(User::whereKey($trialing->id)->exists())->toBeTrue();
});
test('it never deletes a past-due subscriber', function () {
config(['subscriptions.enabled' => false]);
$pastDue = User::factory()->create(['last_active_at' => now()->subDays(30)]);
Transaction::factory()->for($pastDue)->create();
$pastDue->subscriptions()->create([
'type' => 'default',
'stripe_id' => 'sub_pastdue123',
'stripe_status' => 'past_due',
'stripe_price' => 'price_test123',
]);
artisan('encryption:delete-accounts', ['--force' => true])->assertSuccessful();
expect(User::whereKey($pastDue->id)->exists())->toBeTrue();
});
test('it deletes a subscriber who is only in the cancellation grace period', function () {
config(['subscriptions.enabled' => false]);
$grace = User::factory()->create(['last_active_at' => now()->subDays(30)]);
Transaction::factory()->for($grace)->create();
$grace->subscriptions()->create([
'type' => 'default',
'stripe_id' => 'sub_grace123',
'stripe_status' => 'active',
'stripe_price' => 'price_test123',
'ends_at' => now()->addDays(5),
]);
artisan('encryption:delete-accounts', ['--force' => true])->assertSuccessful();
expect(User::whereKey($grace->id)->exists())->toBeFalse();
});
test('it never deletes the demo account', function () {
$demo = User::factory()->create([
'email' => config('app.demo.email'),