ahmido
5770c7b76b
Implements Spec 092 legacy purge. Key changes: - Remove legacy Inventory landing page + view; link Inventory entry directly to Inventory Items. - Update Drift landing copy to "operation runs"; remove URL heuristic from context bar. - Remove legacy redirect shim route and assert 404 for old bookmarks. - Staged job payload change: remove legacy ctor arg; keep legacy field for deserialization compatibility; new payload omits field. - Remove legacy notification artifact. - Remove legacy test shim + update tests; strengthen guard suite with scoped exception for job compat field. - Add spec/plan/tasks/checklist artifacts under specs/092-legacy-purge-final. Tests: - Focused Pest suite for guards, legacy routes, redirect behavior, job compatibility, drift copy. - Pint run: `vendor/bin/sail bin pint --dirty`. Notes: - Deploy B final removal of `backupScheduleRunId` should occur only after the compatibility window defined in the spec. Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #110
72 lines
4.2 KiB
Markdown
72 lines
4.2 KiB
Markdown
# Research — Legacy Purge (Runs / Routes / UI / Test Shims)
|
||
|
||
Date: 2026-02-14
|
||
|
||
This document resolves planning unknowns by grounding the spec in current repository reality (routes, views, tests, and job payload constraints).
|
||
|
||
## Decisions
|
||
|
||
### 1) Legacy deep links return `404 Not Found`
|
||
|
||
- Decision: Legacy run endpoints and legacy tenant-scoped redirect shims MUST return `404 Not Found` (no redirect).
|
||
- Rationale: The spec’s clarifications explicitly choose fail-fast behavior for old bookmarks, and removing redirects eliminates long-term maintenance of compatibility shims.
|
||
- Alternatives considered:
|
||
- Keep redirects indefinitely → rejected (ongoing maintenance + encourages legacy URLs).
|
||
- Redirect with warnings → rejected (still maintains shim surface).
|
||
|
||
Evidence in repo:
|
||
- Legacy run endpoints are already asserted as `404` in `tests/Feature/Operations/LegacyRunRoutesNotFoundTest.php`.
|
||
- A legacy operations list redirect shim exists in `routes/web.php` (`/admin/t/{tenant:external_id}/operations`) and is currently tested as `302` in `tests/Feature/078/TenantListRedirectTest.php`.
|
||
|
||
### 2) Context chrome must not infer tenant scope from URL prefixes
|
||
|
||
- Decision: Remove URL-prefix heuristics (e.g., `str_starts_with($path, '/admin/t/')`) and infer tenant scope from route parameters + workspace/tenant context helpers.
|
||
- Rationale: URL heuristics are brittle and conflict with the “single canonical mental model” objective.
|
||
- Alternatives considered:
|
||
- Keep URL-prefix checks as a fallback → rejected (per spec FR-006).
|
||
|
||
Evidence in repo:
|
||
- URL-prefix detection is currently present in `resources/views/filament/partials/context-bar.blade.php`.
|
||
|
||
### 3) Inventory entry point is the Inventory Items index (canonical)
|
||
|
||
- Decision: Remove redirect-only Inventory landing entry points and route Inventory navigation directly to Inventory Items.
|
||
- Rationale: Redirect-only pages create dead views and increase cognitive overhead; the spec clarifies Inventory Items as the canonical target.
|
||
- Alternatives considered:
|
||
- Keep landing page as a “hub” → rejected (not needed; creates dead/duplicate UI).
|
||
|
||
### 4) Drift copy uses canonical terminology “operation runs”
|
||
|
||
- Decision: Replace the legacy phrase “inventory sync runs” with “operation runs”.
|
||
- Rationale: Consolidates terminology around the canonical Operations mental model.
|
||
- Alternatives considered:
|
||
- “inventory sync operations” / “sync operations” → rejected (still perpetuates legacy specificity).
|
||
|
||
### 5) Guard scope and exclusions
|
||
|
||
- Decision: Add/extend guard tests scanning for removed legacy identifiers/patterns. Exclude `database/migrations/**`, `references/**`, and `docs/**`.
|
||
- Rationale: Migrations are immutable; references/docs are allowed to mention legacy for historical context.
|
||
- Alternatives considered:
|
||
- Scan entire repo including migrations → rejected (would fail on immutable history).
|
||
|
||
## Staged rollout rationale (queued job payload)
|
||
|
||
- Decision: Deliver the queued job payload shape change in two deployments:
|
||
- Deploy A: accept/ignore the legacy field while new dispatches stop sending it.
|
||
- Deploy B: remove the legacy field/property entirely once the queue is drained.
|
||
- Rationale: Laravel queue payloads are serialized; removing a property/constructor argument prematurely can break deserialization of previously queued jobs.
|
||
|
||
## Legacy hotspot inventory (confirmed current matches)
|
||
|
||
- `app/Jobs/RunBackupScheduleJob.php` (legacy queued payload field compatibility/purge target)
|
||
- `app/Notifications/BackupScheduleRunDispatchedNotification.php` (dead legacy notification target)
|
||
- `app/Filament/Pages/InventoryLanding.php` (redirect-only landing removal target)
|
||
- `resources/views/filament/partials/context-bar.blade.php` (URL-prefix heuristic removal target)
|
||
- `routes/web.php` (legacy redirect shim endpoint removal target)
|
||
- `tests/Pest.php` (legacy test shim bootstrap require removal target)
|
||
- `tests/Support/LegacyModels/InventorySyncRun.php` (legacy test shim file removal target)
|
||
|
||
## Open Questions
|
||
|
||
None identified that block Phase 1 design. Any newly discovered legacy identifiers during implementation should be added to the guard list only if they are truly “must be purged” (and should respect the exclusions).
|