Files
L-Ami-Fiduciaire/_bmad-output/implementation-artifacts/1-3-role-assignment-and-member-removal.md
Saad Ibn-Ezzoubayr c89d1879bf feat: complete Epic 1 — team management & permission system
- 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>
2026-03-18 00:12:50 +00:00

459 lines
25 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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.