From 447a41ca828dadc8bd26201f983876d6f42f3774 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 17 Oct 2023 10:19:43 +0200 Subject: [PATCH 1/5] fix: resolving internal links into share spaces and shared files --- .../composables/openWithDefaultApp/index.ts | 1 + .../useOpenWithDefaultApp.ts | 30 +++++++++ .../src/views/shares/SharedWithMe.vue | 29 +++++--- .../src/views/spaces/GenericSpace.vue | 26 ++------ .../useOpenWithDefaultApp.spec.ts | 66 +++++++++++++++++++ .../unit/views/shares/SharedWithMe.spec.ts | 37 +++++++++-- 6 files changed, 154 insertions(+), 35 deletions(-) create mode 100644 packages/web-app-files/src/composables/openWithDefaultApp/index.ts create mode 100644 packages/web-app-files/src/composables/openWithDefaultApp/useOpenWithDefaultApp.ts create mode 100644 packages/web-app-files/tests/unit/composables/openWithDefaultApp/useOpenWithDefaultApp.spec.ts diff --git a/packages/web-app-files/src/composables/openWithDefaultApp/index.ts b/packages/web-app-files/src/composables/openWithDefaultApp/index.ts new file mode 100644 index 00000000000..00906860b84 --- /dev/null +++ b/packages/web-app-files/src/composables/openWithDefaultApp/index.ts @@ -0,0 +1 @@ +export * from './useOpenWithDefaultApp' diff --git a/packages/web-app-files/src/composables/openWithDefaultApp/useOpenWithDefaultApp.ts b/packages/web-app-files/src/composables/openWithDefaultApp/useOpenWithDefaultApp.ts new file mode 100644 index 00000000000..0653ac13ed2 --- /dev/null +++ b/packages/web-app-files/src/composables/openWithDefaultApp/useOpenWithDefaultApp.ts @@ -0,0 +1,30 @@ +import { useFileActions } from '@ownclouders/web-pkg' +import { Resource, SpaceResource } from '@ownclouders/web-client' + +export function useOpenWithDefaultApp() { + const { getDefaultEditorAction } = useFileActions() + + const openWithDefaultApp = ({ + space, + resource + }: { + space: SpaceResource + resource: Resource + }) => { + if (!resource || resource.isFolder) { + return + } + + const fileActionsOptions = { + resources: [resource], + space: space + } + + const defaultEditorAction = getDefaultEditorAction(fileActionsOptions) + if (defaultEditorAction) { + defaultEditorAction.handler({ ...fileActionsOptions }) + } + } + + return { openWithDefaultApp } +} diff --git a/packages/web-app-files/src/views/shares/SharedWithMe.vue b/packages/web-app-files/src/views/shares/SharedWithMe.vue index 7c92200ca1d..1e6bd196f77 100644 --- a/packages/web-app-files/src/views/shares/SharedWithMe.vue +++ b/packages/web-app-files/src/views/shares/SharedWithMe.vue @@ -45,8 +45,9 @@ import { useResourcesViewDefaults } from '../../composables' import { AppLoadingSpinner, InlineFilterOption } from '@ownclouders/web-pkg' import { AppBar, ItemFilterInline } from '@ownclouders/web-pkg' +import { queryItemAsString, useRouteQuery } from '@ownclouders/web-pkg' import SharedWithMeSection from '../../components/Shares/SharedWithMeSection.vue' -import { computed, defineComponent, ref, unref } from 'vue' +import { computed, defineComponent, onMounted, ref, unref } from 'vue' import { Resource } from '@ownclouders/web-client' import SideBar from '../../components/SideBar/SideBar.vue' import FilesViewWrapper from '../../components/FilesViewWrapper.vue' @@ -55,6 +56,7 @@ import { useGroupingSettings } from '@ownclouders/web-pkg' import SharesNavigation from 'web-app-files/src/components/AppBar/SharesNavigation.vue' import { useGettext } from 'vue3-gettext' import { useStore } from '@ownclouders/web-pkg' +import { useOpenWithDefaultApp } from '../../composables/openWithDefaultApp' export default defineComponent({ components: { @@ -68,6 +70,8 @@ export default defineComponent({ }, setup() { + const { openWithDefaultApp } = useOpenWithDefaultApp() + const { areResourcesLoading, sortFields, @@ -123,8 +127,23 @@ export default defineComponent({ return getMatchingSpace(resource) }) + const openWithDefaultAppQuery = useRouteQuery('openWithDefaultApp') + const performLoaderTask = async () => { + await loadResourcesTask.perform() + scrollToResourceFromRoute(unref(items), 'files-app-bar') + if (queryItemAsString(unref(openWithDefaultAppQuery)) === 'true') { + openWithDefaultApp({ + space: unref(selectedShareSpace), + resource: unref(selectedResources)[0] + }) + } + } + + onMounted(() => { + performLoaderTask() + }) + return { - // defaults loadResourcesTask, areResourcesLoading, selectedResources, @@ -133,7 +152,6 @@ export default defineComponent({ sideBarOpen, sideBarActivePanel, selectedShareSpace, - scrollToResourceFromRoute, areHiddenFilesShown, visibilityOptions, @@ -151,11 +169,6 @@ export default defineComponent({ // CERN ...useGroupingSettings({ sortBy: sortBy, sortDir: sortDir }) } - }, - - async created() { - await this.loadResourcesTask.perform() - this.scrollToResourceFromRoute(this.items, 'files-app-bar') } }) diff --git a/packages/web-app-files/src/views/spaces/GenericSpace.vue b/packages/web-app-files/src/views/spaces/GenericSpace.vue index 8332e5b8fbb..44034844602 100644 --- a/packages/web-app-files/src/views/spaces/GenericSpace.vue +++ b/packages/web-app-files/src/views/spaces/GenericSpace.vue @@ -215,6 +215,7 @@ import { useKeyboardTableNavigation, useKeyboardTableSpaceActions } from 'web-app-files/src/composables/keyboardActions' +import { useOpenWithDefaultApp } from '../../composables/openWithDefaultApp' const visibilityObserver = new VisibilityObserver() @@ -260,18 +261,18 @@ export default defineComponent({ setup(props) { const store = useStore() const { $gettext, $ngettext } = useGettext() - const { getDefaultEditorAction } = useFileActions() const openWithDefaultAppQuery = useRouteQuery('openWithDefaultApp') const clientService = useClientService() const hasShareJail = useCapabilityShareJailEnabled() const { breadcrumbsFromPath, concatBreadcrumbs } = useBreadcrumbsFromPath() + const { openWithDefaultApp } = useOpenWithDefaultApp() const { actions: createNewFolder } = useFileActionsCreateNewFolder({ store, space: props.space }) const { isEnabled: isEmbedModeEnabled } = useEmbedMode() - let loadResourcesEventToken + let loadResourcesEventToken: string const canUpload = computed(() => { return store.getters['Files/currentFolder']?.canUpload({ user: store.getters.user }) @@ -432,25 +433,6 @@ export default defineComponent({ } } - const openWithDefaultApp = () => { - const highlightedFile = store.getters['Files/highlightedFile'] - - if (!highlightedFile || highlightedFile.isFolder) { - return - } - - const fileActionsOptions = { - resources: [highlightedFile], - space: props.space - } - - const defaultEditorAction = getDefaultEditorAction(fileActionsOptions) - - if (defaultEditorAction) { - defaultEditorAction.handler({ ...fileActionsOptions }) - } - } - const resourcesViewDefaults = useResourcesViewDefaults() const keyActions = useKeyboardActions() @@ -487,7 +469,7 @@ export default defineComponent({ focusAndAnnounceBreadcrumb(sameRoute) if (unref(openWithDefaultAppQuery) === 'true') { - openWithDefaultApp() + openWithDefaultApp({ space: props.space, resource: store.getters['Files/highlightedFile'] }) } } diff --git a/packages/web-app-files/tests/unit/composables/openWithDefaultApp/useOpenWithDefaultApp.spec.ts b/packages/web-app-files/tests/unit/composables/openWithDefaultApp/useOpenWithDefaultApp.spec.ts new file mode 100644 index 00000000000..24993555a0a --- /dev/null +++ b/packages/web-app-files/tests/unit/composables/openWithDefaultApp/useOpenWithDefaultApp.spec.ts @@ -0,0 +1,66 @@ +import { getComposableWrapper } from 'web-test-helpers' +import { mock } from 'jest-mock-extended' +import { Resource, SpaceResource } from '@ownclouders/web-client' +import { useOpenWithDefaultApp } from '../../../../src/composables/openWithDefaultApp' +import { useFileActions, Action } from '@ownclouders/web-pkg' + +jest.mock('@ownclouders/web-pkg', () => ({ + ...jest.requireActual('@ownclouders/web-pkg'), + useFileActions: jest.fn() +})) + +describe('useOpenWithDefaultApp', () => { + it('should be valid', () => { + expect(useOpenWithDefaultApp).toBeDefined() + }) + describe('method "openWithDefaultApp"', () => { + it('should call the default action handler for files', () => { + getWrapper({ + setup: ({ openWithDefaultApp }, { defaultEditorAction }) => { + openWithDefaultApp({ + space: mock(), + resource: mock({ isFolder: false }) + }) + expect(defaultEditorAction.handler).toHaveBeenCalled() + } + }) + }) + it('should not call the default action handler for folders', () => { + getWrapper({ + setup: ({ openWithDefaultApp }, { defaultEditorAction }) => { + openWithDefaultApp({ + space: mock(), + resource: mock({ isFolder: true }) + }) + expect(defaultEditorAction.handler).not.toHaveBeenCalled() + } + }) + }) + }) +}) + +function getWrapper({ + setup, + defaultEditorAction = mock({ handler: jest.fn() }) +}: { + setup: ( + instance: ReturnType, + mocks: { defaultEditorAction: any } + ) => void + defaultEditorAction?: any +}) { + jest.mocked(useFileActions).mockReturnValue( + mock>({ + getDefaultEditorAction: () => defaultEditorAction + }) + ) + + const mocks = { defaultEditorAction } + + return { + wrapper: getComposableWrapper(() => { + const instance = useOpenWithDefaultApp() + setup(instance, mocks) + }) + } +} diff --git a/packages/web-app-files/tests/unit/views/shares/SharedWithMe.spec.ts b/packages/web-app-files/tests/unit/views/shares/SharedWithMe.spec.ts index 5493bcb37b2..f1f2da689f8 100644 --- a/packages/web-app-files/tests/unit/views/shares/SharedWithMe.spec.ts +++ b/packages/web-app-files/tests/unit/views/shares/SharedWithMe.spec.ts @@ -1,6 +1,6 @@ import SharedWithMe from '../../../../src/views/shares/SharedWithMe.vue' import { useResourcesViewDefaults } from 'web-app-files/src/composables' -import { InlineFilterOption, useSort } from '@ownclouders/web-pkg' +import { queryItemAsString, InlineFilterOption, useSort } from '@ownclouders/web-pkg' import { useResourcesViewDefaultsMock } from 'web-app-files/tests/mocks/useResourcesViewDefaultsMock' import { ref } from 'vue' import { defaultStubs, RouteLocation } from 'web-test-helpers' @@ -13,11 +13,15 @@ import { defaultStoreMockOptions, defaultComponentMocks } from 'web-test-helpers' +import { useOpenWithDefaultApp } from '../../../../src/composables/openWithDefaultApp' -jest.mock('web-app-files/src/composables') +jest.mock('web-app-files/src/composables/resourcesViewDefaults') +jest.mock('web-app-files/src/composables/openWithDefaultApp/useOpenWithDefaultApp') jest.mock('@ownclouders/web-pkg', () => ({ ...jest.requireActual('@ownclouders/web-pkg'), - useSort: jest.fn().mockImplementation(() => useSortMock()) + useSort: jest.fn().mockImplementation(() => useSortMock()), + queryItemAsString: jest.fn(), + useRouteQuery: jest.fn() })) describe('SharedWithMe view', () => { @@ -39,6 +43,18 @@ describe('SharedWithMe view', () => { expect(wrapper.find('oc-spinner-stub').exists()).toBeFalsy() }) }) + describe('open with default app', () => { + it('gets called if given via route query param', async () => { + const { wrapper, mocks } = getMountedWrapper({ openWithDefaultAppQuery: 'true' }) + await wrapper.vm.loadResourcesTask.last + expect(mocks.openWithDefaultApp).toHaveBeenCalled() + }) + it('gets not called if not given via route query param', async () => { + const { wrapper, mocks } = getMountedWrapper() + await wrapper.vm.loadResourcesTask.last + expect(mocks.openWithDefaultApp).not.toHaveBeenCalled() + }) + }) describe('filter', () => { it('shows the share visibility filter', () => { const { wrapper } = getMountedWrapper() @@ -64,7 +80,12 @@ describe('SharedWithMe view', () => { }) }) -function getMountedWrapper({ mocks = {}, loading = false, files = [] } = {}) { +function getMountedWrapper({ + mocks = {}, + loading = false, + files = [], + openWithDefaultAppQuery = '' +} = {}) { jest.mocked(useResourcesViewDefaults).mockImplementation(() => useResourcesViewDefaultsMock({ storeItems: ref(files), @@ -72,11 +93,17 @@ function getMountedWrapper({ mocks = {}, loading = false, files = [] } = {}) { }) ) jest.mocked(useSort).mockImplementation((options) => useSortMock({ items: ref(options.items) })) + jest.mocked(queryItemAsString).mockImplementationOnce(() => openWithDefaultAppQuery) + + const openWithDefaultApp = jest.fn() + jest.mocked(useOpenWithDefaultApp).mockReturnValue({ openWithDefaultApp }) + const defaultMocks = { ...defaultComponentMocks({ currentRoute: mock({ name: 'files-shares-with-me' }) }), - ...(mocks && mocks) + ...(mocks && mocks), + openWithDefaultApp } const storeOptions = { ...defaultStoreMockOptions } const store = createStore(storeOptions) From 063ddf94a076c4af45756d8c8025021df71d3bf2 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 17 Oct 2023 11:50:13 +0200 Subject: [PATCH 2/5] test: remove invalid test steps --- tests/e2e/cucumber/features/smoke/internalLink.feature | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/e2e/cucumber/features/smoke/internalLink.feature b/tests/e2e/cucumber/features/smoke/internalLink.feature index 700d808de43..e77a7ec18f9 100644 --- a/tests/e2e/cucumber/features/smoke/internalLink.feature +++ b/tests/e2e/cucumber/features/smoke/internalLink.feature @@ -17,9 +17,6 @@ Feature: internal link share When "Alice" edits the public link named "Link" of resource "myfolder" changing role to "Invited people" And "Brian" opens the public link "Link" And "Brian" logs in from the internal link - - # comment out after fixing #9776 - # And "Brian" opens shared-with-me page from the internal link And "Brian" opens the "files" app And "Brian" navigates to the shared with me page And "Brian" uploads the following resource From c5b336d4ba5724ef77e6ac81eed57cdad224846c Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 17 Oct 2023 14:03:07 +0200 Subject: [PATCH 3/5] fix: resolving public links to default app if configured --- .../src/views/spaces/DriveResolver.vue | 16 ++++++++++++++-- .../unit/views/spaces/DriveResolver.spec.ts | 3 +-- tests/e2e/cucumber/features/smoke/link.feature | 8 ++++---- .../features/smoke/spaces/publicLink.feature | 8 ++++---- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/web-app-files/src/views/spaces/DriveResolver.vue b/packages/web-app-files/src/views/spaces/DriveResolver.vue index df46cb0e971..b2a9f13f0ad 100644 --- a/packages/web-app-files/src/views/spaces/DriveResolver.vue +++ b/packages/web-app-files/src/views/spaces/DriveResolver.vue @@ -21,6 +21,7 @@ import { computed, defineComponent, onMounted, ref, unref } from 'vue' import { queryItemAsString, useClientService, + useConfigurationManager, useDriveResolver, useGetMatchingSpace, useRouteParam, @@ -65,6 +66,7 @@ export default defineComponent({ const isTrashRoute = useActiveLocation(isLocationTrashActive, 'files-trash-generic') const resolvedDrive = useDriveResolver({ store, driveAliasAndItem }) const { getInternalSpace } = useGetMatchingSpace() + const configurationManager = useConfigurationManager() const loading = ref(true) const isLoading = computed(() => { @@ -103,7 +105,13 @@ export default defineComponent({ return router.push( createLocationSpaces('files-spaces-generic', { params, - query: { ...query, scrollTo: unref(resource).fileId } + query: { + ...query, + scrollTo: unref(resource).fileId, + ...(configurationManager.options.openLinksWithDefaultApp && { + openWithDefaultApp: 'true' + }) + } }) ) } @@ -112,7 +120,11 @@ export default defineComponent({ return router.push({ name: 'resolvePrivateLink', params: { fileId: unref(fileId) }, - query: { openWithDefaultApp: 'false' } + query: { + ...(configurationManager.options.openLinksWithDefaultApp && { + openWithDefaultApp: 'true' + }) + } }) } diff --git a/packages/web-app-files/tests/unit/views/spaces/DriveResolver.spec.ts b/packages/web-app-files/tests/unit/views/spaces/DriveResolver.spec.ts index 75814b95157..3a2eb30aedf 100644 --- a/packages/web-app-files/tests/unit/views/spaces/DriveResolver.spec.ts +++ b/packages/web-app-files/tests/unit/views/spaces/DriveResolver.spec.ts @@ -98,8 +98,7 @@ describe('DriveResolver view', () => { await wrapper.vm.$nextTick() expect(mocks.$router.push).toHaveBeenCalledWith( expect.objectContaining({ - name: 'resolvePrivateLink', - query: { openWithDefaultApp: 'false' } + name: 'resolvePrivateLink' }) ) }) diff --git a/tests/e2e/cucumber/features/smoke/link.feature b/tests/e2e/cucumber/features/smoke/link.feature index 8789152f555..578ab604304 100644 --- a/tests/e2e/cucumber/features/smoke/link.feature +++ b/tests/e2e/cucumber/features/smoke/link.feature @@ -103,11 +103,11 @@ Feature: link | resource | | lorem.txt | When "Brian" opens the public link "textLink" - Then for "Brian" file "shareToBrian.txt" should be selected + And "Brian" closes the file viewer When "Brian" opens the public link "markdownLink" - Then for "Brian" file "shareToBrian.md" should be selected + And "Brian" closes the file viewer When "Brian" opens the public link "pdfLink" - Then for "Brian" file "simple.pdf" should be selected + And "Brian" closes the file viewer When "Brian" opens the public link "imageLink" - Then for "Brian" file "testavatar.jpg" should be selected + And "Brian" closes the file viewer And "Brian" logs out diff --git a/tests/e2e/cucumber/features/smoke/spaces/publicLink.feature b/tests/e2e/cucumber/features/smoke/spaces/publicLink.feature index adb54ac84c8..69c1a821381 100644 --- a/tests/e2e/cucumber/features/smoke/spaces/publicLink.feature +++ b/tests/e2e/cucumber/features/smoke/spaces/publicLink.feature @@ -50,9 +50,9 @@ Feature: spaces public link Then "Brian" should not be able to edit the public link named "spaceLink" And "Brian" should not be able to edit the public link named "folderLink" When "Brian" opens the public link "textLink" - Then for "Brian" file "shareToBrian.txt" should be selected + And "Brian" closes the file viewer When "Brian" opens the public link "markdownLink" - Then for "Brian" file "shareToBrian.md" should be selected + And "Brian" closes the file viewer And "Brian" logs out When "Carol" logs in And "Carol" opens the public link "spaceLink" @@ -61,14 +61,14 @@ Feature: spaces public link When "Carol" opens the public link "folderLink" Then "Carol" should see folder "subFolder" but should not be able to edit When "Carol" opens the public link "pdfLink" - Then for "Carol" file "simple.pdf" should be selected + And "Carol" closes the file viewer And "Carol" logs out When "David" logs in And "David" opens the public link "spaceLink" And "David" edits the public link named "spaceLink" of the space changing role to "Can edit" And "David" edits the public link named "folderLink" of resource "spaceFolder" changing role to "Can edit" When "David" opens the public link "imageLink" - Then for "David" file "testavatar.jpg" should be selected + And "David" closes the file viewer And "David" logs out From 2502c54a5ba9ca87967f25d6dece037f1f4a76fb Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 17 Oct 2023 16:43:31 +0200 Subject: [PATCH 4/5] docs: add changelog item --- .../unreleased/bugfix-link-resolving-into-default-app | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/unreleased/bugfix-link-resolving-into-default-app diff --git a/changelog/unreleased/bugfix-link-resolving-into-default-app b/changelog/unreleased/bugfix-link-resolving-into-default-app new file mode 100644 index 00000000000..5e8fc952091 --- /dev/null +++ b/changelog/unreleased/bugfix-link-resolving-into-default-app @@ -0,0 +1,7 @@ +Bugfix: Link resolving into default app + +Internal and public file links now reliably resolve into the default app when `openLinksWithDefaultApp` is enabled. + +https://github.com/owncloud/web/issues/9799 +https://github.com/owncloud/web/issues/9776 +https://github.com/owncloud/web/pull/9821 From 723f54d6999bf0781e9020149df40305584df725 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 31 Oct 2023 07:16:28 +0100 Subject: [PATCH 5/5] feat: add export via index for useOpenWithDefaultApp composable --- packages/web-app-files/src/composables/index.ts | 1 + packages/web-app-files/src/views/shares/SharedWithMe.vue | 2 +- packages/web-app-files/src/views/spaces/GenericSpace.vue | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/web-app-files/src/composables/index.ts b/packages/web-app-files/src/composables/index.ts index d9b8d9e490f..ac0ddb2e5af 100644 --- a/packages/web-app-files/src/composables/index.ts +++ b/packages/web-app-files/src/composables/index.ts @@ -1,3 +1,4 @@ export * from './actions' export * from './resourcesViewDefaults' +export * from './openWithDefaultApp' export * from './shares' diff --git a/packages/web-app-files/src/views/shares/SharedWithMe.vue b/packages/web-app-files/src/views/shares/SharedWithMe.vue index 1e6bd196f77..888b004ed4c 100644 --- a/packages/web-app-files/src/views/shares/SharedWithMe.vue +++ b/packages/web-app-files/src/views/shares/SharedWithMe.vue @@ -56,7 +56,7 @@ import { useGroupingSettings } from '@ownclouders/web-pkg' import SharesNavigation from 'web-app-files/src/components/AppBar/SharesNavigation.vue' import { useGettext } from 'vue3-gettext' import { useStore } from '@ownclouders/web-pkg' -import { useOpenWithDefaultApp } from '../../composables/openWithDefaultApp' +import { useOpenWithDefaultApp } from '../../composables' export default defineComponent({ components: { diff --git a/packages/web-app-files/src/views/spaces/GenericSpace.vue b/packages/web-app-files/src/views/spaces/GenericSpace.vue index 44034844602..9c402b97953 100644 --- a/packages/web-app-files/src/views/spaces/GenericSpace.vue +++ b/packages/web-app-files/src/views/spaces/GenericSpace.vue @@ -215,7 +215,7 @@ import { useKeyboardTableNavigation, useKeyboardTableSpaceActions } from 'web-app-files/src/composables/keyboardActions' -import { useOpenWithDefaultApp } from '../../composables/openWithDefaultApp' +import { useOpenWithDefaultApp } from '../../composables' const visibilityObserver = new VisibilityObserver()