fix(AppManager): Ensure that app manager can handle empty app keys on…

… the navigation entries Signed-off-by: Ferdinand Thiessen <[email protected]>
nextcloud · Nov 10, 2023 · b3da564 · b3da564
1 parent 4b0e7b6
commit b3da564
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 6 deletions.
diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
@@ -838,10 +838,12 @@ public function getDefaultAppForUser(?IUser $user = null, bool $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)) {
-					uasort($customOrders, function ($a, $b) {
-						return $a['order'] - $b['order'];
-					});
-					$defaultApps = array_keys($customOrders);
+					// filter only entries with app key (when added using closures or NavigationManager::add the app is not guranteed to be set)
+					$customOrders = array_filter($customOrders, fn ($entry) => isset($entry['app']));
+					// sort apps by order
+					usort($customOrders, fn ($a, $b) => $a['order'] - $b['order']);
+					// set default apps to sorted apps
+					$defaultApps = array_map(fn ($entry) => $entry['app'], $customOrders);
 				}
 			}
 		}

diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
@@ -250,6 +250,8 @@ public function getAppRestriction(string $appId): array;
 	 *
 	 * @param ?IUser $user User to query default app for
 	 * @param bool $withFallbacks Include fallback values if no default app was configured manually
+	 *                            Before falling back to predefined default apps,
+	 *                            the user defined app order is considered and the first app would be used as the fallback.
 	 *
 	 * @since 25.0.6
 	 * @since 28.0.0 Added optional $withFallbacks parameter

diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php
@@ -660,6 +660,17 @@ public function provideDefaultApps(): array {
 				true,
 				'settings',
 			],
+			// system default app and user apporder
+			[
+				// system default is settings
+				'unexist,settings',
+				'',
+				// apporder says default app is files (order is lower)
+				'{"files_id":{"app":"files","order":1},"settings_id":{"app":"settings","order":2}}',
+				true,
+				// system default should override apporder
+				'settings'
+			],
 			// user-customized defaultapp
 			[
 				'',
@@ -680,18 +691,26 @@ public function provideDefaultApps(): array {
 			[
 				'unexist,settings',
 				'files',
-				'{"settings":{"app":"settings","order":1},"files":{"app":"files","order":2}}',
+				'{"settings_id":{"app":"settings","order":1},"files_id":{"app":"files","order":2}}',
 				true,
 				'files',
 			],
 			// user-customized apporder fallback
 			[
 				'',
 				'',
-				'{"settings":{"app":"settings","order":1},"files":{"app":"files","order":2}}',
+				'{"settings_id":{"app":"settings","order":1},"files":{"app":"files","order":2}}',
 				true,
 				'settings',
 			],
+			// user-customized apporder fallback with missing app key (entries added by closures does not always have an app key set (Nextcloud 27 spreed app for example))
+			[
+				'',
+				'',
+				'{"spreed":{"order":1},"files":{"app":"files","order":2}}',
+				true,
+				'files',
+			],
 			// user-customized apporder, but called without fallback
 			[
 				'',
@@ -700,6 +719,14 @@ public function provideDefaultApps(): array {
 				false,
 				'',
 			],
+			// user-customized apporder with an app that has multiple routes
+			[
+				'',
+				'',
+				'{"settings_id":{"app":"settings","order":1},"settings_id_2":{"app":"settings","order":3},"id_files":{"app":"files","order":2}}',
+				true,
+				'settings',
+			],
 		];
 	}