fix(links): Insert /f/<fileId> format for links to files

Get rid of the old `/path/?fileId=<id>` format. It never really worked
anyway because the paths were relative to the users' home directory.

Instead, insert links to files/folders as full URLs with the
`/f/<fileId>` format. This makes link previews work both in Text and
Collectives.

Rewrite links in the old format to the new format. But don't touch
absolute link (without origin) to the collectives app to not break
collectives-specific link handling to other pages.

Signed-off-by: Jonas <[email protected]>

cypress/e2e/nodes/Links.spec.js

@@ -180,9 +180,11 @@ describe('test link marks', function() {
 					cy.contains('button', isFolder ? 'Choose' : `Choose ${filename}`).click()
 				})
 
-				return cy.getContent()
-					.find(`a[href*="${encodeURIComponent(filename)}"]`)
-					.should('have.text', text === undefined ? filename : text)
+				cy.getFileId(filename).then((fileId) => {
+					return cy.getContent()
+						.find(`a[href*="/f/${fileId}"]`)
+						.should('have.text', text === undefined ? filename : text)
+				})
 			}
 
 			beforeEach(() => cy.clearContent())

src/components/Menu/ActionInsertLink.vue

@@ -80,6 +80,7 @@
 import { NcActions, NcActionButton, NcActionInput } from '@nextcloud/vue'
 import { getLinkWithPicker } from '@nextcloud/vue/dist/Components/NcRichText.js'
 import { FilePickerType, getFilePickerBuilder } from '@nextcloud/dialogs'
+import { generateUrl } from '@nextcloud/router'
 
 import { getMarkAttributes, isActive } from '@tiptap/core'
 
