Merge pull request #29334 from owncloud/user-display-template

Add user additional info field for share autocomplete
owncloud · Nov 6, 2017 · 6c0d07c · 6c0d07c
2 parents 89d3432 + 61953ed
commit 6c0d07c
Show file tree

Hide file tree

Showing 13 changed files with 335 additions and 41 deletions.
diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php
@@ -64,6 +64,11 @@ class Share20OCS {
 	/** @var IConfig */
 	private $config;
 
+	/**
+	 * @var string
+	 */
+	protected $additionalInfoField;
+
 	/**
 	 * Share20OCS constructor.
 	 *
@@ -97,6 +102,22 @@ public function __construct(
 		$this->currentUser = $currentUser;
 		$this->l = $l10n;
 		$this->config = $config;
+		$this->additionalInfoField = $this->config->getAppValue('core', 'user_additional_info_field', '');
+	}
+
+	/**
+	 * Returns the additional info to display behind the display name as configured.
+	 *
+	 * @param IUser $user user for which to retrieve the additional info
+	 * @return string|null additional info or null if none to be displayed
+	 */
+	protected function getAdditionalUserInfo(IUser $user) {
+		if ($this->additionalInfoField === 'email') {
+			return $user->getEMailAddress();
+		} else if ($this->additionalInfoField === 'id') {
+			return $user->getUID();
+		}
+		return null;
 	}
 
 	/**
@@ -151,6 +172,9 @@ protected function formatShare(\OCP\Share\IShare $share) {
 			$sharedWith = $this->userManager->get($share->getSharedWith());
 			$result['share_with'] = $share->getSharedWith();
 			$result['share_with_displayname'] = $sharedWith !== null ? $sharedWith->getDisplayName() : $share->getSharedWith();
+			if ($sharedWith !== null) {
+				$result['share_with_additional_info'] = $this->getAdditionalUserInfo($sharedWith);
+			}
 		} else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
 			$group = $this->groupManager->get($share->getSharedWith());
 			$result['share_with'] = $share->getSharedWith();

diff --git a/apps/files_sharing/lib/Controller/ShareesController.php b/apps/files_sharing/lib/Controller/ShareesController.php
@@ -102,6 +102,11 @@ class ShareesController extends OCSController  {
 
 	protected $reachedEndFor = [];
 
+	/**
+	 * @var string
+	 */
+	protected $additionalInfoField;
+
 	/**
 	 * @param IGroupManager $groupManager
 	 * @param IUserManager $userManager
@@ -134,6 +139,7 @@ public function __construct($appName,
 		$this->request = $request;
 		$this->logger = $logger;
 		$this->shareManager = $shareManager;
+		$this->additionalInfoField = $this->config->getAppValue('core', 'user_additional_info_field', '');
 	}
 
 	/**
@@ -169,6 +175,18 @@ protected function getUsers($search) {
 		$lowerSearch = strtolower($search);
 		foreach ($users as $uid => $user) {
 			/* @var $user IUser */
+			$entry = [
+				'label' => $user->getDisplayName(),
+				'value' => [
+					'shareType' => Share::SHARE_TYPE_USER,
+					'shareWith' => $uid,
+				],
+			];
+			$additionalInfo = $this->getAdditionalUserInfo($user);
+			if ($additionalInfo !== null) {
+				$entry['value']['shareWithAdditionalInfo'] = $additionalInfo;
+			}
+
 			if (
 				// Check if the uid is the same
 				strtolower($uid) === $lowerSearch
@@ -181,21 +199,9 @@ protected function getUsers($search) {
 				if (strtolower($uid) === $lowerSearch) {
 					$foundUserById = true;
 				}
-				$this->result['exact']['users'][] = [
-					'label' => $user->getDisplayName(),
-					'value' => [
-						'shareType' => Share::SHARE_TYPE_USER,
-						'shareWith' => $uid,
-					],
-				];
+				$this->result['exact']['users'][] = $entry;
 			} else {
-				$this->result['users'][] = [
-					'label' => $user->getDisplayName(),
-					'value' => [
-						'shareType' => Share::SHARE_TYPE_USER,
-						'shareWith' => $uid,
-					],
-				];
+				$this->result['users'][] = $entry;
 			}
 		}
 
@@ -213,13 +219,18 @@ protected function getUsers($search) {
 				}
 
 				if ($addUser) {
-					array_push($this->result['exact']['users'], [
+					$entry = [
 						'label' => $user->getDisplayName(),
 						'value' => [
 							'shareType' => Share::SHARE_TYPE_USER,
 							'shareWith' => $user->getUID(),
 						],
-					]);
+					];
+					$additionalInfo = $this->getAdditionalUserInfo($user);
+					if ($additionalInfo !== null) {
+						$entry['value']['shareWithAdditionalInfo'] = $additionalInfo;
+					}
+					array_push($this->result['exact']['users'], $entry);
 				}
 			}
 		}
