From bdc149aa6b60886a9c0236a5c1aa407542a08d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Sun, 26 Jan 2020 12:14:51 +0100 Subject: [PATCH] Move default board creation to Application and cleanup code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- appinfo/app.php | 16 +- lib/AppInfo/Application.php | 160 ++++++++++-------- lib/Controller/PageController.php | 10 -- lib/Service/BoardService.php | 28 +-- lib/Service/DefaultBoardService.php | 29 ++-- .../unit/Service/DefaultBoardServiceTest.php | 4 +- tests/unit/controller/PageControllerTest.php | 48 +----- 7 files changed, 133 insertions(+), 162 deletions(-) diff --git a/appinfo/app.php b/appinfo/app.php index c59ee8832..bf0ac6c98 100644 --- a/appinfo/app.php +++ b/appinfo/app.php @@ -21,15 +21,19 @@ * */ -if ((@include_once __DIR__ . '/../vendor/autoload.php')===false) { +use OCA\Deck\AppInfo\Application; +use OCP\AppFramework\QueryException; + +if ((@include_once __DIR__ . '/../vendor/autoload.php')=== false) { throw new Exception('Cannot include autoload. Did you run install dependencies using composer?'); } -$app = \OC::$server->query(\OCA\Deck\AppInfo\Application::class); -$app->registerNavigationEntry(); -$app->registerNotifications(); -$app->registerCommentsEntity(); -$app->registerFullTextSearch(); +try { + /** @var Application $app */ + $app = \OC::$server->query(Application::class); + $app->register(); +} catch (QueryException $e) { +} /** Load activity style global so it is availabile in the activity app as well */ \OC_Util::addStyle('deck', 'activity'); diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 27fd4ca20..1dc966dcb 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -24,29 +24,42 @@ namespace OCA\Deck\AppInfo; use Exception; +use InvalidArgumentException; +use OC_Util; use OCA\Deck\Activity\CommentEventHandler; use OCA\Deck\Capabilities; +use OCA\Deck\Collaboration\Resources\ResourceProvider; +use OCA\Deck\Collaboration\Resources\ResourceProviderCard; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\AssignedUsersMapper; use OCA\Deck\Db\CardMapper; use OCA\Deck\Middleware\ExceptionMiddleware; use OCA\Deck\Notification\Notifier; +use OCA\Deck\Service\DefaultBoardService; use OCA\Deck\Service\FullTextSearchService; +use OCA\Deck\Service\PermissionService; use OCP\AppFramework\App; use OCP\Collaboration\Resources\IManager; +use OCP\Collaboration\Resources\IProviderManager; use OCP\Comments\CommentsEntityEvent; use OCP\FullTextSearch\IFullTextSearchManager; use OCP\IGroup; +use OCP\ILogger; +use OCP\IServerContainer; use OCP\IUser; use OCP\IUserManager; use OCP\IURLGenerator; -use OCP\INavigationManager; +use OCP\IUserSession; use OCP\Util; use Symfony\Component\EventDispatcher\GenericEvent; class Application extends App { + public const APP_ID = 'deck'; + + /** @var IServerContainer */ + private $server; /** @var FullTextSearchService */ private $fullTextSearchService; @@ -54,40 +67,60 @@ class Application extends App { /** @var IFullTextSearchManager */ private $fullTextSearchManager; + /** @var ILogger */ + private $logger; - /** - * Application constructor. - * - * @param array $urlParams - * - * @throws \OCP\AppFramework\QueryException - */ public function __construct(array $urlParams = array()) { parent::__construct('deck', $urlParams); $container = $this->getContainer(); - $server = $container->getServer(); + $server = $this->getContainer()->getServer(); - $container->registerService('ExceptionMiddleware', function() use ($server) { - return new ExceptionMiddleware( - $server->getLogger(), - $server->getConfig() - ); - }); - $container->registerMiddleWare('ExceptionMiddleware'); + $this->server = $server; + $this->logger = $server->getLogger(); - $container->registerService('databaseType', function($container) { - return $container->getServer()->getConfig()->getSystemValue('dbtype', 'sqlite'); + $container->registerCapability(Capabilities::class); + $container->registerMiddleWare(ExceptionMiddleware::class); + + $container->registerService('databaseType', static function() use ($server) { + return $server->getConfig()->getSystemValue('dbtype', 'sqlite'); + }); + $container->registerService('database4ByteSupport', static function() use ($server) { + return $server->getDatabaseConnection()->supports4ByteText(); }); - $container->registerService('database4ByteSupport', function($container) { - return $container->getServer()->getDatabaseConnection()->supports4ByteText(); + } + + public function register(): void { + $this->registerNavigationEntry(); + $this->registerUserGroupHooks(); + $this->registerNotifications(); + $this->registerCommentsEntity(); + $this->registerFullTextSearch(); + $this->registerCollaborationResources(); + $this->checkDefaultBoard(); + } + + public function registerNavigationEntry(): void { + $container = $this->getContainer(); + $this->server->getNavigationManager()->add(static function() use ($container) { + $urlGenerator = $container->query(IURLGenerator::class); + return [ + 'id' => 'deck', + 'order' => 10, + 'href' => $urlGenerator->linkToRoute('deck.page.index'), + 'icon' => $urlGenerator->imagePath('deck', 'deck.svg'), + 'name' => 'Deck', + ]; }); + } + private function registerUserGroupHooks(): void { + $container = $this->getContainer(); // Delete user/group acl entries when they get deleted /** @var IUserManager $userManager */ - $userManager = $server->getUserManager(); - $userManager->listen('\OC\User', 'postDelete', function(IUser $user) use ($container) { + $userManager = $this->server->getUserManager(); + $userManager->listen('\OC\User', 'postDelete', static function(IUser $user) use ($container) { // delete existing acl entries for deleted user /** @var AclMapper $aclMapper */ $aclMapper = $container->query(AclMapper::class); @@ -104,8 +137,8 @@ public function __construct(array $urlParams = array()) { }); /** @var IUserManager $userManager */ - $groupManager = $server->getGroupManager(); - $groupManager->listen('\OC\Group', 'postDelete', function(IGroup $group) use ($container) { + $groupManager = $this->server->getGroupManager(); + $groupManager->listen('\OC\Group', 'postDelete', static function(IGroup $group) use ($container) { /** @var AclMapper $aclMapper */ $aclMapper = $container->query(AclMapper::class); $aclMapper->findByParticipant(Acl::PERMISSION_TYPE_GROUP, $group->getGID()); @@ -114,47 +147,21 @@ public function __construct(array $urlParams = array()) { $aclMapper->delete($acl); } }); - - $this->registerCollaborationResources(); - - $this->getContainer()->registerCapability(Capabilities::class); - - - } - - /** - * @throws \OCP\AppFramework\QueryException - */ - public function registerNavigationEntry() { - $container = $this->getContainer(); - $container->query(INavigationManager::class)->add(function() use ($container) { - $urlGenerator = $container->query(IURLGenerator::class); - return [ - 'id' => 'deck', - 'order' => 10, - 'href' => $urlGenerator->linkToRoute('deck.page.index'), - 'icon' => $urlGenerator->imagePath('deck', 'deck.svg'), - 'name' => 'Deck', - ]; - }); } - public function registerNotifications() { - $notificationManager = \OC::$server->getNotificationManager(); + public function registerNotifications(): void { + $notificationManager = $this->server->getNotificationManager(); $notificationManager->registerNotifierService(Notifier::class); } - /** - * @throws \OCP\AppFramework\QueryException - */ - public function registerCommentsEntity() { - $this->getContainer()->getServer()->getEventDispatcher()->addListener(CommentsEntityEvent::EVENT_ENTITY, function(CommentsEntityEvent $event) { + public function registerCommentsEntity(): void { + $this->server->getEventDispatcher()->addListener(CommentsEntityEvent::EVENT_ENTITY, function(CommentsEntityEvent $event) { $event->addEntityCollection('deckCard', function($name) { /** @var CardMapper */ $service = $this->getContainer()->query(CardMapper::class); try { $service->find((int) $name); - } catch (\InvalidArgumentException $e) { + } catch (InvalidArgumentException $e) { return false; } return true; @@ -164,19 +171,15 @@ public function registerCommentsEntity() { } /** - * @throws \OCP\AppFramework\QueryException */ - protected function registerCommentsEventHandler() { - $this->getContainer()->getServer()->getCommentsManager()->registerEventHandler(function () { + protected function registerCommentsEventHandler(): void { + $this->server->getCommentsManager()->registerEventHandler(function () { return $this->getContainer()->query(CommentEventHandler::class); }); } - /** - * @throws \OCP\AppFramework\QueryException - */ - protected function registerCollaborationResources() { - $version = \OC_Util::getVersion()[0]; + protected function registerCollaborationResources(): void { + $version = OC_Util::getVersion()[0]; if ($version < 16) { return; } @@ -190,18 +193,18 @@ protected function registerCollaborationResources() { /** @var IManager $resourceManager */ $resourceManager = $this->getContainer()->query(IManager::class); } else { - /** @var \OCP\Collaboration\Resources\IProviderManager $resourceManager */ - $resourceManager = $this->getContainer()->query(\OCP\Collaboration\Resources\IProviderManager::class); + /** @var IProviderManager $resourceManager */ + $resourceManager = $this->getContainer()->query(IProviderManager::class); } - $resourceManager->registerResourceProvider(\OCA\Deck\Collaboration\Resources\ResourceProvider::class); - $resourceManager->registerResourceProvider(\OCA\Deck\Collaboration\Resources\ResourceProviderCard::class); + $resourceManager->registerResourceProvider(ResourceProvider::class); + $resourceManager->registerResourceProvider(ResourceProviderCard::class); - \OC::$server->getEventDispatcher()->addListener('\OCP\Collaboration\Resources::loadAdditionalScripts', function () { - \OCP\Util::addScript('deck', 'collections'); + $this->server->getEventDispatcher()->addListener('\OCP\Collaboration\Resources::loadAdditionalScripts', static function () { + Util::addScript('deck', 'collections'); }); } - public function registerFullTextSearch() { + public function registerFullTextSearch(): void { if (Util::getVersion()[0] < 16) { return; } @@ -218,7 +221,7 @@ public function registerFullTextSearch() { return; } - $eventDispatcher = \OC::$server->getEventDispatcher(); + $eventDispatcher = $this->server->getEventDispatcher(); $eventDispatcher->addListener( '\OCA\Deck\Card::onCreate', function(GenericEvent $e) { $this->fullTextSearchService->onCardCreated($e); @@ -251,4 +254,19 @@ public function registerFullTextSearch() { ); } + private function checkDefaultBoard(): void { + try { + /** @var IUserSession $userSession */ + $userSession = $this->getContainer()->query(IUserSession::class); + $userId = $userSession->getUser() ? $userSession->getUser()->getUID() : null; + /** @var DefaultBoardService $defaultBoardService */ + $defaultBoardService = $this->getContainer()->query(DefaultBoardService::class); + $permissionService = $this->getContainer()->query(PermissionService::class); + if ($userId !== null && $defaultBoardService->checkFirstRun($userId) && $permissionService->canCreate()) { + $defaultBoardService->createDefaultBoard($this->server->getL10N(self::APP_ID)->t('Personal'), $userId, '0087C5'); + } + } catch (\Throwable $e) { + $this->logger->logException($e); + } + } } diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 268b0914e..bcd45b1c6 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -23,7 +23,6 @@ namespace OCA\Deck\Controller; -use OCA\Deck\Service\DefaultBoardService; use OCA\Deck\Service\PermissionService; use OCP\IRequest; use OCP\AppFramework\Http\TemplateResponse; @@ -32,7 +31,6 @@ class PageController extends Controller { - private $defaultBoardService; private $permissionService; private $userId; private $l10n; @@ -40,7 +38,6 @@ class PageController extends Controller { public function __construct( $AppName, IRequest $request, - DefaultBoardService $defaultBoardService, PermissionService $permissionService, IL10N $l10n, $userId @@ -48,7 +45,6 @@ public function __construct( parent::__construct($AppName, $request); $this->userId = $userId; - $this->defaultBoardService = $defaultBoardService; $this->permissionService = $permissionService; $this->l10n = $l10n; } @@ -67,12 +63,6 @@ public function index() { 'canCreate' => $this->permissionService->canCreate() ]; - if ($this->defaultBoardService->checkFirstRun($this->userId, $this->appName)) { - if ($this->permissionService->canCreate()) { - $this->defaultBoardService->createDefaultBoard($this->l10n->t('Personal'), $this->userId, '0087C5'); - } - } - return new TemplateResponse('deck', 'main', $params); } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index d99f1b251..224d8c44f 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -134,10 +134,10 @@ public function findAll($since = -1, $details = null) { } $permissions = $this->permissionService->matchPermissions($item); $item->setPermissions([ - 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ], - 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT], - 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE], - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] + 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, + 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, + 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false ]); $result[$item->getId()] = $item; } @@ -170,10 +170,10 @@ public function find($boardId) { } $permissions = $this->permissionService->matchPermissions($board); $board->setPermissions([ - 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ], - 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT], - 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE], - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] + 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, + 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, + 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false ]); $this->enrichWithUsers($board); return $board; @@ -307,10 +307,10 @@ public function create($title, $userId, $color) { $this->boardMapper->mapOwner($new_board); $permissions = $this->permissionService->matchPermissions($new_board); $new_board->setPermissions([ - 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ], - 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT], - 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE], - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] + 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, + 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, + 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false ]); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $new_board, ActivityManager::SUBJECT_BOARD_CREATE); $this->changeHelper->boardChanged($new_board->getId()); @@ -630,7 +630,7 @@ public function clone($id) { } $this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ); - + $board = $this->boardMapper->find($id); $newBoard = new Board(); $newBoard->setTitle($board->getTitle() . ' (' . $this->l10n->t('copy') . ')'); @@ -654,7 +654,7 @@ public function clone($id) { $newStack->setBoardId($newBoard->getId()); $this->stackMapper->insert($newStack); } - + return $newBoard; } diff --git a/lib/Service/DefaultBoardService.php b/lib/Service/DefaultBoardService.php index 5e16401f8..303e6c2b2 100644 --- a/lib/Service/DefaultBoardService.php +++ b/lib/Service/DefaultBoardService.php @@ -23,13 +23,12 @@ namespace OCA\Deck\Service; +use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\BoardMapper; -use OCA\Deck\Service\BoardService; -use OCA\Deck\Service\StackService; -use OCA\Deck\Service\CardService; use OCP\IConfig; use OCP\IL10N; use OCA\Deck\BadRequestException; +use OCP\PreConditionNotMetException; class DefaultBoardService { @@ -58,17 +57,21 @@ public function __construct( } /** + * Return true if this is the first time a user is acessing their instance with deck enabled + * * @param $userId - * @param $appName * @return bool - * @throws \OCP\PreConditionNotMetException */ - public function checkFirstRun($userId, $appName) { - $firstRun = $this->config->getUserValue($userId, $appName, 'firstRun', 'yes'); + public function checkFirstRun($userId): bool { + $firstRun = $this->config->getUserValue($userId, Application::APP_ID, 'firstRun', 'yes'); $userBoards = $this->boardMapper->findAllByUser($userId); - + if ($firstRun === 'yes' && count($userBoards) === 0) { - $this->config->setUserValue($userId, $appName, 'firstRun', 'no'); + try { + $this->config->setUserValue($userId, Application::APP_ID, 'firstRun', 'no'); + } catch (PreConditionNotMetException $e) { + return false; + } return true; } @@ -86,7 +89,7 @@ public function checkFirstRun($userId, $appName) { * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @throws BadRequestException */ - public function createDefaultBoard($title, $userId, $color) { + public function createDefaultBoard(string $title, string $userId, string $color) { if ($title === false || $title === null) { throw new BadRequestException('title must be provided'); @@ -104,16 +107,16 @@ public function createDefaultBoard($title, $userId, $color) { $defaultStacks = []; $defaultCards = []; - $boardId = $defaultBoard->getId(); + $boardId = $defaultBoard->getId(); $defaultStacks[] = $this->stackService->create($this->l10n->t('To do'), $boardId, 1); $defaultStacks[] = $this->stackService->create($this->l10n->t('Doing'), $boardId, 1); $defaultStacks[] = $this->stackService->create($this->l10n->t('Done'), $boardId, 1); - + $defaultCards[] = $this->cardService->create($this->l10n->t('Example Task 3'), $defaultStacks[0]->getId(), 'text', 0, $userId); $defaultCards[] = $this->cardService->create($this->l10n->t('Example Task 2'), $defaultStacks[1]->getId(), 'text', 0, $userId); $defaultCards[] = $this->cardService->create($this->l10n->t('Example Task 1'), $defaultStacks[2]->getId(), 'text', 0, $userId); return $defaultBoard; } -} \ No newline at end of file +} diff --git a/tests/unit/Service/DefaultBoardServiceTest.php b/tests/unit/Service/DefaultBoardServiceTest.php index 5ca9bea12..65b46fd91 100644 --- a/tests/unit/Service/DefaultBoardServiceTest.php +++ b/tests/unit/Service/DefaultBoardServiceTest.php @@ -94,7 +94,7 @@ public function testCheckFirstRunCaseTrue() { $this->config->expects($this->once()) ->method('setUserValue'); - $result = $this->service->checkFirstRun($this->userId, $appName); + $result = $this->service->checkFirstRun($this->userId); $this->assertEquals($result, true); } @@ -115,7 +115,7 @@ public function testCheckFirstRunCaseFalse() { ->method('findAllByUser') ->willReturn($userBoards); - $result = $this->service->checkFirstRun($this->userId, $appName); + $result = $this->service->checkFirstRun($this->userId); $this->assertEquals($result, false); } diff --git a/tests/unit/controller/PageControllerTest.php b/tests/unit/controller/PageControllerTest.php index 21a6515c6..22a9a5247 100644 --- a/tests/unit/controller/PageControllerTest.php +++ b/tests/unit/controller/PageControllerTest.php @@ -45,69 +45,25 @@ class PageControllerTest extends \Test\TestCase { public function setUp(): void { $this->l10n = $this->createMock(IL10N::class); $this->request = $this->createMock(IRequest::class); - $this->defaultBoardService = $this->createMock(DefaultBoardService::class); $this->permissionService = $this->createMock(PermissionService::class); $this->config = $this->createMock(IConfig::class); $this->controller = new PageController( - 'deck', $this->request, $this->defaultBoardService, $this->permissionService, $this->l10n, $this->userId + 'deck', $this->request, $this->permissionService, $this->l10n, $this->userId ); } - public function testIndexOnFirstRun() { + public function testIndex() { $board = new Board(); $board->setTitle('Personal'); $board->setOwner($this->userId); $board->setColor('317CCC'); - $this->defaultBoardService->expects($this->once()) - ->method('checkFirstRun') - ->willReturn(true); - $this->permissionService->expects($this->any()) ->method('canCreate') ->willReturn(true); - $this->defaultBoardService->expects($this->once()) - ->method('createDefaultBoard') - ->willReturn($board); - - $response = $this->controller->index(); - $this->assertEquals('main', $response->getTemplateName()); - } - - public function testIndexOnFirstRunNoCreate() { - - $board = new Board(); - $board->setTitle('Personal'); - $board->setOwner($this->userId); - $board->setColor('317CCC'); - - $this->defaultBoardService->expects($this->once()) - ->method('checkFirstRun') - ->willReturn(true); - - $this->permissionService->expects($this->any()) - ->method('canCreate') - ->willReturn(false); - - $this->defaultBoardService->expects($this->never()) - ->method('createDefaultBoard') - ->willReturn($board); - - $response = $this->controller->index(); - $this->assertEquals('main', $response->getTemplateName()); - } - - public function testIndexOnSecondRun() { - - $this->config->setUserValue($this->userId, 'deck', 'firstRun', 'no'); - - $this->defaultBoardService->expects($this->once()) - ->method('checkFirstRun') - ->willReturn(false); - $response = $this->controller->index(); $this->assertEquals('main', $response->getTemplateName()); }