## 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
89 lines
5.7 KiB
Markdown
89 lines
5.7 KiB
Markdown
# Feature Specification: Remove Legacy Graph Options
|
|
|
|
**Feature Branch**: `088-remove-tenant-graphoptions-legacy`
|
|
**Created**: 2026-02-11
|
|
**Status**: Draft
|
|
**Input**: Remove the legacy tenant-based Graph options path and make provider-based resolution the only source of Graph configuration.
|
|
|
|
## Clarifications
|
|
|
|
### Session 2026-02-11
|
|
|
|
- Q: How should `Tenant::graphOptions()` be made unusable (kill-switch)? → A: Keep the method, but make it throw a clear exception so any remaining call sites fail fast in CI/runtime.
|
|
- Q: Should we drop legacy tenant credential DB fields as part of this feature? → A: No — defer DB schema cleanup to a follow-up feature.
|
|
- Q: Which pattern should the CI guardrail block? → A: Block any call to the deprecated tenant accessor (`$tenant->graphOptions()` and `Tenant::graphOptions()`), but do not block other unrelated `graphOptions(...)` methods.
|
|
- Q: Should the CI guardrail scan only production code, or also tests? → A: Scan `app/` only.
|
|
|
|
## User Scenarios & Testing *(mandatory)*
|
|
|
|
### User Story 1 - Safe Refactors via Single Credential Source (Priority: P1)
|
|
|
|
As a maintainer, I want all Microsoft Graph configuration to come from a single, canonical provider-based source, so refactors do not accidentally mix credential sources or introduce inconsistent authentication behavior.
|
|
|
|
**Why this priority**: This removes a major source of drift and breakage, making changes safer and predictable.
|
|
|
|
**Independent Test**: Can be fully tested by running the automated test suite and verifying that Graph configuration is resolved only via the canonical provider connection, with no tenant-stored credential fallback.
|
|
|
|
**Acceptance Scenarios**:
|
|
|
|
1. **Given** a tenant with a valid provider connection configured, **When** any Graph-enabled flow starts, **Then** it uses only the provider connection-derived configuration.
|
|
2. **Given** a tenant without a configured provider connection, **When** a Graph-enabled flow starts, **Then** the flow fails deterministically without using any tenant-stored credentials.
|
|
|
|
---
|
|
|
|
### User Story 2 - Prevent Reintroduction of Legacy Paths (Priority: P2)
|
|
|
|
As a maintainer, I want an automated guardrail that fails CI if the deprecated tenant-based Graph options accessor is reintroduced, so the codebase cannot regress silently.
|
|
|
|
**Why this priority**: Prevents recurring tech debt and ensures the hard cut remains enforced.
|
|
|
|
**Independent Test**: Can be fully tested by introducing a deliberate reference in a sandbox change and confirming CI fails.
|
|
|
|
**Acceptance Scenarios**:
|
|
|
|
1. **Given** a change that reintroduces a usage of the deprecated tenant-based Graph options accessor, **When** tests run in CI, **Then** the pipeline fails with a clear message describing the forbidden pattern.
|
|
|
|
---
|
|
|
|
### Edge Cases
|
|
|
|
- Tenants that previously relied on tenant-stored credentials must not silently keep working; they should fail clearly until a provider connection is configured.
|
|
- Flows that run non-interactively (jobs/scheduled work) must behave consistently with interactive flows: no alternate credential source.
|
|
|
|
## Requirements *(mandatory)*
|
|
|
|
### Assumptions
|
|
|
|
- The canonical source of truth for Microsoft Graph configuration is the provider connection resolved for the tenant (default provider key: `microsoft`).
|
|
- No user-facing navigation or UI changes are required.
|
|
- Provider connection resolution semantics remain unchanged.
|
|
- Dropping/altering deprecated tenant credential fields in the database is out of scope for this feature.
|
|
|
|
### Definitions
|
|
|
|
- **Provider configuration is “missing or invalid”** when `ProviderConnectionResolver::resolveDefault($tenant, 'microsoft')` results in a blocked resolution (e.g. missing default connection, disabled/needs consent, missing/invalid credentials).
|
|
- Downstream Microsoft Graph authentication / HTTP failures are still treated as runtime Graph errors; this feature only removes the legacy tenant-credential configuration path and standardizes option sourcing.
|
|
|
|
### Out of Scope
|
|
|
|
- Any database schema changes to remove deprecated tenant credential fields (follow-up feature).
|
|
|
|
### Functional Requirements
|
|
|
|
- **FR-001**: The system MUST not use the deprecated tenant-based Graph options accessor anywhere under `app/`.
|
|
- **FR-002**: The system MUST derive Microsoft Graph configuration exclusively from the canonical provider-based resolution for the tenant.
|
|
- **FR-003**: The system MUST NOT fallback to tenant-stored credentials if provider-based configuration is missing or invalid.
|
|
- **FR-004**: If provider-based configuration is missing or invalid (see Definitions), the system MUST fail fast with a clear, actionable error indicating that provider configuration is required.
|
|
- **FR-005**: The system MUST include an automated guardrail that fails CI if the deprecated tenant-based Graph options accessor is referenced in application code (instance or static): `$tenant->graphOptions()` or `Tenant::graphOptions()`.
|
|
- **FR-005a**: The guardrail MUST scan `app/` (production code) and SHOULD NOT fail due to references in `tests/`.
|
|
- **FR-006**: The deprecated tenant-based Graph options accessor MUST throw a clear exception if called (kill-switch), and MUST NOT attempt to resolve credentials from tenant-stored fields.
|
|
|
|
## Success Criteria *(mandatory)*
|
|
|
|
### Measurable Outcomes
|
|
|
|
- **SC-001**: CI includes a guardrail that reports 0 allowed references to the deprecated tenant-based Graph options accessor.
|
|
- **SC-002**: 100% of Graph-enabled flows use provider-based resolution as their configuration source.
|
|
- **SC-003**: The existing automated test suite passes without disabling tests or reducing coverage.
|
|
- **SC-004**: When provider configuration is missing, affected flows fail with a consistent, actionable error (no silent fallback behavior).
|