From ec330c7ac4d7d3a145dc06414e5707243f1057d7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 18:40:56 +0200 Subject: [PATCH 01/10] Register the app management in the normal way Signed-off-by: Joas Schilling --- core/templates/layout.user.php | 32 +------------------------------ lib/private/NavigationManager.php | 11 +++++++++++ lib/private/TemplateLayout.php | 6 ------ lib/private/legacy/app.php | 6 ------ 4 files changed, 12 insertions(+), 43 deletions(-) diff --git a/core/templates/layout.user.php b/core/templates/layout.user.php index adeeaf0379758..5eb93ade31fd6 100644 --- a/core/templates/layout.user.php +++ b/core/templates/layout.user.php @@ -61,7 +61,6 @@
@@ -115,25 +104,6 @@ - -
  • - class="active"> - - - - - - - t('Apps')); ?> - - -
    • - - diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index f7bc02943a385..396fc50e79f46 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -178,6 +178,17 @@ private function init() { 'name' => $l->t($nav['name']), ]); } + + if ($this->isAdmin()) { + $l = $this->l10nFac->get('settings'); + $this->add([ + 'id' => 'core_apps', + 'order' => 9999, + 'href' => $this->urlGenerator->linkToRoute('settings.AppSettings.viewApps'), + 'icon' => $this->urlGenerator->imagePath('settings', 'apps.svg'), + 'name' => $l->t('Apps'), + ]); + } } private function isAdmin() { diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php index 956cba400864f..6dc925f8f8c15 100644 --- a/lib/private/TemplateLayout.php +++ b/lib/private/TemplateLayout.php @@ -95,14 +95,8 @@ public function __construct( $renderAs, $appId = '' ) { } } $userDisplayName = \OC_User::getDisplayName(); - $appsMgmtActive = strpos(\OC::$server->getRequest()->getRequestUri(), \OC::$server->getURLGenerator()->linkToRoute('settings.AppSettings.viewApps')) === 0; - if ($appsMgmtActive) { - $l = \OC::$server->getL10N('lib'); - $this->assign('application', $l->t('Apps')); - } $this->assign('user_displayname', $userDisplayName); $this->assign('user_uid', \OC_User::getUser()); - $this->assign('appsmanagement_active', $appsMgmtActive); if (\OC_User::getUser() === false) { $this->assign('userAvatarSet', false); diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index 5343e7ad17220..31fa0a9e2dad8 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -531,9 +531,6 @@ public static function getSettingsNavigation() { // This is private as well. It simply works, so don't ask for more details private static function proceedNavigation($list) { $headerIconCount = 8; - if(OC_User::isAdminUser(OC_User::getUser())) { - $headerIconCount--; - } usort($list, function($a, $b) { if (isset($a['order']) && isset($b['order'])) { return ($a['order'] < $b['order']) ? -1 : 1; @@ -577,9 +574,6 @@ private static function proceedNavigation($list) { public static function proceedAppNavigation($entries) { $headerIconCount = 8; - if(OC_User::isAdminUser(OC_User::getUser())) { - $headerIconCount--; - } $activeAppIndex = -1; $list = self::proceedNavigation($entries); From 054e161eb5f4a5c5c13ee322ae8e93ce66f01b13 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 19:40:41 +0200 Subject: [PATCH 02/10] Manage the right side menu via the navigation manager as well Signed-off-by: Joas Schilling --- apps/files/lib/App.php | 10 ++- core/templates/layout.user.php | 6 -- lib/private/NavigationManager.php | 141 +++++++++++++++++++++++++----- lib/private/Server.php | 8 +- lib/private/legacy/app.php | 90 ++++--------------- 5 files changed, 145 insertions(+), 110 deletions(-) diff --git a/apps/files/lib/App.php b/apps/files/lib/App.php index add27a2d2df02..1aba2a53ef734 100644 --- a/apps/files/lib/App.php +++ b/apps/files/lib/App.php @@ -40,7 +40,15 @@ class App { public static function getNavigationManager() { // TODO: move this into a service in the Application class if (self::$navigationManager === null) { - self::$navigationManager = new \OC\NavigationManager(); + self::$navigationManager = new \OC\NavigationManager( + \OC::$server->getAppManager(), + \OC::$server->getURLGenerator(), + \OC::$server->getL10NFactory(), + \OC::$server->getUserSession(), + \OC::$server->getGroupManager(), + \OC::$server->getConfig() + ); + self::$navigationManager->noDefaultLinks(); } return self::$navigationManager; } diff --git a/core/templates/layout.user.php b/core/templates/layout.user.php index 5eb93ade31fd6..29e3fb36f3930 100644 --- a/core/templates/layout.user.php +++ b/core/templates/layout.user.php @@ -144,12 +144,6 @@ -
  • - > - - t('Log out'));?> - -
    • diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 396fc50e79f46..439b479524628 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -27,7 +27,9 @@ namespace OC; use OC\App\AppManager; +use OC\Group\Manager; use OCP\App\IAppManager; +use OCP\IConfig; use OCP\IGroupManager; use OCP\INavigationManager; use OCP\IURLGenerator; @@ -52,19 +54,23 @@ class NavigationManager implements INavigationManager { private $l10nFac; /** @var IUserSession */ private $userSession; - /** @var IGroupManager */ + /** @var IGroupManager|Manager */ private $groupManager; + /** @var IConfig */ + private $config; - public function __construct(IAppManager $appManager = null, - IURLGenerator $urlGenerator = null, - IFactory $l10nFac = null, - IUserSession $userSession = null, - IGroupManager$groupManager = null) { + public function __construct(IAppManager $appManager, + IURLGenerator $urlGenerator, + IFactory $l10nFac, + IUserSession $userSession, + IGroupManager $groupManager, + IConfig $config) { $this->appManager = $appManager; $this->urlGenerator = $urlGenerator; $this->l10nFac = $l10nFac; $this->userSession = $userSession; $this->groupManager = $groupManager; + $this->config = $config; } /** @@ -85,20 +91,42 @@ public function add($entry) { if(!isset($entry['icon'])) { $entry['icon'] = ''; } + if(!isset($entry['type'])) { + $entry['type'] = 'link'; + } $this->entries[] = $entry; } /** * returns all the added Menu entries + * @param string $type * @return array an array of the added entries */ - public function getAll() { + public function getAll($type = 'link') { $this->init(); foreach ($this->closureEntries as $c) { $this->add($c()); } $this->closureEntries = array(); - return $this->entries; + + if ($type === 'all') { + return $this->entries; + } + + return array_filter($this->entries, function($entry) use ($type) { + return $entry['type'] === $type; + }); + } + + /** + * Do not load the default links + * This is just a hack for the files app + * @internal + */ + public function noDefaultLinks() { + $this->entries = []; + $this->closureEntries = []; + $this->init = true; } /** @@ -134,7 +162,82 @@ private function init() { return; } $this->init = true; - if (is_null($this->appManager)) { + + if ($this->config->getSystemValue('knowledgebaseenabled', true)) { + $l = $this->l10nFac->get('lib'); + $this->add([ + 'type' => 'settings', + 'id' => 'help', + 'order' => 4, + 'href' => $this->urlGenerator->linkToRoute('settings_help'), + 'name' => $l->t('Help'), + 'icon' => $this->urlGenerator->imagePath('settings', 'help.svg'), + ]); + } + + if ($this->userSession->isLoggedIn()) { + if ($this->isAdmin()) { + $l = $this->l10nFac->get('settings'); + // App management + $this->add([ + 'id' => 'core_apps', + 'order' => 9999, + 'href' => $this->urlGenerator->linkToRoute('settings.AppSettings.viewApps'), + 'icon' => $this->urlGenerator->imagePath('settings', 'apps.svg'), + 'name' => $l->t('Apps'), + ]); + } + + $l = $this->l10nFac->get('lib'); + // Personal settings + $this->add([ + 'type' => 'settings', + 'id' => 'personal', + 'order' => 1, + 'href' => $this->urlGenerator->linkToRoute('settings_personal'), + 'name' => $l->t('Personal'), + 'icon' => $this->urlGenerator->imagePath('settings', 'personal.svg'), + ]); + + // Logout + $this->add([ + 'type' => 'settings', + 'id' => 'logout', + 'order' => 99999, + 'href' => $this->urlGenerator->linkToRouteAbsolute( + 'core.login.logout', + ['requesttoken' => \OCP\Util::callRegister()] + ), + 'name' => $l->t('Log out'), + 'icon' => $this->urlGenerator->imagePath('core', 'actions/logout.svg'), + ]); + + if ($this->isSubadmin()) { + // User management + $this->add([ + 'type' => 'settings', + 'id' => 'core_users', + 'order' => 3, + 'href' => $this->urlGenerator->linkToRoute('settings_users'), + 'name' => $l->t('Users'), + 'icon' => $this->urlGenerator->imagePath('settings', 'users.svg'), + ]); + } + + if ($this->isAdmin()) { + // Admin settings + $this->add([ + 'type' => 'settings', + 'id' => 'admin', + 'order' => 2, + 'href' => $this->urlGenerator->linkToRoute('settings.AdminSettings.index'), + 'name' => $l->t('Admin'), + 'icon' => $this->urlGenerator->imagePath('settings', 'admin.svg'), + ]); + } + } + + if ($this->appManager === 'null') { return; } foreach ($this->appManager->getInstalledApps() as $app) { @@ -166,7 +269,7 @@ private function init() { // no icon? - ignore it then } } - if (is_null($icon)) { + if ($icon === null) { $icon = $this->urlGenerator->imagePath('core', 'default-app-icon'); } @@ -178,17 +281,6 @@ private function init() { 'name' => $l->t($nav['name']), ]); } - - if ($this->isAdmin()) { - $l = $this->l10nFac->get('settings'); - $this->add([ - 'id' => 'core_apps', - 'order' => 9999, - 'href' => $this->urlGenerator->linkToRoute('settings.AppSettings.viewApps'), - 'icon' => $this->urlGenerator->imagePath('settings', 'apps.svg'), - 'name' => $l->t('Apps'), - ]); - } } private function isAdmin() { @@ -199,4 +291,11 @@ private function isAdmin() { return false; } + private function isSubadmin() { + $user = $this->userSession->getUser(); + if ($user !== null) { + return $this->groupManager->getSubAdmin()->isSubAdmin($user); + } + return false; + } } diff --git a/lib/private/Server.php b/lib/private/Server.php index c21ff650b24a4..10f9a810de9f5 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -353,13 +353,7 @@ public function __construct($webRoot, \OC\Config $config) { return new \OC\Authentication\TwoFactorAuth\Manager($c->getAppManager(), $c->getSession(), $c->getConfig(), $c->getActivityManager(), $c->getLogger()); }); - $this->registerService(\OCP\INavigationManager::class, function (Server $c) { - return new \OC\NavigationManager($c->getAppManager(), - $c->getURLGenerator(), - $c->getL10NFactory(), - $c->getUserSession(), - $c->getGroupManager()); - }); + $this->registerAlias(\OCP\INavigationManager::class, \OC\NavigationManager::class); $this->registerAlias('NavigationManager', \OCP\INavigationManager::class); $this->registerService(\OC\AllConfig::class, function (Server $c) { diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index 31fa0a9e2dad8..a68f860525854 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -457,77 +457,6 @@ public static function disable($app) { $appManager->disableApp($app); } - /** - * Returns the Settings Navigation - * - * @return string[] - * - * This function returns an array containing all settings pages added. The - * entries are sorted by the key 'order' ascending. - */ - public static function getSettingsNavigation() { - $l = \OC::$server->getL10N('lib'); - $urlGenerator = \OC::$server->getURLGenerator(); - - $settings = array(); - // by default, settings only contain the help menu - if (\OC::$server->getSystemConfig()->getValue('knowledgebaseenabled', true)) { - $settings = array( - array( - "id" => "help", - "order" => 4, - "href" => $urlGenerator->linkToRoute('settings_help'), - "name" => $l->t("Help"), - "icon" => $urlGenerator->imagePath("settings", "help.svg") - ) - ); - } - - // if the user is logged-in - if (\OC::$server->getUserSession()->isLoggedIn()) { - // personal menu - $settings[] = array( - "id" => "personal", - "order" => 1, - "href" => $urlGenerator->linkToRoute('settings_personal'), - "name" => $l->t("Personal"), - "icon" => $urlGenerator->imagePath("settings", "personal.svg") - ); - - //SubAdmins are also allowed to access user management - $userObject = \OC::$server->getUserSession()->getUser(); - $isSubAdmin = false; - if($userObject !== null) { - $isSubAdmin = \OC::$server->getGroupManager()->getSubAdmin()->isSubAdmin($userObject); - } - if ($isSubAdmin) { - // admin users menu - $settings[] = array( - "id" => "core_users", - "order" => 3, - "href" => $urlGenerator->linkToRoute('settings_users'), - "name" => $l->t("Users"), - "icon" => $urlGenerator->imagePath("settings", "users.svg") - ); - } - - // if the user is an admin - if (OC_User::isAdminUser(OC_User::getUser())) { - // admin settings - $settings[] = array( - "id" => "admin", - "order" => 2, - "href" => $urlGenerator->linkToRoute('settings.AdminSettings.index'), - "name" => $l->t("Admin"), - "icon" => $urlGenerator->imagePath("settings", "admin.svg") - ); - } - } - - $navigation = self::proceedNavigation($settings); - return $navigation; - } - // This is private as well. It simply works, so don't ask for more details private static function proceedNavigation($list) { $headerIconCount = 8; @@ -783,8 +712,7 @@ public static function getAppInfo($appId, $path = false, $lang = null) { */ public static function getNavigation() { $entries = OC::$server->getNavigationManager()->getAll(); - $navigation = self::proceedNavigation($entries); - return $navigation; + return self::proceedNavigation($entries); } /** @@ -799,8 +727,20 @@ public static function getNavigation() { */ public static function getHeaderNavigation() { $entries = OC::$server->getNavigationManager()->getAll(); - $navigation = self::proceedAppNavigation($entries); - return $navigation; + return self::proceedAppNavigation($entries); + } + + /** + * Returns the Settings Navigation + * + * @return string[] + * + * This function returns an array containing all settings pages added. The + * entries are sorted by the key 'order' ascending. + */ + public static function getSettingsNavigation() { + $entries = OC::$server->getNavigationManager()->getAll('settings'); + return self::proceedNavigation($entries); } /** From 433958e2e38f4d6935f695e482e80693dc0ffeb9 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 20:26:05 +0200 Subject: [PATCH 03/10] Move app management to the settings menu Signed-off-by: Joas Schilling --- core/css/header.scss | 24 +----------------------- lib/private/NavigationManager.php | 7 +++---- settings/img/apps.svg | 5 ++++- settings/js/apps.js | 6 +----- 4 files changed, 9 insertions(+), 33 deletions(-) diff --git a/core/css/header.scss b/core/css/header.scss index 4b6c0fbd007ac..1ba90aaabff04 100644 --- a/core/css/header.scss +++ b/core/css/header.scss @@ -322,28 +322,6 @@ nav { } /* Apps management */ -.apps-management { - min-height: initial; - height: initial; - margin: 0; - a { - svg, - span { - -ms-filter: 'progid:DXImageTransform.Microsoft.Alpha(Opacity=30)'; - opacity: .3; - } - /* icon opacity and hover effect */ - &:hover svg, - &:focus svg, - &.active svg, - &:hover span, - &:focus span, - &.active span { - -ms-filter: 'progid:DXImageTransform.Microsoft.Alpha(Opacity=100)'; - opacity: 1; - } - } -} #apps { max-height: calc(100vh - 100px); @@ -583,4 +561,4 @@ nav { } } -} \ No newline at end of file +} diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 439b479524628..0b1f43d7de9ae 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -163,8 +163,8 @@ private function init() { } $this->init = true; + $l = $this->l10nFac->get('lib'); if ($this->config->getSystemValue('knowledgebaseenabled', true)) { - $l = $this->l10nFac->get('lib'); $this->add([ 'type' => 'settings', 'id' => 'help', @@ -177,18 +177,17 @@ private function init() { if ($this->userSession->isLoggedIn()) { if ($this->isAdmin()) { - $l = $this->l10nFac->get('settings'); // App management $this->add([ + 'type' => 'settings', 'id' => 'core_apps', - 'order' => 9999, + 'order' => 50, 'href' => $this->urlGenerator->linkToRoute('settings.AppSettings.viewApps'), 'icon' => $this->urlGenerator->imagePath('settings', 'apps.svg'), 'name' => $l->t('Apps'), ]); } - $l = $this->l10nFac->get('lib'); // Personal settings $this->add([ 'type' => 'settings', diff --git a/settings/img/apps.svg b/settings/img/apps.svg index d1509ea3a3779..b6e545799a3d6 100644 --- a/settings/img/apps.svg +++ b/settings/img/apps.svg @@ -1 +1,4 @@ - \ No newline at end of file + + + + diff --git a/settings/js/apps.js b/settings/js/apps.js index d2f26578a7cf7..253cc7b1eaf3d 100644 --- a/settings/js/apps.js +++ b/settings/js/apps.js @@ -537,14 +537,10 @@ OC.Settings.Apps = OC.Settings.Apps || { } } - - - if (navEntries.length > 7) { + if (navEntries.length > 8) { $('#more-apps').show(); - $('.apps-management').hide(); } else { $('#more-apps').hide(); - $('.apps-management').show(); } } }); From 7cc5130e8262b8e42df0c2a017cddcca2b6d8c85 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 20:37:39 +0200 Subject: [PATCH 04/10] Allow apps to register a setting via info.xml Signed-off-by: Joas Schilling --- lib/private/NavigationManager.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 0b1f43d7de9ae..439e49909d003 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -258,6 +258,7 @@ private function init() { } $l = $this->l10nFac->get($app); $order = isset($nav['order']) ? $nav['order'] : 100; + $type = isset($nav['type']) ? $nav['type'] : 'link'; $route = $this->urlGenerator->linkToRoute($nav['route']); $icon = isset($nav['icon']) ? $nav['icon'] : 'app.svg'; foreach ([$icon, "$app.svg"] as $i) { @@ -277,6 +278,7 @@ private function init() { 'order' => $order, 'href' => $route, 'icon' => $icon, + 'type' => $type, 'name' => $l->t($nav['name']), ]); } From e0b040d6235e7f8f8e5869e56a9bda1bf346cafd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 21:15:25 +0200 Subject: [PATCH 05/10] Allow multiple navigation links from info.xml Signed-off-by: Joas Schilling --- apps/files/appinfo/info.xml | 12 +-- apps/files/lib/App.php | 2 +- lib/private/NavigationManager.php | 86 ++++++++++------------ tests/lib/NavigationManagerTest.php | 110 ++++++++++++++++++---------- 4 files changed, 119 insertions(+), 91 deletions(-) diff --git a/apps/files/appinfo/info.xml b/apps/files/appinfo/info.xml index 8b5678b331a4d..c49ec7aa40796 100644 --- a/apps/files/appinfo/info.xml +++ b/apps/files/appinfo/info.xml @@ -55,10 +55,12 @@ OCA\Files\Command\ScanAppData - - Files - files.view.index - 0 - + + + Files + files.view.index + 0 + + diff --git a/apps/files/lib/App.php b/apps/files/lib/App.php index 1aba2a53ef734..34d3ab4384c47 100644 --- a/apps/files/lib/App.php +++ b/apps/files/lib/App.php @@ -48,7 +48,7 @@ public static function getNavigationManager() { \OC::$server->getGroupManager(), \OC::$server->getConfig() ); - self::$navigationManager->noDefaultLinks(); + self::$navigationManager->clear(false); } return self::$navigationManager; } diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 439e49909d003..27de7c6f3604b 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -118,24 +118,13 @@ public function getAll($type = 'link') { }); } - /** - * Do not load the default links - * This is just a hack for the files app - * @internal - */ - public function noDefaultLinks() { - $this->entries = []; - $this->closureEntries = []; - $this->init = true; - } - /** * removes all the entries */ - public function clear() { + public function clear($loadDefaultLinks = true) { $this->entries = []; $this->closureEntries = []; - $this->init = false; + $this->init = !$loadDefaultLinks; } /** @@ -242,45 +231,46 @@ private function init() { foreach ($this->appManager->getInstalledApps() as $app) { // load plugins and collections from info.xml $info = $this->appManager->getAppInfo($app); - if (!isset($info['navigation'])) { - continue; - } - $nav = $info['navigation']; - if (!isset($nav['name'])) { - continue; - } - if (!isset($nav['route'])) { - continue; - } - $role = isset($nav['@attributes']['role']) ? $nav['@attributes']['role'] : 'all'; - if ($role === 'admin' && !$this->isAdmin()) { + if (empty($info['navigations'])) { continue; } - $l = $this->l10nFac->get($app); - $order = isset($nav['order']) ? $nav['order'] : 100; - $type = isset($nav['type']) ? $nav['type'] : 'link'; - $route = $this->urlGenerator->linkToRoute($nav['route']); - $icon = isset($nav['icon']) ? $nav['icon'] : 'app.svg'; - foreach ([$icon, "$app.svg"] as $i) { - try { - $icon = $this->urlGenerator->imagePath($app, $i); - break; - } catch (\RuntimeException $ex) { - // no icon? - ignore it then + foreach ($info['navigations'] as $nav) { + if (!isset($nav['name'])) { + continue; + } + if (!isset($nav['route'])) { + continue; + } + $role = isset($nav['@attributes']['role']) ? $nav['@attributes']['role'] : 'all'; + if ($role === 'admin' && !$this->isAdmin()) { + continue; + } + $l = $this->l10nFac->get($app); + $order = isset($nav['order']) ? $nav['order'] : 100; + $type = isset($nav['type']) ? $nav['type'] : 'link'; + $route = $this->urlGenerator->linkToRoute($nav['route']); + $icon = isset($nav['icon']) ? $nav['icon'] : 'app.svg'; + foreach ([$icon, "$app.svg"] as $i) { + try { + $icon = $this->urlGenerator->imagePath($app, $i); + break; + } catch (\RuntimeException $ex) { + // no icon? - ignore it then + } + } + if ($icon === null) { + $icon = $this->urlGenerator->imagePath('core', 'default-app-icon'); } - } - if ($icon === null) { - $icon = $this->urlGenerator->imagePath('core', 'default-app-icon'); - } - $this->add([ - 'id' => $app, - 'order' => $order, - 'href' => $route, - 'icon' => $icon, - 'type' => $type, - 'name' => $l->t($nav['name']), - ]); + $this->add([ + 'id' => $app, + 'order' => $order, + 'href' => $route, + 'icon' => $icon, + 'type' => $type, + 'name' => $l->t($nav['name']), + ]); + } } } diff --git a/tests/lib/NavigationManagerTest.php b/tests/lib/NavigationManagerTest.php index 64fec802eca7e..0871a9a0910cb 100644 --- a/tests/lib/NavigationManagerTest.php +++ b/tests/lib/NavigationManagerTest.php @@ -14,7 +14,7 @@ use OC\App\AppManager; use OC\NavigationManager; -use OCP\App\IAppManager; +use OCP\IConfig; use OCP\IGroupManager; use OCP\IL10N; use OCP\IURLGenerator; @@ -23,13 +23,41 @@ use OCP\L10N\IFactory; class NavigationManagerTest extends TestCase { + /** @var AppManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $appManager; + /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ + protected $urlGenerator; + /** @var IFactory|\PHPUnit_Framework_MockObject_MockObject */ + protected $l10nFac; + /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ + protected $userSession; + /** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $groupManager; + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + protected $config; + /** @var \OC\NavigationManager */ protected $navigationManager; protected function setUp() { parent::setUp(); - $this->navigationManager = new NavigationManager(); + $this->appManager = $this->createMock(AppManager::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->l10nFac = $this->createMock(IFactory::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->config = $this->createMock(IConfig::class); + $this->navigationManager = new NavigationManager( + $this->appManager, + $this->urlGenerator, + $this->l10nFac, + $this->userSession, + $this->groupManager, + $this->config + ); + + $this->navigationManager->clear(false); } public function addArrayData() { @@ -41,6 +69,7 @@ public function addArrayData() { 'order' => 1, 'icon' => 'optional', 'href' => 'url', + 'type' => 'settings', ], [ 'id' => 'entry id', @@ -49,6 +78,7 @@ public function addArrayData() { 'icon' => 'optional', 'href' => 'url', 'active' => false, + 'type' => 'settings', ], ], [ @@ -67,6 +97,7 @@ public function addArrayData() { 'icon' => '', 'href' => 'url', 'active' => false, + 'type' => 'link', ], ], ]; @@ -79,15 +110,15 @@ public function addArrayData() { * @param array $expectedEntry */ public function testAddArray(array $entry, array $expectedEntry) { - $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists'); + $this->assertEmpty($this->navigationManager->getAll('all'), 'Expected no navigation entry exists'); $this->navigationManager->add($entry); - $navigationEntries = $this->navigationManager->getAll(); - $this->assertEquals(1, sizeof($navigationEntries), 'Expected that 1 navigation entry exists'); + $navigationEntries = $this->navigationManager->getAll('all'); + $this->assertCount(1, $navigationEntries, 'Expected that 1 navigation entry exists'); $this->assertEquals($expectedEntry, $navigationEntries[0]); - $this->navigationManager->clear(); - $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists after clear()'); + $this->navigationManager->clear(false); + $this->assertEmpty($this->navigationManager->getAll('all'), 'Expected no navigation entry exists after clear()'); } /** @@ -109,18 +140,18 @@ public function testAddClosure(array $entry, array $expectedEntry) { $this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by add()'); - $navigationEntries = $this->navigationManager->getAll(); + $navigationEntries = $this->navigationManager->getAll('all'); $this->assertEquals(1, $testAddClosureNumberOfCalls, 'Expected that the closure is called by getAll()'); - $this->assertEquals(1, sizeof($navigationEntries), 'Expected that 1 navigation entry exists'); + $this->assertCount(1, $navigationEntries, 'Expected that 1 navigation entry exists'); $this->assertEquals($expectedEntry, $navigationEntries[0]); - $navigationEntries = $this->navigationManager->getAll(); + $navigationEntries = $this->navigationManager->getAll('all'); $this->assertEquals(1, $testAddClosureNumberOfCalls, 'Expected that the closure is only called once for getAll()'); - $this->assertEquals(1, sizeof($navigationEntries), 'Expected that 1 navigation entry exists'); + $this->assertCount(1, $navigationEntries, 'Expected that 1 navigation entry exists'); $this->assertEquals($expectedEntry, $navigationEntries[0]); - $this->navigationManager->clear(); - $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists after clear()'); + $this->navigationManager->clear(false); + $this->assertEmpty($this->navigationManager->getAll('all'), 'Expected no navigation entry exists after clear()'); } public function testAddArrayClearGetAll() { @@ -134,7 +165,7 @@ public function testAddArrayClearGetAll() { $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists'); $this->navigationManager->add($entry); - $this->navigationManager->clear(); + $this->navigationManager->clear(false); $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists after clear()'); } @@ -160,7 +191,7 @@ public function testAddClosureClearGetAll() { }); $this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by add()'); - $this->navigationManager->clear(); + $this->navigationManager->clear(false); $this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by clear()'); $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists after clear()'); $this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by getAll()'); @@ -169,35 +200,29 @@ public function testAddClosureClearGetAll() { /** * @dataProvider providesNavigationConfig */ - public function testWithAppManager($expected, $config, $isAdmin = false) { + public function testWithAppManager($expected, $navigation, $isAdmin = false) { - $appManager = $this->createMock(AppManager::class); - $urlGenerator = $this->createMock(IURLGenerator::class); - $l10nFac = $this->createMock(IFactory::class); - $userSession = $this->createMock(IUserSession::class); - $groupManager = $this->createMock(IGroupManager::class); $l = $this->createMock(IL10N::class); $l->expects($this->any())->method('t')->willReturnCallback(function($text, $parameters = []) { return vsprintf($text, $parameters); }); - $appManager->expects($this->once())->method('getInstalledApps')->willReturn(['test']); - $appManager->expects($this->once())->method('getAppInfo')->with('test')->willReturn($config); - $l10nFac->expects($this->exactly(count($expected)))->method('get')->with('test')->willReturn($l); - $urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function($appName, $file) { + $this->appManager->expects($this->once())->method('getInstalledApps')->willReturn(['test']); + $this->appManager->expects($this->once())->method('getAppInfo')->with('test')->willReturn($navigation); + $this->l10nFac->expects($this->exactly(count($expected) + 1))->method('get')->willReturn($l); + $this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function($appName, $file) { return "/apps/$appName/img/$file"; }); - $urlGenerator->expects($this->exactly(count($expected)))->method('linkToRoute')->willReturnCallback(function($route) { + $this->urlGenerator->expects($this->exactly(count($expected)))->method('linkToRoute')->willReturnCallback(function() { return "/apps/test/"; }); $user = $this->createMock(IUser::class); $user->expects($this->any())->method('getUID')->willReturn('user001'); - $userSession->expects($this->any())->method('getUser')->willReturn($user); - $groupManager->expects($this->any())->method('isAdmin')->willReturn($isAdmin); - - $navigationManager = new NavigationManager($appManager, $urlGenerator, $l10nFac, $userSession, $groupManager); + $this->userSession->expects($this->any())->method('getUser')->willReturn($user); + $this->groupManager->expects($this->any())->method('isAdmin')->willReturn($isAdmin); - $entries = $navigationManager->getAll(); + $this->navigationManager->clear(); + $entries = $this->navigationManager->getAll('all'); $this->assertEquals($expected, $entries); } @@ -209,18 +234,29 @@ public function providesNavigationConfig() { 'href' => '/apps/test/', 'icon' => '/apps/test/img/app.svg', 'name' => 'Test', - 'active' => false - ]], ['navigation' => ['route' => 'test.page.index', 'name' => 'Test']]], + 'active' => false, + 'type' => 'link', + ]], ['navigations' => [['route' => 'test.page.index', 'name' => 'Test']]]], + 'minimalistic-settings' => [[[ + 'id' => 'test', + 'order' => 100, + 'href' => '/apps/test/', + 'icon' => '/apps/test/img/app.svg', + 'name' => 'Test', + 'active' => false, + 'type' => 'settings', + ]], ['navigations' => [['route' => 'test.page.index', 'name' => 'Test', 'type' => 'settings']]]], 'no admin' => [[[ 'id' => 'test', 'order' => 100, 'href' => '/apps/test/', 'icon' => '/apps/test/img/app.svg', 'name' => 'Test', - 'active' => false - ]], ['navigation' => ['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test']], true], - 'no name' => [[], ['navigation' => ['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index']], true], - 'admin' => [[], ['navigation' => ['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test']]] + 'active' => false, + 'type' => 'link', + ]], ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test']]], true], + 'no name' => [[], ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index']]], true], + 'admin' => [[], ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test']]]] ]; } } From 0229c16e5fe9a20bc150b2162d22df6fe236cb9e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 21:23:18 +0200 Subject: [PATCH 06/10] Don't use the dropdown for one item only Signed-off-by: Joas Schilling --- lib/private/legacy/app.php | 14 ++++++++++++-- settings/js/apps.js | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index a68f860525854..68a8383afbf79 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -459,7 +459,6 @@ public static function disable($app) { // This is private as well. It simply works, so don't ask for more details private static function proceedNavigation($list) { - $headerIconCount = 8; usort($list, function($a, $b) { if (isset($a['order']) && isset($b['order'])) { return ($a['order'] < $b['order']) ? -1 : 1; @@ -482,6 +481,11 @@ private static function proceedNavigation($list) { } unset($navEntry); + if (count($list) <= 8) { + return $list; + } + + $headerIconCount = 7; if($activeAppIndex > ($headerIconCount-1)) { $active = $list[$activeAppIndex]; $lastInHeader = $list[$headerIconCount-1]; @@ -502,7 +506,6 @@ private static function proceedNavigation($list) { } public static function proceedAppNavigation($entries) { - $headerIconCount = 8; $activeAppIndex = -1; $list = self::proceedNavigation($entries); @@ -515,6 +518,13 @@ public static function proceedAppNavigation($entries) { $navEntry['active'] = false; } } + + + if (count($list) <= 8) { + return $list; + } + + $headerIconCount = 7; // move active item to last position if($activeAppIndex > ($headerIconCount-1)) { $active = $list[$activeAppIndex]; diff --git a/settings/js/apps.js b/settings/js/apps.js index 253cc7b1eaf3d..215b3c2c92e9f 100644 --- a/settings/js/apps.js +++ b/settings/js/apps.js @@ -530,7 +530,7 @@ OC.Settings.Apps = OC.Settings.Apps || { } previousEntry = entry; // do not show apps from #appmenu in #navigation - if(i < 7) { + if(i <= 7) { $('#navigation li').eq(i).addClass('in-header'); } else { $('#navigation li').eq(i).removeClass('in-header'); From 0564392cb973dbc3f3669ecdf9fa25524c1c8724 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 21:26:45 +0200 Subject: [PATCH 07/10] Fix the tests Signed-off-by: Joas Schilling --- apps/files/tests/Controller/ViewControllerTest.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php index 1d8c2956551f0..2e497405a4d90 100644 --- a/apps/files/tests/Controller/ViewControllerTest.php +++ b/apps/files/tests/Controller/ViewControllerTest.php @@ -134,6 +134,7 @@ public function testIndexWithRegularBrowser() { 'name' => \OC::$server->getL10N('files')->t('All files'), 'active' => false, 'icon' => '', + 'type' => 'link', ], [ 'id' => 'recent', @@ -143,6 +144,7 @@ public function testIndexWithRegularBrowser() { 'name' => \OC::$server->getL10N('files')->t('Recent'), 'active' => false, 'icon' => '', + 'type' => 'link', ], [ 'id' => 'favorites', @@ -152,6 +154,7 @@ public function testIndexWithRegularBrowser() { 'name' => null, 'active' => false, 'icon' => '', + 'type' => 'link', ], [ 'id' => 'sharingin', @@ -161,6 +164,7 @@ public function testIndexWithRegularBrowser() { 'name' => \OC::$server->getL10N('files_sharing')->t('Shared with you'), 'active' => false, 'icon' => '', + 'type' => 'link', ], [ 'id' => 'sharingout', @@ -170,6 +174,7 @@ public function testIndexWithRegularBrowser() { 'name' => \OC::$server->getL10N('files_sharing')->t('Shared with others'), 'active' => false, 'icon' => '', + 'type' => 'link', ], [ 'id' => 'sharinglinks', @@ -179,6 +184,7 @@ public function testIndexWithRegularBrowser() { 'name' => \OC::$server->getL10N('files_sharing')->t('Shared by link', []), 'active' => false, 'icon' => '', + 'type' => 'link', ], [ 'id' => 'systemtagsfilter', @@ -188,6 +194,7 @@ public function testIndexWithRegularBrowser() { 'name' => \OC::$server->getL10N('systemtags')->t('Tags'), 'active' => false, 'icon' => '', + 'type' => 'link', ], [ 'id' => 'trashbin', @@ -197,7 +204,8 @@ public function testIndexWithRegularBrowser() { 'name' => \OC::$server->getL10N('files_trashbin')->t('Deleted files'), 'active' => false, 'icon' => '', - ], + 'type' => 'link', + ], ]); $expected = new Http\TemplateResponse( From 900f11fd9176b62b79adb564170ba7a02dd31402 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Sun, 26 Mar 2017 14:30:21 -0600 Subject: [PATCH 08/10] Fix order as @jancborchardt requested Signed-off-by: Morris Jobke --- lib/private/NavigationManager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 27de7c6f3604b..2bb51ce7244af 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -170,7 +170,7 @@ private function init() { $this->add([ 'type' => 'settings', 'id' => 'core_apps', - 'order' => 50, + 'order' => 3, 'href' => $this->urlGenerator->linkToRoute('settings.AppSettings.viewApps'), 'icon' => $this->urlGenerator->imagePath('settings', 'apps.svg'), 'name' => $l->t('Apps'), @@ -205,7 +205,7 @@ private function init() { $this->add([ 'type' => 'settings', 'id' => 'core_users', - 'order' => 3, + 'order' => 4, 'href' => $this->urlGenerator->linkToRoute('settings_users'), 'name' => $l->t('Users'), 'icon' => $this->urlGenerator->imagePath('settings', 'users.svg'), From 918f6fd10b2837cc684cf1e0c8a773716fe7d9a3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 22:32:12 +0200 Subject: [PATCH 09/10] Make sure help is always after users Signed-off-by: Joas Schilling --- lib/private/NavigationManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 2bb51ce7244af..7465daef35918 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -157,7 +157,7 @@ private function init() { $this->add([ 'type' => 'settings', 'id' => 'help', - 'order' => 4, + 'order' => 5, 'href' => $this->urlGenerator->linkToRoute('settings_help'), 'name' => $l->t('Help'), 'icon' => $this->urlGenerator->imagePath('settings', 'help.svg'), From 8d3c461151ad9515772609059063f5bdf5a32bf3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 26 Mar 2017 23:07:54 +0200 Subject: [PATCH 10/10] Allow to specify the id Signed-off-by: Joas Schilling --- lib/private/NavigationManager.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 7465daef35918..300c24ff940e9 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -246,6 +246,7 @@ private function init() { continue; } $l = $this->l10nFac->get($app); + $id = isset($nav['id']) ? $nav['id'] : $app; $order = isset($nav['order']) ? $nav['order'] : 100; $type = isset($nav['type']) ? $nav['type'] : 'link'; $route = $this->urlGenerator->linkToRoute($nav['route']); @@ -263,7 +264,7 @@ private function init() { } $this->add([ - 'id' => $app, + 'id' => $id, 'order' => $order, 'href' => $route, 'icon' => $icon,