diff options
28 files changed, 432 insertions, 68 deletions
diff --git a/apps/comments/lib/Activity/Listener.php b/apps/comments/lib/Activity/Listener.php index e9a41f8f6b2..b5378e996dc 100644 --- a/apps/comments/lib/Activity/Listener.php +++ b/apps/comments/lib/Activity/Listener.php @@ -98,7 +98,7 @@ class Listener { /** @var Node $node */ $node = array_shift($nodes); $al = $this->shareHelper->getPathsForAccessList($node); - $users = array_merge($users, $al['users']); + $users += $al['users']; } } @@ -119,7 +119,9 @@ class Listener { ]); foreach ($users as $user => $path) { - $activity->setAffectedUser($user); + // numerical user ids end up as integers from array keys, but string + // is required + $activity->setAffectedUser((string)$user); $activity->setSubject('add_comment_subject', [ 'actor' => $actor, diff --git a/apps/comments/tests/Unit/Activity/ListenerTest.php b/apps/comments/tests/Unit/Activity/ListenerTest.php new file mode 100644 index 00000000000..4454101bb38 --- /dev/null +++ b/apps/comments/tests/Unit/Activity/ListenerTest.php @@ -0,0 +1,187 @@ +<?php +/** + * @copyright Copyright (c) 2018 Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @author Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +namespace OCA\Comments\Tests\Unit\Activity; + +use OCA\Comments\Activity\Listener; +use OCP\Activity\IEvent; +use OCP\Activity\IManager; +use OCP\App\IAppManager; +use OCP\Comments\CommentsEvent; +use OCP\Comments\IComment; +use OCP\Files\Config\ICachedMountFileInfo; +use OCP\Files\Config\IMountProviderCollection; +use OCP\Files\Config\IUserMountCache; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; +use OCP\Files\Node; +use OCP\IUser; +use OCP\IUserSession; +use OCP\Share\IShareHelper; +use Test\TestCase; + +class ListenerTest extends TestCase { + + /** @var Listener */ + protected $listener; + + /** @var IManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $activityManager; + + /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ + protected $session; + + /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $appManager; + + /** @var IMountProviderCollection|\PHPUnit_Framework_MockObject_MockObject */ + protected $mountProviderCollection; + + /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */ + protected $rootFolder; + + /** @var IShareHelper|\PHPUnit_Framework_MockObject_MockObject */ + protected $shareHelper; + + protected function setUp() { + parent::setUp(); + + $this->activityManager = $this->createMock(IManager::class); + $this->session = $this->createMock(IUserSession::class); + $this->appManager = $this->createMock(IAppManager::class); + $this->mountProviderCollection = $this->createMock(IMountProviderCollection::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->shareHelper = $this->createMock(IShareHelper::class); + + $this->listener = new Listener( + $this->activityManager, + $this->session, + $this->appManager, + $this->mountProviderCollection, + $this->rootFolder, + $this->shareHelper + ); + } + + public function testCommentEvent() { + $this->appManager->expects($this->any()) + ->method('isInstalled') + ->with('activity') + ->willReturn(true); + + $comment = $this->createMock(IComment::class); + $comment->expects($this->any()) + ->method('getObjectType') + ->willReturn('files'); + + /** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */ + $event = $this->createMock(CommentsEvent::class); + $event->expects($this->any()) + ->method('getComment') + ->willReturn($comment); + $event->expects($this->any()) + ->method('getEvent') + ->willReturn(CommentsEvent::EVENT_ADD); + + /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $ownerUser */ + $ownerUser = $this->createMock(IUser::class); + $ownerUser->expects($this->any()) + ->method('getUID') + ->willReturn('937393'); + + /** @var \PHPUnit_Framework_MockObject_MockObject $mount */ + $mount = $this->createMock(ICachedMountFileInfo::class); + $mount->expects($this->any()) + ->method('getUser') + ->willReturn($ownerUser); // perhaps not the right user, but does not matter in this scenario + + $mounts = [ $mount, $mount ]; // to make sure duplicates are dealt with + + $userMountCache = $this->createMock(IUserMountCache::class); + $userMountCache->expects($this->any()) + ->method('getMountsForFileId') + ->willReturn($mounts); + + $this->mountProviderCollection->expects($this->any()) + ->method('getMountCache') + ->willReturn($userMountCache); + + $node = $this->createMock(Node::class); + $nodes = [ $node ]; + + $ownerFolder = $this->createMock(Folder::class); + $ownerFolder->expects($this->any()) + ->method('getById') + ->willReturn($nodes); + + $this->rootFolder->expects($this->any()) + ->method('getUserFolder') + ->willReturn($ownerFolder); + + $al = [ 'users' => [ + '873304' => 'i/got/it/here', + '254342' => 'there/i/have/it', + 'sandra' => 'and/here/i/placed/it' + ]]; + $this->shareHelper->expects($this->any()) + ->method('getPathsForAccessList') + ->willReturn($al); + + $this->session->expects($this->any()) + ->method('getUser') + ->willReturn($ownerUser); + + /** @var \PHPUnit_Framework_MockObject_MockObject $activity */ + $activity = $this->createMock(IEvent::class); + $activity->expects($this->exactly(count($al['users']))) + ->method('setAffectedUser'); + $activity->expects($this->once()) + ->method('setApp') + ->with('comments') + ->willReturnSelf(); + $activity->expects($this->once()) + ->method('setType') + ->with('comments') + ->willReturnSelf(); + $activity->expects($this->once()) + ->method('setAuthor') + ->with($ownerUser->getUID()) + ->willReturnSelf(); + $activity->expects($this->once()) + ->method('setObject') + ->with('files', $this->anything()) + ->willReturnSelf(); + $activity->expects($this->once()) + ->method('setMessage') + ->with('add_comment_message', $this->anything()) + ->willReturnSelf(); + + $this->activityManager->expects($this->once()) + ->method('generateEvent') + ->willReturn($activity); + $this->activityManager->expects($this->exactly(count($al['users']))) + ->method('publish'); + + $this->listener->commentEvent($event); + } +} diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 2fb7dfba29f..a6d376aa2a9 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -625,12 +625,23 @@ this.registerAction({ name: 'MoveCopy', - displayName: t('files', 'Move or copy'), + displayName: function(context) { + var permissions = context.fileInfoModel.attributes.permissions; + if (permissions & OC.PERMISSION_UPDATE) { + return t('files', 'Move or copy'); + } + return t('files', 'Copy'); + }, mime: 'all', order: -25, - permissions: OC.PERMISSION_UPDATE, + permissions: $('#isPublic').val() ? OC.PERMISSION_UPDATE : OC.PERMISSION_READ, iconClass: 'icon-external', actionHandler: function (filename, context) { + var permissions = context.fileInfoModel.attributes.permissions; + var actions = OC.dialogs.FILEPICKER_TYPE_COPY; + if (permissions & OC.PERMISSION_UPDATE) { + actions = OC.dialogs.FILEPICKER_TYPE_COPY_MOVE; + } OC.dialogs.filepicker(t('files', 'Target folder'), function(targetPath, type) { if (type === OC.dialogs.FILEPICKER_TYPE_COPY) { context.fileList.copy(filename, targetPath); @@ -638,7 +649,7 @@ if (type === OC.dialogs.FILEPICKER_TYPE_MOVE) { context.fileList.move(filename, targetPath); } - }, false, "httpd/unix-directory", true, OC.dialogs.FILEPICKER_TYPE_COPY_MOVE); + }, false, "httpd/unix-directory", true, actions); } }); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index b46db792678..4dc8a58e175 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -798,6 +798,7 @@ OCA.Files.FileActions.updateFileActionSpinner(moveFileAction, false); }; + var actions = this.isSelectedMovable() ? OC.dialogs.FILEPICKER_TYPE_COPY_MOVE : OC.dialogs.FILEPICKER_TYPE_COPY; OC.dialogs.filepicker(t('files', 'Target folder'), function(targetPath, type) { if (type === OC.dialogs.FILEPICKER_TYPE_COPY) { self.copy(files, targetPath, disableLoadingState); @@ -805,7 +806,7 @@ if (type === OC.dialogs.FILEPICKER_TYPE_MOVE) { self.move(files, targetPath, disableLoadingState); } - }, false, "httpd/unix-directory", true, OC.dialogs.FILEPICKER_TYPE_COPY_MOVE); + }, false, "httpd/unix-directory", true, actions); return false; }, @@ -2871,18 +2872,39 @@ this.$el.find('#headerName a.name>span:first').text(selection); this.$el.find('#modified a>span:first').text(''); this.$el.find('table').addClass('multiselect'); - this.$el.find('.selectedActions .copy-move').toggleClass('hidden', !this.isSelectedCopiableOrMovable()); this.$el.find('.selectedActions .download').toggleClass('hidden', !this.isSelectedDownloadable()); this.$el.find('.delete-selected').toggleClass('hidden', !this.isSelectedDeletable()); + + var $copyMove = this.$el.find('.selectedActions .copy-move'); + if (this.isSelectedCopiable()) { + $copyMove.toggleClass('hidden', false); + if (this.isSelectedMovable()) { + $copyMove.find('.label').text(t('files', 'Move or copy')); + } else { + $copyMove.find('.label').text(t('files', 'Copy')); + } + } else { + $copyMove.toggleClass('hidden', true); + } } }, /** - * Check whether all selected files are copiable or movable + * Check whether all selected files are copiable + */ + isSelectedCopiable: function() { + return _.reduce(this.getSelectedFiles(), function(copiable, file) { + var requiredPermission = $('#isPublic').val() ? OC.PERMISSION_UPDATE : OC.PERMISSION_READ; + return copiable && (file.permissions & requiredPermission); + }, true); + }, + + /** + * Check whether all selected files are movable */ - isSelectedCopiableOrMovable: function() { - return _.reduce(this.getSelectedFiles(), function(copiableOrMovable, file) { - return copiableOrMovable && (file.permissions & OC.PERMISSION_UPDATE); + isSelectedMovable: function() { + return _.reduce(this.getSelectedFiles(), function(movable, file) { + return movable && (file.permissions & OC.PERMISSION_UPDATE); }, true); }, diff --git a/apps/files/templates/list.php b/apps/files/templates/list.php index fd5423c334f..e6b1e54d389 100644 --- a/apps/files/templates/list.php +++ b/apps/files/templates/list.php @@ -53,7 +53,7 @@ <span id="selectedActionsList" class="selectedActions"> <a href="" class="copy-move"> <span class="icon icon-external"></span> - <span><?php p($l->t('Move or copy'))?></span> + <span class="label"><?php p($l->t('Move or copy'))?></span> </a> <a href="" class="download"> <span class="icon icon-download"></span> diff --git a/apps/files/tests/js/fileactionsmenuSpec.js b/apps/files/tests/js/fileactionsmenuSpec.js index 926516b3043..c678d166153 100644 --- a/apps/files/tests/js/fileactionsmenuSpec.js +++ b/apps/files/tests/js/fileactionsmenuSpec.js @@ -271,6 +271,7 @@ describe('OCA.Files.FileActionsMenu tests', function() { $file: $tr, fileList: fileList, fileActions: fileActions, + fileInfoModel: new OCA.Files.FileInfoModel(fileData), dir: fileList.getCurrentDirectory() }; menu = new OCA.Files.FileActionsMenu(); @@ -304,6 +305,7 @@ describe('OCA.Files.FileActionsMenu tests', function() { $file: $tr, fileList: fileList, fileActions: fileActions, + fileInfoModel: new OCA.Files.FileInfoModel(fileData), dir: '/anotherpath/there' }; menu = new OCA.Files.FileActionsMenu(); @@ -336,6 +338,7 @@ describe('OCA.Files.FileActionsMenu tests', function() { $file: $tr, fileList: fileList, fileActions: fileActions, + fileInfoModel: new OCA.Files.FileInfoModel(fileData), dir: '/somepath/dir' }; menu = new OCA.Files.FileActionsMenu(); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 08da15b8a88..1b26a468172 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -94,7 +94,7 @@ describe('OCA.Files.FileList tests', function() { '<input type="checkbox" id="select_all_files" class="select-all checkbox">' + '<a class="name columntitle" data-sort="name"><span>Name</span><span class="sort-indicator"></span></a>' + '<span id="selectedActionsList" class="selectedActions hidden">' + - '<a href class="copy-move">Move or copy</a>' + + '<a href class="copy-move"><span class="label">Move or copy</span></a>' + '<a href class="download"><img src="actions/download.svg">Download</a>' + '<a href class="delete-selected">Delete</a></span>' + '</th>' + @@ -2101,10 +2101,17 @@ describe('OCA.Files.FileList tests', function() { $('#permissions').val(OC.PERMISSION_READ | OC.PERMISSION_UPDATE); $('.select-all').click(); expect(fileList.$el.find('.selectedActions .copy-move').hasClass('hidden')).toEqual(false); + expect(fileList.$el.find('.selectedActions .copy-move .label').text()).toEqual('Move or copy'); testFiles[0].permissions = OC.PERMISSION_READ; $('.select-all').click(); fileList.setFiles(testFiles); $('.select-all').click(); + expect(fileList.$el.find('.selectedActions .copy-move').hasClass('hidden')).toEqual(false); + expect(fileList.$el.find('.selectedActions .copy-move .label').text()).toEqual('Copy'); + testFiles[0].permissions = OC.PERMISSION_NONE; + $('.select-all').click(); + fileList.setFiles(testFiles); + $('.select-all').click(); expect(fileList.$el.find('.selectedActions .copy-move').hasClass('hidden')).toEqual(true); }); it('show doesnt show the download action if one or more files are not downloadable', function () { diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index cb9b7ad6822..12ad285c52b 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -960,6 +960,7 @@ MountConfigListView.prototype = _.extend({ if (result.length === 0 && mainForm.attr('data-can-create') === 'false') { mainForm.hide(); $('a[href="#external-storage"]').parent().hide(); + $('#emptycontent').show(); } onCompletion.resolve(); } diff --git a/apps/files_external/lib/Config/ConfigAdapter.php b/apps/files_external/lib/Config/ConfigAdapter.php index efeb3d75586..34e96df0441 100644 --- a/apps/files_external/lib/Config/ConfigAdapter.php +++ b/apps/files_external/lib/Config/ConfigAdapter.php @@ -168,7 +168,7 @@ class ConfigAdapter implements IMountProvider { $storageConfig->getMountOptions() ); } else { - return new MountPoint( + return new ExternalMountPoint( $storage, '/' . $user->getUID() . '/files' . $storageConfig->getMountPoint(), null, diff --git a/apps/files_external/lib/Lib/PersonalMount.php b/apps/files_external/lib/Lib/PersonalMount.php index c54ed0a79f3..8c8ac0893f6 100644 --- a/apps/files_external/lib/Lib/PersonalMount.php +++ b/apps/files_external/lib/Lib/PersonalMount.php @@ -24,14 +24,14 @@ namespace OCA\Files_External\Lib; -use OC\Files\Mount\MountPoint; use OC\Files\Mount\MoveableMount; +use OCA\Files_External\Config\ExternalMountPoint; use OCA\Files_External\Service\UserStoragesService; /** * Person mount points can be moved by the user */ -class PersonalMount extends MountPoint implements MoveableMount { +class PersonalMount extends ExternalMountPoint implements MoveableMount { /** @var UserStoragesService */ protected $storagesService; diff --git a/apps/files_external/templates/list.php b/apps/files_external/templates/list.php index d006514bcde..ed13ed83701 100644 --- a/apps/files_external/templates/list.php +++ b/apps/files_external/templates/list.php @@ -6,8 +6,7 @@ <div id="emptycontent" class="hidden"> <div class="icon-external"></div> - <h2><?php p($l->t('No external storage configured')); ?></h2> - <p><a href="<?php p(link_to('', 'index.php/settings/personal#files_external' )); ?>"><?php p($l->t('You can add external storages in the personal settings')); ?></a></p> + <h2><?php p($l->t('No external storage configured or you don\'t have the permission to configure them')); ?></h2> </div> <input type="hidden" name="dir" value="" id="dir"> diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index 237985bfd0f..11b3451c32e 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -87,6 +87,11 @@ } ?> +<div id="emptycontent" class="hidden"> + <div class="icon-external"></div> + <h2><?php p($l->t('No external storage configured or you don\'t have the permission to configure them')); ?></h2> +</div> + <form data-can-create="<?php echo $canCreateMounts?'true':'false' ?>" id="files_external" class="section" data-encryption-enabled="<?php echo $_['encryptionEnabled']?'true': 'false'; ?>"> <h2 data-anchor-name="external-storage"><?php p($l->t('External storages')); ?></h2> <?php if (isset($_['dependencies']) and ($_['dependencies'] !== '') and $canCreateMounts) print_unescaped(''.$_['dependencies'].''); ?> diff --git a/apps/systemtags/lib/Activity/Listener.php b/apps/systemtags/lib/Activity/Listener.php index b2b760ac344..7d84726d537 100644 --- a/apps/systemtags/lib/Activity/Listener.php +++ b/apps/systemtags/lib/Activity/Listener.php @@ -184,7 +184,7 @@ class Listener { /** @var Node $node */ $node = array_shift($nodes); $al = $this->shareHelper->getPathsForAccessList($node); - $users = array_merge($users, $al['users']); + $users += $al['users']; } } @@ -202,6 +202,7 @@ class Listener { ->setObject($event->getObjectType(), (int) $event->getObjectId()); foreach ($users as $user => $path) { + $user = (string)$user; // numerical ids could be ints which are not accepted everywhere $activity->setAffectedUser($user); foreach ($tags as $tag) { diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 4e9ce1b646e..e7865b90cf0 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -51,6 +51,7 @@ use OCP\IRequest; use OCA\Theming\Util; use OCP\ITempManager; use OCP\IURLGenerator; +use OCP\App\IAppManager; /** * Class ThemingController @@ -78,6 +79,8 @@ class ThemingController extends Controller { private $scssCacher; /** @var IURLGenerator */ private $urlGenerator; + /** @var IAppManager */ + private $appManager; /** * ThemingController constructor. @@ -93,6 +96,7 @@ class ThemingController extends Controller { * @param IAppData $appData * @param SCSSCacher $scssCacher * @param IURLGenerator $urlGenerator + * @param IAppManager $appManager */ public function __construct( $appName, @@ -105,7 +109,8 @@ class ThemingController extends Controller { ITempManager $tempManager, IAppData $appData, SCSSCacher $scssCacher, - IURLGenerator $urlGenerator + IURLGenerator $urlGenerator, + IAppManager $appManager ) { parent::__construct($appName, $request); @@ -118,6 +123,7 @@ class ThemingController extends Controller { $this->appData = $appData; $this->scssCacher = $scssCacher; $this->urlGenerator = $urlGenerator; + $this->appManager = $appManager; } /** @@ -409,12 +415,13 @@ class ThemingController extends Controller { * @return FileDisplayResponse|NotFoundResponse */ public function getStylesheet() { - $appPath = substr(\OC::$server->getAppManager()->getAppPath('theming'), strlen(\OC::$SERVERROOT) + 1); + $appPath = $this->appManager->getAppPath('theming'); + /* SCSSCacher is required here * We cannot rely on automatic caching done by \OC_Util::addStyle, * since we need to add the cacheBuster value to the url */ - $cssCached = $this->scssCacher->process(\OC::$SERVERROOT, $appPath . '/css/theming.scss', 'theming'); + $cssCached = $this->scssCacher->process($appPath, 'css/theming.scss', 'theming'); if(!$cssCached) { return new NotFoundResponse(); } diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 54b842bc4e8..debc1b71e47 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -106,7 +106,8 @@ class ThemingControllerTest extends TestCase { $this->tempManager, $this->appData, $this->scssCacher, - $this->urlGenerator + $this->urlGenerator, + $this->appManager ); return parent::setUp(); @@ -798,7 +799,7 @@ class ThemingControllerTest extends TestCase { public function testGetStylesheet() { - + $this->appManager->expects($this->once())->method('getAppPath')->with('theming')->willReturn(\OC::$SERVERROOT . '/theming'); $file = $this->createMock(ISimpleFile::class); $file->expects($this->any())->method('getName')->willReturn('theming.css'); $file->expects($this->any())->method('getContent')->willReturn('compiled'); @@ -818,6 +819,7 @@ class ThemingControllerTest extends TestCase { } public function testGetStylesheetFails() { + $this->appManager->expects($this->once())->method('getAppPath')->with('theming')->willReturn(\OC::$SERVERROOT . '/theming'); $file = $this->createMock(ISimpleFile::class); $file->expects($this->any())->method('getName')->willReturn('theming.css'); $file->expects($this->any())->method('getContent')->willReturn('compiled'); @@ -829,6 +831,26 @@ class ThemingControllerTest extends TestCase { $this->assertEquals($response, $actual); } + public function testGetStylesheetOutsideServerroot() { + $this->appManager->expects($this->once())->method('getAppPath')->with('theming')->willReturn('/outside/serverroot/theming'); + $file = $this->createMock(ISimpleFile::class); + $file->expects($this->any())->method('getName')->willReturn('theming.css'); + $file->expects($this->any())->method('getContent')->willReturn('compiled'); + $this->scssCacher->expects($this->once())->method('process')->with('/outside/serverroot/theming', 'css/theming.scss', 'theming')->willReturn(true); + $this->scssCacher->expects($this->once())->method('getCachedCSS')->willReturn($file); + + $response = new Http\FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'text/css']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC1123)); + $response->addHeader('Pragma', 'cache'); + + $actual = $this->themingController->getStylesheet(); + $this->assertEquals($response, $actual); + } + public function testGetJavascript() { $this->themingDefaults ->expects($this->at(0)) diff --git a/build/integration/features/comments.feature b/build/integration/features/comments.feature index 135bb016527..0ee11bc9873 100644 --- a/build/integration/features/comments.feature +++ b/build/integration/features/comments.feature @@ -16,21 +16,21 @@ Feature: comments Scenario: Creating a comment on a shared file belonging to another user Given user "user0" exists - Given user "user1" exists + Given user "12345" exists Given User "user0" uploads file "data/textfile.txt" to "/myFileToComment.txt" Given As "user0" sending "POST" to "/apps/files_sharing/api/v1/shares" with | path | myFileToComment.txt | - | shareWith | user1 | + | shareWith | 12345 | | shareType | 0 | - When "user1" posts a comment with content "A comment from another user" on the file named "/myFileToComment.txt" it should return "201" - Then As "user1" load all the comments of the file named "/myFileToComment.txt" it should return "207" + When "12345" posts a comment with content "A comment from another user" on the file named "/myFileToComment.txt" it should return "201" + Then As "12345" load all the comments of the file named "/myFileToComment.txt" it should return "207" And the response should contain a property "oc:parentId" with value "0" And the response should contain a property "oc:childrenCount" with value "0" And the response should contain a property "oc:verb" with value "comment" And the response should contain a property "oc:actorType" with value "users" And the response should contain a property "oc:objectType" with value "files" And the response should contain a property "oc:message" with value "A comment from another user" - And the response should contain a property "oc:actorDisplayName" with value "user1" + And the response should contain a property "oc:actorDisplayName" with value "12345" And the response should contain only "1" comments Scenario: Creating a comment on a non-shared file belonging to another user @@ -206,4 +206,4 @@ Feature: comments And the response should contain a property "oc:message" with value "My first comment" And the response should contain a property "oc:actorDisplayName" with value "user1" And the response should contain only "1" comments - Then As "user0" edit the last created comment and set text to "My edited comment" it should return "403"
\ No newline at end of file + Then As "user0" edit the last created comment and set text to "My edited comment" it should return "403" diff --git a/build/integration/features/tags.feature b/build/integration/features/tags.feature index 0c6cd06f9f9..3ef7ccb38b0 100644 --- a/build/integration/features/tags.feature +++ b/build/integration/features/tags.feature @@ -114,14 +114,14 @@ Feature: tags Scenario: Assigning a normal tag to a file shared by someone else as regular user should work Given user "user0" exists - Given user "user1" exists + Given user "12345" exists Given "admin" creates a "normal" tag with name "MySuperAwesomeTagName" Given user "user0" uploads file "data/textfile.txt" to "/myFileToTag.txt" Given As "user0" sending "POST" to "/apps/files_sharing/api/v1/shares" with | path | myFileToTag.txt | - | shareWith | user1 | + | shareWith | 12345 | | shareType | 0 | - When "user1" adds the tag "MySuperAwesomeTagName" to "/myFileToTag.txt" shared by "user0" + When "12345" adds the tag "MySuperAwesomeTagName" to "/myFileToTag.txt" shared by "user0" Then The response should have a status code "201" And "/myFileToTag.txt" shared by "user0" has the following tags |MySuperAwesomeTagName| diff --git a/core/Command/App/Install.php b/core/Command/App/Install.php index 8f530975be9..4432a1f40ac 100644 --- a/core/Command/App/Install.php +++ b/core/Command/App/Install.php @@ -25,6 +25,7 @@ namespace OC\Core\Command\App; use OC\Installer; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -39,6 +40,12 @@ class Install extends Command { InputArgument::REQUIRED, 'install the specified app' ) + ->addOption( + 'keep-disabled', + null, + InputOption::VALUE_NONE, + 'don\'t enable the app afterwards' + ) ; } @@ -66,6 +73,12 @@ class Install extends Command { $output->writeln($appId . ' installed'); + if (!$input->getOption('keep-disabled')) { + $appClass = new \OC_App(); + $appClass->enable($appId); + $output->writeln($appId . ' enabled'); + } + return 0; } } diff --git a/core/Command/Db/ConvertFilecacheBigInt.php b/core/Command/Db/ConvertFilecacheBigInt.php index 75d3a48a5c8..5960b7aa9dd 100644 --- a/core/Command/Db/ConvertFilecacheBigInt.php +++ b/core/Command/Db/ConvertFilecacheBigInt.php @@ -54,7 +54,7 @@ class ConvertFilecacheBigInt extends Command { return [ 'activity' => ['activity_id', 'object_id'], 'activity_mq' => ['mail_id'], - 'filecache' => ['fileid', 'storage', 'parent', 'mimetype', 'mimepart'], + 'filecache' => ['fileid', 'storage', 'parent', 'mimetype', 'mimepart', 'mtime', 'storage_mtime'], 'mimetypes' => ['id'], 'storages' => ['numeric_id'], ]; diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 30cbeff3c64..e76a25bd020 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -30,7 +30,7 @@ '<span class="sharingOptionsGroup">' + '{{#if editPermissionPossible}}' + '<span class="shareOption">' + - '<input id="canEdit-{{cid}}-{{shareWith}}" type="checkbox" name="edit" class="permissions checkbox" {{#if hasEditPermission}}checked="checked"{{/if}} />' + + '<input id="canEdit-{{cid}}-{{shareWith}}" type="checkbox" name="edit" class="permissions checkbox" />' + '<label for="canEdit-{{cid}}-{{shareWith}}">{{canEditLabel}}</label>' + '</span>' + '{{/if}}' + @@ -232,7 +232,7 @@ return _.extend(hasPermissionOverride, { cid: this.cid, hasSharePermission: this.model.hasSharePermission(shareIndex), - hasEditPermission: this.model.hasEditPermission(shareIndex), + editPermissionState: this.model.editPermissionState(shareIndex), hasCreatePermission: this.model.hasCreatePermission(shareIndex), hasUpdatePermission: this.model.hasUpdatePermission(shareIndex), hasDeletePermission: this.model.hasDeletePermission(shareIndex), @@ -379,16 +379,18 @@ $.extend(sharee, this.getShareProperties()); var $li = this.$('li[data-share-id=' + permissionChangeShareId + ']'); $li.find('.sharingOptionsGroup .popovermenu').replaceWith(this.popoverMenuTemplate(sharee)); - - var checkBoxId = 'canEdit-' + this.cid + '-' + sharee.shareWith; - checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@)/g, "\\$1"); - var $edit = $li.parent().find(checkBoxId); - if($edit.length === 1) { - $edit.prop('checked', sharee.hasEditPermission); - } } var _this = this; + this.getShareeList().forEach(function(sharee) { + var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareWith; + checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@|\/)/g, "\\$1"); + var $edit = _this.$(checkBoxId); + if($edit.length === 1) { + $edit.prop('checked', sharee.editPermissionState === 'checked'); + $edit.prop('indeterminate', sharee.editPermissionState === 'indeterminate'); + } + }); this.$('.popovermenu').on('afterHide', function() { _this._menuOpen = false; }); @@ -627,8 +629,10 @@ } } else { var numberChecked = $checkboxes.filter(':checked').length; - checked = numberChecked > 0; - $('input[name="edit"]', $li).prop('checked', checked); + checked = numberChecked === $checkboxes.length; + var $editCb = $('input[name="edit"]', $li); + $editCb.prop('checked', checked); + $editCb.prop('indeterminate', !checked && numberChecked > 0); } } else { if ($element.attr('name') === 'edit' && $element.is(':checked')) { diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index afe86fa464b..b699513c734 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -567,12 +567,26 @@ }, /** - * @returns {boolean} - */ - hasEditPermission: function(shareIndex) { - return this.hasCreatePermission(shareIndex) - || this.hasUpdatePermission(shareIndex) - || this.hasDeletePermission(shareIndex); + * @returns {string} + * The state that the 'can edit' permission checkbox should have. + * Possible values: + * - empty string: no permission + * - 'checked': all applicable permissions + * - 'indeterminate': some but not all permissions + */ + editPermissionState: function(shareIndex) { + var hcp = this.hasCreatePermission(shareIndex); + var hup = this.hasUpdatePermission(shareIndex); + var hdp = this.hasDeletePermission(shareIndex); + if (!hcp && !hup && !hdp) { + return ''; + } + if ( (this.createPermissionPossible() && !hcp) + || (this.updatePermissionPossible() && !hup) + || (this.deletePermissionPossible() && !hdp) ) { + return 'indeterminate'; + } + return 'checked'; }, /** diff --git a/core/js/tests/specs/sharedialogshareelistview.js b/core/js/tests/specs/sharedialogshareelistview.js index 8ee2c48fe39..cc0268ba580 100644 --- a/core/js/tests/specs/sharedialogshareelistview.js +++ b/core/js/tests/specs/sharedialogshareelistview.js @@ -89,6 +89,37 @@ describe('OC.Share.ShareDialogShareeListView', function () { updateShareStub.restore(); }); + describe('Sets correct initial checkbox state', function () { + it('marks edit box as indeterminate when only some permissions are given', function () { + shareModel.set('shares', [{ + id: 100, + item_source: 123, + permissions: 1 | OC.PERMISSION_UPDATE, + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One', + itemType: 'folder' + }]); + shareModel.set('itemType', 'folder'); + listView.render(); + expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true); + }); + + it('Checks edit box when all permissions are given', function () { + shareModel.set('shares', [{ + id: 100, + item_source: 123, + permissions: 1 | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE, + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One', + itemType: 'folder' + }]); + shareModel.set('itemType', 'folder'); + listView.render(); + expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true); + }); + }); describe('Manages checkbox events correctly', function () { it('Checks cruds boxes when edit box checked', function () { shareModel.set('shares', [{ @@ -106,7 +137,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(updateShareStub.calledOnce).toEqual(true); }); - it('Checks edit box when create/update/delete are checked', function () { + it('marks edit box as indeterminate when some of create/update/delete are checked', function () { shareModel.set('shares', [{ id: 100, item_source: 123, @@ -119,6 +150,23 @@ describe('OC.Share.ShareDialogShareeListView', function () { shareModel.set('itemType', 'folder'); listView.render(); listView.$el.find("input[name='update']").click(); + expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true); + expect(updateShareStub.calledOnce).toEqual(true); + }); + + it('Checks edit box when all of create/update/delete are checked', function () { + shareModel.set('shares', [{ + id: 100, + item_source: 123, + permissions: 1 | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE, + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One', + itemType: 'folder' + }]); + shareModel.set('itemType', 'folder'); + listView.render(); + listView.$el.find("input[name='update']").click(); expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true); expect(updateShareStub.calledOnce).toEqual(true); }); diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 44c268bdea6..4dcb94a1d33 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -219,7 +219,8 @@ class DIContainer extends SimpleContainer implements IAppContainer { $server->getContentSecurityPolicyManager(), $server->getCsrfTokenManager(), $server->getContentSecurityPolicyNonceManager(), - $server->getAppManager() + $server->getAppManager(), + $server->getL10N('lib') ); }); diff --git a/lib/private/AppFramework/Middleware/Security/Exceptions/NotAdminException.php b/lib/private/AppFramework/Middleware/Security/Exceptions/NotAdminException.php index ca8a2c89416..cd73c81e18a 100644 --- a/lib/private/AppFramework/Middleware/Security/Exceptions/NotAdminException.php +++ b/lib/private/AppFramework/Middleware/Security/Exceptions/NotAdminException.php @@ -1,4 +1,5 @@ <?php +declare(strict_types=1); /** * @copyright Copyright (c) 2016, ownCloud, Inc. * @@ -35,7 +36,7 @@ use OCP\AppFramework\Http; * @package OC\AppFramework\Middleware\Security\Exceptions */ class NotAdminException extends SecurityException { - public function __construct($message = 'Logged in user must be an admin') { + public function __construct(string $message) { parent::__construct($message, Http::STATUS_FORBIDDEN); } } diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index f45c8f8726c..bb3083c835c 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -48,6 +48,7 @@ use OCP\AppFramework\Middleware; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\OCSController; +use OCP\IL10N; use OCP\INavigationManager; use OCP\IURLGenerator; use OCP\IRequest; @@ -87,6 +88,8 @@ class SecurityMiddleware extends Middleware { private $cspNonceManager; /** @var IAppManager */ private $appManager; + /** @var IL10N */ + private $l10n; /** * @param IRequest $request @@ -101,6 +104,7 @@ class SecurityMiddleware extends Middleware { * @param CSRFTokenManager $csrfTokenManager * @param ContentSecurityPolicyNonceManager $cspNonceManager * @param IAppManager $appManager + * @param IL10N $l10n */ public function __construct(IRequest $request, ControllerMethodReflector $reflector, @@ -113,7 +117,8 @@ class SecurityMiddleware extends Middleware { ContentSecurityPolicyManager $contentSecurityPolicyManager, CsrfTokenManager $csrfTokenManager, ContentSecurityPolicyNonceManager $cspNonceManager, - IAppManager $appManager + IAppManager $appManager, + IL10N $l10n ) { $this->navigationManager = $navigationManager; $this->request = $request; @@ -127,6 +132,7 @@ class SecurityMiddleware extends Middleware { $this->csrfTokenManager = $csrfTokenManager; $this->cspNonceManager = $cspNonceManager; $this->appManager = $appManager; + $this->l10n = $l10n; } /** @@ -152,7 +158,7 @@ class SecurityMiddleware extends Middleware { if(!$this->reflector->hasAnnotation('NoAdminRequired')) { if(!$this->isAdminUser) { - throw new NotAdminException(); + throw new NotAdminException($this->l10n->t('Logged in user must be an admin')); } } } diff --git a/settings/Middleware/SubadminMiddleware.php b/settings/Middleware/SubadminMiddleware.php index 9914d65af02..5df17cb13b4 100644 --- a/settings/Middleware/SubadminMiddleware.php +++ b/settings/Middleware/SubadminMiddleware.php @@ -30,6 +30,7 @@ use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Middleware; +use OCP\IL10N; /** * Verifies whether an user has at least subadmin rights. @@ -42,15 +43,20 @@ class SubadminMiddleware extends Middleware { protected $isSubAdmin; /** @var ControllerMethodReflector */ protected $reflector; + /** @var IL10N */ + private $l10n; /** * @param ControllerMethodReflector $reflector * @param bool $isSubAdmin + * @param IL10N $l10n */ public function __construct(ControllerMethodReflector $reflector, - $isSubAdmin) { + $isSubAdmin, + IL10N $l10n) { $this->reflector = $reflector; $this->isSubAdmin = $isSubAdmin; + $this->l10n = $l10n; } /** @@ -62,7 +68,7 @@ class SubadminMiddleware extends Middleware { public function beforeController($controller, $methodName) { if(!$this->reflector->hasAnnotation('NoSubadminRequired')) { if(!$this->isSubAdmin) { - throw new NotAdminException('Logged in user must be a subadmin'); + throw new NotAdminException($this->l10n->t('Logged in user must be a subadmin')); } } } diff --git a/tests/Settings/Middleware/SubadminMiddlewareTest.php b/tests/Settings/Middleware/SubadminMiddlewareTest.php index 834a3fedf23..b464b595ab7 100644 --- a/tests/Settings/Middleware/SubadminMiddlewareTest.php +++ b/tests/Settings/Middleware/SubadminMiddlewareTest.php @@ -15,6 +15,7 @@ use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Settings\Middleware\SubadminMiddleware; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\TemplateResponse; +use OCP\IL10N; /** * Verifies whether an user has at least subadmin rights. @@ -31,6 +32,8 @@ class SubadminMiddlewareTest extends \Test\TestCase { private $reflector; /** @var Controller */ private $controller; + /** @var IL10N */ + private $l10n; protected function setUp() { parent::setUp(); @@ -38,9 +41,10 @@ class SubadminMiddlewareTest extends \Test\TestCase { ->disableOriginalConstructor()->getMock(); $this->controller = $this->getMockBuilder(Controller::class) ->disableOriginalConstructor()->getMock(); + $this->l10n = $this->createMock(IL10N::class); - $this->subadminMiddlewareAsSubAdmin = new SubadminMiddleware($this->reflector, true); - $this->subadminMiddleware = new SubadminMiddleware($this->reflector, false); + $this->subadminMiddlewareAsSubAdmin = new SubadminMiddleware($this->reflector, true, $this->l10n); + $this->subadminMiddleware = new SubadminMiddleware($this->reflector, false, $this->l10n); } /** @@ -86,7 +90,7 @@ class SubadminMiddlewareTest extends \Test\TestCase { public function testAfterNotAdminException() { $expectedResponse = new TemplateResponse('core', '403', array(), 'guest'); $expectedResponse->setStatus(403); - $this->assertEquals($expectedResponse, $this->subadminMiddleware->afterException($this->controller, 'foo', new NotAdminException())); + $this->assertEquals($expectedResponse, $this->subadminMiddleware->afterException($this->controller, 'foo', new NotAdminException(''))); } /** diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index e36bd727bea..a631fe59a60 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -45,13 +45,11 @@ use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Http\TemplateResponse; use OCP\IConfig; +use OCP\IL10N; use OCP\ILogger; use OCP\INavigationManager; use OCP\IRequest; -use OCP\ISession; use OCP\IURLGenerator; -use OCP\IUser; -use OCP\IUserSession; use OCP\Security\ISecureRandom; class SecurityMiddlewareTest extends \Test\TestCase { @@ -82,8 +80,8 @@ class SecurityMiddlewareTest extends \Test\TestCase { private $cspNonceManager; /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ private $appManager; - /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ - private $userSession; + /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ + private $l10n; protected function setUp() { parent::setUp(); @@ -98,6 +96,7 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class); $this->cspNonceManager = $this->createMock(ContentSecurityPolicyNonceManager::class); $this->appManager = $this->createMock(IAppManager::class); + $this->l10n = $this->createMock(IL10N::class); $this->appManager->expects($this->any()) ->method('isEnabledForUser') ->willReturn(true); @@ -124,7 +123,8 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->contentSecurityPolicyManager, $this->csrfTokenManager, $this->cspNonceManager, - $this->appManager + $this->appManager, + $this->l10n ); } @@ -541,7 +541,7 @@ class SecurityMiddlewareTest extends \Test\TestCase { new CrossSiteRequestForgeryException(), ], [ - new NotAdminException(), + new NotAdminException(''), ], ]; } |