Permalinks #24434

PVince81 · 2016-05-04T08:31:57Z

The following routes will redirect to the files app and display the
matching folder. If the fileid is a file, it will scroll to it.

Missing parts and challenges:

have the JS code append "fileid" when switching dirs and using history API
what if the user hits the back button and the URL contains an older fileid ? => no problem
have "fileid" always appear in the URL when switching dirs (and avoid infinite redirect loop)
fileid with other views than "All files" ?!! => not supported

mention-bot · 2016-05-04T08:31:58Z

By analyzing the blame information on this pull request, we identified @ChristophWurst, @LukasReschke and @tanghus to be potential reviewers

PVince81 · 2016-05-04T08:34:52Z

Note: there is no UI yet to create such links, for testing please create these manually.
You can find a fileid by inspecting the "tr" element of the file/folder row and checking "data-id"

PVince81 · 2016-05-04T09:15:29Z

~~if fileid in trash, redirect to trash instead~~ to be done separately

PVince81 · 2016-05-04T09:20:14Z

Okay, delay aside, I found a way to replace the URL after the current folder was loaded. So if you observe the URL (when no fileid) present, it will appear without fileid until the folder is loaded, then the fileid will pop into the URL.

fix discrepancy between encoded slashes and non-encoded slashes (less jumpy)

The fileid will now appear in all URLs under "All files" !

PVince81 · 2016-05-04T09:20:56Z

Basically if we detect a URL change, and the only change is appending the fileid, then we use replaceState instead of pushState.

PVince81 · 2016-05-04T09:23:06Z

~~TODO: fallback if fileid not accessible, fallback to using dir~~

PVince81 · 2016-05-04T09:33:12Z

prevent opening fileid outside of "/$user/files" (except "files_trashbin")

PVince81 · 2016-05-04T12:25:23Z

Okay, looks like $userFolder->getById() already enforces the limit to "files".
I'll leave the trashbin case for a separate PR as it will require some working around this.

PVince81 · 2016-05-06T10:13:12Z

TODO: instead of triggering the "changeDirectory" even twice, do it only once after the fileid is known?

PVince81 · 2016-05-06T10:52:34Z

TEST: make sure activity links still work (due to URL rewriting)

PVince81 · 2016-05-06T10:52:49Z

well, actually... activity app could also use the new URL format

PVince81 · 2016-05-06T11:29:08Z

I added a section in the sidebar:

When clicking the anchor (with title "Local link"), a new field will pop up with the link.
I wasn't sure about this, if this is too much I can also get rid of the field and expect people to right click + copy on the anchor.

This one delivers the short format link, also for files!

PVince81 · 2016-05-06T13:27:22Z

Just added: email about internal share now uses the shortened permalink.

Would be good to get some feedback whether it works usability-wise @owncloud/designers

PVince81 · 2016-05-06T14:15:35Z

More fixes:

not encoding slashes any more (no more ugly %2f in the URL)
added event "afterChangeDirectory" to separately append the "fileid" param in the URL when we got it

PVince81 · 2016-05-06T14:21:53Z

LukasReschke · 2016-05-06T14:23:07Z

lib/private/Share/MailNotifications.php

-				'files.view.index',
-				$args
+				'files.viewcontroller.showFile',
+				['fileId' => $items[0]['item_source']]


Undefined index in the unit test :)

10:20:05 1) MailNotificationsTest::testSendInternalShareMail 10:20:05 Undefined index: item_source 10:20:05 10:20:05 /ssd/jenkins/workspace/server-master-linux-php7-ci/database/sqlite/label/SLAVE/lib/private/Share/MailNotifications.php:124 10:20:05 /ssd/jenkins/workspace/server-master-linux-php7-ci/database/sqlite/label/SLAVE/tests/lib/share/MailNotificationsTest.php:250

LukasReschke · 2016-05-06T14:24:02Z

Most useful feature ever. Code looks good and works. Great stuff!!!! 🚀 💘 ❤️ 🎉

PVince81 · 2016-05-06T14:24:16Z

I think this is ready for review @MorrisJobke @nickvergessen @guruz @rullzer @icewind1991 @schiesbn @VicDeo @georgehrke @Xenopathic @blizzz @ChristophWurst

@LukasReschke mind checking the endpoint ? (disabled CSRF, etc)

LukasReschke · 2016-05-06T14:24:46Z

@LukasReschke mind checking the endpoint ? (disabled CSRF, etc)

That's fine :)

LukasReschke · 2016-05-06T14:27:19Z

apps/files/controller/viewcontroller.php

+	 * Redirects to the file list and highlight the given file id
+	 *
+	 * @param string $fileId file id to show
+	 *


@return missing

missing @return

PVince81 · 2016-05-06T14:30:13Z

BUG: back button goes to root folder after going into 2 subdirs

rullzer · 2016-05-06T14:34:43Z

I love the local link thingy... but can't we just copy it to the clipboard directly?

PVince81 · 2016-05-06T14:35:09Z

@rullzer copy to clipboard requires flash, see how Github does it with its copy link button. No flash, no button.

rullzer · 2016-05-06T14:35:29Z

Aaaah ok then never mind :)

PVince81 · 2016-05-06T14:35:31Z

