Merge pull request #41341 from nextcloud/fix/apporder-on-closures

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

Author: blizzz

apps/theming/lib/Listener/BeforePreferenceListener.php

@@ -46,8 +46,10 @@ public function handle(Event $event): void {
 		}
 
 		switch ($event->getAppId()) {
-			case Application::APP_ID: $this->handleThemingValues($event); break;
-			case 'core': $this->handleCoreValues($event); break;
+			case Application::APP_ID: $this->handleThemingValues($event);
+				break;
+			case 'core': $this->handleCoreValues($event);
+				break;
 		}
 	}
 
@@ -78,8 +80,8 @@ private function handleCoreValues(BeforePreferenceSetEvent|BeforePreferenceDelet
 
 		$value = json_decode($event->getConfigValue(), true, flags:JSON_THROW_ON_ERROR);
 		if (is_array(($value))) {
-			foreach ($value as $appName => $order) {
-				if (!$this->appManager->isEnabledForUser($appName) || !is_array($order) || empty($order) || !is_numeric($order[key($order)])) {
+			foreach ($value as $id => $info) {
+				if (!is_array($info) || empty($info) || !isset($info['app']) || !$this->appManager->isEnabledForUser($info['app']) || !is_numeric($info['order'] ?? '')) {
 					// Invalid config value, refuse the change
 					return;
 				}

apps/theming/src/components/AppOrderSelector.vue

@@ -37,8 +37,7 @@ export interface IApp {
 	icon: string // path to the icon svg
 	label: string // display name
 	default?: boolean // force app as default app
-	app: string
-	key: number
+	app?: string
 }
 
 export default defineComponent({

apps/theming/src/components/UserAppMenuSection.vue

@@ -58,13 +58,11 @@ interface INavigationEntry {
 	/** Whether this is the default app */
 	default?: boolean
 	/** App that registered this navigation entry (not necessarly the same as the id) */
-	app: string
-	/** The key used to identify this entry in the navigations entries */
-	key: number
+	app?: string
 }
 
 /** The app order user setting */
-type IAppOrder = Record<string, Record<number, number>>
+type IAppOrder = Record<string, { order: number, app?: string }>
 
 /** OCS responses */
 interface IOCSResponse<T> {
@@ -131,8 +129,8 @@ export default defineComponent({
 		 */
 		const updateAppOrder = (value: IApp[]) => {
 			const order: IAppOrder = {}
-			value.forEach(({ app, key }, index) => {
-				order[app] = { ...order[app], [key]: index }
+			value.forEach(({ app, id }, index) => {
+				order[id] = { order: index, app }
 			})
 
 			saveSetting('apporder', order)

dist/theming-admin-theming.js

@@ -838,9 +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)) {
-					$customOrders = array_map('min', $customOrders);
-					asort($customOrders);
-					$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);
 				}
 			}
 		}

dist/theming-admin-theming.js.map

@@ -65,6 +65,10 @@ class NavigationManager implements INavigationManager {
 	private $groupManager;
 	/** @var IConfig */
 	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,
@@ -78,6 +82,8 @@ public function __construct(IAppManager $appManager,
 		$this->userSession = $userSession;
 		$this->groupManager = $groupManager;
 		$this->config = $config;
+
+		$this->defaultApp = null;
 	}
 
 	/**
@@ -102,6 +108,12 @@ public function add($entry) {
 
 		$id = $entry['id'];
 		$entry['unread'] = $this->unreadCounters[$id] ?? 0;
+		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;
 	}
@@ -218,6 +230,12 @@ private function init() {
 			]);
 		}
 
+		if ($this->appManager === 'null') {
+			return;
+		}
+
+		$this->defaultApp = $this->appManager->getDefaultAppForUser($this->userSession->getUser(), false);
+
 		if ($this->userSession->isLoggedIn()) {
 			// Profile
 			$this->add([
@@ -311,22 +329,15 @@ private function init() {
 			}
 		}
 
-		if ($this->appManager === 'null') {
-			return;
-		}
-
 		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 = [];
 		}
 
-		// 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())) {
 				continue;
@@ -352,7 +363,7 @@ private function init() {
 				}
 				$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';
@@ -382,12 +393,8 @@ private function init() {
 					// 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,
 				] : []
 				));
 			}

dist/theming-personal-theming.js

@@ -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