Merge pull request #6505 from owncloud/fix-root-node-info-right-sidebar

Fix "no selection" panel for root nodes in all files and public links
owncloud · Mar 2, 2022 · 18668e1 · 18668e1
2 parents af82faf + f33196e
commit 18668e1
Show file tree

Hide file tree

Showing 8 changed files with 193 additions and 69 deletions.
diff --git a/changelog/unreleased/bugfix-right-sidebar-no-selection b/changelog/unreleased/bugfix-right-sidebar-no-selection
@@ -0,0 +1,7 @@
+Bugfix: No selection info right sidebar
+
+We fixed that the right sidebar was not showing the "no selection" info panel anymore in the root of "All files". In addition we also use the same "no selection" info panel now in the root nodes of public links.
+
+https://github.com/owncloud/web/issues/6502
+https://github.com/owncloud/web/issues/6182
+https://github.com/owncloud/web/pull/6505
diff --git a/changelog/unreleased/enhancement-update-ods b/changelog/unreleased/enhancement-update-ods
@@ -1,11 +1,12 @@
-Enhancement: Update ODS to v12.2.0
+Enhancement: Update ODS to v12.2.1
 
-We updated the ownCloud Design System to version 12.2.0. Please refer to the full changelog in the ODS release (linked) for more details. Summary:
+We updated the ownCloud Design System to version 12.2.1. Please refer to the full changelog in the ODS release (linked) for more details. Summary:
 
 - Enhancement - Apply outstanding background color to oc-card: https://github.com/owncloud/owncloud-design-system/pull/1974
 - Enhancement - Redesign OcBreadcrumb: https://github.com/owncloud/web/issues/6218
 - Enhancement - Redesign files table related components: https://github.com/owncloud/owncloud-design-system/pull/1958
 
 https://github.com/owncloud/web/pull/6450
 https://github.com/owncloud/web/pull/6472
-https://github.com/owncloud/owncloud-design-system/releases/tag/v12.2.0
+https://github.com/owncloud/web/pull/6505
+https://github.com/owncloud/owncloud-design-system/releases/tag/v12.2.1
diff --git a/packages/web-app-files/src/components/SideBar/SideBar.vue b/packages/web-app-files/src/components/SideBar/SideBar.vue
@@ -92,7 +92,11 @@ import { VisibilityObserver } from 'web-pkg/src/observer'
 import { DavProperties } from 'web-pkg/src/constants'
 
 import { buildResource } from '../../helpers/resources'
-import { isLocationCommonActive, isLocationSharesActive } from '../../router'
+import {
+  isLocationCommonActive,
+  isLocationPublicActive,
+  isLocationSharesActive
+} from '../../router'
 import { computed } from '@vue/composition-api'
 
 import FileInfo from './FileInfo.vue'
