ahmido
060a82a1ed
Feature 048: Backup/Restore UI Graph-Safety (Phase 1) Dieses PR entfernt Microsoft Graph Calls aus UI-Renderpfaden (Filament/Livewire mount/render/options/typeahead/labels) in den kritischen Backup/Restore Screens und fügt Fail-Hard Guard Tests hinzu, die regressionssicher verhindern, dass UI-Rendering wieder Graph aufruft. ⸻ Motivation Backup/Restore UI wurde teilweise “fragil”, weil UI-Komponenten (z.B. Group Typeahead/Option Labels) Graph/Entra direkt beim Rendern triggern konnten. Das führt zu: • langsamen/unstabilen Seiten (429/Timeout/Permissions) • schwer reproduzierbaren UI-Fehlern im MSP-Scale • unnötiger Kopplung von “Page render” an Graph-Verfügbarkeit Ziel: UI muss DB-only rendern; Graph darf nur in Jobs/Run-Execution stattfinden. ⸻ Scope / Changes 1) Restore Wizard: Entfernt Graph-Typeahead & Label-Resolution • Group Mapping ist jetzt DB-only: • manuelle GUID Eingabe / Skip • GUID Validation • Helper Text, wo die Object ID zu finden ist • Keine Graph calls mehr in options() / getOptionLabelUsing() / typeahead beim Rendern. 2) Fail-Hard Guard Tests (Graph-Safety) • Neue Test-Infrastruktur: FailHardGraphClient (GraphClientInterface darf nicht aufgerufen werden) • Guard Tests als Pest Feature Tests (HTTP GET): • Backup Sets Index rendert mit fail-hard Graph client • Restore Wizard Route rendert mit fail-hard Graph client • Assertions: • 200 OK • plus stable UI marker string • Masking/Fallback Format ist deterministisch: Unresolved (…<last8>) 3) Spec/Plan/Tasks/Checklist • Spec 048 aktualisiert, Tasks abgehakt • requirements.md Checklist Gate: PASS ⸻ Out of Scope / Non-Goals • Kein Umbau der “Execution”-Actions zu Jobs (Capture snapshot, Restore rerun, Dry-Run execution etc.) → eigener Folge-Spec (Phase 2). • Keine Entra Group Name Resolution (separates Group-Inventory/Cache Feature). • Keine neuen Tabellen/Migrations in Phase 1. ⸻ How to verify (manual) Mit absichtlich kaputtem Tenant/Auth (Graph failt): 1. Öffne Backups & Restore → Backup Sets ✅ muss laden (UI render DB-only) 2. Öffne Restore Runs → Create Restore Run (Wizard) ✅ muss laden, kein Group-Typeahead mehr 3. Starte eine Restore Operation ❌ darf fehlschlagen (Graph kaputt) – wichtig ist: Render bleibt stabil, Run zeigt Fehler sauber pro Item. ⸻ Tests / Validation Executed: • ./vendor/bin/pint --dirty ✅ • ./vendor/bin/sail artisan test tests/Feature/Filament/BackupSetGraphSafetyTest.php tests/Feature/Filament/RestoreWizardGraphSafetyTest.php ✅ • (optional) Combined targeted suite ✅ ⸻ Notes • This PR intentionally focuses on UI Graph-Safety only. • Any future reintroduction of Graph search/typeahead in UI must go through contracts first and be executed asynchronously, never in UI render paths. Co-authored-by: Ahmed Darrazi <ahmeddarrazi@adsmac.local> Reviewed-on: #55
151 lines
8.0 KiB
Markdown
151 lines
8.0 KiB
Markdown
# Tasks: Backup/Restore UI Graph-Safety (Phase 1)
|
||
|
||
**Input**: Design documents from `/specs/048-backup-restore-ui-graph-safety/`
|
||
**Prerequisites**: plan.md (required), spec.md (required), research.md, data-model.md, contracts/, quickstart.md
|
||
|
||
**Tests**: REQUIRED (Pest). This feature explicitly adds guard tests (FR-004).
|
||
|
||
**Organization**: Tasks are grouped by user story to enable independent implementation and testing.
|
||
|
||
## Format: `- [ ] T### [P?] [US#?] Description with file path`
|
||
|
||
- **[P]**: Can run in parallel (different files, no dependencies)
|
||
- **[US#]**: Which user story this task belongs to (US1, US2)
|
||
- Include exact file paths in descriptions
|
||
|
||
---
|
||
|
||
## Phase 1: Setup (Shared Infrastructure)
|
||
|
||
**Purpose**: Confirm scope, lock stable UI markers as concrete strings, and ensure the contracts/quickstart reflect the intended test approach.
|
||
- [x] T001 Confirm tenant-scoped admin URLs for target pages in specs/048-backup-restore-ui-graph-safety/contracts/admin-pages.openapi.yaml
|
||
- [x] T002 Lock stable marker strings and record them in specs/048-backup-restore-ui-graph-safety/quickstart.md:
|
||
- Backup Sets index marker: `Created by`
|
||
- Restore wizard create marker: `Create restore run` (and wizard step: `Select Backup Set`)
|
||
|
||
---
|
||
|
||
## Phase 2: Foundational (Blocking Prerequisites)
|
||
|
||
**Purpose**: Shared test helpers and a clear boundary that fails-hard when Graph is touched.
|
||
|
||
**⚠️ CRITICAL**: No user story work should be considered “done” unless the fail-hard Graph binding is used in the story’s feature tests.
|
||
- [x] T003 [P] Create a fail-hard Graph client test double in tests/Support/FailHardGraphClient.php (implements App\\Services\\Graph\\GraphClientInterface and throws on any method)
|
||
- [x] T004 Add a reusable binding helper for tests (either a helper function in tests/Pest.php or a trait in tests/Support/) that binds GraphClientInterface to FailHardGraphClient
|
||
|
||
**Checkpoint**: Foundation ready — both page-render tests can now be implemented.
|
||
|
||
---
|
||
|
||
## Phase 3: User Story 1 — Backup Sets index renders Graph-free (Priority: P1) 🎯 MVP
|
||
|
||
**Goal**: As a tenant admin, I can open the Backup Sets index page even when Graph is disabled.
|
||
|
||
**Independent Test**: A Pest feature test does an HTTP GET to the tenant-scoped Filament Backup Sets index route and asserts assertOk() + assertSee('Created by') — with Graph bound to fail-hard.
|
||
|
||
### Tests
|
||
- [x] T005 [P] [US1] Add Pest feature test in tests/Feature/Filament/BackupSetGraphSafetyTest.php:
|
||
- bind GraphClientInterface to FailHardGraphClient (fail-hard on ANY invocation)
|
||
- HTTP GET App\\Filament\\Resources\\BackupSetResource::getUrl('index', tenant: $tenant)
|
||
- assertOk() + assertSee('Created by')
|
||
- [x] T006 [US1] In tests/Feature/Filament/BackupSetGraphSafetyTest.php, add tenant isolation assertions (second tenant data must not render) while still using fail-hard Graph binding
|
||
|
||
### Implementation
|
||
|
||
- [x] T007 [US1] Audit Backup Sets render path for any Graph usage and refactor to DB-only if needed in app/Filament/Resources/BackupSetResource.php and app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php
|
||
|
||
**Checkpoint**: Backup Sets index renders (assertOk() + assertSee('Created by')) with fail-hard Graph binding.
|
||
|
||
---
|
||
|
||
## Phase 4: User Story 2 — Restore wizard renders Graph-free + DB-only group mapping (Priority: P1)
|
||
|
||
**Goal**: As a tenant admin, I can open the Restore wizard page and interact with group mapping without any Graph calls in render/mount/options/typeahead.
|
||
|
||
**Independent Test**: Pest feature tests that (a) render the Restore wizard create page without Graph, and (b) render the group mapping section (using query params to preselect a backup set with group assignments) and verify fallback labels use `…<last8>`.
|
||
|
||
### Tests
|
||
- [x] T008 [P] [US2] Add Pest feature test in tests/Feature/Filament/RestoreWizardGraphSafetyTest.php:
|
||
- bind GraphClientInterface to FailHardGraphClient (fail-hard on ANY invocation)
|
||
- HTTP GET App\\Filament\\Resources\\RestoreRunResource::getUrl('create', tenant: $tenant)
|
||
- assertOk() + assertSee('Create restore run') + assertSee('Select Backup Set')
|
||
- [x] T009 [P] [US2] Extend tests/Feature/Filament/RestoreWizardGraphSafetyTest.php (or a second file):
|
||
- seed a BackupSet + BackupItem with group assignment targets (groupId present)
|
||
- HTTP GET create URL with `?backup_set_id=` (and optional `backup_item_ids`/`scope_mode`) to force group mapping render
|
||
- keep fail-hard Graph binding (no Graph/typeahead/label resolution allowed)
|
||
- [x] T010 [US2] In the group mapping render test, assert all DB-only UX requirements:
|
||
- assertSee('…<last8>') masked fallback appears for source labels
|
||
- assertSee('Paste the target Entra ID group Object ID') helper text appears
|
||
- assertSee('Use SKIP to omit the assignment.') helper text appears
|
||
|
||
### Implementation
|
||
- [x] T011 [US2] Remove Graph-dependent typeahead/search from group mapping controls in app/Filament/Resources/RestoreRunResource.php (no Graph/typeahead; remove getSearchResultsUsing paths)
|
||
- [x] T012 [US2] Remove Graph-dependent option label resolution in app/Filament/Resources/RestoreRunResource.php (no Graph label resolution; remove getOptionLabelUsing paths)
|
||
- [x] T013 [US2] Implement DB-only group mapping UX in app/Filament/Resources/RestoreRunResource.php:
|
||
- manual target group objectId input (GUID/UUID)
|
||
- GUID validation (if not SKIP)
|
||
- helper text: “Paste the target Entra ID group Object ID (GUID). Names are not resolved in this phase.” + “Use SKIP to omit the assignment.”
|
||
- no Graph/typeahead
|
||
- [x] T014 [US2] Make unresolved group detection DB-only in app/Filament/Resources/RestoreRunResource.php (remove GroupResolver usage from unresolvedGroups() and any other render-time helpers)
|
||
- [x] T015 [US2] Implement masked fallback label formatting `…<last8>` in app/Filament/Resources/RestoreRunResource.php (update formatGroupLabel() and ensure all source labels route through it)
|
||
- [x] T016 [US2] Remove now-unused methods/imports after refactor (e.g., targetGroupOptions(), resolveTargetGroupLabel(), GroupResolver import) in app/Filament/Resources/RestoreRunResource.php
|
||
|
||
**Checkpoint**: Restore wizard renders (assertOk() + assertSee('Create restore run') + assertSee('Select Backup Set')) and group mapping renders DB-only; tests pass with fail-hard Graph binding.
|
||
|
||
---
|
||
|
||
## Phase 5: Polish & Cross-Cutting Concerns
|
||
|
||
**Purpose**: Keep docs and tooling aligned with the guardrail.
|
||
- [x] T017 [P] Update specs/048-backup-restore-ui-graph-safety/quickstart.md with the final test file names and the exact `artisan test --filter=...` / file commands
|
||
- [x] T018 [P] Update specs/048-backup-restore-ui-graph-safety/contracts/admin-pages.openapi.yaml if any page paths/markers changed during implementation
|
||
- [x] T019 Run formatting (./vendor/bin/pint --dirty) and targeted tests (./vendor/bin/sail artisan test --filter=graph\-safety or the exact files)
|
||
|
||
---
|
||
|
||
## Dependencies & Execution Order
|
||
|
||
### Phase Dependencies
|
||
|
||
- **Setup (Phase 1)**: No dependencies
|
||
- **Foundational (Phase 2)**: Depends on Setup completion — BLOCKS both user stories
|
||
- **US1 + US2 (Phases 3–4)**: Depend on Foundational completion; can proceed in parallel
|
||
- **Polish (Phase 5)**: Depends on desired user stories being complete
|
||
|
||
### User Story Dependencies
|
||
|
||
- **US1 (P1)**: Depends on T003–T004; otherwise independent
|
||
- **US2 (P1)**: Depends on T003–T004; otherwise independent
|
||
|
||
### Parallel Opportunities
|
||
|
||
- T003 and T004 can be developed in parallel if split across files.
|
||
- US1 test work (T005–T006) and US2 test work (T008–T010) can be developed in parallel.
|
||
|
||
---
|
||
|
||
## Parallel Example
|
||
|
||
```bash
|
||
# Parallelizable work after Phase 2:
|
||
# - US1: tests/Feature/Filament/BackupSetGraphSafetyTest.php
|
||
# - US2: tests/Feature/Filament/RestoreWizardGraphSafetyTest.php
|
||
# - Shared helper: tests/Support/FailHardGraphClient.php
|
||
```
|
||
|
||
---
|
||
|
||
## Implementation Strategy
|
||
|
||
### MVP Scope (Recommended)
|
||
|
||
1. Phase 1–2 (setup + fail-hard Graph test binding)
|
||
2. Phase 3 (US1: Backup Sets index renders Graph-free)
|
||
3. Validate tests + stop
|
||
|
||
### Then
|
||
|
||
4. Phase 4 (US2: Restore wizard + group mapping Graph-free)
|
||
5. Phase 5 polish
|
||
|