fix: resolving internal links into share spaces and shared files

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

packages/web-app-files/src/composables/index.ts

@@ -1,3 +1,4 @@
 export * from './actions'
 export * from './resourcesViewDefaults'
+export * from './openWithDefaultApp'
 export * from './shares'

packages/web-app-files/src/composables/openWithDefaultApp/index.ts

@@ -0,0 +1 @@
+export * from './useOpenWithDefaultApp'

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 }
+}

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')
   }
 })
 </script>

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'
+          })
+        }
       })
     }
 

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<Resource, any, any[]>()
 
     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'] })
       }
     }
 

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<SpaceResource>(),
+            resource: mock<Resource>({ isFolder: false })
+          })
+          expect(defaultEditorAction.handler).toHaveBeenCalled()
+        }
+      })
+    })
+    it('should not call the default action handler for folders', () => {
+      getWrapper({
+        setup: ({ openWithDefaultApp }, { defaultEditorAction }) => {
+          openWithDefaultApp({
+            space: mock<SpaceResource>(),
+            resource: mock<Resource>({ isFolder: true })
+          })
+          expect(defaultEditorAction.handler).not.toHaveBeenCalled()
+        }
+      })
+    })
+  })
+})
+
+function getWrapper({
+  setup,
+  defaultEditorAction = mock<Action>({ handler: jest.fn() })
+}: {
+  setup: (
+    instance: ReturnType<typeof useOpenWithDefaultApp>,
+    mocks: { defaultEditorAction: any }
+  ) => void
+  defaultEditorAction?: any
+}) {
+  jest.mocked(useFileActions).mockReturnValue(
+    mock<ReturnType<typeof useFileActions>>({
+      getDefaultEditorAction: () => defaultEditorAction
+    })
+  )
+
+  const mocks = { defaultEditorAction }
+
+  return {
+    wrapper: getComposableWrapper(() => {
+      const instance = useOpenWithDefaultApp()
+      setup(instance, mocks)
+    })
+  }
+}

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,19 +80,30 @@ describe('SharedWithMe view', () => {
   })
 })
 
-function getMountedWrapper({ mocks = {}, loading = false, files = [] } = {}) {
+function getMountedWrapper({
+  mocks = {},
+  loading = false,
+  files = [],
+  openWithDefaultAppQuery = ''
+} = {}) {
   jest.mocked(useResourcesViewDefaults).mockImplementation(() =>
     useResourcesViewDefaultsMock({
       storeItems: ref(files),
       areResourcesLoading: ref(loading)
     })
   )
   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<RouteLocation>({ name: 'files-shares-with-me' })
     }),
-    ...(mocks && mocks)
+    ...(mocks && mocks),
+    openWithDefaultApp
   }
   const storeOptions = { ...defaultStoreMockOptions }
   const store = createStore(storeOptions)

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'
       })
     )
   })

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