fix(encryption): never email or delete users who are still being billed (#656)

## What

The `encryption:delete-accounts` and `encryption:notify-removal`
commands target users who still hold legacy client-side encrypted data.
They should **never** warn or delete an account that is still being
billed.

## Changes

- Add `excludeBilledUsers()` to the shared
`FindsUsersWithLegacyEncryption` trait. It drops any user where
`hasActiveSubscriptionOrTrial()` is true — a valid subscription outside
its grace period, or an active generic trial. This reuses the existing
domain method (whose docblock already states such users must cancel
before deletion) instead of reimplementing Cashier's subscription-status
logic in SQL.
- Both commands apply the filter right after fetching their candidates.
- Eager-load `subscriptions` in the base query to avoid an N+1 when the
filter runs.

## Tests

- `it never deletes a subscribed user` (delete command)
- `it never warns a subscribed user` (notify command)

Both new tests plus the existing suite pass (8/8). Pint clean.
This commit is contained in:
Víctor Falcón 2026-07-07 18:04:27 +02:00 committed by GitHub
parent d504e70309
commit 7d750fa1a8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 61 additions and 9 deletions

View File

@ -4,6 +4,7 @@ namespace App\Console\Commands\Concerns;
use App\Models\User;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Support\Collection;
trait FindsUsersWithLegacyEncryption
{
@ -11,11 +12,14 @@ trait FindsUsersWithLegacyEncryption
* Users who still have at least one client-side encrypted transaction or account
* (a non-null `*_iv` column is the source of truth, not the stale `accounts.encrypted` flag).
*
* Subscriptions are eager-loaded so callers can filter with {@see excludeBilledUsers()}
* without an N+1 query.
*
* @return Builder<User>
*/
protected function usersWithLegacyEncryption(): Builder
{
return User::query()->where(function (Builder $query): void {
return User::query()->with('subscriptions')->where(function (Builder $query): void {
$query->whereHas('transactions', function (Builder $transactions): void {
$transactions->whereNotNull('description_iv')
->orWhereNotNull('notes_iv');
@ -24,4 +28,16 @@ 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.
*
* @param Collection<int, User> $users
* @return Collection<int, User>
*/
protected function excludeBilledUsers(Collection $users): Collection
{
return $users->reject(fn (User $user): bool => $user->hasActiveSubscriptionOrTrial())->values();
}
}

View File

@ -36,13 +36,15 @@ class DeleteEncryptedDataAccountsCommand extends Command
$days = (int) $this->option('days');
$cutoff = now()->subDays($days);
$users = $this->usersWithLegacyEncryption()
->where('email', '!=', config('app.demo.email'))
->where(function (Builder $query) use ($cutoff): void {
$query->whereNull('last_active_at')
->orWhere('last_active_at', '<', $cutoff);
})
->get();
$users = $this->excludeBilledUsers(
$this->usersWithLegacyEncryption()
->where('email', '!=', config('app.demo.email'))
->where(function (Builder $query) use ($cutoff): void {
$query->whereNull('last_active_at')
->orWhere('last_active_at', '<', $cutoff);
})
->get()
);
if ($users->isEmpty()) {
$this->info('No accounts to delete.');

View File

@ -39,7 +39,7 @@ class NotifyEncryptedDataRemovalCommand extends Command
*/
public function handle(): int
{
$users = $this->usersWithLegacyEncryption()->get();
$users = $this->excludeBilledUsers($this->usersWithLegacyEncryption()->get());
if ($users->isEmpty()) {
$this->info('No users with encrypted data found.');

View File

@ -28,6 +28,23 @@ test('it soft-deletes users with encrypted data who did not sign in within the g
expect(User::whereKey($clean->id)->exists())->toBeTrue();
});
test('it never deletes a subscribed user', function () {
config(['subscriptions.enabled' => true]);
$subscribed = User::factory()->create(['last_active_at' => now()->subDays(30)]);
Transaction::factory()->for($subscribed)->create();
$subscribed->subscriptions()->create([
'type' => 'default',
'stripe_id' => 'sub_test123',
'stripe_status' => 'active',
'stripe_price' => 'price_test123',
]);
artisan('encryption:delete-accounts', ['--force' => true])->assertSuccessful();
expect(User::whereKey($subscribed->id)->exists())->toBeTrue();
});
test('it never deletes the demo account', function () {
$demo = User::factory()->create([
'email' => config('app.demo.email'),

View File

@ -28,6 +28,23 @@ test('it queues a warning email for users with encrypted transactions or account
Queue::assertNotPushed(SendUpdateEmailJob::class, fn (SendUpdateEmailJob $job) => $job->user->is($clean));
});
test('it never warns a subscribed user', function () {
config(['subscriptions.enabled' => true]);
$subscribed = User::factory()->create();
Transaction::factory()->for($subscribed)->create();
$subscribed->subscriptions()->create([
'type' => 'default',
'stripe_id' => 'sub_test123',
'stripe_status' => 'active',
'stripe_price' => 'price_test123',
]);
artisan('encryption:notify-removal', ['--force' => true])->assertSuccessful();
Queue::assertNotPushed(SendUpdateEmailJob::class);
});
test('it queues on the emails queue', function () {
$user = User::factory()->create();
Transaction::factory()->for($user)->create();