From bfc44a855e8c164ca6e5b0912c0606989ab03f4b Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 24 Oct 2017 10:17:40 +0200 Subject: [PATCH 1/2] Add user additional info field for share autocomplete --- apps/files_sharing/lib/API/Share20OCS.php | 24 ++++++ .../lib/Controller/ShareesController.php | 58 +++++++++---- .../tests/API/Share20OCSTest.php | 85 ++++++++++++++++++- apps/files_sharing/tests/API/ShareesTest.php | 48 +++++++++-- core/css/share.css | 9 ++ core/js/sharedialogshareelistview.js | 5 ++ core/js/sharedialogview.js | 53 +++++++++--- core/js/shareitemmodel.js | 13 +++ .../tests/specs/sharedialogshareelistview.js | 35 ++++++++ core/js/tests/specs/sharedialogviewSpec.js | 18 ++++ settings/Panels/Admin/FileSharing.php | 1 + settings/js/admin.js | 7 +- .../templates/panels/admin/filesharing.php | 8 ++ 13 files changed, 323 insertions(+), 41 deletions(-) diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php index 0c406325a119..22df2d1a4f8f 100644 --- 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 index 6e3bde539b0f..4d716bd33762 100644 --- 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 index 4827946243bc..a53a968487c3 100644 --- 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 index 020506d5319a..6af96e8520ca 100644 --- 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([ diff --git a/core/css/share.css b/core/css/share.css index d1d61605f955..dcf37dde437c 100644 --- a/core/css/share.css +++ b/core/css/share.css @@ -63,6 +63,15 @@ vertical-align: middle; } +.share-autocomplete-item .autocomplete-item-displayname { + margin-right: 5px; +} + +.share-autocomplete-item .autocomplete-item-additional-info, +#shareWithList .user-additional-info { + color: #888; +} + #shareWithList { list-style-type:none; padding:8px; diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index d919d228e8c5..46a308b3697f 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -22,6 +22,9 @@ '
' + '{{/if}}' + '{{shareWithDisplayName}}' + + '{{#if shareWithAdditionalInfo}}' + + '({{shareWithAdditionalInfo}})' + + '{{/if}}' + '{{#if mailNotificationEnabled}} {{#unless isRemoteShare}}' + '' + '' + @@ -115,6 +118,7 @@ var shareWith = this.model.getShareWith(shareIndex); var shareWithDisplayName = this.model.getShareWithDisplayName(shareIndex); var shareType = this.model.getShareType(shareIndex); + var shareWithAdditionalInfo = this.model.getShareWithAdditionalInfo(shareIndex); var hasPermissionOverride = {}; if (shareType === OC.Share.SHARE_TYPE_GROUP) { @@ -133,6 +137,7 @@ wasMailSent: this.model.notificationMailWasSent(shareIndex), shareWith: shareWith, shareWithDisplayName: shareWithDisplayName, + shareWithAdditionalInfo: shareWithAdditionalInfo, shareType: shareType, shareId: this.model.get('shares')[shareIndex].id, modSeed: shareType !== OC.Share.SHARE_TYPE_USER, diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 5e3eb7467e11..e89c5240e939 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -44,6 +44,23 @@ ''; + var TEMPLATE_AUTOCOMPLETE_ITEM = + '
  • ' + + '' + + '' + + '' + + '
    • '; + /** * @class OCA.Share.ShareDialogView * @member {OC.Share.ShareItemModel} model @@ -299,27 +316,25 @@ }); } } - var insert = $(" From 61953ed5b9afd8042b1bf7205334b344e89669d2 Mon Sep 17 00:00:00 2001 From: Felix Heidecke Date: Fri, 3 Nov 2017 14:45:17 +0100 Subject: [PATCH 2/2] Trunkate overflowing sharee name/info on smaller screen sizes --- core/css/share.css | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/css/share.css b/core/css/share.css index dcf37dde437c..5dd8e10c68ac 100644 --- a/core/css/share.css +++ b/core/css/share.css @@ -52,7 +52,15 @@ .share-autocomplete-item { display: flex; + max-width: 220px; } + +@media (min-width: 1152px) { + .share-autocomplete-item { + max-width: 20vw; + } +} + .share-autocomplete-item .autocomplete-item-text { margin-left: 10px; margin-right: 10px; @@ -67,6 +75,10 @@ margin-right: 5px; } +.share-autocomplete-item .avatardiv { + flex-shrink: 0; +} + .share-autocomplete-item .autocomplete-item-additional-info, #shareWithList .user-additional-info { color: #888;