Centralize version loading (#10746)

* Centralize version loading
owncloud · Apr 9, 2024 · 70d7033 · 70d7033
1 parent 9c0b07e
commit 70d7033
Show file tree

Hide file tree

Showing 8 changed files with 148 additions and 181 deletions.
diff --git a/changelog/unreleased/bugfix-versions-loaded-multiple-times-when-opening-sidebar b/changelog/unreleased/bugfix-versions-loaded-multiple-times-when-opening-sidebar
@@ -0,0 +1,8 @@
+Bugfix: Versions loaded multiple times when opening sidebar
+
+We've fixed a bug, where the versions endpoint was fetched multiple times when the sidebar was opened
+and therefore added unnecessary load to the server.
+
+https://github.com/owncloud/web/pull/10746
+https://github.com/owncloud/web/issues/10381
+https://github.com/owncloud/web/issues/10619
diff --git a/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue b/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue
@@ -136,7 +136,6 @@ import {
   useUserStore,
   useCapabilityStore,
   useConfigStore,
-  useClientService,
   useResourcesStore,
   formatDateFromJSDate
 } from '@ownclouders/web-pkg'
@@ -174,18 +173,12 @@ export default defineComponent({
       type: Boolean,
       required: false,
       default: true
-    },
-    versionsEnabled: {
-      type: Boolean,
-      required: false,
-      default: true
     }
   },
   setup(props) {
     const configStore = useConfigStore()
     const userStore = useUserStore()
     const capabilityStore = useCapabilityStore()
-    const clientService = useClientService()
     const { getMatchingSpace } = useGetMatchingSpace()
 
     const language = useGettext()
@@ -196,6 +189,7 @@ export default defineComponent({
     const { user } = storeToRefs(userStore)
 
     const resource = inject<Ref<Resource>>('resource')
+    const versions = inject<Ref<Resource[]>>('versions')
     const space = inject<Ref<SpaceResource>>('space')
 
     const previewService = usePreviewService()
@@ -204,15 +198,6 @@ export default defineComponent({
     const authStore = useAuthStore()
     const { publicLinkContextReady } = storeToRefs(authStore)
 
-    const versions = ref<Resource[]>([])
-    const loadVersions = async (fileId: Resource['fileId']) => {
-      try {
-        versions.value = await clientService.webdav.listFileVersions(fileId)
-      } catch (e) {
-        console.error(e)
-      }
-    }
-
     const isPreviewEnabled = computed(() => {
       if (unref(resource).isFolder) {
         return false
@@ -261,23 +246,6 @@ export default defineComponent({
       return formatRelativeDateFromJSDate(new Date(date), language.current)
     }
 
-    watch(
-      resource,
-      () => {
-        if (unref(resource)) {
-          loadPreviewTask.perform(unref(resource))
-          if (
-            props.versionsEnabled &&
-            !unref(resource).isFolder &&
-            !unref(publicLinkContextReady)
-          ) {
-            loadVersions(unref(resource).fileId)
-          }
-        }
-      },
-      { immediate: true }
-    )
-
     const contextualHelper = {
       isEnabled: configStore.options.contextHelpers,
       data: tagsHelper({ configStore })
@@ -296,6 +264,16 @@ export default defineComponent({
       return upperFirst(displayDate)
     })
 
+    watch(
+      resource,
+      () => {
+        if (unref(resource)) {
+          loadPreviewTask.perform(unref(resource))
+        }
+      },
+      { immediate: true }
+    )
+
     return {
       user,
       preview,

diff --git a/packages/web-app-files/src/components/SideBar/Versions/FileVersions.vue b/packages/web-app-files/src/components/SideBar/Versions/FileVersions.vue
@@ -1,7 +1,6 @@
 <template>
   <div id="oc-file-versions-sidebar" class="-oc-mt-s">
-    <oc-loader v-if="areVersionsLoading" />
-    <ul v-else-if="versions.length" class="oc-m-rm oc-position-relative">
+    <ul v-if="versions.length" class="oc-m-rm oc-position-relative">
       <li class="spacer oc-pb-l" aria-hidden="true"></li>
       <li
         v-for="(item, index) in versions"
@@ -63,9 +62,8 @@ import {
   useDownloadFile,
   useResourcesStore
 } from '@ownclouders/web-pkg'
-import { computed, defineComponent, inject, Ref, ref, unref, watch } from 'vue'
+import { computed, defineComponent, inject, Ref, unref } from 'vue'
 import { isShareSpaceResource, Resource, SpaceResource } from '@ownclouders/web-client/src/helpers'
-import { useTask } from 'vue-concurrency'
 import { useGettext } from 'vue3-gettext'
 
 export default defineComponent({
@@ -85,30 +83,7 @@ export default defineComponent({
 
     const space = inject<Ref<SpaceResource>>('space')
     const resource = inject<Ref<Resource>>('resource')
-
-    const versions = ref<Resource[]>([])
-    const fetchVersionsTask = useTask(function* () {
-      try {
-        versions.value = yield clientService.webdav.listFileVersions(unref(resource).fileId)
-      } catch (e) {
-        console.error(e)
-      }
-    })
-    const areVersionsLoading = computed(() => {
-      return !fetchVersionsTask.last || fetchVersionsTask.isRunning
-    })
-    watch(
-      [() => unref(resource)?.id, () => unref(resource)?.etag],
-      ([id, etag]) => {
-        if (!id || !etag) {
-          return
-        }
-        fetchVersionsTask.perform()
-      },
-      {
-        immediate: true
-      }
-    )
+    const versions = inject<Ref<Resource[]>>('versions')
 
     const isRevertible = computed(() => {
       if (props.isReadOnly) {
@@ -138,8 +113,6 @@ export default defineComponent({
           })
         }
       }
-
-      fetchVersionsTask.perform()
     }
     const downloadVersion = (version: Resource) => {
       return downloadFile(unref(space), unref(resource), version.name)
@@ -158,16 +131,12 @@ export default defineComponent({
       space,
       resource,
       versions,
-      areVersionsLoading,
       isRevertible,
       revertToVersion,
       downloadVersion,
       formatVersionDateRelative,
       formatVersionDate,
-      formatVersionFileSize,
-
-      // HACK: exported for unit tests
-      fetchVersionsTask
+      formatVersionFileSize
     }
   }
 })

diff --git a/packages/web-app-files/src/composables/extensions/useFileSideBars.ts b/packages/web-app-files/src/composables/extensions/useFileSideBars.ts
@@ -22,9 +22,11 @@ import {
   useCapabilityStore
 } from '@ownclouders/web-pkg'
 import {
+  isIncomingShareResource,
   isProjectSpaceResource,
   isShareResource,
   isShareSpaceResource,
+  isSpaceResource,
   SpaceResource
 } from '@ownclouders/web-client/src/helpers'
 import { Resource } from '@ownclouders/web-client'
@@ -227,7 +229,9 @@ export const useSideBarPanels = () => {
               }
               if (
                 isLocationTrashActive(router, 'files-trash-generic') ||
-                isLocationPublicActive(router, 'files-public-link')
+                isLocationPublicActive(router, 'files-public-link') ||
+                isIncomingShareResource(items[0]) ||
+                isSpaceResource(items[0])
               ) {
                 return false
               }

diff --git a/packages/web-app-files/tests/unit/components/FilesList/ResourceDetails.spec.ts b/packages/web-app-files/tests/unit/components/FilesList/ResourceDetails.spec.ts
@@ -46,7 +46,6 @@ describe('ResourceDetails component', () => {
         })
       })
     }
-    mocks.$clientService.webdav.listFileVersions.mockResolvedValue([])
 
     const file = {
       id: '0',
@@ -76,7 +75,8 @@ describe('ResourceDetails component', () => {
           provide: {
             ...mocks,
             space,
-            resource: file
+            resource: file,
+            versions: []
           }
         }
       })

diff --git a/packages/web-app-files/tests/unit/components/SideBar/Details/FileDetails.spec.ts b/packages/web-app-files/tests/unit/components/SideBar/Details/FileDetails.spec.ts
@@ -207,14 +207,14 @@ function createWrapper({
   const publicLocation = createLocationPublic('files-public-link')
   const currentRoute = isPublicLinkContext ? publicLocation : spacesLocation
   const mocks = defaultComponentMocks({ currentRoute: mock<RouteLocation>(currentRoute as any) })
-  mocks.$clientService.webdav.listFileVersions.mockResolvedValue(versions)
   const capabilities = { files: { tags: tagsEnabled } }
   return {
     wrapper: mount(FileDetails, {
       global: {
         stubs: { 'router-link': true, 'resource-icon': true },
         provide: {
           ...mocks,
+          versions,
           resource,
           space: mockDeep<SpaceResource>()
         },