From 945170d5ff6805bd227b77bab11c2e1526c6bc8d Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Fri, 20 Oct 2023 09:49:09 +0200 Subject: [PATCH 1/4] fix: resolving external URLs via file ID --- .../unreleased/bugfix-external-url-resolving | 6 + .../OcNotificationMessage.vue | 2 +- packages/web-app-external/src/App.vue | 109 +++++++++++++++++- .../Shares/Collaborators/EditDropdown.vue | 2 +- .../web-pkg/src/composables/fileInfo/index.ts | 1 + .../fileInfo/useLoadFileInfoById.ts | 38 ++++++ 6 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 changelog/unreleased/bugfix-external-url-resolving create mode 100644 packages/web-pkg/src/composables/fileInfo/index.ts create mode 100644 packages/web-pkg/src/composables/fileInfo/useLoadFileInfoById.ts diff --git a/changelog/unreleased/bugfix-external-url-resolving b/changelog/unreleased/bugfix-external-url-resolving new file mode 100644 index 00000000000..30eb803ccde --- /dev/null +++ b/changelog/unreleased/bugfix-external-url-resolving @@ -0,0 +1,6 @@ +Bugfix: Resolving external URLs + +Resolving external URLs when only the file ID is given has been fixed. + +https://github.com/owncloud/web/issues/9804 +https://github.com/owncloud/web/pull/9833 diff --git a/packages/design-system/src/components/OcNotificationMessage/OcNotificationMessage.vue b/packages/design-system/src/components/OcNotificationMessage/OcNotificationMessage.vue index 15ec854d055..319a2c87598 100644 --- a/packages/design-system/src/components/OcNotificationMessage/OcNotificationMessage.vue +++ b/packages/design-system/src/components/OcNotificationMessage/OcNotificationMessage.vue @@ -11,7 +11,7 @@ {{ title }} - diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index d8c5a1bcd60..c8a21597c54 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -39,10 +39,26 @@ import { mapGetters } from 'vuex' import { computed, defineComponent, unref } from 'vue' import { urlJoin } from 'web-client/src/utils' import AppTopBar from 'web-pkg/src/components/AppTopBar.vue' -import { queryItemAsString, useAppDefaults, useRouteQuery } from 'web-pkg/src/composables' +import { + queryItemAsString, + useAppDefaults, + useClientService, + useRoute, + useRouteParam, + useRouteQuery, + useRouter, + useStore +} from 'web-pkg/src/composables' import { configurationManager } from 'web-pkg/src/configuration' import ErrorScreen from './components/ErrorScreen.vue' import LoadingScreen from './components/LoadingScreen.vue' +import { + Resource, + SpaceResource, + buildShareSpaceResource, + isMountPointSpaceResource +} from 'web-client/src/helpers' +import { useLoadFileInfoById } from 'web-pkg/src/composables/fileInfo' export default defineComponent({ name: 'ExternalApp', @@ -52,14 +68,99 @@ export default defineComponent({ LoadingScreen }, setup() { + const store = useStore() + const router = useRouter() + const currentRoute = useRoute() + const clientService = useClientService() + const { loadFileInfoByIdTask } = useLoadFileInfoById({ clientService }) const appName = useRouteQuery('app') const applicationName = computed(() => queryItemAsString(unref(appName))) + + const fileIdQueryItem = useRouteQuery('fileId') + const fileId = computed(() => { + return queryItemAsString(unref(fileIdQueryItem)) + }) + + const driveAliasAndItem = useRouteParam('driveAliasAndItem') + + const getMatchingSpace = (id): SpaceResource => { + return store.getters['runtime/spaces/spaces'].find((space) => id.startsWith(space.id)) + } + const findMatchingMountPoint = (id: string | number): SpaceResource => { + return store.getters['runtime/spaces/spaces'].find( + (space) => isMountPointSpaceResource(space) && space.root?.remoteItem?.id === id + ) + } + + const addMissingDriveAliasAndItem = async () => { + const id = unref(fileId) + let path: string + let matchingSpace = getMatchingSpace(id) + if (matchingSpace) { + path = await clientService.owncloudSdk.files.getPathForFileId(id) + const driveAliasAndItem = matchingSpace.getDriveAliasAndItem({ path } as Resource) + console.log(unref(currentRoute).query) + return router.push({ + params: { + ...unref(currentRoute).params, + driveAliasAndItem + }, + query: { + ...(unref(currentRoute).query?.app && { app: unref(currentRoute).query?.app }), + contextRouteName: 'files-spaces-generic' + } + }) + } + + // no matching space found => the file doesn't lie in own spaces => it's a share. + // do PROPFINDs on parents until root of accepted share is found in `mountpoint` spaces + await store.dispatch('runtime/spaces/loadMountPoints', { + graphClient: clientService.graphAuthenticated + }) + let mountPoint = findMatchingMountPoint(id) + const resource = await loadFileInfoByIdTask.perform(id) + const sharePathSegments = mountPoint ? [] : [unref(resource).name] + let tmpResource = unref(resource) + while (!mountPoint) { + try { + tmpResource = await loadFileInfoByIdTask.perform(tmpResource.parentFolderId) + } catch (e) { + throw Error(e) + } + mountPoint = findMatchingMountPoint(tmpResource.id) + if (!mountPoint) { + sharePathSegments.unshift(tmpResource.name) + } + } + matchingSpace = buildShareSpaceResource({ + shareId: mountPoint.nodeId, + shareName: mountPoint.name, + serverUrl: configurationManager.serverUrl + }) + path = urlJoin(...sharePathSegments) + + const driveAliasAndItem = matchingSpace.getDriveAliasAndItem({ path } as Resource) + return router.push({ + params: { + ...unref(currentRoute).params, + driveAliasAndItem + }, + query: { + shareId: matchingSpace.shareId, + ...(unref(currentRoute).query?.app && { app: unref(currentRoute).query?.app }), + contextRouteName: 'files-shares-with-me' + } + }) + } + return { ...useAppDefaults({ applicationId: 'external', applicationName }), - applicationName + applicationName, + driveAliasAndItem, + addMissingDriveAliasAndItem } }, @@ -94,6 +195,10 @@ export default defineComponent({ async created() { this.loading = true try { + if (!this.driveAliasAndItem) { + await this.addMissingDriveAliasAndItem() + } + this.resource = await this.getFileInfo(this.currentFileContext, { davProperties: [] }) diff --git a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/EditDropdown.vue b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/EditDropdown.vue index 08cad53e625..aa1421abc3f 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/EditDropdown.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/EditDropdown.vue @@ -29,8 +29,8 @@ > diff --git a/packages/web-pkg/src/composables/fileInfo/index.ts b/packages/web-pkg/src/composables/fileInfo/index.ts new file mode 100644 index 00000000000..2ffc69d647f --- /dev/null +++ b/packages/web-pkg/src/composables/fileInfo/index.ts @@ -0,0 +1 @@ +export * from './useLoadFileInfoById' diff --git a/packages/web-pkg/src/composables/fileInfo/useLoadFileInfoById.ts b/packages/web-pkg/src/composables/fileInfo/useLoadFileInfoById.ts new file mode 100644 index 00000000000..42cd94126c5 --- /dev/null +++ b/packages/web-pkg/src/composables/fileInfo/useLoadFileInfoById.ts @@ -0,0 +1,38 @@ +import { ClientService } from 'web-pkg/src/services' +import { useClientService } from 'web-pkg/src/composables' +import { useTask } from 'vue-concurrency' +import { buildSpace, buildWebDavSpacesPath } from 'web-client/src/helpers' +import { DavProperty } from 'web-client/src/webdav/constants' + +export interface LoadFileInfoByIdOptions { + clientService?: ClientService + davProperties?: DavProperty[] +} + +export const useLoadFileInfoById = (options: LoadFileInfoByIdOptions) => { + const { webdav } = options.clientService || useClientService() + const davProperties = options.davProperties || [ + DavProperty.FileId, + DavProperty.FileParent, + DavProperty.Name, + DavProperty.ResourceType + ] + + const loadFileInfoByIdTask = useTask(function* (signal, fileId: string | number) { + const space = buildSpace({ + id: fileId, + webDavPath: buildWebDavSpacesPath(fileId) + }) + return yield webdav.getFileInfo( + space, + {}, + { + davProperties + } + ) + }) + + return { + loadFileInfoByIdTask + } +} From 40aafa052313b10be0958197752b74ede4d5faa2 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Fri, 20 Oct 2023 11:08:49 +0200 Subject: [PATCH 2/4] test: fix unit tests --- packages/web-app-external/src/App.vue | 1 - packages/web-app-external/tests/unit/app.spec.ts | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index c8a21597c54..26ff70396b3 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -99,7 +99,6 @@ export default defineComponent({ if (matchingSpace) { path = await clientService.owncloudSdk.files.getPathForFileId(id) const driveAliasAndItem = matchingSpace.getDriveAliasAndItem({ path } as Resource) - console.log(unref(currentRoute).query) return router.push({ params: { ...unref(currentRoute).params, diff --git a/packages/web-app-external/tests/unit/app.spec.ts b/packages/web-app-external/tests/unit/app.spec.ts index 905b0ca92f3..d8d400031da 100644 --- a/packages/web-app-external/tests/unit/app.spec.ts +++ b/packages/web-app-external/tests/unit/app.spec.ts @@ -11,6 +11,7 @@ import { useAppDefaultsMock } from 'web-test-helpers/src/mocks/useAppDefaultsMoc import { ref } from 'vue' import { mock } from 'jest-mock-extended' import { RouteLocation } from 'web-test-helpers' +import { useRouteParam } from 'web-pkg/src/composables/router/useRouteParam' jest.mock('web-pkg/src/composables/appDefaults', () => { const { queryItemAsString } = jest.requireActual('web-pkg/src/composables/appDefaults') @@ -21,6 +22,8 @@ jest.mock('web-pkg/src/composables/appDefaults', () => { } }) +jest.mock('web-pkg/src/composables/router/useRouteParam') + const componentStubs = { AppTopBar: true, ErrorScreen: true, @@ -124,6 +127,7 @@ function createShallowMountWrapper(makeRequest = jest.fn().mockResolvedValue({ s makeRequest }) ) + jest.mocked(useRouteParam).mockReturnValue(ref('foo')) const storeOptions = defaultStoreMockOptions storeOptions.getters.capabilities.mockImplementation(() => ({ From 1f9713080bfd3a11da7d8618244715ee82ae7412 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 24 Oct 2023 10:57:37 +0200 Subject: [PATCH 3/4] improve method naming --- packages/web-app-external/src/App.vue | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index 26ff70396b3..1a65f68d3bd 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -83,10 +83,10 @@ export default defineComponent({ const driveAliasAndItem = useRouteParam('driveAliasAndItem') - const getMatchingSpace = (id): SpaceResource => { + const getMatchingSpaceByFileId = (id): SpaceResource => { return store.getters['runtime/spaces/spaces'].find((space) => id.startsWith(space.id)) } - const findMatchingMountPoint = (id: string | number): SpaceResource => { + const getMatchingMountPoint = (id: string | number): SpaceResource => { return store.getters['runtime/spaces/spaces'].find( (space) => isMountPointSpaceResource(space) && space.root?.remoteItem?.id === id ) @@ -95,7 +95,7 @@ export default defineComponent({ const addMissingDriveAliasAndItem = async () => { const id = unref(fileId) let path: string - let matchingSpace = getMatchingSpace(id) + let matchingSpace = getMatchingSpaceByFileId(id) if (matchingSpace) { path = await clientService.owncloudSdk.files.getPathForFileId(id) const driveAliasAndItem = matchingSpace.getDriveAliasAndItem({ path } as Resource) @@ -116,7 +116,7 @@ export default defineComponent({ await store.dispatch('runtime/spaces/loadMountPoints', { graphClient: clientService.graphAuthenticated }) - let mountPoint = findMatchingMountPoint(id) + let mountPoint = getMatchingMountPoint(id) const resource = await loadFileInfoByIdTask.perform(id) const sharePathSegments = mountPoint ? [] : [unref(resource).name] let tmpResource = unref(resource) @@ -126,7 +126,7 @@ export default defineComponent({ } catch (e) { throw Error(e) } - mountPoint = findMatchingMountPoint(tmpResource.id) + mountPoint = getMatchingMountPoint(tmpResource.id) if (!mountPoint) { sharePathSegments.unshift(tmpResource.name) } From 9c85ef62f48ea744c2378a9d462ecfd92dcf396e Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 24 Oct 2023 14:49:11 +0200 Subject: [PATCH 4/4] fix: redirect after closing external apps --- packages/web-app-external/src/App.vue | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index 1a65f68d3bd..0529b774668 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -59,6 +59,7 @@ import { isMountPointSpaceResource } from 'web-client/src/helpers' import { useLoadFileInfoById } from 'web-pkg/src/composables/fileInfo' +import { dirname } from 'path' export default defineComponent({ name: 'ExternalApp', @@ -105,8 +106,10 @@ export default defineComponent({ driveAliasAndItem }, query: { + fileId: id, ...(unref(currentRoute).query?.app && { app: unref(currentRoute).query?.app }), - contextRouteName: 'files-spaces-generic' + contextRouteName: 'files-spaces-generic', + contextRouteParams: { driveAliasAndItem: dirname(driveAliasAndItem) } as any } }) } @@ -145,9 +148,16 @@ export default defineComponent({ driveAliasAndItem }, query: { + fileId: id, shareId: matchingSpace.shareId, ...(unref(currentRoute).query?.app && { app: unref(currentRoute).query?.app }), - contextRouteName: 'files-shares-with-me' + contextRouteName: path === '/' ? 'files-shares-with-me' : 'files-spaces-generic', + contextRouteParams: { + driveAliasAndItem: dirname(driveAliasAndItem) + } as any, + contextRouteQuery: { + shareId: matchingSpace.shareId + } as any } }) }