dont show remote and email options if we have an exact match for loca…

…l user email Signed-off-by: Robin Appelman <robin@icewind.nl>
nextcloud · Dec 4, 2019 · 55b78fc · 55b78fc
1 parent 19935a6
commit 55b78fc
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 21 deletions.
diff --git a/lib/private/Collaboration/Collaborators/Search.php b/lib/private/Collaboration/Collaborators/Search.php
@@ -85,6 +85,13 @@ public function search($search, array $shareTypes, $lookup, $limit, $offset) {
 			$searchResult->unsetResult($emailType);
 		}
 
+		// if we have an exact local user match, there is no need to show the remote and email matches
+		$userType = new SearchResultType('users');
+		if($searchResult->hasExactIdMatch($userType)) {
+			$searchResult->unsetResult($remoteType);
+			$searchResult->unsetResult($emailType);
+		}
+
 		return [$searchResult->asArray(), (bool)$hasMoreResults];
 	}
 

diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php
@@ -71,16 +71,17 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
 			foreach ($userGroups as $userGroup) {
 				$usersTmp = $this->groupManager->displayNamesInGroup($userGroup, $search, $limit, $offset);
 				foreach ($usersTmp as $uid => $userDisplayName) {
-					$users[(string) $uid] = $userDisplayName;
+					$users[(string) $uid] = $this->userManager->get($uid);
 				}
 			}
 		} else {
 			// Search in all users
 			$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
 
 			foreach ($usersTmp as $user) {
-				if ($user->isEnabled()) { // Don't keep deactivated users
-					$users[(string) $user->getUID()] = $user->getDisplayName();
+				if ($user->isEnabled()) {
+					// Don't keep deactivated users
+					$users[(string) $user->getUID()] = $user;
 				}
 			}
 		}
@@ -93,12 +94,19 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
 
 		$foundUserById = false;
 		$lowerSearch = strtolower($search);
-		foreach ($users as $uid => $userDisplayName) {
+		foreach ($users as $uid => $user) {
 			$uid = (string) $uid;
-			if (strtolower($uid) === $lowerSearch || strtolower($userDisplayName) === $lowerSearch) {
+			$userDisplayName = $user->getDisplayName();
+			$userEmail = $user->getEMailAddress();
+			if (
+				strtolower($uid) === $lowerSearch ||
+				strtolower($userDisplayName) === $lowerSearch ||
+				strtolower($userEmail) === $lowerSearch
+			) {
 				if (strtolower($uid) === $lowerSearch) {
 					$foundUserById = true;
 				}
+
 				$result['exact'][] = [
 					'label' => $userDisplayName,
 					'value' => [
@@ -148,6 +156,9 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
 
 		$type = new SearchResultType('users');
 		$searchResult->addResultSet($type, $result['wide'], $result['exact']);
+		if (!empty($result['exact'])) {
+			$searchResult->markExactIdMatch($type);
+		}
 
 		return $hasMoreResults;
 	}

diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php
@@ -255,7 +255,10 @@ public function dataGetUsers() {
 					['label' => 'Test One', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
 				],
 				true,
-				false,
+				[
+					['test', null],
+					['test1', $this->getUserMock('test1', 'Test One')],
+				],
 			],
 			[
 				'test',
@@ -269,7 +272,10 @@ public function dataGetUsers() {
 				[],
 				[],
 				true,
-				false,
+				[
+					['test', null],
+					['test1', $this->getUserMock('test1', 'Test One')],
+				],
 			],
 			[
 				'test',
@@ -292,7 +298,13 @@ public function dataGetUsers() {
 					['label' => 'Test Two', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test2']],
 				],
 				false,
-				false,
+				[
+					['test', null],
+					['test1', $this->getUserMock('test1', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+					['test1', $this->getUserMock('test1', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+				],
 			],
 			[
 				'test',
@@ -312,7 +324,13 @@ public function dataGetUsers() {
 				[],
 				[],
 				true,
-				false,
+				[
+					['test', null],
+					['test1', $this->getUserMock('test1', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+					['test1', $this->getUserMock('test1', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+				],
 			],
 			[
 				'test',
@@ -334,7 +352,10 @@ public function dataGetUsers() {
 					['label' => 'Test Two', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test2']],
 				],
 				false,
-				false,
+				[
+					['test', $this->getUserMock('test', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+				],
 			],
 			[
 				'test',
@@ -354,7 +375,10 @@ public function dataGetUsers() {
 				],
 				[],
 				true,
-				false,
+				[
+					['test', $this->getUserMock('test', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+				],
 			],
 		];
 	}
@@ -370,7 +394,7 @@ public function dataGetUsers() {
 	 * @param array $exactExpected
 	 * @param array $expected
 	 * @param bool $reachedEnd
-	 * @param bool|IUser $singleUser
+	 * @param array|bool $users
 	 */
 	public function testSearch(
 		$searchTerm,
@@ -381,7 +405,7 @@ public function testSearch(
 		array $exactExpected,
 		array $expected,
 		$reachedEnd,
-		$singleUser
+		$users
 	) {
 		$this->config->expects($this->any())
 			->method('getAppValue')
@@ -391,7 +415,8 @@ function($appName, $key, $default)
 				{
 					if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') {
 						return $shareWithGroupOnly ? 'yes' : 'no';
-					} else if ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
+					}
+					if ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
 						return $shareeEnumeration ? 'yes' : 'no';
 					}
 					return $default;
@@ -410,12 +435,12 @@ function($appName, $key, $default)
 				->with($searchTerm, $this->limit, $this->offset)
 				->willReturn($userResponse);
 		} else {
-			if ($singleUser !== false) {
+			if ($users instanceof IUser) {
 				$this->groupManager->expects($this->exactly(2))
 					->method('getUserGroupIds')
 					->withConsecutive(
 						[$this->user],
-						[$singleUser]
+						[$users]
 					)
 					->willReturn($groupResponse);
 			} else {
@@ -431,11 +456,17 @@ function($appName, $key, $default)
 				->willReturnMap($userResponse);
 		}
 
-		if ($singleUser !== false) {
-			$this->userManager->expects($this->once())
-				->method('get')
-				->with($searchTerm)
-				->willReturn($singleUser);
+		if ($users !== false) {
+			if ($users instanceof IUser) {
+				$this->userManager->expects($this->once())
+					->method('get')
+					->with($searchTerm)
+					->willReturn($users);
+			} else {
+				$this->userManager->expects($this->exactly(count($users)))
+					->method('get')
+					->willReturnMap($users);
+			}
 		}