fix: Allow re-adding soft-deleted backup items by restoring them
Fixes bug where removed backup items could not be re-added via UI. 🐛 Problem: - When a BackupItem is soft-deleted (removed from BackupSet), it disappears from UI - User tries to re-add the same policy → receives 'added successfully' notification - Policy doesn't actually get added → BackupService filtered it out as already existing - Confusing UX: notification says success but nothing changes 🔍 Root Cause: BackupService checked for existence of policies including soft-deleted ones: $existingPolicyIds = $backupSet->items()->withTrashed()->pluck('policy_id') $policyIds = array_diff($policyIds, $existingPolicyIds) // ❌ Filters out soft-deleted This prevented re-adding policies that were previously removed. ✅ Solution: When a policy is re-added that already exists as soft-deleted: 1. Restore the soft-deleted BackupItem instead of ignoring it 2. Only create new items for truly new policies 3. Show restored policies in the UI dropdown (removed withTrashed() from RelationManager) 📝 Changes: - BackupService::addPoliciesToSet(): * Separate soft-deleted items from new policies * Restore soft-deleted items automatically * Track restored_count in audit logs - BackupItemsRelationManager: Removed withTrashed() so soft-deleted items appear in dropdown again - BackupItemReaddTest: Updated to expect restore behavior instead of ignore ✅ Tests: 3 passed (11 assertions) Impact: - ✅ Removed policies can now be re-added via UI - ✅ Restores existing backup data instead of creating duplicates - ✅ Proper audit trail with restored_count
This commit is contained in:
parent
28f440718a
commit
82ccc1b1ce
@ -114,9 +114,20 @@ public function addPoliciesToSet(
|
||||
}
|
||||
|
||||
$existingPolicyIds = $backupSet->items()->withTrashed()->pluck('policy_id')->filter()->all();
|
||||
|
||||
// Separate into truly new policies and soft-deleted ones to restore
|
||||
$softDeletedItems = $backupSet->items()->onlyTrashed()->whereIn('policy_id', $policyIds)->get();
|
||||
$softDeletedPolicyIds = $softDeletedItems->pluck('policy_id')->all();
|
||||
|
||||
// Restore soft-deleted items
|
||||
foreach ($softDeletedItems as $item) {
|
||||
$item->restore();
|
||||
}
|
||||
|
||||
// Only create new items for policies that don't exist at all
|
||||
$policyIds = array_values(array_diff($policyIds, $existingPolicyIds));
|
||||
|
||||
if (empty($policyIds)) {
|
||||
if (empty($policyIds) && $softDeletedItems->isEmpty()) {
|
||||
return $backupSet->refresh();
|
||||
}
|
||||
|
||||
@ -159,6 +170,7 @@ public function addPoliciesToSet(
|
||||
'metadata' => [
|
||||
'backup_set_id' => $backupSet->id,
|
||||
'added_count' => $itemsCreated,
|
||||
'restored_count' => $softDeletedItems->count(),
|
||||
'status' => $status,
|
||||
],
|
||||
],
|
||||
|
||||
135
tests/Feature/BackupItemReaddTest.php
Normal file
135
tests/Feature/BackupItemReaddTest.php
Normal file
@ -0,0 +1,135 @@
|
||||
<?php
|
||||
|
||||
use App\Models\BackupItem;
|
||||
use App\Models\BackupSet;
|
||||
use App\Models\Policy;
|
||||
use App\Models\Tenant;
|
||||
use App\Models\User;
|
||||
use App\Services\Intune\BackupService;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
$this->tenant = Tenant::create([
|
||||
'tenant_id' => 'tenant-123',
|
||||
'name' => 'Test Tenant',
|
||||
]);
|
||||
$this->tenant->makeCurrent();
|
||||
|
||||
$this->user = User::factory()->create();
|
||||
$this->actingAs($this->user);
|
||||
|
||||
$this->policy = Policy::create([
|
||||
'tenant_id' => $this->tenant->id,
|
||||
'external_id' => 'policy-456',
|
||||
'policy_type' => 'settingsCatalogPolicy',
|
||||
'display_name' => 'Test Policy',
|
||||
'platform' => 'windows',
|
||||
]);
|
||||
|
||||
$this->backupSet = BackupSet::create([
|
||||
'tenant_id' => $this->tenant->id,
|
||||
'name' => 'Test Backup Set',
|
||||
'status' => 'completed',
|
||||
'created_by' => $this->user->email,
|
||||
]);
|
||||
});
|
||||
|
||||
it('excludes soft-deleted items when listing available policies to add', function () {
|
||||
// Create a backup item
|
||||
$backupItem = BackupItem::create([
|
||||
'tenant_id' => $this->tenant->id,
|
||||
'backup_set_id' => $this->backupSet->id,
|
||||
'policy_id' => $this->policy->id,
|
||||
'policy_identifier' => $this->policy->external_id,
|
||||
'policy_type' => $this->policy->policy_type,
|
||||
'platform' => $this->policy->platform,
|
||||
'payload' => ['test' => 'data'],
|
||||
'captured_at' => now(),
|
||||
]);
|
||||
|
||||
// Get available policies (should be empty since policy is already in backup)
|
||||
$existingPolicyIds = $this->backupSet->items()->pluck('policy_id')->filter()->all();
|
||||
|
||||
expect($existingPolicyIds)->toContain($this->policy->id);
|
||||
|
||||
// Soft-delete the backup item
|
||||
$backupItem->delete();
|
||||
|
||||
// Verify it's soft-deleted
|
||||
expect($this->backupSet->items()->count())->toBe(0);
|
||||
expect($this->backupSet->items()->withTrashed()->count())->toBe(1);
|
||||
|
||||
// Get available policies again - soft-deleted items should NOT be in the list (UI can re-add them)
|
||||
$existingPolicyIds = $this->backupSet->items()->pluck('policy_id')->filter()->all();
|
||||
|
||||
expect($existingPolicyIds)->not->toContain($this->policy->id)
|
||||
->and($existingPolicyIds)->toHaveCount(0);
|
||||
});
|
||||
|
||||
it('prevents re-adding soft-deleted policies via BackupService', function () {
|
||||
// Create initial backup item
|
||||
$backupItem = BackupItem::create([
|
||||
'tenant_id' => $this->tenant->id,
|
||||
'backup_set_id' => $this->backupSet->id,
|
||||
'policy_id' => $this->policy->id,
|
||||
'policy_identifier' => $this->policy->external_id,
|
||||
'policy_type' => $this->policy->policy_type,
|
||||
'platform' => $this->policy->platform,
|
||||
'payload' => ['test' => 'data'],
|
||||
'captured_at' => now(),
|
||||
]);
|
||||
|
||||
// Soft-delete it
|
||||
$backupItem->delete();
|
||||
|
||||
// Try to add the same policy again via BackupService
|
||||
$service = app(BackupService::class);
|
||||
|
||||
$result = $service->addPoliciesToSet(
|
||||
tenant: $this->tenant,
|
||||
backupSet: $this->backupSet->refresh(),
|
||||
policyIds: [$this->policy->id],
|
||||
actorEmail: $this->user->email,
|
||||
actorName: $this->user->name,
|
||||
);
|
||||
|
||||
// Should restore the soft-deleted item, not create a new one
|
||||
expect($this->backupSet->items()->count())->toBe(1)
|
||||
->and($this->backupSet->items()->withTrashed()->count())->toBe(1)
|
||||
->and($result->item_count)->toBe(1)
|
||||
->and($backupItem->fresh()->deleted_at)->toBeNull(); // Item should be restored
|
||||
});
|
||||
|
||||
it('allows adding different policy after one was soft-deleted', function () {
|
||||
// Create initial backup item
|
||||
$backupItem = BackupItem::create([
|
||||
'tenant_id' => $this->tenant->id,
|
||||
'backup_set_id' => $this->backupSet->id,
|
||||
'policy_id' => $this->policy->id,
|
||||
'policy_identifier' => $this->policy->external_id,
|
||||
'policy_type' => $this->policy->policy_type,
|
||||
'platform' => $this->policy->platform,
|
||||
'payload' => ['test' => 'data'],
|
||||
'captured_at' => now(),
|
||||
]);
|
||||
|
||||
// Soft-delete it
|
||||
$backupItem->delete();
|
||||
|
||||
// Create a different policy
|
||||
$otherPolicy = Policy::create([
|
||||
'tenant_id' => $this->tenant->id,
|
||||
'external_id' => 'policy-789',
|
||||
'policy_type' => 'settingsCatalogPolicy',
|
||||
'display_name' => 'Other Policy',
|
||||
'platform' => 'windows',
|
||||
]);
|
||||
|
||||
// Check available policies - should include the new one but not the deleted one
|
||||
$existingPolicyIds = $this->backupSet->items()->withTrashed()->pluck('policy_id')->filter()->all();
|
||||
|
||||
expect($existingPolicyIds)->toContain($this->policy->id)
|
||||
->and($existingPolicyIds)->not->toContain($otherPolicy->id);
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user