Skip to content

[stable33] fix: detect that the signer search has more results #8288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vitormattos merged 3 commits into from
Sep 7, 2026
+67 −25
Merged

[stable33] fix: detect that the signer search has more results #8288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17c5f02
fix(collaborators): detect that the signer search has more results
maia-andre Sep 7, 2026
d4d5806
refactor(collaborators): drop the unused method of the signer plugin
maia-andre Sep 7, 2026
54d28f0
test: cover the signer search payload and its case-insensitive matching
maia-andre Sep 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions lib/Collaboration/Collaborators/SignerPlugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -71,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'],
Expand Down
80 changes: 64 additions & 16 deletions tests/php/Unit/Collaboration/Collaborators/SignerPluginTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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,
Expand All @@ -151,17 +148,23 @@ 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'] ?? [];

$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',
Expand All @@ -170,8 +173,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);
Expand All @@ -190,13 +194,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 {
Expand Down Expand Up @@ -271,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'],
];
Expand Down Expand Up @@ -302,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']);
}

Expand Down Expand Up @@ -407,4 +415,44 @@ 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' => [
'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,
],
];
}
}
Loading