From b8fd3caf8306e8616fcb2f2c0add95b74c2ec024 Mon Sep 17 00:00:00 2001 From: Alexis Degrugillier Date: Sat, 14 Feb 2015 11:00:26 -0500 Subject: Harmonize share configuration view. Before, for shares that don't need options, only a button to remove it was visible. It was source of confusion for users. I changed the look of those shares by using the same layout as others (minus the help). As there is no configuration possible for the url, the field is disabled but it is possible to change the name of the share. See #787 --- app/views/configure/sharing.phtml | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'app/views') diff --git a/app/views/configure/sharing.phtml b/app/views/configure/sharing.phtml index da7557480..deb1ed6b7 100644 --- a/app/views/configure/sharing.phtml +++ b/app/views/configure/sharing.phtml @@ -4,7 +4,8 @@
+ data-simple='
+
' data-advanced='
@@ -26,16 +27,17 @@
+
+ formType() === 'advanced') { ?> -
- - - -
- - + - + + + +
+ formType() === 'advanced') { ?> +
-- cgit v1.2.3 From 5b90e1f4a0057aa78fd7d8d4d748b01676ec9073 Mon Sep 17 00:00:00 2001 From: Alexis Degrugillier Date: Sun, 1 Mar 2015 09:18:06 -0500 Subject: Introduce user queries objects There is now an object to manipulate user queries. It allows to move logic to handle those from the view and the controller in the model. Thus making the view and the controller easier to read. I introduced a new interface to start using dependency injection. There is still some rough edges but we are moving in the right direction. The new object is fully tested but it still need some improvements, for instance, it is still tied to the search object. There might be a better way to do that. --- app/Controllers/configureController.php | 83 +++--------- app/Exceptions/DAOException.php | 5 + app/Models/CategoryDAO.php | 2 +- app/Models/ConfigurationSetter.php | 7 +- app/Models/EntryDAO.php | 2 +- app/Models/FeedDAO.php | 2 +- app/Models/Searchable.php | 6 + app/Models/UserQuery.php | 226 +++++++++++++++++++++++++++++++ app/views/configure/queries.phtml | 47 +++---- tests/app/Models/UserQueryTest.php | 229 ++++++++++++++++++++++++++++++++ 10 files changed, 505 insertions(+), 104 deletions(-) create mode 100644 app/Exceptions/DAOException.php create mode 100644 app/Models/Searchable.php create mode 100644 app/Models/UserQuery.php create mode 100644 tests/app/Models/UserQueryTest.php (limited to 'app/views') diff --git a/app/Controllers/configureController.php b/app/Controllers/configureController.php index 38ccd2b2d..fc92aa0c2 100755 --- a/app/Controllers/configureController.php +++ b/app/Controllers/configureController.php @@ -241,13 +241,16 @@ class FreshRSS_configure_Controller extends Minz_ActionController { * checking if categories and feeds are still in use. */ public function queriesAction() { + $category_dao = new FreshRSS_CategoryDAO(); + $feed_dao = FreshRSS_Factory::createFeedDao(); if (Minz_Request::isPost()) { - $queries = Minz_Request::param('queries', array()); + $params = Minz_Request::param('queries', array()); - foreach ($queries as $key => $query) { + foreach ($params as $key => $query) { if (!$query['name']) { $query['name'] = _t('conf.query.number', $key + 1); } + $queries[] = new FreshRSS_UserQuery($query, $feed_dao, $category_dao); } FreshRSS_Context::$user_conf->queries = $queries; FreshRSS_Context::$user_conf->save(); @@ -255,62 +258,9 @@ class FreshRSS_configure_Controller extends Minz_ActionController { Minz_Request::good(_t('feedback.conf.updated'), array('c' => 'configure', 'a' => 'queries')); } else { - $this->view->query_get = array(); - $cat_dao = new FreshRSS_CategoryDAO(); - $feed_dao = FreshRSS_Factory::createFeedDao(); + $this->view->queries = array(); foreach (FreshRSS_Context::$user_conf->queries as $key => $query) { - if (!isset($query['get'])) { - continue; - } - - switch ($query['get'][0]) { - case 'c': - $category = $cat_dao->searchById(substr($query['get'], 2)); - - $deprecated = true; - $cat_name = ''; - if ($category) { - $cat_name = $category->name(); - $deprecated = false; - } - - $this->view->query_get[$key] = array( - 'type' => 'category', - 'name' => $cat_name, - 'deprecated' => $deprecated, - ); - break; - case 'f': - $feed = $feed_dao->searchById(substr($query['get'], 2)); - - $deprecated = true; - $feed_name = ''; - if ($feed) { - $feed_name = $feed->name(); - $deprecated = false; - } - - $this->view->query_get[$key] = array( - 'type' => 'feed', - 'name' => $feed_name, - 'deprecated' => $deprecated, - ); - break; - case 's': - $this->view->query_get[$key] = array( - 'type' => 'favorite', - 'name' => 'favorite', - 'deprecated' => false, - ); - break; - case 'a': - $this->view->query_get[$key] = array( - 'type' => 'all', - 'name' => 'all', - 'deprecated' => false, - ); - break; - } + $this->view->queries[$key] = new FreshRSS_UserQuery($query, $feed_dao, $category_dao); } } @@ -325,16 +275,17 @@ class FreshRSS_configure_Controller extends Minz_ActionController { * lean data. */ public function addQueryAction() { - $whitelist = array('get', 'order', 'name', 'search', 'state'); - $queries = FreshRSS_Context::$user_conf->queries; - $query = Minz_Request::params(); - $query['name'] = _t('conf.query.number', count($queries) + 1); - foreach ($query as $key => $value) { - if (!in_array($key, $whitelist)) { - unset($query[$key]); - } + $category_dao = new FreshRSS_CategoryDAO(); + $feed_dao = FreshRSS_Factory::createFeedDao(); + $queries = array(); + foreach (FreshRSS_Context::$user_conf->queries as $key => $query) { + $queries[$key] = new FreshRSS_UserQuery($query, $feed_dao, $category_dao); } - $queries[] = $query; + $params = Minz_Request::params(); + $params['url'] = Minz_Url::display(array('params' => $params)); + $params['name'] = _t('conf.query.number', count($queries) + 1); + $queries[] = new FreshRSS_UserQuery($params, $feed_dao, $category_dao); + FreshRSS_Context::$user_conf->queries = $queries; FreshRSS_Context::$user_conf->save(); diff --git a/app/Exceptions/DAOException.php b/app/Exceptions/DAOException.php new file mode 100644 index 000000000..6bd8f4ff0 --- /dev/null +++ b/app/Exceptions/DAOException.php @@ -0,0 +1,5 @@ +prefix . 'category`(name) VALUES(?)'; $stm = $this->bd->prepare($sql); diff --git a/app/Models/ConfigurationSetter.php b/app/Models/ConfigurationSetter.php index eeb1f2f4c..d7689752f 100644 --- a/app/Models/ConfigurationSetter.php +++ b/app/Models/ConfigurationSetter.php @@ -117,12 +117,7 @@ class FreshRSS_ConfigurationSetter { private function _queries(&$data, $values) { $data['queries'] = array(); foreach ($values as $value) { - $value = array_filter($value); - $params = $value; - unset($params['name']); - unset($params['url']); - $value['url'] = Minz_Url::display(array('params' => $params)); - $data['queries'][] = $value; + $data['queries'][] = $value->toArray(); } } diff --git a/app/Models/EntryDAO.php b/app/Models/EntryDAO.php index cf75a02c9..b8a1a43b0 100644 --- a/app/Models/EntryDAO.php +++ b/app/Models/EntryDAO.php @@ -1,6 +1,6 @@ prefix . 'feed` (url, category, name, website, description, lastUpdate, priority, httpAuth, error, keep_history, ttl) VALUES(?, ?, ?, ?, ?, ?, 10, ?, 0, -2, -2)'; $stm = $this->bd->prepare($sql); diff --git a/app/Models/Searchable.php b/app/Models/Searchable.php new file mode 100644 index 000000000..d5bcea49d --- /dev/null +++ b/app/Models/Searchable.php @@ -0,0 +1,6 @@ +category_dao = $category_dao; + $this->feed_dao = $feed_dao; + if (isset($query['get'])) { + $this->parseGet($query['get']); + } + if (isset($query['name'])) { + $this->name = $query['name']; + } + if (isset($query['order'])) { + $this->order = $query['order']; + } + if (!isset($query['search'])) { + $query['search'] = ''; + } + // linked to deeply with the search object, need to use dependency injection + $this->search = new FreshRSS_Search($query['search']); + if (isset($query['state'])) { + $this->state = $query['state']; + } + if (isset($query['url'])) { + $this->url = $query['url']; + } + } + + /** + * Convert the current object to an array. + * + * @return array + */ + public function toArray() { + return array_filter(array( + 'get' => $this->get, + 'name' => $this->name, + 'order' => $this->order, + 'search' => $this->search->__toString(), + 'state' => $this->state, + 'url' => $this->url, + )); + } + + /** + * Parse the get parameter in the query string to extract its name and + * type + * + * @param string $get + */ + private function parseGet($get) { + $this->get = $get; + if (preg_match('/(?P[acfs])(_(?P\d+))?/', $get, $matches)) { + switch ($matches['type']) { + case 'a': + $this->parseAll(); + break; + case 'c': + $this->parseCategory($matches['id']); + break; + case 'f': + $this->parseFeed($matches['id']); + break; + case 's': + $this->parseFavorite(); + break; + } + } + } + + /** + * Parse the query string when it is an "all" query + */ + private function parseAll() { + $this->get_name = 'all'; + $this->get_type = 'all'; + } + + /** + * Parse the query string when it is a "category" query + * + * @param integer $id + * @throws FreshRSS_DAOException + */ + private function parseCategory($id) { + if (is_null($this->category_dao)) { + throw new FreshRSS_DAOException('Category DAO is not loaded i UserQuery'); + } + $category = $this->category_dao->searchById($id); + if ($category) { + $this->get_name = $category->name(); + } else { + $this->deprecated = true; + } + $this->get_type = 'category'; + } + + /** + * Parse the query string when it is a "feed" query + * + * @param integer $id + * @throws FreshRSS_DAOException + */ + private function parseFeed($id) { + if (is_null($this->feed_dao)) { + throw new FreshRSS_DAOException('Feed DAO is not loaded i UserQuery'); + } + $feed = $this->feed_dao->searchById($id); + if ($feed) { + $this->get_name = $feed->name(); + } else { + $this->deprecated = true; + } + $this->get_type = 'feed'; + } + + /** + * Parse the query string when it is a "favorite" query + */ + private function parseFavorite() { + $this->get_name = 'favorite'; + $this->get_type = 'favorite'; + } + + /** + * Check if the current user query is deprecated. + * It is deprecated if the category or the feed used in the query are + * not existing. + * + * @return boolean + */ + public function isDeprecated() { + return $this->deprecated; + } + + /** + * Check if the user query has parameters. + * If the type is 'all', it is considered equal to no parameters + * + * @return boolean + */ + public function hasParameters() { + if ($this->get_type === 'all') { + return false; + } + if ($this->hasSearch()) { + return true; + } + if ($this->state) { + return true; + } + if ($this->order) { + return true; + } + if ($this->get) { + return true; + } + return false; + } + + /** + * Check if there is a search in the search object + * + * @return boolean + */ + public function hasSearch() { + return $this->search->getRawInput() != ""; + } + + public function getGet() { + return $this->get; + } + + public function getGetName() { + return $this->get_name; + } + + public function getGetType() { + return $this->get_type; + } + + public function getName() { + return $this->name; + } + + public function getOrder() { + return $this->order; + } + + public function getSearch() { + return $this->search; + } + + public function getState() { + return $this->state; + } + + public function getUrl() { + return $this->url; + } + +} diff --git a/app/views/configure/queries.phtml b/app/views/configure/queries.phtml index 5f449deb3..69efcf365 100644 --- a/app/views/configure/queries.phtml +++ b/app/views/configure/queries.phtml @@ -6,27 +6,28 @@ - queries as $key => $query) { ?> + queries as $key => $query) { ?>
- "/> - "/> - "/> - "/> + + + + +
- + @@ -35,23 +36,11 @@
- query_get[$key]) && - $this->query_get[$key]['deprecated']); - ?> - - + hasParameters()) { ?>
- + isDeprecated()) { ?>
@@ -60,20 +49,20 @@
    - -
    • + hasSearch()) { ?> +
  • getSearch()->getRawInput()); ?>
    • - -
    • + getState()) { ?> +
  • getState()); ?>
    • - -
    • + getOrder()) { ?> +
  • getOrder())); ?>
    • - -
  • query_get[$key]['type'], $this->query_get[$key]['name']); ?>
    • + getGet()) { ?> +
  • getGetType(), $query->getGetName()); ?>
diff --git a/tests/app/Models/UserQueryTest.php b/tests/app/Models/UserQueryTest.php new file mode 100644 index 000000000..2234be6e1 --- /dev/null +++ b/tests/app/Models/UserQueryTest.php @@ -0,0 +1,229 @@ + 'a'); + $user_query = new FreshRSS_UserQuery($query); + $this->assertEquals('all', $user_query->getGetName()); + $this->assertEquals('all', $user_query->getGetType()); + } + + public function test__construct_whenFavoriteQuery_storesFavoriteParameters() { + $query = array('get' => 's'); + $user_query = new FreshRSS_UserQuery($query); + $this->assertEquals('favorite', $user_query->getGetName()); + $this->assertEquals('favorite', $user_query->getGetType()); + } + + /** + * @expectedException Exceptions/FreshRSS_DAOException + * @expectedExceptionMessage Category DAO is not loaded in UserQuery + */ + public function test__construct_whenCategoryQueryAndNoDao_throwsException() { + $this->markTestIncomplete('There is a problem with the exception autoloading. We need to make a better autoloading process'); + $query = array('get' => 'c_1'); + new FreshRSS_UserQuery($query); + } + + public function test__construct_whenCategoryQuery_storesCategoryParameters() { + $category_name = 'some category name'; + $cat = $this->getMock('FreshRSS_Category'); + $cat->expects($this->atLeastOnce()) + ->method('name') + ->withAnyParameters() + ->willReturn($category_name); + $cat_dao = $this->getMock('FreshRSS_Searchable'); + $cat_dao->expects($this->atLeastOnce()) + ->method('searchById') + ->withAnyParameters() + ->willReturn($cat); + $query = array('get' => 'c_1'); + $user_query = new FreshRSS_UserQuery($query, null, $cat_dao); + $this->assertEquals($category_name, $user_query->getGetName()); + $this->assertEquals('category', $user_query->getGetType()); + } + + /** + * @expectedException Exceptions/FreshRSS_DAOException + * @expectedExceptionMessage Feed DAO is not loaded in UserQuery + */ + public function test__construct_whenFeedQueryAndNoDao_throwsException() { + $this->markTestIncomplete('There is a problem with the exception autoloading. We need to make a better autoloading process'); + $query = array('get' => 'c_1'); + new FreshRSS_UserQuery($query); + } + + public function test__construct_whenFeedQuery_storesFeedParameters() { + $feed_name = 'some feed name'; + $feed = $this->getMock('FreshRSS_Feed', array(), array('', false)); + $feed->expects($this->atLeastOnce()) + ->method('name') + ->withAnyParameters() + ->willReturn($feed_name); + $feed_dao = $this->getMock('FreshRSS_Searchable'); + $feed_dao->expects($this->atLeastOnce()) + ->method('searchById') + ->withAnyParameters() + ->willReturn($feed); + $query = array('get' => 'f_1'); + $user_query = new FreshRSS_UserQuery($query, $feed_dao, null); + $this->assertEquals($feed_name, $user_query->getGetName()); + $this->assertEquals('feed', $user_query->getGetType()); + } + + public function test__construct_whenUnknownQuery_doesStoreParameters() { + $query = array('get' => 'q'); + $user_query = new FreshRSS_UserQuery($query); + $this->assertNull($user_query->getGetName()); + $this->assertNull($user_query->getGetType()); + } + + public function test__construct_whenName_storesName() { + $name = 'some name'; + $query = array('name' => $name); + $user_query = new FreshRSS_UserQuery($query); + $this->assertEquals($name, $user_query->getName()); + } + + public function test__construct_whenOrder_storesOrder() { + $order = 'some order'; + $query = array('order' => $order); + $user_query = new FreshRSS_UserQuery($query); + $this->assertEquals($order, $user_query->getOrder()); + } + + public function test__construct_whenState_storesState() { + $state = 'some state'; + $query = array('state' => $state); + $user_query = new FreshRSS_UserQuery($query); + $this->assertEquals($state, $user_query->getState()); + } + + public function test__construct_whenUrl_storesUrl() { + $url = 'some url'; + $query = array('url' => $url); + $user_query = new FreshRSS_UserQuery($query); + $this->assertEquals($url, $user_query->getUrl()); + } + + public function testToArray_whenNoData_returnsEmptyArray() { + $user_query = new FreshRSS_UserQuery(array()); + $this->assertInternalType('array', $user_query->toArray()); + $this->assertCount(0, $user_query->toArray()); + } + + public function testToArray_whenData_returnsArray() { + $query = array( + 'get' => 's', + 'name' => 'some name', + 'order' => 'some order', + 'search' => 'some search', + 'state' => 'some state', + 'url' => 'some url', + ); + $user_query = new FreshRSS_UserQuery($query); + $this->assertInternalType('array', $user_query->toArray()); + $this->assertCount(6, $user_query->toArray()); + $this->assertEquals($query, $user_query->toArray()); + } + + public function testHasSearch_whenSearch_returnsTrue() { + $query = array( + 'search' => 'some search', + ); + $user_query = new FreshRSS_UserQuery($query); + $this->assertTrue($user_query->hasSearch()); + } + + public function testHasSearch_whenNoSearch_returnsFalse() { + $user_query = new FreshRSS_UserQuery(array()); + $this->assertFalse($user_query->hasSearch()); + } + + public function testHasParameters_whenAllQuery_returnsFalse() { + $query = array('get' => 'a'); + $user_query = new FreshRSS_UserQuery($query); + $this->assertFalse($user_query->hasParameters()); + } + + public function testHasParameters_whenNoParameter_returnsFalse() { + $query = array(); + $user_query = new FreshRSS_UserQuery($query); + $this->assertFalse($user_query->hasParameters()); + } + + public function testHasParameters_whenParameter_returnTrue() { + $query = array('get' => 's'); + $user_query = new FreshRSS_UserQuery($query); + $this->assertTrue($user_query->hasParameters()); + } + + public function testIsDeprecated_whenCategoryExists_returnFalse() { + $cat = $this->getMock('FreshRSS_Category'); + $cat_dao = $this->getMock('FreshRSS_Searchable'); + $cat_dao->expects($this->atLeastOnce()) + ->method('searchById') + ->withAnyParameters() + ->willReturn($cat); + $query = array('get' => 'c_1'); + $user_query = new FreshRSS_UserQuery($query, null, $cat_dao); + $this->assertFalse($user_query->isDeprecated()); + } + + public function testIsDeprecated_whenCategoryDoesNotExist_returnTrue() { + $cat_dao = $this->getMock('FreshRSS_Searchable'); + $cat_dao->expects($this->atLeastOnce()) + ->method('searchById') + ->withAnyParameters() + ->willReturn(null); + $query = array('get' => 'c_1'); + $user_query = new FreshRSS_UserQuery($query, null, $cat_dao); + $this->assertTrue($user_query->isDeprecated()); + } + + public function testIsDeprecated_whenFeedExists_returnFalse() { + $feed = $this->getMock('FreshRSS_Feed', array(), array('', false)); + $feed_dao = $this->getMock('FreshRSS_Searchable'); + $feed_dao->expects($this->atLeastOnce()) + ->method('searchById') + ->withAnyParameters() + ->willReturn($feed); + $query = array('get' => 'f_1'); + $user_query = new FreshRSS_UserQuery($query, $feed_dao, null); + $this->assertFalse($user_query->isDeprecated()); + } + + public function testIsDeprecated_whenFeedDoesNotExist_returnTrue() { + $feed_dao = $this->getMock('FreshRSS_Searchable'); + $feed_dao->expects($this->atLeastOnce()) + ->method('searchById') + ->withAnyParameters() + ->willReturn(null); + $query = array('get' => 'f_1'); + $user_query = new FreshRSS_UserQuery($query, $feed_dao, null); + $this->assertTrue($user_query->isDeprecated()); + } + + public function testIsDeprecated_whenAllQuery_returnFalse() { + $query = array('get' => 'a'); + $user_query = new FreshRSS_UserQuery($query); + $this->assertFalse($user_query->isDeprecated()); + } + + public function testIsDeprecated_whenFavoriteQuery_returnFalse() { + $query = array('get' => 's'); + $user_query = new FreshRSS_UserQuery($query); + $this->assertFalse($user_query->isDeprecated()); + } + + public function testIsDeprecated_whenUnknownQuery_returnFalse() { + $query = array('get' => 'q'); + $user_query = new FreshRSS_UserQuery($query); + $this->assertFalse($user_query->isDeprecated()); + } + +} -- cgit v1.2.3 From dd4fb519be801254a8aa9baedb2a7f87dd608f2b Mon Sep 17 00:00:00 2001 From: Alexis Degrugillier Date: Sat, 14 Mar 2015 09:43:08 -0400 Subject: Add an unsaved changes alert on config pages Before, you could leave a configuration page without knowing if you saved your changes or not. Now, there is an alert poping up if you have unsaved changes. It will ask you if you want to stay on the page and save your changes or leave the page and loose your changes. See #739 --- app/views/auth/index.phtml | 12 ++++++------ app/views/configure/archiving.phtml | 6 +++--- app/views/configure/display.phtml | 28 ++++++++++++++-------------- app/views/configure/queries.phtml | 1 + app/views/configure/reading.phtml | 34 +++++++++++++++++----------------- app/views/configure/sharing.phtml | 4 ++-- app/views/configure/shortcut.phtml | 28 ++++++++++++++-------------- p/scripts/main.js | 28 ++++++++++++++++++++++++++++ 8 files changed, 85 insertions(+), 56 deletions(-) (limited to 'app/views') diff --git a/app/views/auth/index.phtml b/app/views/auth/index.phtml index f7a862ac9..8e4df8c2c 100644 --- a/app/views/auth/index.phtml +++ b/app/views/auth/index.phtml @@ -9,7 +9,7 @@
- auth_type, array('form', 'persona', 'http_auth', 'none'))) { ?> @@ -25,7 +25,7 @@
@@ -35,7 +35,7 @@
@@ -45,7 +45,7 @@
@@ -58,7 +58,7 @@ token; ?>
/> + echo FreshRSS_Auth::accessNeedsAction() ? '' : ' disabled="disabled"'; ?> data-leave-validation=""/> array('output' => 'rss', 'token' => $token)), 'html', true); ?>
@@ -69,7 +69,7 @@
diff --git a/app/views/configure/archiving.phtml b/app/views/configure/archiving.phtml index 875463137..52ee98a48 100644 --- a/app/views/configure/archiving.phtml +++ b/app/views/configure/archiving.phtml @@ -10,14 +10,14 @@
- +  
- ttl_default; ?>"> '20min', 1500 => '25min', 1800 => '30min', 2700 => '45min', 3600 => '1h', 5400 => '1.5h', 7200 => '2h', 10800 => '3h', 14400 => '4h', 18800 => '5h', 21600 => '6h', 25200 => '7h', 28800 => '8h', diff --git a/app/views/configure/display.phtml b/app/views/configure/display.phtml index 02249bc55..91b0b8189 100644 --- a/app/views/configure/display.phtml +++ b/app/views/configure/display.phtml @@ -9,7 +9,7 @@
- @@ -24,7 +24,7 @@