From bcd5a5bd7b6d8aae5ac6015aa654349937765a41 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 29 Aug 2026 17:06:03 +0930 Subject: [PATCH] Require --scope (local/remote/both) for admin:fixProfileCounts --all Bulk --all reconciliation previously scanned both local and remote profiles implicitly. Now --all requires an explicit --scope of local, remote, or both. --active stays local-only and rejects a non-local --scope. Adds the BelongsTo return type to Profile::user() so the whereHas('user') scope filter passes Larastan, and adds tests for scope requirement/validation and local/remote filtering. --- app/Console/Commands/FixProfileCounts.php | 48 ++++++++++++++-- app/Models/Profile.php | 3 +- .../Account/ProfileCountReconcileTest.php | 56 +++++++++++++++++++ 3 files changed, 100 insertions(+), 7 deletions(-) diff --git a/app/Console/Commands/FixProfileCounts.php b/app/Console/Commands/FixProfileCounts.php index 50698fb9c..c9bbb2cb4 100644 --- a/app/Console/Commands/FixProfileCounts.php +++ b/app/Console/Commands/FixProfileCounts.php @@ -18,8 +18,9 @@ class FixProfileCounts extends Command */ protected $signature = 'admin:fixProfileCounts {id? : Profile id or username to resync (omit with --all)} - {--all : Scan all profiles and resync any with drifted counts} + {--all : Scan all profiles and resync any with drifted counts (requires --scope)} {--active=* : Scan only local accounts active within N days (default 30). Bulk mode; mutually exclusive with --all} + {--scope= : Which profiles to scan in --all mode: local, remote, or both} {--type= : Restrict to a single metric: followers, following, or statuses (default: all three)} {--dispatch : Queue FollowServiceWarmCache for follower/following instead of recomputing inline} {--dry-run : Report drift without changing anything} @@ -37,12 +38,19 @@ class FixProfileCounts extends Command */ protected const METRICS = ['followers', 'following', 'statuses']; + /** + * Valid --scope values for --all mode. + * + * @var array + */ + protected const SCOPES = ['local', 'remote', 'both']; + /** * The console command description. * * @var string */ - protected $description = 'Resync a profile\'s cached counts (followers, following, statuses) from source-of-truth tables. Use --all or --active for bulk reconciliation.'; + protected $description = 'Resync a profile\'s cached counts (followers, following, statuses) from source-of-truth tables. Use --all --scope=local|remote|both, or --active, for bulk reconciliation.'; /** * Execute the console command. @@ -78,6 +86,27 @@ class FixProfileCounts extends Command return 1; } + $scope = $this->option('scope'); + if ($scope !== null && ! in_array($scope, self::SCOPES, true)) { + $this->error('Invalid --scope "'.$scope.'". Use one of: '.implode(', ', self::SCOPES).'.'); + + return 1; + } + + if ($all && $scope === null) { + $this->error('--all requires --scope (local, remote, or both).'); + + return 1; + } + + if ($active && $scope !== null && $scope !== 'local') { + // --active filters on users.last_active_at, which only exists for + // local accounts, so remote/both make no sense here. + $this->error('--active only applies to local accounts; --scope must be omitted or "local".'); + + return 1; + } + if ($id) { $profile = ctype_digit((string) $id) ? Profile::find($id) @@ -97,11 +126,11 @@ class FixProfileCounts extends Command // Bulk mode (--all or --active): scan and only touch drifted profiles. $dryRun = $this->option('dry-run'); - $scope = $active + $scopeLabel = $active ? 'local accounts active in the last '.$activeDays.' days' - : 'all drifted profiles'; + : $scope.' profiles'; - if (! $dryRun && ! $this->option('force') && ! $this->confirm('Resync cached counts for '.$scope.'?', true)) { + if (! $dryRun && ! $this->option('force') && ! $this->confirm('Resync cached counts for '.$scopeLabel.'?', true)) { $this->comment('Aborted.'); return 0; @@ -118,7 +147,14 @@ class FixProfileCounts extends Command $q->whereNotNull('last_active_at') ->where('last_active_at', '>=', $cutoff); }); + } elseif ($scope === 'local') { + // Local profiles have no domain. + $query->whereNull('domain'); + } elseif ($scope === 'remote') { + // Remote/federated profiles have a domain set. + $query->whereNotNull('domain'); } + // scope === 'both' applies no domain filter. $fixed = 0; $scanned = 0; @@ -130,7 +166,7 @@ class FixProfileCounts extends Command }); $this->newLine(); - $this->info('Scanned '.$scanned.' profiles ('.$scope.'); '.($this->option('dry-run') ? 'drifted' : 'resynced').': '.$fixed.'.'); + $this->info('Scanned '.$scanned.' profiles ('.$scopeLabel.'); '.($this->option('dry-run') ? 'drifted' : 'resynced').': '.$fixed.'.'); return 0; } diff --git a/app/Models/Profile.php b/app/Models/Profile.php index 5e85e1f19..48f4cb54f 100644 --- a/app/Models/Profile.php +++ b/app/Models/Profile.php @@ -7,6 +7,7 @@ use App\Services\FollowerService; use App\Util\Lexer\PrettyNumber; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasOne; use Illuminate\Database\Eloquent\SoftDeletes; @@ -60,7 +61,7 @@ class Profile extends Model ]; } - public function user() + public function user(): BelongsTo { return $this->belongsTo(User::class); } diff --git a/tests/Feature/Account/ProfileCountReconcileTest.php b/tests/Feature/Account/ProfileCountReconcileTest.php index 5ef3ce6b2..46b63bb7d 100644 --- a/tests/Feature/Account/ProfileCountReconcileTest.php +++ b/tests/Feature/Account/ProfileCountReconcileTest.php @@ -1,6 +1,7 @@ artisan('admin:fixProfileCounts', ['id' => '1', '--type' => 'bogus']) ->assertExitCode(1); }); + + it('requires --scope when using --all', function () { + $this->artisan('admin:fixProfileCounts', ['--all' => true, '--force' => true]) + ->assertExitCode(1); + }); + + it('rejects an invalid --scope', function () { + $this->artisan('admin:fixProfileCounts', ['--all' => true, '--scope' => 'bogus', '--force' => true]) + ->assertExitCode(1); + }); + + it('rejects --active combined with a non-local --scope', function () { + $this->artisan('admin:fixProfileCounts', ['--active' => ['30'], '--scope' => 'remote']) + ->assertExitCode(1); + }); + + it('with --scope=local only reconciles local profiles', function () { + $localUser = User::factory()->create(); + $localUser->refresh(); + $local = $localUser->profile; + $local->followers_count = 100; // drift + $local->save(); + + $remote = Profile::create([ + 'username' => 'remote@x.example', + 'domain' => 'x.example', + 'followers_count' => 100, // drift, must be left alone + ]); + + $this->artisan('admin:fixProfileCounts', ['--all' => true, '--scope' => 'local', '--force' => true]) + ->assertExitCode(0); + + expect((int) $local->fresh()->followers_count)->toBe(0); + expect((int) $remote->fresh()->followers_count)->toBe(100); + }); + + it('with --scope=remote only reconciles remote profiles', function () { + $localUser = User::factory()->create(); + $localUser->refresh(); + $local = $localUser->profile; + $local->followers_count = 100; // drift, must be left alone + $local->save(); + + $remote = Profile::create([ + 'username' => 'remote2@x.example', + 'domain' => 'x.example', + 'followers_count' => 100, // drift + ]); + + $this->artisan('admin:fixProfileCounts', ['--all' => true, '--scope' => 'remote', '--force' => true]) + ->assertExitCode(0); + + expect((int) $local->fresh()->followers_count)->toBe(100); + expect((int) $remote->fresh()->followers_count)->toBe(0); + }); });