From e8122f21c5bdba3e284931b3b076f5b127c062e0 Mon Sep 17 00:00:00 2001 From: szaimen Date: Fri, 11 Mar 2022 12:23:15 +0100 Subject: show warning in update admin overview Signed-off-by: szaimen Signed-off-by: nextcloud-command --- apps/updatenotification/lib/Settings/Admin.php | 51 +++++++- .../src/components/UpdateNotification.vue | 8 ++ .../tests/Settings/AdminTest.php | 128 ++++++++++++++++++++- 3 files changed, 185 insertions(+), 2 deletions(-) (limited to 'apps') diff --git a/apps/updatenotification/lib/Settings/Admin.php b/apps/updatenotification/lib/Settings/Admin.php index 880b43906eb..1ca0d83cb5d 100644 --- a/apps/updatenotification/lib/Settings/Admin.php +++ b/apps/updatenotification/lib/Settings/Admin.php @@ -29,6 +29,8 @@ declare(strict_types=1); */ namespace OCA\UpdateNotification\Settings; +use OC\User\Backend; +use OCP\User\Backend\ICountUsersBackend; use OCA\UpdateNotification\UpdateChecker; use OCP\AppFramework\Http\TemplateResponse; use OCP\IConfig; @@ -38,6 +40,8 @@ use OCP\L10N\IFactory; use OCP\Settings\ISettings; use OCP\Support\Subscription\IRegistry; use OCP\Util; +use OCP\IUserManager; +use Psr\Log\LoggerInterface; class Admin implements ISettings { /** @var IConfig */ @@ -52,6 +56,10 @@ class Admin implements ISettings { private $l10nFactory; /** @var IRegistry */ private $subscriptionRegistry; + /** @var IUserManager */ + private $userManager; + /** @var LoggerInterface */ + private $logger; public function __construct( IConfig $config, @@ -59,7 +67,9 @@ class Admin implements ISettings { IGroupManager $groupManager, IDateTimeFormatter $dateTimeFormatter, IFactory $l10nFactory, - IRegistry $subscriptionRegistry + IRegistry $subscriptionRegistry, + IUserManager $userManager, + LoggerInterface $logger ) { $this->config = $config; $this->updateChecker = $updateChecker; @@ -67,6 +77,8 @@ class Admin implements ISettings { $this->dateTimeFormatter = $dateTimeFormatter; $this->l10nFactory = $l10nFactory; $this->subscriptionRegistry = $subscriptionRegistry; + $this->userManager = $userManager; + $this->logger = $logger; } /** @@ -111,6 +123,7 @@ class Admin implements ISettings { 'downloadLink' => empty($updateState['downloadLink']) ? '' : $updateState['downloadLink'], 'changes' => $this->filterChanges($updateState['changes'] ?? []), 'webUpdaterEnabled' => !$this->config->getSystemValue('upgrade.disable-web', false), + 'isWebUpdaterRecommended' => $this->isWebUpdaterRecommended(), 'updaterEnabled' => empty($updateState['updaterEnabled']) ? false : $updateState['updaterEnabled'], 'versionIsEol' => empty($updateState['versionIsEol']) ? false : $updateState['versionIsEol'], 'isDefaultUpdateServerURL' => $isDefaultUpdateServerURL, @@ -184,4 +197,40 @@ class Admin implements ISettings { public function getPriority(): int { return 11; } + + private function isWebUpdaterRecommended(): bool { + return $this->getUserCount() < 100; + } + + // Copied from https://github.com/nextcloud/server/blob/a06001e0851abc6073af678b742da3e1aa96eec9/lib/private/Support/Subscription/Registry.php#L187-L214 + private function getUserCount(): int { + $userCount = 0; + $backends = $this->userManager->getBackends(); + foreach ($backends as $backend) { + // TODO: change below to 'if ($backend instanceof ICountUsersBackend) {' + if ($backend->implementsActions(Backend::COUNT_USERS)) { + /** @var ICountUsersBackend $backend */ + $backendUsers = $backend->countUsers(); + if ($backendUsers !== false) { + $userCount += $backendUsers; + } else { + // TODO what if the user count can't be determined? + $this->logger->warning('Can not determine user count for ' . get_class($backend), ['app' => 'updatenotification']); + } + } + } + + $disabledUsers = $this->config->getUsersForUserValue('core', 'enabled', 'false'); + $disabledUsersCount = count($disabledUsers); + $userCount = $userCount - $disabledUsersCount; + + if ($userCount < 0) { + $userCount = 0; + + // this should never happen + $this->logger->warning("Total user count was negative (users: $userCount, disabled: $disabledUsersCount)", ['app' => 'updatenotification']); + } + + return $userCount; + } } diff --git a/apps/updatenotification/src/components/UpdateNotification.vue b/apps/updatenotification/src/components/UpdateNotification.vue index 09c229fb36e..0f978358e3f 100644 --- a/apps/updatenotification/src/components/UpdateNotification.vue +++ b/apps/updatenotification/src/components/UpdateNotification.vue @@ -41,6 +41,12 @@ + +
dateTimeFormatter = $this->createMock(IDateTimeFormatter::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->subscriptionRegistry = $this->createMock(IRegistry::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->admin = new Admin( - $this->config, $this->updateChecker, $this->groupManager, $this->dateTimeFormatter, $this->l10nFactory, $this->subscriptionRegistry + $this->config, + $this->updateChecker, + $this->groupManager, + $this->dateTimeFormatter, + $this->l10nFactory, + $this->subscriptionRegistry, + $this->userManager, + $this->logger ); } public function testGetFormWithUpdate() { + $backend1 = $this->createMock(UserInterface::class); + $backend2 = $this->createMock(UserInterface::class); + $backend3 = $this->createMock(UserInterface::class); + $backend1 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(false); + $backend2 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(true); + $backend3 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(true); + $backend1 + ->expects($this->never()) + ->method('countUsers'); + $backend2 + ->expects($this->once()) + ->method('countUsers') + ->with() + ->willReturn(false); + $backend3 + ->expects($this->once()) + ->method('countUsers') + ->with() + ->willReturn(5); + $this->userManager + ->expects($this->once()) + ->method('getBackends') + ->with() + ->willReturn([$backend1, $backend2, $backend3]); $channels = [ 'daily', 'beta', @@ -145,6 +196,7 @@ class AdminTest extends TestCase { 'downloadLink' => 'https://downloads.nextcloud.org/server', 'changes' => [], 'webUpdaterEnabled' => true, + 'isWebUpdaterRecommended' => true, 'updaterEnabled' => true, 'versionIsEol' => false, 'isDefaultUpdateServerURL' => true, @@ -161,6 +213,42 @@ class AdminTest extends TestCase { } public function testGetFormWithUpdateAndChangedUpdateServer() { + $backend1 = $this->createMock(UserInterface::class); + $backend2 = $this->createMock(UserInterface::class); + $backend3 = $this->createMock(UserInterface::class); + $backend1 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(false); + $backend2 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(true); + $backend3 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(true); + $backend1 + ->expects($this->never()) + ->method('countUsers'); + $backend2 + ->expects($this->once()) + ->method('countUsers') + ->with() + ->willReturn(false); + $backend3 + ->expects($this->once()) + ->method('countUsers') + ->with() + ->willReturn(5); + $this->userManager + ->expects($this->once()) + ->method('getBackends') + ->with() + ->willReturn([$backend1, $backend2, $backend3]); $channels = [ 'daily', 'beta', @@ -232,6 +320,7 @@ class AdminTest extends TestCase { 'downloadLink' => 'https://downloads.nextcloud.org/server', 'changes' => [], 'webUpdaterEnabled' => false, + 'isWebUpdaterRecommended' => true, 'updaterEnabled' => true, 'versionIsEol' => false, 'isDefaultUpdateServerURL' => false, @@ -248,6 +337,42 @@ class AdminTest extends TestCase { } public function testGetFormWithUpdateAndCustomersUpdateServer() { + $backend1 = $this->createMock(UserInterface::class); + $backend2 = $this->createMock(UserInterface::class); + $backend3 = $this->createMock(UserInterface::class); + $backend1 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(false); + $backend2 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(true); + $backend3 + ->expects($this->once()) + ->method('implementsActions') + ->with(Backend::COUNT_USERS) + ->willReturn(true); + $backend1 + ->expects($this->never()) + ->method('countUsers'); + $backend2 + ->expects($this->once()) + ->method('countUsers') + ->with() + ->willReturn(false); + $backend3 + ->expects($this->once()) + ->method('countUsers') + ->with() + ->willReturn(5); + $this->userManager + ->expects($this->once()) + ->method('getBackends') + ->with() + ->willReturn([$backend1, $backend2, $backend3]); $channels = [ 'daily', 'beta', @@ -319,6 +444,7 @@ class AdminTest extends TestCase { 'downloadLink' => 'https://downloads.nextcloud.org/server', 'changes' => [], 'webUpdaterEnabled' => true, + 'isWebUpdaterRecommended' => true, 'updaterEnabled' => true, 'versionIsEol' => false, 'isDefaultUpdateServerURL' => true, -- cgit v1.2.3