From d67e2c84bc989afb30b5a663834dca1eebee3e76 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Sun, 22 Feb 2026 15:18:26 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20resolve=205=20consistency=20issues=20fro?= =?UTF-8?q?m=20project=20analysis=20(F1=E2=80=93F7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: Replace 'EnsureActiveWorkspace' with 'EnsureWorkspaceSelected' in spec (3 occurrences) — aligns with R1 decision F2: Add audit logging to SwitchWorkspaceController (T036) + context_bar reason — closes FR-005 gap F3: Remove stale WorkspaceContext.php MODIFY from plan project structure F4/F5: Add WorkspaceRedirectResolver.php + 2 missing test files to plan project structure F6: Add single-workspace sub-case (EC3) note to T030 F7: Add archived workspace scenario (EC2) note to T024 --- specs/107-workspace-chooser/contracts/routes.md | 2 +- specs/107-workspace-chooser/data-model.md | 2 +- specs/107-workspace-chooser/plan.md | 6 ++++-- specs/107-workspace-chooser/spec.md | 7 ++++--- specs/107-workspace-chooser/tasks.md | 6 +++--- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/specs/107-workspace-chooser/contracts/routes.md b/specs/107-workspace-chooser/contracts/routes.md index 87ce94d..d01f1b9 100644 --- a/specs/107-workspace-chooser/contracts/routes.md +++ b/specs/107-workspace-chooser/contracts/routes.md @@ -35,7 +35,7 @@ ### `GET /admin/choose-workspace` (Filament Page: `ChooseWorkspace`) ### `POST /admin/switch-workspace` (named: `admin.switch-workspace`) -**Change**: No structural change. This route continues to serve the context-bar dropdown workspace switcher. In a future iteration, audit logging may be added here as well. +**Change**: Redirect logic replaced with `WorkspaceRedirectResolver`. Audit logging added via `WorkspaceAuditLogger::log()` — emits `workspace.selected` with reason `context_bar` to satisfy FR-005 (every workspace selection must be audited). **Controller**: `SwitchWorkspaceController` diff --git a/specs/107-workspace-chooser/data-model.md b/specs/107-workspace-chooser/data-model.md index bddad7e..9d1713d 100644 --- a/specs/107-workspace-chooser/data-model.md +++ b/specs/107-workspace-chooser/data-model.md @@ -69,7 +69,7 @@ ### Audit Log Metadata Schema (for workspace selection events) ```jsonc { "method": "auto" | "manual", - "reason": "single_membership" | "last_used" | "chooser", + "reason": "single_membership" | "last_used" | "chooser" | "context_bar", "prev_workspace_id": 123 | null // previous workspace if switching } ``` diff --git a/specs/107-workspace-chooser/plan.md b/specs/107-workspace-chooser/plan.md index 1d22d51..69da0ec 100644 --- a/specs/107-workspace-chooser/plan.md +++ b/specs/107-workspace-chooser/plan.md @@ -79,7 +79,7 @@ ### Source Code (repository root) │ ├── Audit/ │ │ └── AuditActionId.php # MODIFY — add 2 enum cases │ └── Workspaces/ -│ └── WorkspaceContext.php # MODIFY — add clearSession + audit helper +│ └── WorkspaceRedirectResolver.php # NEW — tenant-count branching helper (R4) resources/ └── views/ @@ -92,7 +92,9 @@ ### Source Code (repository root) └── Workspaces/ ├── EnsureWorkspaceSelectedMiddlewareTest.php # NEW ├── ChooseWorkspacePageTest.php # NEW - └── WorkspaceSwitchUserMenuTest.php # NEW + ├── WorkspaceSwitchUserMenuTest.php # NEW + ├── WorkspaceRedirectResolverTest.php # NEW + └── WorkspaceAuditTrailTest.php # NEW ``` **Structure Decision**: Standard Laravel monolith structure. All changes are in existing directories. No new folders needed. diff --git a/specs/107-workspace-chooser/spec.md b/specs/107-workspace-chooser/spec.md index 518e1b0..ee28e17 100644 --- a/specs/107-workspace-chooser/spec.md +++ b/specs/107-workspace-chooser/spec.md @@ -153,7 +153,7 @@ ## Requirements *(mandatory)* - **Membership = switch-right**: no separate `workspace.switch` capability in v1. Any workspace member may select/switch to that workspace. - **`workspace.manage`** gates: visibility of "Manage workspaces" link in chooser; access to Workspace CRUD (existing separate resource). - **404 vs 403**: non-member attempting to select a workspace they're not in → 404 (deny-as-not-found); results in no selection change. -- **Server-side enforcement**: `EnsureActiveWorkspace` middleware validates membership on every request; chooser only lists workspaces with valid membership. +- **Server-side enforcement**: `EnsureWorkspaceSelected` middleware validates membership on every request; chooser only lists workspaces with valid membership. **Constitution alignment (audit):** @@ -176,7 +176,7 @@ ### Functional Requirements - **FR-007**: Route parameter `?choose=1` MUST force the chooser to display regardless of auto-resume eligibility. - **FR-008**: User menu MUST contain a "Switch workspace" entry that links to `/admin/choose-workspace?choose=1`. Entry is visible only when the user has >1 workspace membership. - **FR-009**: After workspace selection (auto or manual), the system MUST apply existing tenant-count branching: 0 tenants → Managed Tenants index, 1 tenant → Tenant Dashboard directly, >1 tenants → Choose Tenant page. No "smart redirect" to the last-visited page in v1. -- **FR-010**: The `EnsureActiveWorkspace` middleware MUST run on all `/admin/*` routes except the chooser page itself, login/logout routes, and OAuth callbacks. +- **FR-010**: The `EnsureWorkspaceSelected` middleware MUST run on all `/admin/*` routes except the chooser page itself, login/logout routes, and OAuth callbacks. - **FR-011**: Chooser queries MUST NOT produce N+1 problems (eager load memberships + `withCount('tenants')`). --- @@ -222,7 +222,7 @@ ## Terminology & Copy --- -## Middleware: EnsureActiveWorkspace (v1 Algorithm) +## Middleware: EnsureWorkspaceSelected (v1 Algorithm) The middleware runs on all `/admin/*` routes (except chooser, login/logout, OAuth callbacks). @@ -265,6 +265,7 @@ ## Audit Events (v1) | Auto-resume: single membership | `workspace.auto_selected` | `auto` | `single_membership` | `actor_id`, `workspace_id`, `prev_workspace_id` (if any) | | Auto-resume: last used | `workspace.auto_selected` | `auto` | `last_used` | `actor_id`, `workspace_id`, `prev_workspace_id` (if any) | | Manual selection from chooser | `workspace.selected` | `manual` | `chooser` | `actor_id`, `workspace_id`, `prev_workspace_id` (if any) | +| Context-bar switch (dropdown) | `workspace.selected` | `manual` | `context_bar` | `actor_id`, `workspace_id`, `prev_workspace_id` (if any) | --- diff --git a/specs/107-workspace-chooser/tasks.md b/specs/107-workspace-chooser/tasks.md index de6f245..d9a6468 100644 --- a/specs/107-workspace-chooser/tasks.md +++ b/specs/107-workspace-chooser/tasks.md @@ -120,7 +120,7 @@ ### Implementation for User Story 4 - [ ] T021 [US4] Enhance middleware step 3: detect stale session (revoked membership or archived workspace), clear session, emit Filament `Notification::make()->danger()` with "Your access to {workspace_name} was removed." flash, redirect to chooser in `app/Http/Middleware/EnsureWorkspaceSelected.php` - [ ] T022 [US4] Enhance middleware step 6 error path: detect stale `last_workspace_id` (revoked or archived), clear `last_workspace_id` on user record, emit flash warning, redirect to chooser in `app/Http/Middleware/EnsureWorkspaceSelected.php` - [ ] T023 [US4] Write test `it_clears_session_when_active_workspace_membership_revoked` — verify session cleared + warning notification + chooser redirect in `tests/Feature/Workspaces/EnsureWorkspaceSelectedMiddlewareTest.php` -- [ ] T024 [US4] Write test `it_redirects_to_chooser_when_last_workspace_membership_revoked_and_shows_warning` — verify `last_workspace_id` cleared + warning + chooser in `tests/Feature/Workspaces/EnsureWorkspaceSelectedMiddlewareTest.php` +- [ ] T024 [US4] Write test `it_redirects_to_chooser_when_last_workspace_membership_revoked_and_shows_warning` — verify `last_workspace_id` cleared + warning + chooser, including archived workspace scenario (edge case EC2) in `tests/Feature/Workspaces/EnsureWorkspaceSelectedMiddlewareTest.php` - [ ] T025 [US4] Write test `it_handles_archived_workspace_in_session` — verify archived workspace treated as stale in `tests/Feature/Workspaces/EnsureWorkspaceSelectedMiddlewareTest.php` **Checkpoint**: Stale/revoked membership detection is active. Users see clear warning notifications instead of broken states. @@ -139,7 +139,7 @@ ### Implementation for User Story 5 - [ ] T027 [US5] Replace form POST with `wire:click="selectWorkspace({{ $workspace->id }})"` in chooser Blade template in `resources/views/filament/pages/choose-workspace.blade.php` - [ ] T028 [US5] Use `WorkspaceRedirectResolver` in `ChooseWorkspace::redirectAfterWorkspaceSelected()` for tenant-count branching in `app/Filament/Pages/ChooseWorkspace.php` - [ ] T029 [US5] Register "Switch workspace" user menu item via `->userMenuItems()` with `MenuItem::make()->url('/admin/choose-workspace?choose=1')->icon('heroicon-o-arrows-right-left')` and `->visible()` callback (>1 workspace membership) in `app/Providers/Filament/AdminPanelProvider.php` -- [ ] T030 [US5] Write test `it_forces_chooser_with_choose_param` — verify `?choose=1` bypasses auto-resume in `tests/Feature/Workspaces/EnsureWorkspaceSelectedMiddlewareTest.php` +- [ ] T030 [US5] Write test `it_forces_chooser_with_choose_param` — verify `?choose=1` bypasses auto-resume, including the single-workspace sub-case (edge case EC3: forced chooser shown even with 1 membership) in `tests/Feature/Workspaces/EnsureWorkspaceSelectedMiddlewareTest.php` - [ ] T031 [US5] Write test `it_persists_last_used_workspace_on_manual_selection` and `it_emits_audit_event_on_manual_selection` — verify `last_workspace_id` update + `workspace.selected` audit log in `tests/Feature/Workspaces/ChooseWorkspacePageTest.php` - [ ] T032 [US5] Write test `it_shows_switch_workspace_menu_when_multiple_workspaces` and `it_hides_switch_workspace_menu_when_single_workspace` in `tests/Feature/Workspaces/WorkspaceSwitchUserMenuTest.php` - [ ] T033 [US5] Write test `it_rejects_non_member_workspace_selection_with_404` — verify deny-as-not-found for non-member attempt in `tests/Feature/Workspaces/ChooseWorkspacePageTest.php` @@ -167,7 +167,7 @@ ## Phase 9: Polish & Cross-Cutting Concerns **Purpose**: Deduplicate remaining tenant-branching copies, full suite validation, formatting. -- [ ] T036 Replace inline tenant-count branching in `SwitchWorkspaceController::__invoke()` with `WorkspaceRedirectResolver` in `app/Http/Controllers/SwitchWorkspaceController.php` +- [ ] T036 [US6] Replace inline tenant-count branching in `SwitchWorkspaceController::__invoke()` with `WorkspaceRedirectResolver` AND add `WorkspaceAuditLogger::log()` for `workspace.selected` (method: `manual`, reason: `context_bar`) to satisfy FR-005 audit coverage for the context-bar switch path, in `app/Http/Controllers/SwitchWorkspaceController.php` - [ ] T037 Replace inline tenant-count branching in `/admin` route handler with `WorkspaceRedirectResolver` in `routes/web.php` - [ ] T038 Run full test suite via `vendor/bin/sail artisan test --compact` and verify no regressions - [ ] T039 Run Pint formatting via `vendor/bin/sail bin pint --dirty --format agent`