From ff318138a2754e038ee032cf1d745c442921fc73 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 6 May 2025 19:39:17 +0200 Subject: [PATCH] refactor(updatenotification): use `OCP\ServerVersion` Signed-off-by: Ferdinand Thiessen --- .../updatenotification/lib/Settings/Admin.php | 9 +- .../tests/Settings/AdminTest.php | 104 +++++++++--------- 2 files changed, 54 insertions(+), 59 deletions(-) diff --git a/apps/updatenotification/lib/Settings/Admin.php b/apps/updatenotification/lib/Settings/Admin.php index f2115bd42ee..22228f1bccc 100644 --- a/apps/updatenotification/lib/Settings/Admin.php +++ b/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 $groupIds + * @param string[] $groupIds * @return list */ protected function getSelectedGroups(array $groupIds): array { diff --git a/apps/updatenotification/tests/Settings/AdminTest.php b/apps/updatenotification/tests/Settings/AdminTest.php index 3652c8f9081..d228c29f119 100644 --- a/apps/updatenotification/tests/Settings/AdminTest.php +++ b/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()); }