From 9f98f0bd5fe85cd6e42688467c5bca9a75090fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Sat, 11 Nov 2023 23:02:22 +0100 Subject: [PATCH 1/4] fix: Switch to route based urls (fix #4467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- appinfo/routes.php | 5 +++++ lib/Controller/PageController.php | 26 ++++++++++++++++++++++++++ src/router.js | 1 + 3 files changed, 32 insertions(+) diff --git a/appinfo/routes.php b/appinfo/routes.php index 467a3ddc6..dbfc60325 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -25,6 +25,11 @@ return [ 'routes' => [ ['name' => 'page#index', 'url' => '/', 'verb' => 'GET'], + ['name' => 'page#indexList', 'url' => '/board', 'verb' => 'GET'], + ['name' => 'page#indexBoard', 'url' => '/board/{boardId}', 'verb' => 'GET'], + ['name' => 'page#indexBoardDetails', 'url' => '/board/{boardId}/details', 'verb' => 'GET'], + ['name' => 'page#indexCard', 'url' => '/board/{boardId}/card/{cardId}', 'verb' => 'GET'], + ['name' => 'page#redirectToCard', 'url' => '/card/{cardId}', 'verb' => 'GET'], // boards diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 8008d187d..acc9b2a6f 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -23,6 +23,8 @@ namespace OCA\Deck\Controller; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use \OCP\AppFramework\Http\RedirectResponse; use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\Acl; @@ -99,6 +101,30 @@ class PageController extends Controller { return $response; } + #[NoAdminRequired] + #[NoCSRFRequired] + public function indexList(): TemplateResponse { + return $this->index(); + } + + #[NoAdminRequired] + #[NoCSRFRequired] + public function indexBoard(int $boardId): TemplateResponse { + return $this->index($boardId); + } + + #[NoAdminRequired] + #[NoCSRFRequired] + public function indexBoardDetails(int $boardId): TemplateResponse { + return $this->index($boardId); + } + + #[NoAdminRequired] + #[NoCSRFRequired] + public function indexCard(int $cardId): TemplateResponse { + return $this->index(cardId: $cardId); + } + /** * @NoAdminRequired * @NoCSRFRequired diff --git a/src/router.js b/src/router.js index 4a4fa961a..7825ad5c1 100644 --- a/src/router.js +++ b/src/router.js @@ -34,6 +34,7 @@ import Overview from './components/overview/Overview.vue' Vue.use(Router) export default new Router({ + mode: 'history', base: generateUrl('/apps/deck/'), linkActiveClass: 'active', routes: [ From 7127f8831848770229452e7f091c18a229f08d5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Sat, 11 Nov 2023 23:07:16 +0100 Subject: [PATCH 2/4] fix: Keep existing links working MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- src/router.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/router.js b/src/router.js index 7825ad5c1..998034c01 100644 --- a/src/router.js +++ b/src/router.js @@ -33,7 +33,7 @@ import Overview from './components/overview/Overview.vue' Vue.use(Router) -export default new Router({ +const router = new Router({ mode: 'history', base: generateUrl('/apps/deck/'), linkActiveClass: 'active', @@ -158,3 +158,15 @@ export default new Router({ }, ], }) + +router.beforeEach((to, from, next) => { + // Redirect if fullPath begins with a hash (ignore hashes later in path) + if (to.fullPath.substring(0, 2) === '/#') { + const path = to.fullPath.substring(2) + next(path) + return + } + next() +}) + +export default router From 8ccc12433ed0685df083498e30ee520aac8d3138 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Sat, 11 Nov 2023 23:15:16 +0100 Subject: [PATCH 3/4] fix: Adapt reference url patterns to new routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Reference/BoardReferenceProvider.php | 8 +-- lib/Reference/CardReferenceProvider.php | 25 ++++----- .../Reference/CardReferenceProviderTest.php | 54 ++++++++++++++++--- 3 files changed, 65 insertions(+), 22 deletions(-) diff --git a/lib/Reference/BoardReferenceProvider.php b/lib/Reference/BoardReferenceProvider.php index 62f0a7362..31932bd6f 100644 --- a/lib/Reference/BoardReferenceProvider.php +++ b/lib/Reference/BoardReferenceProvider.php @@ -55,8 +55,8 @@ class BoardReferenceProvider implements IReferenceProvider { $startIndex = $this->urlGenerator->getAbsoluteURL('/index.php/apps/' . Application::APP_ID); // link example: https://nextcloud.local/index.php/apps/deck/#/board/2 - $noIndexMatch = preg_match('/^' . preg_quote($start, '/') . '\/#\/board\/[0-9]+$/', $referenceText) === 1; - $indexMatch = preg_match('/^' . preg_quote($startIndex, '/') . '\/#\/board\/[0-9]+$/', $referenceText) === 1; + $noIndexMatch = preg_match('/^' . preg_quote($start, '/') . '(?:\/#!?)?\/board\/[0-9]+$/', $referenceText) === 1; + $indexMatch = preg_match('/^' . preg_quote($startIndex, '/') . '(?:\/#!?)?\/board\/[0-9]+$/', $referenceText) === 1; return $noIndexMatch || $indexMatch; } @@ -108,9 +108,9 @@ class BoardReferenceProvider implements IReferenceProvider { $start = $this->urlGenerator->getAbsoluteURL('/apps/' . Application::APP_ID); $startIndex = $this->urlGenerator->getAbsoluteURL('/index.php/apps/' . Application::APP_ID); - preg_match('/^' . preg_quote($start, '/') . '\/#\/board\/([0-9]+)$/', $url, $matches); + preg_match('/^' . preg_quote($start, '/') . '(?:\/#!?)?\/board\/([0-9]+)$/', $url, $matches); if (!$matches) { - preg_match('/^' . preg_quote($startIndex, '/') . '\/#\/board\/([0-9]+)$/', $url, $matches); + preg_match('/^' . preg_quote($startIndex, '/') . '(?:\/#!?)?\/board\/([0-9]+)$/', $url, $matches); } if ($matches && count($matches) > 1) { return (int) $matches[1]; diff --git a/lib/Reference/CardReferenceProvider.php b/lib/Reference/CardReferenceProvider.php index f3dcac5cd..d11e83e74 100644 --- a/lib/Reference/CardReferenceProvider.php +++ b/lib/Reference/CardReferenceProvider.php @@ -108,8 +108,8 @@ class CardReferenceProvider extends ADiscoverableReferenceProvider implements IS $startIndex = $this->urlGenerator->getAbsoluteURL('/index.php/apps/' . Application::APP_ID); // link example: https://nextcloud.local/index.php/apps/deck/#/board/2/card/11 - $noIndexMatchFull = preg_match('/^' . preg_quote($start, '/') . '\/#\/board\/[0-9]+\/card\/[0-9]+$/', $referenceText) === 1; - $indexMatchFull = preg_match('/^' . preg_quote($startIndex, '/') . '\/#\/board\/[0-9]+\/card\/[0-9]+$/', $referenceText) === 1; + $noIndexMatchFull = preg_match('/^' . preg_quote($start, '/') . '(?:\/#!?)?\/board\/[0-9]+\/card\/[0-9]+$/', $referenceText) === 1; + $indexMatchFull = preg_match('/^' . preg_quote($startIndex, '/') . '(?:\/#!?)?\/board\/[0-9]+\/card\/[0-9]+$/', $referenceText) === 1; // link example: https://nextcloud.local/index.php/apps/deck/card/11 $noIndexMatch = preg_match('/^' . preg_quote($start, '/') . '\/card\/[0-9]+$/', $referenceText) === 1; @@ -125,16 +125,17 @@ class CardReferenceProvider extends ADiscoverableReferenceProvider implements IS if ($this->matchReference($referenceText)) { $ids = $this->getBoardCardId($referenceText); if ($ids !== null) { - [$boardId, $cardId] = $ids; + [, $cardId] = $ids; try { $card = $this->cardService->find((int) $cardId)->jsonSerialize(); $stack = $this->stackService->find((int) $card['stackId'])->jsonSerialize(); - $board = $this->boardService->find((int)($boardId ?? $stack['boardId']))->jsonSerialize(); + $board = $this->boardService->find((int) $stack['boardId'])->jsonSerialize(); } catch (NoPermissionException $e) { // Skip throwing if user has no permissions return null; } + $boardId = $board['id']; $card = $this->sanitizeSerializedCard($card); $board = $this->sanitizeSerializedBoard($board); @@ -159,14 +160,14 @@ class CardReferenceProvider extends ADiscoverableReferenceProvider implements IS $result = $cardDetails->jsonSerialize(); unset($result['assignedUsers']); return $result; - }, $stack['cards']); + }, $stack['cards'] ?? []); return $stack; } private function sanitizeSerializedBoard(array $board): array { unset($board['labels']); - $board['owner'] = $board['owner']->jsonSerialize(); + $board['owner'] = $board['owner']?->jsonSerialize(); unset($board['acl']); unset($board['users']); @@ -176,18 +177,18 @@ class CardReferenceProvider extends ADiscoverableReferenceProvider implements IS private function sanitizeSerializedCard(array $card): array { $card['labels'] = array_map(function (Label $label) { return $label->jsonSerialize(); - }, $card['labels']); + }, $card['labels'] ?? []); $card['assignedUsers'] = array_map(function (Assignment $assignment) { $result = $assignment->jsonSerialize(); $result['participant'] = $result['participant']->jsonSerialize(); return $result; - }, $card['assignedUsers']); - $card['owner'] = $card['owner']->jsonSerialize(); + }, $card['assignedUsers'] ?? []); + $card['owner'] = $card['owner']?->jsonSerialize() ?? $card['owner']; unset($card['relatedStack']); unset($card['relatedBoard']); $card['attachments'] = array_map(function (Attachment $attachment) { return $attachment->jsonSerialize(); - }, $card['attachments']); + }, $card['attachments'] ?? []); return $card; } @@ -196,12 +197,12 @@ class CardReferenceProvider extends ADiscoverableReferenceProvider implements IS $start = $this->urlGenerator->getAbsoluteURL('/apps/' . Application::APP_ID); $startIndex = $this->urlGenerator->getAbsoluteURL('/index.php/apps/' . Application::APP_ID); - preg_match('/^' . preg_quote($start, '/') . '\/#\/board\/([0-9]+)\/card\/([0-9]+)$/', $url, $matches); + preg_match('/^' . preg_quote($start, '/') . '(?:\/#!?)?\/board\/([0-9]+)\/card\/([0-9]+)$/', $url, $matches); if ($matches && count($matches) > 2) { return [$matches[1], $matches[2]]; } - preg_match('/^' . preg_quote($startIndex, '/') . '\/#\/board\/([0-9]+)\/card\/([0-9]+)$/', $url, $matches2); + preg_match('/^' . preg_quote($startIndex, '/') . '(?:\/#!?)?\/board\/([0-9]+)\/card\/([0-9]+)$/', $url, $matches2); if ($matches2 && count($matches2) > 2) { return [$matches2[1], $matches2[2]]; } diff --git a/tests/unit/Reference/CardReferenceProviderTest.php b/tests/unit/Reference/CardReferenceProviderTest.php index 3824d98af..de2ac1362 100644 --- a/tests/unit/Reference/CardReferenceProviderTest.php +++ b/tests/unit/Reference/CardReferenceProviderTest.php @@ -22,6 +22,9 @@ namespace Reference; +use OCA\Deck\Db\Board; +use OCA\Deck\Db\Card; +use OCA\Deck\Db\Stack; use OCA\Deck\Reference\CardReferenceProvider; use OCA\Deck\Service\BoardService; use OCA\Deck\Service\CardService; @@ -51,16 +54,55 @@ class CardReferenceProviderTest extends TestCase { ); } - public function testUrl() { + public static function dataUrl(): array { + return [ + ['https://nextcloud.com', null], + ['https://localhost/apps/deck/#!/board/2/card/11', 11], + ['https://localhost/index.php/apps/deck/#!/board/2/card/11', 11], + ['https://localhost/apps/deck/#/board/2/card/11', 11], + ['https://localhost/index.php/apps/deck/#/board/2/card/11', 11], + ['https://localhost/apps/deck/board/2/card/11', 11], + ['https://localhost/index.php/apps/deck/board/2/card/11', 11], + ['https://localhost/apps/deck/card/11', 11], + ['https://localhost/index.php/apps/deck/card/11', 11], + ]; + } + + /** + * @dataProvider dataUrl + */ + public function testUrl($url, $id) { $this->urlGenerator->expects($this->any()) ->method('getAbsoluteURL') ->willReturnCallback(function ($path) { return 'https://localhost/' . ltrim($path, '/'); }); - self::assertFalse($this->provider->matchReference('https://nextcloud.com')); - self::assertTrue($this->provider->matchReference('https://localhost/apps/deck/#/board/2/card/11')); - self::assertTrue($this->provider->matchReference('https://localhost/index.php/apps/deck/#/board/2/card/11')); - self::assertTrue($this->provider->matchReference('https://localhost/apps/deck/card/11')); - self::assertTrue($this->provider->matchReference('https://localhost/index.php/apps/deck/card/11')); + $matchExpect = $id !== null; + self::assertEquals($matchExpect, $this->provider->matchReference($url)); + + $card = Card::fromRow([ + 'id' => $id, + 'stackId' => 1234, + ]); + $stack = Stack::fromRow([ + 'boardId' => 9876, + ]); + $board = Board::fromRow([ + 'id' => 9876, + ]); + + $this->cardService->method('find')->with($id)->willReturn($card); + $this->stackService->method('find')->with(1234)->willReturn($stack); + $this->boardService->method('find')->with(9876)->willReturn($board); + + $reference = $this->provider->resolveReference($url); + + if ($id !== null) { + self::assertEquals($id, $reference->jsonSerialize()['richObject']['card']['id']); + self::assertEquals(9876, $reference->jsonSerialize()['richObject']['board']['id']); + + } else { + self::assertNull($reference); + } } } From 1a57c107411d765582af682ef1d07b6ab671c767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Sun, 12 Nov 2023 00:52:57 +0100 Subject: [PATCH 4/4] perf: Already pass board list as initial state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Controller/PageController.php | 46 ++++++++----------- src/App.vue | 6 ++- .../navigation/AppNavigationBoardCategory.vue | 3 ++ src/store/main.js | 2 +- tests/unit/controller/PageControllerTest.php | 8 +++- 5 files changed, 34 insertions(+), 31 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index acc9b2a6f..7c81a6093 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -23,12 +23,10 @@ namespace OCA\Deck\Controller; -use OCP\AppFramework\Http\Attribute\NoAdminRequired; -use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use \OCP\AppFramework\Http\RedirectResponse; -use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\Acl; use OCA\Deck\Db\CardMapper; +use OCA\Deck\Service\BoardService; use OCA\Deck\Service\CardService; use OCA\Deck\Service\ConfigService; use OCA\Deck\Service\PermissionService; @@ -36,23 +34,24 @@ use OCA\Files\Event\LoadSidebar; use OCA\Text\Event\LoadEditor; use OCA\Viewer\Event\LoadViewer; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Services\IInitialState; use OCP\Collaboration\Resources\LoadAdditionalScriptsEvent as CollaborationResourcesEvent; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; -use OCP\IInitialStateService; use OCP\IRequest; use OCP\IURLGenerator; class PageController extends Controller { - private IInitialStateService $initialState; - public function __construct( string $AppName, IRequest $request, private PermissionService $permissionService, - IInitialStateService $initialStateService, + private IInitialState $initialState, + private BoardService $boardService, private ConfigService $configService, private IEventDispatcher $eventDispatcher, private CardMapper $cardMapper, @@ -61,21 +60,16 @@ class PageController extends Controller { private IConfig $config, ) { parent::__construct($AppName, $request); - - $this->initialState = $initialStateService; } - /** - * Handle main html view from templates/main.php - * This will return the main angular application - * - * @NoAdminRequired - * @NoCSRFRequired - */ - public function index() { - $this->initialState->provideInitialState(Application::APP_ID, 'maxUploadSize', (int)\OCP\Util::uploadLimit()); - $this->initialState->provideInitialState(Application::APP_ID, 'canCreate', $this->permissionService->canCreate()); - $this->initialState->provideInitialState(Application::APP_ID, 'config', $this->configService->getAll()); + #[NoAdminRequired] + #[NoCSRFRequired] + public function index(): TemplateResponse { + $this->initialState->provideInitialState('maxUploadSize', (int)\OCP\Util::uploadLimit()); + $this->initialState->provideInitialState('canCreate', $this->permissionService->canCreate()); + $this->initialState->provideInitialState('config', $this->configService->getAll()); + + $this->initialState->provideInitialState('initialBoards', $this->boardService->findAll()); $this->eventDispatcher->dispatchTyped(new LoadSidebar()); $this->eventDispatcher->dispatchTyped(new CollaborationResourcesEvent()); @@ -110,25 +104,23 @@ class PageController extends Controller { #[NoAdminRequired] #[NoCSRFRequired] public function indexBoard(int $boardId): TemplateResponse { - return $this->index($boardId); + return $this->index(); } #[NoAdminRequired] #[NoCSRFRequired] public function indexBoardDetails(int $boardId): TemplateResponse { - return $this->index($boardId); + return $this->index(); } #[NoAdminRequired] #[NoCSRFRequired] public function indexCard(int $cardId): TemplateResponse { - return $this->index(cardId: $cardId); + return $this->index(); } - /** - * @NoAdminRequired - * @NoCSRFRequired - */ + #[NoAdminRequired] + #[NoCSRFRequired] public function redirectToCard($cardId): RedirectResponse { try { $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ); diff --git a/src/App.vue b/src/App.vue index 920bf4a3f..c1fc4eaa8 100644 --- a/src/App.vue +++ b/src/App.vue @@ -49,6 +49,7 @@ import AppNavigation from './components/navigation/AppNavigation.vue' import { NcModal, NcContent, NcAppContent } from '@nextcloud/vue' import { BoardApi } from './services/BoardApi.js' import { emit, subscribe } from '@nextcloud/event-bus' +import { loadState } from '@nextcloud/initial-state' const boardApi = new BoardApi() @@ -108,7 +109,10 @@ export default { }, }, created() { - this.$store.dispatch('loadBoards') + const initialState = loadState('deck', 'initialBoards', null) + if (initialState !== null) { + this.$store.dispatch('loadBoards') + } this.$store.dispatch('loadSharees') }, mounted() { diff --git a/src/components/navigation/AppNavigationBoardCategory.vue b/src/components/navigation/AppNavigationBoardCategory.vue index 805128174..069777f81 100644 --- a/src/components/navigation/AppNavigationBoardCategory.vue +++ b/src/components/navigation/AppNavigationBoardCategory.vue @@ -89,5 +89,8 @@ export default { } }, }, + mounted() { + this.opened = this.boards.length > 0 + }, } diff --git a/src/store/main.js b/src/store/main.js index 3df2e70fa..2692e84d8 100644 --- a/src/store/main.js +++ b/src/store/main.js @@ -66,7 +66,7 @@ export default new Vuex.Store({ sidebarShown: false, currentBoard: null, currentCard: null, - boards: [], + boards: loadState('deck', 'initialBoards', []), sharees: [], assignableUsers: [], boardFilter: BOARD_FILTERS.ALL, diff --git a/tests/unit/controller/PageControllerTest.php b/tests/unit/controller/PageControllerTest.php index f2ed9edfa..f596567a0 100644 --- a/tests/unit/controller/PageControllerTest.php +++ b/tests/unit/controller/PageControllerTest.php @@ -25,12 +25,13 @@ namespace OCA\Deck\Controller; use OCA\Deck\Db\CardMapper; +use OCA\Deck\Service\BoardService; use OCA\Deck\Service\CardService; use OCA\Deck\Service\ConfigService; use OCA\Deck\Service\PermissionService; +use OCP\AppFramework\Services\IInitialState; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; -use OCP\IInitialStateService; use OCP\IRequest; use OCP\IURLGenerator; use PHPUnit\Framework\TestCase; @@ -40,6 +41,7 @@ class PageControllerTest extends TestCase { private $request; private $permissionService; private $initialState; + private $boardService; private $configService; private $eventDispatcher; /** @@ -61,7 +63,8 @@ class PageControllerTest extends TestCase { $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->initialState = $this->createMock(IInitialState::class); + $this->boardService = $this->createMock(BoardService::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->cardMapper = $this->createMock(CardMapper::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); @@ -73,6 +76,7 @@ class PageControllerTest extends TestCase { $this->request, $this->permissionService, $this->initialState, + $this->boardService, $this->configService, $this->eventDispatcher, $this->cardMapper,