add serverside possibility to order by oldest first

8 files changed, 160 insertions, 70 deletions

diff --git a/businesslayer/itembusinesslayer.php b/businesslayer/itembusinesslayer.php
index 23f6bcd2d..82d445161 100644
--- a/businesslayer/itembusinesslayer.php
+++ b/businesslayer/itembusinesslayer.php
@@ -56,19 +56,18 @@ class ItemBusinessLayer extends BusinessLayer {
 
 		switch($type){
 			case FeedType::FEED:
-				$items = $this->itemMapper->findAllNewFeed($id, $updatedSince,
-					                                   $status, $userId);
-				break;
+				return $this->itemMapper->findAllNewFeed(
+					$id, $updatedSince, $status, $userId
+				);
 			case FeedType::FOLDER:
-				$items = $this->itemMapper->findAllNewFolder($id, $updatedSince,
-					                                   $status, $userId);
-				break;
+				return $this->itemMapper->findAllNewFolder(
+					$id, $updatedSince, $status, $userId
+				);
 			default:
-				$items = $this->itemMapper->findAllNew($updatedSince, $status,
-					                               $userId);
+				return $this->itemMapper->findAllNew(
+					$updatedSince, $status, $userId
+				);
 		}
-
-		return $items;
 	}
 
 
@@ -80,26 +79,27 @@ class ItemBusinessLayer extends BusinessLayer {
 	 * @param int $offset only items lower than this id are returned, 0 for no offset
 	 * @param boolean $showAll if unread items should also be returned
 	 * @param string $userId the name of the user
+	 * @param boolean $oldestFirst if it should be ordered by oldest first
 	 * @return array of items
 	 */
-	public function findAll($id, $type, $limit, $offset, $showAll, $userId){
+	public function findAll($id, $type, $limit, $offset, $showAll, $userId, 
+	                        $oldestFirst=false){
 		$status = $this->statusFlag->typeToStatus($type, $showAll);
 
 		switch($type){
 			case FeedType::FEED:
-				$items = $this->itemMapper->findAllFeed($id, $limit, $offset,
-					                                   $status, $userId);
-				break;
+				return $this->itemMapper->findAllFeed(
+					$id, $limit, $offset,$status, $userId, $oldestFirst
+				);
 			case FeedType::FOLDER:
-				$items = $this->itemMapper->findAllFolder($id, $limit, $offset,
-					                                   $status, $userId);
-				break;
+				return $this->itemMapper->findAllFolder(
+					$id, $limit, $offset, $status, $userId, $oldestFirst
+				);
 			default:
-				$items = $this->itemMapper->findAll($limit, $offset, $status,
-					                               $userId);
+				return $this->itemMapper->findAll(
+					$limit, $offset, $status, $userId, $oldestFirst
+				);
 		}
-
-		return $items;
 	}
 
 
diff --git a/controller/itemcontroller.php b/controller/itemcontroller.php
index dfc059505..0f590950e 100644
--- a/controller/itemcontroller.php
+++ b/controller/itemcontroller.php
@@ -57,6 +57,8 @@ class ItemController extends Controller {
 	public function index($type, $id, $limit, $offset=0) {
 		$showAll = $this->settings->getUserValue($this->userId, $this->appName,
 			'showAll') === '1';
+		$oldestFirst = $this->settings->getUserValue($this->userId, $this->appName,
+			'oldestFirst') === '1';
 
 		$this->settings->setUserValue($this->userId, $this->appName,
 			'lastViewedFeedId', $id);
@@ -77,8 +79,9 @@ class ItemController extends Controller {
 				$params['starred'] = $this->itemBusinessLayer->starredCount($this->userId);
 			}
 						
-			$params['items'] = $this->itemBusinessLayer->findAll($id, $type, $limit, 
-				                                       $offset, $showAll, $this->userId);
+			$params['items'] = $this->itemBusinessLayer->findAll(
+				$id, $type, $limit, $offset, $showAll, $this->userId, $oldestFirst
+			);
 			
 		// this gets thrown if there are no items
 		// in that case just return an empty array
diff --git a/controller/pagecontroller.php b/controller/pagecontroller.php
index 88154803f..db0fa48bf 100644
--- a/controller/pagecontroller.php
+++ b/controller/pagecontroller.php
@@ -51,17 +51,17 @@ class PageController extends Controller {
 	 * @NoAdminRequired
 	 */
 	public function settings() {
-		$showAll = $this->settings->getUserValue($this->userId, $this->appName, 
-			'showAll');
-		$compact = $this->settings->getUserValue($this->userId, $this->appName, 
-			'compact');
-		$language = $this->l10n->getLanguageCode();
-
-		return [
-			'showAll' => $showAll === '1',
-			'compact' => $compact === '1',
-			'language' => $language
-		];
+		$settings = ['showAll', 'compact', 'readOnScroll', 'oldestFirst'];
+
+		$result = ['language' => $this->l10n->getLanguageCode()];
+
+		foreach ($settings as $setting) {
+			$result[$setting] = $this->settings->getUserValue(
+				$this->userId, $this->appName, $setting
+			) === '1';
+		}
+
+		return $result;
 	}
 
 
@@ -70,17 +70,19 @@ class PageController extends Controller {
 	 *
 	 * @param bool $showAll
 	 * @param bool $compact
+	 * @param bool $readOnScroll
+	 * @param bool $oldestFirst
 	 */
-	public function updateSettings($showAll, $compact) {
-		if($showAll !== null) {
-			$this->settings->setUserValue($this->userId, $this->appName, 
-				'showAll', $showAll);
-		}
-
-		if($compact !== null) {
-			$this->settings->setUserValue($this->userId, $this->appName,
-				'compact', $compact);
+	public function updateSettings($showAll, $compact, $readOnScroll, $oldestFirst) {
+		$settings = ['showAll', 'compact', 'readOnScroll', 'oldestFirst'];
+		
+		foreach ($settings as $setting) {
+			if(${$setting} !== null) {
+				$this->settings->setUserValue($this->userId, $this->appName, 
+				                              $setting, ${$setting});
+			}
 		}
 	}
 
+
 }
\ No newline at end of file
diff --git a/db/itemmapper.php b/db/itemmapper.php
index 201022735..ff17596c2 100644
--- a/db/itemmapper.php
+++ b/db/itemmapper.php
@@ -24,7 +24,13 @@ class ItemMapper extends Mapper implements IMapper {
 	}
 
 
-	private function makeSelectQuery($prependTo){
+	private function makeSelectQuery($prependTo, $oldestFirst=false){
+		if($oldestFirst) {
+			$ordering = 'ASC';
+		} else {
+			$ordering = 'DESC';
+		}
+
 		return 'SELECT `items`.* FROM `*PREFIX*news_items` `items` '.
 			'JOIN `*PREFIX*news_feeds` `feeds` ' .
 				'ON `feeds`.`id` = `items`.`feed_id` '.
@@ -35,10 +41,10 @@ class ItemMapper extends Mapper implements IMapper {
 				'ON `folders`.`id` = `feeds`.`folder_id` ' .
 			'WHERE `feeds`.`folder_id` = 0 ' .
 				'OR `folders`.`deleted_at` = 0 ' .
-			'ORDER BY `items`.`id` DESC';
+			'ORDER BY `items`.`id` ' . $ordering;
 	}
 
-	private function makeSelectQueryStatus($prependTo, $status) {
+	private function makeSelectQueryStatus($prependTo, $status, $oldestFirst=false) {
 		// Hi this is Ray and you're watching Jack Ass
 		// Now look closely: this is how we adults handle weird bugs in our
 		// code: we take them variables and we cast the shit out of them
@@ -57,7 +63,7 @@ class ItemMapper extends Mapper implements IMapper {
 			// SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT.
 			// think twice when changing this
 			'AND ((`items`.`status` & ' . $status . ') = ' . $status . ') ' .
-			$prependTo
+			$prependTo, $oldestFirst
 		);
 	}
 
@@ -161,38 +167,38 @@ class ItemMapper extends Mapper implements IMapper {
 	}
 
 
-	public function findAllFeed($id, $limit, $offset, $status, $userId){
+	public function findAllFeed($id, $limit, $offset, $status, $userId, $oldestFirst=false){
 		$params = [$userId, $id];
 		$sql = 'AND `items`.`feed_id` = ? ';
 		if($offset !== 0){
 			$sql .= 'AND `items`.`id` < ? ';
 			$params[] = $offset;
 		}
-		$sql = $this->makeSelectQueryStatus($sql, $status);
+		$sql = $this->makeSelectQueryStatus($sql, $status, $oldestFirst);
 		return $this->findEntities($sql, $params, $limit);
 	}
 
 
-	public function findAllFolder($id, $limit, $offset, $status, $userId){
+	public function findAllFolder($id, $limit, $offset, $status, $userId, $oldestFirst=false){
 		$params = [$userId, $id];
 		$sql = 'AND `feeds`.`folder_id` = ? ';
 		if($offset !== 0){
 			$sql .= 'AND `items`.`id` < ? ';
 			$params[] = $offset;
 		}
-		$sql = $this->makeSelectQueryStatus($sql, $status);
+		$sql = $this->makeSelectQueryStatus($sql, $status, $oldestFirst);
 		return $this->findEntities($sql, $params, $limit);
 	}
 
 
-	public function findAll($limit, $offset, $status, $userId){
+	public function findAll($limit, $offset, $status, $userId, $oldestFirst=false){
 		$params = [$userId];
 		$sql = '';
 		if($offset !== 0){
 			$sql .= 'AND `items`.`id` < ? ';
 			$params[] = $offset;
 		}
-		$sql = $this->makeSelectQueryStatus($sql, $status);
+		$sql = $this->makeSelectQueryStatus($sql, $status, $oldestFirst);
 		return $this->findEntities($sql, $params, $limit);
 	}
 
diff --git a/tests/unit/businesslayer/ItemBusinessLayerTest.php b/tests/unit/businesslayer/ItemBusinessLayerTest.php
index 1b31963c1..48c077460 100644
--- a/tests/unit/businesslayer/ItemBusinessLayerTest.php
+++ b/tests/unit/businesslayer/ItemBusinessLayerTest.php
@@ -127,7 +127,8 @@ class ItemBusinessLayerTest extends \PHPUnit_Framework_TestCase {
 					$this->equalTo($this->limit),
 					$this->equalTo($this->offset),
 					$this->equalTo($this->status),
-					$this->equalTo($this->user))
+					$this->equalTo($this->user),
+					$this->equalTo(false))
 			->will($this->returnValue($this->response));
 
 		$result = $this->itemBusinessLayer->findAll(
@@ -164,13 +165,14 @@ class ItemBusinessLayerTest extends \PHPUnit_Framework_TestCase {
 			->with(	$this->equalTo($this->limit),
 					$this->equalTo($this->offset),
 					$this->equalTo($this->status),
-					$this->equalTo($this->user))
+					$this->equalTo($this->user),
+					$this->equalTo(true))
 			->will($this->returnValue($this->response));
 
 		$result = $this->itemBusinessLayer->findAll(
 			$this->id, $type, $this->limit,
 			$this->offset, $this->showAll,
-			$this->user);
+			$this->user, true);
 		$this->assertEquals($this->response, $result);
 	}
 
diff --git a/tests/unit/controller/ItemControllerTest.php b/tests/unit/controller/ItemControllerTest.php
index b4f87a0e9..1ebcdce55 100644
--- a/tests/unit/controller/ItemControllerTest.php
+++ b/tests/unit/controller/ItemControllerTest.php
@@ -188,20 +188,26 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase {
 	}
 
 
-	private function itemsApiExpects($id, $type){
-		$this->settings->expects($this->once())
+	private function itemsApiExpects($id, $type, $oldestFirst='1'){
+		$this->settings->expects($this->at(0))
 			->method('getUserValue')
 			->with($this->equalTo($this->user),
 				$this->equalTo($this->appName),
 				$this->equalTo('showAll'))
 			->will($this->returnValue('1'));
 		$this->settings->expects($this->at(1))
+			->method('getUserValue')
+			->with($this->equalTo($this->user),
+				$this->equalTo($this->appName),
+				$this->equalTo('oldestFirst'))
+			->will($this->returnValue($oldestFirst));
+		$this->settings->expects($this->at(2))
 			->method('setUserValue')
 			->with($this->equalTo($this->user),
 				$this->equalTo($this->appName),
 				$this->equalTo('lastViewedFeedId'),
 				$this->equalTo($id));
-		$this->settings->expects($this->at(2))
+		$this->settings->expects($this->at(3))
 			->method('setUserValue')
 			->with($this->equalTo($this->user),
 				$this->equalTo($this->appName),
@@ -219,7 +225,7 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase {
 			'starred' => 3111
 		];
 
-		$this->itemsApiExpects(2, FeedType::FEED);
+		$this->itemsApiExpects(2, FeedType::FEED, '0');
 
 		$this->feedBusinessLayer->expects($this->once())
 			->method('findAll')
@@ -244,7 +250,8 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase {
 				$this->equalTo(3), 
 				$this->equalTo(0),
 				$this->equalTo(true), 
-				$this->equalTo($this->user))
+				$this->equalTo($this->user),
+				$this->equalTo(false))
 			->will($this->returnValue($result['items']));
 
 		$response = $this->controller->index(FeedType::FEED, 2, 3);
@@ -264,13 +271,14 @@ class ItemControllerTest extends \PHPUnit_Framework_TestCase {
 				$this->equalTo(3), 
 				$this->equalTo(10),
 				$this->equalTo(true), 
-				$this->equalTo($this->user))
+				$this->equalTo($this->user),
+				$this->equalTo(true))
 			->will($this->returnValue($result['items']));
 
 		$this->feedBusinessLayer->expects($this->never())
 			->method('findAll');
 
-		$response = $this->controller->index(FeedType::FEED, 2, 3, 10);
+		$response = $this->controller->index(FeedType::FEED, 2, 3, 10, true);
 		$this->assertEquals($result, $response);
 	}
 
diff --git a/tests/unit/controller/PageControllerTest.php b/tests/unit/controller/PageControllerTest.php
index d4c53e468..1e898c146 100644
--- a/tests/unit/controller/PageControllerTest.php
+++ b/tests/unit/controller/PageControllerTest.php
@@ -60,7 +60,9 @@ class PageControllerTest extends \PHPUnit_Framework_TestCase {
 		$result = [
 			'showAll' => true,
 			'compact' => true,
-			'language' => 'de'
+			'readOnScroll' => true,
+			'oldestFirst' => true,
+			'language' => 'de',
 		];
 
 		$this->l10n->expects($this->once())
@@ -78,6 +80,18 @@ class PageControllerTest extends \PHPUnit_Framework_TestCase {
 				$this->equalTo($this->appName),
 				$this->equalTo('compact'))
 			->will($this->returnValue('1'));
+		$this->settings->expects($this->at(2))
+			->method('getUserValue')
+			->with($this->equalTo($this->user),
+				$this->equalTo($this->appName),
+				$this->equalTo('readOnScroll'))
+			->will($this->returnValue('1'));
+		$this->settings->expects($this->at(3))
+			->method('getUserValue')
+			->with($this->equalTo($this->user),
+				$this->equalTo($this->appName),
+				$this->equalTo('oldestFirst'))
+			->will($this->returnValue('1'));
 
 		$response = $this->controller->settings();
 		$this->assertEquals($result, $response);
@@ -97,7 +111,19 @@ class PageControllerTest extends \PHPUnit_Framework_TestCase {
 				$this->equalTo($this->appName),
 				$this->equalTo('compact'), 
 				$this->equalTo(true));
-		$this->controller->updateSettings(true, true);
+		$this->settings->expects($this->at(2))
+			->method('setUserValue')
+			->with($this->equalTo($this->user),
+				$this->equalTo($this->appName),
+				$this->equalTo('readOnScroll'), 
+				$this->equalTo(true));
+		$this->settings->expects($this->at(3))
+			->method('setUserValue')
+			->with($this->equalTo($this->user),
+				$this->equalTo($this->appName),
+				$this->equalTo('oldestFirst'), 
+				$this->equalTo(true));
+		$this->controller->updateSettings(true, true, true, true);
 
 	}
 
@@ -113,7 +139,7 @@ class PageControllerTest extends \PHPUnit_Framework_TestCase {
 				$this->equalTo('showAll'), 
 				$this->equalTo(true));
 
-		$this->controller->updateSettings(true, null);
+		$this->controller->updateSettings(true, null, null, null);
 
 	}
 }
\ No newline at end of file
diff --git a/tests/unit/db/ItemMapperTest.php b/tests/unit/db/ItemMapperTest.php
index 15379f485..7a855b7bb 100644
--- a/tests/unit/db/ItemMapperTest.php
+++ b/tests/unit/db/ItemMapperTest.php
@@ -63,7 +63,12 @@ class ItemMapperTest extends \OCP\AppFramework\Db\MapperTestUtility {
 	}
 
 
-	private function makeSelectQuery($prependTo){
+	private function makeSelectQuery($prependTo, $oldestFirst=false){
+		if($oldestFirst) {
+			$ordering = 'ASC';
+		} else {
+			$ordering = 'DESC';
+		}
 		return 'SELECT `items`.* FROM `*PREFIX*news_items` `items` '.
 			'JOIN `*PREFIX*news_feeds` `feeds` ' .
 				'ON `feeds`.`id` = `items`.`feed_id` '.
@@ -74,15 +79,15 @@ class ItemMapperTest extends \OCP\AppFramework\Db\MapperTestUtility {
 				'ON `folders`.`id` = `feeds`.`folder_id` ' .
 			'WHERE `feeds`.`folder_id` = 0 ' .
 				'OR `folders`.`deleted_at` = 0 ' .
-			'ORDER BY `items`.`id` DESC';
+			'ORDER BY `items`.`id` ' . $ordering;
 	}
 
-	private function makeSelectQueryStatus($prependTo, $status) {
+	private function makeSelectQueryStatus($prependTo, $status, $oldestFirst=false) {
 		$status = (int) $status;
 
 		return $this->makeSelectQuery(
 			'AND ((`items`.`status` & ' . $status . ') = ' . $status . ') ' .
-			$prependTo
+			$prependTo, $oldestFirst
 		);
 	}
 
@@ -227,6 +232,19 @@ class ItemMapperTest extends \OCP\AppFramework\Db\MapperTestUtility {
 	}
 
 
+	public function testFindAllFeedOldestFirst(){
+		$sql = 'AND `items`.`feed_id` = ? ' .
+			'AND `items`.`id` < ? ';
+		$sql = $this->makeSelectQueryStatus($sql, $this->status, true);
+		$params = [$this->user, $this->id, $this->offset];
+		$this->setMapperResult($sql, $params, $this->rows);
+		$result = $this->mapper->findAllFeed($this->id, $this->limit,
+				$this->offset, $this->status, $this->user, true);
+
+		$this->assertEquals($this->items, $result);
+	}
+
+
 	public function testFindAllFeedOffsetZero(){
 		$sql = 'AND `items`.`feed_id` = ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
@@ -252,6 +270,19 @@ class ItemMapperTest extends \OCP\AppFramework\Db\MapperTestUtility {
 	}
 
 
+	public function testFindAllFolderOldestFirst(){
+		$sql = 'AND `feeds`.`folder_id` = ? ' .
+			'AND `items`.`id` < ? ';
+		$sql = $this->makeSelectQueryStatus($sql, $this->status, true);
+		$params = [$this->user, $this->id, $this->offset];
+		$this->setMapperResult($sql, $params, $this->rows);
+		$result = $this->mapper->findAllFolder($this->id, $this->limit,
+				$this->offset, $this->status, $this->user, true);
+
+		$this->assertEquals($this->items, $result);
+	}
+
+
 	public function testFindAllFolderOffsetZero(){
 		$sql = 'AND `feeds`.`folder_id` = ? ';
 		$sql = $this->makeSelectQueryStatus($sql, $this->status);
@@ -276,6 +307,18 @@ class ItemMapperTest extends \OCP\AppFramework\Db\MapperTestUtility {
 	}
 
 
+	public function testFindAllOldestFirst(){
+		$sql = 'AND `items`.`id` < ? ';
+		$sql = $this->makeSelectQueryStatus($sql, $this->status, true);
+		$params = [$this->user, $this->offset];
+		$this->setMapperResult($sql, $params, $this->rows);
+		$result = $this->mapper->findAll($this->limit,
+				$this->offset, $this->status, $this->user, true);
+
+		$this->assertEquals($this->items, $result);
+	}
+
+
 	public function testFindAllOffsetZero(){
 		$sql = $this->makeSelectQueryStatus('', $this->status);
 		$params = [$this->user];