From 4e937bd03a59c9753c65f2929187860f477c53af Mon Sep 17 00:00:00 2001 From: Luka Trovic Date: Mon, 28 Feb 2022 10:39:04 +0100 Subject: [PATCH] fix: generate fixed link for activity emails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Luka Trovic fix: generate fixed link for activity emails Signed-off-by: Luka Trovic Fix tests Signed-off-by: Julius Härtl --- appinfo/routes.php | 1 + lib/Activity/DeckProvider.php | 8 +++-- lib/Controller/PageController.php | 29 ++++++++++++++++- lib/Service/BoardService.php | 9 +++++- lib/Service/CardService.php | 14 ++++++++ tests/unit/Activity/DeckProviderTest.php | 8 ++++- tests/unit/Service/BoardServiceTest.php | 5 +++ tests/unit/Service/CardServiceTest.php | 6 ++++ tests/unit/controller/PageControllerTest.php | 34 +++++++++++++++++--- 9 files changed, 104 insertions(+), 10 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index 9ffa71655..0f553333a 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -25,6 +25,7 @@ return [ 'routes' => [ ['name' => 'page#index', 'url' => '/', 'verb' => 'GET'], + ['name' => 'page#redirectToCard', 'url' => '/card/{cardId}', 'verb' => 'GET'], // boards ['name' => 'board#index', 'url' => '/boards', 'verb' => 'GET'], diff --git a/lib/Activity/DeckProvider.php b/lib/Activity/DeckProvider.php index e01f760cc..2bca8fc64 100644 --- a/lib/Activity/DeckProvider.php +++ b/lib/Activity/DeckProvider.php @@ -35,6 +35,7 @@ use OCP\IConfig; use OCP\IURLGenerator; use OCP\IUserManager; use OCP\L10N\IFactory; +use OCA\Deck\Service\CardService; class DeckProvider implements IProvider { @@ -52,8 +53,10 @@ class DeckProvider implements IProvider { private $l10nFactory; /** @var IConfig */ private $config; + /** @var CardService */ + private $cardService; - public function __construct(IURLGenerator $urlGenerator, ActivityManager $activityManager, IUserManager $userManager, ICommentsManager $commentsManager, IFactory $l10n, IConfig $config, $userId) { + public function __construct(IURLGenerator $urlGenerator, ActivityManager $activityManager, IUserManager $userManager, ICommentsManager $commentsManager, IFactory $l10n, IConfig $config, $userId, CardService $cardService) { $this->userId = $userId; $this->urlGenerator = $urlGenerator; $this->activityManager = $activityManager; @@ -61,6 +64,7 @@ class DeckProvider implements IProvider { $this->userManager = $userManager; $this->l10nFactory = $l10n; $this->config = $config; + $this->cardService = $cardService; } /** @@ -131,7 +135,7 @@ class DeckProvider implements IProvider { if (array_key_exists('board', $subjectParams)) { $archivedParam = $subjectParams['card']['archived'] ? 'archived/' : ''; - $card['link'] = $this->deckUrl('/board/' . $subjectParams['board']['id'] . '/' . $archivedParam . 'card/' . $event->getObjectId()); + $card['link'] = $this->cardService->getRedirectUrlForCard($event->getObjectId()); } $params['card'] = $card; } diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 591603608..711b7e8ad 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -34,12 +34,20 @@ use OCP\IInitialStateService; use OCP\IRequest; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Controller; +use OCA\Deck\Db\CardMapper; +use OCP\IURLGenerator; +use \OCP\AppFramework\Http\RedirectResponse; +use OCA\Deck\Db\Acl; +use OCA\Deck\Service\CardService; class PageController extends Controller { private $permissionService; private $initialState; private $configService; private $eventDispatcher; + private $cardMapper; + private $urlGenerator; + private $cardService; public function __construct( $AppName, @@ -47,7 +55,10 @@ class PageController extends Controller { PermissionService $permissionService, IInitialStateService $initialStateService, ConfigService $configService, - IEventDispatcher $eventDispatcher + IEventDispatcher $eventDispatcher, + CardMapper $cardMapper, + IURLGenerator $urlGenerator, + CardService $cardService ) { parent::__construct($AppName, $request); @@ -55,6 +66,9 @@ class PageController extends Controller { $this->initialState = $initialStateService; $this->configService = $configService; $this->eventDispatcher = $eventDispatcher; + $this->cardMapper = $cardMapper; + $this->urlGenerator = $urlGenerator; + $this->cardService = $cardService; } /** @@ -85,4 +99,17 @@ class PageController extends Controller { return $response; } + + /** + * @NoAdminRequired + * @NoCSRFRequired + */ + public function redirectToCard($cardId): RedirectResponse { + try { + $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ); + return new RedirectResponse($this->cardService->getCardUrl($cardId)); + } catch (\Exception $e) { + return new RedirectResponse($this->urlGenerator->linkToRouteAbsolute('deck.page.index')); + } + } } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index afd45721c..609beea6e 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -50,6 +50,7 @@ use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\LabelMapper; use OCP\IUserManager; use OCA\Deck\BadRequestException; +use OCP\IURLGenerator; class BoardService { private $boardMapper; @@ -68,8 +69,8 @@ class BoardService { private $activityManager; private $eventDispatcher; private $changeHelper; - private $boardsCache = null; + private $urlGenerator; public function __construct( @@ -87,6 +88,7 @@ class BoardService { ActivityManager $activityManager, IEventDispatcher $eventDispatcher, ChangeHelper $changeHelper, + IURLGenerator $urlGenerator, $userId ) { $this->boardMapper = $boardMapper; @@ -104,6 +106,7 @@ class BoardService { $this->eventDispatcher = $eventDispatcher; $this->changeHelper = $changeHelper; $this->userId = $userId; + $this->urlGenerator = $urlGenerator; } /** @@ -694,4 +697,8 @@ class BoardService { } $board->setUsers(array_values($boardUsers)); } + + public function getBoardUrl($endpoint) { + return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . '#' . $endpoint; + } } diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 729570ab1..d925f67d9 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -46,6 +46,7 @@ use OCA\Deck\BadRequestException; use OCP\Comments\ICommentsManager; use OCP\EventDispatcher\IEventDispatcher; use OCP\IUserManager; +use OCP\IURLGenerator; class CardService { private $cardMapper; @@ -63,6 +64,7 @@ class CardService { private $changeHelper; private $eventDispatcher; private $userManager; + private $urlGenerator; public function __construct( CardMapper $cardMapper, @@ -79,6 +81,7 @@ class CardService { IUserManager $userManager, ChangeHelper $changeHelper, IEventDispatcher $eventDispatcher, + IURLGenerator $urlGenerator, $userId ) { $this->cardMapper = $cardMapper; @@ -96,6 +99,7 @@ class CardService { $this->changeHelper = $changeHelper; $this->eventDispatcher = $eventDispatcher; $this->currentUser = $userId; + $this->urlGenerator = $urlGenerator; } public function enrich($card) { @@ -602,4 +606,14 @@ class CardService { $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card)); } + + public function getCardUrl($cardId) { + $boardId = $this->cardMapper->findBoardId($cardId); + + return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . "#/board/$boardId/card/$cardId"; + } + + public function getRedirectUrlForCard($cardId) { + return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . "card/$cardId"; + } } diff --git a/tests/unit/Activity/DeckProviderTest.php b/tests/unit/Activity/DeckProviderTest.php index 409dd1432..4302834cb 100644 --- a/tests/unit/Activity/DeckProviderTest.php +++ b/tests/unit/Activity/DeckProviderTest.php @@ -38,6 +38,7 @@ use OCP\L10N\IFactory; use OCP\RichObjectStrings\IValidator; use PHPUnit\Framework\TestCase; use PHPUnit_Framework_MockObject_MockObject as MockObject; +use OCA\Deck\Service\CardService; class DeckProviderTest extends TestCase { @@ -56,6 +57,9 @@ class DeckProviderTest extends TestCase { /** @var ICommentsManager|MockObject */ private $commentsManager; + /** @var CardService|MockObject */ + private $cardService; + /** @var string */ private $userId = 'admin'; @@ -67,7 +71,9 @@ class DeckProviderTest extends TestCase { $this->commentsManager = $this->createMock(ICommentsManager::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->config = $this->createMock(IConfig::class); - $this->provider = new DeckProvider($this->urlGenerator, $this->activityManager, $this->userManager, $this->commentsManager, $this->l10nFactory, $this->config, $this->userId); + $this->config = $this->createMock(IConfig::class); + $this->cardService = $this->createMock(CardService::class); + $this->provider = new DeckProvider($this->urlGenerator, $this->activityManager, $this->userManager, $this->commentsManager, $this->l10nFactory, $this->config, $this->userId, $this->cardService); } private function mockEvent($objectType, $objectId, $objectName, $subject, $subjectParameters = []) { diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index 7dba39b5d..e59662489 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -42,6 +42,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\IGroupManager; use \Test\TestCase; +use OCP\IURLGenerator; class BoardServiceTest extends TestCase { @@ -74,6 +75,8 @@ class BoardServiceTest extends TestCase { /** @var IEventDispatcher */ private $eventDispatcher; private $userId = 'admin'; + /** @var IURLGenerator */ + private $urlGenerator; public function setUp(): void { parent::setUp(); @@ -91,6 +94,7 @@ class BoardServiceTest extends TestCase { $this->activityManager = $this->createMock(ActivityManager::class); $this->changeHelper = $this->createMock(ChangeHelper::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->service = new BoardService( $this->boardMapper, @@ -107,6 +111,7 @@ class BoardServiceTest extends TestCase { $this->activityManager, $this->eventDispatcher, $this->changeHelper, + $this->urlGenerator, $this->userId ); diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index 292913686..622b22c5b 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/tests/unit/Service/CardServiceTest.php @@ -43,6 +43,7 @@ use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Test\TestCase; +use OCP\IURLGenerator; class CardServiceTest extends TestCase { @@ -76,6 +77,9 @@ class CardServiceTest extends TestCase { /** @var ChangeHelper|MockObject */ private $changeHelper; + /** @var IURLGenerator|MockObject */ + private $urlGenerator; + public function setUp(): void { parent::setUp(); $this->cardMapper = $this->createMock(CardMapper::class); @@ -92,6 +96,7 @@ class CardServiceTest extends TestCase { $this->userManager = $this->createMock(IUserManager::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->changeHelper = $this->createMock(ChangeHelper::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->cardService = new CardService( $this->cardMapper, $this->stackMapper, @@ -107,6 +112,7 @@ class CardServiceTest extends TestCase { $this->userManager, $this->changeHelper, $this->eventDispatcher, + $this->urlGenerator, 'user1' ); } diff --git a/tests/unit/controller/PageControllerTest.php b/tests/unit/controller/PageControllerTest.php index 508bc9917..a7d78adc3 100644 --- a/tests/unit/controller/PageControllerTest.php +++ b/tests/unit/controller/PageControllerTest.php @@ -24,32 +24,56 @@ namespace OCA\Deck\Controller; +use OCA\Deck\Db\CardMapper; +use OCA\Deck\Service\CardService; use OCA\Deck\Service\ConfigService; use OCA\Deck\Service\PermissionService; use OCP\EventDispatcher\IEventDispatcher; use OCP\IInitialStateService; -use OCP\IL10N; use OCP\IRequest; +use OCP\IURLGenerator; +use PHPUnit\Framework\TestCase; -class PageControllerTest extends \Test\TestCase { +class PageControllerTest extends TestCase { private $controller; private $request; - private $l10n; private $permissionService; private $initialState; private $configService; private $eventDispatcher; + /** + * @var mixed|CardMapper|\PHPUnit\Framework\MockObject\MockObject + */ + private $cardMapper; + /** + * @var mixed|IURLGenerator|\PHPUnit\Framework\MockObject\MockObject + */ + private $urlGenerator; + /** + * @var mixed|CardService|\PHPUnit\Framework\MockObject\MockObject + */ + private $cardService; public function setUp(): void { - $this->l10n = $this->createMock(IL10N::class); $this->request = $this->createMock(IRequest::class); $this->permissionService = $this->createMock(PermissionService::class); $this->configService = $this->createMock(ConfigService::class); $this->initialState = $this->createMock(IInitialStateService::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->cardMapper = $this->createMock(CardMapper::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->cardService = $this->createMock(CardService::class); $this->controller = new PageController( - 'deck', $this->request, $this->permissionService, $this->initialState, $this->configService, $this->eventDispatcher + 'deck', + $this->request, + $this->permissionService, + $this->initialState, + $this->configService, + $this->eventDispatcher, + $this->cardMapper, + $this->urlGenerator, + $this->cardService ); }