ahmido
33a2b1a242
## Summary
Fixes the sidebar context bug where navigating to the **Required Permissions** page (`/admin/tenants/{id}/required-permissions`) would switch the sidebar from workspace navigation to tenant-scoped navigation, confusing users.
## Problem
The `EnsureFilamentTenantSelected` middleware detected a tenant ID in the URL and called `setTenant()`, which switched the entire sidebar to tenant-scoped navigation. The Required Permissions page is logically a **workspace-level** page that happens to reference a tenant — it should keep showing workspace nav.
## Changes
### Middleware (`EnsureFilamentTenantSelected.php`)
- **`isWorkspaceScopedPageWithTenant()`** — new private helper that detects workspace-scoped pages containing a tenant parameter via regex
- **Livewire referer bypass** — checks if a Livewire request originates from a workspace-scoped page and preserves workspace nav
- **`setTenant()` bypass** — skips tenant activation and `rememberLastTenantId()` for workspace-scoped pages
### Tests
- **`RequiredPermissionsSidebarTest.php`** (NEW) — 7 tests covering:
- Workspace nav visible on required-permissions page
- Tenant nav absent on required-permissions page
- Direct URL access preserves workspace nav
- 404 for non-member tenants
- 404 for tenants without entitlement
- Tenant pages still show tenant sidebar (regression guard)
- Scoped tenant resolves correctly on tenant pages
### Pre-existing test fixes
- **`RequiredPermissionsEmptyStateTest`** — fixed URL assertion (dynamic `TenantResource::getUrl()` instead of hardcoded `/admin/onboarding`)
- **`RequiredPermissionsLinksTest`** — fixed URL assertion + multiline HTML `data-testid` assertion
- **`RequiredPermissionsFiltersTest`** — fixed `entra_permissions` config leak from branch 105
## Test Results
| Suite | Result |
|-------|--------|
| RequiredPermissions (26 tests) | **26 pass** (73 assertions) |
| Full regression (1571 tests) | **1562 pass**, 2 fail (pre-existing OpsUx), 7 skipped |
The 2 failures are pre-existing in `OpsUx/OperationCatalogCoverageTest` and `OpsUx/OperationSummaryKeysSpecTest` — unrelated to this feature.
## Spec Artifacts
- `specs/106-required-permissions-sidebar-context/plan.md`
- `specs/106-required-permissions-sidebar-context/tasks.md`
- `specs/106-required-permissions-sidebar-context/research.md`
- `specs/106-required-permissions-sidebar-context/data-model.md`
- `specs/106-required-permissions-sidebar-context/quickstart.md`
Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #129
70 lines
2.5 KiB
Markdown
70 lines
2.5 KiB
Markdown
# Quickstart: 106 — Required Permissions Sidebar Context Fix
|
|
|
|
**Branch**: `106-required-permissions-sidebar-context`
|
|
|
|
## What This Changes
|
|
|
|
The `EnsureFilamentTenantSelected` middleware is updated to recognize the Required Permissions page (`/admin/tenants/{tenant}/required-permissions`) as a workspace-scoped page. The page retains its `{tenant}` route parameter for data display and authorization, but the middleware no longer sets Filament's tenant context when serving this page. This causes the sidebar to render workspace-level navigation instead of tenant-level navigation.
|
|
|
|
## Files Modified
|
|
|
|
| File | Change |
|
|
|---|---|
|
|
| `app/Support/Middleware/EnsureFilamentTenantSelected.php` | Add workspace-scoped page allowlist check; add Livewire referer check |
|
|
| `tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php` | New test file — sidebar context assertions |
|
|
|
|
## Implementation Steps
|
|
|
|
### 1. Add workspace-scoped page path helper
|
|
|
|
In `EnsureFilamentTenantSelected`, add a private method:
|
|
|
|
```php
|
|
private function isWorkspaceScopedPageWithTenant(string $path): bool
|
|
{
|
|
return preg_match('#^/admin/tenants/[^/]+/required-permissions$#', $path) === 1;
|
|
}
|
|
```
|
|
|
|
### 2. Add Livewire referer check (before existing checks)
|
|
|
|
In the `/livewire/update` block, add:
|
|
|
|
```php
|
|
if (preg_match('#^/admin/tenants/[^/]+/required-permissions$#', $refererPath) === 1) {
|
|
$this->configureNavigationForRequest($panel);
|
|
return $next($request);
|
|
}
|
|
```
|
|
|
|
### 3. Split `{tenant}` param handling
|
|
|
|
In the `$tenantParameter !== null` block, after all 8 authorization checks pass, add:
|
|
|
|
```php
|
|
if ($this->isWorkspaceScopedPageWithTenant($path)) {
|
|
// Workspace-scoped page: authorize but do NOT set Filament tenant context.
|
|
// This preserves workspace sidebar while still validating tenant access.
|
|
$this->configureNavigationForRequest($panel);
|
|
return $next($request);
|
|
}
|
|
```
|
|
|
|
Place this **before** `Filament::setTenant($tenant, true)`.
|
|
|
|
### 4. Write tests
|
|
|
|
Create `tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php` with assertions:
|
|
- Sidebar shows workspace navigation items (Operations, Manage workspaces)
|
|
- Sidebar does NOT show tenant navigation items (Inventory, Backups & Restore)
|
|
- Livewire updates preserve workspace sidebar
|
|
- Other `{tenant}` pages still get tenant sidebar
|
|
|
|
## Verification
|
|
|
|
```bash
|
|
vendor/bin/sail artisan test --compact --filter=RequiredPermissionsSidebar
|
|
vendor/bin/sail artisan test --compact tests/Feature/RequiredPermissions/
|
|
vendor/bin/sail bin pint --dirty --format agent
|
|
```
|