diff --git a/__mocks__/sdk.js b/__mocks__/sdk.js index 7bbde9fc529..d863eb8c216 100644 --- a/__mocks__/sdk.js +++ b/__mocks__/sdk.js @@ -9,27 +9,21 @@ import fixturePublicFiles from '../__fixtures__/publicFiles' import fixtureRecipients from '../__fixtures__/recipients' import { DateTime } from 'luxon' +const mockPath = (path) => { + if (path.startsWith('/files/')) { + path = '/' + path.split('/').splice(3).join('/') + } + return path +} + export default { files: { list: (path = '/') => { - if (path.startsWith('/files/')) { - path = path.split('/').splice(3).join('/') - if (path === '') { - path = '/' - } - } - - return fixtureFiles[path] + return fixtureFiles[mockPath(path)] }, getFavoriteFiles: () => fixtureFavoriteFiles, fileInfo: (path) => { - if (path.startsWith('/files/')) { - path = path.split('/').splice(3).join('/') - if (path === '') { - path = '/' - } - } - return fixtureFiles[path][0] + return fixtureFiles[mockPath(path)][0] } }, users: { diff --git a/changelog/unreleased/bugfix-app-file-handling b/changelog/unreleased/bugfix-app-file-handling new file mode 100644 index 00000000000..a9973e6a359 --- /dev/null +++ b/changelog/unreleased/bugfix-app-file-handling @@ -0,0 +1,6 @@ +Bugfix: File handling in apps + +We fixed loading and saving files in apps in all contexts. It's now possible to open files in apps in personal files, favorites, share views and spaces. + +https://github.com/owncloud/web/pull/6456 + diff --git a/packages/web-app-files/src/helpers/resource/resource.ts b/packages/web-app-files/src/helpers/resource/resource.ts index 329edc34a05..a76f9a863b3 100644 --- a/packages/web-app-files/src/helpers/resource/resource.ts +++ b/packages/web-app-files/src/helpers/resource/resource.ts @@ -3,5 +3,6 @@ export interface Resource { id: number | string path: string + webDavPath: string downloadURL?: string } diff --git a/packages/web-app-files/src/helpers/resources.js b/packages/web-app-files/src/helpers/resources.js index 253d9313d67..d4845efb797 100644 --- a/packages/web-app-files/src/helpers/resources.js +++ b/packages/web-app-files/src/helpers/resources.js @@ -177,11 +177,11 @@ export function buildSpace(space) { } export function buildWebDavFilesPath(userId, path) { - return `/files/${userId}/${path}` + return '/' + `files/${userId}/${path}`.split('/').filter(Boolean).join('/') } export function buildWebDavSpacesPath(spaceId, path) { - return `/spaces/${spaceId}/${path}` + return '/' + `spaces/${spaceId}/${path}`.split('/').filter(Boolean).join('/') } export function attachIndicators(resource, sharesTree) { diff --git a/packages/web-app-files/tests/unit/helpers/resource/sameResource.spec.ts b/packages/web-app-files/tests/unit/helpers/resource/sameResource.spec.ts index d2bf7ba18de..3cf1e10352b 100644 --- a/packages/web-app-files/tests/unit/helpers/resource/sameResource.spec.ts +++ b/packages/web-app-files/tests/unit/helpers/resource/sameResource.spec.ts @@ -4,20 +4,30 @@ describe('isSameResource', () => { test('evaluates to false if one of the resources is nullish', () => { expect(isSameResource(null, null)).toBe(false) expect(isSameResource(undefined, undefined)).toBe(false) - expect(isSameResource(null, { id: 1, path: '' })).toBe(false) - expect(isSameResource(undefined, { id: 1, path: '' })).toBe(false) - expect(isSameResource({ id: 1, path: '' }, null)).toBe(false) - expect(isSameResource({ id: 1, path: '' }, undefined)).toBe(false) + expect(isSameResource(null, { id: 1, path: '', webDavPath: '' })).toBe(false) + expect(isSameResource(undefined, { id: 1, path: '', webDavPath: '' })).toBe(false) + expect(isSameResource({ id: 1, path: '', webDavPath: '' }, null)).toBe(false) + expect(isSameResource({ id: 1, path: '', webDavPath: '' }, undefined)).toBe(false) }) test('evaluates to false if ids are of different types', () => { - expect(isSameResource({ id: 1, path: '' }, { id: '1', path: '' })).toBe(false) + expect( + isSameResource({ id: 1, path: '', webDavPath: '' }, { id: '1', path: '', webDavPath: '' }) + ).toBe(false) }) test('evaluates to false if ids are different values', () => { - expect(isSameResource({ id: 1, path: '' }, { id: 2, path: '' })).toBe(false) - expect(isSameResource({ id: '1', path: '' }, { id: '2', path: '' })).toBe(false) + expect( + isSameResource({ id: 1, path: '', webDavPath: '' }, { id: 2, path: '', webDavPath: '' }) + ).toBe(false) + expect( + isSameResource({ id: '1', path: '', webDavPath: '' }, { id: '2', path: '', webDavPath: '' }) + ).toBe(false) }) test('evaluates to true if ids are the same', () => { - expect(isSameResource({ id: 1, path: '' }, { id: 1, path: '' })).toBe(true) - expect(isSameResource({ id: '1', path: '' }, { id: '1', path: '' })).toBe(true) + expect( + isSameResource({ id: 1, path: '', webDavPath: '' }, { id: 1, path: '', webDavPath: '' }) + ).toBe(true) + expect( + isSameResource({ id: '1', path: '', webDavPath: '' }, { id: '1', path: '', webDavPath: '' }) + ).toBe(true) }) }) diff --git a/packages/web-app-media-viewer/src/App.vue b/packages/web-app-media-viewer/src/App.vue index dbfd73adabc..d2ab82de869 100644 --- a/packages/web-app-media-viewer/src/App.vue +++ b/packages/web-app-media-viewer/src/App.vue @@ -185,7 +185,8 @@ export default { x: this.thumbDimensions, y: this.thumbDimensions, // strip double quotes from etag - c: this.activeMediaFile.etag.substr(1, this.activeMediaFile.etag.length - 2), + // we have no etag, e.g. on shared with others page + c: this.activeMediaFile.etag?.substr(1, this.activeMediaFile.etag.length - 2), scalingup: 0, preview: 1, a: 1 @@ -256,7 +257,7 @@ export default { // update route and url updateLocalHistory() { - this.$route.params.filePath = this.activeMediaFile.path + this.$route.params.filePath = this.activeMediaFile.webDavPath history.pushState({}, document.title, this.$router.resolve(this.$route).href) }, diff --git a/packages/web-pkg/src/composables/appDefaults/useAppFileHandling.ts b/packages/web-pkg/src/composables/appDefaults/useAppFileHandling.ts index f78439256d0..515c6163b43 100644 --- a/packages/web-pkg/src/composables/appDefaults/useAppFileHandling.ts +++ b/packages/web-pkg/src/composables/appDefaults/useAppFileHandling.ts @@ -29,30 +29,31 @@ export function useAppFileHandling(options: AppFileHandlingOptions): AppFileHand const isPublicLinkContext = options.isPublicLinkContext const publicLinkPassword = options.publicLinkPassword - const getUrlForResource = ({ path, downloadURL }: Resource, query: QueryParameters = null) => { - const queryStr = !query ? '' : qs.stringify(query) - if (!unref(isPublicLinkContext)) { - const urlPath = ['..', 'dav', 'files', store.getters.user.id, path.replace(/^\//, '')].join( - '/' - ) - return [client.files.getFileUrl(urlPath), queryStr].filter(Boolean).join('?') - } + const getUrlForResource = ( + { webDavPath, downloadURL }: Resource, + query: QueryParameters = null + ) => { + const queryStr = qs.stringify(query) + if (unref(isPublicLinkContext)) { + // If the resource does not contain the downloadURL we fallback to the normal + // public files path. + if (!downloadURL) { + // TODO: check whether we can fix the resource to always contain public-files in the webDavPath + const urlPath = ['public-files', webDavPath].join('/') + return [client.files.getFileUrl(urlPath), queryStr].filter(Boolean).join('?') + } - // If the resource does not contain the downloadURL we fallback to the normal - // public files path. - if (!downloadURL) { - const urlPath = ['..', 'dav', 'public-files', path].join('/') - return [client.files.getFileUrl(urlPath), queryStr].filter(Boolean).join('?') - } + // In a public context, i.e. public shares, the downloadURL contains a pre-signed url to + // download the file. + const [url, signedQuery] = downloadURL.split('?') - // In a public context, i.e. public shares, the downloadURL contains a pre-signed url to - // download the file. - const [url, signedQuery] = downloadURL.split('?') + // Since the pre-signed url contains query parameters and the caller of this method + // can also provide query parameters we have to combine them. + const combinedQuery = [queryStr, signedQuery].filter(Boolean).join('&') + return [url, combinedQuery].filter(Boolean).join('?') + } - // Since the pre-signed url contains query parameters and the caller of this method - // can also provide query parameters we have to combine them. - const combinedQuery = [queryStr, signedQuery].filter(Boolean).join('&') - return [url, combinedQuery].filter(Boolean).join('?') + return [client.files.getFileUrl(webDavPath), queryStr].filter(Boolean).join('?') } const getFileContents = async (filePath: string) => { diff --git a/packages/web-runtime/src/router/index.js b/packages/web-runtime/src/router/index.js index ac969f0722b..dca91e3e3d8 100644 --- a/packages/web-runtime/src/router/index.js +++ b/packages/web-runtime/src/router/index.js @@ -50,7 +50,9 @@ const base = document.querySelector('base') export const router = patchRouter( new Router({ parseQuery(query) { - return qs.parse(query) + return qs.parse(query, { + allowDots: true + }) }, stringifyQuery(obj) { return qs.stringify(obj, {