@@ -229,6 +240,21 @@ protected function getUsers($search) {
 		}
 	}
 
+	/**
+	 * Returns the additional info to display behind the display name as configured.
+	 *
+	 * @param IUser $user user for which to retrieve the additional info
+	 * @return string|null additional info or null if none to be displayed
+	 */
+	protected function getAdditionalUserInfo(IUser $user) {
+		if ($this->additionalInfoField === 'email') {
+			return $user->getEMailAddress();
+		} else if ($this->additionalInfoField === 'id') {
+			return $user->getUID();
+		}
+		return null;
+	}
+
 	/**
 	 * @param string $search
 	 */

diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php
@@ -305,6 +305,7 @@ public function dataGetShare() {
 			'share_type' => Share::SHARE_TYPE_USER,
 			'share_with' => 'userId',
 			'share_with_displayname' => 'userDisplay',
+			'share_with_additional_info' => null,
 			'uid_owner' => 'initiatorId',
 			'displayname_owner' => 'initiatorDisplay',
 			'item_type' => 'file',
@@ -2295,8 +2296,8 @@ public function testUpdateLinkShareKeepNameWhenNotSpecified() {
 		$this->assertEquals($expected->getMeta(), $result->getMeta());
 		$this->assertEquals($expected->getData(), $result->getData());
 	}
-	
-	public function dataFormatShare() {
+
+	private function getMockFileFolder() {
 		$file = $this->createMock('\OCP\Files\File');
 		$folder = $this->createMock('\OCP\Files\Folder');
 		$parent = $this->createMock('\OCP\Files\Folder');
@@ -2323,6 +2324,11 @@ public function dataFormatShare() {
 		$file->method('getStorage')->willReturn($storage);
 		$folder->method('getStorage')->willReturn($storage);
 
+		return [$file, $folder];
+	}
+
+	public function dataFormatShare() {
+		list($file, $folder) = $this->getMockFileFolder();
 		$owner = $this->createMock('\OCP\IUser');
 		$owner->method('getDisplayName')->willReturn('ownerDN');
 		$initiator = $this->createMock('\OCP\IUser');
@@ -2396,6 +2402,7 @@ public function dataFormatShare() {
 				'file_target' => 'myTarget',
 				'share_with' => 'recipient',
 				'share_with_displayname' => 'recipientDN',
+				'share_with_additional_info' => null,
 				'mail_send' => 0,
 				'mimetype' => 'myMimeType',
 			], $share, [
@@ -2741,4 +2748,78 @@ public function testUpdateShareApiDisabled() {
 
 		$this->assertEquals($expected, $result);
 	}
+
+	public function additionalInfoDataProvider() {
+		return [
+			['', null],
+			['unsupported', null],
+			['email', 'email@example.com'],
+			['id', 'recipient_id'],
+		];
+	}
+
+	/**
+	 * @dataProvider additionalInfoDataProvider
+	 */
+	public function testGetShareAdditionalInfo($configValue, $expectedInfo) {
+		$config = $this->createMock(IConfig::class);
+		$config->method('getAppValue')
+			->will($this->returnValueMap([
+				['core', 'shareapi_default_permissions', \OCP\Constants::PERMISSION_ALL, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE],
+				['core', 'user_additional_info_field', '', $configValue],
+			]));
+
+		$initiator = $this->createMock(IUser::class);
+		$recipient = $this->createMock(IUser::class);
+		$owner = $this->createMock(IUser::class);
+		$this->userManager->method('get')->will($this->returnValueMap([
+			['initiator', $initiator],
+			['recipient', $recipient],
+			['owner', $owner],
+		]));
+
+		$recipient->method('getUID')->willReturn('recipient_id');
+		$recipient->method('getEMailAddress')->willReturn('email@example.com');
+
+		$ocs = new Share20OCS(
+			$this->shareManager,
+			$this->groupManager,
+			$this->userManager,
+			$this->request,
+			$this->rootFolder,
+			$this->urlGenerator,
+			$this->currentUser,
+			$this->l,
+			$config
+		);
+
+		list($file,) = $this->getMockFileFolder();
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setShareType(Share::SHARE_TYPE_USER)
+			->setSharedWith('recipient')
+			->setSharedBy('initiator')
+			->setShareOwner('owner')
+			->setPermissions(\OCP\Constants::PERMISSION_READ)
+			->setNode($file)
+			->setShareTime(new \DateTime('2000-01-01T00:01:02'))
+			->setTarget('myTarget')
+			->setId(42);
+
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser->getUID())
+			->will($this->returnSelf());
+
+		$this->rootFolder->method('getById')
+			->with($share->getNodeId())
+			->willReturn([$share->getNode()]);
+
+		$this->rootFolder->method('getRelativePath')
+			->with($share->getNode()->getPath())
+			->will($this->returnArgument(0));
+
+		$result = $this->invokePrivate($ocs, 'formatShare', [$share]);
+
+		$this->assertEquals($expectedInfo, $result['share_with_additional_info']);
+	}
 }
diff --git a/apps/files_sharing/tests/API/ShareesTest.php b/apps/files_sharing/tests/API/ShareesTest.php
@@ -140,7 +140,7 @@ protected function getUserMock($uid, $displayName, $email = null, $terms = []) {
 
 		$user->expects($this->any())
 			->method('getEMailAddress')
-			->willReturn(null);
+			->willReturn($email);
 
 		$user->expects($this->any())
 			->method('getSearchTerms')
@@ -443,11 +443,19 @@ public function dataGetUsers() {
 				[],
 				// fuzzy match expected
 				[
-					['label' => 'Another One', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'another1']],
+					[
+						'label' => 'Another One',
+						'value' => [
+							'shareType' => Share::SHARE_TYPE_USER,
+							'shareWith' => 'another1',
+							'shareWithAdditionalInfo' => 'another1'
+						]
+					],
 				],
 				true,
 				false,
 				true,
+				'id'
 			],
 			[
 				// pick user directly by name
@@ -464,13 +472,21 @@ public function dataGetUsers() {
 				],
 				// exact expected
 				[
-					['label' => 'Another One', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'another1']],
+					[
+						'label' => 'Another One',
+						'value' => [
+							'shareType' => Share::SHARE_TYPE_USER,
+							'shareWith' => 'another1',
+							'shareWithAdditionalInfo' => 'email@example.com'
+						]
+					],
 				],
 				// fuzzy match expected
 				[],
 				true,
-				$this->getUserMock('another1', 'Another One'),
+				$this->getUserMock('another1', 'Another One', 'email@example.com'),
 				true,
+				'email'
 			],
 		];
 	}
