diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php index 5f93c636e5fc5..2b0f5082f21ec 100644 --- a/apps/files/lib/Controller/ViewController.php +++ b/apps/files/lib/Controller/ViewController.php @@ -80,7 +80,7 @@ protected function getStorageInfo(string $dir = '/') { */ #[NoAdminRequired] #[NoCSRFRequired] - public function showFile(?string $fileid = null): Response { + public function showFile(?string $fileid = null, ?string $opendetails = null, ?string $openfile = null): Response { if (!$fileid) { return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index')); } @@ -89,7 +89,9 @@ public function showFile(?string $fileid = null): Response { try { return $this->redirectToFile((int) $fileid); } catch (NotFoundException $e) { - return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', ['fileNotFound' => true])); + // Keep the fileid even if not found, it will be used + // to detect the file could not be found and warn the user + return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.indexViewFileid', ['fileid' => $fileid, 'view' => 'files'])); } } @@ -98,38 +100,35 @@ public function showFile(?string $fileid = null): Response { * @param string $dir * @param string $view * @param string $fileid - * @param bool $fileNotFound * @return TemplateResponse|RedirectResponse */ #[NoAdminRequired] #[NoCSRFRequired] - public function indexView($dir = '', $view = '', $fileid = null, $fileNotFound = false) { - return $this->index($dir, $view, $fileid, $fileNotFound); + public function indexView($dir = '', $view = '', $fileid = null) { + return $this->index($dir, $view, $fileid); } /** * @param string $dir * @param string $view * @param string $fileid - * @param bool $fileNotFound * @return TemplateResponse|RedirectResponse */ #[NoAdminRequired] #[NoCSRFRequired] - public function indexViewFileid($dir = '', $view = '', $fileid = null, $fileNotFound = false) { - return $this->index($dir, $view, $fileid, $fileNotFound); + public function indexViewFileid($dir = '', $view = '', $fileid = null) { + return $this->index($dir, $view, $fileid); } /** * @param string $dir * @param string $view * @param string $fileid - * @param bool $fileNotFound * @return TemplateResponse|RedirectResponse */ #[NoAdminRequired] #[NoCSRFRequired] - public function index($dir = '', $view = '', $fileid = null, $fileNotFound = false) { + public function index($dir = '', $view = '', $fileid = null) { if ($fileid !== null && $view !== 'trashbin') { try { return $this->redirectToFileIfInTrashbin((int) $fileid); @@ -158,8 +157,6 @@ public function index($dir = '', $view = '', $fileid = null, $fileNotFound = fal if (count($nodes) === 1 && $relativePath !== $dir && $nodePath !== $dir) { return $this->redirectToFile((int) $fileid); } - } else { // fileid does not exist anywhere - $fileNotFound = true; } } @@ -247,10 +244,12 @@ private function redirectToFileIfInTrashbin($fileId): RedirectResponse { * Redirects to the file list and highlight the given file id * * @param int $fileId file id to show + * @param string|null $openDetails open details parameter + * @param string|null $openFile open file parameter * @return RedirectResponse redirect response or not found response * @throws NotFoundException */ - private function redirectToFile(int $fileId) { + private function redirectToFile(int $fileId, ?string $openDetails = null, ?string $openFile = null): RedirectResponse { $uid = $this->userSession->getUser()->getUID(); $baseFolder = $this->rootFolder->getUserFolder($uid); $node = $baseFolder->getFirstNodeById($fileId); @@ -272,6 +271,19 @@ private function redirectToFile(int $fileId) { // open the file by default (opening the viewer) $params['openfile'] = 'true'; } + + // Forward open parameters if any. + // - openfile is true by default + // - opendetails is undefined by default + // - both will be evaluated as truthy + if ($openDetails !== null) { + $params['opendetails'] = $openDetails !== 'false' ? 'true' : 'false'; + } + + if ($openFile !== null) { + $params['openfile'] = $openFile !== 'false' ? 'true' : 'false'; + } + return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.indexViewFileid', $params)); } diff --git a/apps/files/src/components/FilesListVirtual.vue b/apps/files/src/components/FilesListVirtual.vue index 64f606891e4c3..8cf3d57d02297 100644 --- a/apps/files/src/components/FilesListVirtual.vue +++ b/apps/files/src/components/FilesListVirtual.vue @@ -209,6 +209,8 @@ export default defineComponent({ this.unselectFile() } } + + this.openSidebarForFile(this.fileId) }) }, immediate: true, @@ -249,6 +251,7 @@ export default defineComponent({ sidebarAction.exec(node, this.currentView, this.currentFolder.path) } } + logger.error(`Failed to open sidebar on file ${fileId}, file isn't cached yet !`, { fileId, node }) }, scrollToFile(fileId: number|null, warn = true) { diff --git a/apps/files/src/views/FilesList.vue b/apps/files/src/views/FilesList.vue index 6d3332029148d..c36c18196ac5e 100644 --- a/apps/files/src/views/FilesList.vue +++ b/apps/files/src/views/FilesList.vue @@ -493,14 +493,24 @@ export default defineComponent({ }, }, - mounted() { - this.fetchContent() - + async mounted() { subscribe('files:node:deleted', this.onNodeDeleted) subscribe('files:node:updated', this.onUpdatedNode) // reload on settings change subscribe('files:config:updated', this.fetchContent) + + // Finally, fetch the current directory contents + await this.fetchContent() + if (this.fileId) { + // If we have a fileId, let's check if the file exists + const node = this.dirContents.find(node => node.fileid.toString() === this.fileId.toString()) + // If the file isn't in the current directory nor if + // the current directory is the file, we show an error + if (!node && this.currentFolder.fileid.toString() !== this.fileId.toString()) { + showError(t('files', 'The file could not be found')) + } + } }, unmounted() { diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php index 75d8cf2a8cf0f..4676110d1b0b9 100644 --- a/apps/files/tests/Controller/ViewControllerTest.php +++ b/apps/files/tests/Controller/ViewControllerTest.php @@ -8,17 +8,21 @@ namespace OCA\Files\Tests\Controller; use OC\Files\FilenameValidator; +use OC\Route\Router; +use OC\URLGenerator; use OCA\Files\Controller\ViewController; use OCA\Files\Service\UserConfig; use OCA\Files\Service\ViewConfig; use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Services\IInitialState; +use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\Template\ITemplateManager; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -26,39 +30,53 @@ use OCP\IUser; use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Container\ContainerInterface; +use Psr\Log\LoggerInterface; use Test\TestCase; /** * Class ViewControllerTest * + * @group RoutingWeirdness + * * @package OCA\Files\Tests\Controller */ class ViewControllerTest extends TestCase { - private IRequest&MockObject $request; - private IURLGenerator&MockObject $urlGenerator; - private IL10N&MockObject $l10n; + private ContainerInterface&MockObject $container; + private IAppManager&MockObject $appManager; + private ICacheFactory&MockObject $cacheFactory; private IConfig&MockObject $config; private IEventDispatcher $eventDispatcher; - private IUser&MockObject $user; - private IUserSession&MockObject $userSession; - private IAppManager&MockObject $appManager; - private IRootFolder&MockObject $rootFolder; + private IEventLogger&MockObject $eventLogger; private IInitialState&MockObject $initialState; + private IL10N&MockObject $l10n; + private IRequest&MockObject $request; + private IRootFolder&MockObject $rootFolder; private ITemplateManager&MockObject $templateManager; + private IURLGenerator $urlGenerator; + private IUser&MockObject $user; + private IUserSession&MockObject $userSession; + private LoggerInterface&MockObject $logger; private UserConfig&MockObject $userConfig; private ViewConfig&MockObject $viewConfig; + private Router $router; private ViewController&MockObject $viewController; protected function setUp(): void { parent::setUp(); - $this->request = $this->getMockBuilder(IRequest::class)->getMock(); - $this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)->getMock(); - $this->l10n = $this->getMockBuilder(IL10N::class)->getMock(); - $this->config = $this->getMockBuilder(IConfig::class)->getMock(); + $this->appManager = $this->createMock(IAppManager::class); + $this->config = $this->createMock(IConfig::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); - $this->userSession = $this->getMockBuilder(IUserSession::class)->getMock(); - $this->appManager = $this->getMockBuilder('\OCP\App\IAppManager')->getMock(); + $this->initialState = $this->createMock(IInitialState::class); + $this->l10n = $this->createMock(IL10N::class); + $this->request = $this->createMock(IRequest::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->templateManager = $this->createMock(ITemplateManager::class); + $this->userConfig = $this->createMock(UserConfig::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->viewConfig = $this->createMock(ViewConfig::class); + $this->user = $this->getMockBuilder(IUser::class)->getMock(); $this->user->expects($this->any()) ->method('getUID') @@ -66,14 +84,38 @@ protected function setUp(): void { $this->userSession->expects($this->any()) ->method('getUser') ->willReturn($this->user); - $this->rootFolder = $this->getMockBuilder('\OCP\Files\IRootFolder')->getMock(); - $this->initialState = $this->createMock(IInitialState::class); - $this->templateManager = $this->createMock(ITemplateManager::class); - $this->userConfig = $this->createMock(UserConfig::class); - $this->viewConfig = $this->createMock(ViewConfig::class); - $filenameValidator = $this->createMock(FilenameValidator::class); + // Make sure we know the app is enabled + $this->appManager->expects($this->any()) + ->method('cleanAppId') + ->willReturnArgument(0); + $this->appManager->expects($this->any()) + ->method('getAppPath') + ->willReturnCallback(fn (string $appid): string => \OC::$SERVERROOT . '/apps/' . $appid); + + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->eventLogger = $this->createMock(IEventLogger::class); + $this->container = $this->createMock(ContainerInterface::class); + $this->router = new Router( + $this->logger, + $this->request, + $this->config, + $this->eventLogger, + $this->container, + $this->appManager, + ); + // Create a real URLGenerator instance to generate URLs + $this->urlGenerator = new URLGenerator( + $this->config, + $this->userSession, + $this->cacheFactory, + $this->request, + $this->router + ); + + $filenameValidator = $this->createMock(FilenameValidator::class); $this->viewController = $this->getMockBuilder(ViewController::class) ->setConstructorArgs([ 'files', @@ -148,7 +190,6 @@ public function testIndexWithRegularBrowser() { public function testShowFileRouteWithTrashedFile() { $this->appManager->expects($this->once()) ->method('isEnabledForUser') - ->with('files_trashbin') ->willReturn(true); $parentNode = $this->getMockBuilder(Folder::class)->getMock(); diff --git a/cypress/e2e/files/files.cy.ts b/cypress/e2e/files/files.cy.ts index 900be849d9bd4..ac82086360ffc 100644 --- a/cypress/e2e/files/files.cy.ts +++ b/cypress/e2e/files/files.cy.ts @@ -1,16 +1,57 @@ +import type { User } from "@nextcloud/cypress" + /** * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ describe('Files', { testIsolation: true }, () => { + let currentUser: User + beforeEach(() => { cy.createRandomUser().then((user) => { - cy.login(user) + currentUser = user }) }) it('Login with a user and open the files app', () => { + cy.login(currentUser) cy.visit('/apps/files') cy.get('[data-cy-files-list] [data-cy-files-list-row-name="welcome.txt"]').should('be.visible') }) + + it('Opens a valid file shows it as active', () => { + cy.uploadContent(currentUser, new Blob(), 'text/plain', '/original.txt').then((response) => { + const fileId = Number.parseInt(response.headers['oc-fileid'] ?? '0') + + cy.login(currentUser) + cy.visit('/apps/files/files/' + fileId) + + cy.get(`[data-cy-files-list-row-fileid=${fileId}]`) + .should('be.visible') + cy.get(`[data-cy-files-list-row-fileid=${fileId}]`) + .invoke('attr', 'data-cy-files-list-row-name').should('eq', 'original.txt') + cy.get(`[data-cy-files-list-row-fileid=${fileId}]`) + .invoke('attr', 'class').should('contain', 'active') + cy.contains('The file could not be found').should('not.exist') + }) + }) + + it('Opens a valid folder shows its content', () => { + cy.mkdir(currentUser, '/folder').then(() => { + cy.login(currentUser) + cy.visit('/apps/files/files?dir=/folder') + + cy.get('[data-cy-files-content-breadcrumbs]').contains('folder').should('be.visible') + cy.contains('The file could not be found').should('not.exist') + }) + }) + + it('Opens an unknown file show an error', () => { + cy.intercept('PROPFIND', /\/remote.php\/dav\//).as('propfind') + cy.login(currentUser) + cy.visit('/apps/files/files/123456') + + cy.wait('@propfind') + cy.contains('The file could not be found').should('be.visible') + }) }) diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index 646d1d4e6ed6e..d47569754d93b 100644 --- a/lib/private/Route/Router.php +++ b/lib/private/Route/Router.php @@ -156,11 +156,12 @@ public function loadRoutes($app = null) { $this->root->addCollection($collection); } } + if (!isset($this->loadedApps['core'])) { $this->loadedApps['core'] = true; $this->useCollection('root'); $this->setupRoutes($this->getAttributeRoutes('core'), 'core'); - require_once __DIR__ . '/../../../core/routes.php'; + require __DIR__ . '/../../../core/routes.php'; // Also add the OCS collection $collection = $this->getCollection('root.ocs'); @@ -475,7 +476,7 @@ private function getAttributeRoutes(string $app): array { * @param string $appName */ private function requireRouteFile($file, $appName) { - $this->setupRoutes(include_once $file, $appName); + $this->setupRoutes(include $file, $appName); }