From 19361579f2d9e7608be2159d9bae34df3bd6527f Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Thu, 15 Jan 2026 21:08:14 +0100 Subject: [PATCH 1/3] spec(052): async add policies --- .../checklists/requirements.md | 32 +++ specs/052-async-add-policies/plan.md | 108 ++++++++++ specs/052-async-add-policies/spec.md | 196 ++++++++++++++++++ specs/052-async-add-policies/tasks.md | 104 ++++++++++ 4 files changed, 440 insertions(+) create mode 100644 specs/052-async-add-policies/checklists/requirements.md create mode 100644 specs/052-async-add-policies/plan.md create mode 100644 specs/052-async-add-policies/spec.md create mode 100644 specs/052-async-add-policies/tasks.md diff --git a/specs/052-async-add-policies/checklists/requirements.md b/specs/052-async-add-policies/checklists/requirements.md new file mode 100644 index 0000000..50ebed6 --- /dev/null +++ b/specs/052-async-add-policies/checklists/requirements.md @@ -0,0 +1,32 @@ +# Specification Quality Checklist: Async “Add Policies” to Backup Set (052) + +**Purpose**: Validate specification completeness and quality before proceeding to implementation +**Created**: 2026-01-14 +**Feature**: [specs/052-async-add-policies/spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details leaked into user requirements (spec focuses on behaviors, run observability, and safety rules) +- [x] Focused on user value (fast submit, dedupe, observable progress, safe failures) +- [x] Written in operator language (Backup Sets, Add Policies, runs, failures) +- [x] Mandatory sections present (User Scenarios & Testing, Requirements, Success Criteria) + +## Requirement Completeness + +- [x] Scope is bounded (explicit in-scope/out-of-scope list) +- [x] Requirements are testable and unambiguous (FR-001..FR-005 + NFRs + acceptance scenarios) +- [x] Idempotency and dedupe behavior is explicitly specified +- [x] Tenant isolation and authorization are explicitly specified +- [x] Data minimization and safe logging rules are explicitly specified +- [x] “No UI-time Graph calls” is explicitly specified and test-gated + +## Feature Readiness + +- [x] Plan includes concrete touch points and test strategy (plan.md) +- [x] Tasks are grouped by user story and prioritize tests-first (tasks.md) + +## Notes + +- Items marked incomplete require spec updates before implementation. +- Constitution gate: this checklist must exist for features that change runtime behavior. + diff --git a/specs/052-async-add-policies/plan.md b/specs/052-async-add-policies/plan.md new file mode 100644 index 0000000..c60222f --- /dev/null +++ b/specs/052-async-add-policies/plan.md @@ -0,0 +1,108 @@ +# Implementation Plan: Async “Add Policies” to Backup Set (052) + +**Branch**: `feat/052-async-add-policies` | **Date**: 2026-01-14 | **Spec**: [specs/052-async-add-policies/spec.md](spec.md) +**Input**: Feature specification from `/specs/052-async-add-policies/spec.md` + +## Summary + +Convert Backup Set → Add Policies (“Add selected”) to job-only execution by creating/reusing a `BulkOperationRun`, dispatching a queued job to do Graph/snapshot work, and returning immediately with observable feedback and a “View run” link. + +## Technical Context + +**Language/Version**: PHP 8.4 (Laravel 12) +**Admin UI**: Filament v4 + Livewire v3 +**Storage**: PostgreSQL +**Queue**: Laravel queues (Sail-first locally) +**Testing**: Pest v4 +**Constraints**: No Graph calls during interactive request/response; tenant isolation; idempotent dedupe; safe error persistence. + +## Constitution Check + +- Inventory-first: this feature updates backup storage (explicit snapshot capture), never “inventory on render”. +- Read/write separation: interactive action is “write intent only” (enqueue); job performs DB writes with auditability. +- Graph contract path: any Graph reads happen only inside the job via `GraphClientInterface` (indirectly via existing services). +- Deterministic capabilities: selection hashing and idempotency key are deterministic. +- Tenant isolation: run records and backup set mutations are scoped to the active tenant. +- Automation: job has run record lifecycle + counts + failures; dedupe prevents duplicates. +- Data minimization: failures and notifications store sanitized messages only. + +## Project Structure + +### Documentation (this feature) + +```text +specs/052-async-add-policies/ +├── plan.md +├── spec.md +├── tasks.md +└── checklists/ + └── requirements.md +``` + +### Source Code (planned touch points) + +```text +app/Livewire/BackupSetPolicyPickerTable.php # “Add selected” handler becomes enqueue-only +app/Jobs/AddPoliciesToBackupSetJob.php # new queued job (name may vary) +app/Models/BulkOperationRun.php # reused run record +app/Services/BulkOperationService.php # reused counters + failure sanitization +app/Support/RunIdempotency.php # reused deterministic key builder + active-run lookup +app/Filament/Resources/BulkOperationRunResource.php # existing run UI used for “View run” +tests/Feature/Filament/BackupSetPolicyPickerTableTest.php # updated for async behavior +tests/Feature/* # new/updated tests for idempotency + tenant isolation +``` + +## Execution Model + +### Action Handler (UI request) + +Responsibilities (must remain fast): +1. Resolve tenant + user context and authorize “add policies to backup set”. +2. Convert selected policy records into an ID list (no Graph/snapshot work). +3. Compute a deterministic idempotency key using: + - tenant id + - target backup_set_id + - operation type (e.g., `backup_set.add_policies`) + - context payload containing (sorted) policy IDs + option flags (include assignments/scope tags/foundations) +4. Reuse an active run if one exists; otherwise create a new `BulkOperationRun` with `resource=backup_set`, `action=add_policies`, `item_ids` = selected policy IDs, `total_items` = count, and `idempotency_key` set. +5. Dispatch a queued job (always async; no sync shortcut) with the run id + backup_set_id + option flags. +6. Return immediately with a Filament notification and a “View run” link (DB notification preferred). + +### Queued Job + +Responsibilities (all heavy work): +1. Load the `BulkOperationRun` and ensure it is still active (`pending` → `running`). +2. Validate the backup set exists and belongs to the run tenant. +3. Process the run’s stored policy IDs deterministically (sequentially or chunked): + - Add/capture each policy into the backup set using existing capture services. + - Update counts and record per-item failures with safe reasons. + - Continue through all items (no circuit-breaker abort). +4. Complete the run with: + - `completed` if no failures + - `completed_with_errors` (partial) if some failures + - `failed` if the job cannot proceed (e.g., backup set missing) +5. Emit a DB notification to the initiating user summarizing the outcome and linking to the run. + +### Idempotency & Concurrency + +- Primary dedupe mechanism: `RunIdempotency::findActiveBulkOperationRun(tenant_id, idempotency_key)`. +- Secondary guard: existing partial unique index on `(tenant_id, idempotency_key)` for active statuses. +- Race handling: if concurrent submissions collide, prefer “find existing run and redirect” over throwing. +- Dedupe scope: tenant-wide (idempotency key does not include `user_id`). + +## Testing Strategy (Pest) + +Minimum required tests for 052: +- Action dispatch test: `Add selected` queues the job and creates/reuses a run. +- Fail-hard Graph guard test: binding `GraphClientInterface` (or a capture service) to a mock that must not be called during the action. +- Idempotency test: calling the action twice with the same selection queues only one job and creates only one active run. +- Tenant isolation test: run view under another tenant context is forbidden (403). + +Target commands: +- `./vendor/bin/sail artisan test --filter=BackupSetAddPolicies` +- `./vendor/bin/pint --dirty` + +## Rollout Notes + +- Requires queue workers running for background processing (Sail locally; Dokploy workers in staging/prod). +- No destructive migrations expected; if schema is extended for better observability, it must remain backwards compatible. diff --git a/specs/052-async-add-policies/spec.md b/specs/052-async-add-policies/spec.md new file mode 100644 index 0000000..78d4b73 --- /dev/null +++ b/specs/052-async-add-policies/spec.md @@ -0,0 +1,196 @@ +# Feature Specification: Async “Add Policies” to Backup Set (052) + +**Feature Branch**: `feat/052-async-add-policies` +**Created**: 2026-01-14 +**Status**: Draft (implementation-ready) +**Input**: Make Backup Sets → “Add Policies” (Add selected) non-blocking by moving all Graph/snapshot work into a queued job. The UI action only creates/reuses a Run record and dispatches the job. + +## Purpose + +Make “Add selected” in the Backup Set → Add Policies flow reliable and fast by ensuring heavy work (Graph reads, snapshot capture, per-policy loops) never runs inline in an interactive request. + +Primary outcomes: +- Prevent request timeouts and long waits. +- Prevent duplicate work from double-clicks/retries. +- Provide observable progress and safe failure visibility via an existing Run record type. + +## Clarifications + +### Session 2026-01-15 + +- Q: Who may start “Add selected”? → A: Roles `Owner`, `Manager`, `Operator` may start; `Readonly` may not. +- Q: Is idempotency/dedupe per-user or tenant-wide? → A: Tenant-wide (dedupe key does not include `user_id`). +- Q: Should execution ever run synchronously for small selections? → A: No; always async (never in-request). +- Q: What happens for empty selection? → A: No run/job; show “No policies selected” (current behavior). +- Q: How are run counts defined? → A: `total_items` equals the number of selected policies at submit time; “already in backup set” counts as `skipped`. +- Q: Do we use a circuit breaker (e.g., abort if >50% fails)? → A: No; process all items and surface partial results. +- Q: How should failures be persisted? → A: Stable `reason_code` + sanitized short text; never store secrets/tokens/raw payload dumps. + +## Pinned Decisions (052 defaults) + +- **Authorization**: start allowed for `Owner`, `Manager`, `Operator`; forbidden for `Readonly`. +- **Dedupe scope**: tenant-wide idempotency (no `user_id` in dedupe key). +- **Execution**: always async; action never performs Graph/snapshot work inline. +- **Empty selection**: no run/job; show “No policies selected”. +- **Counts**: `total_items` = selected policies; “already in backup set” = `skipped`. +- **Circuit breaker**: none; job processes all items. +- **Failure format**: `reason_code` + sanitized short text (no secrets). + +## In Scope (052) + +- Convert the “Add selected” action (Backup Sets → Add Policies modal) to job-only execution. +- Ensure an observable Run exists (status, counts, errors) using an existing run record type (prefer `BulkOperationRun`). +- Ensure idempotency: repeated clicks while queued/running reuse the same run for the same tenant + backup set + selection. +- Emit DB notification + optional existing progress widget integration (if already supported by the run type); no new UI framework work required. +- Add guard tests ensuring no Graph calls occur in the action handler/request. + +## Out of Scope (052) + +- UI redesign/polish (layout, navigation reorg, fancy tables, etc.) +- New Monitoring area or unified operations dashboard +- Group browsing/typeahead improvements (directory cache / later) +- New run tables if an existing run record can be reused +- Changing what a “policy version” means or how snapshots are stored + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Add selected policies without blocking (Priority: P1) + +As a tenant admin, I can add selected policies to a backup set without the UI hanging or timing out, because the system queues background work and gives me a Run record to monitor. + +**Why this priority**: This is a common workflow and currently risks long requests/timeouts when Graph capture is slow or throttled. + +**Independent Test**: Trigger “Add selected” and assert the request returns quickly, a Run exists, and a job is queued (no Graph work happens inline). + +**Acceptance Scenarios**: + +1. **Given** I am on a Backup Set and select policies in “Add Policies”, **When** I click “Add selected”, **Then** the UI returns quickly and a queued Run is created (or reused) with a link to its detail view. +2. **Given** the job runs, **When** it completes, **Then** the backup set contains the added policies and the Run shows final status + counts. + +--- + +### User Story 2 - Double click / repeated submissions are deduplicated (Priority: P2) + +As a tenant admin, I can click “Add selected” repeatedly (or double click) without creating duplicate work, because identical operations reuse the same active Run. + +**Why this priority**: Prevents accidental duplication, inconsistent outcomes, and unnecessary Graph load. + +**Independent Test**: Call the action twice with the same selection while the first run is active and assert only one Run and one queued job. + +**Acceptance Scenarios**: + +1. **Given** a matching Run for the same tenant + backup set + selection is queued or running, **When** I click “Add selected” again, **Then** the system reuses the existing Run and does not enqueue duplicate work. + +--- + +### User Story 3 - Failures are visible and safe (Priority: P3) + +As a tenant admin, I can see safe failure summaries when some policies cannot be captured from Graph, without secrets or raw payloads being stored or displayed. + +**Why this priority**: Operators must be able to triage issues safely; secret leakage is unacceptable. + +**Independent Test**: Force a simulated Graph failure in the job and assert the Run ends as failed/partial with a sanitized reason code/summary (no secrets). + +**Acceptance Scenarios**: + +1. **Given** Graph returns an error for some policies, **When** the job completes, **Then** the Run is marked partial/completed-with-errors and includes per-item failures with safe reason codes/summaries. +2. **Given** a failure includes sensitive substrings (e.g., “Bearer ”), **When** it is persisted, **Then** stored reasons are redacted/sanitized. + +--- + +### Edge Cases + +- Empty selection (no policies selected) +- Backup set deleted or tenant context missing between queue and execution +- Some selected policies are already in the backup set +- Policies deleted/ignored locally between queue and execution +- Graph throttling/transient failures (429/503) during capture + +## Requirements *(mandatory)* + +**Constitution alignment (required):** This feature performs Graph reads and writes local DB state, so it MUST be idempotent & observable, tenant-scoped, and safe-loggable. Graph calls MUST go through `GraphClientInterface` and MUST NOT occur during UI render or action request handling. + +### Functional Requirements + +- **FR-001 (Job-only action handler)**: The “Add selected” handler MUST: + - validate input + authorization + - create/reuse a Run record + - dispatch a queued job + - return immediately with a notification and a link to the Run + It MUST NOT: + - call `GraphClientInterface` + - call snapshot capture services + - loop over selected policies to do work inline + - run a synchronous “small selection” shortcut + +- **FR-002 (Run record + observability)**: The system MUST persist for each run: + - `tenant_id`, `initiator_user_id` (or equivalent) + - `resource = backup_set`, `action = add_policies` (or existing taxonomy) + - status lifecycle: queued → running → succeeded|failed|partial (map to existing run status semantics where possible, e.g., queued=`pending`, succeeded=`completed`, partial=`completed_with_errors`) + - counts: `total_items` = selected policies at submit time; `processed_items` = succeeded+failed+skipped; “already in backup set” increments `skipped` + - safe error context (summary + per-item outcomes at least for failures) + +- **FR-003 (Idempotency / dedupe)**: While a matching run is queued or running, the action MUST reuse it and MUST NOT enqueue duplicate work. + + Recommended dedupe key: `tenant_id + backup_set_id + operation_type(add_policies) + selection_hash`. + +- **FR-004 (Job execution semantics)**: The queued job MUST: + - load the selection deterministically (IDs or a stored selection payload) + - process items sequentially or in safe batches + - update run progress counts as it goes + - process all items (no circuit-breaker abort) + - record per-item outcomes (at minimum: failure entries with stable `reason_code` + sanitized short text) + + **Reason codes (minimum set):** + - `already_in_backup_set` (skipped) + - `policy_not_found` (failed) + - `policy_ignored` (skipped) + - `backup_set_not_found` (failed) + - `backup_set_archived` (failed) + - `graph_forbidden` (failed) + - `graph_throttled` (failed) + - `graph_transient` (failed) + - `unknown` (failed) + +- **FR-005 (User-visible feedback)**: On action submit, the UI MUST: + - show “Queued” feedback + - provide a “View run” link (DB notification preferred) + +### Non-Functional Requirements + +- **NFR-001 (Tenant isolation and authorization)**: + - Run list/view MUST be tenant-scoped + - Cross-tenant run access MUST be denied (403) + - Only authorized roles can start “Add Policies” + +- **NFR-002 (Data minimization & safe logging)**: + - No access tokens, “Bearer ” strings, or raw Graph payload dumps in notifications, run failures, or logs + - Persist only sanitized/allowlisted error fields and stable identifiers + +- **NFR-003 (Determinism)**: Given the same input selection, results are reproducible and the dedupe key is stable. + +- **NFR-004 (No UI-time Graph calls)**: Rendering the Backup Set UI and Run detail pages MUST not require Graph calls. + +### Key Entities *(include if feature involves data)* + +- **BulkOperationRun**: Existing run record used for long-running operations (status + counts + failures). +- **BackupSet**: Target set to which policies are added. +- **BackupItem**: Persisted item rows representing a policy in a backup set. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001 (Fast submit)**: Clicking “Add selected” returns without timing out and does not perform Graph work in-request (proved by guard tests). +- **SC-002 (Observable run)**: A Run is created/reused and is visible in the UI with status and progress counts. +- **SC-003 (No duplicates)**: Double-click does not create duplicate runs/jobs (proved by idempotency tests). +- **SC-004 (Safe failures)**: Failures show safe reason codes/summaries and do not store secrets/tokens (proved by sanitization tests). + +## Rollout / Migration + +- No destructive migrations required for 052. +- If additional idempotency indexing is needed beyond existing run infrastructure, add a minimal migration (backwards compatible). + +## Open Questions (Optional) + +- Should progress appear in an existing global progress widget (only if already supported; new widget work is out of scope)? diff --git a/specs/052-async-add-policies/tasks.md b/specs/052-async-add-policies/tasks.md new file mode 100644 index 0000000..d80bfa4 --- /dev/null +++ b/specs/052-async-add-policies/tasks.md @@ -0,0 +1,104 @@ +--- + +description: "Task list for implementing Async “Add Policies” to Backup Set (052)" +--- + +# Tasks: Async “Add Policies” to Backup Set (052) + +**Input**: Design documents from `/specs/052-async-add-policies/` +**Prerequisites**: plan.md (required), spec.md (required) + +**Tests**: Required (Pest), per spec.md (SC-001..SC-004). + +**Organization**: Tasks are grouped by user story so each story can be implemented and tested independently. + +## Format: `- [ ] T### [P?] [US#] Description (with file path)` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[US#]**: User story mapping (US1/US2/US3) + +## Path Conventions (Laravel) + +- App code: `app/` +- DB: `database/migrations/` +- Filament admin: `app/Filament/Resources/` +- Livewire tables: `app/Livewire/` +- Tests (Pest): `tests/Feature/`, `tests/Unit/` + +--- + +## 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 + +--- + +## Phase 2: User Story 1 - Add selected policies without blocking (Priority: P1) 🎯 MVP + +**Goal**: “Add selected” returns quickly and queues background work with an observable Run record. + +**Independent Test**: Trigger the action and assert a job is queued and a run exists; no Graph/capture work occurs in-request. + +### 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()`) + +### 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 + +**Checkpoint**: US1 complete — submit is non-blocking, job executes, run is visible. + +--- + +## Phase 3: User Story 2 - Double click / repeated submissions are deduplicated (Priority: P2) + +**Goal**: Matching queued/running operations reuse the same run and do not enqueue duplicates. + +**Independent Test**: Call the action twice for the same tenant + backup set + selection and assert only one run and one job dispatch. + +### Tests ⚠️ + +- [ ] 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 + +**Checkpoint**: US2 complete — double clicks are safe and deduped. + +--- + +## Phase 4: User Story 3 - Failures are visible and safe (Priority: P3) + +**Goal**: Failures are persisted safely and tenant isolation is enforced for run visibility. + +**Independent Test**: Force failure paths and confirm safe failures persisted; cross-tenant access is forbidden. + +### 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) + +### 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) + +**Checkpoint**: US3 complete — failures are safe and observable; tenant isolation holds. + +--- + +## 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 -- 2.45.2 From 593ddf9fd5a633e3684642b470ccc0586b1ae5e9 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Thu, 15 Jan 2026 21:45:08 +0100 Subject: [PATCH 2/3] feat(052): async add policies to backup set --- app/Jobs/AddPoliciesToBackupSetJob.php | 573 ++++++++++++++++++ app/Livewire/BackupSetPolicyPickerTable.php | 213 +++++-- app/Services/BulkOperationService.php | 30 +- .../AddPoliciesToBackupSetJobTest.php | 146 +++++ .../BackupSetPolicyPickerTableTest.php | 262 +++++--- 5 files changed, 1095 insertions(+), 129 deletions(-) create mode 100644 app/Jobs/AddPoliciesToBackupSetJob.php create mode 100644 tests/Feature/BackupSets/AddPoliciesToBackupSetJobTest.php diff --git a/app/Jobs/AddPoliciesToBackupSetJob.php b/app/Jobs/AddPoliciesToBackupSetJob.php new file mode 100644 index 0000000..04cb3b2 --- /dev/null +++ b/app/Jobs/AddPoliciesToBackupSetJob.php @@ -0,0 +1,573 @@ +find($this->bulkRunId); + + if (! $run || $run->status !== 'pending') { + return; + } + + $bulkOperationService->start($run); + + $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( + run: $run, + itemId: (string) $policyId, + reason: 'Already in backup set', + reasonCode: 'already_in_backup_set', + ); + + continue; + } + + $trashed = $trashedItems->get($policyId); + + 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( + 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; + + $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)"; + } + } + + $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(); + } + + /** + * @return array + */ + private function extractPolicyIds(BulkOperationRun $run): array + { + $itemIds = $run->item_ids ?? []; + + $policyIds = []; + + if (is_array($itemIds) && array_key_exists('policy_ids', $itemIds) && is_array($itemIds['policy_ids'])) { + $policyIds = $itemIds['policy_ids']; + } elseif (is_array($itemIds)) { + $policyIds = $itemIds; + } + + $policyIds = array_values(array_unique(array_map('intval', $policyIds))); + $policyIds = array_values(array_filter($policyIds, fn (int $value): bool => $value > 0)); + sort($policyIds); + + return $policyIds; + } + + /** + * @param array $entry + */ + private function appendRunFailure(BulkOperationRun $run, array $entry): void + { + $failures = $run->failures ?? []; + + $failures[] = array_merge([ + 'timestamp' => now()->toIso8601String(), + ], $entry); + + $run->update(['failures' => $failures]); + } + + private function mapGraphFailureReasonCode(?int $status): string + { + return match (true) { + $status === 403 => 'graph_forbidden', + in_array($status, [429, 503], true) => 'graph_throttled', + in_array($status, [408, 500, 502, 504], true) => 'graph_transient', + default => 'unknown', + }; + } + + /** + * @return array{0:array{created:int,restored:int,failures:array},1:array} + */ + private function captureFoundations( + BulkOperationService $bulkOperationService, + FoundationSnapshotService $foundationSnapshots, + Tenant $tenant, + BackupSet $backupSet, + ): array { + $types = config('tenantpilot.foundation_types', []); + $created = 0; + $restored = 0; + $failures = []; + + foreach ($types as $typeConfig) { + $foundationType = $typeConfig['type'] ?? null; + + if (! is_string($foundationType) || $foundationType === '') { + continue; + } + + $result = $foundationSnapshots->fetchAll($tenant, $foundationType); + + foreach (($result['failures'] ?? []) as $failure) { + if (! is_array($failure)) { + continue; + } + + $status = isset($failure['status']) && is_numeric($failure['status']) ? (int) $failure['status'] : null; + $reasonCode = $this->mapGraphFailureReasonCode($status); + $reason = $bulkOperationService->sanitizeFailureReason((string) ($failure['reason'] ?? 'Foundation capture failed.')); + + $failures[] = [ + 'foundation_type' => $foundationType, + 'reason' => $reason, + 'status' => $status, + 'reason_code' => $reasonCode, + ]; + } + + foreach (($result['items'] ?? []) as $snapshot) { + if (! is_array($snapshot)) { + continue; + } + + $sourceId = $snapshot['source_id'] ?? null; + + if (! is_string($sourceId) || $sourceId === '') { + continue; + } + + $existing = BackupItem::withTrashed() + ->where('backup_set_id', $backupSet->getKey()) + ->where('policy_type', $foundationType) + ->where('policy_identifier', $sourceId) + ->first(); + + if ($existing) { + if ($existing->trashed()) { + $existing->restore(); + $restored++; + } + + continue; + } + + BackupItem::create([ + 'tenant_id' => $tenant->getKey(), + 'backup_set_id' => $backupSet->getKey(), + 'policy_id' => null, + 'policy_identifier' => $sourceId, + 'policy_type' => $foundationType, + 'platform' => $typeConfig['platform'] ?? null, + 'payload' => $snapshot['payload'] ?? [], + 'metadata' => $snapshot['metadata'] ?? [], + ]); + + $created++; + } + } + + return [ + [ + 'created' => $created, + 'restored' => $restored, + 'failures' => $failures, + ], + $failures, + ]; + } +} diff --git a/app/Livewire/BackupSetPolicyPickerTable.php b/app/Livewire/BackupSetPolicyPickerTable.php index fdf340c..02c9461 100644 --- a/app/Livewire/BackupSetPolicyPickerTable.php +++ b/app/Livewire/BackupSetPolicyPickerTable.php @@ -2,10 +2,15 @@ namespace App\Livewire; +use App\Filament\Resources\BulkOperationRunResource; +use App\Jobs\AddPoliciesToBackupSetJob; use App\Models\BackupSet; +use App\Models\BulkOperationRun; use App\Models\Policy; use App\Models\Tenant; -use App\Services\Intune\BackupService; +use App\Models\User; +use App\Services\BulkOperationService; +use App\Support\RunIdempotency; use Filament\Actions\BulkAction; use Filament\Notifications\Notification; use Filament\Tables\Columns\TextColumn; @@ -15,6 +20,7 @@ use Filament\Tables\TableComponent; use Illuminate\Contracts\View\View; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\QueryException; use Illuminate\Support\Collection; use Illuminate\Support\Str; @@ -171,14 +177,82 @@ public function table(Table $table): Table BulkAction::make('add_selected_to_backup_set') ->label('Add selected') ->icon('heroicon-m-plus') - ->action(function (Collection $records, BackupService $service): void { + ->authorize(function (): bool { + $user = auth()->user(); + + if (! $user instanceof User) { + return false; + } + + try { + $tenant = Tenant::current(); + } catch (\RuntimeException) { + return false; + } + + if (! $user->canSyncTenant($tenant)) { + return false; + } + + return BackupSet::query() + ->whereKey($this->backupSetId) + ->where('tenant_id', $tenant->getKey()) + ->exists(); + }) + ->action(function (Collection $records, BulkOperationService $bulkOperationService): void { $backupSet = BackupSet::query()->findOrFail($this->backupSetId); - $tenant = $backupSet->tenant ?? Tenant::current(); + $tenant = null; - $beforeFailures = (array) (($backupSet->metadata ?? [])['failures'] ?? []); - $beforeFailureCount = count($beforeFailures); + try { + $tenant = Tenant::current(); + } catch (\RuntimeException) { + $tenant = $backupSet->tenant; + } + $user = auth()->user(); - $policyIds = $records->pluck('id')->all(); + if (! $user instanceof User) { + Notification::make() + ->title('Not allowed') + ->danger() + ->send(); + + return; + } + + if (! $tenant instanceof Tenant) { + Notification::make() + ->title('Not allowed') + ->danger() + ->send(); + + return; + } + + if ((int) $tenant->getKey() !== (int) $backupSet->tenant_id) { + Notification::make() + ->title('Not allowed') + ->danger() + ->send(); + + return; + } + + if (! $user->canSyncTenant($tenant)) { + Notification::make() + ->title('Not allowed') + ->danger() + ->send(); + + return; + } + + $policyIds = $records + ->pluck('id') + ->map(fn (mixed $value): int => (int) $value) + ->filter(fn (int $value): bool => $value > 0) + ->unique() + ->values() + ->all(); if ($policyIds === []) { Notification::make() @@ -189,38 +263,109 @@ public function table(Table $table): Table return; } - $service->addPoliciesToSet( - tenant: $tenant, - backupSet: $backupSet, - policyIds: $policyIds, - actorEmail: auth()->user()?->email, - actorName: auth()->user()?->name, - includeAssignments: $this->include_assignments, - includeScopeTags: $this->include_scope_tags, - includeFoundations: $this->include_foundations, + sort($policyIds); + + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'backup_set.add_policies', + targetId: (string) $backupSet->getKey(), + context: [ + 'policy_ids' => $policyIds, + 'include_assignments' => (bool) $this->include_assignments, + 'include_scope_tags' => (bool) $this->include_scope_tags, + 'include_foundations' => (bool) $this->include_foundations, + ], + ); + + $existingRun = RunIdempotency::findActiveBulkOperationRun( + tenantId: (int) $tenant->getKey(), + idempotencyKey: $idempotencyKey, + ); + + if ($existingRun instanceof BulkOperationRun) { + Notification::make() + ->title('Add policies already queued') + ->body('A matching run is already queued or running. Open the run to monitor progress.') + ->actions([ + \Filament\Actions\Action::make('view_run') + ->label('View run') + ->url(BulkOperationRunResource::getUrl('view', ['record' => $existingRun], tenant: $tenant)), + ]) + ->info() + ->send(); + + return; + } + + $selectionPayload = [ + 'backup_set_id' => (int) $backupSet->getKey(), + 'policy_ids' => $policyIds, + 'options' => [ + 'include_assignments' => (bool) $this->include_assignments, + 'include_scope_tags' => (bool) $this->include_scope_tags, + 'include_foundations' => (bool) $this->include_foundations, + ], + ]; + + try { + $run = $bulkOperationService->createRun( + tenant: $tenant, + user: $user, + resource: 'backup_set', + action: 'add_policies', + itemIds: $selectionPayload, + totalItems: count($policyIds), + idempotencyKey: $idempotencyKey, + ); + } catch (QueryException $exception) { + if ((string) $exception->getCode() === '23505') { + $existingRun = RunIdempotency::findActiveBulkOperationRun( + tenantId: (int) $tenant->getKey(), + idempotencyKey: $idempotencyKey, + ); + + if ($existingRun instanceof BulkOperationRun) { + Notification::make() + ->title('Add policies already queued') + ->body('A matching run is already queued or running. Open the run to monitor progress.') + ->actions([ + \Filament\Actions\Action::make('view_run') + ->label('View run') + ->url(BulkOperationRunResource::getUrl('view', ['record' => $existingRun], tenant: $tenant)), + ]) + ->info() + ->send(); + + return; + } + } + + throw $exception; + } + + AddPoliciesToBackupSetJob::dispatch( + bulkRunId: (int) $run->getKey(), + backupSetId: (int) $backupSet->getKey(), + includeAssignments: (bool) $this->include_assignments, + includeScopeTags: (bool) $this->include_scope_tags, + includeFoundations: (bool) $this->include_foundations, ); $notificationTitle = $this->include_foundations - ? 'Backup items added' - : 'Policies added to backup'; + ? 'Backup items queued' + : 'Policies queued'; - $backupSet->refresh(); - - $afterFailures = (array) (($backupSet->metadata ?? [])['failures'] ?? []); - $afterFailureCount = count($afterFailures); - - if ($afterFailureCount > $beforeFailureCount) { - Notification::make() - ->title($notificationTitle.' with failures') - ->body('Some policies could not be captured from Microsoft Graph. Check the backup set failures list for details.') - ->warning() - ->send(); - } else { - Notification::make() - ->title($notificationTitle) - ->success() - ->send(); - } + Notification::make() + ->title($notificationTitle) + ->body('A background job has been queued. You can monitor progress in the run details or progress widget.') + ->actions([ + \Filament\Actions\Action::make('view_run') + ->label('View run') + ->url(BulkOperationRunResource::getUrl('view', ['record' => $run], tenant: $tenant)), + ]) + ->success() + ->sendToDatabase($user) + ->send(); $this->resetTable(); }), diff --git a/app/Services/BulkOperationService.php b/app/Services/BulkOperationService.php index 6530c55..8c4992d 100644 --- a/app/Services/BulkOperationService.php +++ b/app/Services/BulkOperationService.php @@ -43,15 +43,21 @@ public function createRun( string $resource, string $action, array $itemIds, - int $totalItems + int $totalItems, + ?string $idempotencyKey = null ): BulkOperationRun { - $effectiveTotalItems = max($totalItems, count($itemIds)); + $effectiveTotalItems = $totalItems; + + if (array_is_list($itemIds)) { + $effectiveTotalItems = max($totalItems, count($itemIds)); + } $run = BulkOperationRun::create([ 'tenant_id' => $tenant->id, 'user_id' => $user->id, 'resource' => $resource, 'action' => $action, + 'idempotency_key' => $idempotencyKey, 'status' => 'pending', 'item_ids' => $itemIds, 'total_items' => $effectiveTotalItems, @@ -94,17 +100,23 @@ public function recordSuccess(BulkOperationRun $run): void $run->increment('succeeded'); } - public function recordFailure(BulkOperationRun $run, string $itemId, string $reason): void + public function recordFailure(BulkOperationRun $run, string $itemId, string $reason, ?string $reasonCode = null): void { $reason = $this->sanitizeFailureReason($reason); $failures = $run->failures ?? []; - $failures[] = [ + $failureEntry = [ 'item_id' => $itemId, 'reason' => $reason, 'timestamp' => now()->toIso8601String(), ]; + if (is_string($reasonCode) && $reasonCode !== '') { + $failureEntry['reason_code'] = $reasonCode; + } + + $failures[] = $failureEntry; + $run->update([ 'failures' => $failures, 'processed_items' => $run->processed_items + 1, @@ -118,18 +130,24 @@ public function recordSkipped(BulkOperationRun $run): void $run->increment('skipped'); } - public function recordSkippedWithReason(BulkOperationRun $run, string $itemId, string $reason): void + public function recordSkippedWithReason(BulkOperationRun $run, string $itemId, string $reason, ?string $reasonCode = null): void { $reason = $this->sanitizeFailureReason($reason); $failures = $run->failures ?? []; - $failures[] = [ + $failureEntry = [ 'item_id' => $itemId, 'reason' => $reason, 'type' => 'skipped', 'timestamp' => now()->toIso8601String(), ]; + if (is_string($reasonCode) && $reasonCode !== '') { + $failureEntry['reason_code'] = $reasonCode; + } + + $failures[] = $failureEntry; + $run->update([ 'failures' => $failures, 'processed_items' => $run->processed_items + 1, diff --git a/tests/Feature/BackupSets/AddPoliciesToBackupSetJobTest.php b/tests/Feature/BackupSets/AddPoliciesToBackupSetJobTest.php new file mode 100644 index 0000000..511e76c --- /dev/null +++ b/tests/Feature/BackupSets/AddPoliciesToBackupSetJobTest.php @@ -0,0 +1,146 @@ +actingAs($user); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->id, + 'name' => 'Test backup', + 'status' => 'completed', + 'metadata' => ['failures' => []], + ]); + + $policyA = Policy::factory()->create([ + 'tenant_id' => $tenant->id, + 'ignored_at' => null, + ]); + + $policyB = Policy::factory()->create([ + 'tenant_id' => $tenant->id, + 'ignored_at' => null, + ]); + + $versionA = PolicyVersion::factory()->create([ + 'tenant_id' => $tenant->id, + 'policy_id' => $policyA->id, + 'policy_type' => $policyA->policy_type, + 'platform' => $policyA->platform, + 'snapshot' => ['id' => $policyA->external_id], + ]); + + $run = BulkOperationRun::factory()->create([ + 'tenant_id' => $tenant->id, + 'user_id' => $user->id, + 'resource' => 'backup_set', + 'action' => 'add_policies', + 'status' => 'pending', + 'total_items' => 2, + 'item_ids' => [ + 'backup_set_id' => $backupSet->id, + 'policy_ids' => [$policyA->id, $policyB->id], + 'options' => [ + 'include_assignments' => true, + 'include_scope_tags' => true, + 'include_foundations' => false, + ], + ], + 'failures' => [], + ]); + + $this->mock(PolicyCaptureOrchestrator::class, function (MockInterface $mock) use ($policyA, $policyB, $tenant, $versionA) { + $mock->shouldReceive('capture') + ->twice() + ->andReturnUsing(function ( + Policy $policy, + \App\Models\Tenant $tenantArg, + bool $includeAssignments = false, + bool $includeScopeTags = false, + ?string $createdBy = null, + array $metadata = [] + ) use ($policyA, $policyB, $tenant, $versionA) { + expect($tenantArg->id)->toBe($tenant->id); + expect($includeAssignments)->toBeTrue(); + expect($includeScopeTags)->toBeTrue(); + expect($metadata['backup_set_id'] ?? null)->not->toBeNull(); + + if ($policy->is($policyA)) { + return [ + 'version' => $versionA, + 'captured' => [ + 'payload' => [ + 'id' => $policyA->external_id, + '@odata.type' => '#microsoft.graph.deviceManagementConfigurationPolicy', + ], + 'assignments' => [], + 'scope_tags' => ['ids' => ['0'], 'names' => ['Default']], + 'metadata' => [], + ], + ]; + } + + expect($policy->is($policyB))->toBeTrue(); + + return [ + 'failure' => [ + 'policy_id' => $policyB->id, + 'reason' => 'Forbidden', + 'status' => 403, + ], + ]; + }); + }); + + $job = new AddPoliciesToBackupSetJob( + bulkRunId: (int) $run->getKey(), + backupSetId: (int) $backupSet->getKey(), + includeAssignments: true, + includeScopeTags: true, + includeFoundations: false, + ); + + $job->handle( + bulkOperationService: app(BulkOperationService::class), + captureOrchestrator: app(PolicyCaptureOrchestrator::class), + foundationSnapshots: $this->mock(FoundationSnapshotService::class), + snapshotValidator: app(SnapshotValidator::class), + ); + + $run->refresh(); + $backupSet->refresh(); + + expect($run->status)->toBe('completed_with_errors'); + expect($run->total_items)->toBe(2); + expect($run->processed_items)->toBe(2); + expect($run->succeeded)->toBe(1); + expect($run->failed)->toBe(1); + expect($run->skipped)->toBe(0); + + expect(BackupItem::query() + ->where('backup_set_id', $backupSet->id) + ->where('policy_id', $policyA->id) + ->exists())->toBeTrue(); + + $failureEntry = collect($run->failures ?? []) + ->firstWhere('item_id', (string) $policyB->id); + + expect($failureEntry)->not->toBeNull(); + expect($failureEntry['reason_code'] ?? null)->toBe('graph_forbidden'); + + expect($backupSet->status)->toBe('partial'); +}); diff --git a/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php b/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php index a1a9143..c63f59e 100644 --- a/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php +++ b/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php @@ -1,23 +1,31 @@ create(); - $tenant->makeCurrent(); +test('policy picker table queues add policies job and creates a run (no inline capture)', function () { + Queue::fake(); - $user = User::factory()->create(); + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); $backupSet = BackupSet::factory()->create([ 'tenant_id' => $tenant->id, @@ -30,23 +38,12 @@ 'last_synced_at' => now(), ]); - $this->mock(BackupService::class, function (MockInterface $mock) use ($tenant, $backupSet, $policies, $user) { - $mock->shouldReceive('addPoliciesToSet') - ->once() - ->withArgs(function ($tenantArg, $backupSetArg, $policyIds, $actorEmail, $actorName, $includeAssignments, $includeScopeTags, $includeFoundations) use ($tenant, $backupSet, $policies, $user) { - expect($tenantArg->id)->toBe($tenant->id); - expect($backupSetArg->id)->toBe($backupSet->id); - expect($policyIds)->toBe($policies->pluck('id')->all()); - expect($actorEmail)->toBe($user->email); - expect($actorName)->toBe($user->name); - expect($includeAssignments)->toBeTrue(); - expect($includeScopeTags)->toBeTrue(); - expect($includeFoundations)->toBeTrue(); - - return true; - }); + $this->mock(BackupService::class, function (MockInterface $mock) { + $mock->shouldReceive('addPoliciesToSet')->never(); }); + bindFailHardGraphClient(); + Livewire::actingAs($user) ->test(BackupSetPolicyPickerTable::class, [ 'backupSetId' => $backupSet->id, @@ -54,67 +51,119 @@ ->callTableBulkAction('add_selected_to_backup_set', $policies) ->assertHasNoTableBulkActionErrors(); - $notifications = session('filament.notifications', []); + Queue::assertPushed(AddPoliciesToBackupSetJob::class, 1); - expect($notifications)->not->toBeEmpty(); - expect(collect($notifications)->last()['title'] ?? null)->toBe('Backup items added'); - expect(collect($notifications)->last()['status'] ?? null)->toBe('success'); -}); + $policyIds = $policies + ->pluck('id') + ->map(fn (mixed $value): int => (int) $value) + ->sort() + ->values() + ->all(); -test('policy picker table does not warn if failures already existed but did not increase', function () { - $tenant = Tenant::factory()->create(); - $tenant->makeCurrent(); - - $user = User::factory()->create(); - - $backupSet = BackupSet::factory()->create([ - 'tenant_id' => $tenant->id, - 'name' => 'Test backup', - 'status' => 'partial', - 'metadata' => [ - 'failures' => [ - ['policy_id' => 1, 'reason' => 'Previous failure', 'status' => 500], - ], + $key = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'backup_set.add_policies', + targetId: (string) $backupSet->getKey(), + context: [ + 'policy_ids' => $policyIds, + 'include_assignments' => true, + 'include_scope_tags' => true, + 'include_foundations' => true, ], - ]); + ); - $policies = Policy::factory()->count(1)->create([ - 'tenant_id' => $tenant->id, - 'ignored_at' => null, - 'last_synced_at' => now(), - ]); + $run = BulkOperationRun::query() + ->where('tenant_id', $tenant->id) + ->where('resource', 'backup_set') + ->where('action', 'add_policies') + ->where('idempotency_key', $key) + ->latest('id') + ->first(); - $this->mock(BackupService::class, function (MockInterface $mock) use ($backupSet) { - $mock->shouldReceive('addPoliciesToSet') - ->once() - ->andReturn($backupSet); - }); - - Livewire::actingAs($user) - ->test(BackupSetPolicyPickerTable::class, [ - 'backupSetId' => $backupSet->id, - ]) - ->callTableBulkAction('add_selected_to_backup_set', $policies) - ->assertHasNoTableBulkActionErrors(); + expect($run)->not->toBeNull(); + expect($run?->status)->toBe('pending'); + expect($run?->total_items)->toBe(count($policyIds)); + expect($run?->item_ids['backup_set_id'] ?? null)->toBe($backupSet->getKey()); + expect($run?->item_ids['policy_ids'] ?? null)->toBe($policyIds); + expect($run?->item_ids['options']['include_foundations'] ?? null)->toBeTrue(); $notifications = session('filament.notifications', []); expect($notifications)->not->toBeEmpty(); - expect(collect($notifications)->last()['title'] ?? null)->toBe('Backup items added'); - expect(collect($notifications)->last()['status'] ?? null)->toBe('success'); + expect(collect($notifications)->last()['title'] ?? null)->toBe('Backup items queued'); }); -test('policy picker table warns when new failures were added', function () { - $tenant = Tenant::factory()->create(); - $tenant->makeCurrent(); +test('policy picker table reuses an active run on double click (idempotency)', function () { + Queue::fake(); - $user = User::factory()->create(); + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->id, + 'name' => 'Test backup', + ]); + + $policies = Policy::factory()->count(2)->create([ + 'tenant_id' => $tenant->id, + 'ignored_at' => null, + 'last_synced_at' => now(), + ]); + + $policyIds = $policies + ->pluck('id') + ->map(fn (mixed $value): int => (int) $value) + ->sort() + ->values() + ->all(); + + $key = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'backup_set.add_policies', + targetId: (string) $backupSet->getKey(), + context: [ + 'policy_ids' => $policyIds, + 'include_assignments' => true, + 'include_scope_tags' => true, + 'include_foundations' => true, + ], + ); + + Livewire::actingAs($user) + ->test(BackupSetPolicyPickerTable::class, [ + 'backupSetId' => $backupSet->id, + ]) + ->callTableBulkAction('add_selected_to_backup_set', $policies); + + Livewire::actingAs($user) + ->test(BackupSetPolicyPickerTable::class, [ + 'backupSetId' => $backupSet->id, + ]) + ->callTableBulkAction('add_selected_to_backup_set', $policies); + + expect(BulkOperationRun::query() + ->where('tenant_id', $tenant->id) + ->where('idempotency_key', $key) + ->count())->toBe(1); + + Queue::assertPushed(AddPoliciesToBackupSetJob::class, 1); +}); + +test('policy picker table forbids readonly users from starting add policies (403)', function () { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); $backupSet = BackupSet::factory()->create([ 'tenant_id' => $tenant->id, 'name' => 'Test backup', - 'status' => 'completed', - 'metadata' => ['failures' => []], ]); $policies = Policy::factory()->count(1)->create([ @@ -123,35 +172,70 @@ 'last_synced_at' => now(), ]); - $this->mock(BackupService::class, function (MockInterface $mock) use ($backupSet) { - $mock->shouldReceive('addPoliciesToSet') - ->once() - ->andReturnUsing(function () use ($backupSet) { - $backupSet->update([ - 'status' => 'partial', - 'metadata' => [ - 'failures' => [ - ['policy_id' => 123, 'reason' => 'New failure', 'status' => 500], - ], - ], - ]); + $thrown = null; - return $backupSet->refresh(); - }); - }); + try { + Livewire::actingAs($user) + ->test(BackupSetPolicyPickerTable::class, [ + 'backupSetId' => $backupSet->id, + ]) + ->callTableBulkAction('add_selected_to_backup_set', $policies); + } catch (Throwable $exception) { + $thrown = $exception; + } - Livewire::actingAs($user) - ->test(BackupSetPolicyPickerTable::class, [ - 'backupSetId' => $backupSet->id, - ]) - ->callTableBulkAction('add_selected_to_backup_set', $policies) - ->assertHasNoTableBulkActionErrors(); + expect($thrown)->not->toBeNull(); - $notifications = session('filament.notifications', []); + Queue::assertNothingPushed(); - expect($notifications)->not->toBeEmpty(); - expect(collect($notifications)->last()['title'] ?? null)->toBe('Backup items added with failures'); - expect(collect($notifications)->last()['status'] ?? null)->toBe('warning'); + expect(BulkOperationRun::query()->where('tenant_id', $tenant->id)->exists())->toBeFalse(); +}); + +test('policy picker table rejects cross-tenant starts (403) with no run records created', function () { + Queue::fake(); + + $tenantA = Tenant::factory()->create(); + $tenantB = Tenant::factory()->create(); + + $user = User::factory()->create(); + $user->tenants()->syncWithoutDetaching([ + $tenantA->getKey() => ['role' => 'owner'], + $tenantB->getKey() => ['role' => 'owner'], + ]); + + $this->actingAs($user); + + $tenantA->makeCurrent(); + Filament::setTenant($tenantA, true); + + $backupSetB = BackupSet::factory()->create([ + 'tenant_id' => $tenantB->id, + 'name' => 'Tenant B backup', + ]); + + $policiesB = Policy::factory()->count(1)->create([ + 'tenant_id' => $tenantB->id, + 'ignored_at' => null, + 'last_synced_at' => now(), + ]); + + $thrown = null; + + try { + Livewire::actingAs($user) + ->test(BackupSetPolicyPickerTable::class, [ + 'backupSetId' => $backupSetB->id, + ]) + ->callTableBulkAction('add_selected_to_backup_set', $policiesB); + } catch (Throwable $exception) { + $thrown = $exception; + } + + expect($thrown)->not->toBeNull(); + + Queue::assertNothingPushed(); + + expect(BulkOperationRun::query()->where('tenant_id', $tenantB->id)->exists())->toBeFalse(); }); test('policy picker table can filter by has versions', function () { -- 2.45.2 From d245bc144319fab0edd99fdd75ad8336f95de66d Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Thu, 15 Jan 2026 23:18:03 +0100 Subject: [PATCH 3/3] feat(052): finalize async add policies --- app/Jobs/AddPoliciesToBackupSetJob.php | 740 +++++++++++--------- app/Services/BulkOperationService.php | 6 +- specs/052-async-add-policies/tasks.md | 40 +- tests/Unit/BulkOperationRunProgressTest.php | 30 + 4 files changed, 465 insertions(+), 351 deletions(-) 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'); +}); -- 2.45.2