From a69dc28298e53e51cdaf5b7ed4fbdf2f66e26213 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Wed, 1 Apr 2026 11:52:03 -0300 Subject: [PATCH] fix: VerifyWorkspaceConnections uses TokenExpired/Disconnected escalation, eager load workspace --- app/Jobs/VerifyWorkspaceConnections.php | 41 ++++++++----------- app/Models/SocialAccount.php | 37 +++-------------- .../VerifyWorkspaceConnectionsTest.php | 18 +++++--- 3 files changed, 34 insertions(+), 62 deletions(-) diff --git a/app/Jobs/VerifyWorkspaceConnections.php b/app/Jobs/VerifyWorkspaceConnections.php index 28fa67b5..64b4fb6e 100644 --- a/app/Jobs/VerifyWorkspaceConnections.php +++ b/app/Jobs/VerifyWorkspaceConnections.php @@ -29,24 +29,24 @@ public function __construct(public Workspace $workspace) {} public function handle(ConnectionVerifier $verifier): void { - $connectedAccounts = $this->workspace->socialAccounts() + $accounts = $this->workspace->socialAccounts() + ->with('workspace.owner') ->whereIn('status', [Status::Connected, Status::TokenExpired]) ->get(); - if ($connectedAccounts->isEmpty()) { + if ($accounts->isEmpty()) { return; } - Log::info('Verifying workspace connections', [ - 'workspace_id' => $this->workspace->id, - 'workspace_name' => $this->workspace->name, - 'account_count' => $connectedAccounts->count(), - ]); - $disconnectedAccounts = collect(); - foreach ($connectedAccounts as $account) { + foreach ($accounts as $account) { if ($this->verifyAccount($verifier, $account)) { + // If was TokenExpired but now verified OK, mark as connected again + if ($account->status === Status::TokenExpired) { + $account->markAsConnected(); + } + continue; } @@ -63,11 +63,6 @@ private function verifyAccount(ConnectionVerifier $verifier, SocialAccount $acco try { $verifier->verify($account); - Log::info('Social account connection verified', [ - 'account_id' => $account->id, - 'platform' => $account->platform->value, - ]); - return true; } catch (TokenExpiredException $e) { Log::warning('Social account connection is invalid', [ @@ -76,11 +71,13 @@ private function verifyAccount(ConnectionVerifier $verifier, SocialAccount $acco 'error' => $e->getMessage(), ]); - $account->update([ - 'status' => Status::Disconnected, - 'error_message' => $e->getMessage(), - 'disconnected_at' => now(), - ]); + if ($account->status === Status::TokenExpired) { + // Second failure — escalate to Disconnected + $account->markAsDisconnected($e->getMessage()); + } else { + // First failure — mark as TokenExpired (softer state) + $account->markAsTokenExpired($e->getMessage()); + } return false; } catch (\Exception $e) { @@ -90,6 +87,7 @@ private function verifyAccount(ConnectionVerifier $verifier, SocialAccount $acco 'error' => $e->getMessage(), ]); + // Unknown error — don't mark as disconnected, retry next time return true; } } @@ -105,11 +103,6 @@ private function notifyOwner(Collection $disconnectedAccounts): void return; } - Log::info('Sending workspace disconnection notification', [ - 'workspace_id' => $this->workspace->id, - 'disconnected_count' => $disconnectedAccounts->count(), - ]); - $accountNames = $disconnectedAccounts ->map(fn ($account) => $account->platform->label().' (@'.($account->username ?? $account->display_name).')') ->implode(', '); diff --git a/app/Models/SocialAccount.php b/app/Models/SocialAccount.php index 365f7eb3..a8f9bdc0 100644 --- a/app/Models/SocialAccount.php +++ b/app/Models/SocialAccount.php @@ -133,38 +133,11 @@ public function markAsDisconnected(string $errorMessage): void public function markAsTokenExpired(string $errorMessage): void { - $lock = Cache::lock("social_account_disconnect:{$this->id}", 10); - - if ($lock->get()) { - try { - $this->refresh(); - $wasConnected = $this->status === Status::Connected; - - $this->update([ - 'status' => Status::TokenExpired, - 'error_message' => $errorMessage, - 'disconnected_at' => now(), - ]); - - if ($wasConnected && $this->workspace->owner) { - $platformName = $this->platform->label(); - $accountName = $this->username ?? $this->display_name; - - SendNotification::dispatch( - user: $this->workspace->owner, - workspaceId: $this->workspace_id, - type: Type::AccountDisconnected, - channel: Channel::Both, - title: "{$platformName} token expired", - body: "@{$accountName} needs to be reconnected", - data: ['social_account_id' => $this->id], - mailable: new AccountDisconnected($this), - ); - } - } finally { - $lock->release(); - } - } + $this->update([ + 'status' => Status::TokenExpired, + 'error_message' => $errorMessage, + 'disconnected_at' => now(), + ]); } public function markAsConnected(): void diff --git a/tests/Feature/VerifyWorkspaceConnectionsTest.php b/tests/Feature/VerifyWorkspaceConnectionsTest.php index 9dc377bc..2f351c78 100644 --- a/tests/Feature/VerifyWorkspaceConnectionsTest.php +++ b/tests/Feature/VerifyWorkspaceConnectionsTest.php @@ -38,7 +38,7 @@ Mail::assertNothingSent(); }); -test('job marks account as disconnected and sends email when token is invalid', function () { +test('job marks account as token expired on first failure and disconnected on second', function () { Mail::fake(); $workspace = Workspace::factory()->create(); @@ -50,10 +50,16 @@ app()->instance(ConnectionVerifier::class, $verifier); + // First run — marks as TokenExpired + VerifyWorkspaceConnections::dispatch($workspace); + + expect($account->fresh()->status)->toBe(Status::TokenExpired); + expect($account->fresh()->error_message)->toBe('Token expired'); + + // Second run — escalates to Disconnected VerifyWorkspaceConnections::dispatch($workspace); expect($account->fresh()->status)->toBe(Status::Disconnected); - expect($account->fresh()->error_message)->toBe('Token expired'); Mail::assertQueued(WorkspaceConnectionsDisconnected::class, function ($mail) use ($workspace) { return $mail->workspace->id === $workspace->id @@ -61,7 +67,7 @@ }); }); -test('job sends single email with all disconnected accounts', function () { +test('job sends single email with all failed accounts', function () { Mail::fake(); $workspace = Workspace::factory()->create(); @@ -76,8 +82,8 @@ VerifyWorkspaceConnections::dispatch($workspace); - expect($account1->fresh()->status)->toBe(Status::Disconnected); - expect($account2->fresh()->status)->toBe(Status::Disconnected); + expect($account1->fresh()->status)->toBe(Status::TokenExpired); + expect($account2->fresh()->status)->toBe(Status::TokenExpired); Mail::assertQueued(WorkspaceConnectionsDisconnected::class, function ($mail) use ($workspace) { return $mail->workspace->id === $workspace->id @@ -107,7 +113,7 @@ VerifyWorkspaceConnections::dispatch($workspace); expect($validAccount->fresh()->status)->toBe(Status::Connected); - expect($invalidAccount->fresh()->status)->toBe(Status::Disconnected); + expect($invalidAccount->fresh()->status)->toBe(Status::TokenExpired); Mail::assertQueued(WorkspaceConnectionsDisconnected::class, function ($mail) use ($invalidAccount) { return $mail->disconnectedAccounts->count() === 1