ahmido
b35e3a6518
Summary Dieses PR führt Spec 048: Backup/Restore UI Graph-Safety ein. Ziel: Backup/Restore-Screens in Filament sollen stabil und skalierbar bleiben, indem keine Microsoft Graph Calls beim UI-Rendern (mount/render/options/typeahead/labels) stattfinden. Stattdessen: DB-only Render + klare Fallbacks + Guard-Tests, die jede versehentliche UI-Graph-Nutzung sofort brechen. ⸻ Motivation / Problem Im aktuellen Stand rutschen Graph Calls gerne in: • Restore Wizard Group-Typeahead / getOptionLabelUsing (Graph /groups) • “Preview/Dry-Run” oder Label-Resolution im UI-Request Das führt zu: • langsamen/fragilen Pages (429/Timeout/Permissions) • schwerer Debugbarkeit im MSP-Scale • unerwarteten Abhängigkeiten vom Graph in reinen UI-Views ⸻ Scope (Phase 1, MVP) In scope • UI Render DB-only: Keine Graph Calls in Filament Render-Pfaden (Backup + Restore) • Fallback Labels für unresolved IDs: • Format: Unresolved (…<last8>) • Group Mapping UX (DB-only): • manuelle GUID Eingabe • GUID Validation • Helper-Text, wo Admins die Object ID finden • kein Graph-Search/typeahead • Fail-hard Guard Tests als Pest Feature Tests (HTTP GET): • Render Backup Sets Index • Render Restore Wizard • Tests assert: 200 OK + stable marker string Out of scope • Job-Orchestration Refactor (Actions wie “Capture snapshot”, “Rerun restore”, “dry-run execution”) → separater Spec/Feature • Entra Group Name Resolution (Group Inventory / Cache) → separater Modul-Scope ⸻ Deliverables • spec.md, plan.md, tasks.md • checklists/requirements.md (constitution gate) • Klar definierte Marker-Regeln für Guard-Tests (statische Headings, keine dynamischen Row-Werte) ⸻ Success Criteria • Guard-Tests schlagen zuverlässig fehl, sobald ein UI-Render Pfad Graph aufruft. • Restore Wizard bleibt bedienbar ohne Graph-Typeahead (GUID manual entry + Validation). • Keine DB-Migrations in dieser Phase. ⸻ Next Step Nach Review/Merge dieses Specs: 1. Implementation gemäß tasks.md (Remove UI Graph calls + Guard Tests) 2. Targeted Tests + Pint 3. Erst danach optional: eigener Spec für “job-based orchestration” (queued preview/execution) Co-authored-by: Ahmed Darrazi <ahmeddarrazi@adsmac.local> Reviewed-on: #54
46 lines
2.5 KiB
Markdown
46 lines
2.5 KiB
Markdown
# Research: Backup/Restore UI Graph-Safety (048)
|
||
|
||
## Decision: Enforce Graph-safety via fail-hard feature renders
|
||
|
||
- **Decision**: Add Pest *feature* tests that `actingAs(...)` and `GET` Filament page URLs while binding `App\Services\Graph\GraphClientInterface` to a fail-hard implementation (throws on any call).
|
||
- **Rationale**: A full HTTP render exercises the real Filament request lifecycle (middleware, tenancy, resource/page boot) and will fail immediately if any UI render path touches Graph.
|
||
- **Alternatives considered**:
|
||
- Livewire component tests only → can miss route/middleware/tenancy boot and won’t reflect “real render” regressions as reliably.
|
||
- Binding Graph to `NullGraphClient` → would allow silent Graph usage to slip through.
|
||
|
||
## Decision: Use `Resource::getUrl()` for stable Filament routes (tenant-scoped)
|
||
|
||
- **Decision**: Use Filament’s URL helpers in tests:
|
||
- `BackupSetResource::getUrl('index', tenant: $tenant)`
|
||
- `RestoreRunResource::getUrl('create', tenant: $tenant)`
|
||
- **Rationale**: Avoids hardcoding route paths and keeps tests resilient to panel path / tenancy prefix changes.
|
||
- **Repo evidence**:
|
||
- `tests/Feature/Filament/InventorySyncRunResourceTest.php` uses `->get(InventorySyncRunResource::getUrl('index', tenant: $tenant))`.
|
||
- **Alternatives considered**:
|
||
- Hardcoded `/admin/t/{tenant}/...` paths → brittle if the panel path or tenant prefix changes.
|
||
|
||
## Decision: Assert HTTP 200 + stable marker
|
||
|
||
- **Decision**: Guard tests assert `->assertOk()` plus a stable marker string per page.
|
||
- **Rationale**: Reduces false positives (e.g., a redirect to login) and makes failures easier to diagnose.
|
||
- **Alternatives considered**:
|
||
- Status-only (200) → may still pass with empty/partial output or wrong page.
|
||
|
||
## Decision: Fallback label masking format
|
||
|
||
- **Decision**: Mask unresolved external IDs as `…<last8>` (last 8 characters, prefixed with ellipsis).
|
||
- **Rationale**: Keeps UI readable while avoiding full identifier disclosure.
|
||
- **Alternatives considered**:
|
||
- Full ID → increases accidental disclosure.
|
||
- Hash → less readable for operators.
|
||
|
||
## Decision: Filament panel + tenancy routing assumptions
|
||
|
||
- **Decision**: Treat the Filament admin panel as tenant-scoped under the configured path/prefix:
|
||
- Panel path: `admin`
|
||
- Tenant route prefix: `t`
|
||
- Tenant slug attribute: `external_id`
|
||
- **Rationale**: This is the repo’s current canonical setup (`App\Providers\Filament\AdminPanelProvider`).
|
||
- **Alternatives considered**:
|
||
- Non-tenant-scoped pages → not applicable (TenantPilot is tenant-first).
|