From 00386a5407fac7d41d114e8add07b86eabeb417f Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 22 Feb 2024 17:18:41 +0100 Subject: [PATCH] fix(settings): Use initial state instead of custom state for server data Signed-off-by: Ferdinand Thiessen --- .../lib/Controller/AppSettingsController.php | 93 ++++++------------- .../lib/Controller/UsersController.php | 91 +++++------------- apps/settings/templates/settings-vue.php | 32 ------- apps/settings/templates/settings/empty.php | 4 +- .../Controller/AppSettingsControllerTest.php | 31 ++++--- .../tests/Controller/UsersControllerTest.php | 34 ++++--- 6 files changed, 84 insertions(+), 201 deletions(-) delete mode 100644 apps/settings/templates/settings-vue.php diff --git a/apps/settings/lib/Controller/AppSettingsController.php b/apps/settings/lib/Controller/AppSettingsController.php index ac98677d0c9..6e1850001fb 100644 --- a/apps/settings/lib/Controller/AppSettingsController.php +++ b/apps/settings/lib/Controller/AppSettingsController.php @@ -47,6 +47,7 @@ use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Services\IInitialState; use OCP\IConfig; use OCP\IL10N; use OCP\INavigationManager; @@ -58,72 +59,26 @@ use Psr\Log\LoggerInterface; #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] class AppSettingsController extends Controller { - /** @var \OCP\IL10N */ - private $l10n; - /** @var IConfig */ - private $config; - /** @var INavigationManager */ - private $navigationManager; - /** @var IAppManager */ - private $appManager; - /** @var CategoryFetcher */ - private $categoryFetcher; - /** @var AppFetcher */ - private $appFetcher; - /** @var IFactory */ - private $l10nFactory; - /** @var BundleFetcher */ - private $bundleFetcher; - /** @var Installer */ - private $installer; - /** @var IURLGenerator */ - private $urlGenerator; - /** @var LoggerInterface */ - private $logger; - /** @var array */ private $allApps = []; - /** - * @param string $appName - * @param IRequest $request - * @param IL10N $l10n - * @param IConfig $config - * @param INavigationManager $navigationManager - * @param IAppManager $appManager - * @param CategoryFetcher $categoryFetcher - * @param AppFetcher $appFetcher - * @param IFactory $l10nFactory - * @param BundleFetcher $bundleFetcher - * @param Installer $installer - * @param IURLGenerator $urlGenerator - * @param LoggerInterface $logger - */ - public function __construct(string $appName, + public function __construct( + string $appName, IRequest $request, - IL10N $l10n, - IConfig $config, - INavigationManager $navigationManager, - IAppManager $appManager, - CategoryFetcher $categoryFetcher, - AppFetcher $appFetcher, - IFactory $l10nFactory, - BundleFetcher $bundleFetcher, - Installer $installer, - IURLGenerator $urlGenerator, - LoggerInterface $logger) { + private IL10N $l10n, + private IConfig $config, + private INavigationManager $navigationManager, + private IAppManager $appManager, + private CategoryFetcher $categoryFetcher, + private AppFetcher $appFetcher, + private IFactory $l10nFactory, + private BundleFetcher $bundleFetcher, + private Installer $installer, + private IURLGenerator $urlGenerator, + private LoggerInterface $logger, + private IInitialState $initialState, + ) { parent::__construct($appName, $request); - $this->l10n = $l10n; - $this->config = $config; - $this->navigationManager = $navigationManager; - $this->appManager = $appManager; - $this->categoryFetcher = $categoryFetcher; - $this->appFetcher = $appFetcher; - $this->l10nFactory = $l10nFactory; - $this->bundleFetcher = $bundleFetcher; - $this->installer = $installer; - $this->urlGenerator = $urlGenerator; - $this->logger = $logger; } /** @@ -132,18 +87,22 @@ class AppSettingsController extends Controller { * @return TemplateResponse */ public function viewApps(): TemplateResponse { - $params = []; - $params['appstoreEnabled'] = $this->config->getSystemValueBool('appstoreenabled', true); - $params['updateCount'] = count($this->getAppsWithUpdates()); - $params['developerDocumentation'] = $this->urlGenerator->linkToDocs('developer-manual'); - $params['bundles'] = $this->getBundles(); $this->navigationManager->setActiveEntry('core_apps'); - $templateResponse = new TemplateResponse('settings', 'settings-vue', ['serverData' => $params, 'pageTitle' => $this->l10n->t('Apps')]); + $this->initialState->provideInitialState('appstoreEnabled', $this->config->getSystemValueBool('appstoreenabled', true)); + $this->initialState->provideInitialState('appstoreBundles', $this->getBundles()); + $this->initialState->provideInitialState('appstoreDeveloperDocs', $this->urlGenerator->linkToDocs('developer-manual')); + $this->initialState->provideInitialState('appstoreUpdateCount', count($this->getAppsWithUpdates())); + $policy = new ContentSecurityPolicy(); $policy->addAllowedImageDomain('https://usercontent.apps.nextcloud.com'); + + $templateResponse = new TemplateResponse('settings', 'settings/empty', ['pageTitle' => $this->l10n->t('Apps')]); $templateResponse->setContentSecurityPolicy($policy); + \OCP\Util::addStyle('settings', 'settings'); + \OCP\Util::addScript('settings', 'vue-settings-apps-users-management'); + return $templateResponse; } diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index c6abe6bff4f..238f08590b5 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -33,8 +33,6 @@ declare(strict_types=1); * along with this program. If not, see * */ -// FIXME: disabled for now to be able to inject IGroupManager and also use -// getSubAdmin() namespace OCA\Settings\Controller; @@ -42,9 +40,7 @@ use InvalidArgumentException; use OC\AppFramework\Http; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\ForbiddenException; -use OC\Group\Manager as GroupManager; use OC\KnownUser\KnownUserService; -use OC\L10N\Factory; use OC\Security\IdentityProof\Manager; use OC\User\Manager as UserManager; use OCA\Settings\BackgroundJobs\VerifyUserData; @@ -59,6 +55,7 @@ use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Services\IInitialState; use OCP\BackgroundJob\IJobList; use OCP\Encryption\IManager; use OCP\EventDispatcher\IEventDispatcher; @@ -67,7 +64,6 @@ use OCP\IGroupManager; use OCP\IL10N; use OCP\IRequest; use OCP\IUser; -use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Mail\IMailer; @@ -75,73 +71,28 @@ use function in_array; #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] class UsersController extends Controller { - /** @var UserManager */ - private $userManager; - /** @var GroupManager */ - private $groupManager; - /** @var IUserSession */ - private $userSession; - /** @var IConfig */ - private $config; - /** @var bool */ - private $isAdmin; - /** @var IL10N */ - private $l10n; - /** @var IMailer */ - private $mailer; - /** @var Factory */ - private $l10nFactory; - /** @var IAppManager */ - private $appManager; - /** @var IAccountManager */ - private $accountManager; - /** @var Manager */ - private $keyManager; - /** @var IJobList */ - private $jobList; - /** @var IManager */ - private $encryptionManager; - /** @var KnownUserService */ - private $knownUserService; - /** @var IEventDispatcher */ - private $dispatcher; - public function __construct( string $appName, IRequest $request, - IUserManager $userManager, - IGroupManager $groupManager, - IUserSession $userSession, - IConfig $config, - bool $isAdmin, - IL10N $l10n, - IMailer $mailer, - IFactory $l10nFactory, - IAppManager $appManager, - IAccountManager $accountManager, - Manager $keyManager, - IJobList $jobList, - IManager $encryptionManager, - KnownUserService $knownUserService, - IEventDispatcher $dispatcher + private UserManager $userManager, + private IGroupManager $groupManager, + private IUserSession $userSession, + private IConfig $config, + private bool $isAdmin, + private IL10N $l10n, + private IMailer $mailer, + private IFactory $l10nFactory, + private IAppManager $appManager, + private IAccountManager $accountManager, + private Manager $keyManager, + private IJobList $jobList, + private IManager $encryptionManager, + private KnownUserService $knownUserService, + private IEventDispatcher $dispatcher, + private IInitialState $initialState, ) { parent::__construct($appName, $request); - $this->userManager = $userManager; - $this->groupManager = $groupManager; - $this->userSession = $userSession; - $this->config = $config; - $this->isAdmin = $isAdmin; - $this->l10n = $l10n; - $this->mailer = $mailer; - $this->l10nFactory = $l10nFactory; - $this->appManager = $appManager; - $this->accountManager = $accountManager; - $this->keyManager = $keyManager; - $this->jobList = $jobList; - $this->encryptionManager = $encryptionManager; - $this->knownUserService = $knownUserService; - $this->dispatcher = $dispatcher; } @@ -231,6 +182,7 @@ class UsersController extends Controller { $userCount -= 1; // we also lower from one the total count } } + $userCount += $this->userManager->countUsersOfGroups($groupsInfo->getGroups()); $disabledUsers = $this->userManager->countDisabledUsersOfGroups($groupsNames); } @@ -280,7 +232,12 @@ class UsersController extends Controller { $serverData['newUserRequireEmail'] = $this->config->getAppValue('core', 'newUser.requireEmail', 'no') === 'yes'; $serverData['newUserSendEmail'] = $this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes'; - return new TemplateResponse('settings', 'settings-vue', ['serverData' => $serverData, 'pageTitle' => $this->l10n->t('Users')]); + $this->initialState->provideInitialState('usersSettings', $serverData); + + \OCP\Util::addStyle('settings', 'settings'); + \OCP\Util::addScript('settings', 'vue-settings-apps-users-management'); + + return new TemplateResponse('settings', 'settings/empty', ['pageTitle' => $this->l10n->t('Users')]); } /** diff --git a/apps/settings/templates/settings-vue.php b/apps/settings/templates/settings-vue.php deleted file mode 100644 index 8db9dc6286b..00000000000 --- a/apps/settings/templates/settings-vue.php +++ /dev/null @@ -1,32 +0,0 @@ - - * - * @author John Molakvoæ - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * - */ - -script('settings', 'vue-settings-apps-users-management'); -style('settings', 'settings'); - -// Do we have some data to inject ? -if (is_array($_['serverData'])) { - ?> - - diff --git a/apps/settings/templates/settings/empty.php b/apps/settings/templates/settings/empty.php index 59e3098e3de..d0a2f9c77c4 100644 --- a/apps/settings/templates/settings/empty.php +++ b/apps/settings/templates/settings/empty.php @@ -4,7 +4,7 @@ * * @author Arthur Schiwon * - * @license GNU AGPL version 3 or any later version + * @license AGPL-3.0-or-later * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -21,4 +21,4 @@ * */ -# used for Personal/Additional settings as fallback for legacy settings +// Empty template as Vue will take over the `id="conent"` of the base template element diff --git a/apps/settings/tests/Controller/AppSettingsControllerTest.php b/apps/settings/tests/Controller/AppSettingsControllerTest.php index a73ce7f1819..fd22d176075 100644 --- a/apps/settings/tests/Controller/AppSettingsControllerTest.php +++ b/apps/settings/tests/Controller/AppSettingsControllerTest.php @@ -37,6 +37,7 @@ use OCP\App\IAppManager; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Services\IInitialState; use OCP\IConfig; use OCP\IL10N; use OCP\INavigationManager; @@ -81,6 +82,8 @@ class AppSettingsControllerTest extends TestCase { private $urlGenerator; /** @var LoggerInterface|MockObject */ private $logger; + /** @var IInitialState|MockObject */ + private $initialState; protected function setUp(): void { parent::setUp(); @@ -100,6 +103,7 @@ class AppSettingsControllerTest extends TestCase { $this->installer = $this->createMock(Installer::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->logger = $this->createMock(LoggerInterface::class); + $this->initialState = $this->createMock(IInitialState::class); $this->appSettingsController = new AppSettingsController( 'settings', @@ -114,7 +118,8 @@ class AppSettingsControllerTest extends TestCase { $this->bundleFetcher, $this->installer, $this->urlGenerator, - $this->logger + $this->logger, + $this->initialState, ); } @@ -188,18 +193,16 @@ class AppSettingsControllerTest extends TestCase { ->method('setActiveEntry') ->with('core_apps'); + $this->initialState + ->expects($this->exactly(4)) + ->method('provideInitialState'); + $policy = new ContentSecurityPolicy(); $policy->addAllowedImageDomain('https://usercontent.apps.nextcloud.com'); $expected = new TemplateResponse('settings', - 'settings-vue', + 'settings/empty', [ - 'serverData' => [ - 'updateCount' => 0, - 'appstoreEnabled' => true, - 'bundles' => [], - 'developerDocumentation' => '' - ], 'pageTitle' => 'Apps' ], 'user'); @@ -223,18 +226,16 @@ class AppSettingsControllerTest extends TestCase { ->method('setActiveEntry') ->with('core_apps'); + $this->initialState + ->expects($this->exactly(4)) + ->method('provideInitialState'); + $policy = new ContentSecurityPolicy(); $policy->addAllowedImageDomain('https://usercontent.apps.nextcloud.com'); $expected = new TemplateResponse('settings', - 'settings-vue', + 'settings/empty', [ - 'serverData' => [ - 'updateCount' => 0, - 'appstoreEnabled' => false, - 'bundles' => [], - 'developerDocumentation' => '' - ], 'pageTitle' => 'Apps' ], 'user'); diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index eddb290212a..93feba01c60 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -35,6 +35,7 @@ use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\ForbiddenException; use OC\Group\Manager; use OC\KnownUser\KnownUserService; +use OC\User\Manager as UserManager; use OCA\Settings\Controller\UsersController; use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; @@ -42,21 +43,19 @@ use OCP\Accounts\IAccountProperty; use OCP\Accounts\PropertyDoesNotExistException; use OCP\App\IAppManager; use OCP\AppFramework\Http; +use OCP\AppFramework\Services\IInitialState; use OCP\BackgroundJob\IJobList; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; use OCP\EventDispatcher\IEventDispatcher; -use OCP\IAvatarManager; use OCP\IConfig; use OCP\IGroupManager; use OCP\IL10N; use OCP\IRequest; use OCP\IUser; -use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Mail\IMailer; -use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; /** @@ -67,7 +66,7 @@ use PHPUnit\Framework\MockObject\MockObject; class UsersControllerTest extends \Test\TestCase { /** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */ private $groupManager; - /** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */ + /** @var UserManager|\PHPUnit\Framework\MockObject\MockObject */ private $userManager; /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ private $userSession; @@ -79,33 +78,29 @@ class UsersControllerTest extends \Test\TestCase { private $l10nFactory; /** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */ private $appManager; - /** @var IAvatarManager|\PHPUnit\Framework\MockObject\MockObject */ - private $avatarManager; /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ private $l; - /** @var AccountManager | \PHPUnit\Framework\MockObject\MockObject */ + /** @var AccountManager|\PHPUnit\Framework\MockObject\MockObject */ private $accountManager; - /** @var ISecureRandom | \PHPUnit\Framework\MockObject\MockObject */ - private $secureRandom; - /** @var \OCA\Settings\Mailer\NewUserMailHelper|\PHPUnit\Framework\MockObject\MockObject */ - private $newUserMailHelper; /** @var IJobList | \PHPUnit\Framework\MockObject\MockObject */ private $jobList; - /** @var \OC\Security\IdentityProof\Manager |\PHPUnit\Framework\MockObject\MockObject */ + /** @var \OC\Security\IdentityProof\Manager|\PHPUnit\Framework\MockObject\MockObject */ private $securityManager; - /** @var IManager | \PHPUnit\Framework\MockObject\MockObject */ + /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ private $encryptionManager; /** @var KnownUserService|\PHPUnit\Framework\MockObject\MockObject */ private $knownUserService; - /** @var IEncryptionModule | \PHPUnit\Framework\MockObject\MockObject */ + /** @var IEncryptionModule|\PHPUnit\Framework\MockObject\MockObject */ private $encryptionModule; /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ private $dispatcher; + /** @var IInitialState|\PHPUnit\Framework\MockObject\MockObject*/ + private $initialState; protected function setUp(): void { parent::setUp(); - $this->userManager = $this->createMock(IUserManager::class); + $this->userManager = $this->createMock(UserManager::class); $this->groupManager = $this->createMock(Manager::class); $this->userSession = $this->createMock(IUserSession::class); $this->config = $this->createMock(IConfig::class); @@ -119,6 +114,7 @@ class UsersControllerTest extends \Test\TestCase { $this->encryptionManager = $this->createMock(IManager::class); $this->knownUserService = $this->createMock(KnownUserService::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); + $this->initialState = $this->createMock(IInitialState::class); $this->l->method('t') ->willReturnCallback(function ($text, $parameters = []) { @@ -155,7 +151,8 @@ class UsersControllerTest extends \Test\TestCase { $this->jobList, $this->encryptionManager, $this->knownUserService, - $this->dispatcher + $this->dispatcher, + $this->initialState, ); } else { return $this->getMockBuilder(UsersController::class) @@ -177,9 +174,10 @@ class UsersControllerTest extends \Test\TestCase { $this->jobList, $this->encryptionManager, $this->knownUserService, - $this->dispatcher + $this->dispatcher, + $this->initialState, ] - )->setMethods($mockedMethods)->getMock(); + )->onlyMethods($mockedMethods)->getMock(); } }