Merge pull request #44073 from nextcloud/fix/provide-correct-icon

fix(AppManager): Allow to query dark **or** bright icon

4 files changed, 44 insertions, 16 deletions

diff --git a/apps/updatenotification/lib/Notification/AppUpdateNotifier.php b/apps/updatenotification/lib/Notification/AppUpdateNotifier.php
index d5509bfcd75..8fb8047aac7 100644
--- a/apps/updatenotification/lib/Notification/AppUpdateNotifier.php
+++ b/apps/updatenotification/lib/Notification/AppUpdateNotifier.php
@@ -84,9 +84,9 @@ class AppUpdateNotifier implements INotifier {
 		// 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 5dec4e7ccde..c6e6ba2975e 100644
--- a/lib/private/App/AppManager.php
+++ b/lib/private/App/AppManager.php
@@ -109,8 +109,8 @@ class AppManager implements IAppManager {
 	) {
 	}
 
-	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 968771388dc..142e8bb8515 100644
--- a/lib/public/App/IAppManager.php
+++ b/lib/public/App/IAppManager.php
@@ -65,10 +65,11 @@ interface IAppManager {
 	 * 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 09a64f84469..dfbaedff957 100644
--- a/tests/lib/App/AppManagerTest.php
+++ b/tests/lib/App/AppManagerTest.php
@@ -135,12 +135,16 @@ class AppManagerTest extends TestCase {
 	/**
 	 * @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 @@ class AppManagerTest extends TestCase {
 		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',
+			],
 		];
 	}