- Story 1.1: Permission enum, config, AuthorizesPermissions & HasWorkspaceScope traits, member→worker migration - Story 1.2: Team page with member list, invitation system with queued email - Story 1.3: Role assignment (Manager/Worker) and member removal with activity logging - Story 1.4: Owner-only permission toggle matrix for Managers (manage team, view logs, configure portal) - Story 1.5: Role-based access enforcement — Workers see only assigned declarations/clients, sidebar scoping - Story 1.6: Workspace switcher dropdown for multi-workspace users with session-based switching - 83 new/modified files, 182 tests passing with zero regressions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
459 lines
25 KiB
Markdown
459 lines
25 KiB
Markdown
# Story 1.3: Role Assignment & Member Removal
|
||
|
||
Status: done
|
||
|
||
<!-- Note: Validation is optional. Run validate-create-story for quality check before dev-story. -->
|
||
|
||
## Story
|
||
|
||
As a firm owner,
|
||
I want to change a team member's role or remove them from the workspace,
|
||
So that I can adjust team structure as my firm's needs evolve.
|
||
|
||
## Acceptance Criteria
|
||
|
||
1. **Action menu on member rows** — Each active team member row (except the current user) displays a "..." action button that opens a DropdownMenu with "Changer le rôle" and "Retirer de l'espace" options.
|
||
|
||
2. **Change Role dialog** — Clicking "Changer le rôle" opens a Dialog with a Select dropdown pre-filled with the member's current role (Manager/Worker — Owner is never assignable). Confirming sends a `PATCH /team/{workspaceUser}/role` request.
|
||
|
||
3. **Role change resets permissions** — When a member's role changes, the `permissions` JSON column is reset to `config('permissions.defaults')` for the new role. Changing from Manager to Worker clears all permissions. Changing from Worker to Manager sets Manager defaults.
|
||
|
||
4. **Remove member confirmation** — Clicking "Retirer de l'espace" opens a confirmation Dialog with the member's name and a destructive "Retirer" button. Confirming sends a `DELETE /team/{workspaceUser}` request.
|
||
|
||
5. **Removal detaches, not deletes** — Removing a member detaches them from the workspace (`workspace_user` pivot row deleted). Their user account remains intact.
|
||
|
||
6. **Self-action prevention** — The action menu is NOT shown for the current authenticated user's own row. Owners cannot change their own role or remove themselves.
|
||
|
||
7. **Manager with `can_manage_team` access** — Managers with `can_manage_team` permission can also change roles and remove members, EXCEPT they cannot modify or remove the Owner.
|
||
|
||
8. **Activity logging** — All role changes and removals are logged via Spatie Activity Log with actor, target user name, old role, and new role (or "removed").
|
||
|
||
9. **Success toasts** — A success toast confirms each action: "Rôle mis à jour" for role changes, "Membre retiré" for removals.
|
||
|
||
10. **Immediate list update** — The team list updates immediately after each action (Inertia redirect back).
|
||
|
||
## Tasks / Subtasks
|
||
|
||
- [x] Task 1: Add `updateRole` and `remove` methods to `TeamController` (AC: #2, #3, #4, #5, #6, #7, #8, #9, #10)
|
||
- [x] 1.1 Add `updateRole(Request $request, WorkspaceUser $workspaceUser)` method
|
||
- Validate role is `in:manager,worker` via `UpdateTeamMemberRoleRequest`
|
||
- Block if target is Owner (abort 404)
|
||
- Block if target is current user (abort 404)
|
||
- Block if actor is Manager and target is Owner (abort 404)
|
||
- Update `role` column on `workspace_user` pivot
|
||
- Reset `permissions` to `config('permissions.defaults.{new_role}')`
|
||
- Log activity via `activity()->performedOn($workspaceUser)->withProperties([...])->log('role_changed')`
|
||
- Redirect back with `'success' => 'Rôle mis à jour'`
|
||
- [x] 1.2 Add `remove(WorkspaceUser $workspaceUser)` method
|
||
- Block if target is Owner (abort 404)
|
||
- Block if target is current user (abort 404)
|
||
- Block if actor is Manager and target is Owner (abort 404)
|
||
- Detach user from workspace: `$workspace->users()->detach($workspaceUser->user_id)`
|
||
- Log activity via `activity()->withProperties([...])->log('member_removed')`
|
||
- Redirect back with `'success' => 'Membre retiré'`
|
||
- [x] 1.3 Both methods use `authorizePermission(Permission::CanManageTeam)` for authorization (Owners auto-pass, Workers abort 404, Managers check permission)
|
||
|
||
- [x] Task 2: Create `UpdateTeamMemberRoleRequest` form request (AC: #2, #7)
|
||
- [x] 2.1 Create `app/Http/Requests/UpdateTeamMemberRoleRequest.php`
|
||
- [x] 2.2 `authorize()`: verify user is Owner OR Manager with `can_manage_team` permission
|
||
- [x] 2.3 `rules()`: validate `role` (required, in: manager, worker)
|
||
|
||
- [x] Task 3: Add routes (AC: #2, #4)
|
||
- [x] 3.1 Add inside workspace middleware group in `routes/web.php`:
|
||
- `Route::patch('team/{workspaceUserId}/role', [TeamController::class, 'updateRole'])->name('team.updateRole')`
|
||
- `Route::delete('team/{workspaceUserId}', [TeamController::class, 'remove'])->name('team.remove')`
|
||
|
||
- [x] Task 4: Update `TeamController::index()` to pass action URLs and auth user ID (AC: #1, #6)
|
||
- [x] 4.1 Pass `authUserId` prop (current user's ID) to frontend for self-action prevention
|
||
- [x] 4.2 Pass `updateRoleUrl` pattern and `removeUrl` pattern — OR pass per-member action URLs in the members array
|
||
- [x] 4.3 Include `workspace_user_id` (pivot ID) in each member's data for route model binding
|
||
|
||
- [x] Task 5: Update `team/Index.vue` page (AC: #1, #2, #4, #6, #9, #10)
|
||
- [x] 5.1 Add "Actions" column to table (only visible when `canManageTeam` is true)
|
||
- [x] 5.2 Add DropdownMenu with MoreHorizontal icon trigger for each active member row (not for pending invitations, not for current user)
|
||
- [x] 5.3 Add "Changer le rôle" DropdownMenuItem opening a role change Dialog with Select and confirm Button
|
||
- [x] 5.4 Add "Retirer de l'espace" DropdownMenuItem opening a destructive confirmation Dialog
|
||
- [x] 5.5 Use `router.patch()` for role update and `router.delete()` for removal via Inertia
|
||
- [x] 5.6 Hide action menu for Owner rows (when viewed by a Manager)
|
||
|
||
- [x] Task 6: Update TypeScript types (AC: #1)
|
||
- [x] 6.1 Add `workspace_user_id: number` to `TeamMember` type
|
||
- [x] 6.2 Add `authUserId: number` and action URL props to `TeamPageProps`
|
||
- [x] 6.3 Add `updateRoleUrl` and `removeUrl` function type or string template
|
||
|
||
- [x] Task 7: Write tests (AC: #1–#10)
|
||
- [x] 7.1 Create `tests/Feature/Team/TeamRoleAssignmentTest.php`
|
||
- [x] 7.2 Test: Owner can change a member's role from Worker to Manager
|
||
- [x] 7.3 Test: Owner can change a member's role from Manager to Worker
|
||
- [x] 7.4 Test: Role change resets permissions to defaults for new role
|
||
- [x] 7.5 Test: Owner cannot change own role (returns 404)
|
||
- [x] 7.6 Test: Manager with `can_manage_team` can change Worker's role
|
||
- [x] 7.7 Test: Manager with `can_manage_team` cannot change Owner's role (returns 404)
|
||
- [x] 7.8 Test: Manager without `can_manage_team` gets 404
|
||
- [x] 7.9 Test: Worker gets 404 on role change attempt
|
||
- [x] 7.10 Test: Role change is logged in activity log
|
||
- [x] 7.11 Create `tests/Feature/Team/TeamMemberRemovalTest.php`
|
||
- [x] 7.12 Test: Owner can remove a member (user detached, not deleted)
|
||
- [x] 7.13 Test: Owner cannot remove themselves (returns 404)
|
||
- [x] 7.14 Test: Manager with `can_manage_team` can remove Worker
|
||
- [x] 7.15 Test: Manager with `can_manage_team` cannot remove Owner (returns 404)
|
||
- [x] 7.16 Test: Manager without `can_manage_team` gets 404
|
||
- [x] 7.17 Test: Worker gets 404 on remove attempt
|
||
- [x] 7.18 Test: Member removal is logged in activity log
|
||
- [x] 7.19 Test: Removed user's account still exists (not deleted)
|
||
- [x] 7.20 Run full test suite: `composer test`
|
||
|
||
## Dev Notes
|
||
|
||
### Architecture Constraints (MUST FOLLOW)
|
||
|
||
- **Enum library**: Use `bensampo/laravel-enum` ^6.12 (NOT native PHP enums). Use `->is()` for comparisons (NOT `===`).
|
||
- **Model casts**: Use method-based `protected function casts(): array` (NEVER `$casts` property)
|
||
- **Authorization**: Always `abort(404)` for permission failures (NEVER `abort(403)`) — intentional to hide workspace existence
|
||
- **No Policies/Gates**: This project uses custom `authorizeXxx()` methods in traits/controllers, NOT Laravel Policies or Gates
|
||
- **No Spatie Permission package**: Permissions use the JSON column on `workspace_user` pivot — do NOT install `spatie/laravel-permission`
|
||
- **Mass assignment**: Explicit `$fillable` arrays (NEVER `$guarded = []`)
|
||
- **Workspace scoping**: Always from `session('current_workspace_id')`, never from request params
|
||
- **Validation**: Use dedicated FormRequest classes, never inline `$request->validate()`
|
||
- **URLs in Vue**: All URLs must be passed as props from PHP controllers via `route()` helper — never hardcode routes in Vue
|
||
- **Inertia render paths**: Use lowercase subdirectory: `'team/Index'` (not `'Team/Index'`)
|
||
- **Activity logging**: New business operations must log via Spatie Activity Log with `activity()` helper
|
||
|
||
### Route Model Binding for WorkspaceUser
|
||
|
||
The `WorkspaceUser` model extends `Pivot` (not `Model`). Laravel's route model binding works differently with Pivot models. You **MUST** verify this works:
|
||
|
||
- `WorkspaceUser` extends `Illuminate\Database\Eloquent\Relations\Pivot`
|
||
- Route model binding on Pivot models may NOT work out of the box
|
||
- **If route model binding fails**, use explicit resolution: receive `int $workspaceUserId` parameter and manually query `WorkspaceUser::findOrFail($workspaceUserId)`
|
||
- **CRITICAL**: Always verify the resolved WorkspaceUser belongs to the current workspace (from session). If `$workspaceUser->workspace_id !== session('current_workspace_id')`, `abort(404)`.
|
||
|
||
### Controller Pattern (extending Story 1.2's TeamController)
|
||
|
||
Add methods to the EXISTING `app/Http/Controllers/TeamController.php`:
|
||
|
||
```php
|
||
use App\Http\Requests\UpdateTeamMemberRoleRequest;
|
||
|
||
public function updateRole(UpdateTeamMemberRoleRequest $request, int $workspaceUserId): RedirectResponse
|
||
{
|
||
$workspaceUser = WorkspaceUser::where('id', $workspaceUserId)
|
||
->where('workspace_id', session('current_workspace_id'))
|
||
->firstOrFail();
|
||
|
||
// Cannot modify Owner
|
||
if ($workspaceUser->role->is(WorkspaceUserRole::Owner)) {
|
||
abort(404);
|
||
}
|
||
|
||
// Cannot modify self
|
||
if ($workspaceUser->user_id === auth()->id()) {
|
||
abort(404);
|
||
}
|
||
|
||
$oldRole = $workspaceUser->role->value;
|
||
$newRole = $request->validated('role');
|
||
|
||
$workspaceUser->update([
|
||
'role' => $newRole,
|
||
'permissions' => config("permissions.defaults.{$newRole}", []),
|
||
]);
|
||
|
||
// Log with actor context
|
||
$targetUser = User::find($workspaceUser->user_id);
|
||
activity()
|
||
->performedOn($workspaceUser)
|
||
->withProperties([
|
||
'target_user' => $targetUser->name,
|
||
'old_role' => $oldRole,
|
||
'new_role' => $newRole,
|
||
])
|
||
->log('role_changed');
|
||
|
||
return redirect()->back()->with('success', 'Rôle mis à jour');
|
||
}
|
||
|
||
public function remove(int $workspaceUserId): RedirectResponse
|
||
{
|
||
$this->authorizePermission(Permission::CanManageTeam);
|
||
|
||
$workspaceUser = WorkspaceUser::where('id', $workspaceUserId)
|
||
->where('workspace_id', session('current_workspace_id'))
|
||
->firstOrFail();
|
||
|
||
// Cannot remove Owner
|
||
if ($workspaceUser->role->is(WorkspaceUserRole::Owner)) {
|
||
abort(404);
|
||
}
|
||
|
||
// Cannot remove self
|
||
if ($workspaceUser->user_id === auth()->id()) {
|
||
abort(404);
|
||
}
|
||
|
||
$targetUser = User::find($workspaceUser->user_id);
|
||
|
||
activity()
|
||
->withProperties([
|
||
'target_user' => $targetUser->name,
|
||
'target_email' => $targetUser->email,
|
||
'role' => $workspaceUser->role->value,
|
||
])
|
||
->log('member_removed');
|
||
|
||
$workspaceUser->delete(); // Delete the pivot row
|
||
|
||
return redirect()->back()->with('success', 'Membre retiré');
|
||
}
|
||
```
|
||
|
||
### WorkspaceUser Pivot ID
|
||
|
||
The `workspace_user` table has an auto-incrementing `id` column (standard for Pivot models using `->withTimestamps()`). Check this — if the table does NOT have an `id` column, you need to use composite key lookup `(workspace_id, user_id)` instead.
|
||
|
||
To pass the pivot ID to frontend, update the members mapping in `index()`:
|
||
```php
|
||
'workspace_user_id' => $user->pivot->id,
|
||
```
|
||
|
||
**If the pivot table has no `id` column:** Use `user_id` instead and adjust routes to use `{userId}` with manual workspace scoping.
|
||
|
||
### Permission Defaults Application
|
||
|
||
When resetting permissions on role change, use the config values:
|
||
```php
|
||
// config/permissions.php
|
||
'defaults' => [
|
||
'owner' => ['*'],
|
||
'manager' => [
|
||
'can_manage_team' => false,
|
||
'can_view_activity_logs' => true,
|
||
'can_configure_portal' => false,
|
||
],
|
||
'worker' => [],
|
||
]
|
||
```
|
||
|
||
For `updateRole`, set `permissions` to `config("permissions.defaults.{$newRole}", [])`.
|
||
|
||
### Activity Log Pattern
|
||
|
||
This project uses `spatie/laravel-activitylog`. For custom logging (not model-level), use the `activity()` helper:
|
||
|
||
```php
|
||
activity()
|
||
->performedOn($model) // optional: attach to a model
|
||
->causedBy(auth()->user()) // auto-detected if auth user exists
|
||
->withProperties([...]) // key-value pairs for context
|
||
->log('description'); // the log message
|
||
```
|
||
|
||
The `causedBy` is auto-set when an authenticated user exists, so you don't need to call it explicitly.
|
||
|
||
### Frontend Action Pattern
|
||
|
||
**Available UI components for actions:**
|
||
- `DropdownMenu` + variants (installed, in `@/components/ui/dropdown-menu`)
|
||
- `Dialog` (installed, in `@/components/ui/dialog`) — use for both role change and remove confirmation
|
||
- `Select` (installed) — for role selection
|
||
- **NO** `AlertDialog` installed — use `Dialog` with destructive button styling for confirmations
|
||
- **NO** `Popover` installed — use `Dialog` for role change
|
||
|
||
**Action column pattern:**
|
||
```vue
|
||
<DropdownMenu>
|
||
<DropdownMenuTrigger as-child>
|
||
<Button variant="ghost" size="icon">
|
||
<MoreHorizontal class="h-4 w-4" />
|
||
</Button>
|
||
</DropdownMenuTrigger>
|
||
<DropdownMenuContent align="end">
|
||
<DropdownMenuItem @click="openRoleDialog(member)">
|
||
<UserCog class="mr-2 h-4 w-4" />
|
||
Changer le rôle
|
||
</DropdownMenuItem>
|
||
<DropdownMenuSeparator />
|
||
<DropdownMenuItem class="text-destructive" @click="openRemoveDialog(member)">
|
||
<UserMinus class="mr-2 h-4 w-4" />
|
||
Retirer de l'espace
|
||
</DropdownMenuItem>
|
||
</DropdownMenuContent>
|
||
</DropdownMenu>
|
||
```
|
||
|
||
**Inertia programmatic requests (NOT useForm for these):**
|
||
```vue
|
||
import { router } from '@inertiajs/vue3';
|
||
|
||
function submitRoleChange() {
|
||
router.patch(roleChangeUrl.value, { role: selectedRole.value }, {
|
||
onSuccess: () => { showRoleDialog.value = false; },
|
||
});
|
||
}
|
||
|
||
function submitRemove() {
|
||
router.delete(removeUrl.value, {
|
||
onSuccess: () => { showRemoveDialog.value = false; },
|
||
});
|
||
}
|
||
```
|
||
|
||
**Lucide icons to use:** `MoreHorizontal` (action trigger), `UserCog` (change role), `UserMinus` (remove member).
|
||
|
||
### Self-Action Prevention Logic
|
||
|
||
Frontend: Hide the action menu for rows where `member.id === authUserId`.
|
||
Backend: Double-check in both `updateRole` and `remove` — `abort(404)` if `$workspaceUser->user_id === auth()->id()`.
|
||
|
||
Owners viewing their own row: no action menu shown. The Owner row also has no action menu when a Manager views the team (Managers cannot modify Owners).
|
||
|
||
### French Labels for New UI Elements
|
||
|
||
- Action menu: no label (icon-only "..." button)
|
||
- Change role option: "Changer le rôle"
|
||
- Remove option: "Retirer de l'espace"
|
||
- Role change dialog title: "Changer le rôle"
|
||
- Role change dialog description: "Sélectionnez le nouveau rôle pour {member.name}"
|
||
- Role change confirm button: "Confirmer"
|
||
- Remove dialog title: "Retirer le membre"
|
||
- Remove dialog description: "Êtes-vous sûr de vouloir retirer {member.name} de l'espace de travail ? Cette action est irréversible."
|
||
- Remove confirm button: "Retirer" (destructive styling)
|
||
- Success toasts: "Rôle mis à jour", "Membre retiré"
|
||
|
||
### Project Structure Notes
|
||
|
||
- **Modified controller**: `app/Http/Controllers/TeamController.php` (add `updateRole`, `remove` methods)
|
||
- **New form request**: `app/Http/Requests/UpdateTeamMemberRoleRequest.php`
|
||
- **Modified routes**: `routes/web.php` (add PATCH and DELETE team routes)
|
||
- **Modified Vue page**: `resources/js/pages/team/Index.vue` (add actions column, dialogs)
|
||
- **Modified types**: `resources/js/types/team.ts` (add `workspace_user_id`, `authUserId`, action URLs)
|
||
- **New tests**: `tests/Feature/Team/TeamRoleAssignmentTest.php`, `tests/Feature/Team/TeamMemberRemovalTest.php`
|
||
- **No new models, no new migrations, no new packages**
|
||
|
||
### Testing Standards
|
||
|
||
- **Framework**: Pest 4 with `test()` closures and `expect()` assertions
|
||
- **`RefreshDatabase`**: Auto-applied via `Pest.php` for Feature tests — do NOT add manually
|
||
- **Run command**: `composer test` (clears config → runs Pint → runs tests)
|
||
- **Test both**: Happy path (authorized) AND sad path (unauthorized → 404)
|
||
- **Route helper**: Use `route()` helper, never hardcoded URLs
|
||
- **Activity log testing**: Use `Activity::all()` or `Activity::latest()->first()` to assert log entries exist with correct properties
|
||
- **Session setup**: `session(['current_workspace_id' => $workspace->id])`
|
||
- **Auth**: `$this->actingAs($user)`
|
||
- **Factory pattern**: Create workspace, attach users with roles via `$workspace->users()->attach($user, ['role' => 'worker', 'permissions' => []])`
|
||
- **Rollback step count**: Adding NO new migrations — existing `RenameFoldersToDeclarationsTest` rollback step count should NOT change
|
||
|
||
### References
|
||
|
||
- [Source: _bmad-output/planning-artifacts/epics.md#Epic-1 — Story 1.3 requirements and acceptance criteria]
|
||
- [Source: _bmad-output/planning-artifacts/architecture.md#Route-Structure — team routes (PATCH role, DELETE member)]
|
||
- [Source: _bmad-output/planning-artifacts/architecture.md#D1-Permission-Toggle-Storage — JSON permissions reset on role change]
|
||
- [Source: _bmad-output/planning-artifacts/architecture.md#Phase-1-Files — file locations]
|
||
- [Source: _bmad-output/planning-artifacts/ux-design-specification.md — "Action at the point of recognition" principle, contextual menus]
|
||
- [Source: _bmad-output/project-context.md — All coding rules and conventions]
|
||
- [Source: _bmad-output/implementation-artifacts/1-1-*.md — Permission system, AuthorizesPermissions trait]
|
||
- [Source: _bmad-output/implementation-artifacts/1-2-*.md — TeamController, team page, invitation patterns]
|
||
|
||
### Previous Story Intelligence (Story 1.1 + 1.2 Learnings)
|
||
|
||
- **bensampo/laravel-enum**: Uses `->is()` for comparisons, NOT `===` (Enum instance vs string constant)
|
||
- **`permissions` column**: Already cast to `array` on WorkspaceUser model — do NOT `json_encode()` when setting via `update()` (causes double-encoding)
|
||
- **Migration count matters**: Adding new migrations shifts rollback step counts — but this story adds NO migrations
|
||
- **FK constraints**: Use explicit `->on('table_name')` (never bare `->constrained()`)
|
||
- **Scope discipline**: Only modify files directly required by acceptance criteria — no cosmetic changes
|
||
- **Flash messages**: `HandleInertiaRequests` already shares `success`/`error` flash keys (fixed in Story 1.2 code review)
|
||
- **Toast display**: `AppSidebarLayout` already has flash message toast display (added in Story 1.2 code review)
|
||
- **Wayfinder routes**: Use wayfinder-generated typed routes for breadcrumbs (e.g., `dashboard()`, `teamIndex()`)
|
||
- **Role serialization**: Use `$user->pivot->role?->value` when passing enum values to Inertia (avoid passing enum objects)
|
||
- **Code review findings from 1.2**: [H1] Empty state Dialog structure matters. [H2] Always use wayfinder routes for breadcrumbs. [H3] Flash message handling must be in HandleInertiaRequests. [M1] Role labels should come from props, not be hardcoded in Vue.
|
||
- **Config defaults not consumed yet**: Story 1.1 code review noted that `config('permissions.defaults')` was defined but not consumed. THIS STORY must consume it when resetting permissions on role change.
|
||
|
||
### Git Intelligence
|
||
|
||
Recent commits (4 total on branch `l-ami-fiduciaire-v1.0.0`):
|
||
- `5dffd2d` chore: complete Epic 0 retrospective
|
||
- `fd43a6f` feat: complete Epic 0
|
||
- `d380df4` chore: add BMAD workflow commands
|
||
- `35545c2` feat: full codebase with Story 0.1 complete
|
||
|
||
Stories 1.1 and 1.2 are implemented but not yet committed (visible in git status as modified/untracked files). Current test suite: 117 tests, 317 assertions, 0 failures.
|
||
|
||
### Critical Implementation Warnings
|
||
|
||
1. **Do NOT create a new migration** — The `workspace_user` table already has `role` and `permissions` columns. This story only updates existing pivot rows.
|
||
|
||
2. **Do NOT install any new packages** — Use existing UI components (DropdownMenu, Dialog, Select). No `AlertDialog` or `Popover` needed.
|
||
|
||
3. **WorkspaceUser extends Pivot, NOT Model** — Route model binding may not auto-resolve. Use manual query with `WorkspaceUser::where(...)` and scope to current workspace.
|
||
|
||
4. **Reset permissions on role change** — This is the FIRST story that actually consumes `config('permissions.defaults')`. When changing Worker→Manager, apply Manager defaults. When changing Manager→Worker, set empty array.
|
||
|
||
5. **Detach, NOT delete user** — `remove()` only deletes the `workspace_user` pivot row. The user account in the `users` table must remain intact.
|
||
|
||
6. **Manager cannot modify Owner** — Both `updateRole()` and `remove()` must check if target is Owner and abort(404). A Manager with `can_manage_team` can modify Workers and other Managers, but NEVER the Owner.
|
||
|
||
7. **Owner protection is CRITICAL** — This is a security boundary. The Owner must never be removable or demotable. Test this thoroughly.
|
||
|
||
8. **Activity log for non-model operations** — Use `activity()` helper (not model-level LogsActivity trait) since we're logging business operations, not model mutations directly.
|
||
|
||
## Dev Agent Record
|
||
|
||
### Agent Model Used
|
||
|
||
Claude Opus 4.6
|
||
|
||
### Debug Log References
|
||
|
||
- Fixed pivot `id` not being available: added `'id'` to `withPivot()` call in `TeamController::index()` — Pivot models don't include `id` by default, causing `route()` generation to fail with null parameter.
|
||
|
||
### Completion Notes List
|
||
|
||
- Implemented `updateRole()` and `remove()` methods on `TeamController` with full authorization checks (Owner auto-pass, Manager permission check, Worker block), self-action prevention, and activity logging.
|
||
- Used manual `WorkspaceUser::where()` lookup instead of route model binding since `WorkspaceUser` extends `Pivot` — scoped to current workspace via session.
|
||
- Role changes reset permissions to `config('permissions.defaults.{role}')` — this is the first story consuming these config defaults.
|
||
- `remove()` deletes the pivot row only (`$workspaceUser->delete()`), user account remains intact.
|
||
- Created `UpdateTeamMemberRoleRequest` with `authorize()` mirroring the `InviteTeamMemberRequest` pattern and `failedAuthorization()` returning 404.
|
||
- Frontend uses `DropdownMenu` with `MoreHorizontal` trigger, `Dialog` for role change (with `Select`) and removal confirmation (with destructive button styling).
|
||
- Action URLs passed as per-member props from PHP controller — no hardcoded routes in Vue.
|
||
- Self-action prevention: frontend hides action menu for `authUserId` rows; backend double-checks in both methods.
|
||
- Owner rows also hidden from Manager action menus.
|
||
- 17 new tests across 2 test files covering all acceptance criteria, authorization paths, permission resets, activity logging, and user account preservation.
|
||
- Full test suite: 134 tests, 350 assertions, 0 failures, no regressions.
|
||
|
||
### File List
|
||
|
||
- `app/Http/Controllers/TeamController.php` — Modified: added `updateRole()`, `remove()` methods; updated `index()` to pass `authUserId`, `workspace_user_id`, `updateRoleUrl`, `removeUrl` per member; added `'id'` to `withPivot()`
|
||
- `app/Http/Requests/UpdateTeamMemberRoleRequest.php` — New: form request with authorization and role validation
|
||
- `routes/web.php` — Modified: added PATCH `team/{workspaceUserId}/role` and DELETE `team/{workspaceUserId}` routes
|
||
- `resources/js/pages/team/Index.vue` — Modified: added actions column with DropdownMenu, role change Dialog, and remove confirmation Dialog
|
||
- `resources/js/types/team.ts` — Modified: added `workspace_user_id`, `updateRoleUrl`, `removeUrl` to `TeamMember`; added `authUserId` to `TeamPageProps`
|
||
- `tests/Feature/Team/TeamRoleAssignmentTest.php` — New: 10 tests for role assignment (9 original + 1 added in review)
|
||
- `tests/Feature/Team/TeamMemberRemovalTest.php` — New: 8 tests for member removal
|
||
|
||
## Senior Developer Review (AI)
|
||
|
||
**Reviewer:** Saad on 2026-03-15
|
||
**Outcome:** Approved (after fixes applied)
|
||
|
||
### Findings & Fixes Applied
|
||
|
||
| # | Severity | Issue | Fix |
|
||
|---|----------|-------|-----|
|
||
| H1 | HIGH | `User::find()` could return null in `updateRole()` and `remove()`, causing 500 error | Changed to `User::findOrFail()` |
|
||
| H2 | HIGH | No DB transaction around multi-step operations (update + activity log) | Wrapped both methods in `DB::transaction()` |
|
||
| M1 | MEDIUM | No loading/disabled state on role change and remove buttons — double-click risk | Added `roleChangeProcessing`/`removeProcessing` refs with `:disabled` bindings |
|
||
| M2 | MEDIUM | Same-role submission allowed, pointlessly resetting permissions | Added `isRoleUnchanged` computed, disabled confirm button when unchanged |
|
||
| M3 | MEDIUM | Authorization approach inconsistency between `updateRole` (FormRequest) and `remove` (trait) | Added `authorizePermission()` call to `updateRole()` for consistency |
|
||
| L1 | LOW | Redundant `getMemberData()` calls (3x per row) in template | Not fixed — minor perf, acceptable for team-size lists |
|
||
| L2 | LOW | Missing test: Manager changing another Manager's role | Added test: "manager with can_manage_team can change another manager role" |
|
||
| L3 | LOW | Activity logged before delete without transaction in `remove()` | Fixed via H2 — delete now runs first inside transaction |
|
||
|
||
### Post-Review Test Results
|
||
|
||
- **135 tests, 353 assertions, 0 failures** (up from 134/350 — 1 new test added)
|
||
|
||
## Change Log
|
||
|
||
- 2026-03-15: Implemented Story 1.3 — Role Assignment & Member Removal. Added `updateRole` and `remove` endpoints with authorization, permission reset on role change, activity logging, frontend action menus with dialogs, and 17 comprehensive tests.
|
||
- 2026-03-15: Code review — Fixed 5 HIGH/MEDIUM issues (null safety, DB transactions, loading states, same-role guard, auth consistency). Added 1 missing test. 135 tests, 353 assertions, 0 failures.
|