[full-ci] Optimize share loading part II (#7638)

* Remove loadCurrentFileOutgoingShares and loadIncomingShares mutations * Refactor space share and member loading * Small adjustments * Fix reactive share indicator loading in sidebar details panel * Fix unit tests * Small adjustments * Fix permission display * Simplify code in sharesTreeLoading * Decouple space members from shares * Decouple space members from shares * Fix loading of indirect shares within spaces * Fix indirect share loading on shared with others/via link pages * Fix unit tests * Introduce includeRoot property to sharesTree loading * Remove unneeded loading states * Adjust changelog item * Remove space member loading state * Fix share loading on favorites page * Remove unneeded states and mutations * Simplify useGraphClient usage * Adjust space mutation names
owncloud · Sep 19, 2022 · 8914c3f · 8914c3f
1 parent 5104117
commit 8914c3f
Show file tree

Hide file tree

Showing 28 changed files with 347 additions and 597 deletions.
diff --git a/changelog/unreleased/bugfix-shares-loading b/changelog/unreleased/bugfix-shares-loading
@@ -0,0 +1,18 @@
+Bugfix: Shares loading
+
+We've improved the loading of shares:
+
+* Share loading now happens more globally in the sidebar component instead of in each sidebar panel.
+* Shares won't be loaded for resources without a path anymore, which massively increases performance.
+* Several issues with (re-)share permissions have been fixed.
+* `loadCurrentFileOutgoingShares` and `loadIncomingShares` mutations have been removed. Instead, incoming and outgoing shares are now being loaded via `loadSharesTree()`. This avoids `getShare()` requests from being executed multiple times.
+* Space member loading has been decoupled from shares loading in store. This reduces fetching of space members to a minimum and improves the structure of the code.
+* Reactive loading of share indicators in sidebar details panel has been fixed.
+* Reactive loading of space member count in the spaces overview has been fixed.
+* Loading of indirect shares within spaces has been fixed.
+
+https://github.com/owncloud/web/issues/7506
+https://github.com/owncloud/web/issues/7593
+https://github.com/owncloud/web/issues/7592
+https://github.com/owncloud/web/pull/7580
+https://github.com/owncloud/web/pull/7638
diff --git a/changelog/unreleased/bugfix-shares-tree-loading b/changelog/unreleased/bugfix-shares-tree-loading
diff --git a/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue b/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue
@@ -209,7 +209,7 @@ export default defineComponent({
     timeout: null
   }),
   computed: {
-    ...mapGetters('Files', ['versions', 'sharesTree', 'sharesTreeLoading']),
+    ...mapGetters('Files', ['versions', 'sharesTree', 'sharesTreeLoading', 'highlightedFile']),
     ...mapGetters(['user', 'configuration']),
 
     file() {
@@ -357,7 +357,7 @@ export default defineComponent({
       handler() {
         // missing early return
         this.sharedItem = null
-        this.shareIndicators = getIndicators(this.file, this.sharesTree)
+        this.shareIndicators = getIndicators(this.highlightedFile, this.sharesTree)
         const sharePathParentOrCurrent = this.getParentSharePath(this.file.path, this.sharesTree)
         if (sharePathParentOrCurrent === null) {
           return

diff --git a/packages/web-app-files/src/components/SideBar/Details/SpaceDetails.vue b/packages/web-app-files/src/components/SideBar/Details/SpaceDetails.vue
@@ -121,11 +121,8 @@ export default defineComponent({
     return { loadImageTask, spaceImage }
   },
   computed: {
-    ...mapGetters('Files', [
-      'highlightedFile',
-      'currentFileOutgoingCollaborators',
-      'currentFileOutgoingLinks'
-    ]),
+    ...mapGetters('Files', ['currentFileOutgoingLinks']),
+    ...mapGetters('runtime/spaces', ['spaceMembers']),
     ...mapGetters(['user']),
     space() {
       return this.displayedItem.value
@@ -188,7 +185,7 @@ export default defineComponent({
     },
     ownerUsernames() {
       const userId = this.user?.id
-      return this.currentFileOutgoingCollaborators
+      return this.spaceMembers
         .filter((share) => share.role.name === spaceRoleManager.name)
         .map((share) => {
           if (share.collaborator.name === userId) {
@@ -207,7 +204,7 @@ export default defineComponent({
       return this.linkShareCount > 0
     },
     memberShareCount() {
-      return this.currentFileOutgoingCollaborators.length
+      return this.spaceMembers.length
     },
     linkShareCount() {
       return this.currentFileOutgoingLinks.length

diff --git a/...src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue b/...src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue
@@ -88,7 +88,6 @@ import {
 } from 'web-pkg/src/composables'
 
 import { defineComponent } from '@vue/runtime-core'
-import { useGraphClient } from 'web-client/src/composables'
 
 // just a dummy function to trick gettext tools
 const $gettext = (str) => {
@@ -118,7 +117,6 @@ export default defineComponent({
 
   setup() {
     return {
-      ...useGraphClient(),
       hasResharing: useCapabilityFilesSharingResharing(),
       hasShareJail: useCapabilityShareJailEnabled()
     }
@@ -139,6 +137,7 @@ export default defineComponent({
   },
   computed: {
     ...mapGetters('Files', ['currentFileOutgoingCollaborators', 'highlightedFile']),
+    ...mapGetters('runtime/spaces', ['spaceMembers']),
     ...mapGetters(['configuration', 'user', 'capabilities']),
 
     $_announcementWhenCollaboratorAdded() {
@@ -170,6 +169,7 @@ export default defineComponent({
 
   methods: {
     ...mapActions('Files', ['addShare']),
+    ...mapActions('runtime/spaces', ['addSpaceMember']),
 
     async fetchRecipients(query) {
       try {
@@ -211,7 +211,10 @@ export default defineComponent({
             )
           })
 
-          const exists = this.currentFileOutgoingCollaborators.find((existingCollaborator) => {
+          const existingShares = this.resourceIsSpace
+            ? this.spaceMembers
+            : this.currentFileOutgoingCollaborators
+          const exists = existingShares.find((existingCollaborator) => {
             return (
               collaborator.value.shareWith === existingCollaborator.collaborator.name &&
               parseInt(collaborator.value.shareType, 10) === existingCollaborator.shareType
@@ -296,9 +299,9 @@ export default defineComponent({
               path = `/${this.highlightedFile.name}`
             }
 
-            this.addShare({
+            const addMethod = this.resourceIsSpace ? this.addSpaceMember : this.addShare
+            addMethod({
               client: this.$client,
-              graphClient: this.graphClient,
               path,
               $gettext: this.$gettext,
               shareWith: collaborator.value.shareWith,

diff --git a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/ListItem.vue b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/ListItem.vue
@@ -111,7 +111,6 @@ import { SharePermissions, ShareTypes } from 'web-client/src/helpers/share'
 import { useCapabilityFilesSharingResharing } from 'web-pkg/src/composables'
 import { extractDomSelector } from 'web-client/src/helpers/resource'
 import { defineComponent } from '@vue/composition-api'
-import { useGraphClient } from 'web-client/src/composables'
 import * as uuid from 'uuid'
 import { formatDateFromDateTime, formatRelativeDateFromDateTime } from 'web-pkg/src/helpers'
 
@@ -137,7 +136,6 @@ export default defineComponent({
   },
   setup() {
     return {
-      ...useGraphClient(),
       hasResharing: useCapabilityFilesSharingResharing()
     }
   },
@@ -321,6 +319,7 @@ export default defineComponent({
   methods: {
     ...mapActions(['showMessage']),
     ...mapActions('Files', ['changeShare']),
+    ...mapActions('runtime/spaces', ['changeSpaceMember']),
 
     removeShare() {
       this.$emit('onDelete', this.share)
@@ -360,12 +359,10 @@ export default defineComponent({
     saveShareChanges({ role, permissions, expirationDate }) {
       const bitmask = role.hasCustomPermissions
         ? SharePermissions.permissionsToBitmask(permissions)
-        : SharePermissions.permissionsToBitmask(
-            role.permissions(this.hasResharing || this.shareType === ShareTypes.space)
-          )
-      this.changeShare({
+        : SharePermissions.permissionsToBitmask(role.permissions(this.hasResharing || this.isSpace))
+      const changeMethod = this.isSpace ? this.changeSpaceMember : this.changeShare
+      changeMethod({
         client: this.$client,
-        graphClient: this.graphClient,
         share: this.share,
         permissions: bitmask,
         expirationDate: expirationDate || '',

diff --git a/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue b/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue
@@ -65,7 +65,6 @@
 </template>
 
 <script lang="ts">
-import { useTask } from 'vue-concurrency'
 import { mapGetters, mapActions, mapState } from 'vuex'
 import { watch, computed, ref, unref } from '@vue/composition-api'
 import {
@@ -78,12 +77,9 @@ import {
 import { createLocationSpaces, isLocationSpacesActive } from '../../../router'
 import { textUtils } from '../../../helpers/textUtils'
 import { getParentPaths } from '../../../helpers/path'
-import { buildSpaceShare } from '../../../helpers/resources'
 import { ShareTypes } from 'web-client/src/helpers/share'
-import { sortSpaceMembers } from '../../../helpers/space'
 import InviteCollaboratorForm from './Collaborators/InviteCollaborator/InviteCollaboratorForm.vue'
 import CollaboratorListItem from './Collaborators/ListItem.vue'
-import { useGraphClient } from 'web-client/src/composables'
 import {
   shareInviteCollaboratorHelp,
   shareInviteCollaboratorHelpCern
@@ -98,7 +94,6 @@ export default {
   setup() {
     const store = useStore()
     const currentSpace = ref(null)
-    const spaceMembers = ref([])
     const currentStorageId = useRouteParam('storageId')
     watch(
       currentStorageId,
@@ -110,40 +105,12 @@ export default {
       { immediate: true }
     )
     const isCurrentSpaceTypeProject = computed(() => unref(currentSpace)?.driveType === 'project')
-
-    const { graphClient } = useGraphClient({ store })
-
-    const loadSpaceMembersTask = useTask(function* (signal, ref) {
-      const promises = []
-      const spaceShares = []
-
-      for (const role of Object.keys(unref(currentSpace).spaceRoles)) {
-        for (const userId of unref(currentSpace).spaceRoles[role]) {
-          promises.push(
-            unref(graphClient)
-              .users.getUser(userId)
-              .then((resolved) => {
-                spaceShares.push(
-                  buildSpaceShare({ ...resolved.data, role }, unref(currentSpace).id)
-                )
-              })
-          )
-        }
-      }
-
-      yield Promise.all(promises).then(() => {
-        spaceMembers.value = sortSpaceMembers(spaceShares)
-      })
-    })
-
     const sharesListCollapsed = !store.getters.configuration.options.sidebar.shares.showAllOnLoad
 
     return {
       currentStorageId,
       currentSpace,
-      spaceMembers,
       isCurrentSpaceTypeProject,
-      loadSpaceMembersTask,
       sharesListCollapsed,
       hasProjectSpaces: useCapabilityProjectSpacesEnabled(),
       hasShareJail: useCapabilityShareJailEnabled(),
@@ -153,6 +120,7 @@ export default {
   computed: {
     ...mapGetters('Files', ['highlightedFile', 'currentFileOutgoingCollaborators']),
     ...mapGetters(['configuration']),
+    ...mapGetters('runtime/spaces', ['spaceMembers']),
     ...mapState('Files', ['incomingShares', 'sharesTree']),
     ...mapState(['user']),
 
@@ -233,8 +201,7 @@ export default {
         if (shares) {
           shares.forEach((share) => {
             if (share.outgoing && this.$_isCollaboratorShare(share)) {
-              share.key = 'indirect-collaborator-' + share.id
-              allShares.push(share)
+              allShares.push({ ...share, key: 'indirect-collaborator-' + share.id })
             }
           })
         }
@@ -271,11 +238,6 @@ export default {
       )
     }
   },
-  mounted() {
-    if (this.showSpaceMembers) {
-      this.loadSpaceMembersTask.perform()
-    }
-  },
   methods: {
     ...mapActions('Files', ['deleteShare']),
     ...mapActions(['createModal', 'hideModal', 'showMessage']),

diff --git a/packages/web-app-files/src/components/SideBar/Shares/SharesPanel.vue b/packages/web-app-files/src/components/SideBar/Shares/SharesPanel.vue
@@ -18,7 +18,6 @@ import { computed, defineComponent, unref } from '@vue/composition-api'
 import FileLinks from './FileLinks.vue'
 import FileShares from './FileShares.vue'
 import SpaceMembers from './SpaceMembers.vue'
-import { mapGetters, mapState } from 'vuex'
 import { useStore } from 'web-pkg/src/composables'
 import { useIncomingParentShare } from '../../../composables/parentShare'
 
@@ -41,27 +40,13 @@ export default defineComponent({
   },
   setup() {
     const store = useStore()
-    const currentFileOutgoingSharesLoading = computed(
-      () => store.getters['Files/currentFileOutgoingSharesLoading']
-    )
-    const incomingSharesLoading = computed(() => store.getters['Files/incomingSharesLoading'])
-    const sharesTreeLoading = computed(() => store.getters['Files/sharesTreeLoading'])
-    const sharesLoading = computed(
-      () =>
-        unref(currentFileOutgoingSharesLoading) ||
-        unref(incomingSharesLoading) ||
-        unref(sharesTreeLoading)
-    )
+    const sharesLoading = computed(() => store.getters['Files/sharesTreeLoading'])
 
     return {
       ...useIncomingParentShare(),
       sharesLoading
     }
   },
-  computed: {
-    ...mapGetters('Files', ['currentFileOutgoingSharesLoading']),
-    ...mapState('Files', ['incomingSharesLoading', 'sharesTreeLoading'])
-  },
   watch: {
     sharesLoading: {
       handler: function (sharesLoading, old) {