diff --git a/app/Jobs/AddPoliciesToBackupSetJob.php b/app/Jobs/AddPoliciesToBackupSetJob.php index 04cb3b2..37d3eea 100644 --- a/app/Jobs/AddPoliciesToBackupSetJob.php +++ b/app/Jobs/AddPoliciesToBackupSetJob.php @@ -44,284 +44,116 @@ public function handle( ): void { $run = BulkOperationRun::with(['tenant', 'user'])->find($this->bulkRunId); - if (! $run || $run->status !== 'pending') { + if (! $run) { return; } - $bulkOperationService->start($run); + $started = BulkOperationRun::query() + ->whereKey($run->getKey()) + ->where('status', 'pending') + ->update(['status' => 'running']); + + if ($started === 0) { + return; + } + + $run->refresh(); $tenant = $run->tenant ?? Tenant::query()->find($run->tenant_id); - if (! $tenant instanceof Tenant) { - $this->appendRunFailure($run, [ - 'type' => 'run', - 'item_id' => (string) $this->backupSetId, - 'reason_code' => 'backup_set_not_found', - 'reason' => $bulkOperationService->sanitizeFailureReason('Tenant not found for run.'), - ]); - - $bulkOperationService->fail($run, 'Tenant not found for run.'); - - return; - } - - $backupSet = BackupSet::withTrashed() - ->where('tenant_id', $tenant->getKey()) - ->whereKey($this->backupSetId) - ->first(); - - if (! $backupSet) { - $this->appendRunFailure($run, [ - 'type' => 'run', - 'item_id' => (string) $this->backupSetId, - 'reason_code' => 'backup_set_not_found', - 'reason' => $bulkOperationService->sanitizeFailureReason('Backup set not found.'), - ]); - - $bulkOperationService->fail($run, 'Backup set not found.'); - - return; - } - - if ($backupSet->trashed()) { - $this->appendRunFailure($run, [ - 'type' => 'run', - 'item_id' => (string) $backupSet->getKey(), - 'reason_code' => 'backup_set_archived', - 'reason' => $bulkOperationService->sanitizeFailureReason('Backup set is archived.'), - ]); - - $bulkOperationService->fail($run, 'Backup set is archived.'); - - return; - } - - $policyIds = $this->extractPolicyIds($run); - - if ($policyIds === []) { - $bulkOperationService->complete($run); - - return; - } - - if ((int) $run->total_items !== count($policyIds)) { - $run->update(['total_items' => count($policyIds)]); - } - - $existingBackupFailures = (array) Arr::get($backupSet->metadata ?? [], 'failures', []); - $newBackupFailures = []; - - $didMutateBackupSet = false; - $backupSetItemMutations = 0; - $foundationMutations = 0; - $foundationFailures = 0; - - /** @var array $activePolicyIds */ - $activePolicyIds = BackupItem::query() - ->where('backup_set_id', $backupSet->getKey()) - ->whereIn('policy_id', $policyIds) - ->pluck('policy_id') - ->filter() - ->map(fn (mixed $value): int => (int) $value) - ->values() - ->all(); - - $activePolicyIdSet = array_fill_keys($activePolicyIds, true); - - /** @var EloquentCollection $trashedItems */ - $trashedItems = BackupItem::onlyTrashed() - ->where('backup_set_id', $backupSet->getKey()) - ->whereIn('policy_id', $policyIds) - ->get() - ->keyBy('policy_id'); - - /** @var EloquentCollection $policies */ - $policies = Policy::query() - ->where('tenant_id', $tenant->getKey()) - ->whereIn('id', $policyIds) - ->get() - ->keyBy('id'); - - foreach ($policyIds as $policyId) { - if (isset($activePolicyIdSet[$policyId])) { - $bulkOperationService->recordSkippedWithReason( + try { + if (! $tenant instanceof Tenant) { + $this->markRunFailed( + bulkOperationService: $bulkOperationService, run: $run, - itemId: (string) $policyId, - reason: 'Already in backup set', - reasonCode: 'already_in_backup_set', + tenant: null, + itemId: (string) $this->backupSetId, + reasonCode: 'unknown', + reason: 'Tenant not found for run.', ); - continue; + return; } - $trashed = $trashedItems->get($policyId); + $backupSet = BackupSet::withTrashed() + ->where('tenant_id', $tenant->getKey()) + ->whereKey($this->backupSetId) + ->first(); - if ($trashed instanceof BackupItem) { - $trashed->restore(); - - $activePolicyIdSet[$policyId] = true; - $didMutateBackupSet = true; - $backupSetItemMutations++; - - $bulkOperationService->recordSuccess($run); - - continue; - } - - $policy = $policies->get($policyId); - - if (! $policy instanceof Policy) { - $newBackupFailures[] = [ - 'policy_id' => $policyId, - 'reason' => $bulkOperationService->sanitizeFailureReason('Policy not found.'), - 'status' => null, - 'reason_code' => 'policy_not_found', - ]; - $didMutateBackupSet = true; - - $bulkOperationService->recordFailure( + if (! $backupSet) { + $this->markRunFailed( + bulkOperationService: $bulkOperationService, run: $run, - itemId: (string) $policyId, - reason: 'Policy not found.', - reasonCode: 'policy_not_found', - ); - - continue; - } - - if ($policy->ignored_at) { - $bulkOperationService->recordSkippedWithReason( - run: $run, - itemId: (string) $policyId, - reason: 'Policy is ignored locally', - reasonCode: 'policy_ignored', - ); - - continue; - } - - try { - $captureResult = $captureOrchestrator->capture( - policy: $policy, tenant: $tenant, - includeAssignments: $this->includeAssignments, - includeScopeTags: $this->includeScopeTags, - createdBy: $run->user?->email ? Str::limit($run->user->email, 255, '') : null, - metadata: [ - 'source' => 'backup', - 'backup_set_id' => $backupSet->getKey(), - ], + itemId: (string) $this->backupSetId, + reasonCode: 'backup_set_not_found', + reason: 'Backup set not found.', ); - } catch (Throwable $throwable) { - $reason = $bulkOperationService->sanitizeFailureReason($throwable->getMessage()); - $newBackupFailures[] = [ - 'policy_id' => $policyId, - 'reason' => $reason, - 'status' => null, - 'reason_code' => 'unknown', - ]; - $didMutateBackupSet = true; + return; + } - $bulkOperationService->recordFailure( + if ($backupSet->trashed()) { + $this->markRunFailed( + bulkOperationService: $bulkOperationService, run: $run, - itemId: (string) $policyId, - reason: $reason, - reasonCode: 'unknown', + tenant: $tenant, + itemId: (string) $backupSet->getKey(), + reasonCode: 'backup_set_archived', + reason: 'Backup set is archived.', ); - continue; + return; } - if (isset($captureResult['failure']) && is_array($captureResult['failure'])) { - $failure = $captureResult['failure']; - $status = isset($failure['status']) && is_numeric($failure['status']) ? (int) $failure['status'] : null; - $reasonCode = $this->mapGraphFailureReasonCode($status); - $reason = $bulkOperationService->sanitizeFailureReason((string) ($failure['reason'] ?? 'Graph capture failed.')); + $policyIds = $this->extractPolicyIds($run); - $newBackupFailures[] = [ - 'policy_id' => $policyId, - 'reason' => $reason, - 'status' => $status, - 'reason_code' => $reasonCode, - ]; - $didMutateBackupSet = true; + if ($policyIds === []) { + $bulkOperationService->complete($run); - $bulkOperationService->recordFailure( - run: $run, - itemId: (string) $policyId, - reason: $reason, - reasonCode: $reasonCode, - ); - - continue; + return; } - $version = $captureResult['version'] ?? null; - $captured = $captureResult['captured'] ?? null; - - if (! $version || ! is_array($captured)) { - $newBackupFailures[] = [ - 'policy_id' => $policyId, - 'reason' => $bulkOperationService->sanitizeFailureReason('Capture result missing version payload.'), - 'status' => null, - 'reason_code' => 'unknown', - ]; - $didMutateBackupSet = true; - - $bulkOperationService->recordFailure( - run: $run, - itemId: (string) $policyId, - reason: 'Capture result missing version payload.', - reasonCode: 'unknown', - ); - - continue; + if ((int) $run->total_items !== count($policyIds)) { + $run->update(['total_items' => count($policyIds)]); } - $payload = $captured['payload'] ?? []; - $metadata = is_array($captured['metadata'] ?? null) ? $captured['metadata'] : []; - $assignments = is_array($captured['assignments'] ?? null) ? $captured['assignments'] : null; - $scopeTags = is_array($captured['scope_tags'] ?? null) ? $captured['scope_tags'] : null; + $existingBackupFailures = (array) Arr::get($backupSet->metadata ?? [], 'failures', []); + $newBackupFailures = []; - if (! is_array($payload)) { - $payload = []; - } + $didMutateBackupSet = false; + $backupSetItemMutations = 0; + $foundationMutations = 0; + $foundationFailures = 0; - $validation = $snapshotValidator->validate($payload); - $warnings = $validation['warnings'] ?? []; + /** @var array $activePolicyIds */ + $activePolicyIds = BackupItem::query() + ->where('backup_set_id', $backupSet->getKey()) + ->whereIn('policy_id', $policyIds) + ->pluck('policy_id') + ->filter() + ->map(fn (mixed $value): int => (int) $value) + ->values() + ->all(); - $odataWarning = BackupItem::odataTypeWarning($payload, $policy->policy_type, $policy->platform); + $activePolicyIdSet = array_fill_keys($activePolicyIds, true); - if ($odataWarning) { - $warnings[] = $odataWarning; - } + /** @var EloquentCollection $trashedItems */ + $trashedItems = BackupItem::onlyTrashed() + ->where('backup_set_id', $backupSet->getKey()) + ->whereIn('policy_id', $policyIds) + ->get() + ->keyBy('policy_id'); - if (! empty($warnings)) { - $existingWarnings = is_array($metadata['warnings'] ?? null) ? $metadata['warnings'] : []; - $metadata['warnings'] = array_values(array_unique(array_merge($existingWarnings, $warnings))); - } + /** @var EloquentCollection $policies */ + $policies = Policy::query() + ->where('tenant_id', $tenant->getKey()) + ->whereIn('id', $policyIds) + ->get() + ->keyBy('id'); - if (is_array($scopeTags)) { - $metadata['scope_tag_ids'] = $scopeTags['ids'] ?? null; - $metadata['scope_tag_names'] = $scopeTags['names'] ?? null; - } - - try { - BackupItem::create([ - 'tenant_id' => $tenant->getKey(), - 'backup_set_id' => $backupSet->getKey(), - 'policy_id' => $policy->getKey(), - 'policy_version_id' => $version->getKey(), - 'policy_identifier' => $policy->external_id, - 'policy_type' => $policy->policy_type, - 'platform' => $policy->platform, - 'payload' => $payload, - 'metadata' => $metadata, - 'assignments' => $assignments, - ]); - } catch (QueryException $exception) { - if ((string) $exception->getCode() === '23505') { + foreach ($policyIds as $policyId) { + if (isset($activePolicyIdSet[$policyId])) { $bulkOperationService->recordSkippedWithReason( run: $run, itemId: (string) $policyId, @@ -332,106 +164,304 @@ public function handle( continue; } - throw $exception; - } + $trashed = $trashedItems->get($policyId); - $activePolicyIdSet[$policyId] = true; - $didMutateBackupSet = true; - $backupSetItemMutations++; + if ($trashed instanceof BackupItem) { + $trashed->restore(); - $bulkOperationService->recordSuccess($run); - } + $activePolicyIdSet[$policyId] = true; + $didMutateBackupSet = true; + $backupSetItemMutations++; - if ($this->includeFoundations) { - [$foundationOutcome, $foundationFailureEntries] = $this->captureFoundations( - bulkOperationService: $bulkOperationService, - foundationSnapshots: $foundationSnapshots, - tenant: $tenant, - backupSet: $backupSet, - ); + $bulkOperationService->recordSuccess($run); - if (($foundationOutcome['created'] ?? 0) > 0 || ($foundationOutcome['restored'] ?? 0) > 0) { - $didMutateBackupSet = true; - $foundationMutations = (int) $foundationOutcome['created'] + (int) $foundationOutcome['restored']; - } + continue; + } - if ($foundationFailureEntries !== []) { - $didMutateBackupSet = true; - $foundationFailures = count($foundationFailureEntries); - $newBackupFailures = array_merge($newBackupFailures, $foundationFailureEntries); + $policy = $policies->get($policyId); - foreach ($foundationFailureEntries as $foundationFailure) { - $this->appendRunFailure($run, [ - 'type' => 'foundation', - 'item_id' => (string) ($foundationFailure['foundation_type'] ?? 'foundation'), - 'reason_code' => (string) ($foundationFailure['reason_code'] ?? 'unknown'), - 'reason' => (string) ($foundationFailure['reason'] ?? 'Foundation capture failed.'), - 'status' => $foundationFailure['status'] ?? null, + if (! $policy instanceof Policy) { + $newBackupFailures[] = [ + 'policy_id' => $policyId, + 'reason' => $bulkOperationService->sanitizeFailureReason('Policy not found.'), + 'status' => null, + 'reason_code' => 'policy_not_found', + ]; + $didMutateBackupSet = true; + + $bulkOperationService->recordFailure( + run: $run, + itemId: (string) $policyId, + reason: 'Policy not found.', + reasonCode: 'policy_not_found', + ); + + continue; + } + + if ($policy->ignored_at) { + $bulkOperationService->recordSkippedWithReason( + run: $run, + itemId: (string) $policyId, + reason: 'Policy is ignored locally', + reasonCode: 'policy_ignored', + ); + + continue; + } + + try { + $captureResult = $captureOrchestrator->capture( + policy: $policy, + tenant: $tenant, + includeAssignments: $this->includeAssignments, + includeScopeTags: $this->includeScopeTags, + createdBy: $run->user?->email ? Str::limit($run->user->email, 255, '') : null, + metadata: [ + 'source' => 'backup', + 'backup_set_id' => $backupSet->getKey(), + ], + ); + } catch (Throwable $throwable) { + $reason = $bulkOperationService->sanitizeFailureReason($throwable->getMessage()); + + $newBackupFailures[] = [ + 'policy_id' => $policyId, + 'reason' => $reason, + 'status' => null, + 'reason_code' => 'unknown', + ]; + $didMutateBackupSet = true; + + $bulkOperationService->recordFailure( + run: $run, + itemId: (string) $policyId, + reason: $reason, + reasonCode: 'unknown', + ); + + continue; + } + + if (isset($captureResult['failure']) && is_array($captureResult['failure'])) { + $failure = $captureResult['failure']; + $status = isset($failure['status']) && is_numeric($failure['status']) ? (int) $failure['status'] : null; + $reasonCode = $this->mapGraphFailureReasonCode($status); + $reason = $bulkOperationService->sanitizeFailureReason((string) ($failure['reason'] ?? 'Graph capture failed.')); + + $newBackupFailures[] = [ + 'policy_id' => $policyId, + 'reason' => $reason, + 'status' => $status, + 'reason_code' => $reasonCode, + ]; + $didMutateBackupSet = true; + + $bulkOperationService->recordFailure( + run: $run, + itemId: (string) $policyId, + reason: $reason, + reasonCode: $reasonCode, + ); + + continue; + } + + $version = $captureResult['version'] ?? null; + $captured = $captureResult['captured'] ?? null; + + if (! $version || ! is_array($captured)) { + $newBackupFailures[] = [ + 'policy_id' => $policyId, + 'reason' => $bulkOperationService->sanitizeFailureReason('Capture result missing version payload.'), + 'status' => null, + 'reason_code' => 'unknown', + ]; + $didMutateBackupSet = true; + + $bulkOperationService->recordFailure( + run: $run, + itemId: (string) $policyId, + reason: 'Capture result missing version payload.', + reasonCode: 'unknown', + ); + + continue; + } + + $payload = $captured['payload'] ?? []; + $metadata = is_array($captured['metadata'] ?? null) ? $captured['metadata'] : []; + $assignments = is_array($captured['assignments'] ?? null) ? $captured['assignments'] : null; + $scopeTags = is_array($captured['scope_tags'] ?? null) ? $captured['scope_tags'] : null; + + if (! is_array($payload)) { + $payload = []; + } + + $validation = $snapshotValidator->validate($payload); + $warnings = $validation['warnings'] ?? []; + + $odataWarning = BackupItem::odataTypeWarning($payload, $policy->policy_type, $policy->platform); + + if ($odataWarning) { + $warnings[] = $odataWarning; + } + + if (! empty($warnings)) { + $existingWarnings = is_array($metadata['warnings'] ?? null) ? $metadata['warnings'] : []; + $metadata['warnings'] = array_values(array_unique(array_merge($existingWarnings, $warnings))); + } + + if (is_array($scopeTags)) { + $metadata['scope_tag_ids'] = $scopeTags['ids'] ?? null; + $metadata['scope_tag_names'] = $scopeTags['names'] ?? null; + } + + try { + BackupItem::create([ + 'tenant_id' => $tenant->getKey(), + 'backup_set_id' => $backupSet->getKey(), + 'policy_id' => $policy->getKey(), + 'policy_version_id' => $version->getKey(), + 'policy_identifier' => $policy->external_id, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'payload' => $payload, + 'metadata' => $metadata, + 'assignments' => $assignments, ]); + } catch (QueryException $exception) { + if ((string) $exception->getCode() === '23505') { + $bulkOperationService->recordSkippedWithReason( + run: $run, + itemId: (string) $policyId, + reason: 'Already in backup set', + reasonCode: 'already_in_backup_set', + ); + + continue; + } + + throw $exception; + } + + $activePolicyIdSet[$policyId] = true; + $didMutateBackupSet = true; + $backupSetItemMutations++; + + $bulkOperationService->recordSuccess($run); + } + + if ($this->includeFoundations) { + [$foundationOutcome, $foundationFailureEntries] = $this->captureFoundations( + bulkOperationService: $bulkOperationService, + foundationSnapshots: $foundationSnapshots, + tenant: $tenant, + backupSet: $backupSet, + ); + + if (($foundationOutcome['created'] ?? 0) > 0 || ($foundationOutcome['restored'] ?? 0) > 0) { + $didMutateBackupSet = true; + $foundationMutations = (int) $foundationOutcome['created'] + (int) $foundationOutcome['restored']; + } + + if ($foundationFailureEntries !== []) { + $didMutateBackupSet = true; + $foundationFailures = count($foundationFailureEntries); + $newBackupFailures = array_merge($newBackupFailures, $foundationFailureEntries); + + foreach ($foundationFailureEntries as $foundationFailure) { + $this->appendRunFailure($run, [ + 'type' => 'foundation', + 'item_id' => (string) ($foundationFailure['foundation_type'] ?? 'foundation'), + 'reason_code' => (string) ($foundationFailure['reason_code'] ?? 'unknown'), + 'reason' => (string) ($foundationFailure['reason'] ?? 'Foundation capture failed.'), + 'status' => $foundationFailure['status'] ?? null, + ]); + } } } - } - if ($didMutateBackupSet) { - $allFailures = array_merge($existingBackupFailures, $newBackupFailures); - $mutations = $backupSetItemMutations + $foundationMutations; + if ($didMutateBackupSet) { + $allFailures = array_merge($existingBackupFailures, $newBackupFailures); + $mutations = $backupSetItemMutations + $foundationMutations; - $backupSetStatus = match (true) { - $mutations === 0 && count($allFailures) > 0 => 'failed', - count($allFailures) > 0 => 'partial', - default => 'completed', - }; + $backupSetStatus = match (true) { + $mutations === 0 && count($allFailures) > 0 => 'failed', + count($allFailures) > 0 => 'partial', + default => 'completed', + }; - $backupSet->update([ - 'status' => $backupSetStatus, - 'item_count' => $backupSet->items()->count(), - 'completed_at' => now(), - 'metadata' => ['failures' => $allFailures], - ]); - } - - $bulkOperationService->complete($run); - - if (! $run->user) { - return; - } - - $message = "Added {$run->succeeded} policies"; - if ($run->skipped > 0) { - $message .= " ({$run->skipped} skipped)"; - } - if ($run->failed > 0) { - $message .= " ({$run->failed} failed)"; - } - - if ($this->includeFoundations) { - $message .= ". Foundations: {$foundationMutations} items"; - - if ($foundationFailures > 0) { - $message .= " ({$foundationFailures} failed)"; + $backupSet->update([ + 'status' => $backupSetStatus, + 'item_count' => $backupSet->items()->count(), + 'completed_at' => now(), + 'metadata' => ['failures' => $allFailures], + ]); } + + $bulkOperationService->complete($run); + + if (! $run->user) { + return; + } + + $message = "Added {$run->succeeded} policies"; + if ($run->skipped > 0) { + $message .= " ({$run->skipped} skipped)"; + } + if ($run->failed > 0) { + $message .= " ({$run->failed} failed)"; + } + + if ($this->includeFoundations) { + $message .= ". Foundations: {$foundationMutations} items"; + + if ($foundationFailures > 0) { + $message .= " ({$foundationFailures} failed)"; + } + } + + $message .= '.'; + + $partial = $run->status === 'completed_with_errors' || $foundationFailures > 0; + + $notification = Notification::make() + ->title($partial ? 'Add Policies Completed (partial)' : 'Add Policies Completed') + ->body($message) + ->actions([ + \Filament\Actions\Action::make('view_run') + ->label('View run') + ->url(BulkOperationRunResource::getUrl('view', ['record' => $run], tenant: $tenant)), + ]); + + if ($partial) { + $notification->warning(); + } else { + $notification->success(); + } + + $notification + ->sendToDatabase($run->user) + ->send(); + } catch (Throwable $throwable) { + $run->refresh(); + + if (in_array($run->status, ['completed', 'completed_with_errors'], true)) { + throw $throwable; + } + + $this->markRunFailed( + bulkOperationService: $bulkOperationService, + run: $run, + tenant: $tenant instanceof Tenant ? $tenant : null, + itemId: (string) $this->backupSetId, + reasonCode: 'unknown', + reason: $throwable->getMessage(), + ); + + throw $throwable; } - - $message .= '.'; - - $notification = Notification::make() - ->title($run->failed > 0 ? 'Add Policies Completed (partial)' : 'Add Policies Completed') - ->body($message) - ->actions([ - \Filament\Actions\Action::make('view_run') - ->label('View run') - ->url(BulkOperationRunResource::getUrl('view', ['record' => $run], tenant: $tenant)), - ]); - - if ($run->failed > 0) { - $notification->warning(); - } else { - $notification->success(); - } - - $notification - ->sendToDatabase($run->user) - ->send(); } /** @@ -470,6 +500,56 @@ private function appendRunFailure(BulkOperationRun $run, array $entry): void $run->update(['failures' => $failures]); } + private function markRunFailed( + BulkOperationService $bulkOperationService, + BulkOperationRun $run, + ?Tenant $tenant, + string $itemId, + string $reasonCode, + string $reason, + ): void { + $reason = $bulkOperationService->sanitizeFailureReason($reason); + + $this->appendRunFailure($run, [ + 'type' => 'run', + 'item_id' => $itemId, + 'reason_code' => $reasonCode, + 'reason' => $reason, + ]); + + try { + $bulkOperationService->fail($run, $reason); + } catch (Throwable) { + $run->update(['status' => 'failed']); + } + + $this->notifyRunFailed($run, $tenant, $reason); + } + + private function notifyRunFailed(BulkOperationRun $run, ?Tenant $tenant, string $reason): void + { + if (! $run->user) { + return; + } + + $notification = Notification::make() + ->title('Add Policies Failed') + ->body($reason); + + if ($tenant instanceof Tenant) { + $notification->actions([ + \Filament\Actions\Action::make('view_run') + ->label('View run') + ->url(BulkOperationRunResource::getUrl('view', ['record' => $run], tenant: $tenant)), + ]); + } + + $notification + ->danger() + ->sendToDatabase($run->user) + ->send(); + } + private function mapGraphFailureReasonCode(?int $status): string { return match (true) { diff --git a/app/Services/BulkOperationService.php b/app/Services/BulkOperationService.php index 8c4992d..95f9804 100644 --- a/app/Services/BulkOperationService.php +++ b/app/Services/BulkOperationService.php @@ -171,7 +171,11 @@ public function complete(BulkOperationRun $run): void return; } - $status = $run->failed > 0 ? 'completed_with_errors' : 'completed'; + $failureEntries = collect($run->failures ?? []); + $hasFailures = $run->failed > 0 + || $failureEntries->contains(fn (array $entry): bool => ($entry['type'] ?? 'failed') !== 'skipped'); + + $status = $hasFailures ? 'completed_with_errors' : 'completed'; $updated = BulkOperationRun::query() ->whereKey($run->id) diff --git a/specs/052-async-add-policies/tasks.md b/specs/052-async-add-policies/tasks.md index d80bfa4..c498d5b 100644 --- a/specs/052-async-add-policies/tasks.md +++ b/specs/052-async-add-policies/tasks.md @@ -29,8 +29,8 @@ ## Path Conventions (Laravel) ## Phase 1: Setup (Shared Infrastructure) -- [ ] T001 [P] Confirm existing run infra supports this operation (BulkOperationRun statuses + unique idempotency index) and document mapping queued↔pending, partial↔completed_with_errors in specs/052-async-add-policies/plan.md -- [ ] T002 [P] Decide and standardize taxonomy strings: operationType (`backup_set.add_policies`), resource (`backup_set`), action (`add_policies`) in specs/052-async-add-policies/spec.md and plan.md +- [x] T001 [P] Confirm existing run infra supports this operation (BulkOperationRun statuses + unique idempotency index) and document mapping queued↔pending, partial↔completed_with_errors in specs/052-async-add-policies/plan.md +- [x] T002 [P] Decide and standardize taxonomy strings: operationType (`backup_set.add_policies`), resource (`backup_set`), action (`add_policies`) in specs/052-async-add-policies/spec.md and plan.md --- @@ -42,15 +42,15 @@ ## Phase 2: User Story 1 - Add selected policies without blocking (Priority: P1) ### Tests (write first) ⚠️ -- [ ] T010 [P] [US1] Update/replace sync-path assertions in tests/Feature/Filament/BackupSetPolicyPickerTableTest.php to assert job dispatch + run creation -- [ ] T011 [P] [US1] Add fail-hard guard test to ensure no Graph calls occur during the bulk action (mock `App\\Services\\Graph\\GraphClientInterface` and assert `->never()`) +- [x] T010 [P] [US1] Update/replace sync-path assertions in tests/Feature/Filament/BackupSetPolicyPickerTableTest.php to assert job dispatch + run creation +- [x] T011 [P] [US1] Add fail-hard guard test to ensure no Graph calls occur during the bulk action (mock `App\\Services\\Graph\\GraphClientInterface` and assert `->never()`) ### Implementation -- [ ] T020 [US1] Create queued job to add policies to a backup set in app/Jobs/AddPoliciesToBackupSetJob.php (uses run lifecycle + safe failures via BulkOperationService) -- [ ] T021 [US1] Update bulk action handler in app/Livewire/BackupSetPolicyPickerTable.php to create/reuse BulkOperationRun and dispatch AddPoliciesToBackupSetJob (no inline snapshot capture) -- [ ] T022 [US1] Emit “queued” notification with “View run” link on submit (Filament DB notification preferred) from app/Livewire/BackupSetPolicyPickerTable.php -- [ ] T023 [US1] Emit completion/failure DB notification from app/Jobs/AddPoliciesToBackupSetJob.php with a safe summary +- [x] T020 [US1] Create queued job to add policies to a backup set in app/Jobs/AddPoliciesToBackupSetJob.php (uses run lifecycle + safe failures via BulkOperationService) +- [x] T021 [US1] Update bulk action handler in app/Livewire/BackupSetPolicyPickerTable.php to create/reuse BulkOperationRun and dispatch AddPoliciesToBackupSetJob (no inline snapshot capture) +- [x] T022 [US1] Emit “queued” notification with “View run” link on submit (Filament DB notification preferred) from app/Livewire/BackupSetPolicyPickerTable.php +- [x] T023 [US1] Emit completion/failure DB notification from app/Jobs/AddPoliciesToBackupSetJob.php with a safe summary **Checkpoint**: US1 complete — submit is non-blocking, job executes, run is visible. @@ -64,13 +64,13 @@ ## Phase 3: User Story 2 - Double click / repeated submissions are deduplicated ### Tests ⚠️ -- [ ] T030 [P] [US2] Add idempotency test in tests/Feature/BackupSets/BackupSetAddPoliciesIdempotencyTest.php (or extend existing picker test) asserting one run + one queued job +- [x] T030 [P] [US2] Add idempotency test in tests/Feature/BackupSets/BackupSetAddPoliciesIdempotencyTest.php (or extend existing picker test) asserting one run + one queued job ### Implementation -- [ ] T031 [US2] Implement deterministic selection hashing and idempotency key creation using app/Support/RunIdempotency.php (sorted policy ids + option flags), and reuse active runs via findActiveBulkOperationRun -- [ ] T032 [US2] Handle race conditions safely (unique index collisions) by recovering the existing run rather than failing the request -- [ ] T033 [US2] Ensure the action is always async (no `dispatchSync` path for small selections) in app/Livewire/BackupSetPolicyPickerTable.php +- [x] T031 [US2] Implement deterministic selection hashing and idempotency key creation using app/Support/RunIdempotency.php (sorted policy ids + option flags), and reuse active runs via findActiveBulkOperationRun +- [x] T032 [US2] Handle race conditions safely (unique index collisions) by recovering the existing run rather than failing the request +- [x] T033 [US2] Ensure the action is always async (no `dispatchSync` path for small selections) in app/Livewire/BackupSetPolicyPickerTable.php **Checkpoint**: US2 complete — double clicks are safe and deduped. @@ -84,15 +84,15 @@ ## Phase 4: User Story 3 - Failures are visible and safe (Priority: P3) ### Tests ⚠️ -- [ ] T040 [P] [US3] Add tenant isolation test for the created run (403 cross-tenant) in tests/Feature/BackupSets/BackupSetAddPoliciesTenantIsolationTest.php (or extend tests/Feature/RunAuthorizationTenantIsolationTest.php) -- [ ] T041 [P] [US3] Add sanitization test: failure reason containing token-like content is stored as redacted (exercise BulkOperationService::sanitizeFailureReason) +- [x] T040 [P] [US3] Add tenant isolation test for the created run (403 cross-tenant) in tests/Feature/BackupSets/BackupSetAddPoliciesTenantIsolationTest.php (or extend tests/Feature/RunAuthorizationTenantIsolationTest.php) +- [x] T041 [P] [US3] Add sanitization test: failure reason containing token-like content is stored as redacted (exercise BulkOperationService::sanitizeFailureReason) ### Implementation -- [ ] T042 [US3] Ensure job records per-item failures with sanitized reasons and does not store raw Graph payloads in run failures or notifications (app/Jobs/AddPoliciesToBackupSetJob.php) -- [ ] T043 [US3] Record stable failure `reason_code` values (per spec.md) alongside sanitized short text in run failures (app/Jobs/AddPoliciesToBackupSetJob.php and/or app/Services/BulkOperationService.php) -- [ ] T044 [US3] Record “already in backup set” as `skipped` (with reason_code `already_in_backup_set`) and ensure counts match spec.md (app/Jobs/AddPoliciesToBackupSetJob.php) -- [ ] T045 [US3] Ensure job processes all items (no circuit breaker abort) and run status reflects partial completion (app/Jobs/AddPoliciesToBackupSetJob.php) +- [x] T042 [US3] Ensure job records per-item failures with sanitized reasons and does not store raw Graph payloads in run failures or notifications (app/Jobs/AddPoliciesToBackupSetJob.php) +- [x] T043 [US3] Record stable failure `reason_code` values (per spec.md) alongside sanitized short text in run failures (app/Jobs/AddPoliciesToBackupSetJob.php and/or app/Services/BulkOperationService.php) +- [x] T044 [US3] Record “already in backup set” as `skipped` (with reason_code `already_in_backup_set`) and ensure counts match spec.md (app/Jobs/AddPoliciesToBackupSetJob.php) +- [x] T045 [US3] Ensure job processes all items (no circuit breaker abort) and run status reflects partial completion (app/Jobs/AddPoliciesToBackupSetJob.php) **Checkpoint**: US3 complete — failures are safe and observable; tenant isolation holds. @@ -100,5 +100,5 @@ ### Implementation ## Phase 5: Polish & Validation -- [ ] T050 [P] Run formatting on changed files with ./vendor/bin/pint --dirty -- [ ] T051 Run targeted tests: ./vendor/bin/sail artisan test --filter=BackupSetAddPolicies +- [x] T050 [P] Run formatting on changed files with ./vendor/bin/pint --dirty +- [x] T051 Run targeted tests: ./vendor/bin/sail artisan test --filter=BackupSetAddPolicies diff --git a/tests/Unit/BulkOperationRunProgressTest.php b/tests/Unit/BulkOperationRunProgressTest.php index a36a66f..0ea4e9d 100644 --- a/tests/Unit/BulkOperationRunProgressTest.php +++ b/tests/Unit/BulkOperationRunProgressTest.php @@ -59,3 +59,33 @@ expect($run->total_items)->toBe(2); }); + +test('bulk operation completion treats non-skipped failure entries as errors even when failed is zero', function () { + $tenant = Tenant::factory()->create(); + $user = User::factory()->create(); + + $service = app(BulkOperationService::class); + $run = $service->createRun($tenant, $user, 'backup_set', 'add_policies', ['1'], 1); + + $service->start($run); + $service->recordSuccess($run); + + $run->update([ + 'failures' => [ + [ + 'type' => 'foundation', + 'item_id' => 'foundation', + 'reason' => 'Forbidden', + 'reason_code' => 'graph_forbidden', + 'timestamp' => now()->toIso8601String(), + ], + ], + ]); + + $service->complete($run); + + $run->refresh(); + + expect($run->failed)->toBe(0) + ->and($run->status)->toBe('completed_with_errors'); +});