Merge pull request #63104 from nextcloud/feat/app-menu-order-by-topic

App menu sorting improvement

Author: susnux

lib/private/NavigationManager.php

@@ -28,6 +28,32 @@
  * @psalm-import-type NavigationEntryOutput from INavigationManager
  */
 class NavigationManager implements INavigationManager {
+	/**
+	 * Default app menu order, grouped by topic with four apps per row.
+	 * Apps ship very different orders in their info.xml, so the values are
+	 * negative to keep this group in front of everything else.
+	 * Apps that are not listed keep their own order.
+	 */
+	private const DEFAULT_APP_ORDER = [
+		// Basics
+		'dashboard' => -100,
+		'files' => -99,
+		'office' => -98,
+		'photos' => -97,
+		// Collaboration
+		'spreed' => -96,
+		'mail' => -95,
+		'calendar' => -94,
+		'contacts' => -93,
+		// Productivity
+		'deck' => -92,
+		'collectives' => -91,
+		'tables' => -90,
+		'circles' => -89,
+		// All other apps follow, starting with Activity
+		'activity' => -88,
+	];
+
 	/** @var array<string, NavigationEntryOutput> */
 	protected array $entries = [];
 	/** @var list<callable(): NavigationEntry> */
@@ -80,8 +106,11 @@ public function add(array|callable $entry): void {
 				$entry['app'] = $id;
 			}
 
-			// Set order from user defined app order
-			$entry['order'] = (int)($this->customAppOrder[$id]['order'] ?? $entry['order'] ?? 100);
+			// Set order from user defined app order, then the default app order.
+			// The default order is skipped for users that sorted the apps themselves,
+			// so a newly installed app does not jump to the front of their order.
+			$defaultOrder = $this->customAppOrder === [] ? (self::DEFAULT_APP_ORDER[$id] ?? null) : null;
+			$entry['order'] = (int)($this->customAppOrder[$id]['order'] ?? $defaultOrder ?? $entry['order'] ?? 100);
 		}
 
 		$this->entries[$id] = $entry;

tests/lib/NavigationManagerTest.php

@@ -480,6 +480,53 @@ function (string $userId, string $appName, string $key, mixed $default = '') use
 		$this->assertEquals($expected, $entries);
 	}
 
+	/**
+	 * Known apps get a default order, all other apps keep the order from their info.xml.
+	 */
+	public function testDefaultAppOrder(): void {
+		$this->userSession->method('isLoggedIn')->willReturn(false);
+		$this->appManager->method('getEnabledApps')->willReturn([]);
+		$this->appManager->method('isEnabledForUser')->willReturn(true);
+
+		// order as shipped by the apps themselves
+		$apps = ['circles' => 80, 'activity' => 1, 'other' => 2, 'spreed' => -5, 'files' => 0, 'dashboard' => -10];
+		foreach ($apps as $id => $order) {
+			$this->navigationManager->add(['id' => $id, 'name' => $id, 'href' => '/', 'order' => $order]);
+		}
+
+		$this->assertSame(
+			['dashboard', 'files', 'spreed', 'circles', 'activity', 'other'],
+			array_keys($this->navigationManager->getAll()),
+		);
+	}
+
+	/**
+	 * Users that sorted the apps themselves keep their order, also for apps they never sorted.
+	 */
+	public function testDefaultAppOrderIsSkippedForCustomOrder(): void {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('user001');
+		$this->userSession->method('getUser')->willReturn($user);
+		$this->userSession->method('isLoggedIn')->willReturn(true);
+		$this->appManager->method('getEnabledAppsForUser')->willReturn([]);
+		$this->appManager->method('isEnabledForUser')->willReturn(true);
+		$this->config->method('getUserValue')
+			->willReturnCallback(static function (string $userId, string $appName, string $key, mixed $default = '') {
+				return $key === 'apporder' ? json_encode(['other' => ['app' => 'other', 'order' => 0]]) : $default;
+			});
+
+		// `circles` is not part of the user order, so it keeps the order from its info.xml
+		// instead of moving to the front of the user order
+		foreach (['other' => 2, 'circles' => 80] as $id => $order) {
+			$this->navigationManager->add(['id' => $id, 'name' => $id, 'href' => '/', 'order' => $order]);
+		}
+
+		$this->assertSame(
+			['other', 'circles'],
+			array_keys($this->navigationManager->getAll()),
+		);
+	}
+
 	/**
 	 * Navigation entries of enabled apps that are not booted yet must not be resolved.
 	 */