diff --git a/cypress/e2e/nodes/Links.spec.js b/cypress/e2e/nodes/Links.spec.js index 248f868d791..4af18b7bb52 100644 --- a/cypress/e2e/nodes/Links.spec.js +++ b/cypress/e2e/nodes/Links.spec.js @@ -2,6 +2,7 @@ import { initUserAndFiles, randUser } from '../../utils/index.js' const user = randUser() const fileName = 'empty.md' +let fileId = null describe('test link marks', function() { before(function() { @@ -18,6 +19,9 @@ describe('test link marks', function() { }, }) cy.openFile(fileName, { force: true }) + cy.getFileId(fileName).then((id) => { + fileId = id + }) }) describe('link bubble', function() { @@ -181,7 +185,7 @@ describe('test link marks', function() { }) return cy.getContent() - .find(`a[href*="${encodeURIComponent(filename)}"]`) + .find(`a[href*="/f/${fileId}"]`) .should('have.text', text === undefined ? filename : text) } @@ -204,9 +208,12 @@ describe('test link marks', function() { }) it('link to directory', () => { cy.createFolder(`${window.__currentDirectory}/dummy folder`) - cy.getFile(fileName).then($el => { - cy.getContent().type(`${text}{selectAll}`) - checkLinkFile('dummy folder', text, true) + cy.getFileId('dummy folder').then((id) => { + fileId = id + cy.getFile(fileName).then($el => { + cy.getContent().type(`${text}{selectAll}`) + checkLinkFile('dummy folder', text, true) + }) }) }) }) diff --git a/cypress/e2e/workspace.spec.js b/cypress/e2e/workspace.spec.js index 453bd62de32..9dc4f48dcf4 100644 --- a/cypress/e2e/workspace.spec.js +++ b/cypress/e2e/workspace.spec.js @@ -158,11 +158,15 @@ describe('Workspace', function() { }) it('relative folder links', function() { + let fileId = null cy.createFolder(`${this.testFolder}/sub-folder`) cy.createFolder(`${this.testFolder}/sub-folder/alpha`) cy.uploadFile('test.md', 'text/markdown', `${this.testFolder}/sub-folder/alpha/test.md`) cy.visitTestFolder() + cy.getFileId('test.md').then((id) => { + fileId = id + }) cy.openWorkspace() .type('link me') @@ -180,8 +184,7 @@ describe('Workspace', function() { cy.getEditor() .find('a') .should('have.attr', 'href') - .and('contains', `dir=/${this.testFolder}/sub-folder/alpha`) - .and('contains', '#relPath=sub-folder/alpha/test.md') + .and('contains', `/f/${fileId})`) cy.getContent() .type('{leftArrow}') diff --git a/src/components/Menu/ActionInsertLink.vue b/src/components/Menu/ActionInsertLink.vue index 2cafe67f016..a26694ff5d4 100644 --- a/src/components/Menu/ActionInsertLink.vue +++ b/src/components/Menu/ActionInsertLink.vue @@ -80,12 +80,12 @@ 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' import { Document, Loading, LinkOff, Web, Shape } from '../icons.js' import { BaseActionEntry } from './BaseActionEntry.js' -import { optimalPath } from '../../helpers/files.js' import { useFileMixin } from '../Editor.provider.js' import { useMenuIDMixin } from './MenuBar.provider.js' @@ -144,9 +144,7 @@ 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}` + const href = generateUrl(`/f/${fileInfo.id}`) this.setLink(href, fileInfo.name) this.startPath = fileInfo.path + (fileInfo.type === 'dir' ? `/${fileInfo.name}/` : '') }) diff --git a/src/helpers/files.js b/src/helpers/files.js index a5ca85fddb3..1e6f4760434 100644 --- a/src/helpers/files.js +++ b/src/helpers/files.js @@ -37,25 +37,6 @@ import FilePlusSvg from '@mdi/svg/svg/file-plus.svg' const FILE_ACTION_IDENTIFIER = 'Edit with text app' -const optimalPath = function(from, to) { - const current = from.split('/') - const target = to.split('/') - current.pop() // ignore filename - while (current[0] === target[0]) { - current.shift() - target.shift() - // Handle case where target is the current directory - if (current.length === 0 && target.length === 0) { - return '.' - } - } - const relativePath = current.fill('..').concat(target) - const absolutePath = to.split('/') - return relativePath.length < absolutePath.length - ? relativePath.join('/') - : to -} - const registerFileCreate = () => { const newFileMenuPlugin = { attach(menu) { @@ -260,7 +241,6 @@ export const FilesWorkspaceHeader = new Header({ }) export { - optimalPath, registerFileActionFallback, registerFileCreate, FILE_ACTION_IDENTIFIER, diff --git a/src/helpers/links.js b/src/helpers/links.js index b45ab1ba7e2..cd9830aa96c 100644 --- a/src/helpers/links.js +++ b/src/helpers/links.js @@ -20,34 +20,8 @@ * */ -import { loadState } from '@nextcloud/initial-state' import { generateUrl } from '@nextcloud/router' -const absolutePath = function(base, rel) { - if (!rel) { - return base - } - if (rel[0] === '/') { - return rel - } - base = base.split('/') - rel = rel.split('/') - while (rel[0] === '..' || rel[0] === '.') { - if (rel[0] === '..') { - base.pop() - } - rel.shift() - } - return base.concat(rel).join('/') -} - -const basedir = function(file) { - const end = file.lastIndexOf('/') - return (end > 0) - ? file.slice(0, end) - : file.slice(0, end + 1) // basedir('/toplevel') should return '/' -} - const domHref = function(node, relativePath) { const ref = node.attrs.href if (!ref) { @@ -62,22 +36,16 @@ 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/` 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 + return generateUrl(`/f/${id}`) } return ref } @@ -89,8 +57,8 @@ const parseHref = function(dom) { } const match = ref.match(/\?dir=([^&]*)&openfile=([^&]*)#relPath=([^&]*)/) if (match) { - const [, , id, path] = match - return `${path}?fileId=${id}` + const [, , id] = match + return generateUrl(`/f/${id}`) } return ref } diff --git a/src/tests/helpers/links.spec.js b/src/tests/helpers/links.spec.js index 12d4e614602..d1f980eee68 100644 --- a/src/tests/helpers/links.spec.js +++ b/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:test@mail.example') }) - 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('/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('/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('/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('/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('/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('/f/123') }) -}) + test('old relative link format with ../ (from file picker) is rewritten', () => { + expect(insertAndExtract({href: '../otherfile?fileId=123'})) + .toBe('/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('/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: '/f/123'})) + .toBe('/f/123') }) + + test('absolute link to collectives page is unchanged', () => { + expect(insertAndExtract({href: '/apps/collectives/page?fileId=123'})) + .toBe('/apps/collectives/page?fileId=123') + }) + })