Merge pull request #9821 from owncloud/fix-internal-share-space-resol…

…ving fix: resolving internal links into share spaces and shared files
owncloud · Oct 31, 2023 · 91ef1c1 · 91ef1c1
2 parents 2f9f896 + 723f54d
commit 91ef1c1
Show file tree

Hide file tree

Showing 13 changed files with 185 additions and 50 deletions.
diff --git a/changelog/unreleased/bugfix-link-resolving-into-default-app 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
@@ -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
@@ -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
@@ -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
@@ -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>
diff --git 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
@@ -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'] })
       }
     }
 

diff --git a/...ges/web-app-files/tests/unit/composables/openWithDefaultApp/useOpenWithDefaultApp.spec.ts b/...ges/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)
+    })
+  }
+}
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
@@ -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)

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
@@ -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
@@ -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