From 18c2615b9f63b6ef2cc5d42112bd67b654b89161 Mon Sep 17 00:00:00 2001 From: bernardhanna Date: Wed, 1 Jul 2026 09:32:14 +0100 Subject: [PATCH] support copilot: add user_role_remove for ambassador-style removals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Role removal requests (e.g. remove ambassador from vward@eircom.net) now flow through triage, dry-run, APPROVE, and execution — mirroring role add. - new UserRoleRemoveService with batch dry-run + removeRole execution - role_remove case type in heuristic + Cursor triage schema/prompt - natural-language parser for "remove the ambassador role from..." - dry-run and completion email copy for removals Co-authored-by: Cursor --- .../ExecuteApprovedSupportActionJob.php | 4 + .../ProcessSupportCaseDiagnosticsJob.php | 16 ++ .../Agents/CursorCliTriageProvider.php | 15 +- .../Support/Agents/TriageAgentService.php | 12 +- .../Support/SupportApprovalEmailService.php | 102 +++++--- .../Support/SupportRoleRequestParser.php | 6 + .../Support/UserRoleRemoveService.php | 218 ++++++++++++++++++ config/support_gmail.php | 1 + .../Support/SupportRoleRequestParserTest.php | 15 ++ .../Support/UserRoleRemoveServiceTest.php | 70 ++++++ 10 files changed, 426 insertions(+), 33 deletions(-) create mode 100644 app/Services/Support/UserRoleRemoveService.php create mode 100644 tests/Unit/Support/UserRoleRemoveServiceTest.php diff --git a/app/Jobs/Support/ExecuteApprovedSupportActionJob.php b/app/Jobs/Support/ExecuteApprovedSupportActionJob.php index 32261eb03..dbb7f9c31 100644 --- a/app/Jobs/Support/ExecuteApprovedSupportActionJob.php +++ b/app/Jobs/Support/ExecuteApprovedSupportActionJob.php @@ -12,6 +12,7 @@ use App\Services\Support\UserProfileUpdateService; use App\Services\Support\UserRestoreService; use App\Services\Support\UserRoleAddService; +use App\Services\Support\UserRoleRemoveService; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; @@ -35,6 +36,7 @@ public function handle( ArtisanCommandRunner $artisanRunner, ContentUpdateService $contentUpdate, UserRoleAddService $userRoleAdd, + UserRoleRemoveService $userRoleRemove, ): void { $approval = SupportApproval::findOrFail($this->supportApprovalId); @@ -92,6 +94,8 @@ public function handle( } elseif ($action === 'user_role_add') { // Re-read the role + emails from the case email at execution time. $result = $userRoleAdd->addFromCase($case, dryRun: false, viaEmailApproval: true); + } elseif ($action === 'user_role_remove') { + $result = $userRoleRemove->removeFromCase($case, dryRun: false, viaEmailApproval: true); } elseif ($action === 'code_change') { $result = $cursorAgent->launchCodeAgent( prompt: (string) ($payload['cursor_prompt'] ?? ''), diff --git a/app/Jobs/Support/ProcessSupportCaseDiagnosticsJob.php b/app/Jobs/Support/ProcessSupportCaseDiagnosticsJob.php index 859e0a2d3..22e497fec 100644 --- a/app/Jobs/Support/ProcessSupportCaseDiagnosticsJob.php +++ b/app/Jobs/Support/ProcessSupportCaseDiagnosticsJob.php @@ -12,6 +12,7 @@ use App\Services\Support\UserProfileUpdateService; use App\Services\Support\UserRestoreService; use App\Services\Support\UserRoleAddService; +use App\Services\Support\UserRoleRemoveService; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; @@ -32,6 +33,7 @@ public function handle( UserRestoreService $userRestore, UserProfileUpdateService $userProfileUpdate, UserRoleAddService $userRoleAdd, + UserRoleRemoveService $userRoleRemove, ArtisanCommandRunner $artisanRunner, ContentUpdateService $contentUpdate, ): void { @@ -79,6 +81,20 @@ public function handle( ); } + if ($case->case_type === 'role_remove') { + $dryRunResult = $userRoleRemove->removeFromCase($case, dryRun: true); + $logger->log( + case: $case, + actionName: 'user_role_remove', + actionType: 'write', + input: ['dry_run' => true], + output: $dryRunResult, + succeeded: (bool) ($dryRunResult['ok'] ?? false), + executedBy: 'agent', + correlationId: $case->correlation_id, + ); + } + if ($case->case_type === 'role_add') { $dryRunResult = $userRoleAdd->addFromCase($case, dryRun: true); $logger->log( diff --git a/app/Services/Support/Agents/CursorCliTriageProvider.php b/app/Services/Support/Agents/CursorCliTriageProvider.php index c9ac3ff18..c7edbffb9 100644 --- a/app/Services/Support/Agents/CursorCliTriageProvider.php +++ b/app/Services/Support/Agents/CursorCliTriageProvider.php @@ -28,6 +28,7 @@ class CursorCliTriageProvider implements TriageProvider 'certificate_issue', 'role_issue', 'role_add', + 'role_remove', 'code_change', 'artisan_command', 'content_update', @@ -138,6 +139,9 @@ private function buildPrompt(SupportCase $case, string $rawText): string users identified by email. Put the affected emails in target_email/secondary_emails, put the role being granted in "role_name" (singular, e.g. "leading teacher"), and set "role_operation" to "add". Include EVERY email listed in the request (one per person), even for long pasted tables. +Use "role_remove" when the request is to remove/revoke/delete a role (e.g. "ambassador") from one +or more users. Use the same target_email/secondary_emails and role_name fields, and set +"role_operation" to "remove". {$artisanBlock}{$contentBlock} JSON schema to return: { @@ -151,8 +155,8 @@ private function buildPrompt(SupportCase $case, string $rawText): string "reasoning_summary": "", "profile_firstname": "", "profile_lastname": "", - "role_name": "", - "role_operation": "", + "role_name": "", + "role_operation": "", "change_summary": "", "change_area": "", "cursor_prompt": "", @@ -296,6 +300,7 @@ private function normalize(array $data): array 'profile_update' => 'user_profile_update', 'account_restore' => 'user_restore', 'role_add' => 'user_role_add', + 'role_remove' => 'user_role_remove', 'code_change' => 'code_change', 'artisan_command' => 'artisan_command', 'content_update' => 'content_update', @@ -352,7 +357,11 @@ private function normalizeRoleOperation(mixed $value): ?string } $value = strtolower(trim($value)); - return in_array($value, ['add', 'grant', 'assign'], true) ? 'add' : null; + return match (true) { + in_array($value, ['add', 'grant', 'assign'], true) => 'add', + in_array($value, ['remove', 'revoke', 'delete'], true) => 'remove', + default => null, + }; } private function stringOrNull(mixed $value): ?string diff --git a/app/Services/Support/Agents/TriageAgentService.php b/app/Services/Support/Agents/TriageAgentService.php index 5cf243d71..4594f2edd 100644 --- a/app/Services/Support/Agents/TriageAgentService.php +++ b/app/Services/Support/Agents/TriageAgentService.php @@ -59,14 +59,21 @@ private function heuristicTriage(SupportCase $case): array $text = Str::lower($rawText); $profile = $this->profileParser->parse($rawText); $roleRequest = $this->roleParser->parse($rawText); - $hasRoleRequest = $roleRequest['role'] !== null + $hasRoleAddRequest = $roleRequest['role'] !== null && $roleRequest['operation'] === 'add' && $roleRequest['emails'] !== []; + $hasRoleRemoveRequest = $roleRequest['role'] !== null + && $roleRequest['operation'] === 'remove' + && $roleRequest['emails'] !== []; + $hasRoleRequest = $hasRoleAddRequest || $hasRoleRemoveRequest; // V1 heuristic placeholder (replace with LLM later, keep output schema stable). $caseType = 'unknown'; $runbook = 'unknown'; - if ($hasRoleRequest) { + if ($hasRoleRemoveRequest) { + $caseType = 'role_remove'; + $runbook = 'remove_user_role'; + } elseif ($hasRoleAddRequest) { $caseType = 'role_add'; $runbook = 'add_user_role'; } elseif (Str::contains($text, ['soft-deleted', 'deleted', 'restore account', 'account missing'])) { @@ -120,6 +127,7 @@ private function heuristicTriage(SupportCase $case): array $requestedAction = match ($caseType) { 'profile_update' => 'user_profile_update', 'role_add' => 'user_role_add', + 'role_remove' => 'user_role_remove', default => null, }; diff --git a/app/Services/Support/SupportApprovalEmailService.php b/app/Services/Support/SupportApprovalEmailService.php index 61e0eb19a..d1cbd433f 100644 --- a/app/Services/Support/SupportApprovalEmailService.php +++ b/app/Services/Support/SupportApprovalEmailService.php @@ -26,6 +26,7 @@ public function approvalSubject(SupportCase $case): string 'profile_update' => 'Please review — name change', 'account_restore' => 'Please review — account restore', 'role_add' => 'Please review — add user role', + 'role_remove' => 'Please review — remove user role', 'code_change' => 'Please review — proposed code fix (PR into dev)', 'artisan_command' => 'Please review — proposed server maintenance command', 'content_update' => 'Please review — proposed content/copy change', @@ -282,6 +283,20 @@ private function proposedActionForCase(SupportCase $case): array } } + if ($case->case_type === 'role_remove') { + $role = $this->roleResolver->resolve($case); + if ($role['role'] !== null && $role['emails'] !== []) { + return [ + 'action' => 'user_role_remove', + 'payload' => [ + 'operation' => $role['operation'], + 'role' => $role['role'], + 'emails' => $role['emails'], + ], + ]; + } + } + if ($case->case_type === 'role_add') { $role = $this->roleResolver->resolve($case); if ($role['role'] !== null && $role['emails'] !== []) { @@ -514,8 +529,8 @@ private function dryRunPlannedChangeLines(SupportCase $case, array $proposedActi return $this->dryRunContentLines($case); } - if ($action === 'user_role_add') { - return $this->dryRunRoleAddLines($case, $payload); + if ($action === 'user_role_add' || $action === 'user_role_remove') { + return $this->dryRunRoleChangeLines($case, $payload, $action); } return [ @@ -529,10 +544,13 @@ private function dryRunPlannedChangeLines(SupportCase $case, array $proposedActi * @param array $payload * @return list */ - private function dryRunRoleAddLines(SupportCase $case, array $payload): array + private function dryRunRoleChangeLines(SupportCase $case, array $payload, string $action): array { + $actionName = $action === 'user_role_remove' ? 'user_role_remove' : 'user_role_add'; + $isRemove = $action === 'user_role_remove'; + $output = $case->actions() - ->where('action_name', 'user_role_add') + ->where('action_name', $actionName) ->where('action_type', 'write') ->latest() ->first()?->output_json; @@ -544,7 +562,9 @@ private function dryRunRoleAddLines(SupportCase $case, array $payload): array $lines = [ '', - 'We will add the role "'.$role.'" to the following CodeWeek accounts:', + $isRemove + ? 'We will remove the role "'.$role.'" from the following CodeWeek accounts:' + : 'We will add the role "'.$role.'" to the following CodeWeek accounts:', '', ]; @@ -554,22 +574,26 @@ private function dryRunRoleAddLines(SupportCase $case, array $payload): array $lines[] = ' • '.$email; } $lines[] = ''; - $lines[] = 'We will add the role to each account above that does not already have it.'; + $lines[] = $isRemove + ? 'We will remove the role from each account above that currently has it.' + : 'We will add the role to each account above that does not already have it.'; return $lines; } foreach ($items as $item) { - $lines[] = ' • '.$this->roleItemLine((array) $item); + $lines[] = ' • '.$this->roleItemLine((array) $item, $isRemove); } $lines[] = ''; - $lines[] = 'Summary: '.$this->roleSummaryLine($summary); + $lines[] = 'Summary: '.$this->roleSummaryLine($summary, $isRemove); if (($summary['ambiguous'] ?? 0) > 0 || ($summary['user_not_found'] ?? 0) > 0) { $lines[] = ''; $lines[] = 'Accounts marked "not found" or "needs manual check" will be skipped.'; - $lines[] = 'Approving will still add the role to all accounts that are ready.'; + $lines[] = $isRemove + ? 'Approving will still remove the role from all accounts that are ready.' + : 'Approving will still add the role to all accounts that are ready.'; } return $lines; @@ -578,7 +602,7 @@ private function dryRunRoleAddLines(SupportCase $case, array $payload): array /** * @param array $item */ - private function roleItemLine(array $item): string + private function roleItemLine(array $item, bool $isRemove = false): string { $email = (string) ($item['email'] ?? ''); $status = (string) ($item['status'] ?? ''); @@ -587,6 +611,9 @@ private function roleItemLine(array $item): string 'would_add' => 'will be added', 'added' => 'role added', 'already_has_role' => 'already has this role (no change)', + 'would_remove' => 'will be removed', + 'removed' => 'role removed', + 'does_not_have_role' => 'does not have this role (no change)', 'user_not_found' => 'no CodeWeek account found — skipped', 'ambiguous' => 'multiple accounts match — needs manual check, skipped', default => $status, @@ -598,12 +625,20 @@ private function roleItemLine(array $item): string /** * @param array $summary */ - private function roleSummaryLine(array $summary): string + private function roleSummaryLine(array $summary, bool $isRemove = false): string { - $toAdd = (int) (($summary['would_add'] ?? 0) + ($summary['added'] ?? 0)); - $parts = [$toAdd.' to add']; - if (($summary['already_has_role'] ?? 0) > 0) { - $parts[] = (int) $summary['already_has_role'].' already have it'; + if ($isRemove) { + $toChange = (int) (($summary['would_remove'] ?? 0) + ($summary['removed'] ?? 0)); + $parts = [$toChange.' to remove']; + if (($summary['does_not_have_role'] ?? 0) > 0) { + $parts[] = (int) $summary['does_not_have_role'].' do not have it'; + } + } else { + $toChange = (int) (($summary['would_add'] ?? 0) + ($summary['added'] ?? 0)); + $parts = [$toChange.' to add']; + if (($summary['already_has_role'] ?? 0) > 0) { + $parts[] = (int) $summary['already_has_role'].' already have it'; + } } if (($summary['user_not_found'] ?? 0) > 0) { $parts[] = (int) $summary['user_not_found'].' not found'; @@ -796,6 +831,7 @@ private function completionHeadline(SupportCase $case, string $action, bool $suc 'user_profile_update' => 'Done — name updated on CodeWeek account', 'user_restore' => 'Done — CodeWeek account reactivated', 'user_role_add' => 'Done — user role added', + 'user_role_remove' => 'Done — user role removed', 'code_change' => 'Started — AI coding agent is preparing a PR into dev', 'artisan_command' => 'Done — maintenance command completed on the server', 'content_update' => 'Done — content updated', @@ -840,8 +876,8 @@ private function buildCompletionBody( $lines = array_merge($lines, $this->completionFailureLines($action, $result, $email, $case->id)); } - // Role-add is a batch action; the per-account list is shown above, so skip the single email line. - if ($email !== '' && $action !== 'user_role_add') { + // Role add/remove are batch actions; the per-account list is shown above. + if ($email !== '' && ! in_array($action, ['user_role_add', 'user_role_remove'], true)) { $lines[] = ''; $lines[] = 'Account email: '.$email; } @@ -852,10 +888,10 @@ private function buildCompletionBody( $lines[] = ''; if ($succeeded) { - $lines[] = $action === 'user_role_add' + $lines[] = in_array($action, ['user_role_add', 'user_role_remove'], true) ? 'No further action is needed. You do not need to reply to this email.' : 'No further action is needed. The supporter can sign in with their usual email and password.'; - if ($action !== 'user_role_add') { + if (! in_array($action, ['user_role_add', 'user_role_remove'], true)) { $lines[] = 'You do not need to reply to this email.'; } } else { @@ -978,8 +1014,8 @@ private function completionSuccessLines(SupportCase $case, string $action, array return $lines; } - if ($action === 'user_role_add') { - return $this->completionRoleAddLines($inner); + if ($action === 'user_role_add' || $action === 'user_role_remove') { + return $this->completionRoleChangeLines($inner, $action); } return [ @@ -991,23 +1027,32 @@ private function completionSuccessLines(SupportCase $case, string $action, array * @param array $inner * @return list */ - private function completionRoleAddLines(array $inner): array + private function completionRoleChangeLines(array $inner, string $action): array { + $isRemove = $action === 'user_role_remove'; $role = (string) ($inner['role'] ?? 'the requested role'); $items = is_array($inner['items'] ?? null) ? $inner['items'] : []; $summary = is_array($inner['summary'] ?? null) ? $inner['summary'] : []; - $addedCount = (int) ($summary['added'] ?? 0); + $changedCount = (int) ($isRemove ? ($summary['removed'] ?? 0) : ($summary['added'] ?? 0)); - $lines = [ - $addedCount === 1 + if ($isRemove) { + $headline = $changedCount === 1 + ? 'We removed the role "'.$role.'" from 1 CodeWeek account.' + : 'We removed the role "'.$role.'" from '.$changedCount.' CodeWeek accounts.'; + } else { + $headline = $changedCount === 1 ? 'We added the role "'.$role.'" to 1 CodeWeek account.' - : 'We added the role "'.$role.'" to '.$addedCount.' CodeWeek accounts.', + : 'We added the role "'.$role.'" to '.$changedCount.' CodeWeek accounts.'; + } + + $lines = [ + $headline, '', 'Details:', ]; foreach ($items as $item) { - $lines[] = ' • '.$this->roleItemLine((array) $item); + $lines[] = ' • '.$this->roleItemLine((array) $item, $isRemove); } $skipped = (int) ($summary['user_not_found'] ?? 0) + (int) ($summary['ambiguous'] ?? 0); @@ -1104,9 +1149,10 @@ private function humanizeError(string $code, string $action, int $caseId): strin str_contains($code, 'no_effective_change') => 'The content already matched the requested text — no change was needed.', str_starts_with($code, 'role_not_found') => 'We could not find a matching role. Check the role name (e.g. "leading teacher").', str_starts_with($code, 'role_not_allowed') => 'That role is not on the list the copilot is allowed to add.', - str_contains($code, 'no_role_specified') => 'The request did not say which role to add.', + str_contains($code, 'no_role_specified') => 'The request did not say which role to change.', str_contains($code, 'no_target_emails') => 'The request did not include any valid email addresses.', str_contains($code, 'only_add_operation') => 'Only adding roles is supported right now (not removing).', + str_contains($code, 'only_remove_operation') => 'Only removing roles is supported for this action.', $action === 'user_restore' && str_contains($code, 'verification') => 'The account was changed but we could not confirm it is fully active. Please verify in Nova.', default => 'Technical detail: '.$code, }; diff --git a/app/Services/Support/SupportRoleRequestParser.php b/app/Services/Support/SupportRoleRequestParser.php index b3deff91c..b759bc078 100644 --- a/app/Services/Support/SupportRoleRequestParser.php +++ b/app/Services/Support/SupportRoleRequestParser.php @@ -61,6 +61,12 @@ private function parseLabelled(string $text): array } elseif (preg_match('/(?:^|\n)\s*(?:remove|revoke)\s+role\s*[:\-]\s*([^\n\r]+)/iu', $text, $m)) { $role = $this->cleanRole($m[1]); $operation = 'remove'; + } elseif (preg_match('/\b(?:remove|revoke|delete)\s+(?:the\s+)?["\']?([^"\'\n\r]+?)["\']?\s+role\b/iu', $text, $m)) { + $role = $this->cleanRole($m[1]); + $operation = 'remove'; + } elseif (preg_match('/\b(?:remove|revoke|delete)\s+the\s+role\s+["\']?([^"\'\n\r]+?)["\']?\s+from\b/iu', $text, $m)) { + $role = $this->cleanRole($m[1]); + $operation = 'remove'; } elseif (preg_match('/(?:^|\n)\s*role\s*(?:to\s+(add|remove))?\s*[:\-]\s*([^\n\r]+)/iu', $text, $m)) { $operation = strtolower(trim($m[1] ?? '')) === 'remove' ? 'remove' : 'add'; $role = $this->cleanRole($m[2]); diff --git a/app/Services/Support/UserRoleRemoveService.php b/app/Services/Support/UserRoleRemoveService.php new file mode 100644 index 000000000..f4af1accb --- /dev/null +++ b/app/Services/Support/UserRoleRemoveService.php @@ -0,0 +1,218 @@ +resolver->resolve($case); + + return $this->removeRole( + case: $case, + roleName: $resolved['role'], + emails: $resolved['emails'], + dryRun: $dryRun, + viaEmailApproval: $viaEmailApproval, + operation: $resolved['operation'], + ); + } + + /** + * @param list $emails + */ + public function removeRole( + SupportCase $case, + ?string $roleName, + array $emails, + bool $dryRun, + bool $viaEmailApproval = false, + string $operation = 'remove', + ): array { + $tool = 'user_role_remove'; + $input = [ + 'operation' => $operation, + 'role' => $roleName, + 'emails' => $emails, + 'dry_run' => $dryRun, + ]; + + if ($operation !== 'remove') { + return SupportJson::fail($tool, $input, 'only_remove_operation_supported'); + } + + if ($roleName === null || trim($roleName) === '') { + return SupportJson::fail($tool, $input, 'no_role_specified'); + } + + $role = $this->resolveRole($roleName); + if ($role === null) { + return SupportJson::fail($tool, $input, 'role_not_found:'.$roleName); + } + + if (! $this->roleIsAllowed($role->name)) { + return SupportJson::fail($tool, $input, 'role_not_allowed:'.$role->name); + } + + $emails = $this->normalizeEmails($emails); + if ($emails === []) { + return SupportJson::fail($tool, $input, 'no_target_emails'); + } + + if (! $dryRun && config('support_gmail.dry_run') && ! $viaEmailApproval) { + return SupportJson::fail($tool, $input, 'dry_run_mode_requires_email_approval'); + } + + $items = []; + $counts = ['removed' => 0, 'would_remove' => 0, 'does_not_have_role' => 0, 'user_not_found' => 0, 'ambiguous' => 0]; + + foreach ($emails as $email) { + $item = $this->processEmail($case, $role, $email, $dryRun); + $items[] = $item; + $counts[$item['status']] = ($counts[$item['status']] ?? 0) + 1; + } + + $warnings = []; + if ($this->roleLooksPrivileged($role->name)) { + $warnings[] = 'privileged_role_removed:'.$role->name; + } + + return SupportJson::ok($tool, $input, [ + 'operation' => 'remove', + 'role' => $role->name, + 'dry_run' => $dryRun, + 'summary' => $counts, + 'items' => $items, + ], $warnings); + } + + /** + * @return array{email: string, status: string, user_id: ?int, matched_user_ids: list} + */ + private function processEmail(SupportCase $case, Role $role, string $email, bool $dryRun): array + { + $matches = User::query() + ->whereRaw('LOWER(email) = ?', [$email]) + ->orWhereRaw('LOWER(email_display) = ?', [$email]) + ->get(); + + if ($matches->isEmpty()) { + return ['email' => $email, 'status' => 'user_not_found', 'user_id' => null, 'matched_user_ids' => []]; + } + + if ($matches->count() > 1) { + return [ + 'email' => $email, + 'status' => 'ambiguous', + 'user_id' => null, + 'matched_user_ids' => $matches->pluck('id')->map(fn ($id) => (int) $id)->values()->all(), + ]; + } + + /** @var User $user */ + $user = $matches->first(); + + if (! $user->hasRole($role->name)) { + return ['email' => $email, 'status' => 'does_not_have_role', 'user_id' => (int) $user->id, 'matched_user_ids' => [(int) $user->id]]; + } + + if ($dryRun) { + return ['email' => $email, 'status' => 'would_remove', 'user_id' => (int) $user->id, 'matched_user_ids' => [(int) $user->id]]; + } + + DB::transaction(fn () => $user->removeRole($role->name)); + + return ['email' => $email, 'status' => 'removed', 'user_id' => (int) $user->id, 'matched_user_ids' => [(int) $user->id]]; + } + + private function resolveRole(string $roleName): ?Role + { + $guard = (string) config('auth.defaults.guard', 'web'); + $needle = $this->normalizeRoleName($roleName); + + $roles = Role::query()->get(); + + $exact = $roles->first(fn (Role $r) => $this->normalizeRoleName($r->name) === $needle && $r->guard_name === $guard) + ?? $roles->first(fn (Role $r) => $this->normalizeRoleName($r->name) === $needle); + if ($exact !== null) { + return $exact; + } + + $singular = $this->singularizeRoleName($needle); + $bySingular = $roles->first(fn (Role $r) => $this->singularizeRoleName($this->normalizeRoleName($r->name)) === $singular && $r->guard_name === $guard) + ?? $roles->first(fn (Role $r) => $this->singularizeRoleName($this->normalizeRoleName($r->name)) === $singular); + + return $bySingular; + } + + private function roleIsAllowed(string $roleName): bool + { + $allowed = config('support_gmail.role_add_allowed_roles', []); + $allowed = is_array($allowed) ? array_filter(array_map('trim', $allowed)) : []; + + if ($allowed === []) { + return true; + } + + $needle = $this->normalizeRoleName($roleName); + foreach ($allowed as $candidate) { + if ($this->normalizeRoleName((string) $candidate) === $needle) { + return true; + } + } + + return false; + } + + private function roleLooksPrivileged(string $roleName): bool + { + return (bool) preg_match('/\b(admin|administrator|super|owner|root|staff)\b/i', $roleName); + } + + private function normalizeRoleName(string $roleName): string + { + return Str::of($roleName)->lower()->replaceMatches('/\s+/', ' ')->trim()->toString(); + } + + private function singularizeRoleName(string $roleName): string + { + $words = explode(' ', $roleName); + $last = array_pop($words); + if ($last !== null) { + $words[] = Str::singular($last); + } + + return implode(' ', $words); + } + + /** + * @param list $emails + * @return list + */ + private function normalizeEmails(array $emails): array + { + $out = []; + foreach ($emails as $email) { + $normalized = SupportEmailAddress::normalize((string) $email); + if ($normalized !== null && $normalized !== '' && filter_var($normalized, FILTER_VALIDATE_EMAIL)) { + $out[$normalized] = $normalized; + } + } + + return array_values($out); + } +} diff --git a/config/support_gmail.php b/config/support_gmail.php index 23e7f1df2..1268c04db 100644 --- a/config/support_gmail.php +++ b/config/support_gmail.php @@ -78,6 +78,7 @@ 'user_restore', 'user_profile_update', 'user_role_add', + 'user_role_remove', 'code_change', 'artisan_command', 'content_update', diff --git a/tests/Unit/Support/SupportRoleRequestParserTest.php b/tests/Unit/Support/SupportRoleRequestParserTest.php index 5eac90887..fd92e2986 100644 --- a/tests/Unit/Support/SupportRoleRequestParserTest.php +++ b/tests/Unit/Support/SupportRoleRequestParserTest.php @@ -50,6 +50,21 @@ public function test_returns_null_role_when_no_role_present(): void $this->assertNull($parsed['role']); } + public function test_parses_natural_language_remove_request(): void + { + $text = <<<'TEXT' + Please remove the "ambassador" role from the following user. + + Veronica Ward vward@eircom.net + TEXT; + + $parsed = (new SupportRoleRequestParser())->parse($text); + + $this->assertSame('remove', $parsed['operation']); + $this->assertSame('ambassador', $parsed['role']); + $this->assertSame(['vward@eircom.net'], $parsed['emails']); + } + public function test_deduplicates_emails(): void { $text = "add role: leading teacher\nx@example.com\nX@Example.com\n"; diff --git a/tests/Unit/Support/UserRoleRemoveServiceTest.php b/tests/Unit/Support/UserRoleRemoveServiceTest.php new file mode 100644 index 000000000..79d6e56cc --- /dev/null +++ b/tests/Unit/Support/UserRoleRemoveServiceTest.php @@ -0,0 +1,70 @@ + 'manual', + 'processing_mode' => 'manual', + 'subject' => 'codeweek-support — remove ambassador role', + 'raw_message' => 'test', + 'status' => 'investigating', + 'risk_level' => 'low', + 'correlation_id' => 'cid', + ]); + } + + public function test_dry_run_plans_role_removals_without_applying(): void + { + Role::create(['name' => 'ambassador', 'guard_name' => 'web']); + $user = User::factory()->create(['email' => 'amb@example.com']); + $user->assignRole('ambassador'); + + $result = app(UserRoleRemoveService::class)->removeRole( + case: $this->makeCase(), + roleName: 'ambassadors', + emails: ['amb@example.com', 'missing@example.com'], + dryRun: true, + ); + + $this->assertTrue($result['ok']); + $this->assertSame('ambassador', $result['result']['role']); + $this->assertSame(1, $result['result']['summary']['would_remove']); + $this->assertSame(1, $result['result']['summary']['user_not_found']); + $this->assertTrue($user->fresh()->hasRole('ambassador')); + } + + public function test_apply_removes_role_from_existing_users(): void + { + config(['support_gmail.dry_run' => true]); + Role::create(['name' => 'ambassador', 'guard_name' => 'web']); + $a = User::factory()->create(['email' => 'a@example.com']); + $b = User::factory()->create(['email' => 'b@example.com']); + $a->assignRole('ambassador'); + + $result = app(UserRoleRemoveService::class)->removeRole( + case: $this->makeCase(), + roleName: 'ambassador', + emails: ['a@example.com', 'b@example.com'], + dryRun: false, + viaEmailApproval: true, + ); + + $this->assertTrue($result['ok']); + $this->assertSame(1, $result['result']['summary']['removed']); + $this->assertSame(1, $result['result']['summary']['does_not_have_role']); + $this->assertFalse($a->fresh()->hasRole('ambassador')); + } +}