121 lines
6.5 KiB
Markdown
121 lines
6.5 KiB
Markdown
# Phase 0 — Research: Legacy Runs Removal (Spec 087)
|
||
|
||
**Branch**: `087-legacy-runs-removal`
|
||
**Date**: 2026-02-12
|
||
**Input Spec**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/087-legacy-runs-removal/spec.md`
|
||
|
||
## What “Legacy Runs” Means In This Repo
|
||
|
||
**Decision**: “Legacy runs” for this spec are the tenant-scoped run tables + UI surfaces that duplicate `operation_runs` for:
|
||
- Inventory sync
|
||
- Entra group sync
|
||
- Backup schedule execution
|
||
|
||
**Evidence (current repo state)**:
|
||
- Legacy tables exist:
|
||
- `inventory_sync_runs` ([database/migrations/2026_01_07_142719_create_inventory_sync_runs_table.php](../../database/migrations/2026_01_07_142719_create_inventory_sync_runs_table.php))
|
||
- `entra_group_sync_runs` ([database/migrations/2026_01_11_120004_create_entra_group_sync_runs_table.php](../../database/migrations/2026_01_11_120004_create_entra_group_sync_runs_table.php))
|
||
- `backup_schedule_runs` ([database/migrations/2026_01_05_011034_create_backup_schedule_runs_table.php](../../database/migrations/2026_01_05_011034_create_backup_schedule_runs_table.php))
|
||
- These legacy tables already have bridging columns to canonical runs (`operation_run_id` was added later), indicating an ongoing migration to `operation_runs`:
|
||
- [database/migrations/2026_02_10_090213_add_operation_run_id_to_inventory_sync_runs_table.php](../../database/migrations/2026_02_10_090213_add_operation_run_id_to_inventory_sync_runs_table.php)
|
||
- [database/migrations/2026_02_10_090214_add_operation_run_id_to_entra_group_sync_runs_table.php](../../database/migrations/2026_02_10_090214_add_operation_run_id_to_entra_group_sync_runs_table.php)
|
||
- [database/migrations/2026_02_10_090215_add_operation_run_id_to_backup_schedule_runs_table.php](../../database/migrations/2026_02_10_090215_add_operation_run_id_to_backup_schedule_runs_table.php)
|
||
|
||
## Canonical Run Types: Spec vs Current Code
|
||
|
||
### Problem
|
||
The spec mandates canonical `run_type` identifiers:
|
||
- `inventory_sync`
|
||
- `drift_generate_findings`
|
||
- `entra_group_sync`
|
||
- `backup_schedule_run`
|
||
- `backup_schedule_retention`
|
||
- `backup_schedule_purge`
|
||
|
||
But the current code uses dotted `OperationRunType` values such as:
|
||
- `inventory.sync`
|
||
- `directory_groups.sync`
|
||
- `drift.generate`
|
||
- `backup_schedule.run_now` / `backup_schedule.retry` / `backup_schedule.scheduled`
|
||
|
||
([app/Support/OperationRunType.php](../../app/Support/OperationRunType.php))
|
||
|
||
### Decision
|
||
Adopt the spec’s underscore identifiers as the canonical stored values going forward.
|
||
|
||
### Rationale
|
||
- The spec explicitly requires a standardized and enforced contract.
|
||
- Stored `type` values are used across UI and notifications; a single canonical scheme reduces “type sprawl.”
|
||
|
||
### Backwards Compatibility Plan (for existing rows)
|
||
- Provide a one-time data migration that rewrites existing `operation_runs.type` values from the old dotted values to the new underscore values *for the affected categories only*.
|
||
- Keep a compatibility mapping in code (temporary) only if needed to avoid breaking filters/UI during rollout.
|
||
|
||
### Alternatives Considered
|
||
- Keep dotted values and treat the spec’s list as “display labels.” Rejected because it violates FR-012’s explicit identifiers.
|
||
- Introduce a parallel “canonical_type” column. Rejected because it increases complexity and duplication.
|
||
|
||
## Drift + Inventory References (FK Cutover)
|
||
|
||
### Problem
|
||
Drift and inventory currently reference legacy run IDs:
|
||
- Findings: `baseline_run_id` / `current_run_id` are FKs to `inventory_sync_runs` ([database/migrations/2026_01_13_223311_create_findings_table.php](../../database/migrations/2026_01_13_223311_create_findings_table.php))
|
||
- Inventory items: `last_seen_run_id` references `inventory_sync_runs` ([database/migrations/2026_01_07_142720_create_inventory_items_table.php](../../database/migrations/2026_01_07_142720_create_inventory_items_table.php))
|
||
|
||
But inventory items already have the new canonical field:
|
||
- `last_seen_operation_run_id` ([database/migrations/2026_02_10_091433_add_last_seen_operation_run_id_to_inventory_items_table.php](../../database/migrations/2026_02_10_091433_add_last_seen_operation_run_id_to_inventory_items_table.php))
|
||
|
||
### Decision
|
||
Migrate drift + inventory references to `operation_runs` and drop the legacy FK columns.
|
||
|
||
### Migration Strategy
|
||
- Add new nullable columns:
|
||
- `findings.baseline_operation_run_id`
|
||
- `findings.current_operation_run_id`
|
||
- Backfill by joining through `inventory_sync_runs.operation_run_id` where available.
|
||
- Update app code to read/write the new fields.
|
||
- Drop old `*_run_id` FK columns.
|
||
- Only after that, drop the legacy tables.
|
||
|
||
### Rationale
|
||
- Required to drop legacy tables without breaking referential integrity.
|
||
- Aligns with spec acceptance scenario: “historical drift findings where no safe mapping exists” remain functional (references can be null).
|
||
|
||
## Legacy UI Surfaces Found (To Remove)
|
||
|
||
**Inventory sync runs**:
|
||
- [app/Filament/Resources/InventorySyncRunResource.php](../../app/Filament/Resources/InventorySyncRunResource.php)
|
||
|
||
**Entra group sync runs**:
|
||
- [app/Filament/Resources/EntraGroupSyncRunResource.php](../../app/Filament/Resources/EntraGroupSyncRunResource.php)
|
||
|
||
**Backup schedule run history** (RelationManager + modal):
|
||
- [app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleRunsRelationManager.php](../../app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleRunsRelationManager.php)
|
||
|
||
## Architecture Guard (NoLegacyRuns)
|
||
|
||
### Decision
|
||
Add a CI/architecture Pest test similar to existing “NoLegacy…” guards:
|
||
- [tests/Feature/Guards/NoLegacyBulkOperationsTest.php](../../tests/Feature/Guards/NoLegacyBulkOperationsTest.php)
|
||
|
||
### Guard Scope (per spec clarification)
|
||
Scan for forbidden tokens under:
|
||
- `/Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/`
|
||
- `/Users/ahmeddarrazi/Documents/projects/TenantAtlas/database/`
|
||
- `/Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/`
|
||
- `/Users/ahmeddarrazi/Documents/projects/TenantAtlas/routes/`
|
||
|
||
Exclude:
|
||
- `vendor/`, `storage/`, `specs/`, `spechistory/`, `references/`, `bootstrap/cache/`
|
||
|
||
### Initial Forbidden Tokens (starting list)
|
||
- `InventorySyncRun`, `inventory_sync_runs`, `InventorySyncRunResource`
|
||
- `EntraGroupSyncRun`, `entra_group_sync_runs`, `EntraGroupSyncRunResource`
|
||
- `BackupScheduleRun`, `backup_schedule_runs`, `BackupScheduleRunsRelationManager`
|
||
|
||
## Open Questions
|
||
None remaining for planning.
|
||
|
||
- “No backfill” is already clarified in the spec; the only data movement here is migrating *references* to existing `operation_runs`.
|
||
- Authorization semantics for canonical operations pages are clarified (404 vs 403) and must be preserved.
|