diff --git a/app/Jobs/Support/ExecuteApprovedSupportActionJob.php b/app/Jobs/Support/ExecuteApprovedSupportActionJob.php index bd899eddd..5435d42ca 100644 --- a/app/Jobs/Support/ExecuteApprovedSupportActionJob.php +++ b/app/Jobs/Support/ExecuteApprovedSupportActionJob.php @@ -79,14 +79,8 @@ public function handle( viaEmailApproval: true, ); } elseif ($action === 'user_profile_update') { - $result = $userProfileUpdate->updateProfile( - case: $case, - email: (string) ($payload['email'] ?? ''), - firstname: isset($payload['firstname']) ? (string) $payload['firstname'] : null, - lastname: isset($payload['lastname']) ? (string) $payload['lastname'] : null, - dryRun: false, - viaEmailApproval: true, - ); + // Re-read names from the case email (approval payload may be from an older parser). + $result = $userProfileUpdate->updateFromCase($case, dryRun: false, viaEmailApproval: true); } else { $result = [ 'ok' => false, diff --git a/app/Services/Support/SupportApprovalEmailService.php b/app/Services/Support/SupportApprovalEmailService.php index a91571874..738a13d16 100644 --- a/app/Services/Support/SupportApprovalEmailService.php +++ b/app/Services/Support/SupportApprovalEmailService.php @@ -625,6 +625,9 @@ private function completionFailureLines(string $action, array $result, string $e $lines[] = ' • An unexpected error occurred. Please check the case in Nova.'; } else { foreach ($errors as $error) { + if (str_contains(strtolower((string) $error), 'matched_user_ids')) { + continue; + } $lines[] = ' • '.$this->humanizeError((string) $error, $action, $caseId); } } diff --git a/app/Services/Support/SupportProfileRequestParser.php b/app/Services/Support/SupportProfileRequestParser.php index 46d65a634..8d7b1c07d 100644 --- a/app/Services/Support/SupportProfileRequestParser.php +++ b/app/Services/Support/SupportProfileRequestParser.php @@ -23,7 +23,13 @@ final class SupportProfileRequestParser ]; /** - * @return array{email: ?string, firstname: ?string, lastname: ?string} + * @return array{ + * email: ?string, + * firstname: ?string, + * lastname: ?string, + * current_firstname: ?string, + * current_lastname: ?string, + * } */ public function parse(string $text): array { @@ -33,6 +39,8 @@ public function parse(string $text): array 'email' => $this->extractFirstEmail($normalized), 'firstname' => $this->sanitizeName($this->extractRequestedName($normalized, 'first')), 'lastname' => $this->sanitizeName($this->extractRequestedName($normalized, 'last')), + 'current_firstname' => $this->sanitizeName($this->extractCurrentName($normalized, 'first')), + 'current_lastname' => $this->sanitizeName($this->extractCurrentName($normalized, 'last')), ]; } @@ -45,10 +53,23 @@ private function extractFirstEmail(string $text): ?string } private function extractRequestedName(string $text, string $which): ?string + { + return $this->extractPrefixedName($text, $which, ['requested', 'new']); + } + + private function extractCurrentName(string $text, string $which): ?string + { + return $this->extractPrefixedName($text, $which, ['current']); + } + + /** + * @param list $prefixes + */ + private function extractPrefixedName(string $text, string $which, array $prefixes): ?string { $field = $which === 'first' ? 'first\\s*name' : 'last\\s*name'; - foreach (['requested', 'new'] as $prefix) { + foreach ($prefixes as $prefix) { $pattern = '/(?:^|\n)\s*'.preg_quote($prefix, '/').'\s+'.$field.'\s*[:*]?\s*([^\n\r]+)/iu'; if (preg_match($pattern, $text, $m)) { return $this->cleanCapturedName($m[1]); diff --git a/app/Services/Support/UserProfileUpdateService.php b/app/Services/Support/UserProfileUpdateService.php index ef676079e..9af624b80 100644 --- a/app/Services/Support/UserProfileUpdateService.php +++ b/app/Services/Support/UserProfileUpdateService.php @@ -30,6 +30,8 @@ public function updateFromCase(SupportCase $case, bool $dryRun, bool $viaEmailAp lastname: $parsed['lastname'], dryRun: $dryRun, viaEmailApproval: $viaEmailApproval, + currentFirstname: $parsed['current_firstname'] ?? null, + currentLastname: $parsed['current_lastname'] ?? null, ); } @@ -40,6 +42,8 @@ public function updateProfile( ?string $lastname, bool $dryRun, bool $viaEmailApproval = false, + ?string $currentFirstname = null, + ?string $currentLastname = null, ): array { $tool = 'user_profile_update'; $input = [ @@ -67,7 +71,13 @@ public function updateProfile( return SupportJson::fail($tool, $input, 'no_matching_user_found'); } - $user = $this->resolveUserForProfileUpdate($matches, $firstname, $lastname); + $user = $this->resolveUserForProfileUpdate( + $matches, + $firstname, + $lastname, + $currentFirstname, + $currentLastname, + ); if ($user === null) { return SupportJson::fail($tool, $input, [ 'ambiguous_user_match', @@ -159,8 +169,13 @@ public function updateProfile( * * @param Collection $matches */ - private function resolveUserForProfileUpdate(Collection $matches, ?string $firstname, ?string $lastname): ?User - { + private function resolveUserForProfileUpdate( + Collection $matches, + ?string $firstname, + ?string $lastname, + ?string $currentFirstname = null, + ?string $currentLastname = null, + ): ?User { if ($matches->count() === 1) { return $matches->first(); } @@ -172,6 +187,18 @@ private function resolveUserForProfileUpdate(Collection $matches, ?string $first $candidates = $active->isNotEmpty() ? $active : $matches->values(); + if ($currentFirstname !== null || $currentLastname !== null) { + $byCurrent = $candidates->filter( + fn (User $user) => $this->userMatchesCurrentNames($user, $currentFirstname, $currentLastname), + )->values(); + if ($byCurrent->count() === 1) { + return $byCurrent->first(); + } + if ($byCurrent->isNotEmpty()) { + $candidates = $byCurrent; + } + } + $needingUpdate = $candidates->filter(function (User $user) use ($firstname, $lastname) { if ($firstname !== null && $firstname !== $user->firstname) { return true; @@ -187,9 +214,77 @@ private function resolveUserForProfileUpdate(Collection $matches, ?string $first return $needingUpdate->first(); } + if ($needingUpdate->count() > 1) { + return $this->pickUserWithFewestChanges($needingUpdate, $firstname, $lastname); + } + return null; } + private function userMatchesCurrentNames(User $user, ?string $currentFirstname, ?string $currentLastname): bool + { + if ($currentFirstname !== null && ! $this->namesEqual($currentFirstname, $user->firstname)) { + return false; + } + if ($currentLastname !== null && ! $this->namesEqual($currentLastname, $user->lastname)) { + return false; + } + + return true; + } + + /** + * @param Collection $candidates + */ + private function pickUserWithFewestChanges(Collection $candidates, ?string $firstname, ?string $lastname): ?User + { + $ranked = $candidates->map(function (User $user) use ($firstname, $lastname) { + $fieldsToChange = 0; + if ($firstname !== null && $firstname !== $user->firstname) { + $fieldsToChange++; + } + if ($lastname !== null && $lastname !== $user->lastname) { + $fieldsToChange++; + } + + $tieScore = 0; + if ($firstname !== null && $this->namesEqual($firstname, $user->firstname)) { + $tieScore += 2; + } + if ($lastname !== null && $user->lastname !== '' && $user->lastname !== 'Last Name') { + $tieScore += 1; + } + + return ['user' => $user, 'fields_to_change' => $fieldsToChange, 'tie_score' => $tieScore]; + })->sort(function (array $a, array $b): int { + if ($a['fields_to_change'] !== $b['fields_to_change']) { + return $a['fields_to_change'] <=> $b['fields_to_change']; + } + if ($a['tie_score'] !== $b['tie_score']) { + return $b['tie_score'] <=> $a['tie_score']; + } + + return $b['user']->id <=> $a['user']->id; + })->values(); + + if ($ranked->isEmpty()) { + return null; + } + + $best = $ranked->first(); + $sameRank = $ranked->filter( + fn (array $row) => $row['fields_to_change'] === $best['fields_to_change'] + && $row['tie_score'] === $best['tie_score'], + ); + + return $sameRank->count() === 1 ? $best['user'] : null; + } + + private function namesEqual(?string $a, ?string $b): bool + { + return Str::lower(trim((string) $a)) === Str::lower(trim((string) $b)); + } + private function isValidEmail(string $email): bool { return filter_var($email, FILTER_VALIDATE_EMAIL) !== false; diff --git a/tests/Unit/Support/SupportProfileRequestParserTest.php b/tests/Unit/Support/SupportProfileRequestParserTest.php index ff201cff8..8d864754c 100644 --- a/tests/Unit/Support/SupportProfileRequestParserTest.php +++ b/tests/Unit/Support/SupportProfileRequestParserTest.php @@ -34,6 +34,22 @@ public function test_rejects_placeholder_last_name(): void $this->assertNull($parsed['lastname']); } + public function test_parses_current_and_requested_names(): void + { + $text = <<<'TEXT' + Current first name: Bernard + Current last name: Hanna + Requested first name: Bernard + Requested last name: Hannaa + TEXT; + + $parsed = (new SupportProfileRequestParser())->parse($text); + + $this->assertSame('Bernard', $parsed['current_firstname']); + $this->assertSame('Hanna', $parsed['current_lastname']); + $this->assertSame('Hannaa', $parsed['lastname']); + } + public function test_parses_hanna_to_hannaa_request_without_bleeding_lines(): void { $text = <<<'TEXT' diff --git a/tests/Unit/Support/UserProfileUpdateServiceTest.php b/tests/Unit/Support/UserProfileUpdateServiceTest.php index 284225c95..545a221c5 100644 --- a/tests/Unit/Support/UserProfileUpdateServiceTest.php +++ b/tests/Unit/Support/UserProfileUpdateServiceTest.php @@ -79,6 +79,47 @@ public function test_profile_update_resolves_duplicate_email_to_user_needing_cha $this->assertStringContainsString('resolved_duplicate_email_to_user_id:', (string) ($payload['warnings'][0] ?? '')); } + public function test_hanna_to_hannaa_resolves_duplicate_by_fewest_field_changes(): void + { + /** @var User $correct */ + $correct = User::factory()->create([ + 'email' => 'dup@example.com', + 'firstname' => 'Bernard', + 'lastname' => 'Hanna', + ]); + User::factory()->create([ + 'email' => 'dup@example.com', + 'firstname' => 'Bernard Hanna', + 'lastname' => '', + ]); + + $case = SupportCase::create([ + 'source_channel' => 'manual', + 'processing_mode' => 'manual', + 'subject' => 'test', + 'raw_message' => 'test', + 'status' => 'investigating', + 'risk_level' => 'low', + 'correlation_id' => 'cid', + ]); + + $payload = app(UserProfileUpdateService::class)->updateProfile( + $case, + 'dup@example.com', + 'Bernard', + 'Hannaa', + dryRun: false, + viaEmailApproval: true, + currentFirstname: 'Bernard', + currentLastname: 'Hanna', + ); + + $this->assertTrue($payload['ok']); + $correct->refresh(); + $this->assertSame('Hannaa', $correct->lastname); + $this->assertSame($correct->id, $payload['result']['before']['user_id'] ?? null); + } + public function test_profile_update_execute_changes_user(): void { config(['support_gmail.dry_run' => true]);