From 1b67542e0b20d548963087d208c3ee1633e1a5a2 Mon Sep 17 00:00:00 2001 From: John Molakvoæ Date: Thu, 17 Aug 2023 08:55:30 +0200 Subject: fix(files): trashbin redirect and default fileid Sidebar open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ --- apps/files/appinfo/routes.php | 6 +- apps/files/lib/Controller/ViewController.php | 136 ++++++++++++----- apps/files/src/actions/openInFilesAction.spec.ts | 4 +- apps/files/src/actions/openInFilesAction.ts | 2 +- apps/files/src/actions/sidebarAction.ts | 6 +- apps/files/src/components/FilesListVirtual.vue | 12 +- apps/files/src/views/Sidebar.vue | 58 +++---- apps/files/tests/Controller/ViewControllerTest.php | 169 ++------------------- 8 files changed, 159 insertions(+), 234 deletions(-) (limited to 'apps') diff --git a/apps/files/appinfo/routes.php b/apps/files/appinfo/routes.php index 05d0a37fd70..97f47facff0 100644 --- a/apps/files/appinfo/routes.php +++ b/apps/files/appinfo/routes.php @@ -139,16 +139,14 @@ $application->registerRoutes( 'verb' => 'GET' ], [ - 'name' => 'view#index', + 'name' => 'view#indexView', 'url' => '/{view}', 'verb' => 'GET', - 'postfix' => 'view', ], [ - 'name' => 'view#index', + 'name' => 'view#indexViewFileid', 'url' => '/{view}/{fileid}', 'verb' => 'GET', - 'postfix' => 'fileid', ], ], 'ocs' => [ diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php index e8621c1fa14..8fe7eea01a0 100644 --- a/apps/files/lib/Controller/ViewController.php +++ b/apps/files/lib/Controller/ViewController.php @@ -153,17 +153,36 @@ class ViewController extends Controller { * * @param string $fileid * @return TemplateResponse|RedirectResponse - * @throws NotFoundException */ - public function showFile(string $fileid = null, int $openfile = 1): Response { + public function showFile(string $fileid = null): Response { + if (!$fileid) { + return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index')); + } + // This is the entry point from the `/f/{fileid}` URL which is hardcoded in the server. try { - return $this->redirectToFile($fileid, $openfile !== 0); + return $this->redirectToFile((int) $fileid); } catch (NotFoundException $e) { return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', ['fileNotFound' => true])); } } + + /** + * @NoCSRFRequired + * @NoAdminRequired + * @UseSession + * + * @param string $dir + * @param string $view + * @param string $fileid + * @param bool $fileNotFound + * @return TemplateResponse|RedirectResponse + */ + public function indexView($dir = '', $view = '', $fileid = null, $fileNotFound = false) { + return $this->index($dir, $view, $fileid, $fileNotFound); + } + /** * @NoCSRFRequired * @NoAdminRequired @@ -173,11 +192,30 @@ class ViewController extends Controller { * @param string $view * @param string $fileid * @param bool $fileNotFound - * @param string $openfile - the openfile URL parameter if it was present in the initial request * @return TemplateResponse|RedirectResponse - * @throws NotFoundException */ - public function index($dir = '', $view = '', $fileid = null, $fileNotFound = false, $openfile = null) { + public function indexViewFileid($dir = '', $view = '', $fileid = null, $fileNotFound = false) { + return $this->index($dir, $view, $fileid, $fileNotFound); + } + + /** + * @NoCSRFRequired + * @NoAdminRequired + * @UseSession + * + * @param string $dir + * @param string $view + * @param string $fileid + * @param bool $fileNotFound + * @return TemplateResponse|RedirectResponse + */ + public function index($dir = '', $view = '', $fileid = null, $fileNotFound = false) { + if ($fileid !== null && $view !== 'trashbin') { + try { + return $this->redirectToFileIfInTrashbin((int) $fileid); + } catch (NotFoundException $e) {} + } + // Load the files we need \OCP\Util::addStyle('files', 'merged'); \OCP\Util::addScript('files', 'merged-index', 'files'); @@ -192,8 +230,6 @@ class ViewController extends Controller { $favElements['folders'] = []; } - $contentItems = []; - try { // If view is files, we use the directory, otherwise we use the root storage $storageInfo = $this->getStorageInfo(($view === 'files' && $dir) ? $dir : '/'); @@ -237,19 +273,19 @@ class ViewController extends Controller { $policy->addAllowedWorkerSrcDomain('\'self\''); $response->setContentSecurityPolicy($policy); - $this->provideInitialState($dir, $openfile); + $this->provideInitialState($dir, $fileid); return $response; } /** - * Add openFileInfo in initialState if $openfile is set. + * Add openFileInfo in initialState. * @param string $dir - the ?dir= URL param - * @param string $openfile - the ?openfile= URL param + * @param string $fileid - the fileid URL param * @return void */ - private function provideInitialState(string $dir, ?string $openfile): void { - if ($openfile === null) { + private function provideInitialState(string $dir, ?string $fileid): void { + if ($fileid === null) { return; } @@ -261,7 +297,7 @@ class ViewController extends Controller { $uid = $user->getUID(); $userFolder = $this->rootFolder->getUserFolder($uid); - $nodes = $userFolder->getById((int) $openfile); + $nodes = $userFolder->getById((int) $fileid); $node = array_shift($nodes); if ($node === null) { @@ -293,44 +329,70 @@ class ViewController extends Controller { } /** - * Redirects to the file list and highlight the given file id + * Redirects to the trashbin file list and highlight the given file id * - * @param string $fileId file id to show - * @param bool $setOpenfile - whether or not to set the openfile URL parameter + * @param int $fileId file id to show * @return RedirectResponse redirect response or not found response - * @throws \OCP\Files\NotFoundException + * @throws NotFoundException */ - private function redirectToFile($fileId, bool $setOpenfile = false) { + private function redirectToFileIfInTrashbin($fileId): RedirectResponse { $uid = $this->userSession->getUser()->getUID(); $baseFolder = $this->rootFolder->getUserFolder($uid); - $files = $baseFolder->getById($fileId); + $nodes = $baseFolder->getById($fileId); $params = []; - if (empty($files) && $this->appManager->isEnabledForUser('files_trashbin')) { + if (empty($nodes) && $this->appManager->isEnabledForUser('files_trashbin')) { + /** @var Folder */ $baseFolder = $this->rootFolder->get($uid . '/files_trashbin/files/'); - $files = $baseFolder->getById($fileId); + $nodes = $baseFolder->getById($fileId); $params['view'] = 'trashbin'; + + if (!empty($nodes)) { + $node = current($nodes); + $params['fileid'] = $fileId; + if ($node instanceof Folder) { + // set the full path to enter the folder + $params['dir'] = $baseFolder->getRelativePath($node->getPath()); + } else { + // set parent path as dir + $params['dir'] = $baseFolder->getRelativePath($node->getParent()->getPath()); + } + return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.indexViewFileid', $params)); + } } + throw new NotFoundException(); + } - if (!empty($files)) { - $file = current($files); - if ($file instanceof Folder) { + /** + * Redirects to the file list and highlight the given file id + * + * @param int $fileId file id to show + * @return RedirectResponse redirect response or not found response + * @throws NotFoundException + */ + private function redirectToFile(int $fileId) { + $uid = $this->userSession->getUser()->getUID(); + $baseFolder = $this->rootFolder->getUserFolder($uid); + $nodes = $baseFolder->getById($fileId); + $params = []; + + try { + $this->redirectToFileIfInTrashbin($fileId); + } catch (NotFoundException $e) {} + + if (!empty($nodes)) { + $node = current($nodes); + $params['fileid'] = $fileId; + if ($node instanceof Folder) { // set the full path to enter the folder - $params['dir'] = $baseFolder->getRelativePath($file->getPath()); + $params['dir'] = $baseFolder->getRelativePath($node->getPath()); } else { // set parent path as dir - $params['dir'] = $baseFolder->getRelativePath($file->getParent()->getPath()); - // and scroll to the entry - $params['scrollto'] = $file->getName(); - - if ($setOpenfile) { - // forward the openfile URL parameter. - $params['openfile'] = $fileId; - } + $params['dir'] = $baseFolder->getRelativePath($node->getParent()->getPath()); } - - return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', $params)); + return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.indexViewFileid', $params)); } - throw new \OCP\Files\NotFoundException(); + + throw new NotFoundException(); } } diff --git a/apps/files/src/actions/openInFilesAction.spec.ts b/apps/files/src/actions/openInFilesAction.spec.ts index 302a6c45c6b..4fc402c73a8 100644 --- a/apps/files/src/actions/openInFilesAction.spec.ts +++ b/apps/files/src/actions/openInFilesAction.spec.ts @@ -78,7 +78,7 @@ describe('Open in files action execute tests', () => { // Silent action expect(exec).toBe(null) expect(goToRouteMock).toBeCalledTimes(1) - expect(goToRouteMock).toBeCalledWith(null, { fileid: 1, view: 'files' }, { fileid: 1, dir: '/Foo', openfile: true }) + expect(goToRouteMock).toBeCalledWith(null, { fileid: 1, view: 'files' }, { fileid: 1, dir: '/Foo' }) }) test('Open in files with folder', async () => { @@ -98,6 +98,6 @@ describe('Open in files action execute tests', () => { // Silent action expect(exec).toBe(null) expect(goToRouteMock).toBeCalledTimes(1) - expect(goToRouteMock).toBeCalledWith(null, { fileid: 1, view: 'files' }, { fileid: 1, dir: '/Foo/Bar', openfile: true }) + expect(goToRouteMock).toBeCalledWith(null, { fileid: 1, view: 'files' }, { fileid: 1, dir: '/Foo/Bar' }) }) }) diff --git a/apps/files/src/actions/openInFilesAction.ts b/apps/files/src/actions/openInFilesAction.ts index 283bfc63d50..9d3ceaf3b7b 100644 --- a/apps/files/src/actions/openInFilesAction.ts +++ b/apps/files/src/actions/openInFilesAction.ts @@ -44,7 +44,7 @@ export const action = new FileAction({ window.OCP.Files.Router.goToRoute( null, // use default route { view: 'files', fileid: node.fileid }, - { dir, fileid: node.fileid, openfile: true }, + { dir, fileid: node.fileid }, ) return null }, diff --git a/apps/files/src/actions/sidebarAction.ts b/apps/files/src/actions/sidebarAction.ts index 0073d1c8490..849cf78368d 100644 --- a/apps/files/src/actions/sidebarAction.ts +++ b/apps/files/src/actions/sidebarAction.ts @@ -42,6 +42,10 @@ export const action = new FileAction({ return false } + if (!nodes[0]) { + return false + } + // Only work if the sidebar is available if (!window?.OCA?.Files?.Sidebar) { return false @@ -53,7 +57,7 @@ export const action = new FileAction({ async exec(node: Node, view: Navigation) { try { // TODO: migrate Sidebar to use a Node instead - window?.OCA?.Files?.Sidebar?.open?.(node.path) + await window.OCA.Files.Sidebar.open(node.path) // Silently update current fileid window.OCP.Files.Router.goToRoute( diff --git a/apps/files/src/components/FilesListVirtual.vue b/apps/files/src/components/FilesListVirtual.vue index c943b899897..8c7242561e9 100644 --- a/apps/files/src/components/FilesListVirtual.vue +++ b/apps/files/src/components/FilesListVirtual.vue @@ -69,15 +69,17 @@