From c8d7a5acaabb5c2f8bae1ccb70ce9c76cfd86c5c Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Fri, 8 Mar 2024 01:05:24 +0100
Subject: [PATCH] fix(AppManager): Allow to query dark **or** bright icon

The navigation needs the bright icon, while the notifications and activity need a dark icon.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 .../lib/Notification/AppUpdateNotifier.php    |  4 +-
 lib/private/App/AppManager.php                |  4 +-
 lib/public/App/IAppManager.php                |  3 +-
 tests/lib/App/AppManagerTest.php              | 49 ++++++++++++++-----
 4 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/apps/updatenotification/lib/Notification/AppUpdateNotifier.php b/apps/updatenotification/lib/Notification/AppUpdateNotifier.php
index d5509bfcd7581..8fb8047aac75e 100644
--- a/apps/updatenotification/lib/Notification/AppUpdateNotifier.php
+++ b/apps/updatenotification/lib/Notification/AppUpdateNotifier.php
@@ -84,9 +84,9 @@ public function prepare(INotification $notification, string $languageCode): INot
 		// Prepare translation factory for requested language
 		$l = $this->l10nFactory->get(Application::APP_NAME, $languageCode);
 		
-		$icon = $this->appManager->getAppIcon($appId);
+		$icon = $this->appManager->getAppIcon($appId, true);
 		if ($icon === null) {
-			$icon = $this->urlGenerator->imagePath('core', 'default-app-icon');
+			$icon = $this->urlGenerator->imagePath('core', 'actions/change.svg');
 		}
 
 		$action = $notification->createAction();
diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
index 5dec4e7ccde34..c6e6ba2975eca 100644
--- a/lib/private/App/AppManager.php
+++ b/lib/private/App/AppManager.php
@@ -109,8 +109,8 @@ public function __construct(
 	) {
 	}
 
-	public function getAppIcon(string $appId): ?string {
-		$possibleIcons = [$appId . '.svg', 'app.svg', $appId . '-dark.svg', 'app-dark.svg'];
+	public function getAppIcon(string $appId, bool $dark = false): ?string {
+		$possibleIcons = $dark ? [$appId . '-dark.svg', 'app-dark.svg'] : [$appId . '.svg', 'app.svg'];
 		$icon = null;
 		foreach ($possibleIcons as $iconName) {
 			try {
diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
index 968771388dce8..142e8bb8515ef 100644
--- a/lib/public/App/IAppManager.php
+++ b/lib/public/App/IAppManager.php
@@ -65,10 +65,11 @@ public function getAppVersion(string $appId, bool $useCache = true): string;
 	 * Returns the app icon or null if none is found
 	 *
 	 * @param string $appId
+	 * @param bool $dark Enable to request a dark icon variant, default is a white icon
 	 * @return string|null
 	 * @since 29.0.0
 	 */
-	public function getAppIcon(string $appId): string|null;
+	public function getAppIcon(string $appId, bool $dark = false): string|null;
 
 	/**
 	 * Check if an app is enabled for user
diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php
index 09a64f84469dc..dfbaedff95793 100644
--- a/tests/lib/App/AppManagerTest.php
+++ b/tests/lib/App/AppManagerTest.php
@@ -135,12 +135,16 @@ protected function setUp(): void {
 	/**
 	 * @dataProvider dataGetAppIcon
 	 */
-	public function testGetAppIcon($callback, string|null $expected) {
+	public function testGetAppIcon($callback, ?bool $dark, string|null $expected) {
 		$this->urlGenerator->expects($this->atLeastOnce())
 			->method('imagePath')
 			->willReturnCallback($callback);
 
-		$this->assertEquals($expected, $this->manager->getAppIcon('test'));
+		if ($dark !== null) {
+			$this->assertEquals($expected, $this->manager->getAppIcon('test', $dark));
+		} else {
+			$this->assertEquals($expected, $this->manager->getAppIcon('test'));
+		}
 	}
 
 	public function dataGetAppIcon(): array {
@@ -162,36 +166,59 @@ public function dataGetAppIcon(): array {
 		return [
 			'does not find anything' => [
 				$nothing,
+				false,
+				null,
+			],
+			'nothing if request dark but only bright available' => [
+				$createCallback(['app.svg']),
+				true,
 				null,
 			],
-			'only app.svg' => [
+			'nothing if request bright but only dark available' => [
+				$createCallback(['app-dark.svg']),
+				false,
+				null,
+			],
+			'bright and only app.svg' => [
 				$createCallback(['app.svg']),
+				false,
 				'/path/app.svg',
 			],
-			'only app-dark.svg' => [
+			'dark and only app-dark.svg' => [
 				$createCallback(['app-dark.svg']),
+				true,
 				'/path/app-dark.svg',
 			],
-			'only appname -dark.svg' => [
+			'dark only appname -dark.svg' => [
 				$createCallback(['test-dark.svg']),
+				true,
 				'/path/test-dark.svg',
 			],
-			'only appname.svg' => [
+			'bright and only appname.svg' => [
 				$createCallback(['test.svg']),
+				false,
 				'/path/test.svg',
 			],
 			'priotize custom over default' => [
 				$createCallback(['app.svg', 'test.svg']),
+				false,
 				'/path/test.svg',
 			],
-			'priotize default over dark' => [
-				$createCallback(['test-dark.svg', 'app-dark.svg', 'app.svg']),
-				'/path/app.svg',
+			'defaults to bright' => [
+				$createCallback(['test-dark.svg', 'test.svg']),
+				null,
+				'/path/test.svg',
 			],
-			'priotize custom over default' => [
-				$createCallback(['test.svg', 'test-dark.svg', 'app-dark.svg']),
+			'no dark icon on default' => [
+				$createCallback(['test-dark.svg', 'test.svg', 'app-dark.svg', 'app.svg']),
+				false,
 				'/path/test.svg',
 			],
+			'no bright icon on dark' => [
+				$createCallback(['test-dark.svg', 'test.svg', 'app-dark.svg', 'app.svg']),
+				true,
+				'/path/test-dark.svg',
+			],
 		];
 	}