fix default value, update js/php tests

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();
 	}
 
 }

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');

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
+				});
+			}
 		},
 
 		/**

apps/files/tests/controller/ViewControllerTest.php

@@ -1,5 +1,6 @@
 <?php
 /**
+ * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  * @author Joas Schilling <nickvergessen@owncloud.com>
  * @author Lukas Reschke <lukas@owncloud.com>
  * @author Vincent Petry <pvince81@owncloud.com>
@@ -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',

apps/files/tests/controller/apicontrollertest.php

@@ -1,5 +1,6 @@
 <?php
 /**
+ * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  * @author Lukas Reschke <lukas@owncloud.com>
  * @author Morris Jobke <hey@morrisjobke.de>
  * @author Roeland Jago Douma <rullzer@owncloud.com>
@@ -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);
+	}
+
 }

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);