refactor(updatenotification): use `OCP\ServerVersion`

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

apps/updatenotification/lib/Settings/Admin.php

@@ -18,9 +18,9 @@ use OCP\IDateTimeFormatter;
 use OCP\IGroupManager;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
+use OCP\ServerVersion;
 use OCP\Settings\ISettings;
 use OCP\Support\Subscription\IRegistry;
-use OCP\Util;
 use Psr\Log\LoggerInterface;
 
 class Admin implements ISettings {
@@ -35,6 +35,7 @@ class Admin implements ISettings {
 		private IUserManager $userManager,
 		private LoggerInterface $logger,
 		private IInitialState $initialState,
+		private ServerVersion $serverVersion,
 	) {
 	}
 
@@ -48,14 +49,14 @@ class Admin implements ISettings {
 			'stable',
 			'production',
 		];
-		$currentChannel = Util::getChannel();
+		$currentChannel = $this->serverVersion->getChannel();
 		if ($currentChannel === 'git') {
 			$channels[] = 'git';
 		}
 
 		$updateState = $this->updateChecker->getUpdateState();
 
-		$notifyGroups = json_decode($this->config->getAppValue('updatenotification', 'notify_groups', '["admin"]'), true);
+		$notifyGroups = $this->appConfig->getValueArray(Application::APP_NAME, 'notify_groups', ['admin']);
 
 		$defaultUpdateServerURL = 'https://updates.nextcloud.com/updater_server/';
 		$updateServerURL = $this->config->getSystemValue('updater.server.url', $defaultUpdateServerURL);
@@ -113,7 +114,7 @@ class Admin implements ISettings {
 	}
 
 	/**
-	 * @param list<string> $groupIds
+	 * @param string[] $groupIds
 	 * @return list<array{id: string, displayname: string}>
 	 */
 	protected function getSelectedGroups(array $groupIds): array {

apps/updatenotification/tests/Settings/AdminTest.php

@@ -4,10 +4,12 @@ declare(strict_types=1);
 
 /**
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
+ * SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 namespace OCA\UpdateNotification\Tests\Settings;
 
+use OCA\UpdateNotification\AppInfo\Application;
 use OCA\UpdateNotification\Settings\Admin;
 use OCA\UpdateNotification\UpdateChecker;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -20,34 +22,27 @@ use OCP\IGroupManager;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\L10N\ILanguageIterator;
+use OCP\ServerVersion;
 use OCP\Support\Subscription\IRegistry;
-use OCP\Util;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class AdminTest extends TestCase {
-	/** @var IFactory|\PHPUnit\Framework\MockObject\MockObject */
-	protected $l10nFactory;
-	/** @var Admin */
-	private $admin;
-	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
-	private $config;
-	/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
-	private $appConfig;
-	/** @var UpdateChecker|\PHPUnit\Framework\MockObject\MockObject */
-	private $updateChecker;
-	/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
-	private $groupManager;
-	/** @var IDateTimeFormatter|\PHPUnit\Framework\MockObject\MockObject */
-	private $dateTimeFormatter;
-	/** @var IRegistry|\PHPUnit\Framework\MockObject\MockObject */
-	private $subscriptionRegistry;
-	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
-	private $userManager;
-	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
-	private $logger;
-	/** IInitialState|\PHPUnit\Framework\MockObject\MockObject */
-	private $initialState;
+
+	private Admin $admin;
+
+	private IFactory&MockObject $l10nFactory;
+	private IConfig&MockObject $config;
+	private IAppConfig&MockObject $appConfig;
+	private UpdateChecker&MockObject $updateChecker;
+	private IGroupManager&MockObject $groupManager;
+	private IDateTimeFormatter&MockObject $dateTimeFormatter;
+	private IRegistry&MockObject $subscriptionRegistry;
+	private IUserManager&MockObject $userManager;
+	private LoggerInterface&MockObject $logger;
+	private IInitialState&MockObject $initialState;
+	private ServerVersion&MockObject $serverVersion;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -62,6 +57,7 @@ class AdminTest extends TestCase {
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->initialState = $this->createMock(IInitialState::class);
+		$this->serverVersion = $this->createMock(ServerVersion::class);
 
 		$this->admin = new Admin(
 			$this->config,
@@ -73,11 +69,15 @@ class AdminTest extends TestCase {
 			$this->subscriptionRegistry,
 			$this->userManager,
 			$this->logger,
-			$this->initialState
+			$this->initialState,
+			$this->serverVersion,
 		);
 	}
 
 	public function testGetFormWithUpdate(): void {
+		$this->serverVersion->expects(self::atLeastOnce())
+			->method('getChannel')
+			->willReturn('daily');
 		$this->userManager
 			->expects($this->once())
 			->method('countUsersTotal')
@@ -88,20 +88,16 @@ class AdminTest extends TestCase {
 			'stable',
 			'production',
 		];
-		$currentChannel = Util::getChannel();
-		if ($currentChannel === 'git') {
-			$channels[] = 'git';
-		}
 		$this->appConfig
 			->expects($this->once())
 			->method('getValueInt')
 			->with('core', 'lastupdatedat', 0)
 			->willReturn(12345);
-		$this->config
+		$this->appConfig
 			->expects($this->once())
-			->method('getAppValue')
-			->with('updatenotification', 'notify_groups', '["admin"]')
-			->willReturn('["admin"]');
+			->method('getValueArray')
+			->with(Application::APP_NAME, 'notify_groups', ['admin'])
+			->willReturn(['admin']);
 		$this->config
 			->method('getSystemValue')
 			->willReturnMap([
@@ -154,7 +150,7 @@ class AdminTest extends TestCase {
 				'isNewVersionAvailable' => true,
 				'isUpdateChecked' => true,
 				'lastChecked' => 'LastCheckedReturnValue',
-				'currentChannel' => Util::getChannel(),
+				'currentChannel' => 'daily',
 				'channels' => $channels,
 				'newVersion' => '8.1.2',
 				'newVersionString' => 'Nextcloud 8.1.2',
@@ -172,11 +168,14 @@ class AdminTest extends TestCase {
 				'hasValidSubscription' => true,
 			]);
 
-		$expected = new TemplateResponse('updatenotification', 'admin', [], '');
+		$expected = new TemplateResponse(Application::APP_NAME, 'admin', [], '');
 		$this->assertEquals($expected, $this->admin->getForm());
 	}
 
 	public function testGetFormWithUpdateAndChangedUpdateServer(): void {
+		$this->serverVersion->expects(self::atLeastOnce())
+			->method('getChannel')
+			->willReturn('beta');
 		$this->userManager
 			->expects($this->once())
 			->method('countUsersTotal')
@@ -187,10 +186,6 @@ class AdminTest extends TestCase {
 			'stable',
 			'production',
 		];
-		$currentChannel = Util::getChannel();
-		if ($currentChannel === 'git') {
-			$channels[] = 'git';
-		}
 
 		$this->appConfig
 			->expects($this->once())
@@ -202,11 +197,11 @@ class AdminTest extends TestCase {
 			->method('getSystemValueBool')
 			->with('updatechecker', true)
 			->willReturn(true);
-		$this->config
+		$this->appConfig
 			->expects($this->once())
-			->method('getAppValue')
-			->with('updatenotification', 'notify_groups', '["admin"]')
-			->willReturn('["admin"]');
+			->method('getValueArray')
+			->with(Application::APP_NAME, 'notify_groups', ['admin'])
+			->willReturn(['admin']);
 		$this->config
 			->method('getSystemValue')
 			->willReturnMap([
@@ -254,7 +249,7 @@ class AdminTest extends TestCase {
 				'isNewVersionAvailable' => true,
 				'isUpdateChecked' => true,
 				'lastChecked' => 'LastCheckedReturnValue',
-				'currentChannel' => Util::getChannel(),
+				'currentChannel' => 'beta',
 				'channels' => $channels,
 				'newVersion' => '8.1.2',
 				'newVersionString' => 'Nextcloud 8.1.2',
@@ -272,11 +267,14 @@ class AdminTest extends TestCase {
 				'hasValidSubscription' => true,
 			]);
 
-		$expected = new TemplateResponse('updatenotification', 'admin', [], '');
+		$expected = new TemplateResponse(Application::APP_NAME, 'admin', [], '');
 		$this->assertEquals($expected, $this->admin->getForm());
 	}
 
 	public function testGetFormWithUpdateAndCustomersUpdateServer(): void {
+		$this->serverVersion->expects(self::atLeastOnce())
+			->method('getChannel')
+			->willReturn('production');
 		$this->userManager
 			->expects($this->once())
 			->method('countUsersTotal')
@@ -287,10 +285,6 @@ class AdminTest extends TestCase {
 			'stable',
 			'production',
 		];
-		$currentChannel = Util::getChannel();
-		if ($currentChannel === 'git') {
-			$channels[] = 'git';
-		}
 
 		$this->appConfig
 			->expects($this->once())
@@ -302,11 +296,11 @@ class AdminTest extends TestCase {
 			->method('getSystemValueBool')
 			->with('updatechecker', true)
 			->willReturn(true);
-		$this->config
-			->expects($this->once())
-			->method('getAppValue')
-			->with('updatenotification', 'notify_groups', '["admin"]')
-			->willReturn('["admin"]');
+		$this->appConfig
+			->expects(self::once())
+			->method('getValueArray')
+			->with(Application::APP_NAME, 'notify_groups', ['admin'])
+			->willReturn(['admin']);
 		$this->config
 			->method('getSystemValue')
 			->willReturnMap([
@@ -354,7 +348,7 @@ class AdminTest extends TestCase {
 				'isNewVersionAvailable' => true,
 				'isUpdateChecked' => true,
 				'lastChecked' => 'LastCheckedReturnValue',
-				'currentChannel' => Util::getChannel(),
+				'currentChannel' => 'production',
 				'channels' => $channels,
 				'newVersion' => '8.1.2',
 				'newVersionString' => 'Nextcloud 8.1.2',
@@ -372,7 +366,7 @@ class AdminTest extends TestCase {
 				'hasValidSubscription' => true,
 			]);
 
-		$expected = new TemplateResponse('updatenotification', 'admin', [], '');
+		$expected = new TemplateResponse(Application::APP_NAME, 'admin', [], '');
 		$this->assertEquals($expected, $this->admin->getForm());
 	}