fix: private link error messages and resolving hidden shares

changelog/unreleased/bugfix-private-link-error-messages

@@ -0,0 +1,6 @@
+Bugfix: Private link error messages
+
+Private links not showing proper error messages has been fixed.
+
+https://github.com/owncloud/web/pull/10321
+https://github.com/owncloud/web/issues/10315

changelog/unreleased/enhancement-show-hide-shares

@@ -8,3 +8,4 @@ Furthermore, accepting and rejecting shares has been renamed to "enable sync"/"d
 https://github.com/owncloud/web/issues/9531
 https://github.com/owncloud/web/pull/9718
 https://github.com/owncloud/web/pull/10097
+https://github.com/owncloud/web/pull/10321

packages/web-runtime/src/pages/resolvePrivateLink.vue

@@ -15,18 +15,15 @@
       </template>
       <template v-else-if="errorMessage">
         <div class="oc-card-header oc-link-resolve-error-title">
-          <h2 v-if="isUnacceptedShareError">
-            {{ resource.name }}
-          </h2>
-          <h2 v-else key="private-link-error">
+          <h2 key="private-link-error">
             <span v-text="$gettext('An error occurred while resolving the private link')" />
           </h2>
         </div>
         <div class="oc-card-body oc-link-resolve-error-message">
           <p class="oc-text-xlarge">{{ errorMessage }}</p>
           <p
             v-if="isUnacceptedShareError"
-            v-text="$gettext('Note: You can reload this page after you accept the share.')"
+            v-text="$gettext('You can reload this page after you have enabled syncing the share.')"
           />
         </div>
       </template>
@@ -53,7 +50,8 @@ import {
   useRouteQuery,
   useConfigurationManager,
   createLocationSpaces,
-  createLocationShares
+  createLocationShares,
+  useClientService
 } from '@ownclouders/web-pkg'
 import { unref, defineComponent, computed, onMounted, ref, Ref } from 'vue'
 // import { createLocationSpaces } from 'web-app-files/src/router'
