diff --git a/app/Console/Commands/Concerns/FindsUsersWithLegacyEncryption.php b/app/Console/Commands/Concerns/FindsUsersWithLegacyEncryption.php index 2c931a9d..4aa40eac 100644 --- a/app/Console/Commands/Concerns/FindsUsersWithLegacyEncryption.php +++ b/app/Console/Commands/Concerns/FindsUsersWithLegacyEncryption.php @@ -5,6 +5,7 @@ namespace App\Console\Commands\Concerns; use App\Models\User; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; +use Laravel\Cashier\Subscription; trait FindsUsersWithLegacyEncryption { @@ -33,11 +34,32 @@ 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. + * * @param Collection $users * @return Collection */ protected function excludeBilledUsers(Collection $users): Collection { - return $users->reject(fn (User $user): bool => $user->hasActiveSubscriptionOrTrial())->values(); + 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() + ); } } diff --git a/tests/Feature/Console/DeleteEncryptedDataAccountsCommandTest.php b/tests/Feature/Console/DeleteEncryptedDataAccountsCommandTest.php index cf1b2c03..ab5fcc99 100644 --- a/tests/Feature/Console/DeleteEncryptedDataAccountsCommandTest.php +++ b/tests/Feature/Console/DeleteEncryptedDataAccountsCommandTest.php @@ -28,8 +28,8 @@ 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]); +test('it never deletes a subscribed user even when the subscriptions flag is off', function () { + config(['subscriptions.enabled' => false]); $subscribed = User::factory()->create(['last_active_at' => now()->subDays(30)]); Transaction::factory()->for($subscribed)->create(); diff --git a/tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php b/tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php index 80f34244..1981a431 100644 --- a/tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php +++ b/tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php @@ -28,8 +28,8 @@ 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]); +test('it never warns a subscribed user even when the subscriptions flag is off', function () { + config(['subscriptions.enabled' => false]); $subscribed = User::factory()->create(); Transaction::factory()->for($subscribed)->create();