@@ -120,7 +124,7 @@ export default {
   },
 
   computed: {
-    ...mapGetters('Files', ['highlightedFile', 'selectedFiles', 'currentFolder']),
+    ...mapGetters('Files', ['highlightedFile', 'selectedFiles', 'publicLinkPassword']),
     ...mapGetters(['fileSideBars', 'capabilities']),
     ...mapState('Files/sidebar', { sidebarActivePanel: 'activePanel' }),
     activeAvailablePanelName() {
@@ -189,7 +193,13 @@ export default {
       return this.selectedFiles && this.selectedFiles.length > 1
     },
     isRootFolder() {
-      return !this.highlightedFile?.path
+      const pathSegments = this.highlightedFile?.path?.split('/').filter(Boolean) || []
+      if (isLocationPublicActive(this.$router, 'files-public-files')) {
+        // root node of a public link has the public link token as path
+        // root path `/` like for personal home doesn't exist for public links
+        return pathSegments.length === 1
+      }
+      return !pathSegments.length
     },
     highlightedFileThumbnail() {
       return this.highlightedFile?.thumbnail
@@ -304,12 +314,22 @@ export default {
         this.selectedFile = this.highlightedFile
         return
       }
+
       this.loading = true
       try {
-        const item = await this.$client.files.fileInfo(
-          this.highlightedFile.webDavPath,
-          DavProperties.Default
-        )
+        let item
+        if (isLocationPublicActive(this.$router, 'files-public-files')) {
+          item = await this.$client.publicFiles.getFileInfo(
+            this.highlightedFile.webDavPath,
+            this.publicLinkPassword,
+            DavProperties.PublicLink
+          )
+        } else {
+          item = await this.$client.files.fileInfo(
+            this.highlightedFile.webDavPath,
+            DavProperties.Default
+          )
+        }
 
         this.selectedFile = buildResource(item)
         this.$set(this.selectedFile, 'thumbnail', this.highlightedFile.thumbnail || null)

diff --git a/packages/web-app-files/tests/unit/components/SideBar/SideBar.spec.js b/packages/web-app-files/tests/unit/components/SideBar/SideBar.spec.js
@@ -12,6 +12,13 @@ import SideBar from '@files/src/components/SideBar/SideBar.vue'
 import { createLocationSpaces } from '../../../../src/router'
 
 jest.mock('web-pkg/src/observer')
+jest.mock('@files/src/helpers/resources', () => {
+  const original = jest.requireActual('@files/src/helpers/resources')
+  return {
+    ...original,
+    buildResource: jest.fn()
+  }
+})
 
 const simpleOwnFolder = {
   type: 'folder',
@@ -21,63 +28,151 @@ const simpleOwnFolder = {
   size: '740'
 }
 
+const selectors = {
+  noSelectionInfoPanel: 'noselection-stub'
+}
+
 describe('SideBar', () => {
-  it('fetches file info if 1 item is selected', () => {
-    const mockFileInfo = jest.fn()
-    mockFileInfo.mockReturnValueOnce(Files['/'][1])
-
-    createWrapper({
-      item: simpleOwnFolder,
-      selectedItems: [simpleOwnFolder],
-      mocks: { $client: { files: { fileInfo: mockFileInfo } } }
+  describe('file info', () => {
+    beforeEach(() => {
+      buildResource.mockImplementation((item) => item)
     })
+    afterEach(() => {
+      jest.clearAllMocks()
+    })
+    it('fetches file info if 1 item is selected', () => {
+      const mockFileInfo = jest.fn()
+      mockFileInfo.mockReturnValueOnce(Files['/'][1])
 
-    expect(mockFileInfo).toHaveBeenCalledTimes(1)
-  })
-
-  it('fetches file info if the selected item changes', async () => {
-    const spyOnFetchFileInfo = jest
-      .spyOn(SideBar.methods, 'fetchFileInfo')
-      .mockImplementation(jest.fn)
+      createWrapper({
+        item: simpleOwnFolder,
+        selectedItems: [simpleOwnFolder],
+        mocks: { $client: { files: { fileInfo: mockFileInfo } } }
+      })
 
-    const wrapper = createWrapper({
-      item: simpleOwnFolder,
-      selectedItems: [simpleOwnFolder]
+      expect(mockFileInfo).toHaveBeenCalledTimes(1)
     })
 
-    // fetchFileInfo is called once in created()
-    expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(1)
+    it('fetches file info if the selected item changes', async () => {
+      const spyOnFetchFileInfo = jest
+        .spyOn(SideBar.methods, 'fetchFileInfo')
+        .mockImplementation(jest.fn)
+
+      const wrapper = createWrapper({
+        item: simpleOwnFolder,
+        selectedItems: [simpleOwnFolder]
+      })
+
+      // fetchFileInfo is called once in created()
+      expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(1)
+
+      // it should be called again when a different file is loaded
+      const resource = Files['/'][4]
+      resource.path = `${resource.name}`
+      wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', resource)
+      await wrapper.vm.$nextTick()
+      expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(2)
+
+      // and again if the file is renamed
+      const renamedResource = renameResource(Object.assign({}, resource), 'foobar.png', '')
+      wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', Object.assign(renamedResource))
+      await wrapper.vm.$nextTick()
+      expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(3)
+    })
 
-    // it should be called again when a different file is loaded
-    const resource = buildResource(Files['/'][4])
-    resource.path = `${resource.name}`
-    wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', resource)
-    await wrapper.vm.$nextTick()
-    expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(2)
+    it('does not fetch file info if multiple items are selected', () => {
+      const mockFileInfo = jest.fn().mockReturnValue({})
 
-    // and again if the file is renamed
-    const renamedResource = renameResource(Object.assign({}, resource), 'foobar.png', '')
-    wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', Object.assign(renamedResource))
-    await wrapper.vm.$nextTick()
-    expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(3)
+      createWrapper({
+        item: simpleOwnFolder,
+        selectedItems: [simpleOwnFolder, simpleOwnFolder],
+        mocks: { $client: { files: { fileInfo: mockFileInfo } } }
+      })
 
-    jest.resetAllMocks()
+      expect(mockFileInfo).not.toHaveBeenCalled()
+    })
   })
 
-  it('does not fetch file info if multiple items are selected', () => {
-    const mockFileInfo = jest.fn()
-
-    createWrapper({
-      item: simpleOwnFolder,
-      selectedItems: [simpleOwnFolder, simpleOwnFolder],
-      mocks: { $client: { files: { fileInfo: mockFileInfo } } }
+  describe('no selection info panel', () => {
+    afterEach(() => {
+      jest.clearAllMocks()
+    })
+    describe('for public links', () => {
+      it.each([
+        [
+          'shows in root node',
+          {
+            path: '/publicLinkToken',
+            noSelectionExpected: true
+          }
+        ],
+        [
+          'does not show in non-root node',
+          {
+            path: '/publicLinkToken/some-folder',
+            noSelectionExpected: false
+          }
+        ]
+      ])('%s', async (name, { path, noSelectionExpected }) => {
+        const item = { path }
+        const mockFileInfo = jest.fn()
+        mockFileInfo.mockReturnValue(item)
+        buildResource.mockReturnValue(item)
+        const wrapper = createWrapper({
+          item,
+          selectedItems: [],
+          mocks: {
+            $client: { publicFiles: { getFileInfo: mockFileInfo } }
+          },
+          currentRouteName: 'files-public-files'
+        })
+        await wrapper.vm.$nextTick()
+        await wrapper.vm.$nextTick()
+        expect(wrapper.find(selectors.noSelectionInfoPanel).exists()).toBe(noSelectionExpected)
+      })
+    })
+    describe('for all files', () => {
+      it.each([
+        [
+          'shows in root node',
+          {
+            path: '/',
+            noSelectionExpected: true
+          }
+        ],
+        [
+          'does not show in non-root node',
+          {
+            path: '/some-folder',
+            noSelectionExpected: false
+          }
+        ]
+      ])('%s', async (name, { path, noSelectionExpected }) => {
+        const item = { path }
+        const mockFileInfo = jest.fn()
+        mockFileInfo.mockReturnValue(item)
+        buildResource.mockReturnValue(item)
+        const wrapper = createWrapper({
+          item,
+          selectedItems: [],
+          mocks: {
+            $client: { files: { fileInfo: mockFileInfo } }
+          }
+        })
+        await wrapper.vm.$nextTick()
+        await wrapper.vm.$nextTick()
+        expect(wrapper.find(selectors.noSelectionInfoPanel).exists()).toBe(noSelectionExpected)
+      })
     })
-
-    expect(mockFileInfo).not.toHaveBeenCalled()
   })
 })
 
-function createWrapper({ item, selectedItems, mocks }) {
+function createWrapper({
+  item,
+  selectedItems,
+  mocks,
+  currentRouteName = 'files-spaces-personal-home'
+}) {
   const localVue = createLocalVue()
   localVue.use(Vuex)
   localVue.use(VueCompositionAPI)
@@ -111,7 +206,8 @@ function createWrapper({ item, selectedItems, mocks }) {
           },
           getters: {
             highlightedFile: (state) => state.highlightedFile,
-            selectedFiles: () => selectedItems
+            selectedFiles: () => selectedItems,
+            publicLinkPassword: () => ''
           },
           mutations: {
             SET_HIGHLIGHTED_FILE(state, file) {
@@ -137,7 +233,7 @@ function createWrapper({ item, selectedItems, mocks }) {
     mocks: merge(
       {
         $router: {
-          currentRoute: createLocationSpaces('files-spaces-personal-home'),
+          currentRoute: createLocationSpaces(currentRouteName),
           resolve: (r) => {
             return { href: r.name }
           }

diff --git a/packages/web-app-files/tests/unit/views/__snapshots__/SharedWithOthers.spec.js.snap b/packages/web-app-files/tests/unit/views/__snapshots__/SharedWithOthers.spec.js.snap
@@ -8,10 +8,10 @@ exports[`SharedWithOthers when the wrapper is not loading anymore when length of
         <!---->
       </th>
       <th class="oc-th oc-table-cell oc-table-cell-align-left oc-table-cell-align-middle oc-table-cell-width-expand oc-text-nowrap oc-table-header-cell oc-table-header-cell-name" aria-sort="none" style="top: 0px;"><span class="oc-table-thead-content header-text">Name</span>
-        <oc-button-stub type="button" size="medium" arialabel="Sort by name" variation="passive" appearance="raw" justifycontent="center" gapsize="medium" class="oc-button-sort"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></oc-button-stub>
+        <oc-button-stub type="button" size="medium" arialabel="Sort by name" variation="passive" appearance="raw" justifycontent="center" gapsize="medium" class="oc-button-sort oc-invisible"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></oc-button-stub>
       </th>
       <th class="oc-th oc-table-cell oc-table-cell-align-right oc-table-cell-align-middle oc-table-cell-width-auto oc-text-nowrap oc-table-header-cell oc-table-header-cell-sharedWith" aria-sort="none" style="top: 0px;"><span class="oc-table-thead-content header-text">Shared with</span>
-        <oc-button-stub type="button" size="medium" arialabel="Sort by sharedWith" variation="passive" appearance="raw" justifycontent="center" gapsize="medium" class="oc-button-sort"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></oc-button-stub>
+        <oc-button-stub type="button" size="medium" arialabel="Sort by sharedWith" variation="passive" appearance="raw" justifycontent="center" gapsize="medium" class="oc-button-sort oc-invisible"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></oc-button-stub>
       </th>
       <th class="oc-th oc-table-cell oc-table-cell-align-right oc-table-cell-align-middle oc-table-cell-width-auto oc-text-nowrap oc-table-header-cell oc-table-header-cell-actions oc-pr-m" style="top: 0px;"><span class="oc-table-thead-content header-text">Actions</span>
         <!---->

diff --git a/packages/web-app-files/tests/unit/views/__snapshots__/Trashbin.spec.js.snap b/packages/web-app-files/tests/unit/views/__snapshots__/Trashbin.spec.js.snap
@@ -7,7 +7,7 @@ exports[`Trashbin View when the view is not loading anymore when length of the p
       <th class="oc-th oc-table-cell oc-table-cell-align-left oc-table-cell-align-middle oc-table-cell-width-shrink oc-text-nowrap oc-table-header-cell oc-table-header-cell-select oc-pl-m " style="top: 0px;"><span class="oc-table-thead-content"><div class="resource-table-select-all"><span><input id="resource-table-select-all" type="checkbox" name="checkbox" class="oc-checkbox oc-rounded oc-checkbox-l" value=""> <label for="resource-table-select-all" class="oc-invisible-sr oc-cursor-pointer">Select all resources</label></span></div></span>
         <!---->
       </th>
-      <th class="oc-th oc-table-cell oc-table-cell-align-left oc-table-cell-align-middle oc-table-cell-width-expand oc-text-nowrap oc-table-header-cell oc-table-header-cell-name" aria-sort="none" style="top: 0px;"><span class="oc-table-thead-content header-text">Name</span> <button aria-label="Sort by name" class="oc-button-sort oc-button oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-raw"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></button></th>
+      <th class="oc-th oc-table-cell oc-table-cell-align-left oc-table-cell-align-middle oc-table-cell-width-expand oc-text-nowrap oc-table-header-cell oc-table-header-cell-name" aria-sort="none" style="top: 0px;"><span class="oc-table-thead-content header-text">Name</span> <button aria-label="Sort by name" class="oc-button-sort oc-button oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-raw oc-invisible"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></button></th>
       <th class="oc-th oc-table-cell oc-table-cell-align-right oc-table-cell-align-middle oc-table-cell-width-auto oc-text-nowrap oc-table-header-cell oc-table-header-cell-status" style="top: 0px;"><span class="oc-table-thead-content header-text">Status</span>
         <!---->
       </th>

diff --git a/packages/web-runtime/package.json b/packages/web-runtime/package.json
@@ -6,7 +6,7 @@
   "dependencies": {
     "@fortawesome/fontawesome-free": "^5.15.4",
     "@popperjs/core": "^2.4.0",
-    "@vue/composition-api": "^1.4.6",
+    "@vue/composition-api": "^1.4.9",
     "axios": "^0.25.0",
     "cross-fetch": "^3.0.6",
     "easygettext": "^2.16.1",
@@ -17,7 +17,7 @@
     "lodash-es": "^4.17.21",
     "luxon": "^2.3.0",
     "oidc-client": "1.11.5",
-    "owncloud-design-system": "12.2.0",
+    "owncloud-design-system": "^12.2.1",
     "owncloud-sdk": "~2.0.0",
     "p-queue": "^6.1.1",
     "popper-max-size-modifier": "^0.2.0",