@@ -71,6 +69,8 @@ export default defineComponent({
     const id = useRouteParam('fileId')
     const configurationManager = useConfigurationManager()
     const { $gettext } = useGettext()
+    const clientService = useClientService()
+
     const resource: Ref<Resource> = ref()
     const sharedParentResource: Ref<Resource> = ref()
     const isUnacceptedShareError = ref(false)
@@ -90,60 +90,74 @@ export default defineComponent({
     })
 
     const resolvePrivateLinkTask = useTask(function* (signal, id) {
+      if (
+        [
+          `${SHARE_JAIL_ID}$${SHARE_JAIL_ID}!${SHARE_JAIL_ID}`,
+          `${SHARE_JAIL_ID}$${SHARE_JAIL_ID}`
+        ].includes(id)
+      ) {
+        return router.push(createLocationShares('files-shares-with-me'))
+      }
+
+      let result: Awaited<ReturnType<typeof getResourceContext>>
       try {
-        if (
-          [
-            `${SHARE_JAIL_ID}$${SHARE_JAIL_ID}!${SHARE_JAIL_ID}`,
-            `${SHARE_JAIL_ID}$${SHARE_JAIL_ID}`
-          ].includes(id)
-        ) {
-          return router.push(createLocationShares('files-shares-with-me'))
-        }
+        result = yield getResourceContext(id)
+      } catch (e) {
+        // error means the resurce is an unaccepted/unsynced share
+        isUnacceptedShareError.value = true
+        throw Error(e)
+      }
 
-        const result = yield getResourceContext(id)
-        const { space, resource } = result
-        let { path } = result
+      const { space, resource } = result
+      let { path } = result
 
-        let resourceIsNestedInShare = false
-        if (isShareSpaceResource(space)) {
-          sharedParentResource.value = resource
-          resourceIsNestedInShare = path !== '/'
-        }
+      if (!path) {
+        // empty path means the user has no access to the resource or it doesn't exist
+        throw new Error('The file or folder does not exist')
+      }
 
-        let fileId: string
-        let targetLocation: RouteLocationNamedRaw
-        if (unref(resource).type === 'folder') {
-          fileId = unref(resource).fileId
-          targetLocation = createLocationSpaces('files-spaces-generic')
-        } else {
-          fileId = unref(resource).parentFolderId
-          path = dirname(path)
-          targetLocation =
-            space.driveType === 'share' && !resourceIsNestedInShare
-              ? createLocationShares('files-shares-with-me')
-              : createLocationSpaces('files-spaces-generic')
+      let resourceIsNestedInShare = false
+      let isHiddenShare = false
+      if (isShareSpaceResource(space)) {
+        sharedParentResource.value = resource
+        resourceIsNestedInShare = path !== '/'
+        if (!resourceIsNestedInShare && space.shareId) {
+          const { shareInfo } = yield clientService.owncloudSdk.shares.getShare(space.shareId)
+          isHiddenShare = shareInfo.hidden === 'true'
         }
+      }
 
-        const { params, query } = createFileRouteOptions(space, { fileId, path })
-        const openWithDefault =
-          configurationManager.options.openLinksWithDefaultApp &&
-          unref(openWithDefaultApp) !== 'false' &&
-          !unref(details)
-
-        targetLocation.params = params
-        targetLocation.query = {
-          ...query,
-          scrollTo:
-            targetLocation.name === 'files-shares-with-me' ? space.shareId : unref(resource).fileId,
-          ...(unref(details) && { details: unref(details) }),
-          ...(openWithDefault && { openWithDefaultApp: 'true' })
-        }
+      let fileId: string
+      let targetLocation: RouteLocationNamedRaw
+      if (unref(resource).type === 'folder') {
+        fileId = unref(resource).fileId
+        targetLocation = createLocationSpaces('files-spaces-generic')
+      } else {
+        fileId = unref(resource).parentFolderId
+        path = dirname(path)
+        targetLocation =
+          space.driveType === 'share' && !resourceIsNestedInShare
+            ? createLocationShares('files-shares-with-me')
+            : createLocationSpaces('files-spaces-generic')
+      }
 
-        router.push(targetLocation)
-      } catch (e) {
-        isUnacceptedShareError.value = true
-        throw Error(e)
+      const { params, query } = createFileRouteOptions(space, { fileId, path })
+      const openWithDefault =
+        configurationManager.options.openLinksWithDefaultApp &&
+        unref(openWithDefaultApp) !== 'false' &&
+        !unref(details)
+
+      targetLocation.params = params
+      targetLocation.query = {
+        ...query,
+        scrollTo:
+          targetLocation.name === 'files-shares-with-me' ? space.shareId : unref(resource).fileId,
+        ...(unref(details) && { details: unref(details) }),
+        ...(isHiddenShare && { 'q_share-visibility': 'hidden' }),
+        ...(openWithDefault && { openWithDefaultApp: 'true' })
       }
+
+      router.push(targetLocation)
     })
 
     const loading = computed(() => {
@@ -160,30 +174,13 @@ export default defineComponent({
 
     const errorMessage = computed(() => {
       if (unref(isUnacceptedShareError)) {
-        if (
-          !unref(sharedParentResource) ||
-          unref(resource).name === unref(sharedParentResource).name
-        ) {
-          if (unref(resource).type === 'file') {
-            return $gettext(
-              'This file has been shared with you. Accept it in "Shares" > "Shared with me" to view it.'
-            )
-          }
+        if (!unref(sharedParentResource)) {
           return $gettext(
-            'This folder has been shared with you. Accept it in "Shares" > "Shared with me" to view it.'
-          )
-        }
-
-        if (unref(resource).type === 'file') {
-          return $gettext(
-            'This file has been shared with you via "%{parentShareName}". Accept the share "%{parentShareName}" in "Shares" > "Shared with me" to view it.',
-            {
-              parentShareName: unref(sharedParentResource).name
-            }
+            'This file or folder has been shared with you. Enable the sync in "Shares" > "Shared with me" to view it.'
           )
         } else {
           return $gettext(
-            'This folder has been shared with you via "%{parentShareName}". Accept the share "%{parentShareName}" in "Shares" > "Shared with me" to view it.',
+            'This file or folder has been shared with you via "%{parentShareName}". Enable the sync for the share "%{parentShareName}" in "Shares" > "Shared with me" to view it.',
             {
               parentShareName: unref(sharedParentResource).name
             }

packages/web-runtime/tests/unit/pages/resolvePrivateLink.spec.ts

@@ -39,7 +39,7 @@ describe('resolvePrivateLink', () => {
     const driveAliasAndItem = 'personal/home'
     const space = mock<SpaceResource>({ getDriveAliasAndItem: () => driveAliasAndItem })
     const resource = mock<Resource>({ fileId })
-    const { wrapper, mocks } = getWrapper({ space, resource, fileId })
+    const { wrapper, mocks } = getWrapper({ space, resource, fileId, path: '/' })
     await wrapper.vm.resolvePrivateLinkTask.last
     expect(mocks.$router.push).toHaveBeenCalledWith(
       expect.objectContaining({
@@ -74,25 +74,57 @@ describe('resolvePrivateLink', () => {
         expect.objectContaining({ name: 'files-shares-with-me' })
       )
     })
+    it('adds the hidden share param for hidden shares', async () => {
+      const fileId = '1'
+      const driveAliasAndItem = 'shares/someShare'
+      const space = mock<SpaceResource>({
+        driveType: 'share',
+        getDriveAliasAndItem: () => driveAliasAndItem
+      })
+      const resource = mock<Resource>({ fileId, type: 'file' })
+      const { wrapper, mocks } = getWrapper({
+        space,
+        resource,
+        fileId,
+        path: '/',
+        hiddenShare: true
+      })
+      await wrapper.vm.resolvePrivateLinkTask.last
+      expect(mocks.$router.push).toHaveBeenCalledWith(
+        expect.objectContaining({
+          query: expect.objectContaining({ 'q_share-visibility': 'hidden' })
+        })
+      )
+    })
   })
   it('passes the details query param if given via query', async () => {
     const details = 'sharing'
-    const { wrapper, mocks } = getWrapper({ details })
+    const { wrapper, mocks } = getWrapper({ details, path: '/' })
     await wrapper.vm.resolvePrivateLinkTask.last
     expect(mocks.$router.push).toHaveBeenCalledWith(
       expect.objectContaining({ query: expect.objectContaining({ details }) })
     )
   })
+  it('throws an error if the path is empty', async () => {
+    const { wrapper } = getWrapper()
+    try {
+      await wrapper.vm.resolvePrivateLinkTask.last
+    } catch (e) {}
+
+    expect(wrapper.find('.oc-link-resolve-error-message p').text()).toEqual(
+      'The file or folder does not exist'
+    )
+  })
   describe('openWithDefaultApp', () => {
     it('correctly passes the openWithDefaultApp param if enabled and given via query', async () => {
-      const { wrapper, mocks } = getWrapper()
+      const { wrapper, mocks } = getWrapper({ path: '/' })
       await wrapper.vm.resolvePrivateLinkTask.last
       expect(mocks.$router.push).toHaveBeenCalledWith(
         expect.objectContaining({ query: expect.objectContaining({ openWithDefaultApp: 'true' }) })
       )
     })
     it('does not pass the openWithDefaultApp param when details param is given', async () => {
-      const { wrapper, mocks } = getWrapper({ details: 'sharing' })
+      const { wrapper, mocks } = getWrapper({ details: 'sharing', path: '/' })
       await wrapper.vm.resolvePrivateLinkTask.last
       expect(mocks.$router.push).toHaveBeenCalledWith(
         expect.objectContaining({
@@ -101,7 +133,7 @@ describe('resolvePrivateLink', () => {
       )
     })
     it('does not pass the openWithDefaultApp param when disabled via config', async () => {
-      const { wrapper, mocks } = getWrapper({ openLinksWithDefaultApp: false })
+      const { wrapper, mocks } = getWrapper({ openLinksWithDefaultApp: false, path: '/' })
       await wrapper.vm.resolvePrivateLinkTask.last
       expect(mocks.$router.push).toHaveBeenCalledWith(
         expect.objectContaining({
@@ -110,7 +142,7 @@ describe('resolvePrivateLink', () => {
       )
     })
     it('does not pass the openWithDefaultApp param when not requested via query', async () => {
-      const { wrapper, mocks } = getWrapper({ openWithDefaultAppQuery: 'false' })
+      const { wrapper, mocks } = getWrapper({ openWithDefaultAppQuery: 'false', path: '/' })
       await wrapper.vm.resolvePrivateLinkTask.last
       expect(mocks.$router.push).toHaveBeenCalledWith(
         expect.objectContaining({
@@ -127,6 +159,7 @@ function getWrapper({
   path = '',
   fileId = '',
   details = '',
+  hiddenShare = false,
   openWithDefaultAppQuery = 'true',
   openLinksWithDefaultApp = true
 } = {}) {
@@ -155,6 +188,10 @@ function getWrapper({
   )
 
   const mocks = { ...defaultComponentMocks() }
+  mocks.$clientService.owncloudSdk.shares.getShare.mockResolvedValue({
+    shareInfo: { hidden: hiddenShare ? 'true' : 'false' }
+  })
+
   const storeOptions = defaultStoreMockOptions
   const store = createStore(storeOptions)