Merge pull request #40617 from nextcloud/enh/allow-user-to-set-apporder

Read apporder from configuration value
nextcloud · Sep 28, 2023 · c9ed1e9 · c9ed1e9
2 parents 99e287b + 8049702
commit c9ed1e9
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 12 deletions.
diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
@@ -825,13 +825,27 @@ 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', ''));
+		$defaultApps = array_filter($defaultApps);
 
 		$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

diff --git 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';

diff --git 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());
 	}

diff --git 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);
+	}
 }