fix: Allow to set custom app order on navigation entries added by closures to NavigationManager

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

lib/private/App/AppManager.php

@@ -838,8 +838,9 @@ class AppManager implements IAppManager {
 				/* 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);
+					uasort($customOrders, function ($a, $b) {
+						return $a['order'] - $b['order'];
+					});
 					$defaultApps = array_keys($customOrders);
 				}
 			}

lib/private/NavigationManager.php

@@ -67,6 +67,8 @@ class NavigationManager implements INavigationManager {
 	private $config;
 	/** The default app for the current user (cached for the `add` function) */
 	private ?string $defaultApp;
+	/** User defined app order (cached for the `add` function) */
+	private array $customAppOrder;
 
 	public function __construct(IAppManager $appManager,
 						 IURLGenerator $urlGenerator,
@@ -106,8 +108,12 @@ class NavigationManager implements INavigationManager {
 
 		$id = $entry['id'];
 		$entry['unread'] = $this->unreadCounters[$id] ?? 0;
-		// This is the default app that will always be shown first
-		$entry['default'] = ($entry['app'] ?? false) === $this->defaultApp;
+		if ($entry['type'] === 'link') {
+			// This is the default app that will always be shown first
+			$entry['default'] = ($entry['app'] ?? false) === $this->defaultApp;
+			// Set order from user defined app order
+			$entry['order'] = $this->customAppOrder[$id]['order'] ?? $entry['order'] ?? 100;
+		}
 
 		$this->entries[$id] = $entry;
 	}
@@ -326,10 +332,10 @@ class NavigationManager implements INavigationManager {
 		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);
+			$this->customAppOrder = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR);
 		} else {
 			$apps = $this->appManager->getInstalledApps();
-			$customOrders = [];
+			$this->customAppOrder = [];
 		}
 
 		foreach ($apps as $app) {
@@ -357,7 +363,7 @@ class NavigationManager implements INavigationManager {
 				}
 				$l = $this->l10nFac->get($app);
 				$id = $nav['id'] ?? $app . ($key === 0 ? '' : $key);
-				$order = $customOrders[$app][$key] ?? $nav['order'] ?? 100;
+				$order = $nav['order'] ?? 100;
 				$type = $nav['type'];
 				$route = !empty($nav['route']) ? $this->urlGenerator->linkToRoute($nav['route']) : '';
 				$icon = $nav['icon'] ?? 'app.svg';

tests/lib/App/AppManagerTest.php

@@ -680,7 +680,7 @@ class AppManagerTest extends TestCase {
 			[
 				'unexist,settings',
 				'files',
-				'{"settings":[1],"files":[2]}',
+				'{"settings":{"app":"settings","order":1},"files":{"app":"files","order":2}}',
 				true,
 				'files',
 			],
@@ -688,7 +688,7 @@ class AppManagerTest extends TestCase {
 			[
 				'',
 				'',
-				'{"settings":[1],"files":[2]}',
+				'{"settings":{"app":"settings","order":1},"files":{"app":"files","order":2}}',
 				true,
 				'settings',
 			],
@@ -696,7 +696,7 @@ class AppManagerTest extends TestCase {
 			[
 				'',
 				'',
-				'{"settings":[1],"files":[2]}',
+				'{"settings":{"app":"settings","order":1},"files":{"app":"files","order":2}}',
 				false,
 				'',
 			],

tests/lib/NavigationManagerTest.php

@@ -365,7 +365,6 @@ class NavigationManagerTest extends TestCase {
 						'unread' => 0,
 						'default' => true,
 						'app' => 'test',
-						'key' => 0,
 					]],
 					['logout' => $defaults['logout']]
 				),
@@ -416,7 +415,6 @@ class NavigationManagerTest extends TestCase {
 						'unread' => 0,
 						'default' => false,
 						'app' => 'test',
-						'key' => 0,
 					],
 						'test1' => [
 							'id' => 'test1',
@@ -430,7 +428,6 @@ class NavigationManagerTest extends TestCase {
 							'unread' => 0,
 							'default' => true, // because of order
 							'app' => 'test',
-							'key' => 1,
 						]],
 					['logout' => $defaults['logout']]
 				),
@@ -458,7 +455,6 @@ class NavigationManagerTest extends TestCase {
 						'unread' => 0,
 						'default' => true,
 						'app' => 'test',
-						'key' => 0,
 					]],
 					['logout' => $defaults['logout']]
 				),
@@ -514,7 +510,6 @@ class NavigationManagerTest extends TestCase {
 				'unread' => 0,
 				'default' => true,
 				'app' => 'test',
-				'key' => 0,
 			],
 		];
 		$navigation = ['navigations' => [
@@ -528,7 +523,7 @@ class NavigationManagerTest extends TestCase {
 				function (string $userId, string $appName, string $key, mixed $default = '') use ($testOrder) {
 					$this->assertEquals('user001', $userId);
 					if ($key === 'apporder') {
-						return json_encode(['test' => [$testOrder]]);
+						return json_encode(['test' => ['app' => 'test', 'order' => $testOrder]]);
 					}
 					return $default;
 				}