fix: generate fixed link for activity emails

Signed-off-by: Luka Trovic <luka@nextcloud.com>

fix: generate fixed link for activity emails

Signed-off-by: Luka Trovic <luka@nextcloud.com>

Fix tests

Signed-off-by: Julius Härtl <jus@bitgrid.net>

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'],

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;
 		}

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'));
+		}
+	}
 }

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;
 	}
 
 	/**
@@ -697,4 +700,8 @@ class BoardService {
 		}
 		$board->setUsers(array_values($boardUsers));
 	}
+
+	public function getBoardUrl($endpoint) {
+		return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . '#' . $endpoint;
+	}
 }

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";
+	}
 }

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 = []) {

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
 		);
 

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'
 		);
 	}

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
 		);
 	}