From 7a4709da69c040f597c8847b1af0bad39fd6f4cb Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:31:00 +0200 Subject: [PATCH] fix(ui): scope env refreshes, block duplicate container names, use a confirm modal - refreshEnvs is now sent only to the environment variable list, and rows no longer listen to it, so adding or removing variables cannot queue an update for a removed row ("Snapshot missing"). Locking a row refreshes that row directly; the compose editor gets its own event. - saveCustomName() no longer saves a custom container name that another application on the server already uses; it restores the stored value. - Cancelling a GitHub runner uses the Coolify confirmation modal instead of a native browser dialog. --- app/Livewire/Project/Application/Advanced.php | 4 +- app/Livewire/Project/Application/General.php | 3 +- .../Project/Database/Redis/General.php | 3 +- app/Livewire/Project/Service/StackForm.php | 4 +- .../Shared/EnvironmentVariable/Show.php | 5 +- .../server/github-runner-executions.blade.php | 11 +- .../RefreshEnvsTargetingTest.php | 188 ++++++++++++++++++ .../AdvancedContainerNamingTest.php | 34 ++++ .../EnvironmentVariableListRefreshTest.php | 78 ++++++++ 9 files changed, 317 insertions(+), 13 deletions(-) create mode 100644 tests/Feature/EnvironmentVariable/RefreshEnvsTargetingTest.php create mode 100644 tests/v4/Browser/EnvironmentVariableListRefreshTest.php diff --git a/app/Livewire/Project/Application/Advanced.php b/app/Livewire/Project/Application/Advanced.php index 7161358ed0..da94470872 100644 --- a/app/Livewire/Project/Application/Advanced.php +++ b/app/Livewire/Project/Application/Advanced.php @@ -245,7 +245,6 @@ class Advanced extends Component return; } - $customInternalName = $this->customInternalName; $server = $this->application->destination->server; $allApplications = $server->applications(); @@ -253,9 +252,8 @@ class Advanced extends Component return $application->id !== $this->application->id && $application->settings->custom_internal_name === $this->customInternalName; }); if ($foundSameInternalName->isNotEmpty()) { + $this->customInternalName = $this->application->settings->custom_internal_name; $this->dispatch('error', 'This custom container name is already in use by another application on this server.'); - $this->customInternalName = $customInternalName; - $this->syncData(true); return; } diff --git a/app/Livewire/Project/Application/General.php b/app/Livewire/Project/Application/General.php index 8a336b659b..531aa834c0 100644 --- a/app/Livewire/Project/Application/General.php +++ b/app/Livewire/Project/Application/General.php @@ -5,6 +5,7 @@ namespace App\Livewire\Project\Application; use App\Enums\StaticImageTypes; use App\Jobs\ApplicationDeploymentJob; use App\Livewire\Project\Service\Storage; +use App\Livewire\Project\Shared\EnvironmentVariable\All; use App\Models\Application; use App\Rules\ValidGitBranch; use App\Services\Dns\ManagedDnsRecordCleanup; @@ -557,7 +558,7 @@ class General extends Component $showToast && $this->dispatch('success', 'Docker compose file loaded.'); $this->dispatch('compose_loaded'); $this->dispatch('storageCountsChanged')->to(Storage::class); - $this->dispatch('refreshEnvs'); + $this->dispatch('refreshEnvs')->to(All::class); } catch (\Throwable $e) { // Refresh model to get restored values from Application::loadComposeFile $this->application->refresh(); diff --git a/app/Livewire/Project/Database/Redis/General.php b/app/Livewire/Project/Database/Redis/General.php index d25f875459..3ab0d3eda9 100644 --- a/app/Livewire/Project/Database/Redis/General.php +++ b/app/Livewire/Project/Database/Redis/General.php @@ -4,6 +4,7 @@ namespace App\Livewire\Project\Database\Redis; use App\Actions\Database\StartDatabaseProxy; use App\Actions\Database\StopDatabaseProxy; +use App\Livewire\Project\Shared\EnvironmentVariable\All; use App\Models\Server; use App\Models\StandaloneRedis; use App\Support\ValidationPatterns; @@ -202,7 +203,7 @@ class General extends Component } catch (Exception $e) { return handleError($e, $this); } finally { - $this->dispatch('refreshEnvs'); + $this->dispatch('refreshEnvs')->to(All::class); } } diff --git a/app/Livewire/Project/Service/StackForm.php b/app/Livewire/Project/Service/StackForm.php index 94a0d06c9c..fd968c7f0e 100644 --- a/app/Livewire/Project/Service/StackForm.php +++ b/app/Livewire/Project/Service/StackForm.php @@ -2,6 +2,7 @@ namespace App\Livewire\Project\Service; +use App\Livewire\Project\Shared\EnvironmentVariable\All; use App\Models\Service; use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; @@ -179,7 +180,8 @@ class StackForm extends Component $this->service->refresh(); $this->service->saveComposeConfigs(); - $this->dispatch('refreshEnvs'); + $this->dispatch('refreshEnvs')->to(EditCompose::class); + $this->dispatch('refreshEnvs')->to(All::class); $this->dispatch('refreshServices'); $notify && $this->dispatch('success', 'Service saved.'); } catch (\Throwable $e) { diff --git a/app/Livewire/Project/Shared/EnvironmentVariable/Show.php b/app/Livewire/Project/Shared/EnvironmentVariable/Show.php index 70cdef7f6b..6aee8fceba 100644 --- a/app/Livewire/Project/Shared/EnvironmentVariable/Show.php +++ b/app/Livewire/Project/Shared/EnvironmentVariable/Show.php @@ -93,7 +93,6 @@ class Show extends Component public array $problematicVariables = []; protected $listeners = [ - 'refreshEnvs' => 'refresh', 'refresh', 'compose_loaded' => '$refresh', ]; @@ -296,8 +295,8 @@ class Show extends Component } $this->serialize(); $this->env->save(); - $this->checkEnvs(); - $this->dispatch('refreshEnvs'); + $this->refresh(); + $this->dispatch('refreshEnvs')->to(All::class); } public function instantSave() diff --git a/resources/views/livewire/server/github-runner-executions.blade.php b/resources/views/livewire/server/github-runner-executions.blade.php index e89499303f..13db5599fe 100644 --- a/resources/views/livewire/server/github-runner-executions.blade.php +++ b/resources/views/livewire/server/github-runner-executions.blade.php @@ -40,10 +40,13 @@ @endif @if ($execution->isActive()) @can('update', $server) - - Cancel - + @endcan @endif diff --git a/tests/Feature/EnvironmentVariable/RefreshEnvsTargetingTest.php b/tests/Feature/EnvironmentVariable/RefreshEnvsTargetingTest.php new file mode 100644 index 0000000000..2835dc28f1 --- /dev/null +++ b/tests/Feature/EnvironmentVariable/RefreshEnvsTargetingTest.php @@ -0,0 +1,188 @@ + 0]); + config(['constants.ssh.mux_enabled' => false]); + Server::flushIdentityMap(); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + $this->server = Server::factory()->create([ + 'team_id' => $this->team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $this->team->id])->id, + ]); + $this->destination = StandaloneDocker::query()->where('server_id', $this->server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); + + $this->application = Application::factory()->create([ + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); +}); + +/** + * A `refreshEnvs` dispatch without a target reaches every environment variable row + * as well as the list, which races row removal ("Snapshot missing on Livewire component"). + */ +function assertNoPageWideRefreshEnvs(Testable $component): void +{ + $pageWide = collect(data_get($component->effects, 'dispatches')) + ->filter(fn (array $dispatch): bool => $dispatch['name'] === 'refreshEnvs' + && ! isset($dispatch['to']) + && empty($dispatch['self'])); + + expect($pageWide)->toBeEmpty(); +} + +test('reloading the compose file refreshes only the environment variable list', function () { + $this->application->update([ + 'build_pack' => 'dockercompose', + 'git_repository' => 'https://github.com/coollabsio/compose-app', + 'git_branch' => 'main', + 'base_directory' => '/', + 'docker_compose_location' => '/docker-compose.yml', + ]); + Process::fake(function ($process) { + $command = is_array($process->command) ? implode(' ', $process->command) : $process->command; + if (str_contains($command, 'git --version')) { + return Process::result(output: 'git version 2.43.0'); + } + if (str_contains($command, 'head -c')) { + return Process::result(output: "services:\n web:\n image: nginx:alpine\n"); + } + + return Process::result(output: ''); + }); + + $component = Livewire::test(ApplicationGeneral::class, ['application' => $this->application->fresh()]) + ->call('loadComposeFile') + ->assertDispatched('success', 'Docker compose file loaded.') + ->assertDispatchedTo(All::class, 'refreshEnvs'); + + assertNoPageWideRefreshEnvs($component); +}); + +test('saving redis credentials refreshes only the environment variable list', function () { + $database = create_standalone_redis($this->environment->id, $this->destination, ['redis_password' => 'old-password'])->fresh(); + + $component = Livewire::test(RedisGeneral::class, ['database' => $database]) + ->set('redisPassword', 'new-password') + ->call('submit') + ->assertDispatched('success', 'Database updated.') + ->assertDispatchedTo(All::class, 'refreshEnvs'); + + assertNoPageWideRefreshEnvs($component); + expect($database->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->firstOrFail()->value) + ->toBe('new-password'); +}); + +test('saving a service refreshes the compose editor and the environment variable list only', function () { + Process::fake(); + $service = Service::factory()->create([ + 'name' => 'stack-service', + 'connect_to_docker_network' => false, + 'docker_compose_raw' => "services:\n app:\n image: nginx:alpine\n", + 'server_id' => $this->server->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'environment_id' => $this->environment->id, + ]); + + $component = Livewire::test(StackForm::class, ['service' => $service]) + ->call('submit') + ->assertDispatched('success', 'Service saved.') + ->assertDispatchedTo(All::class, 'refreshEnvs') + ->assertDispatchedTo(EditCompose::class, 'refreshEnvs'); + + assertNoPageWideRefreshEnvs($component); +}); + +test('locking a variable updates the row itself and refreshes only the list', function () { + $env = EnvironmentVariable::create([ + 'key' => 'LOCK_ME', + 'value' => 'secret', + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + + $component = Livewire::test(Show::class, ['env' => $env, 'type' => 'application']) + ->call('lock') + ->assertSet('isLocked', true) + ->assertSet('is_shown_once', true) + ->assertSet('valuesLoaded', false) + ->assertDispatchedTo(All::class, 'refreshEnvs'); + + assertNoPageWideRefreshEnvs($component); + expect($env->fresh()->is_shown_once)->toBeTruthy(); +}); + +test('environment variable rows do not handle the list-wide refreshEnvs event', function () { + $env = EnvironmentVariable::create([ + 'key' => 'ROW_KEY', + 'value' => 'value', + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + + $row = Livewire::test(Show::class, ['env' => $env, 'type' => 'application']); + + expect(fn () => $row->dispatch('refreshEnvs'))->toThrow(EventHandlerDoesNotExist::class); +}); + +test('refreshing the list renders added rows with current data and drops deleted rows', function () { + $removed = EnvironmentVariable::create([ + 'key' => 'REMOVED_KEY', + 'value' => 'value', + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + + $component = Livewire::test(All::class, ['resource' => $this->application]) + ->call('loadEnvironmentVariables') + ->assertSee('REMOVED_KEY'); + + $removed->delete(); + EnvironmentVariable::create([ + 'key' => 'ADDED_KEY', + 'value' => 'value', + 'comment' => 'fresh comment', + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + + $component->dispatch('refreshEnvs') + ->assertSee('ADDED_KEY') + ->assertSee('fresh comment') + ->assertDontSee('REMOVED_KEY'); +}); diff --git a/tests/Feature/Livewire/Project/Application/AdvancedContainerNamingTest.php b/tests/Feature/Livewire/Project/Application/AdvancedContainerNamingTest.php index ac381762cb..97659c3210 100644 --- a/tests/Feature/Livewire/Project/Application/AdvancedContainerNamingTest.php +++ b/tests/Feature/Livewire/Project/Application/AdvancedContainerNamingTest.php @@ -148,3 +148,37 @@ it('rejects a container name prefix already used on the server', function () { expect($application->settings()->first()->custom_container_name_prefix)->toBeNull(); }); + +it('does not save a custom container name already used on the server', function () { + $application = createApplicationForContainerNamingTest(); + $application->settings->update(['custom_internal_name' => 'original-name']); + Application::factory()->create([ + 'environment_id' => $application->environment_id, + 'destination_id' => $application->destination_id, + 'destination_type' => $application->destination_type, + ])->settings->update(['custom_internal_name' => 'taken-name']); + + $application = $application->fresh(['environment.project', 'settings', 'destination']); + + Livewire::test(Advanced::class, ['application' => $application]) + ->set('customInternalName', 'taken-name') + ->call('saveCustomName') + ->assertDispatched('error', 'This custom container name is already in use by another application on this server.') + ->assertNotDispatched('success') + ->assertSet('customInternalName', 'original-name'); + + expect($application->settings()->first()->custom_internal_name)->toBe('original-name'); +}); + +it('saves a unique custom container name', function () { + $application = createApplicationForContainerNamingTest(); + $application = $application->fresh(['environment.project', 'settings', 'destination']); + + Livewire::test(Advanced::class, ['application' => $application]) + ->set('customInternalName', 'Unique Name') + ->call('saveCustomName') + ->assertDispatched('success', 'Custom name saved.') + ->assertSet('customInternalName', 'unique-name'); + + expect($application->settings()->first()->custom_internal_name)->toBe('unique-name'); +}); diff --git a/tests/v4/Browser/EnvironmentVariableListRefreshTest.php b/tests/v4/Browser/EnvironmentVariableListRefreshTest.php new file mode 100644 index 0000000000..c1af6766d9 --- /dev/null +++ b/tests/v4/Browser/EnvironmentVariableListRefreshTest.php @@ -0,0 +1,78 @@ +set('app.maintenance.store', 'array'); + $this->stack = seedBrowserResourceStack(); + $this->application = createBrowserApplication($this->stack, [ + 'uuid' => 'app-browser-env-refresh', + 'name' => 'Env Refresh App', + 'build_pack' => 'dockerfile', + 'dockerfile' => "FROM nginx:alpine\n", + ]); + + foreach (['ALPHA_KEY', 'BRAVO_KEY', 'CHARLIE_KEY', 'DELTA_KEY'] as $key) { + EnvironmentVariable::create([ + 'key' => $key, + 'value' => strtolower($key), + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + } +}); + +it('locks and deletes environment variables repeatedly without Livewire errors', function () { + loginAndSkipBoarding(); + + $url = applicationConfigurationUrl( + $this->stack['project'], + $this->stack['environment'], + $this->application + ).'/environment-variables'; + + $page = visit($url); + $page->click('Accept and close') + ->assertSee('ALPHA_KEY') + ->assertSee('DELTA_KEY'); + + // Collect uncaught errors (e.g. "Snapshot missing on Livewire component") raised by the actions below. + $page->script(<<<'JS' + (() => { + window.__envListErrors = []; + window.addEventListener('error', (event) => window.__envListErrors.push(String(event.message))); + window.addEventListener('unhandledrejection', (event) => window.__envListErrors.push(String(event.reason))); + const consoleError = console.error; + console.error = (...args) => { + window.__envListErrors.push(args.map(String).join(' ')); + consoleError(...args); + }; + })() + JS); + + $page->click('button[title="ALPHA_KEY"]') + ->click('[data-environment-variable-update-actions] button:has-text("Lock"):visible') + ->wait(1); + $page->script('window.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape" }))'); + $page->wait(0.5); + + foreach (['BRAVO_KEY', 'CHARLIE_KEY', 'DELTA_KEY'] as $key) { + $page->click("button[title=\"{$key}\"]") + ->click('[data-environment-variable-delete-action] button:visible') + ->fill('[x-model="userConfirmationText"]:visible', $key) + ->click('button:has([x-text="step2ButtonText"]):visible') + ->assertSee('Environment variable deleted successfully.') + ->assertDontSee($key); + } + + $page->assertSee('ALPHA_KEY') + ->screenshot(filename: 'environment-variable-list-refresh'); + + expect($page->script('window.__envListErrors'))->toBe([]) + ->and($this->application->environment_variables()->pluck('key')->all())->toBe(['ALPHA_KEY']) + ->and($this->application->environment_variables()->where('key', 'ALPHA_KEY')->value('is_shown_once'))->toBeTruthy(); +});