Skip to content

Commit 165375f

Browse files
luka-nextcloudjuliusknorr
authored andcommitted
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>
1 parent 4b622d8 commit 165375f

9 files changed

Lines changed: 104 additions & 10 deletions

File tree

appinfo/routes.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
return [
2626
'routes' => [
2727
['name' => 'page#index', 'url' => '/', 'verb' => 'GET'],
28+
['name' => 'page#redirectToCard', 'url' => '/card/{cardId}', 'verb' => 'GET'],
2829

2930
// boards
3031
['name' => 'board#index', 'url' => '/boards', 'verb' => 'GET'],

lib/Activity/DeckProvider.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use OCP\IURLGenerator;
3636
use OCP\IUserManager;
3737
use OCP\L10N\IFactory;
38+
use OCA\Deck\Service\CardService;
3839

3940
class DeckProvider implements IProvider {
4041

@@ -52,15 +53,18 @@ class DeckProvider implements IProvider {
5253
private $l10nFactory;
5354
/** @var IConfig */
5455
private $config;
56+
/** @var CardService */
57+
private $cardService;
5558

56-
public function __construct(IURLGenerator $urlGenerator, ActivityManager $activityManager, IUserManager $userManager, ICommentsManager $commentsManager, IFactory $l10n, IConfig $config, $userId) {
59+
public function __construct(IURLGenerator $urlGenerator, ActivityManager $activityManager, IUserManager $userManager, ICommentsManager $commentsManager, IFactory $l10n, IConfig $config, $userId, CardService $cardService) {
5760
$this->userId = $userId;
5861
$this->urlGenerator = $urlGenerator;
5962
$this->activityManager = $activityManager;
6063
$this->commentsManager = $commentsManager;
6164
$this->userManager = $userManager;
6265
$this->l10nFactory = $l10n;
6366
$this->config = $config;
67+
$this->cardService = $cardService;
6468
}
6569

6670
/**
@@ -131,7 +135,7 @@ public function parse($language, IEvent $event, IEvent $previousEvent = null) {
131135

132136
if (array_key_exists('board', $subjectParams)) {
133137
$archivedParam = $subjectParams['card']['archived'] ? 'archived/' : '';
134-
$card['link'] = $this->deckUrl('/board/' . $subjectParams['board']['id'] . '/' . $archivedParam . 'card/' . $event->getObjectId());
138+
$card['link'] = $this->cardService->getRedirectUrlForCard($event->getObjectId());
135139
}
136140
$params['card'] = $card;
137141
}

lib/Controller/PageController.php

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,41 @@
3434
use OCP\IRequest;
3535
use OCP\AppFramework\Http\TemplateResponse;
3636
use OCP\AppFramework\Controller;
37+
use OCA\Deck\Db\CardMapper;
38+
use OCP\IURLGenerator;
39+
use \OCP\AppFramework\Http\RedirectResponse;
40+
use OCA\Deck\Db\Acl;
41+
use OCA\Deck\Service\CardService;
3742

3843
class PageController extends Controller {
3944
private $permissionService;
4045
private $initialState;
4146
private $configService;
4247
private $eventDispatcher;
48+
private $cardMapper;
49+
private $urlGenerator;
50+
private $cardService;
4351

4452
public function __construct(
4553
$AppName,
4654
IRequest $request,
4755
PermissionService $permissionService,
4856
IInitialStateService $initialStateService,
4957
ConfigService $configService,
50-
IEventDispatcher $eventDispatcher
58+
IEventDispatcher $eventDispatcher,
59+
CardMapper $cardMapper,
60+
IURLGenerator $urlGenerator,
61+
CardService $cardService
5162
) {
5263
parent::__construct($AppName, $request);
5364

5465
$this->permissionService = $permissionService;
5566
$this->initialState = $initialStateService;
5667
$this->configService = $configService;
5768
$this->eventDispatcher = $eventDispatcher;
69+
$this->cardMapper = $cardMapper;
70+
$this->urlGenerator = $urlGenerator;
71+
$this->cardService = $cardService;
5872
}
5973

6074
/**
@@ -85,4 +99,17 @@ public function index() {
8599

86100
return $response;
87101
}
102+
103+
/**
104+
* @NoAdminRequired
105+
* @NoCSRFRequired
106+
*/
107+
public function redirectToCard($cardId): RedirectResponse {
108+
try {
109+
$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ);
110+
return new RedirectResponse($this->cardService->getCardUrl($cardId));
111+
} catch (\Exception $e) {
112+
return new RedirectResponse($this->urlGenerator->linkToRouteAbsolute('deck.page.index'));
113+
}
114+
}
88115
}

lib/Service/BoardService.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
use OCA\Deck\Db\LabelMapper;
5151
use OCP\IUserManager;
5252
use OCA\Deck\BadRequestException;
53+
use OCP\IURLGenerator;
5354

5455
class BoardService {
5556
private $boardMapper;
@@ -68,8 +69,8 @@ class BoardService {
6869
private $activityManager;
6970
private $eventDispatcher;
7071
private $changeHelper;
71-
7272
private $boardsCache = null;
73+
private $urlGenerator;
7374

7475

7576
public function __construct(
@@ -87,6 +88,7 @@ public function __construct(
8788
ActivityManager $activityManager,
8889
IEventDispatcher $eventDispatcher,
8990
ChangeHelper $changeHelper,
91+
IURLGenerator $urlGenerator,
9092
$userId
9193
) {
9294
$this->boardMapper = $boardMapper;
@@ -104,6 +106,7 @@ public function __construct(
104106
$this->eventDispatcher = $eventDispatcher;
105107
$this->changeHelper = $changeHelper;
106108
$this->userId = $userId;
109+
$this->urlGenerator = $urlGenerator;
107110
}
108111

109112
/**
@@ -697,4 +700,8 @@ private function enrichWithUsers($board, $since = -1) {
697700
}
698701
$board->setUsers(array_values($boardUsers));
699702
}
703+
704+
public function getBoardUrl($endpoint) {
705+
return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . '#' . $endpoint;
706+
}
700707
}

lib/Service/CardService.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
use OCP\Comments\ICommentsManager;
4747
use OCP\EventDispatcher\IEventDispatcher;
4848
use OCP\IUserManager;
49+
use OCP\IURLGenerator;
4950

5051
class CardService {
5152
private $cardMapper;
@@ -63,6 +64,7 @@ class CardService {
6364
private $changeHelper;
6465
private $eventDispatcher;
6566
private $userManager;
67+
private $urlGenerator;
6668

6769
public function __construct(
6870
CardMapper $cardMapper,
@@ -79,6 +81,7 @@ public function __construct(
7981
IUserManager $userManager,
8082
ChangeHelper $changeHelper,
8183
IEventDispatcher $eventDispatcher,
84+
IURLGenerator $urlGenerator,
8285
$userId
8386
) {
8487
$this->cardMapper = $cardMapper;
@@ -96,6 +99,7 @@ public function __construct(
9699
$this->changeHelper = $changeHelper;
97100
$this->eventDispatcher = $eventDispatcher;
98101
$this->currentUser = $userId;
102+
$this->urlGenerator = $urlGenerator;
99103
}
100104

101105
public function enrich($card) {
@@ -602,4 +606,14 @@ public function removeLabel($cardId, $labelId) {
602606

603607
$this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card));
604608
}
609+
610+
public function getCardUrl($cardId) {
611+
$boardId = $this->cardMapper->findBoardId($cardId);
612+
613+
return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . "#/board/$boardId/card/$cardId";
614+
}
615+
616+
public function getRedirectUrlForCard($cardId) {
617+
return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . "card/$cardId";
618+
}
605619
}

tests/unit/Activity/DeckProviderTest.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
use OCP\RichObjectStrings\IValidator;
3939
use PHPUnit\Framework\TestCase;
4040
use PHPUnit_Framework_MockObject_MockObject as MockObject;
41+
use OCA\Deck\Service\CardService;
4142

4243
class DeckProviderTest extends TestCase {
4344

@@ -56,6 +57,9 @@ class DeckProviderTest extends TestCase {
5657
/** @var ICommentsManager|MockObject */
5758
private $commentsManager;
5859

60+
/** @var CardService|MockObject */
61+
private $cardService;
62+
5963
/** @var string */
6064
private $userId = 'admin';
6165

@@ -67,7 +71,9 @@ public function setUp(): void {
6771
$this->commentsManager = $this->createMock(ICommentsManager::class);
6872
$this->l10nFactory = $this->createMock(IFactory::class);
6973
$this->config = $this->createMock(IConfig::class);
70-
$this->provider = new DeckProvider($this->urlGenerator, $this->activityManager, $this->userManager, $this->commentsManager, $this->l10nFactory, $this->config, $this->userId);
74+
$this->config = $this->createMock(IConfig::class);
75+
$this->cardService = $this->createMock(CardService::class);
76+
$this->provider = new DeckProvider($this->urlGenerator, $this->activityManager, $this->userManager, $this->commentsManager, $this->l10nFactory, $this->config, $this->userId, $this->cardService);
7177
}
7278

7379
private function mockEvent($objectType, $objectId, $objectName, $subject, $subjectParameters = []) {

tests/unit/Service/BoardServiceTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use OCP\IUserManager;
4343
use OCP\IGroupManager;
4444
use \Test\TestCase;
45+
use OCP\IURLGenerator;
4546

4647
class BoardServiceTest extends TestCase {
4748

@@ -74,6 +75,8 @@ class BoardServiceTest extends TestCase {
7475
/** @var IEventDispatcher */
7576
private $eventDispatcher;
7677
private $userId = 'admin';
78+
/** @var IURLGenerator */
79+
private $urlGenerator;
7780

7881
public function setUp(): void {
7982
parent::setUp();
@@ -91,6 +94,7 @@ public function setUp(): void {
9194
$this->activityManager = $this->createMock(ActivityManager::class);
9295
$this->changeHelper = $this->createMock(ChangeHelper::class);
9396
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
97+
$this->urlGenerator = $this->createMock(IURLGenerator::class);
9498

9599
$this->service = new BoardService(
96100
$this->boardMapper,
@@ -107,6 +111,7 @@ public function setUp(): void {
107111
$this->activityManager,
108112
$this->eventDispatcher,
109113
$this->changeHelper,
114+
$this->urlGenerator,
110115
$this->userId
111116
);
112117

tests/unit/Service/CardServiceTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
use PHPUnit\Framework\MockObject\MockObject;
4444
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
4545
use Test\TestCase;
46+
use OCP\IURLGenerator;
4647

4748
class CardServiceTest extends TestCase {
4849

@@ -76,6 +77,9 @@ class CardServiceTest extends TestCase {
7677
/** @var ChangeHelper|MockObject */
7778
private $changeHelper;
7879

80+
/** @var IURLGenerator|MockObject */
81+
private $urlGenerator;
82+
7983
public function setUp(): void {
8084
parent::setUp();
8185
$this->cardMapper = $this->createMock(CardMapper::class);
@@ -92,6 +96,7 @@ public function setUp(): void {
9296
$this->userManager = $this->createMock(IUserManager::class);
9397
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
9498
$this->changeHelper = $this->createMock(ChangeHelper::class);
99+
$this->urlGenerator = $this->createMock(IURLGenerator::class);
95100
$this->cardService = new CardService(
96101
$this->cardMapper,
97102
$this->stackMapper,
@@ -107,6 +112,7 @@ public function setUp(): void {
107112
$this->userManager,
108113
$this->changeHelper,
109114
$this->eventDispatcher,
115+
$this->urlGenerator,
110116
'user1'
111117
);
112118
}

tests/unit/controller/PageControllerTest.php

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,32 +24,56 @@
2424

2525
namespace OCA\Deck\Controller;
2626

27+
use OCA\Deck\Db\CardMapper;
28+
use OCA\Deck\Service\CardService;
2729
use OCA\Deck\Service\ConfigService;
2830
use OCA\Deck\Service\PermissionService;
2931
use OCP\EventDispatcher\IEventDispatcher;
3032
use OCP\IInitialStateService;
31-
use OCP\IL10N;
3233
use OCP\IRequest;
34+
use OCP\IURLGenerator;
35+
use PHPUnit\Framework\TestCase;
3336

34-
class PageControllerTest extends \Test\TestCase {
37+
class PageControllerTest extends TestCase {
3538
private $controller;
3639
private $request;
37-
private $l10n;
3840
private $permissionService;
3941
private $initialState;
4042
private $configService;
4143
private $eventDispatcher;
44+
/**
45+
* @var mixed|CardMapper|\PHPUnit\Framework\MockObject\MockObject
46+
*/
47+
private $cardMapper;
48+
/**
49+
* @var mixed|IURLGenerator|\PHPUnit\Framework\MockObject\MockObject
50+
*/
51+
private $urlGenerator;
52+
/**
53+
* @var mixed|CardService|\PHPUnit\Framework\MockObject\MockObject
54+
*/
55+
private $cardService;
4256

4357
public function setUp(): void {
44-
$this->l10n = $this->createMock(IL10N::class);
4558
$this->request = $this->createMock(IRequest::class);
4659
$this->permissionService = $this->createMock(PermissionService::class);
4760
$this->configService = $this->createMock(ConfigService::class);
4861
$this->initialState = $this->createMock(IInitialStateService::class);
4962
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
63+
$this->cardMapper = $this->createMock(CardMapper::class);
64+
$this->urlGenerator = $this->createMock(IURLGenerator::class);
65+
$this->cardService = $this->createMock(CardService::class);
5066

5167
$this->controller = new PageController(
52-
'deck', $this->request, $this->permissionService, $this->initialState, $this->configService, $this->eventDispatcher
68+
'deck',
69+
$this->request,
70+
$this->permissionService,
71+
$this->initialState,
72+
$this->configService,
73+
$this->eventDispatcher,
74+
$this->cardMapper,
75+
$this->urlGenerator,
76+
$this->cardService
5377
);
5478
}
5579

0 commit comments

Comments
 (0)