From dfcf94e1dfdb0aba1adb4e221deb16c31f89790e Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Thu, 27 Feb 2020 18:40:42 +0300 Subject: [PATCH] Show pending remote shares at Shared with you tab --- apps/files_sharing/appinfo/routes.php | 5 ++ apps/files_sharing/js/app.js | 25 +++++-- apps/files_sharing/js/sharedfilelist.js | 4 +- .../lib/Controller/RemoteOcsController.php | 66 ++++++++++++++++--- .../Controller/RemoteOcsControllerTest.php | 32 ++++++++- .../tests/js/sharedfilelistSpec.js | 6 +- changelog/unreleased/37022 | 5 ++ 7 files changed, 124 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/37022 diff --git a/apps/files_sharing/appinfo/routes.php b/apps/files_sharing/appinfo/routes.php index eaaf6fb58962..3977b19eb786 100644 --- a/apps/files_sharing/appinfo/routes.php +++ b/apps/files_sharing/appinfo/routes.php @@ -106,6 +106,11 @@ 'url' => '/api/v1/remote_shares', 'verb' => 'GET' ], + [ + 'name' => 'RemoteOcs#getAllShares', + 'url' => '/api/v1/remote_shares/all', + 'verb' => 'GET' + ], [ 'name' => 'RemoteOcs#getOpenShares', 'url' => '/api/v1/remote_shares/pending', diff --git a/apps/files_sharing/js/app.js b/apps/files_sharing/js/app.js index 81ae2e119473..b487d3a10816 100644 --- a/apps/files_sharing/js/app.js +++ b/apps/files_sharing/js/app.js @@ -201,14 +201,15 @@ OCA.Sharing.App = { fileList.fileSummary.$el.find('.filesize').remove(); }, - _setShareState: function(fileId, state) { + _setShareState: function(fileId, state, isRemote) { var method = 'POST'; if (state === OC.Share.STATE_REJECTED) { method = 'DELETE'; } + var endPoint = isRemote === true ? 'remote_shares/pending/' : 'shares/pending/'; var xhr = $.ajax({ - url: OC.linkToOCS('apps/files_sharing/api/v1') + 'shares/pending/' + encodeURIComponent(fileId) + '?format=json', + url: OC.linkToOCS('apps/files_sharing/api/v1') + endPoint + encodeURIComponent(fileId) + '?format=json', contentType: 'application/json', dataType: 'json', type: method, @@ -225,12 +226,18 @@ OCA.Sharing.App = { }, _shareStateActionHandler: function(context, newState) { + var targetFileData = context.fileList.elementToFile(context.$file); + var isRemote = targetFileData.shareLocationType === 'remote'; function responseCallback(response, status) { if (status === 'success') { // note: there could be multiple shares/responses but // we assume that the relevant content is the same // for all (state, file_target) var data = response.ocs.data[0]; + // If we declined a remote share we need to hide it from the list + if (isRemote === true && newState === OC.Share.STATE_REJECTED) { + return context.fileList.remove(context.$file.attr('data-file'), {updateSummary:true}); + } var meta = response.ocs.meta; if (meta.status === 'ok') { context.fileInfoModel.set({ @@ -246,7 +253,7 @@ OCA.Sharing.App = { } context.fileList.showFileBusyState(context.$file, true); - this._setShareState(context.fileInfoModel.get('shares')[0].id, newState) + this._setShareState(context.fileInfoModel.get('shares')[0].id, newState, isRemote) .then(responseCallback); }, @@ -287,8 +294,16 @@ OCA.Sharing.App = { var targetFileData = context.fileList.elementToFile(context.$file); if (targetFileData.shareLocationType === 'remote') { // accept and reject will be removed for remote shares - delete(actions.Accept); - delete(actions.Reject); + if (shareState === OC.Share.STATE_PENDING) { + delete(actions.Download); + delete(actions.Details); + delete(actions.Delete); + delete(actions.Unshare); + } else { + delete(actions.Accept); + delete(actions.Reject); + } + return actions; } diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js index e016af895e8c..5ac24ffb9ef1 100644 --- a/apps/files_sharing/js/sharedfilelist.js +++ b/apps/files_sharing/js/sharedfilelist.js @@ -244,7 +244,7 @@ if (!!this._sharedWithUser) { var remoteShares = $.ajax({ - url: OC.linkToOCS('apps/files_sharing/api/v1') + 'remote_shares', + url: OC.linkToOCS('apps/files_sharing/api/v1') + 'remote_shares/all', /* jshint camelcase: false */ data: { format: 'json', @@ -339,7 +339,7 @@ .map(function(share) { var file = { shareOwner: share.owner + '@' + share.remote.replace(/.*?:\/\//g, ""), - shareState: share.accepted ? OC.Share.STATE_ACCEPTED : OC.Share.STATE_PENDING, + shareState: !!parseInt(share.accepted) ? OC.Share.STATE_ACCEPTED : OC.Share.STATE_PENDING, name: OC.basename(share.mountpoint), mtime: share.mtime * 1000, mimetype: share.mimetype, diff --git a/apps/files_sharing/lib/Controller/RemoteOcsController.php b/apps/files_sharing/lib/Controller/RemoteOcsController.php index a777f5a2b8df..93fccbee5b88 100644 --- a/apps/files_sharing/lib/Controller/RemoteOcsController.php +++ b/apps/files_sharing/lib/Controller/RemoteOcsController.php @@ -21,10 +21,12 @@ namespace OCA\Files_Sharing\Controller; +use OC\Files\View; use OC\OCS\Result; use OCA\Files_Sharing\External\Manager; use OCP\AppFramework\OCSController; use OCP\IRequest; +use OCP\Share; class RemoteOcsController extends OCSController { /** @var IRequest */ @@ -77,7 +79,13 @@ public function getOpenShares() { */ public function acceptShare($id) { if ($this->externalManager->acceptShare((int) $id)) { - return new Result(); + $share = $this->externalManager->getShare($id); + return new Result( + [[ + 'state' => Share::STATE_ACCEPTED, + 'file_target' => $share['mountpoint'] + ]] + ); } // Make sure the user has no notification for something that does not exist anymore. @@ -108,15 +116,49 @@ public function declineShare($id) { * @NoCSRFRequired * @NoAdminRequired * + * @param bool $includingPending * @return Result */ - public function getShares() { - $shares = $this->externalManager->getAcceptedShares(); - $shares = \array_map([$this, 'extendShareInfo'], $shares); + public function getShares($includingPending = false) { + $shares = \array_map( + [$this, 'extendShareInfo'], + $this->externalManager->getAcceptedShares() + ); + + if ($includingPending === true) { + /** + * pending shares have mountpoint looking like + * {{TemporaryMountPointName#/filename.ext}} + * so we need to cut it off + */ + $openShares = \array_map( + function ($share) { + $share['mountpoint'] = \substr( + $share['mountpoint'], + \strlen('{{TemporaryMountPointName#') + ); + + $share['mountpoint'] = \rtrim($share['mountpoint'], '}'); + return $share; + }, + $this->externalManager->getOpenShares() + ); + $shares = \array_merge($shares, $openShares); + } return new Result($shares); } + /** + * @NoCSRFRequired + * @NoAdminRequired + * + * @return Result + */ + public function getAllShares() { + return $this->getShares(true); + } + /** * @NoCSRFRequired * @NoAdminRequired @@ -161,17 +203,25 @@ public function unshare($id) { /** * @param array $share Share with info from the share_external table * @return array enriched share info with data from the filecache + * @throws \Exception */ private function extendShareInfo($share) { - $view = new \OC\Files\View('/' . $this->uid . '/files/'); - $info = $view->getFileInfo($share['mountpoint']); - + $info = $this->getFileInfo($share['mountpoint']); $share['mimetype'] = $info->getMimetype(); $share['mtime'] = $info->getMtime(); $share['permissions'] = $info->getPermissions(); $share['type'] = $info->getType(); $share['file_id'] = $info->getId(); - return $share; } + + /** + * @param string $mountpoint + * @return false|\OC\Files\FileInfo + * @throws \Exception + */ + protected function getFileInfo($mountpoint) { + $view = new View('/' . $this->uid . '/files/'); + return $view->getFileInfo($mountpoint); + } } diff --git a/apps/files_sharing/tests/Controller/RemoteOcsControllerTest.php b/apps/files_sharing/tests/Controller/RemoteOcsControllerTest.php index 1e87cdfb2d91..4f31eca93f59 100644 --- a/apps/files_sharing/tests/Controller/RemoteOcsControllerTest.php +++ b/apps/files_sharing/tests/Controller/RemoteOcsControllerTest.php @@ -21,6 +21,8 @@ namespace OCA\Files_Sharing\Tests\Controller; +use OC\Files\FileInfo; +use OC\Files\View; use OCA\Files_Sharing\Controller\RemoteOcsController; use OCA\Files_Sharing\External\Manager; use OCP\IRequest; @@ -37,7 +39,7 @@ class RemoteOcsControllerTest extends TestCase { /** @var Manager */ protected $externalManager; - /** @var RemoteOcsController */ + /** @var RemoteOcsController | MockObject */ protected $controller; protected function setUp(): void { @@ -125,9 +127,21 @@ public function testGetShares() { $this->assertEquals(100, $result->getStatusCode()); } + public function testGetAllShares() { + $this->externalManager->expects($this->once()) + ->method('getAcceptedShares') + ->willReturn([]); + $this->externalManager->expects($this->once()) + ->method('getOpenShares') + ->willReturn([['mountpoint' => '{{TemporaryMountPointName#/filename.ext}}']]); + $result = $this->controller->getAllShares(); + $this->assertEquals(100, $result->getStatusCode()); + } + public function getShareDataProvider() { return [ [false, 404], + [['mountpoint' => '/share'], 100], ]; } @@ -144,6 +158,22 @@ public function testGetShare($getShareResult, $expectedStatusCode) { ->with($shareId) ->willReturn($getShareResult); + if ($getShareResult !== false) { + $this->controller = $this->getMockBuilder(RemoteOcsController::class) + ->setConstructorArgs([ + $this->appName, + $this->request, + $this->externalManager, + 'user' + ]) + ->setMethods(['getFileInfo']) + ->getMock(); + $fileInfo = $this->createMock(FileInfo::class); + $this->controller->expects($this->once()) + ->method('getFileInfo') + ->willReturn($fileInfo); + } + $result = $this->controller->getShare($shareId); $this->assertEquals($expectedStatusCode, $result->getStatusCode()); } diff --git a/apps/files_sharing/tests/js/sharedfilelistSpec.js b/apps/files_sharing/tests/js/sharedfilelistSpec.js index 2f89274dbb47..c2546b7c8551 100644 --- a/apps/files_sharing/tests/js/sharedfilelistSpec.js +++ b/apps/files_sharing/tests/js/sharedfilelistSpec.js @@ -145,7 +145,7 @@ describe('OCA.Sharing.FileList tests', function() { expect(fakeServer.requests[1].url).toEqual( OC.linkToOCS('apps/files_sharing/api/v1') + - 'remote_shares?format=json&include_tags=true' + 'remote_shares/all?format=json&include_tags=true' ); fakeServer.requests[0].respond( @@ -226,7 +226,7 @@ describe('OCA.Sharing.FileList tests', function() { ); expect(fakeServer.requests[1].url).toEqual( OC.linkToOCS('apps/files_sharing/api/v1') + - 'remote_shares?format=json&include_tags=true' + 'remote_shares/all?format=json&include_tags=true' ); fakeServer.requests[0].respond( @@ -312,7 +312,7 @@ describe('OCA.Sharing.FileList tests', function() { ); expect(fakeServer.requests[1].url).toEqual( OC.linkToOCS('apps/files_sharing/api/v1') + - 'remote_shares?format=json&include_tags=true' + 'remote_shares/all?format=json&include_tags=true' ); fakeServer.requests[0].respond( diff --git a/changelog/unreleased/37022 b/changelog/unreleased/37022 new file mode 100644 index 000000000000..b21e470c797c --- /dev/null +++ b/changelog/unreleased/37022 @@ -0,0 +1,5 @@ +Bugfix: Show pending remote shares at the Shared with you tab + +Fixed missing pending remote shares in the file list at the Shared with you tab. + +https://github.com/owncloud/core/pull/37022