@rullzer (else we'd probably have that button for all link fields)

rullzer · 2016-05-06T14:37:30Z

apps/files/controller/viewcontroller.php

 	 * @return TemplateResponse
 	 * @throws \OCP\Files\NotFoundException
 	 */
-	public function index($dir = '', $view = '') {
+	public function index($dir = '', $view = '', $fileid = null) {
+		if ($fileid !== null) {


So fileid always has preference if it is there. Sounds good

georgehrke · 2016-05-06T14:39:31Z

You can also use Document.execCommand to copy stuff to the clipboard, but I'm not sure how good it is supported in various browsers.

There are some wrapper libs that have fallbacks like https://clipboardjs.com

rullzer · 2016-05-06T14:40:00Z

apps/files/controller/viewcontroller.php

+		$params = [];
+
+		if (!empty($files)) {
+			$file = current($files);


I was thinking... maybe it is good to loop over all files (if a file is shared via multiple ways)... and pick the one with the highest permission?

Of course harder then it sounds because which do you pick if 1 has edit and 1 has share permissions.

Also not that common scenario...

@rullzer yeah good point, possibly something for later.

Also I thought about adding a "trashbin" route in case the file is detected in trash instead, but something for later also.

PVince81 · 2016-05-06T14:44:38Z

@georgehrke I'll make a ticket to research that. Could definitely be useful for the many link-copy locations we have: #24472

The following routes will redirect to the files app and display the matching folder. If the fileid is a file, it will scroll to it. - http://localhost/owncloud/index.php/f/$fileid - http://localhost/owncloud/index.php/files/?dir=somedir&fileid=$fileid

PVince81 · 2016-05-06T14:53:21Z

TEST: file share + rename file as recipient + open permalink => correct filename used
TEST: browser back button after several navigations

PVince81 · 2016-05-06T15:01:17Z

Fixed back button and unit test.

rullzer · 2016-05-10T07:44:03Z

👍 very very cool stuff!

LiamHD · 2016-07-13T19:36:26Z

I cannot find the permalink in the curren owncloud version. What this feature removed?

georgehrke · 2016-07-13T20:04:34Z

@LiamHD This feature will be released with ownCloud 9.1

lock · 2019-08-05T04:02:42Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

PVince81 added 2 - Developing app:files labels May 4, 2016

PVince81 added this to the 9.1-current milestone May 4, 2016

PVince81 mentioned this pull request May 4, 2016

Permalinks for folders and files that work across internal users when sharing #11732

Closed

PVince81 changed the title ~~Add route to resolve fileid to files app URL~~ Permalinks May 6, 2016

PVince81 mentioned this pull request May 6, 2016

Use permalinks in activities owncloud/activity#521

Closed

4 tasks

PVince81 force-pushed the permalinks branch from ae0fd79 to 1822ad4 Compare May 6, 2016 13:11

PVince81 force-pushed the permalinks branch from 1822ad4 to a5859a0 Compare May 6, 2016 14:15

PVince81 added 3 - To Review and removed 2 - Developing labels May 6, 2016

LukasReschke reviewed May 6, 2016
View reviewed changes

rullzer reviewed May 6, 2016
View reviewed changes

Vincent Petry added 4 commits May 6, 2016 16:46

Auto-add fileid in URL for currently displayed folder

fdeafef

Add anchor icon in sidebar to copy permalink

112b703

Send permalink in internal share notification emails

caefe23

Do not encode slashes in "dir" URL param in files JS

254576e

PVince81 force-pushed the permalinks branch from a5859a0 to 254576e Compare May 6, 2016 15:00

LukasReschke merged commit e8e72aa into master May 10, 2016

LukasReschke deleted the permalinks branch May 10, 2016 07:44

dvergeylen mentioned this pull request May 12, 2016

Enhancement : Create a Unique Share URL linking to website OR local hard disk (if client is installed) #21688

Closed

PVince81 mentioned this pull request Oct 17, 2016

Fix url links to lucene result page with OC 9.1 owncloud-archive/search_lucene#124

Closed

This was referenced Mar 27, 2017

Share "local link" from the file explorer desktop menu owncloud/client#5653

Closed

Use "Local Links" on the client owncloud/client#5023

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019

Permalinks #24434

Permalinks #24434

Conversation

PVince81 commented May 4, 2016 • edited by ghost Loading

mention-bot commented May 4, 2016

PVince81 commented May 4, 2016

PVince81 commented May 4, 2016 • edited Loading

PVince81 commented May 4, 2016 • edited Loading

PVince81 commented May 4, 2016

PVince81 commented May 4, 2016 • edited Loading

PVince81 commented May 4, 2016 • edited Loading

PVince81 commented May 4, 2016

PVince81 commented May 6, 2016 • edited Loading

PVince81 commented May 6, 2016 • edited Loading

PVince81 commented May 6, 2016

PVince81 commented May 6, 2016

PVince81 commented May 6, 2016

PVince81 commented May 6, 2016

PVince81 commented May 6, 2016

Choose a reason for hiding this comment

LukasReschke commented May 6, 2016

PVince81 commented May 6, 2016

LukasReschke commented May 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PVince81 commented May 6, 2016 • edited Loading

rullzer commented May 6, 2016

PVince81 commented May 6, 2016

rullzer commented May 6, 2016

PVince81 commented May 6, 2016

Choose a reason for hiding this comment

georgehrke commented May 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PVince81 commented May 6, 2016

PVince81 commented May 6, 2016 • edited Loading

PVince81 commented May 6, 2016

rullzer commented May 10, 2016

LiamHD commented Jul 13, 2016

georgehrke commented Jul 13, 2016

lock bot commented Aug 5, 2019

PVince81 commented May 4, 2016 •

edited by ghost

Loading

PVince81 commented May 4, 2016 •

edited

Loading

PVince81 commented May 4, 2016 •

edited

Loading

PVince81 commented May 4, 2016 •

edited

Loading

PVince81 commented May 4, 2016 •

edited

Loading

PVince81 commented May 6, 2016 •

edited

Loading

PVince81 commented May 6, 2016 •

edited

Loading

PVince81 commented May 6, 2016 •

edited

Loading

PVince81 commented May 6, 2016 •

edited

Loading