Fix duplicate proxy restart notifications

- Remove redundant ProxyStatusChangedUI dispatch from RestartProxyJob
  (ProxyStatusChanged event already triggers the listener that dispatches it)
- Remove redundant Traefik version check from RestartProxyJob
  (already handled by ProxyStatusChangedNotification listener)
- Add lastNotifiedStatus tracking to prevent duplicate toasts
- Remove notifications for unknown/default statuses (too noisy)
- Simplify RestartProxyJob to only handle stop/start logic

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Andras Bacsai
2025-12-03 16:09:47 +01:00
parent e4810a28d2
commit b00d8902f4
3 changed files with 68 additions and 84 deletions

View File

@@ -4,7 +4,6 @@ namespace App\Jobs;
use App\Actions\Proxy\StartProxy; use App\Actions\Proxy\StartProxy;
use App\Actions\Proxy\StopProxy; use App\Actions\Proxy\StopProxy;
use App\Enums\ProxyTypes;
use App\Events\ProxyStatusChangedUI; use App\Events\ProxyStatusChangedUI;
use App\Models\Server; use App\Models\Server;
use Illuminate\Bus\Queueable; use Illuminate\Bus\Queueable;
@@ -14,7 +13,6 @@ use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\Middleware\WithoutOverlapping; use Illuminate\Queue\Middleware\WithoutOverlapping;
use Illuminate\Queue\SerializesModels; use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Facades\Log;
class RestartProxyJob implements ShouldBeEncrypted, ShouldQueue class RestartProxyJob implements ShouldBeEncrypted, ShouldQueue
{ {
@@ -36,8 +34,6 @@ class RestartProxyJob implements ShouldBeEncrypted, ShouldQueue
public function handle() public function handle()
{ {
try { try {
$teamId = $this->server->team_id;
// Stop proxy // Stop proxy
StopProxy::run($this->server, restarting: true); StopProxy::run($this->server, restarting: true);
@@ -45,26 +41,14 @@ class RestartProxyJob implements ShouldBeEncrypted, ShouldQueue
$this->server->proxy->force_stop = false; $this->server->proxy->force_stop = false;
$this->server->save(); $this->server->save();
// Start proxy asynchronously to get activity // Start proxy asynchronously - the ProxyStatusChanged event will be dispatched
// when the remote process completes, which triggers ProxyStatusChangedNotification
// listener that handles UI updates and Traefik version checks
$activity = StartProxy::run($this->server, force: true, restarting: true); $activity = StartProxy::run($this->server, force: true, restarting: true);
// Store activity ID and dispatch event with it // Store activity ID for reference
if ($activity && is_object($activity)) { if ($activity && is_object($activity)) {
$this->activity_id = $activity->id; $this->activity_id = $activity->id;
ProxyStatusChangedUI::dispatch($teamId, $this->activity_id);
}
// Check Traefik version after restart (same as original behavior)
if ($this->server->proxyType() === ProxyTypes::TRAEFIK->value) {
$traefikVersions = get_traefik_versions();
if ($traefikVersions !== null) {
CheckTraefikVersionForServerJob::dispatch($this->server, $traefikVersions);
} else {
Log::warning('Traefik version check skipped: versions.json data unavailable', [
'server_id' => $this->server->id,
'server_name' => $this->server->name,
]);
}
} }
} catch (\Throwable $e) { } catch (\Throwable $e) {

View File

@@ -6,7 +6,6 @@ use App\Actions\Proxy\CheckProxy;
use App\Actions\Proxy\StartProxy; use App\Actions\Proxy\StartProxy;
use App\Actions\Proxy\StopProxy; use App\Actions\Proxy\StopProxy;
use App\Enums\ProxyTypes; use App\Enums\ProxyTypes;
use App\Jobs\CheckTraefikVersionForServerJob;
use App\Jobs\RestartProxyJob; use App\Jobs\RestartProxyJob;
use App\Models\Server; use App\Models\Server;
use App\Services\ProxyDashboardCacheService; use App\Services\ProxyDashboardCacheService;
@@ -29,6 +28,8 @@ class Navbar extends Component
public ?string $proxyStatus = 'unknown'; public ?string $proxyStatus = 'unknown';
public ?string $lastNotifiedStatus = null;
public function getListeners() public function getListeners()
{ {
$teamId = auth()->user()->currentTeam()->id; $teamId = auth()->user()->currentTeam()->id;
@@ -133,6 +134,11 @@ class Navbar extends Component
$this->dispatch('activityMonitor', $event['activityId']); $this->dispatch('activityMonitor', $event['activityId']);
} }
// Skip notification if we already notified about this status (prevents duplicates)
if ($this->lastNotifiedStatus === $this->proxyStatus) {
return;
}
switch ($this->proxyStatus) { switch ($this->proxyStatus) {
case 'running': case 'running':
$this->loadProxyConfiguration(); $this->loadProxyConfiguration();
@@ -140,6 +146,7 @@ class Navbar extends Component
// Don't show during normal start/restart flows (starting, restarting, stopping) // Don't show during normal start/restart flows (starting, restarting, stopping)
if (in_array($previousStatus, ['exited', 'stopped', 'unknown', null])) { if (in_array($previousStatus, ['exited', 'stopped', 'unknown', null])) {
$this->dispatch('success', 'Proxy is running.'); $this->dispatch('success', 'Proxy is running.');
$this->lastNotifiedStatus = $this->proxyStatus;
} }
break; break;
case 'exited': case 'exited':
@@ -147,25 +154,30 @@ class Navbar extends Component
// Don't show during normal stop/restart flows (stopping, restarting) // Don't show during normal stop/restart flows (stopping, restarting)
if (in_array($previousStatus, ['running'])) { if (in_array($previousStatus, ['running'])) {
$this->dispatch('info', 'Proxy has exited.'); $this->dispatch('info', 'Proxy has exited.');
$this->lastNotifiedStatus = $this->proxyStatus;
} }
break; break;
case 'stopping': case 'stopping':
$this->dispatch('info', 'Proxy is stopping.'); $this->dispatch('info', 'Proxy is stopping.');
$this->lastNotifiedStatus = $this->proxyStatus;
break; break;
case 'starting': case 'starting':
$this->dispatch('info', 'Proxy is starting.'); $this->dispatch('info', 'Proxy is starting.');
$this->lastNotifiedStatus = $this->proxyStatus;
break; break;
case 'restarting': case 'restarting':
$this->dispatch('info', 'Proxy is restarting.'); $this->dispatch('info', 'Proxy is restarting.');
$this->lastNotifiedStatus = $this->proxyStatus;
break; break;
case 'error': case 'error':
$this->dispatch('error', 'Proxy restart failed. Check logs.'); $this->dispatch('error', 'Proxy restart failed. Check logs.');
$this->lastNotifiedStatus = $this->proxyStatus;
break; break;
case 'unknown': case 'unknown':
$this->dispatch('info', 'Proxy status is unknown.'); // Don't notify for unknown status - too noisy
break; break;
default: default:
$this->dispatch('info', 'Proxy status updated.'); // Don't notify for other statuses
break; break;
} }

View File

@@ -4,23 +4,17 @@ namespace Tests\Unit\Jobs;
use App\Actions\Proxy\StartProxy; use App\Actions\Proxy\StartProxy;
use App\Actions\Proxy\StopProxy; use App\Actions\Proxy\StopProxy;
use App\Enums\ProxyTypes;
use App\Events\ProxyStatusChangedUI; use App\Events\ProxyStatusChangedUI;
use App\Jobs\CheckTraefikVersionForServerJob;
use App\Jobs\RestartProxyJob; use App\Jobs\RestartProxyJob;
use App\Models\Server; use App\Models\Server;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Queue\Middleware\WithoutOverlapping; use Illuminate\Queue\Middleware\WithoutOverlapping;
use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Queue;
use Mockery; use Mockery;
use Spatie\Activitylog\Models\Activity; use Spatie\Activitylog\Models\Activity;
use Tests\TestCase; use Tests\TestCase;
class RestartProxyJobTest extends TestCase class RestartProxyJobTest extends TestCase
{ {
use RefreshDatabase;
protected function tearDown(): void protected function tearDown(): void
{ {
Mockery::close(); Mockery::close();
@@ -43,12 +37,8 @@ class RestartProxyJobTest extends TestCase
{ {
// Mock Server // Mock Server
$server = Mockery::mock(Server::class); $server = Mockery::mock(Server::class);
$server->shouldReceive('getAttribute')->with('team_id')->andReturn(1);
$server->shouldReceive('getAttribute')->with('proxy')->andReturn((object) ['force_stop' => true]); $server->shouldReceive('getAttribute')->with('proxy')->andReturn((object) ['force_stop' => true]);
$server->shouldReceive('save')->once(); $server->shouldReceive('save')->once();
$server->shouldReceive('proxyType')->andReturn(ProxyTypes::TRAEFIK->value);
$server->shouldReceive('getAttribute')->with('id')->andReturn(1);
$server->shouldReceive('getAttribute')->with('name')->andReturn('test-server');
// Mock Activity // Mock Activity
$activity = Mockery::mock(Activity::class); $activity = Mockery::mock(Activity::class);
@@ -66,27 +56,12 @@ class RestartProxyJobTest extends TestCase
->with($server, force: true, restarting: true) ->with($server, force: true, restarting: true)
->andReturn($activity); ->andReturn($activity);
// Mock Events
Event::fake();
Queue::fake();
// Mock get_traefik_versions helper
$this->app->instance('traefik_versions', ['latest' => '2.10']);
// Execute job // Execute job
$job = new RestartProxyJob($server); $job = new RestartProxyJob($server);
$job->handle(); $job->handle();
// Assert activity ID was set // Assert activity ID was set
$this->assertEquals(123, $job->activity_id); $this->assertEquals(123, $job->activity_id);
// Assert event was dispatched
Event::assertDispatched(ProxyStatusChangedUI::class, function ($event) {
return $event->teamId === 1 && $event->activityId === 123;
});
// Assert Traefik version check was dispatched
Queue::assertPushed(CheckTraefikVersionForServerJob::class);
} }
public function test_job_handles_errors_gracefully() public function test_job_handles_errors_gracefully()
@@ -111,50 +86,17 @@ class RestartProxyJobTest extends TestCase
// Assert error event was dispatched // Assert error event was dispatched
Event::assertDispatched(ProxyStatusChangedUI::class, function ($event) { Event::assertDispatched(ProxyStatusChangedUI::class, function ($event) {
return $event->teamId === 1 && $event->activityId === null; return $event->teamId === 1;
}); });
} }
public function test_job_skips_traefik_version_check_for_non_traefik_proxies()
{
// Mock Server with non-Traefik proxy
$server = Mockery::mock(Server::class);
$server->shouldReceive('getAttribute')->with('team_id')->andReturn(1);
$server->shouldReceive('getAttribute')->with('proxy')->andReturn((object) ['force_stop' => true]);
$server->shouldReceive('save')->once();
$server->shouldReceive('proxyType')->andReturn(ProxyTypes::CADDY->value);
// Mock Activity
$activity = Mockery::mock(Activity::class);
$activity->id = 123;
// Mock Actions
$stopProxyMock = Mockery::mock('alias:'.StopProxy::class);
$stopProxyMock->shouldReceive('run')->once();
$startProxyMock = Mockery::mock('alias:'.StartProxy::class);
$startProxyMock->shouldReceive('run')->once()->andReturn($activity);
Event::fake();
Queue::fake();
// Execute job
$job = new RestartProxyJob($server);
$job->handle();
// Assert Traefik version check was NOT dispatched
Queue::assertNotPushed(CheckTraefikVersionForServerJob::class);
}
public function test_job_clears_force_stop_flag() public function test_job_clears_force_stop_flag()
{ {
// Mock Server // Mock Server
$proxy = (object) ['force_stop' => true]; $proxy = (object) ['force_stop' => true];
$server = Mockery::mock(Server::class); $server = Mockery::mock(Server::class);
$server->shouldReceive('getAttribute')->with('team_id')->andReturn(1);
$server->shouldReceive('getAttribute')->with('proxy')->andReturn($proxy); $server->shouldReceive('getAttribute')->with('proxy')->andReturn($proxy);
$server->shouldReceive('save')->once(); $server->shouldReceive('save')->once();
$server->shouldReceive('proxyType')->andReturn('NONE');
// Mock Activity // Mock Activity
$activity = Mockery::mock(Activity::class); $activity = Mockery::mock(Activity::class);
@@ -167,8 +109,6 @@ class RestartProxyJobTest extends TestCase
Mockery::mock('alias:'.StartProxy::class) Mockery::mock('alias:'.StartProxy::class)
->shouldReceive('run')->once()->andReturn($activity); ->shouldReceive('run')->once()->andReturn($activity);
Event::fake();
// Execute job // Execute job
$job = new RestartProxyJob($server); $job = new RestartProxyJob($server);
$job->handle(); $job->handle();
@@ -176,4 +116,52 @@ class RestartProxyJobTest extends TestCase
// Assert force_stop was set to false // Assert force_stop was set to false
$this->assertFalse($proxy->force_stop); $this->assertFalse($proxy->force_stop);
} }
public function test_job_stores_activity_id_when_activity_returned()
{
// Mock Server
$server = Mockery::mock(Server::class);
$server->shouldReceive('getAttribute')->with('proxy')->andReturn((object) ['force_stop' => true]);
$server->shouldReceive('save')->once();
// Mock Activity
$activity = Mockery::mock(Activity::class);
$activity->id = 456;
// Mock Actions
Mockery::mock('alias:'.StopProxy::class)
->shouldReceive('run')->once();
Mockery::mock('alias:'.StartProxy::class)
->shouldReceive('run')->once()->andReturn($activity);
// Execute job
$job = new RestartProxyJob($server);
$job->handle();
// Assert activity ID was stored
$this->assertEquals(456, $job->activity_id);
}
public function test_job_handles_string_return_from_start_proxy()
{
// Mock Server
$server = Mockery::mock(Server::class);
$server->shouldReceive('getAttribute')->with('proxy')->andReturn((object) ['force_stop' => true]);
$server->shouldReceive('save')->once();
// Mock Actions - StartProxy returns 'OK' string when proxy is disabled
Mockery::mock('alias:'.StopProxy::class)
->shouldReceive('run')->once();
Mockery::mock('alias:'.StartProxy::class)
->shouldReceive('run')->once()->andReturn('OK');
// Execute job
$job = new RestartProxyJob($server);
$job->handle();
// Assert activity ID remains null when string returned
$this->assertNull($job->activity_id);
}
} }