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 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/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..888b004ed4c 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' 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/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/src/views/spaces/GenericSpace.vue b/packages/web-app-files/src/views/spaces/GenericSpace.vue index 8332e5b8fbb..9c402b97953 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' 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) 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/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 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