From e19c49295a4bba82b2e1fa613ad03608c79d6443 Mon Sep 17 00:00:00 2001
From: Joas Schilling <nickvergessen@owncloud.com>
Date: Tue, 20 Oct 2015 12:02:08 +0200
Subject: [PATCH] Use speaking ids

---
 settings/controller/appsettingscontroller.php | 47 ++++++++++++++++---
 settings/js/apps.js                           |  6 +--
 settings/templates/apps.php                   |  2 +-
 .../controller/AppSettingsControllerTest.php  | 26 ++++++----
 4 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/settings/controller/appsettingscontroller.php b/settings/controller/appsettingscontroller.php
index 9022e48a34..bde00df062 100644
--- a/settings/controller/appsettingscontroller.php
+++ b/settings/controller/appsettingscontroller.php
@@ -42,6 +42,8 @@ use OCP\IConfig;
  * @package OC\Settings\Controller
  */
 class AppSettingsController extends Controller {
+	const CAT_ENABLED = 0;
+	const CAT_DISABLED = 1;
 
 	/** @var \OCP\IL10N */
 	private $l10n;
@@ -94,12 +96,39 @@ class AppSettingsController extends Controller {
 		return new DataResponse();
 	}
 
+	/**
+	 * @param string|int $category
+	 * @return int
+	 */
+	protected function getCategory($category) {
+		if (is_string($category)) {
+			foreach ($this->listCategories() as $cat) {
+				if (isset($cat['ident']) && $cat['ident'] === $category) {
+					$category = (int) $cat['id'];
+					break;
+				}
+			}
+
+			// Didn't find the category, falling back to enabled
+			if (is_string($category)) {
+				$category = self::CAT_ENABLED;
+			}
+		}
+		return (int) $category;
+	}
+
 	/**
 	 * @NoCSRFRequired
-	 * @param int $category
+	 * @param string $category
 	 * @return TemplateResponse
 	 */
-	public function viewApps($category = 0) {
+	public function viewApps($category = '') {
+		$categoryId = $this->getCategory($category);
+		if ($categoryId === self::CAT_ENABLED) {
+			// Do not use an arbitrary input string, because we put the category in html
+			$category = 'enabled';
+		}
+
 		$params = [];
 		$params['experimentalEnabled'] = $this->config->getSystemValue('appstore.experimental.enabled', false);
 		$params['category'] = $category;
@@ -123,8 +152,8 @@ class AppSettingsController extends Controller {
 			return $this->cache->get('listCategories');
 		}
 		$categories = [
-			['id' => 0, 'displayName' => (string)$this->l10n->t('Enabled')],
-			['id' => 1, 'displayName' => (string)$this->l10n->t('Not enabled')],
+			['id' => self::CAT_ENABLED, 'ident' => 'enabled', 'displayName' => (string)$this->l10n->t('Enabled')],
+			['id' => self::CAT_DISABLED, 'ident' => 'disabled', 'displayName' => (string)$this->l10n->t('Not enabled')],
 		];
 
 		if($this->ocsClient->isAppStoreEnabled()) {
@@ -132,9 +161,12 @@ class AppSettingsController extends Controller {
 			$ocs = $this->ocsClient->getCategories(\OC_Util::getVersion());
 			if ($ocs) {
 				foreach($ocs as $k => $v) {
+					$name = str_replace('ownCloud ', '', $v);
+					$ident = str_replace(' ', '-', urlencode(strtolower($name)));
 					$categories[] = [
 						'id' => $k,
-						'displayName' => str_replace('ownCloud ', '', $v)
+						'ident' => $ident,
+						'displayName' => $name,
 					];
 				}
 			}
@@ -148,12 +180,13 @@ class AppSettingsController extends Controller {
 	/**
 	 * Get all available apps in a category
 	 *
-	 * @param int $category
+	 * @param string $category
 	 * @param bool $includeUpdateInfo Should we check whether there is an update
 	 *                                in the app store?
 	 * @return array
 	 */
-	public function listApps($category = 0, $includeUpdateInfo = true) {
+	public function listApps($category = '', $includeUpdateInfo = true) {
+		$category = $this->getCategory($category);
 		$cacheName = 'listApps-' . $category . '-' . (int) $includeUpdateInfo;
 
 		if(!is_null($this->cache->get($cacheName))) {
diff --git a/settings/js/apps.js b/settings/js/apps.js
index 0734ad54ae..df1181b1e7 100644
--- a/settings/js/apps.js
+++ b/settings/js/apps.js
@@ -40,8 +40,8 @@ OC.Settings.Apps = OC.Settings.Apps || {
 		}
 
 		var categories = [
-			{displayName: t('settings', 'Enabled'), id: '0'},
-			{displayName: t('settings', 'Not enabled'), id: '1'}
+			{displayName: t('settings', 'Enabled'), ident: 'enabled', id: '0'},
+			{displayName: t('settings', 'Not enabled'), ident: 'disabled', id: '1'}
 		];
 
 		var source   = $("#categories-template").html();
@@ -49,7 +49,7 @@ OC.Settings.Apps = OC.Settings.Apps || {
 		var html = template(categories);
 		$('#apps-categories').html(html);
 
-		OC.Settings.Apps.loadCategory(parseInt($('#app-navigation').attr('data-category'), 10));
+		OC.Settings.Apps.loadCategory($('#app-navigation').attr('data-category'));
 
 		this._loadCategoriesCall = $.ajax(OC.generateUrl('settings/apps/categories'), {
 			data:{},
diff --git a/settings/templates/apps.php b/settings/templates/apps.php
index 5d9abe021a..bb01d991e5 100644
--- a/settings/templates/apps.php
+++ b/settings/templates/apps.php
@@ -24,7 +24,7 @@ script(
 ?>
 <script id="categories-template" type="text/x-handlebars-template">
 {{#each this}}
-	<li id="app-category-{{id}}" data-category-id="{{id}}" tabindex="0">
+	<li id="app-category-{{ident}}" data-category-id="{{ident}}" tabindex="0">
 		<a>{{displayName}}</a>
 	</li>
 {{/each}}
diff --git a/tests/settings/controller/AppSettingsControllerTest.php b/tests/settings/controller/AppSettingsControllerTest.php
index 62eebc2de5..09595d0566 100644
--- a/tests/settings/controller/AppSettingsControllerTest.php
+++ b/tests/settings/controller/AppSettingsControllerTest.php
@@ -132,10 +132,12 @@ class AppSettingsControllerTest extends TestCase {
 		$expected = [
 			[
 				'id' => 0,
+				'ident' => 'enabled',
 				'displayName' => 'Enabled',
 			],
 			[
 				'id' => 1,
+				'ident' => 'disabled',
 				'displayName' => 'Not enabled',
 			],
 		];
@@ -157,27 +159,33 @@ class AppSettingsControllerTest extends TestCase {
 		$expected = [
 			[
 				'id' => 0,
+				'ident' => 'enabled',
 				'displayName' => 'Enabled',
 			],
 			[
 				'id' => 1,
+				'ident' => 'disabled',
 				'displayName' => 'Not enabled',
 			],
 			[
 				'id' => 0,
+				'ident' => 'tools',
 				'displayName' => 'Tools',
 			],
 			[
 				'id' => 1,
-				'displayName' => 'Awesome Games',
+				'ident' => 'games',
+				'displayName' => 'Games',
 			],
 			[
 				'id' => 2,
-				'displayName' => 'PIM',
+				'ident' => 'productivity',
+				'displayName' => 'Productivity',
 			],
 			[
 				'id' => 3,
-				'displayName' => 'Papershop',
+				'ident' => 'multimedia',
+				'displayName' => 'Multimedia',
 			],
 		];
 
@@ -201,9 +209,9 @@ class AppSettingsControllerTest extends TestCase {
 			->will($this->returnValue(
 				[
 					'ownCloud Tools',
-					'Awesome Games',
-					'ownCloud PIM',
-					'Papershop',
+					'Games',
+					'ownCloud Productivity',
+					'Multimedia',
 				]
 			));
 
@@ -223,7 +231,7 @@ class AppSettingsControllerTest extends TestCase {
 		$policy = new ContentSecurityPolicy();
 		$policy->addAllowedImageDomain('https://apps.owncloud.com');
 
-		$expected = new TemplateResponse('settings', 'apps', ['experimentalEnabled' => false, 'category' => 0], 'user');
+		$expected = new TemplateResponse('settings', 'apps', ['experimentalEnabled' => false, 'category' => 'enabled'], 'user');
 		$expected->setContentSecurityPolicy($policy);
 
 		$this->assertEquals($expected, $this->appSettingsController->viewApps());
@@ -242,9 +250,9 @@ class AppSettingsControllerTest extends TestCase {
 		$policy = new ContentSecurityPolicy();
 		$policy->addAllowedImageDomain('https://apps.owncloud.com');
 
-		$expected = new TemplateResponse('settings', 'apps', ['experimentalEnabled' => false, 'category' => 1], 'user');
+		$expected = new TemplateResponse('settings', 'apps', ['experimentalEnabled' => false, 'category' => 'disabled'], 'user');
 		$expected->setContentSecurityPolicy($policy);
 
-		$this->assertEquals($expected, $this->appSettingsController->viewApps(1));
+		$this->assertEquals($expected, $this->appSettingsController->viewApps('disabled'));
 	}
 }