From 7d750fa1a8446742cf08b4f1c13df3ea3bd55f9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Tue, 7 Jul 2026 18:04:27 +0200 Subject: [PATCH] fix(encryption): never email or delete users who are still being billed (#656) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. --- .../FindsUsersWithLegacyEncryption.php | 18 +++++++++++++++++- .../DeleteEncryptedDataAccountsCommand.php | 16 +++++++++------- .../NotifyEncryptedDataRemovalCommand.php | 2 +- .../DeleteEncryptedDataAccountsCommandTest.php | 17 +++++++++++++++++ .../NotifyEncryptedDataRemovalCommandTest.php | 17 +++++++++++++++++ 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/app/Console/Commands/Concerns/FindsUsersWithLegacyEncryption.php b/app/Console/Commands/Concerns/FindsUsersWithLegacyEncryption.php index fce90ce2..2c931a9d 100644 --- a/app/Console/Commands/Concerns/FindsUsersWithLegacyEncryption.php +++ b/app/Console/Commands/Concerns/FindsUsersWithLegacyEncryption.php @@ -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 */ 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 $users + * @return Collection + */ + protected function excludeBilledUsers(Collection $users): Collection + { + return $users->reject(fn (User $user): bool => $user->hasActiveSubscriptionOrTrial())->values(); + } } diff --git a/app/Console/Commands/DeleteEncryptedDataAccountsCommand.php b/app/Console/Commands/DeleteEncryptedDataAccountsCommand.php index 1b45a774..1e1a76af 100644 --- a/app/Console/Commands/DeleteEncryptedDataAccountsCommand.php +++ b/app/Console/Commands/DeleteEncryptedDataAccountsCommand.php @@ -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.'); diff --git a/app/Console/Commands/NotifyEncryptedDataRemovalCommand.php b/app/Console/Commands/NotifyEncryptedDataRemovalCommand.php index fe14f651..4ebb8f82 100644 --- a/app/Console/Commands/NotifyEncryptedDataRemovalCommand.php +++ b/app/Console/Commands/NotifyEncryptedDataRemovalCommand.php @@ -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.'); diff --git a/tests/Feature/Console/DeleteEncryptedDataAccountsCommandTest.php b/tests/Feature/Console/DeleteEncryptedDataAccountsCommandTest.php index 8489411f..cf1b2c03 100644 --- a/tests/Feature/Console/DeleteEncryptedDataAccountsCommandTest.php +++ b/tests/Feature/Console/DeleteEncryptedDataAccountsCommandTest.php @@ -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'), diff --git a/tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php b/tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php index 5077fac7..80f34244 100644 --- a/tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php +++ b/tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php @@ -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();