fix: resolve 5 consistency issues from project analysis (F1–F7)
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
This commit is contained in:
parent
d27cfec5cb
commit
d67e2c84bc
@ -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`
|
||||
|
||||
|
||||
@ -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
|
||||
}
|
||||
```
|
||||
|
||||
@ -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.
|
||||
|
||||
@ -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) |
|
||||
|
||||
---
|
||||
|
||||
|
||||
@ -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`
|
||||
|
||||
Loading…
Reference in New Issue
Block a user