From 3f492dd82681ca92e9e86acfcf1c15dfcfcf34cf Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 12 Apr 2016 10:19:52 +0200 Subject: [PATCH 1/3] load file sorting mode from the db --- apps/files/controller/viewcontroller.php | 1 + apps/files/js/app.js | 3 ++- apps/files/js/filelist.js | 6 +++++- apps/files/templates/index.php | 1 + 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/files/controller/viewcontroller.php b/apps/files/controller/viewcontroller.php index 800cf008fa7..b71c8e38a79 100644 --- a/apps/files/controller/viewcontroller.php +++ b/apps/files/controller/viewcontroller.php @@ -213,6 +213,7 @@ class ViewController extends Controller { $params['mailNotificationEnabled'] = $this->config->getAppValue('core', 'shareapi_allow_mail_notification', 'no'); $params['mailPublicNotificationEnabled'] = $this->config->getAppValue('core', 'shareapi_allow_public_notification', 'no'); $params['allowShareWithLink'] = $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'); + $params['defaultFileSorting'] = $this->config->getAppValue('files', 'file_sorting', 'name'); $params['appNavigation'] = $nav; $params['appContents'] = $contentItems; $this->navigationManager->setActiveEntry('files_index'); diff --git a/apps/files/js/app.js b/apps/files/js/app.js index ff505d417f1..8662cd7c852 100644 --- a/apps/files/js/app.js +++ b/apps/files/js/app.js @@ -72,7 +72,8 @@ fileActions: fileActions, allowLegacyActions: true, scrollTo: urlParams.scrollto, - filesClient: OC.Files.getClient() + filesClient: OC.Files.getClient(), + sorting: $('#defaultFileSorting').val() } ); this.files.initialize(); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 71af4a21b9b..322dfcaa66e 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -239,7 +239,11 @@ this.fileSummary = this._createSummary(); - this.setSort('name', 'asc'); + if (options.sorting) { + this.setSort(options.sorting, 'asc'); + } else { + this.setSort('name', 'asc'); + } var breadcrumbOptions = { onClick: _.bind(this._onClickBreadCrumb, this), diff --git a/apps/files/templates/index.php b/apps/files/templates/index.php index e825c300d31..c108c6f0613 100644 --- a/apps/files/templates/index.php +++ b/apps/files/templates/index.php @@ -18,4 +18,5 @@ + Date: Tue, 12 Apr 2016 11:08:26 +0200 Subject: [PATCH 2/3] persist file sorting changes --- apps/files/appinfo/application.php | 3 ++- apps/files/appinfo/routes.php | 6 ++++++ apps/files/controller/apicontroller.php | 19 +++++++++++++++++-- apps/files/controller/viewcontroller.php | 14 +++++++++++--- apps/files/js/app.js | 5 ++++- apps/files/js/filelist.js | 7 ++++++- apps/files/templates/index.php | 1 + 7 files changed, 47 insertions(+), 8 deletions(-) diff --git a/apps/files/appinfo/application.php b/apps/files/appinfo/application.php index 593e0533c80..2d2decf6288 100644 --- a/apps/files/appinfo/application.php +++ b/apps/files/appinfo/application.php @@ -43,7 +43,8 @@ class Application extends App { $server->getUserSession(), $c->query('TagService'), $server->getPreviewManager(), - $server->getShareManager() + $server->getShareManager(), + $server->getConfig() ); }); diff --git a/apps/files/appinfo/routes.php b/apps/files/appinfo/routes.php index 731c671b60a..6ad938101a2 100644 --- a/apps/files/appinfo/routes.php +++ b/apps/files/appinfo/routes.php @@ -1,6 +1,7 @@ + * @author Christoph Wurst * @author Lukas Reschke * @author Roeland Jago Douma * @author Tobias Kaminsky @@ -48,6 +49,11 @@ $application->registerRoutes( 'verb' => 'GET', 'requirements' => array('tagName' => '.+'), ), + array( + 'name' => 'API#updateFileSorting', + 'url' => '/api/v1/sorting', + 'verb' => 'POST' + ), [ 'name' => 'view#index', 'url' => '/', diff --git a/apps/files/controller/apicontroller.php b/apps/files/controller/apicontroller.php index ad286284386..82d3ebb58b7 100644 --- a/apps/files/controller/apicontroller.php +++ b/apps/files/controller/apicontroller.php @@ -1,6 +1,7 @@ + * @author Christoph Wurst * @author Lukas Reschke * @author Morris Jobke * @author Roeland Jago Douma @@ -29,13 +30,13 @@ namespace OCA\Files\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Controller; +use OCP\IConfig; use OCP\IRequest; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\DataDisplayResponse; use OCA\Files\Service\TagService; use OCP\IPreview; use OCP\Share\IManager; -use OCP\Files\FileInfo; use OCP\Files\Node; use OCP\IUserSession; @@ -53,6 +54,8 @@ class ApiController extends Controller { private $previewManager; /** IUserSession */ private $userSession; + /** IConfig */ + private $config; /** * @param string $appName @@ -65,12 +68,14 @@ class ApiController extends Controller { IUserSession $userSession, TagService $tagService, IPreview $previewManager, - IManager $shareManager) { + IManager $shareManager, + IConfig $config) { parent::__construct($appName, $request); $this->userSession = $userSession; $this->tagService = $tagService; $this->previewManager = $previewManager; $this->shareManager = $shareManager; + $this->config = $config; } /** @@ -196,4 +201,14 @@ class ApiController extends Controller { return $shareTypes; } + public function updateFileSorting($mode, $direction) { + $allowedMode = ['name', 'size', 'mtime']; + $allowedDirection = ['asc', 'desc']; + if (!in_array($mode, $allowedMode) || !in_array($direction, $allowedDirection)) { + return $this->buildResponse(null)->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); + } + $this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'file_sorting', $mode); + $this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'file_sorting_direction', $direction); + } + } diff --git a/apps/files/controller/viewcontroller.php b/apps/files/controller/viewcontroller.php index b71c8e38a79..a3718729339 100644 --- a/apps/files/controller/viewcontroller.php +++ b/apps/files/controller/viewcontroller.php @@ -1,5 +1,6 @@ * @author Lukas Reschke * @author Thomas Müller * @@ -27,11 +28,12 @@ use OCP\AppFramework\Controller; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\IConfig; use OCP\IL10N; use OCP\INavigationManager; use OCP\IRequest; use OCP\IURLGenerator; -use OCP\IConfig; +use OCP\IUserSession; use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** @@ -54,6 +56,8 @@ class ViewController extends Controller { protected $config; /** @var EventDispatcherInterface */ protected $eventDispatcher; + /** @var IUserSession */ + protected $userSession; /** * @param string $appName @@ -70,7 +74,8 @@ class ViewController extends Controller { INavigationManager $navigationManager, IL10N $l10n, IConfig $config, - EventDispatcherInterface $eventDispatcherInterface) { + EventDispatcherInterface $eventDispatcherInterface, + IUserSession $userSession) { parent::__construct($appName, $request); $this->appName = $appName; $this->request = $request; @@ -79,6 +84,7 @@ class ViewController extends Controller { $this->l10n = $l10n; $this->config = $config; $this->eventDispatcher = $eventDispatcherInterface; + $this->userSession = $userSession; } /** @@ -213,7 +219,9 @@ class ViewController extends Controller { $params['mailNotificationEnabled'] = $this->config->getAppValue('core', 'shareapi_allow_mail_notification', 'no'); $params['mailPublicNotificationEnabled'] = $this->config->getAppValue('core', 'shareapi_allow_public_notification', 'no'); $params['allowShareWithLink'] = $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'); - $params['defaultFileSorting'] = $this->config->getAppValue('files', 'file_sorting', 'name'); + $user = $this->userSession->getUser()->getUID(); + $params['defaultFileSorting'] = $this->config->getUserValue($user, 'files', 'file_sorting', 'name'); + $params['defaultFileSortingDirection'] = $this->config->getUserValue($user, 'files', 'file_sorting_direction', 'name'); $params['appNavigation'] = $nav; $params['appContents'] = $contentItems; $this->navigationManager->setActiveEntry('files_index'); diff --git a/apps/files/js/app.js b/apps/files/js/app.js index 8662cd7c852..4ed805d2681 100644 --- a/apps/files/js/app.js +++ b/apps/files/js/app.js @@ -73,7 +73,10 @@ allowLegacyActions: true, scrollTo: urlParams.scrollto, filesClient: OC.Files.getClient(), - sorting: $('#defaultFileSorting').val() + sorting: { + mode: $('#defaultFileSorting').val(), + direction: $('#defaultFileSortingDirection').val() + } } ); this.files.initialize(); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 322dfcaa66e..fef371b8479 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -240,7 +240,7 @@ this.fileSummary = this._createSummary(); if (options.sorting) { - this.setSort(options.sorting, 'asc'); + this.setSort(options.sorting.mode, options.sorting.direction); } else { this.setSort('name', 'asc'); } @@ -1401,6 +1401,11 @@ this.reload(); } } + + $.post(OC.generateUrl('/apps/files/api/v1/sorting'), { + mode: sort, + direction: direction + }); }, /** diff --git a/apps/files/templates/index.php b/apps/files/templates/index.php index c108c6f0613..db464ad2eca 100644 --- a/apps/files/templates/index.php +++ b/apps/files/templates/index.php @@ -19,4 +19,5 @@ + Date: Tue, 12 Apr 2016 11:51:50 +0200 Subject: [PATCH 3/3] fix default value, update js/php tests --- apps/files/controller/apicontroller.php | 15 ++++- apps/files/controller/viewcontroller.php | 2 +- apps/files/js/filelist.js | 23 ++++---- .../tests/controller/ViewControllerTest.php | 22 +++++++- .../tests/controller/apicontrollertest.php | 55 +++++++++++++++++-- apps/files/tests/js/filelistSpec.js | 16 ++++++ 6 files changed, 116 insertions(+), 17 deletions(-) diff --git a/apps/files/controller/apicontroller.php b/apps/files/controller/apicontroller.php index 82d3ebb58b7..43d426476fe 100644 --- a/apps/files/controller/apicontroller.php +++ b/apps/files/controller/apicontroller.php @@ -34,6 +34,7 @@ use OCP\IConfig; use OCP\IRequest; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\AppFramework\Http\Response; use OCA\Files\Service\TagService; use OCP\IPreview; use OCP\Share\IManager; @@ -201,14 +202,26 @@ class ApiController extends Controller { return $shareTypes; } + /** + * Change the default sort mode + * + * @NoAdminRequired + * + * @param string $mode + * @param string $direction + * @return Response + */ public function updateFileSorting($mode, $direction) { $allowedMode = ['name', 'size', 'mtime']; $allowedDirection = ['asc', 'desc']; if (!in_array($mode, $allowedMode) || !in_array($direction, $allowedDirection)) { - return $this->buildResponse(null)->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); + $response = new Response(); + $response->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); + return $response; } $this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'file_sorting', $mode); $this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'file_sorting_direction', $direction); + return new Response(); } } diff --git a/apps/files/controller/viewcontroller.php b/apps/files/controller/viewcontroller.php index a3718729339..6c5f4c6d2a0 100644 --- a/apps/files/controller/viewcontroller.php +++ b/apps/files/controller/viewcontroller.php @@ -221,7 +221,7 @@ class ViewController extends Controller { $params['allowShareWithLink'] = $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'); $user = $this->userSession->getUser()->getUID(); $params['defaultFileSorting'] = $this->config->getUserValue($user, 'files', 'file_sorting', 'name'); - $params['defaultFileSortingDirection'] = $this->config->getUserValue($user, 'files', 'file_sorting_direction', 'name'); + $params['defaultFileSortingDirection'] = $this->config->getUserValue($user, 'files', 'file_sorting_direction', 'asc'); $params['appNavigation'] = $nav; $params['appContents'] = $contentItems; $this->navigationManager->setActiveEntry('files_index'); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index fef371b8479..78c014e431e 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -240,9 +240,9 @@ this.fileSummary = this._createSummary(); if (options.sorting) { - this.setSort(options.sorting.mode, options.sorting.direction); + this.setSort(options.sorting.mode, options.sorting.direction, false, false); } else { - this.setSort('name', 'asc'); + this.setSort('name', 'asc', false, false); } var breadcrumbOptions = { @@ -694,14 +694,14 @@ sort = $target.attr('data-sort'); if (sort) { if (this._sort === sort) { - this.setSort(sort, (this._sortDirection === 'desc')?'asc':'desc', true); + this.setSort(sort, (this._sortDirection === 'desc')?'asc':'desc', true, true); } else { if ( sort === 'name' ) { //default sorting of name is opposite to size and mtime - this.setSort(sort, 'asc', true); + this.setSort(sort, 'asc', true, true); } else { - this.setSort(sort, 'desc', true); + this.setSort(sort, 'desc', true, true); } } } @@ -1369,8 +1369,9 @@ * @param sort sort attribute name * @param direction sort direction, one of "asc" or "desc" * @param update true to update the list, false otherwise (default) + * @param persist true to save changes in the database (default) */ - setSort: function(sort, direction, update) { + setSort: function(sort, direction, update, persist) { var comparator = FileList.Comparators[sort] || FileList.Comparators.name; this._sort = sort; this._sortDirection = (direction === 'desc')?'desc':'asc'; @@ -1402,10 +1403,12 @@ } } - $.post(OC.generateUrl('/apps/files/api/v1/sorting'), { - mode: sort, - direction: direction - }); + if (persist) { + $.post(OC.generateUrl('/apps/files/api/v1/sorting'), { + mode: sort, + direction: direction + }); + } }, /** diff --git a/apps/files/tests/controller/ViewControllerTest.php b/apps/files/tests/controller/ViewControllerTest.php index 657ab6cb338..0446cc8982c 100644 --- a/apps/files/tests/controller/ViewControllerTest.php +++ b/apps/files/tests/controller/ViewControllerTest.php @@ -1,5 +1,6 @@ * @author Joas Schilling * @author Lukas Reschke * @author Vincent Petry @@ -33,6 +34,7 @@ use OCP\AppFramework\Http\RedirectResponse; use OCP\INavigationManager; use OCP\IL10N; use OCP\IConfig; +use OCP\IUserSession; use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** @@ -55,6 +57,10 @@ class ViewControllerTest extends TestCase { private $eventDispatcher; /** @var ViewController */ private $viewController; + /** @var IUser */ + private $user; + /** @var IUserSession */ + private $userSession; public function setUp() { parent::setUp(); @@ -64,6 +70,11 @@ class ViewControllerTest extends TestCase { $this->l10n = $this->getMock('\OCP\IL10N'); $this->config = $this->getMock('\OCP\IConfig'); $this->eventDispatcher = $this->getMock('\Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $this->userSession = $this->getMock('\OCP\IUserSession'); + $this->user = $this->getMock('\OCP\IUser'); + $this->userSession->expects($this->any()) + ->method('getUser') + ->will($this->returnValue($this->user)); $this->viewController = $this->getMockBuilder('\OCA\Files\Controller\ViewController') ->setConstructorArgs([ 'files', @@ -72,7 +83,8 @@ class ViewControllerTest extends TestCase { $this->navigationManager, $this->l10n, $this->config, - $this->eventDispatcher + $this->eventDispatcher, + $this->userSession ]) ->setMethods([ 'getStorageInfo', @@ -143,6 +155,12 @@ class ViewControllerTest extends TestCase { 'owner' => 'MyName', 'ownerDisplayName' => 'MyDisplayName', ])); + $this->config->expects($this->exactly(2)) + ->method('getUserValue') + ->will($this->returnValueMap([ + [$this->user->getUID(), 'files', 'file_sorting', 'name', 'name'], + [$this->user->getUID(), 'files', 'file_sorting_direction', 'asc', 'asc'] + ])); $this->config ->expects($this->any()) @@ -224,6 +242,8 @@ class ViewControllerTest extends TestCase { 'owner' => 'MyName', 'ownerDisplayName' => 'MyDisplayName', 'isPublic' => false, + 'defaultFileSorting' => 'name', + 'defaultFileSortingDirection' => 'asc', 'mailNotificationEnabled' => 'no', 'mailPublicNotificationEnabled' => 'no', 'allowShareWithLink' => 'yes', diff --git a/apps/files/tests/controller/apicontrollertest.php b/apps/files/tests/controller/apicontrollertest.php index a9b248a36fe..59f53e8ee81 100644 --- a/apps/files/tests/controller/apicontrollertest.php +++ b/apps/files/tests/controller/apicontrollertest.php @@ -1,5 +1,6 @@ * @author Lukas Reschke * @author Morris Jobke * @author Roeland Jago Douma @@ -43,6 +44,8 @@ use OCP\Image; class ApiControllerTest extends TestCase { /** @var string */ private $appName = 'files'; + /** @var \OCP\IUser */ + private $user; /** @var IRequest */ private $request; /** @var TagService */ @@ -53,19 +56,21 @@ class ApiControllerTest extends TestCase { private $apiController; /** @var \OCP\Share\IManager */ private $shareManager; + /** @var \OCP\IConfig */ + private $config; public function setUp() { $this->request = $this->getMockBuilder('\OCP\IRequest') ->disableOriginalConstructor() ->getMock(); - $user = $this->getMock('\OCP\IUser'); - $user->expects($this->any()) + $this->user = $this->getMock('\OCP\IUser'); + $this->user->expects($this->any()) ->method('getUID') ->will($this->returnValue('user1')); $userSession = $this->getMock('\OCP\IUserSession'); $userSession->expects($this->any()) ->method('getUser') - ->will($this->returnValue($user)); + ->will($this->returnValue($this->user)); $this->tagService = $this->getMockBuilder('\OCA\Files\Service\TagService') ->disableOriginalConstructor() ->getMock(); @@ -75,6 +80,7 @@ class ApiControllerTest extends TestCase { $this->preview = $this->getMockBuilder('\OCP\IPreview') ->disableOriginalConstructor() ->getMock(); + $this->config = $this->getMock('\OCP\IConfig'); $this->apiController = new ApiController( $this->appName, @@ -82,7 +88,8 @@ class ApiControllerTest extends TestCase { $userSession, $this->tagService, $this->preview, - $this->shareManager + $this->shareManager, + $this->config ); } @@ -335,4 +342,44 @@ class ApiControllerTest extends TestCase { $this->assertEquals(Http::STATUS_OK, $ret->getStatus()); } + + public function testUpdateFileSorting() { + $mode = 'mtime'; + $direction = 'desc'; + + $this->config->expects($this->at(0)) + ->method('setUserValue') + ->with($this->user->getUID(), 'files', 'file_sorting', $mode); + $this->config->expects($this->at(1)) + ->method('setUserValue') + ->with($this->user->getUID(), 'files', 'file_sorting_direction', $direction); + + $expected = new HTTP\Response(); + $actual = $this->apiController->updateFileSorting($mode, $direction); + $this->assertEquals($expected, $actual); + } + + public function invalidSortingModeData() { + return [ + ['color', 'asc'], + ['name', 'size'], + ['foo', 'bar'] + ]; + } + + /** + * @dataProvider invalidSortingModeData + */ + public function testUpdateInvalidFileSorting($mode, $direction) { + $this->config->expects($this->never()) + ->method('setUserValue'); + + $expected = new Http\Response(null); + $expected->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); + + $result = $this->apiController->updateFileSorting($mode, $direction); + + $this->assertEquals($expected, $result); + } + } diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index a83c8c4c0bc..cc3bcd74b46 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -2106,6 +2106,8 @@ describe('OCA.Files.FileList tests', function() { it('Toggles the sort indicator when clicking on a column header', function() { var ASC_CLASS = fileList.SORT_INDICATOR_ASC_CLASS; var DESC_CLASS = fileList.SORT_INDICATOR_DESC_CLASS; + var request; + var sortingUrl = OC.generateUrl('/apps/files/api/v1/sorting'); fileList.$el.find('.column-size .columntitle').click(); // moves triangle to size column, check indicator on name is hidden expect( @@ -2118,6 +2120,10 @@ describe('OCA.Files.FileList tests', function() { expect( fileList.$el.find('.column-size .sort-indicator').hasClass(DESC_CLASS) ).toEqual(true); + // check if changes are persisted + expect(fakeServer.requests.length).toEqual(1); + request = fakeServer.requests[0]; + expect(request.url).toEqual(sortingUrl); // click again on size column, reverses direction fileList.$el.find('.column-size .columntitle').click(); @@ -2127,6 +2133,10 @@ describe('OCA.Files.FileList tests', function() { expect( fileList.$el.find('.column-size .sort-indicator').hasClass(ASC_CLASS) ).toEqual(true); + // check if changes are persisted + expect(fakeServer.requests.length).toEqual(2); + request = fakeServer.requests[1]; + expect(request.url).toEqual(sortingUrl); // click again on size column, reverses direction fileList.$el.find('.column-size .columntitle').click(); @@ -2136,6 +2146,9 @@ describe('OCA.Files.FileList tests', function() { expect( fileList.$el.find('.column-size .sort-indicator').hasClass(DESC_CLASS) ).toEqual(true); + expect(fakeServer.requests.length).toEqual(3); + request = fakeServer.requests[2]; + expect(request.url).toEqual(sortingUrl); // click on mtime column, moves indicator there fileList.$el.find('.column-mtime .columntitle').click(); @@ -2148,6 +2161,9 @@ describe('OCA.Files.FileList tests', function() { expect( fileList.$el.find('.column-mtime .sort-indicator').hasClass(DESC_CLASS) ).toEqual(true); + expect(fakeServer.requests.length).toEqual(4); + request = fakeServer.requests[3]; + expect(request.url).toEqual(sortingUrl); }); it('Uses correct sort comparator when inserting files', function() { testFiles.sort(OCA.Files.FileList.Comparators.size);