fix: preserve existing comments in bulk update and always show save notification

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 <noreply@anthropic.com>
This commit is contained in:
Andras Bacsai 2025-11-18 22:26:07 +01:00
parent dfb180601a
commit d640911bb9
2 changed files with 138 additions and 5 deletions

View file

@ -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;
}

View file

@ -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');
});