feat(NavigationManager): Always sort the default app first

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

lib/private/App/AppManager.php

@@ -823,9 +823,9 @@ class AppManager implements IAppManager {
 		return $this->defaultEnabled;
 	}
 
-	public function getDefaultAppForUser(?IUser $user = null): string {
+	public function getDefaultAppForUser(?IUser $user = null, bool $withFallbacks = true): string {
 		// Set fallback to always-enabled files app
-		$appId = 'files';
+		$appId = $withFallbacks ? 'files' : '';
 		$defaultApps = explode(',', $this->config->getSystemValueString('defaultapp', ''));
 		$defaultApps = array_filter($defaultApps);
 
@@ -834,7 +834,7 @@ class AppManager implements IAppManager {
 		if ($user !== null) {
 			$userDefaultApps = explode(',', $this->config->getUserValue($user->getUID(), 'core', 'defaultapp'));
 			$defaultApps = array_filter(array_merge($userDefaultApps, $defaultApps));
-			if (empty($defaultApps)) {
+			if (empty($defaultApps) && $withFallbacks) {
 				/* Fallback on user defined apporder */
 				$customOrders = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR);
 				if (!empty($customOrders)) {
@@ -845,7 +845,7 @@ class AppManager implements IAppManager {
 			}
 		}
 
-		if (empty($defaultApps)) {
+		if (empty($defaultApps) && $withFallbacks) {
 			$defaultApps = ['dashboard','files'];
 		}
 

lib/private/NavigationManager.php

@@ -123,26 +123,44 @@ class NavigationManager implements INavigationManager {
 			});
 		}
 
-		return $this->proceedNavigation($result);
+		return $this->proceedNavigation($result, $type);
 	}
 
 	/**
-	 * Sort navigation entries by order, name and set active flag
+	 * Sort navigation entries default app is always sorted first, then by order, name and set active flag
 	 *
 	 * @param array $list
 	 * @return array
 	 */
-	private function proceedNavigation(array $list): array {
+	private function proceedNavigation(array $list, string $type): array {
 		uasort($list, function ($a, $b) {
-			if (isset($a['order']) && isset($b['order'])) {
+			if (($a['default'] ?? false) xor ($b['default'] ?? false)) {
+				// Always sort the default app first
+				return ($a['default'] ?? false) ? -1 : 1;
+			} elseif (isset($a['order']) && isset($b['order'])) {
+				// Sort by order
 				return ($a['order'] < $b['order']) ? -1 : 1;
 			} elseif (isset($a['order']) || isset($b['order'])) {
+				// Sort the one that has an order property first
 				return isset($a['order']) ? -1 : 1;
 			} else {
+				// Sort by name otherwise
 				return ($a['name'] < $b['name']) ? -1 : 1;
 			}
 		});
 
+		if ($type === 'all' || $type === 'link') {
+			// There might be the case that no default app was set, in this case the first app is the default app.
+			// Otherwise the default app is already the ordered first, so setting the default prop will make no difference.
+			foreach ($list as $index => &$navEntry) {
+				if ($navEntry['type'] === 'link') {
+					$navEntry['default'] = true;
+					break;
+				}
+			}
+			unset($navEntry);
+		}
+
 		$activeApp = $this->getActiveEntry();
 		if ($activeApp !== null) {
 			foreach ($list as $index => &$navEntry) {
@@ -293,6 +311,8 @@ class NavigationManager implements INavigationManager {
 			$customOrders = [];
 		}
 
+		// The default app of the current user without fallbacks
+		$defaultApp = $this->appManager->getDefaultAppForUser($this->userSession->getUser(), false);
 
 		foreach ($apps as $app) {
 			if (!$this->userSession->isLoggedIn() && !$this->appManager->isEnabledForUser($app, $this->userSession->getUser())) {
@@ -335,14 +355,28 @@ class NavigationManager implements INavigationManager {
 					$icon = $this->urlGenerator->imagePath('core', 'default-app-icon');
 				}
 
-				$this->add([
+				$this->add(array_merge([
+					// Navigation id
 					'id' => $id,
+					// Order where this entry should be shown
 					'order' => $order,
+					// Target of the navigation entry
 					'href' => $route,
+					// The icon used for the naviation entry
 					'icon' => $icon,
+					// Type of the navigation entry ('link' vs 'settings')
 					'type' => $type,
+					// Localized name of the navigation entry
 					'name' => $l->t($nav['name']),
-				]);
+				], $type === 'link' ? [
+					// This is the default app that will always be shown first
+					'default' => $defaultApp === $id,
+					// App that registered this navigation entry (not necessarly the same as the id)
+					'app' => $app,
+					// The key used to identify this entry in the navigations entries
+					'key' => $key,
+				] : []
+				));
 			}
 		}
 	}

lib/public/App/IAppManager.php

@@ -248,9 +248,13 @@ interface IAppManager {
 	 *
 	 * If `user` is not passed, the currently logged in user will be used
 	 *
+	 * @param ?IUser $user User to query default app for
+	 * @param bool $withFallbacks Include fallback values if no default app was configured manually
+	 *
 	 * @since 25.0.6
+	 * @since 28.0.0 Added optional $withFallbacks parameter
 	 */
-	public function getDefaultAppForUser(?IUser $user = null): string;
+	public function getDefaultAppForUser(?IUser $user = null, bool $withFallbacks = true): string;
 
 	/**
 	 * Get the global default apps with fallbacks

tests/lib/NavigationManagerTest.php

@@ -95,7 +95,7 @@ class NavigationManagerTest extends TestCase {
 					//'icon'	=> 'optional',
 					'href' => 'url',
 					'active' => true,
-					'unread' => 0
+					'unread' => 0,
 				],
 				'entry id2' => [
 					'id' => 'entry id',
@@ -106,7 +106,8 @@ class NavigationManagerTest extends TestCase {
 					'active' => false,
 					'type' => 'link',
 					'classes' => '',
-					'unread' => 0
+					'unread' => 0,
+					'default' => true,
 				]
 			]
 		];
@@ -349,7 +350,10 @@ class NavigationManagerTest extends TestCase {
 						'active' => false,
 						'type' => 'link',
 						'classes' => '',
-						'unread' => 0
+						'unread' => 0,
+						'default' => true,
+						'app' => 'test',
+						'key' => 0,
 					]],
 					['logout' => $defaults['logout']]
 				),
@@ -372,7 +376,7 @@ class NavigationManagerTest extends TestCase {
 						'active' => false,
 						'type' => 'settings',
 						'classes' => '',
-						'unread' => 0
+						'unread' => 0,
 					]],
 					['logout' => $defaults['logout']]
 				),
@@ -382,6 +386,47 @@ class NavigationManagerTest extends TestCase {
 					],
 				]]
 			],
+			'with-multiple' => [
+				array_merge(
+					['accessibility_settings' => $defaults['accessibility_settings']],
+					['settings' => $defaults['settings']],
+					['test' => [
+						'id' => 'test',
+						'order' => 100,
+						'href' => '/apps/test/',
+						'icon' => '/apps/test/img/app.svg',
+						'name' => 'Test',
+						'active' => false,
+						'type' => 'link',
+						'classes' => '',
+						'unread' => 0,
+						'default' => false,
+						'app' => 'test',
+						'key' => 0,
+					],
+						'test1' => [
+							'id' => 'test1',
+							'order' => 50,
+							'href' => '/apps/test/',
+							'icon' => '/apps/test/img/app.svg',
+							'name' => 'Other test',
+							'active' => false,
+							'type' => 'link',
+							'classes' => '',
+							'unread' => 0,
+							'default' => true, // because of order
+							'app' => 'test',
+							'key' => 1,
+						]],
+					['logout' => $defaults['logout']]
+				),
+				['navigations' => [
+					'navigation' => [
+						['route' => 'test.page.index', 'name' => 'Test'],
+						['route' => 'test.page.index', 'name' => 'Other test', 'order' => 50],
+					]
+				]]
+			],
 			'admin' => [
 				array_merge(
 					$adminSettings,
@@ -395,7 +440,10 @@ class NavigationManagerTest extends TestCase {
 						'active' => false,
 						'type' => 'link',
 						'classes' => '',
-						'unread' => 0
+						'unread' => 0,
+						'default' => true,
+						'app' => 'test',
+						'key' => 0,
 					]],
 					['logout' => $defaults['logout']]
 				),
@@ -448,6 +496,9 @@ class NavigationManagerTest extends TestCase {
 				'active' => false,
 				'classes' => '',
 				'unread' => 0,
+				'default' => true,
+				'app' => 'test',
+				'key' => 0,
 			],
 		];
 		$navigation = ['navigations' => [