@@ -488,6 +504,7 @@ public function dataGetUsers() {
 	 * @param bool $reachedEnd
 	 * @param mixed $singleUser false for testing search or user mock when we are testing a direct match
 	 * @param mixed $shareeEnumerationGroupMembers restrict enumeration to group members
+	 * @param mixed $additionalUserInfoField
 	 */
 	public function testGetUsers(
 		$searchTerm,
@@ -499,8 +516,26 @@ public function testGetUsers(
 		$expected,
 		$reachedEnd,
 		$singleUser,
-		$shareeEnumerationGroupMembers = false
+		$shareeEnumerationGroupMembers = false,
+		$additionalUserInfoField = null
 	) {
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('core', 'user_additional_info_field', '')
+			->willReturn($additionalUserInfoField);
+
+		$this->sharees = new ShareesController(
+			'files_sharing',
+			$this->request,
+			$this->groupManager,
+			$this->userManager,
+			$this->contactsManager,
+			$this->config,
+			$this->session,
+			$this->getMockBuilder(IURLGenerator::class)->disableOriginalConstructor()->getMock(),
+			$this->getMockBuilder(ILogger::class)->disableOriginalConstructor()->getMock(),
+			$this->shareManager
+		);
 		$this->invokePrivate($this->sharees, 'limit', [2]);
 		$this->invokePrivate($this->sharees, 'offset', [0]);
 		$this->invokePrivate($this->sharees, 'shareWithGroupOnly', [$shareWithGroupOnly]);
@@ -1437,9 +1472,6 @@ public function dataSearchInvalid() {
 	 * @param string $message
 	 */
 	public function testSearchInvalid($message, $search = '', $itemType = null, $page = 1, $perPage = 200) {
-		$this->config->expects($this->never())
-			->method('getAppValue');
-
 		/** @var ShareesController | \PHPUnit_Framework_MockObject_MockObject $sharees */
 		$sharees = $this->getMockBuilder(ShareesController::class)
 			->setConstructorArgs([