ahmido/TenantAtlas
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects 1 Releases Wiki Activity
558b5d3807
TenantAtlas/specs/087-legacy-runs-removal/research.md
ahmido d6e7de597a feat(spec-087): remove legacy runs (#106) 
Implements Spec 087: Legacy Runs Removal (rigorous).

### What changed
- Canonicalized run history: **`operation_runs` is the only run system** for inventory sync, Entra group sync, backup schedule execution/retention/purge.
- Removed legacy UI surfaces (Filament Resources / relation managers) for legacy run models.
- Legacy run URLs now return **404** (no redirects), with RBAC semantics preserved (404 vs 403 as specified).
- Canonicalized affected `operation_runs.type` values (dotted → underscore) via migration.
- Drift + inventory references now point to canonical operation runs; includes backfills and then drops legacy FK columns.
- Drops legacy run tables after cutover.
- Added regression guards to prevent reintroducing legacy run tokens or “backfilling” canonical runs from legacy tables.

### Migrations
- `2026_02_12_000001..000006_*` canonicalize types, add/backfill operation_run_id references, drop legacy columns, and drop legacy run tables.

### Tests
Focused pack for this spec passed:
- `tests/Feature/Guards/NoLegacyRunsTest.php`
- `tests/Feature/Guards/NoLegacyRunBackfillTest.php`
- `tests/Feature/Operations/LegacyRunRoutesNotFoundTest.php`
- `tests/Feature/Monitoring/MonitoringOperationsTest.php`
- `tests/Feature/Jobs/RunInventorySyncJobTest.php`

### Notes / impact
- Destructive cleanup is handled via migrations (drops legacy tables) after code cutover; deploy should run migrations in the same release.

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #106
2026-02-12 12:40:51 +00:00

121 lines
6.5 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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.
Reference in New Issue View Git Blame Copy Permalink