From d640911bb94e7ee4353fbf5d116af5204258fa12 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 18 Nov 2025 22:26:07 +0100 Subject: [PATCH] fix: preserve existing comments in bulk update and always show save notification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes two UX issues with environment variable bulk updates: 1. Comment Preservation (High Priority Bug): - When bulk updating environment variables via Developer view, existing manually-entered comments are now preserved when no inline comment is provided - Only overwrites existing comments when an inline comment (#comment) is explicitly provided in the pasted content - Previously: pasting "KEY=value" would erase existing comment to null - Now: pasting "KEY=value" preserves existing comment, "KEY=value #new" overwrites it 2. Save Notification (UX Improvement): - "Save all Environment variables" button now always shows success notification - Previously: only showed notification when changes were detected - Now: provides feedback even when no changes were made - Consistent with other save operations in the codebase Changes: - Modified updateOrCreateVariables() to only update comment field when inline comment is provided (null check prevents overwriting existing comments) - Modified handleBulkSubmit() to always dispatch success notification unless error occurred - Added comprehensive test coverage for bulk update comment preservation scenarios Tests: - Added 4 new feature tests covering comment preservation edge cases - All 22 existing unit tests for parseEnvFormatToArray pass - Code formatted with Pint 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Shared/EnvironmentVariable/All.php | 10 +- .../EnvironmentVariableCommentTest.php | 133 ++++++++++++++++++ 2 files changed, 138 insertions(+), 5 deletions(-) diff --git a/app/Livewire/Project/Shared/EnvironmentVariable/All.php b/app/Livewire/Project/Shared/EnvironmentVariable/All.php index ded717b26..f867aaf3d 100644 --- a/app/Livewire/Project/Shared/EnvironmentVariable/All.php +++ b/app/Livewire/Project/Shared/EnvironmentVariable/All.php @@ -193,8 +193,8 @@ private function handleBulkSubmit() } } - // Only show success message if changes were actually made and no errors occurred - if ($changesMade && ! $errorOccurred) { + // Always show success message unless an error occurred + if (! $errorOccurred) { $this->dispatch('success', 'Environment variables updated.'); } } @@ -294,9 +294,9 @@ private function updateOrCreateVariables($isPreview, $variables) $changed = true; } - // Always update comment from inline comment (overwrites existing) - // Set to comment if provided, otherwise set to null if no comment - if ($found->comment !== $comment) { + // Only update comment from inline comment if one is provided (overwrites existing) + // If $comment is null, don't touch existing comment field to preserve it + if ($comment !== null && $found->comment !== $comment) { $found->comment = $comment; $changed = true; } diff --git a/tests/Feature/EnvironmentVariableCommentTest.php b/tests/Feature/EnvironmentVariableCommentTest.php index 5efb2b953..8b02ad8bf 100644 --- a/tests/Feature/EnvironmentVariableCommentTest.php +++ b/tests/Feature/EnvironmentVariableCommentTest.php @@ -148,3 +148,136 @@ ->call('submit') ->assertHasErrors(['comment' => 'max']); }); + +test('bulk update preserves existing comments when no inline comment provided', function () { + // Create existing variable with a manually-entered comment + $env = EnvironmentVariable::create([ + 'key' => 'DATABASE_URL', + 'value' => 'postgres://old-host', + 'comment' => 'Production database', + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + + // User switches to Developer view and pastes new value without inline comment + $bulkContent = "DATABASE_URL=postgres://new-host\nOTHER_VAR=value"; + + Livewire::test(\App\Livewire\Project\Shared\EnvironmentVariable\All::class, [ + 'resource' => $this->application, + 'type' => 'application', + ]) + ->set('variablesInput', $bulkContent) + ->call('saveVariables'); + + // Refresh the environment variable + $env->refresh(); + + // The value should be updated + expect($env->value)->toBe('postgres://new-host'); + + // The manually-entered comment should be PRESERVED + expect($env->comment)->toBe('Production database'); +}); + +test('bulk update overwrites existing comments when inline comment provided', function () { + // Create existing variable with a comment + $env = EnvironmentVariable::create([ + 'key' => 'API_KEY', + 'value' => 'old-key', + 'comment' => 'Old comment', + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + + // User pastes new value WITH inline comment + $bulkContent = 'API_KEY=new-key #Updated production key'; + + Livewire::test(\App\Livewire\Project\Shared\EnvironmentVariable\All::class, [ + 'resource' => $this->application, + 'type' => 'application', + ]) + ->set('variablesInput', $bulkContent) + ->call('saveVariables'); + + // Refresh the environment variable + $env->refresh(); + + // The value should be updated + expect($env->value)->toBe('new-key'); + + // The comment should be OVERWRITTEN with the inline comment + expect($env->comment)->toBe('Updated production key'); +}); + +test('bulk update handles mixed inline and stored comments correctly', function () { + // Create two variables with comments + $env1 = EnvironmentVariable::create([ + 'key' => 'VAR_WITH_COMMENT', + 'value' => 'value1', + 'comment' => 'Existing comment 1', + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + + $env2 = EnvironmentVariable::create([ + 'key' => 'VAR_WITHOUT_COMMENT', + 'value' => 'value2', + 'comment' => 'Existing comment 2', + 'resourceable_type' => Application::class, + 'resourceable_id' => $this->application->id, + ]); + + // Bulk paste: one with inline comment, one without + $bulkContent = "VAR_WITH_COMMENT=new_value1 #New inline comment\nVAR_WITHOUT_COMMENT=new_value2"; + + Livewire::test(\App\Livewire\Project\Shared\EnvironmentVariable\All::class, [ + 'resource' => $this->application, + 'type' => 'application', + ]) + ->set('variablesInput', $bulkContent) + ->call('saveVariables'); + + // Refresh both variables + $env1->refresh(); + $env2->refresh(); + + // First variable: comment should be overwritten with inline comment + expect($env1->value)->toBe('new_value1'); + expect($env1->comment)->toBe('New inline comment'); + + // Second variable: comment should be preserved + expect($env2->value)->toBe('new_value2'); + expect($env2->comment)->toBe('Existing comment 2'); +}); + +test('bulk update creates new variables with inline comments', function () { + // Bulk paste creates new variables, some with inline comments + $bulkContent = "NEW_VAR1=value1 #Comment for var1\nNEW_VAR2=value2\nNEW_VAR3=value3 #Comment for var3"; + + Livewire::test(\App\Livewire\Project\Shared\EnvironmentVariable\All::class, [ + 'resource' => $this->application, + 'type' => 'application', + ]) + ->set('variablesInput', $bulkContent) + ->call('saveVariables'); + + // Check that variables were created with correct comments + $var1 = EnvironmentVariable::where('key', 'NEW_VAR1') + ->where('resourceable_id', $this->application->id) + ->first(); + $var2 = EnvironmentVariable::where('key', 'NEW_VAR2') + ->where('resourceable_id', $this->application->id) + ->first(); + $var3 = EnvironmentVariable::where('key', 'NEW_VAR3') + ->where('resourceable_id', $this->application->id) + ->first(); + + expect($var1->value)->toBe('value1'); + expect($var1->comment)->toBe('Comment for var1'); + + expect($var2->value)->toBe('value2'); + expect($var2->comment)->toBeNull(); + + expect($var3->value)->toBe('value3'); + expect($var3->comment)->toBe('Comment for var3'); +});