@@ -144,10 +145,8 @@ export default {
 				.then((file) => {
 					const client = OC.Files.getClient()
 					client.getFileInfo(file).then((_status, fileInfo) => {
-						const path = optimalPath(this.relativePath, `${fileInfo.path}/${fileInfo.name}`)
-						const encodedPath = path.split('/').map(encodeURIComponent).join('/') + (fileInfo.type === 'dir' ? '/' : '')
-						const href = `${encodedPath}?fileId=${fileInfo.id}`
-						this.setLink(href, fileInfo.name)
+						const url = new URL(generateUrl(`/f/${fileInfo.id}`), window.location)
+						this.setLink(url.href, fileInfo.name)
 						this.startPath = fileInfo.path + (fileInfo.type === 'dir' ? `/${fileInfo.name}/` : '')
 					})
 					this.menuOpen = false

src/helpers/links.js

@@ -20,7 +20,6 @@
  *
  */
 
-import { loadState } from '@nextcloud/initial-state'
 import { generateUrl } from '@nextcloud/router'
 
 const absolutePath = function(base, rel) {
@@ -62,22 +61,17 @@ const domHref = function(node, relativePath) {
 	if (ref.startsWith('#')) {
 		return ref
 	}
-	// Don't rewrite URL in Collectives context
-	if (loadState('core', 'active-app') === 'collectives') {
+	// Don't rewrite links to the collectives app
+	if (ref.includes('/apps/collectives/')) {
 		return ref
 	}
 
+	// Rewrite links with old format from file picker to `/f/<fileId>`
 	const match = ref.match(/^([^?]*)\?fileId=(\d+)/)
 	if (match) {
-		const [, relPath, id] = match
-		const currentDir = basedir(relativePath || OCA.Viewer?.file || '/')
-		const dir = absolutePath(currentDir, basedir(relPath))
-		if (relPath.length > 1 && relPath.endsWith('/')) {
-			// is directory
-			return generateUrl(`/apps/files/?dir=${dir}&fileId=${id}`)
-		} else {
-			return generateUrl(`/apps/files/?dir=${dir}&openfile=${id}#relPath=${relPath}`)
-		}
+		const [, , id] = match
+		const url = new URL(generateUrl(`/f/${id}`), window.location)
+		return url.href
 	}
 	return ref
 }
@@ -90,7 +84,8 @@ const parseHref = function(dom) {
 	const match = ref.match(/\?dir=([^&]*)&openfile=([^&]*)#relPath=([^&]*)/)
 	if (match) {
 		const [, , id, path] = match
-		return `${path}?fileId=${id}`
+		const url = new URL(generateUrl(`/f/${id}`), window.location)
+		return url.href
 	}
 	return ref
 }

src/tests/helpers/links.spec.js

@@ -1,5 +1,4 @@
 import { domHref, parseHref } from '../../helpers/links'
-import { loadState } from '@nextcloud/initial-state'
 
 global.OCA = {
 	Viewer: {
@@ -13,9 +12,6 @@ global.OC = {
 
 global._oc_webroot = ''
 
-jest.mock('@nextcloud/initial-state')
-loadState.mockImplementation((app, key) => 'files')
-
 describe('Preparing href attributes for the DOM', () => {
 
 	test('leave empty hrefs alone', () => {
@@ -36,24 +32,24 @@ describe('Preparing href attributes for the DOM', () => {
 			.toBe('mailTo:[email protected]')
 	})
 
-	test('relative link with fileid', () => {
+	test('relative link with fileid (old format from file picker)', () => {
 		expect(domHref({attrs: {href: 'otherfile?fileId=123'}}))
-			.toBe('/apps/files/?dir=/Wiki&openfile=123#relPath=otherfile')
+			.toBe('http://localhost/f/123')
 	})
 
-	test('relative path with ../', () => {
+	test('relative path with ../ (old format from file picker)', () => {
 		expect(domHref({attrs: {href: '../other/otherfile?fileId=123'}}))
-			.toBe('/apps/files/?dir=/other&openfile=123#relPath=../other/otherfile')
+			.toBe('http://localhost/f/123')
 	})
 
-	test('absolute path', () => {
+	test('absolute path (old format from file picker)', () => {
 		expect(domHref({attrs: {href: '/other/otherfile?fileId=123'}}))
-			.toBe('/apps/files/?dir=/other&openfile=123#relPath=/other/otherfile')
+			.toBe('http://localhost/f/123')
 	})
 
-	test('absolute path', () => {
+	test('absolute path (old format from file picker)', () => {
 		expect(domHref({attrs: {href: '/otherfile?fileId=123'}}))
-			.toBe('/apps/files/?dir=/&openfile=123#relPath=/otherfile')
+			.toBe('http://localhost/f/123')
 	})
 
 })
@@ -74,9 +70,9 @@ describe('Extracting short urls from the DOM', () => {
 		expect(parseHref(domStub())).toBe(undefined)
 	})
 
-	test('relative link with fileid', () => {
+	test('relative link with fileid (old format from file picker)', () => {
 		expect(parseHref(domStub('?dir=/other&openfile=123#relPath=../other/otherfile')))
-			.toBe('../other/otherfile?fileId=123')
+			.toBe('http://localhost/f/123')
 	})
 
 })
@@ -101,20 +97,29 @@ describe('Inserting hrefs into the dom and extracting them again', () => {
 		expect(insertAndExtract({})).toBe(undefined)
 	})
 
-	test('default relative link format is unchanged', () => {
+	test('old relative link format (from file picker) is rewritten', () => {
 		expect(insertAndExtract({href: 'otherfile?fileId=123'}))
-			.toBe('otherfile?fileId=123')
+			.toBe('http://localhost/f/123')
 	})
 
-})
+	test('old relative link format with ../ (from file picker) is rewritten', () => {
+		expect(insertAndExtract({href: '../otherfile?fileId=123'}))
+			.toBe('http://localhost/f/123')
+	})
 
-describe('Preparing href attributes for the DOM in Collectives app', () => {
-	beforeAll(() => {
-		loadState.mockImplementation((app, key) => 'collectives')
+	test('old absolute link format (from file picker) is rewritten', () => {
+		expect(insertAndExtract({href: '/otherfile?fileId=123'}))
+			.toBe('http://localhost/f/123')
 	})
 
-	test('relative link with fileid in Collectives', () => {
-		expect(domHref({attrs: {href: 'otherfile?fileId=123'}}))
-			.toBe('otherfile?fileId=123')
+	test('default absolute link format is unchanged', () => {
+		expect(insertAndExtract({href: 'http://localhost/f/123'}))
+			.toBe('http://localhost/f/123')
 	})
+
+	test('absolute link to collectives page is unchanged', () => {
+		expect(insertAndExtract({href: '/apps/collectives/page?fileId=123'}))
+			.toBe('/apps/collectives/page?fileId=123')
+	})
+
 })