From 17c5f0209eb57dd2a88147395b0e417074736f1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Maia?= Date: Sun, 6 Sep 2026 22:53:07 -0300 Subject: [PATCH 1/3] fix(collaborators): detect that the signer search has more results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The limit was incremented before the mapper call and the extra row was then looked for beyond that same incremented limit. As IdentifyMethodMapper::searchByIdentifierValue() applies setMaxResults(), it can never return more rows than it was asked for, so the condition was never true: the plugin never reported more results and never removed the extra row it had asked for. A search with more signers than the page could show returned one item too many, and that item appeared again at the top of the next page, because the offset is applied in SQL. The mapper is still asked for one row beyond the page, and that row is now what tells the plugin there is more to show, in the same way AccountPhonePlugin and ContactPhonePlugin do it. The previous pagination test could not catch this: it returned 31 rows from a mapper call made with a limit of 26. The scenarios now stay within what the mapper can answer and cover the boundary, where the number of rows found is exactly the size of the page. Ref #8053 Assisted-by: Claude Code:claude-opus-5 Signed-off-by: André Maia --- .../Collaborators/SignerPlugin.php | 8 ++-- .../Collaborators/SignerPluginTest.php | 43 ++++++++++++++++--- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/lib/Collaboration/Collaborators/SignerPlugin.php b/lib/Collaboration/Collaborators/SignerPlugin.php index 2f87a37d8f..10792cca0a 100644 --- a/lib/Collaboration/Collaborators/SignerPlugin.php +++ b/lib/Collaboration/Collaborators/SignerPlugin.php @@ -37,20 +37,18 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b $user = $this->userSession->getUser()->getUID(); $method = $this->getMethod(); - $limit++; $identifiers = $this->identifyMethodMapper->searchByIdentifierValue( $search, $user, $method, - $limit, + $limit + 1, $offset, ); $result = ['wide' => [], 'exact' => []]; - $hasMore = false; - if (count($identifiers) > $limit) { - $hasMore = true; + $hasMore = count($identifiers) > $limit; + if ($hasMore) { array_pop($identifiers); } diff --git a/tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php b/tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php index b922edbb6a..c0c72f50ee 100644 --- a/tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php +++ b/tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php @@ -159,9 +159,15 @@ public function testSearchIsCaseInsensitive(): void { $this->assertCount(1, $exactResults, 'Should match case-insensitively'); } - public function testSearchHandlesPagination(): void { + #[DataProvider('providerPaginationScenarios')] + public function testSearchTrimsTheRowUsedToDetectMoreResults( + int $limit, + int $rowsFound, + bool $expectedHasMore, + int $expectedCount, + ): void { $identifiers = []; - for ($i = 0; $i < 31; $i++) { + for ($i = 0; $i < $rowsFound; $i++) { $identifiers[] = [ 'identifier_value' => 'user' . $i . '@example.com', 'identifier_key' => 'email', @@ -170,8 +176,9 @@ public function testSearchHandlesPagination(): void { } $mapper = $this->createMock(IdentifyMethodMapper::class); - $mapper->method('searchByIdentifierValue') - ->with('user', 'current', 'email', 26, 0) + $mapper->expects($this->once()) + ->method('searchByIdentifierValue') + ->with('user', 'current', 'email', $limit + 1, 0) ->willReturn($identifiers); $user = $this->createMock(IUser::class); @@ -190,13 +197,13 @@ public function testSearchHandlesPagination(): void { ); $searchResult = new SearchResult(); - $hasMore = $plugin->search('user', 25, 0, $searchResult); + $hasMore = $plugin->search('user', $limit, 0, $searchResult); $results = $searchResult->asArray(); $allResults = array_merge($results['signer'] ?? [], $results['exact']['signer'] ?? []); - $this->assertTrue($hasMore, 'Should indicate more results available'); - $this->assertCount(30, $allResults, 'Should return all results after trimming one'); + $this->assertSame($expectedHasMore, $hasMore); + $this->assertCount($expectedCount, $allResults); } public function testSearchRespectOffset(): void { @@ -407,4 +414,26 @@ public static function providerSearchScenarios(): array { ], ]; } + public static function providerPaginationScenarios(): array { + return [ + 'one row more than the page can show' => [ + 'limit' => 25, + 'rowsFound' => 26, + 'expectedHasMore' => true, + 'expectedCount' => 25, + ], + 'exactly one page' => [ + 'limit' => 25, + 'rowsFound' => 25, + 'expectedHasMore' => false, + 'expectedCount' => 25, + ], + 'less than one page' => [ + 'limit' => 25, + 'rowsFound' => 10, + 'expectedHasMore' => false, + 'expectedCount' => 10, + ], + ]; + } } From d4d5806faa1267c9ea2e8d87499158f495caa233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Maia?= Date: Sun, 6 Sep 2026 22:53:21 -0300 Subject: [PATCH 2/3] refactor(collaborators): drop the unused method of the signer plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit canValidateMethod() was private and had no caller anywhere in the project. It never ran, so no test and no mutant could reach it. Ref #8053 Assisted-by: Claude Code:claude-opus-5 Signed-off-by: André Maia --- lib/Collaboration/Collaborators/SignerPlugin.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/Collaboration/Collaborators/SignerPlugin.php b/lib/Collaboration/Collaborators/SignerPlugin.php index 10792cca0a..5f68cdeef2 100644 --- a/lib/Collaboration/Collaborators/SignerPlugin.php +++ b/lib/Collaboration/Collaborators/SignerPlugin.php @@ -69,10 +69,6 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b return $hasMore; } - private function canValidateMethod(string $method): bool { - return in_array($method, ['email', 'account'], true); - } - private function rowToSearchResultItem(array $row): array { $item = [ 'label' => $row['display_name'], From 54d28f019ef58014a031ca1af50e4a5f5776008d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Maia?= Date: Sun, 6 Sep 2026 22:53:41 -0300 Subject: [PATCH 3/3] test: cover the signer search payload and its case-insensitive matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Infection showed that nothing asserted the label and the value offered for a signer, so the display name and the identifier could be dropped from the result item without any test noticing. It also showed that the search term itself was never written in a different case than the stored data, so lowercasing it was not covered on either side of the comparison. The scenarios now cover an identifier stored in another case, a search typed in another case, and a match by display name where both are written differently. Ref #8053 Assisted-by: Claude Code:claude-opus-5 Signed-off-by: André Maia --- .../Collaborators/SignerPluginTest.php | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php b/tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php index c0c72f50ee..35731365b2 100644 --- a/tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php +++ b/tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php @@ -126,14 +126,11 @@ public function testSearchSeparatesExactAndWideMatches(): void { $this->assertCount(2, $exactResults, 'Should have 2 exact results (matched by value and display name)'); } - public function testSearchIsCaseInsensitive(): void { - $identifiers = [ - ['identifier_value' => 'Test@Example.COM', 'identifier_key' => 'email', 'display_name' => 'Test User'], - ]; - + #[DataProvider('providerCaseInsensitiveMatches')] + public function testSearchIsCaseInsensitive(string $search, array $row): void { $mapper = $this->createMock(IdentifyMethodMapper::class); $mapper->method('searchByIdentifierValue') - ->willReturn($identifiers); + ->willReturn([$row]); $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('current'); @@ -142,7 +139,7 @@ public function testSearchIsCaseInsensitive(): void { $userSession->method('getUser')->willReturn($user); $context = new SignerSearchContext(); - $context->set('email', 'test@example.com'); + $context->set('email', $search); $plugin = new SignerPlugin( $mapper, @@ -151,7 +148,7 @@ public function testSearchIsCaseInsensitive(): void { ); $searchResult = new SearchResult(); - $plugin->search('test@example.com', 25, 0, $searchResult); + $plugin->search($search, 25, 0, $searchResult); $results = $searchResult->asArray(); $exactResults = $results['exact']['signer'] ?? []; @@ -278,7 +275,7 @@ public function testSearchIncludesMethodInResult(): void { $this->assertSame('sms', $items[0]['method']); } - public function testSearchReturnsCorrectShareType(): void { + public function testSearchDescribesTheSignerFoundInTheIdentifyMethods(): void { $identifiers = [ ['identifier_value' => 'test@example.com', 'identifier_key' => 'email', 'display_name' => 'Test User'], ]; @@ -309,6 +306,10 @@ public function testSearchReturnsCorrectShareType(): void { $items = array_merge($results['signer'] ?? [], $results['exact']['signer'] ?? []); $this->assertCount(1, $items); + $this->assertSame('Test User', $items[0]['label']); + $this->assertSame('test@example.com', $items[0]['shareWithDisplayNameUnique']); + $this->assertSame('email', $items[0]['method']); + $this->assertSame('test@example.com', $items[0]['value']['shareWith']); $this->assertSame(SignerPlugin::TYPE_SIGNER, $items[0]['value']['shareType']); } @@ -414,6 +415,24 @@ public static function providerSearchScenarios(): array { ], ]; } + + public static function providerCaseInsensitiveMatches(): array { + return [ + 'stored identifier written in another case' => [ + 'search' => 'test@example.com', + 'row' => ['identifier_value' => 'Test@Example.COM', 'identifier_key' => 'email', 'display_name' => 'Test User'], + ], + 'search typed in another case' => [ + 'search' => 'TEST@Example.com', + 'row' => ['identifier_value' => 'test@example.com', 'identifier_key' => 'email', 'display_name' => 'Test User'], + ], + 'display name and search written in different cases' => [ + 'search' => 'MARIA SILVA', + 'row' => ['identifier_value' => 'someone@example.com', 'identifier_key' => 'email', 'display_name' => 'Maria Silva'], + ], + ]; + } + public static function providerPaginationScenarios(): array { return [ 'one row more than the page can show' => [