diff --git a/apps/files/appinfo/info.xml b/apps/files/appinfo/info.xml index 8b5678b331..c49ec7aa40 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 1aba2a53ef..34d3ab4384 100644 --- a/apps/files/lib/App.php +++ b/apps/files/lib/App.php @@ -48,7 +48,7 @@ class App { \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 439e49909d..27de7c6f36 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -118,24 +118,13 @@ class NavigationManager implements INavigationManager { }); } - /** - * 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 @@ class NavigationManager implements INavigationManager { foreach ($this->appManager->getInstalledApps() as $app) { // load plugins and collections from info.xml $info = $this->appManager->getAppInfo($app); - if (!isset($info['navigation'])) { + if (empty($info['navigations'])) { 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()) { - 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 64fec802ec..0871a9a091 100644 --- a/tests/lib/NavigationManagerTest.php +++ b/tests/lib/NavigationManagerTest.php @@ -14,7 +14,7 @@ namespace Test; 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\IUserSession; 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 @@ class NavigationManagerTest extends TestCase { 'order' => 1, 'icon' => 'optional', 'href' => 'url', + 'type' => 'settings', ], [ 'id' => 'entry id', @@ -49,6 +78,7 @@ class NavigationManagerTest extends TestCase { 'icon' => 'optional', 'href' => 'url', 'active' => false, + 'type' => 'settings', ], ], [ @@ -67,6 +97,7 @@ class NavigationManagerTest extends TestCase { 'icon' => '', 'href' => 'url', 'active' => false, + 'type' => 'link', ], ], ]; @@ -79,15 +110,15 @@ class NavigationManagerTest extends TestCase { * @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 @@ class NavigationManagerTest extends TestCase { $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 @@ class NavigationManagerTest extends TestCase { $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 @@ class NavigationManagerTest extends TestCase { }); $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 @@ class NavigationManagerTest extends TestCase { /** * @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); + $this->userSession->expects($this->any())->method('getUser')->willReturn($user); + $this->groupManager->expects($this->any())->method('isAdmin')->willReturn($isAdmin); - $navigationManager = new NavigationManager($appManager, $urlGenerator, $l10nFac, $userSession, $groupManager); - - $entries = $navigationManager->getAll(); + $this->navigationManager->clear(); + $entries = $this->navigationManager->getAll('all'); $this->assertEquals($expected, $entries); } @@ -209,18 +234,29 @@ class NavigationManagerTest extends TestCase { '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']]]] ]; } }