ahmido
1acbf8cc54
## Summary - remove tenant-based Graph options access from runtime service paths and enforce provider-only resolution - add `MicrosoftGraphOptionsResolver` and `ProviderConfigurationRequiredException` for centralized, actionable provider-config errors - turn `Tenant::graphOptions()` into a fail-fast kill switch to prevent legacy runtime usage - add and update tests (including guardrail) to enforce no reintroduction in `app/` - update Spec 088 artifacts (`spec`, `plan`, `research`, `tasks`, checklist) ## Validation - `vendor/bin/sail bin pint --dirty` - `vendor/bin/sail artisan test --compact --filter=NoLegacyTenantGraphOptions` - `vendor/bin/sail artisan test --compact tests/Feature/Filament` - `CI=1 vendor/bin/sail artisan test --compact` ## Notes - Branch includes the guardrail test for legacy callsite detection in `app/`. - Full suite currently green: 1227 passed, 5 skipped. Co-authored-by: Ahmed Darrazi <ahmeddarrazi@MacBookPro.fritz.box> Reviewed-on: #105
129 lines
6.6 KiB
Markdown
129 lines
6.6 KiB
Markdown
---
|
||
|
||
description: "Task list for implementing Spec 088"
|
||
---
|
||
|
||
# Tasks: Remove Legacy Tenant Graph Options
|
||
|
||
**Input**: Design documents from `/specs/088-remove-tenant-graphoptions-legacy/`
|
||
**Prerequisites**: plan.md (required), spec.md (required), research.md, data-model.md, contracts/, quickstart.md
|
||
|
||
**Tests**: REQUIRED (Pest) — this feature changes runtime behavior and introduces a CI guardrail.
|
||
|
||
## Phase 1: Setup (Shared Infrastructure)
|
||
|
||
- [x] T001 Verify test + format tooling paths (composer.json, phpunit.xml, specs/088-remove-tenant-graphoptions-legacy/quickstart.md)
|
||
- [x] T002 [P] Add a focused test runner note to specs/088-remove-tenant-graphoptions-legacy/quickstart.md (include `--filter=NoLegacyTenantGraphOptions`)
|
||
|
||
---
|
||
|
||
## Phase 2: Foundational (Blocking Prerequisites)
|
||
|
||
**⚠️ CRITICAL**: No user story work can begin until this phase is complete.
|
||
|
||
- [x] T003 Create provider-only Graph options resolver in app/Services/Providers/MicrosoftGraphOptionsResolver.php
|
||
- [x] T004 [P] Create exception for missing/invalid provider config in app/Services/Providers/ProviderConfigurationRequiredException.php
|
||
- [x] T005 Wire resolver dependencies (ProviderConnectionResolver + ProviderGateway) in app/Providers/AppServiceProvider.php
|
||
- [x] T006 Add tests for resolver failure/success paths in tests/Feature/Providers/MicrosoftGraphOptionsResolverTest.php
|
||
- [x] T007 Implement kill-switch: make Tenant::graphOptions() throw in app/Models/Tenant.php
|
||
- [x] T008 Add test asserting kill-switch behavior in tests/Feature/Models/TenantGraphOptionsKillSwitchTest.php
|
||
|
||
**Checkpoint**: A single reusable provider-only resolver exists, with tests, and the legacy accessor fails fast.
|
||
|
||
---
|
||
|
||
## Phase 3: User Story 1 — Safe Refactors via Single Credential Source (Priority: P1) 🎯 MVP
|
||
|
||
**Goal**: All Graph-enabled flows use provider connection-derived configuration only.
|
||
|
||
**Independent Test**: `vendor/bin/sail artisan test --compact --filter=MicrosoftGraphOptionsResolverTest` + run a few refactored service tests (as applicable) and ensure no `app/` references remain.
|
||
|
||
### Implementation
|
||
|
||
- [x] T009 [P] [US1] Refactor graph options usage in app/Services/AssignmentRestoreService.php (replace `$tenant->graphOptions()` with resolver)
|
||
- [x] T010 [P] [US1] Refactor graph options usage in app/Services/AssignmentBackupService.php (replace `$tenant->graphOptions()` with resolver)
|
||
- [x] T011 [P] [US1] Refactor graph options usage in app/Services/Intune/RestoreRiskChecker.php (replace `$tenant->graphOptions()` with resolver)
|
||
- [x] T012 [P] [US1] Refactor graph options usage in app/Services/Intune/TenantPermissionService.php (derive `client_id` from provider-based options; no tenant fallback)
|
||
- [x] T013 [P] [US1] Refactor graph options usage in app/Services/Intune/VersionService.php (replace `$tenant->graphOptions()` with resolver)
|
||
- [x] T014 [P] [US1] Refactor graph options usage in app/Services/Intune/FoundationMappingService.php (replace array merge with provider-based options)
|
||
- [x] T015 [P] [US1] Refactor graph options usage in app/Services/Intune/PolicyCaptureOrchestrator.php (replace `$tenant->graphOptions()` with resolver)
|
||
- [x] T016 [P] [US1] Refactor graph options usage in app/Services/Intune/FoundationSnapshotService.php (replace `$tenant->graphOptions()` with resolver)
|
||
- [x] T017 [P] [US1] Refactor/remove legacy wrapper in app/Services/Intune/TenantConfigService.php (must not return `$tenant->graphOptions()`; use resolver)
|
||
- [x] T018 [P] [US1] Refactor context building in app/Services/Intune/ConfigurationPolicyTemplateResolver.php (replace `array_merge($tenant->graphOptions(), ...)` with provider-based context)
|
||
- [x] T019 [P] [US1] Refactor graph options usage in app/Services/Directory/EntraGroupSyncService.php (replace `$tenant->graphOptions()` with resolver)
|
||
- [x] T020 [P] [US1] Refactor graph options usage in app/Services/Directory/RoleDefinitionsSyncService.php (replace `$tenant->graphOptions()` with resolver)
|
||
- [x] T021 [P] [US1] Refactor options merge in app/Services/Graph/AssignmentFilterResolver.php (replace `array_merge($options, $tenant->graphOptions())` with provider-based options)
|
||
|
||
### Verification
|
||
|
||
- [x] T022 [US1] Run focused tests for this story: `vendor/bin/sail artisan test --compact --filter=MicrosoftGraphOptionsResolver`
|
||
- [x] T023 [US1] Run a repo scan to ensure no `app/` usage remains (confirm via tests/Feature/Guards/NoLegacyTenantGraphOptionsTest.php once added)
|
||
|
||
**Checkpoint**: `app/` contains zero tenant-based graph options usages, and runtime failures are provider-config actionable.
|
||
|
||
---
|
||
|
||
## Phase 4: User Story 2 — Prevent Reintroduction of Legacy Paths (Priority: P2)
|
||
|
||
**Goal**: CI fails if `$tenant->graphOptions()` or `Tenant::graphOptions()` appears anywhere under `app/`.
|
||
|
||
**Independent Test**: `vendor/bin/sail artisan test --compact --filter=NoLegacyTenantGraphOptions`.
|
||
|
||
### Tests (Guardrail)
|
||
|
||
- [x] T024 [P] [US2] Create guard test scanning `app/` in tests/Feature/Guards/NoLegacyTenantGraphOptionsTest.php
|
||
- [x] T025 [US2] Ensure guard failure message is actionable (points to ProviderConnectionResolver + ProviderGateway) in tests/Feature/Guards/NoLegacyTenantGraphOptionsTest.php
|
||
|
||
### Verification
|
||
|
||
- [x] T026 [US2] Run guard test: `vendor/bin/sail artisan test --compact --filter=NoLegacyTenantGraphOptions`
|
||
|
||
---
|
||
|
||
## Phase 5: Polish & Cross-Cutting Concerns
|
||
|
||
- [x] T027 [P] Run formatter: `vendor/bin/sail bin pint --dirty` (app/**, tests/**)
|
||
- [x] T028 Run full affected test folders: `vendor/bin/sail artisan test --compact tests/Feature/Guards`
|
||
- [x] T029 Run a broader suite if needed: `vendor/bin/sail artisan test --compact`
|
||
|
||
---
|
||
|
||
## Dependencies & Execution Order
|
||
|
||
### User Story Dependencies
|
||
|
||
- **US1 (P1)** depends on Phase 2 foundational tasks (resolver + kill-switch).
|
||
- **US2 (P2)** can be implemented after Phase 2, but should run after US1 refactor so the guard passes.
|
||
|
||
### Dependency Graph (high level)
|
||
|
||
- Phase 2 (T003–T008)
|
||
-→ US1 refactors (T009–T021)
|
||
-→ US1 verification (T022–T023)
|
||
-→ US2 guardrail (T024–T026)
|
||
-→ Polish (T027–T029)
|
||
|
||
## Parallel Execution Examples
|
||
|
||
### After Phase 2 is complete
|
||
|
||
These refactors are parallelizable because they touch different files:
|
||
- T009–T016, T018–T021 can run in parallel ([P]).
|
||
|
||
### US2 guardrail
|
||
|
||
- T024 can be done in parallel with late US1 refactors, but it will only pass once all US1 call sites are removed.
|
||
|
||
## Implementation Strategy
|
||
|
||
### MVP (US1)
|
||
|
||
1. Complete Phase 2 (resolver + kill-switch + tests).
|
||
2. Refactor all `app/` call sites (T009–T021).
|
||
3. Run focused tests (T022).
|
||
|
||
### Then lock it in (US2)
|
||
|
||
1. Add guardrail test (T024–T026).
|
||
2. Finish with formatting + regression checks (T027–T029).
|