From ac19ba9c662697b568d28c4c674ee5d3025eb16c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 25 Sep 2023 15:02:23 +0200 Subject: [PATCH 1/7] Read apporder from configuration value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/NavigationManager.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index ac6f16190dcd1..ef6e6f4cb7401 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -285,11 +285,15 @@ private function init() { } if ($this->userSession->isLoggedIn()) { - $apps = $this->appManager->getEnabledAppsForUser($this->userSession->getUser()); + $user = $this->userSession->getUser(); + $apps = $this->appManager->getEnabledAppsForUser($user); + $customOrders = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR); } else { $apps = $this->appManager->getInstalledApps(); + $customOrders = []; } + foreach ($apps as $app) { if (!$this->userSession->isLoggedIn() && !$this->appManager->isEnabledForUser($app, $this->userSession->getUser())) { continue; @@ -315,7 +319,7 @@ private function init() { } $l = $this->l10nFac->get($app); $id = $nav['id'] ?? $app . ($key === 0 ? '' : $key); - $order = isset($nav['order']) ? $nav['order'] : 100; + $order = $customOrders[$app][$key] ?? $nav['order'] ?? 100; $type = $nav['type']; $route = !empty($nav['route']) ? $this->urlGenerator->linkToRoute($nav['route']) : ''; $icon = isset($nav['icon']) ? $nav['icon'] : 'app.svg'; From a3a42591e2e85e5903217052b246e46dbbf4c1c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 25 Sep 2023 15:43:04 +0200 Subject: [PATCH 2/7] Support admin default order for applications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/NavigationManager.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index ef6e6f4cb7401..9ef22132ea585 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -284,10 +284,16 @@ private function init() { return; } + $adminCustomOrders = json_decode($this->config->getSystemValueString('apporder', '[]'), true, flags:JSON_THROW_ON_ERROR); + $forceAdminOrder = $this->config->getSystemValueBool('apporderForce', false); if ($this->userSession->isLoggedIn()) { $user = $this->userSession->getUser(); $apps = $this->appManager->getEnabledAppsForUser($user); - $customOrders = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR); + if ($forceAdminOrder) { + $customOrders = []; + } else { + $customOrders = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR); + } } else { $apps = $this->appManager->getInstalledApps(); $customOrders = []; @@ -319,7 +325,7 @@ private function init() { } $l = $this->l10nFac->get($app); $id = $nav['id'] ?? $app . ($key === 0 ? '' : $key); - $order = $customOrders[$app][$key] ?? $nav['order'] ?? 100; + $order = $customOrders[$app][$key] ?? $adminCustomOrders[$app][$key] ?? $nav['order'] ?? 100; $type = $nav['type']; $route = !empty($nav['route']) ? $this->urlGenerator->linkToRoute($nav['route']) : ''; $icon = isset($nav['icon']) ? $nav['icon'] : 'app.svg'; From 6083d32a805d90b5a3bc83577ab81c946b8dd40c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Sep 2023 14:22:34 +0200 Subject: [PATCH 3/7] Revert "Support admin default order for applications" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit a3a42591e2e85e5903217052b246e46dbbf4c1c7. Signed-off-by: Côme Chilliet --- lib/private/NavigationManager.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 9ef22132ea585..ef6e6f4cb7401 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -284,16 +284,10 @@ private function init() { return; } - $adminCustomOrders = json_decode($this->config->getSystemValueString('apporder', '[]'), true, flags:JSON_THROW_ON_ERROR); - $forceAdminOrder = $this->config->getSystemValueBool('apporderForce', false); if ($this->userSession->isLoggedIn()) { $user = $this->userSession->getUser(); $apps = $this->appManager->getEnabledAppsForUser($user); - if ($forceAdminOrder) { - $customOrders = []; - } else { - $customOrders = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR); - } + $customOrders = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR); } else { $apps = $this->appManager->getInstalledApps(); $customOrders = []; @@ -325,7 +319,7 @@ private function init() { } $l = $this->l10nFac->get($app); $id = $nav['id'] ?? $app . ($key === 0 ? '' : $key); - $order = $customOrders[$app][$key] ?? $adminCustomOrders[$app][$key] ?? $nav['order'] ?? 100; + $order = $customOrders[$app][$key] ?? $nav['order'] ?? 100; $type = $nav['type']; $route = !empty($nav['route']) ? $this->urlGenerator->linkToRoute($nav['route']) : ''; $icon = isset($nav['icon']) ? $nav['icon'] : 'app.svg'; From a4a3d94f058864d188e4daaa300f834ba3cba380 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Sep 2023 14:40:14 +0200 Subject: [PATCH 4/7] Default to first application if no default app is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/App/AppManager.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 88044fbf7b6f5..9d07000c6f8fd 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -825,13 +825,26 @@ public function getDefaultEnabledApps():array { public function getDefaultAppForUser(?IUser $user = null): string { // Set fallback to always-enabled files app $appId = 'files'; - $defaultApps = explode(',', $this->config->getSystemValueString('defaultapp', 'dashboard,files')); + $defaultApps = explode(',', $this->config->getSystemValueString('defaultapp', '')); $user ??= $this->userSession->getUser(); if ($user !== null) { $userDefaultApps = explode(',', $this->config->getUserValue($user->getUID(), 'core', 'defaultapp')); $defaultApps = array_filter(array_merge($userDefaultApps, $defaultApps)); + if (empty($defaultApps)) { + /* Fallback on user defined apporder */ + $customOrders = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR); + if (!empty($customOrders)) { + $customOrders = array_map('min', $customOrders); + asort($customOrders); + $defaultApps = array_keys($customOrders); + } + } + } + + if (empty($defaultApps)) { + $defaultApps = ['dashboard','files']; } // Find the first app that is enabled for the current user From 2d28e8110fd8a5b01d1d540f1d1cf42f117c3bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Sep 2023 14:59:05 +0200 Subject: [PATCH 5/7] Fix and extend tests for AppManager::getDefaultAppForUser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/App/AppManagerTest.php | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php index 3bf2195499f85..73ac7b7990920 100644 --- a/tests/lib/App/AppManagerTest.php +++ b/tests/lib/App/AppManagerTest.php @@ -607,21 +607,43 @@ public function provideDefaultApps(): array { // none specified, default to files [ '', + '', + '{}', 'files', ], // unexisting or inaccessible app specified, default to files [ 'unexist', + '', + '{}', 'files', ], // non-standard app [ 'settings', + '', + '{}', 'settings', ], // non-standard app with fallback [ 'unexist,settings', + '', + '{}', + 'settings', + ], + // user-customized defaultapp + [ + 'unexist,settings', + 'files', + '{"settings":[1],"files":[2]}', + 'files', + ], + // user-customized apporder fallback + [ + '', + '', + '{"settings":[1],"files":[2]}', 'settings', ], ]; @@ -630,7 +652,7 @@ public function provideDefaultApps(): array { /** * @dataProvider provideDefaultApps */ - public function testGetDefaultAppForUser($defaultApps, $expectedApp) { + public function testGetDefaultAppForUser($defaultApps, $userDefaultApps, $userApporder, $expectedApp) { $user = $this->newUser('user1'); $this->userSession->expects($this->once()) @@ -642,10 +664,12 @@ public function testGetDefaultAppForUser($defaultApps, $expectedApp) { ->with('defaultapp', $this->anything()) ->willReturn($defaultApps); - $this->config->expects($this->once()) + $this->config->expects($this->atLeastOnce()) ->method('getUserValue') - ->with('user1', 'core', 'defaultapp') - ->willReturn(''); + ->willReturnMap([ + ['user1', 'core', 'defaultapp', '', $userDefaultApps], + ['user1', 'core', 'apporder', '[]', $userApporder], + ]); $this->assertEquals($expectedApp, $this->manager->getDefaultAppForUser()); } From b11eb9e6d6551dda57b5b8c65a5666c63c4e02c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Sep 2023 16:21:22 +0200 Subject: [PATCH 6/7] Fix and extend NavigationManagerTest.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/NavigationManagerTest.php | 84 +++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 5 deletions(-) diff --git a/tests/lib/NavigationManagerTest.php b/tests/lib/NavigationManagerTest.php index d5c827fe1cb7c..65289799d5941 100644 --- a/tests/lib/NavigationManagerTest.php +++ b/tests/lib/NavigationManagerTest.php @@ -215,6 +215,10 @@ public function testWithAppManager($expected, $navigation, $isAdmin = false) { return vsprintf($text, $parameters); }); + /* Return default value */ + $this->config->method('getUserValue') + ->willReturnArgument(3); + $this->appManager->expects($this->any()) ->method('isEnabledForUser') ->with('theming') @@ -417,12 +421,82 @@ public function providesNavigationConfig() { ], 'no admin' => [ $defaults, - ['navigations' => [[ - '@attributes' => ['role' => 'admin'], - 'route' => 'test.page.index', - 'name' => 'Test' - ]]] + ['navigations' => [ + 'navigation' => [ + ['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test'] + ], + ]], ] ]; } + + public function testWithAppManagerAndApporder() { + $l = $this->createMock(IL10N::class); + $l->expects($this->any())->method('t')->willReturnCallback(function ($text, $parameters = []) { + return vsprintf($text, $parameters); + }); + + $testOrder = 12; + $expected = [ + 'test' => [ + 'type' => 'link', + 'id' => 'test', + 'order' => $testOrder, + 'href' => '/apps/test/', + 'name' => 'Test', + 'icon' => '/apps/test/img/app.svg', + 'active' => false, + 'classes' => '', + 'unread' => 0, + ], + ]; + $navigation = ['navigations' => [ + 'navigation' => [ + ['route' => 'test.page.index', 'name' => 'Test'] + ], + ]]; + + $this->config->method('getUserValue') + ->willReturnCallback( + function (string $userId, string $appName, string $key, mixed $default = '') use ($testOrder) { + $this->assertEquals('user001', $userId); + if ($key === 'apporder') { + return json_encode(['test' => [$testOrder]]); + } + return $default; + } + ); + + $this->appManager->expects($this->any()) + ->method('isEnabledForUser') + ->with('theming') + ->willReturn(true); + $this->appManager->expects($this->once())->method('getAppInfo')->with('test')->willReturn($navigation); + $this->l10nFac->expects($this->any())->method('get')->willReturn($l); + $this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function ($appName, $file) { + return "/apps/$appName/img/$file"; + }); + $this->urlGenerator->expects($this->any())->method('linkToRoute')->willReturnCallback(function ($route) { + if ($route === 'core.login.logout') { + return 'https://example.com/logout'; + } + return '/apps/test/'; + }); + $user = $this->createMock(IUser::class); + $user->expects($this->any())->method('getUID')->willReturn('user001'); + $this->userSession->expects($this->any())->method('getUser')->willReturn($user); + $this->userSession->expects($this->any())->method('isLoggedIn')->willReturn(true); + $this->appManager->expects($this->any()) + ->method('getEnabledAppsForUser') + ->with($user) + ->willReturn(['test']); + $this->groupManager->expects($this->any())->method('isAdmin')->willReturn(false); + $subadmin = $this->createMock(SubAdmin::class); + $subadmin->expects($this->any())->method('isSubAdmin')->with($user)->willReturn(false); + $this->groupManager->expects($this->any())->method('getSubAdmin')->willReturn($subadmin); + + $this->navigationManager->clear(); + $entries = $this->navigationManager->getAll(); + $this->assertEquals($expected, $entries); + } } From 8049702413f4422ea299733c8f69b852732edcb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 28 Sep 2023 09:22:54 +0200 Subject: [PATCH 7/7] Fix behavior when defaultapp is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/App/AppManager.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 9d07000c6f8fd..ccbb2143133d2 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -826,6 +826,7 @@ public function getDefaultAppForUser(?IUser $user = null): string { // Set fallback to always-enabled files app $appId = 'files'; $defaultApps = explode(',', $this->config->getSystemValueString('defaultapp', '')); + $defaultApps = array_filter($defaultApps); $user ??= $this->userSession->getUser();