Merge pull request #25001 from owncloud/issue-24939-better-ux-on-invalid-perma-link

Improve the UX for "not found" perma links

4 files changed, 44 insertions, 34 deletions

diff --git a/apps/files/js/app.js b/apps/files/js/app.js
index 7a3d78f9663..fbfa510e07e 100644
--- a/apps/files/js/app.js
+++ b/apps/files/js/app.js
@@ -53,6 +53,9 @@
 			this.$showHiddenFiles = $('input#showhiddenfilesToggle');
 			var showHidden = $('#showHiddenFiles').val() === "1";
 			this.$showHiddenFiles.prop('checked', showHidden);
+			if ($('#fileNotFound').val() === "1") {
+				OC.Notification.showTemporary(t('files', 'File could not be found'));
+			}
 
 			this._filesConfig = new OC.Backbone.Model({
 				showhidden: showHidden
diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php
index 1b0903b41d3..18b6cf719c5 100644
--- a/apps/files/lib/Controller/ViewController.php
+++ b/apps/files/lib/Controller/ViewController.php
@@ -27,9 +27,9 @@ namespace OCA\Files\Controller;
 use OC\AppFramework\Http\Request;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
-use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\Files\NotFoundException;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\INavigationManager;
@@ -37,7 +37,6 @@ use OCP\IRequest;
 use OCP\IURLGenerator;
 use OCP\IUserSession;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
-use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\Files\Folder;
 use OCP\App\IAppManager;
 
@@ -142,11 +141,15 @@ class ViewController extends Controller {
 	 * @param string $view
 	 * @param string $fileid
 	 * @return TemplateResponse
-	 * @throws \OCP\Files\NotFoundException
 	 */
 	public function index($dir = '', $view = '', $fileid = null) {
+		$fileNotFound = false;
 		if ($fileid !== null) {
-			return $this->showFile($fileid);
+			try {
+				return $this->showFile($fileid);
+			} catch (NotFoundException $e) {
+				$fileNotFound = true;
+			}
 		}
 
 		$nav = new \OCP\Template('files', 'appnavigation', '');
@@ -245,6 +248,7 @@ class ViewController extends Controller {
 		$params['defaultFileSortingDirection'] = $this->config->getUserValue($user, 'files', 'file_sorting_direction', 'asc');
 		$showHidden = (bool) $this->config->getUserValue($this->userSession->getUser()->getUID(), 'files', 'show_hidden', false);
 		$params['showHiddenFiles'] = $showHidden ? 1 : 0;
+		$params['fileNotFound'] = $fileNotFound ? 1 : 0;
 		$params['appNavigation'] = $nav;
 		$params['appContents'] = $contentItems;
 		$this->navigationManager->setActiveEntry('files_index');
@@ -265,40 +269,37 @@ class ViewController extends Controller {
 	 * Redirects to the file list and highlight the given file id
 	 *
 	 * @param string $fileId file id to show
-	 * @return Response redirect response or not found response
+	 * @return RedirectResponse redirect response or not found response
+	 * @throws \OCP\Files\NotFoundException
 	 *
 	 * @NoCSRFRequired
 	 * @NoAdminRequired
 	 */
 	public function showFile($fileId) {
-		try {
-			$uid = $this->userSession->getUser()->getUID();
-			$baseFolder = $this->rootFolder->get($uid . '/files/');
-			$files = $baseFolder->getById($fileId);
-			$params = [];
+		$uid = $this->userSession->getUser()->getUID();
+		$baseFolder = $this->rootFolder->get($uid . '/files/');
+		$files = $baseFolder->getById($fileId);
+		$params = [];
 
-			if (empty($files) && $this->appManager->isEnabledForUser('files_trashbin')) {
-				$baseFolder = $this->rootFolder->get($uid . '/files_trashbin/files/');
-				$files = $baseFolder->getById($fileId);
-				$params['view'] = 'trashbin';
-			}
+		if (empty($files) && $this->appManager->isEnabledForUser('files_trashbin')) {
+			$baseFolder = $this->rootFolder->get($uid . '/files_trashbin/files/');
+			$files = $baseFolder->getById($fileId);
+			$params['view'] = 'trashbin';
+		}
 
-			if (!empty($files)) {
-				$file = current($files);
-				if ($file instanceof Folder) {
-					// set the full path to enter the folder
-					$params['dir'] = $baseFolder->getRelativePath($file->getPath());
-				} else {
-					// set parent path as dir
-					$params['dir'] = $baseFolder->getRelativePath($file->getParent()->getPath());
-					// and scroll to the entry
-					$params['scrollto'] = $file->getName();
-				}
-				return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', $params));
+		if (!empty($files)) {
+			$file = current($files);
+			if ($file instanceof Folder) {
+				// set the full path to enter the folder
+				$params['dir'] = $baseFolder->getRelativePath($file->getPath());
+			} else {
+				// set parent path as dir
+				$params['dir'] = $baseFolder->getRelativePath($file->getParent()->getPath());
+				// and scroll to the entry
+				$params['scrollto'] = $file->getName();
 			}
-		} catch (\OCP\Files\NotFoundException $e) {
-			return new NotFoundResponse();
+			return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', $params));
 		}
-		return new NotFoundResponse();
+		throw new \OCP\Files\NotFoundException();
 	}
 }
diff --git a/apps/files/templates/index.php b/apps/files/templates/index.php
index 7281edd3aec..42ce941a4a5 100644
--- a/apps/files/templates/index.php
+++ b/apps/files/templates/index.php
@@ -14,6 +14,7 @@
 <input type="hidden" name="usedSpacePercent" id="usedSpacePercent" value="<?php p($_['usedSpacePercent']); ?>" />
 <input type="hidden" name="owner" id="owner" value="<?php p($_['owner']); ?>" />
 <input type="hidden" name="ownerDisplayName" id="ownerDisplayName" value="<?php p($_['ownerDisplayName']); ?>" />
+<input type="hidden" name="fileNotFound" id="fileNotFound" value="<?php p($_['fileNotFound']); ?>"" />
 <?php if (!$_['isPublic']) :?>
 <input type="hidden" name="mailNotificationEnabled" id="mailNotificationEnabled" value="<?php p($_['mailNotificationEnabled']) ?>" />
 <input type="hidden" name="mailPublicNotificationEnabled" id="mailPublicNotificationEnabled" value="<?php p($_['mailPublicNotificationEnabled']) ?>" />
diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php
index 049f44fc0af..34c40ecea5c 100644
--- a/apps/files/tests/Controller/ViewControllerTest.php
+++ b/apps/files/tests/Controller/ViewControllerTest.php
@@ -26,6 +26,7 @@ namespace OCA\Files\Tests\Controller;
 
 use OCA\Files\Controller\ViewController;
 use OCP\AppFramework\Http;
+use OCP\Files\NotFoundException;
 use OCP\IUser;
 use OCP\Template;
 use Test\TestCase;
@@ -259,7 +260,8 @@ class ViewControllerTest extends TestCase {
 				'isPublic' => false,
 				'defaultFileSorting' => 'name',
 				'defaultFileSortingDirection' => 'asc',
-				'showHiddenFiles' => false,
+				'showHiddenFiles' => 0,
+				'fileNotFound' => 0,
 				'mailNotificationEnabled' => 'no',
 				'mailPublicNotificationEnabled' => 'no',
 				'allowShareWithLink' => 'yes',
@@ -410,11 +412,14 @@ class ViewControllerTest extends TestCase {
 			->with(123)
 			->will($this->returnValue([]));
 
-		$expected = new Http\NotFoundResponse();
 		if ($useShowFile) {
-			$this->assertEquals($expected, $this->viewController->showFile(123));
+			$this->setExpectedException('OCP\Files\NotFoundException');
+			$this->viewController->showFile(123);
 		} else {
-			$this->assertEquals($expected, $this->viewController->index('/whatever', '', '123'));
+			$response = $this->viewController->index('MyDir', 'MyView', '123');
+			$this->assertInstanceOf('OCP\AppFramework\Http\TemplateResponse', $response);
+			$params = $response->getParams();
+			$this->assertEquals(1, $params['fileNotFound']);
 		}
 	}