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
106 lines
4.4 KiB
Markdown
106 lines
4.4 KiB
Markdown
|
|
# Implementation Plan: Remove Legacy Tenant Graph Options
|
|
|
|
**Branch**: `088-remove-tenant-graphoptions-legacy` | **Date**: 2026-02-11 | **Spec**: [spec.md](spec.md)
|
|
**Input**: Feature specification in [spec.md](spec.md)
|
|
|
|
## Summary
|
|
|
|
Hard cut removal of the deprecated tenant-based Graph options accessor. All Microsoft Graph configuration must be derived exclusively from a resolved `ProviderConnection` via `ProviderConnectionResolver::resolveDefault()` and `ProviderGateway::graphOptions()`. Add a CI guardrail scanning `app/` for `$tenant->graphOptions()` / `Tenant::graphOptions()` and implement a kill-switch by making `Tenant::graphOptions()` throw.
|
|
|
|
## Technical Context
|
|
|
|
**Language/Version**: PHP 8.4.15 (Laravel 12)
|
|
**Primary Dependencies**: Filament v5, Livewire v4, Pest v4
|
|
**Storage**: PostgreSQL (Sail locally)
|
|
**Testing**: Pest via `vendor/bin/sail artisan test --compact`
|
|
**Target Platform**: Containerized Linux deployment (Sail local, Dokploy staging/prod)
|
|
**Project Type**: Laravel web application (monolith)
|
|
**Performance Goals**: N/A (internal refactor + CI guard)
|
|
**Constraints**:
|
|
- No fallback to tenant-stored credentials.
|
|
- Guardrail scans `app/` only (do not fail due to `tests/`).
|
|
- No new Graph endpoints; Graph calls remain routed through `GraphClientInterface`.
|
|
**Scale/Scope**: Repo-wide refactor of call sites in `app/Services/**`.
|
|
|
|
## Constitution Check
|
|
|
|
*GATE: Must pass before implementation. Re-check after design decisions.*
|
|
|
|
- Inventory-first / snapshots: **N/A** (no inventory/snapshot semantics change)
|
|
- Read/write separation: **PASS** (no new write surfaces; existing flows only refactored)
|
|
- Single contract path to Graph: **PASS** (Graph calls remain through `GraphClientInterface`; only options sourcing changes)
|
|
- Deterministic capabilities: **N/A** (no capability mapping changes)
|
|
- RBAC-UX / workspace isolation / tenant isolation: **N/A** (no routing or authorization semantic changes)
|
|
- Run observability: **N/A** (no new operations or run tracking changes)
|
|
- Data minimization / safe logging: **PASS** (no secrets introduced; provider credentials remain in credential store)
|
|
- Filament UI Action Surface Contract: **N/A** (no UI resources/pages are added or modified)
|
|
|
|
## Project Structure
|
|
|
|
### Documentation (this feature)
|
|
|
|
```text
|
|
specs/088-remove-tenant-graphoptions-legacy/
|
|
├── plan.md
|
|
├── research.md
|
|
├── data-model.md
|
|
├── quickstart.md
|
|
├── contracts/
|
|
└── tasks.md # created by /speckit.tasks (later)
|
|
```
|
|
|
|
### Source Code (repository root)
|
|
|
|
```text
|
|
app/
|
|
├── Models/
|
|
│ └── Tenant.php
|
|
├── Services/
|
|
│ ├── Providers/
|
|
│ │ ├── ProviderConnectionResolver.php
|
|
│ │ └── ProviderGateway.php
|
|
│ └── ... (multiple Graph-enabled services)
|
|
|
|
tests/
|
|
└── Feature/
|
|
└── Guards/
|
|
```
|
|
|
|
**Structure Decision**: Laravel monolith; refactor stays within existing services/models.
|
|
|
|
## Implementation Plan
|
|
|
|
### Phase 0 — Outline & Research (completed)
|
|
|
|
- Documented decisions and call sites in [research.md](research.md).
|
|
|
|
### Phase 1 — Design & Implementation (planned)
|
|
|
|
1. **Kill-switch the deprecated accessor**
|
|
- Update `Tenant::graphOptions()` in `app/Models/Tenant.php` to throw a clear exception explaining that provider connections are required.
|
|
|
|
2. **Refactor all `app/` call sites off `$tenant->graphOptions()`**
|
|
- For each call site, resolve the default Microsoft provider connection via `ProviderConnectionResolver::resolveDefault($tenant, 'microsoft')`.
|
|
- If resolution fails, fail fast with a clear, actionable error (include the effective reason code/message).
|
|
- Replace option building with `ProviderGateway::graphOptions($connection, $overrides)`.
|
|
- Update `app/Services/Intune/TenantConfigService.php` (currently returns `$tenant->graphOptions()`), so it no longer provides a tenant-based path.
|
|
|
|
3. **Add CI guardrail (Pest)**
|
|
- Add a new guard test under `tests/Feature/Guards/` that scans `app/` for forbidden patterns:
|
|
- `\$tenant->graphOptions(`
|
|
- `Tenant::graphOptions(`
|
|
- Ensure failure message explains how to fix (use provider connection resolution).
|
|
|
|
4. **Verification**
|
|
- Run the guard test and the most relevant subsets described in [quickstart.md](quickstart.md).
|
|
- Run `vendor/bin/sail bin pint --dirty`.
|
|
|
|
### Phase 2 — Tasks (next step)
|
|
|
|
- Completed: `tasks.md` has been generated and Phase 1 is broken into smaller, reviewable steps.
|
|
|
|
## Complexity Tracking
|
|
|
|
No constitution violations are required for this feature.
|