From 0009adae80cf9b3f8bcc40bd3f75d2a3f2cee6ad Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Tue, 6 Jun 2017 10:21:42 -0100 Subject: [PATCH 1/3] SharedWithDisplayName + SharedWithAvatar Signed-off-by: Maxence Lange Signed-off-by: Morris Jobke --- .../lib/Controller/ShareAPIController.php | 9 +- .../Controller/ShareAPIControllerTest.php | 125 ++++++++++++++++++ core/js/sharedialogshareelistview.js | 19 ++- core/js/shareitemmodel.js | 15 +++ core/js/tests/specs/sharedialogviewSpec.js | 36 ++++- lib/private/Share20/Share.php | 40 ++++++ lib/public/Share/IShare.php | 34 +++++ 7 files changed, 269 insertions(+), 9 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 30a8ded2a4..73c3e5951b 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -212,8 +212,13 @@ class ShareAPIController extends OCSController { // "name (type, owner) [id]", depending on the Circles app version. $hasCircleId = (substr($share->getSharedWith(), -1) === ']'); - $displayNameLength = ($hasCircleId? strrpos($share->getSharedWith(), ' '): strlen($share->getSharedWith())); - $result['share_with_displayname'] = substr($share->getSharedWith(), 0, $displayNameLength); + $result['share_with_displayname'] = $share->getSharedWithDisplayName(); + if (empty($result['share_with_displayname'])) { + $displayNameLength = ($hasCircleId? strrpos($share->getSharedWith(), ' '): strlen($share->getSharedWith())); + $result['share_with_displayname'] = substr($share->getSharedWith(), 0, $displayNameLength); + } + + $result['share_with_avatar'] = $share->getSharedWithAvatar(); $shareWithStart = ($hasCircleId? strrpos($share->getSharedWith(), '[') + 1: 0); $shareWithLength = ($hasCircleId? -1: strpos($share->getSharedWith(), ' ')); diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 89a21d7d1e..c27680ba67 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -2074,6 +2074,131 @@ class ShareAPIControllerTest extends TestCase { ], $share, [], false ]; + // Circle with id, display name and avatar set by the Circles app + $share = \OC::$server->getShareManager()->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_CIRCLE) + ->setSharedBy('initiator') + ->setSharedWith('Circle (Public circle, circleOwner) [4815162342]') + ->setSharedWithDisplayName('The display name') + ->setSharedWithAvatar('path/to/the/avatar') + ->setShareOwner('owner') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder) + ->setShareTime(new \DateTime('2000-01-01T00:01:02')) + ->setTarget('myTarget') + ->setId(42); + + $result[] = [ + [ + 'id' => 42, + 'share_type' => \OCP\Share::SHARE_TYPE_CIRCLE, + 'uid_owner' => 'initiator', + 'displayname_owner' => 'initiator', + 'permissions' => 1, + 'stime' => 946684862, + 'parent' => null, + 'expiration' => null, + 'token' => null, + 'uid_file_owner' => 'owner', + 'displayname_file_owner' => 'owner', + 'path' => 'folder', + 'item_type' => 'folder', + 'storage_id' => 'storageId', + 'storage' => 100, + 'item_source' => 2, + 'file_source' => 2, + 'file_parent' => 1, + 'file_target' => 'myTarget', + 'share_with' => '4815162342', + 'share_with_displayname' => 'The display name', + 'share_with_avatar' => 'path/to/the/avatar', + 'mail_send' => 0, + 'mimetype' => 'myFolderMimeType', + ], $share, [], false + ]; + + // Circle with id set by the Circles app + $share = \OC::$server->getShareManager()->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_CIRCLE) + ->setSharedBy('initiator') + ->setSharedWith('Circle (Public circle, circleOwner) [4815162342]') + ->setShareOwner('owner') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder) + ->setShareTime(new \DateTime('2000-01-01T00:01:02')) + ->setTarget('myTarget') + ->setId(42); + + $result[] = [ + [ + 'id' => 42, + 'share_type' => \OCP\Share::SHARE_TYPE_CIRCLE, + 'uid_owner' => 'initiator', + 'displayname_owner' => 'initiator', + 'permissions' => 1, + 'stime' => 946684862, + 'parent' => null, + 'expiration' => null, + 'token' => null, + 'uid_file_owner' => 'owner', + 'displayname_file_owner' => 'owner', + 'path' => 'folder', + 'item_type' => 'folder', + 'storage_id' => 'storageId', + 'storage' => 100, + 'item_source' => 2, + 'file_source' => 2, + 'file_parent' => 1, + 'file_target' => 'myTarget', + 'share_with' => '4815162342', + 'share_with_displayname' => 'Circle (Public circle, circleOwner)', + 'share_with_avatar' => '', + 'mail_send' => 0, + 'mimetype' => 'myFolderMimeType', + ], $share, [], false + ]; + + // Circle with id not set by the Circles app + $share = \OC::$server->getShareManager()->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_CIRCLE) + ->setSharedBy('initiator') + ->setSharedWith('Circle (Public circle, circleOwner)') + ->setShareOwner('owner') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder) + ->setShareTime(new \DateTime('2000-01-01T00:01:02')) + ->setTarget('myTarget') + ->setId(42); + + $result[] = [ + [ + 'id' => 42, + 'share_type' => \OCP\Share::SHARE_TYPE_CIRCLE, + 'uid_owner' => 'initiator', + 'displayname_owner' => 'initiator', + 'permissions' => 1, + 'stime' => 946684862, + 'parent' => null, + 'expiration' => null, + 'token' => null, + 'uid_file_owner' => 'owner', + 'displayname_file_owner' => 'owner', + 'path' => 'folder', + 'item_type' => 'folder', + 'storage_id' => 'storageId', + 'storage' => 100, + 'item_source' => 2, + 'file_source' => 2, + 'file_parent' => 1, + 'file_target' => 'myTarget', + 'share_with' => 'Circle', + 'share_with_displayname' => 'Circle (Public circle, circleOwner)', + 'share_with_avatar' => '', + 'mail_send' => 0, + 'mimetype' => 'myFolderMimeType', + ], $share, [], false + ]; + $share = \OC::$server->getShareManager()->newShare(); $share->setShareType(\OCP\Share::SHARE_TYPE_USER) ->setSharedBy('initiator') diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 1e873a7208..c8709083c5 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -25,7 +25,7 @@ '
    ' + '{{#each sharees}}' + '
  • ' + - '
    ' + + '
    ' + '{{shareWithDisplayName}}' + '' + '{{#if editPermissionPossible}}' + @@ -188,6 +188,7 @@ getShareeObject: function(shareIndex) { var shareWith = this.model.getShareWith(shareIndex); var shareWithDisplayName = this.model.getShareWithDisplayName(shareIndex); + var shareWithAvatar = this.model.getShareWithAvatar(shareIndex); var shareWithTitle = ''; var shareType = this.model.getShareType(shareIndex); var sharedBy = this.model.getSharedBy(shareIndex); @@ -211,6 +212,10 @@ shareWithTitle = shareWith + " (" + t('core', 'email') + ')'; } else if (shareType === OC.Share.SHARE_TYPE_CIRCLE) { shareWithTitle = shareWith; + // Force "shareWith" in the template to a safe value, as the + // original "shareWith" returned by the model may contain + // problematic characters like "'". + shareWith = 'circle-' + shareIndex; } if (sharedBy !== oc_current_user) { @@ -238,10 +243,11 @@ hasDeletePermission: this.model.hasDeletePermission(shareIndex), shareWith: shareWith, shareWithDisplayName: shareWithDisplayName, + shareWithAvatar: shareWithAvatar, shareWithTitle: shareWithTitle, shareType: shareType, shareId: this.model.get('shares')[shareIndex].id, - modSeed: shareType !== OC.Share.SHARE_TYPE_USER && shareType !== OC.Share.SHARE_TYPE_CIRCLE, + modSeed: shareType !== OC.Share.SHARE_TYPE_USER && (shareType !== OC.Share.SHARE_TYPE_CIRCLE || shareWithAvatar), isRemoteShare: shareType === OC.Share.SHARE_TYPE_REMOTE, isMailShare: shareType === OC.Share.SHARE_TYPE_EMAIL, isCircleShare: shareType === OC.Share.SHARE_TYPE_CIRCLE, @@ -351,9 +357,16 @@ this.$('.avatar').each(function () { var $this = $(this); + if ($this.hasClass('imageplaceholderseed')) { $this.css({width: 32, height: 32}); - $this.imageplaceholder($this.data('seed')); + if ($this.data('avatar')) { + $this.css('border-radius', '0%'); + $this.css('background', 'url(' + $this.data('avatar') + ') no-repeat'); + $this.css('background-size', '31px'); + } else { + $this.imageplaceholder($this.data('seed')); + } } else { // user, size, ie8fix, hidedefault, callback, displayname $this.avatar($this.data('username'), 32, undefined, undefined, undefined, $this.data('displayname')); diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index e7824aca33..93feba9c88 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -43,6 +43,7 @@ * @property {string} token * @property {string} share_with * @property {string} share_with_displayname + * @property {string} share_with_avatar * @property {string} mail_send * @property {Date} expiration optional? * @property {number} stime optional? @@ -405,6 +406,20 @@ return share.share_with_displayname; }, + + /** + * @param shareIndex + * @returns {string} + */ + getShareWithAvatar: function(shareIndex) { + /** @type OC.Share.Types.ShareInfo **/ + var share = this.get('shares')[shareIndex]; + if(!_.isObject(share)) { + throw "Unknown Share"; + } + return share.share_with_avatar; + }, + /** * @param shareIndex * @returns {string} diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index d363915984..265bfbca97 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -427,7 +427,21 @@ describe('OC.Share.ShareDialogView', function() { share_type: OC.Share.SHARE_TYPE_REMOTE, share_with: 'foo@bar.com/baz', share_with_displayname: 'foo@bar.com/baz' - + },{ + id: 103, + item_source: 123, + permissions: 31, + share_type: OC.Share.SHARE_TYPE_CIRCLE, + share_with: 'circle-0', + share_with_displayname: 'Circle (Personal circle, user0)', + share_with_avatar: 'path/to/the/avatar' + },{ + id: 104, + item_source: 123, + permissions: 31, + share_type: OC.Share.SHARE_TYPE_CIRCLE, + share_with: 'circle-1', + share_with_displayname: 'Circle (Public circle, user0)', }] }); }); @@ -439,10 +453,10 @@ describe('OC.Share.ShareDialogView', function() { }); it('test correct function calls', function() { - expect(avatarStub.calledTwice).toEqual(true); + expect(avatarStub.calledThrice).toEqual(true); expect(placeholderStub.callCount).toEqual(4); - expect(dialog.$('.shareWithList').children().length).toEqual(3); - expect(dialog.$('.avatar').length).toEqual(4); + expect(dialog.$('.shareWithList').children().length).toEqual(5); + expect(dialog.$('.avatar').length).toEqual(6); }); it('test avatar owner', function() { @@ -469,6 +483,20 @@ describe('OC.Share.ShareDialogView', function() { expect(args.length).toEqual(1); expect(args[0]).toEqual('foo@bar.com/baz ' + OC.Share.SHARE_TYPE_REMOTE); }); + + it('test avatar for circle', function() { + var avatarElement = dialog.$('.avatar').eq(4); + expect(avatarElement.css('background')).toContain('path/to/the/avatar'); + }); + + it('test avatar for circle without avatar', function() { + var args = avatarStub.getCall(2).args; + expect(args.length).toEqual(6); + // Note that "data-username" is set to "circle-{shareIndex}", + // not to the "shareWith" field. + expect(args[0]).toEqual('circle-4'); + expect(args[5]).toEqual('Circle (Public circle, user0)'); + }); }); }); describe('get suggestions', function() { diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index 1836d6708c..d7810165da 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -48,6 +48,10 @@ class Share implements \OCP\Share\IShare { /** @var string */ private $sharedWith; /** @var string */ + private $sharedWithDisplayName; + /** @var string */ + private $sharedWithAvatar; + /** @var string */ private $sharedBy; /** @var string */ private $shareOwner; @@ -251,6 +255,42 @@ class Share implements \OCP\Share\IShare { return $this->sharedWith; } + /** + * @inheritdoc + */ + public function setSharedWithDisplayName($displayName) { + if (!is_string($displayName)) { + throw new \InvalidArgumentException(); + } + $this->sharedWithDisplayName = $displayName; + return $this; + } + + /** + * @inheritdoc + */ + public function getSharedWithDisplayName() { + return $this->sharedWithDisplayName; + } + + /** + * @inheritdoc + */ + public function setSharedWithAvatar($src) { + if (!is_string($src)) { + throw new \InvalidArgumentException(); + } + $this->sharedWithAvatar = $src; + return $this; + } + + /** + * @inheritdoc + */ + public function getSharedWithAvatar() { + return $this->sharedWithAvatar; + } + /** * @inheritdoc */ diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index c19364c86c..870794d653 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -169,6 +169,40 @@ interface IShare { */ public function getSharedWith(); + /** + * Set the display name of the receiver of this share. + * + * @param string $displayName + * @return \OCP\Share\IShare The modified object + * @since 14.0.0 + */ + public function setSharedWithDisplayName($displayName); + + /** + * Get the display name of the receiver of this share. + * + * @return string + * @since 14.0.0 + */ + public function getSharedWithDisplayName(); + + /** + * Set the avatar of the receiver of this share. + * + * @param string $src + * @return \OCP\Share\IShare The modified object + * @since 14.0.0 + */ + public function setSharedWithAvatar($src); + + /** + * Get the avatar of the receiver of this share. + * + * @return string + * @since 14.0.0 + */ + public function getSharedWithAvatar(); + /** * Set the permissions. * See \OCP\Constants::PERMISSION_* From 2e3517eb00a7eb40bc9202fa4548fc97a39ec52f Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Thu, 6 Jul 2017 07:56:32 +0200 Subject: [PATCH 2/3] display details on sharing panel Signed-off-by: Maxence Lange --- core/js/sharedialogresharerinfoview.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/core/js/sharedialogresharerinfoview.js b/core/js/sharedialogresharerinfoview.js index 8afec33e7e..f4bf9afa0b 100644 --- a/core/js/sharedialogresharerinfoview.js +++ b/core/js/sharedialogresharerinfoview.js @@ -84,7 +84,18 @@ undefined, {escape: false} ); - } else { + } else if (this.model.getReshareType() === OC.Share.SHARE_TYPE_CIRCLE) { + sharedByText = t( + 'core', + 'Shared with you and {circle} by {owner}', + { + circle: this.model.getReshareWithDisplayName(), + owner: ownerDisplayName + }, + undefined, + {escape: false} + ); + } else { sharedByText = t( 'core', 'Shared with you by {owner}', From 4f5814c27bc43a8f10bd35f60bc3254697c50659 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Thu, 6 Jul 2017 11:55:22 +0200 Subject: [PATCH 3/3] tests Signed-off-by: Maxence Lange --- tests/lib/Share20/DefaultShareProviderTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 0fa8aa3d0c..230c8db40c 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -629,6 +629,8 @@ class DefaultShareProviderTest extends \Test\TestCase { $share->setSharedBy('sharedBy'); $share->setShareOwner('shareOwner'); $share->setNode($path); + $share->setSharedWithDisplayName('Displayed Name'); + $share->setSharedWithAvatar('/path/to/image.svg'); $share->setPermissions(1); $share->setTarget('/target'); @@ -644,6 +646,12 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertSame('/target', $share2->getTarget()); $this->assertLessThanOrEqual(new \DateTime(), $share2->getShareTime()); $this->assertSame($path, $share2->getNode()); + + // nothing from setSharedWithDisplayName/setSharedWithAvatar is saved in DB + $this->assertSame('Displayed Name', $share->getSharedWithDisplayName()); + $this->assertSame('/path/to/image.svg', $share->getSharedWithAvatar()); + $this->assertSame(null, $share2->getSharedWithDisplayName()); + $this->assertSame(null, $share2->getSharedWithAvatar()); } public function testCreateGroupShare() { @@ -678,6 +686,8 @@ class DefaultShareProviderTest extends \Test\TestCase { $share->setShareOwner('shareOwner'); $share->setNode($path); $share->setPermissions(1); + $share->setSharedWithDisplayName('Displayed Name'); + $share->setSharedWithAvatar('/path/to/image.svg'); $share->setTarget('/target'); $share2 = $this->provider->create($share); @@ -692,6 +702,13 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertSame('/target', $share2->getTarget()); $this->assertLessThanOrEqual(new \DateTime(), $share2->getShareTime()); $this->assertSame($path, $share2->getNode()); + + // nothing from setSharedWithDisplayName/setSharedWithAvatar is saved in DB + $this->assertSame('Displayed Name', $share->getSharedWithDisplayName()); + $this->assertSame('/path/to/image.svg', $share->getSharedWithAvatar()); + $this->assertSame(null, $share2->getSharedWithDisplayName()); + $this->assertSame(null, $share2->getSharedWithAvatar()); + } public function testCreateLinkShare() {