[full-ci] Fix sharesTree loading (#7580)

* Fix sharesTree loading * Fix parent share fetching in sidebar * Remove logs * Minor adjustment * Add changelog item * Fix loading of share indicators * Move sharesTree loading to the sidebar component * Simplify code * Fix unit tests * Make share indicators in details panel reactive again * Fix space member loading * Fix sidebar panel opening * Remove unused method * Fix e2e tests * Apply small changes according to code review * Fix e2e tests * Import isEqual directly
owncloud · Sep 12, 2022 · 010f1b7 · 010f1b7
1 parent 15c2841
commit 010f1b7
Show file tree

Hide file tree

Showing 20 changed files with 223 additions and 217 deletions.
diff --git a/changelog/unreleased/bugfix-shares-tree-loading b/changelog/unreleased/bugfix-shares-tree-loading
@@ -0,0 +1,13 @@
+Bugfix: Shares tree loading
+
+We've improved loading of the shares tree:
+
+* It 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.
+
+These changes massively improve the sidebar performance and fix several issues with (re-)share permissions.
+
+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
diff --git a/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue b/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue
@@ -352,39 +352,40 @@ export default defineComponent({
   watch: {
     file() {
       this.loadData()
-      this.refreshShareDetailsTree()
-    },
-    sharesTree() {
-      // missing early return
-      this.sharedItem = null
-      this.shareIndicators = getIndicators(this.file, this.sharesTree)
-      const sharePathParentOrCurrent = this.getParentSharePath(this.file.path, this.sharesTree)
-      if (sharePathParentOrCurrent === null) {
-        return
-      }
-      const shares = this.sharesTree[sharePathParentOrCurrent]?.filter((s) =>
-        ShareTypes.containsAnyValue(
-          [...ShareTypes.individuals, ...ShareTypes.unauthenticated],
-          [s.shareType]
+    },
+    sharesTree: {
+      handler() {
+        // missing early return
+        this.sharedItem = null
+        this.shareIndicators = getIndicators(this.file, this.sharesTree)
+        const sharePathParentOrCurrent = this.getParentSharePath(this.file.path, this.sharesTree)
+        if (sharePathParentOrCurrent === null) {
+          return
+        }
+        const shares = this.sharesTree[sharePathParentOrCurrent]?.filter((s) =>
+          ShareTypes.containsAnyValue(
+            [...ShareTypes.individuals, ...ShareTypes.unauthenticated],
+            [s.shareType]
+          )
         )
-      )
-      if (shares.length === 0) {
-        return
-      }
+        if (shares.length === 0) {
+          return
+        }
 
-      this.sharedItem = shares[0]
-      this.sharedByName = this.sharedItem.owner?.name
-      this.sharedByDisplayName = this.sharedItem.owner?.displayName
-      if (this.sharedItem.owner?.additionalInfo) {
-        this.sharedByDisplayName += ' (' + this.sharedItem.owner.additionalInfo + ')'
-      }
-      this.sharedTime = this.sharedItem.stime
-      this.sharedParentDir = sharePathParentOrCurrent
+        this.sharedItem = shares[0]
+        this.sharedByName = this.sharedItem.owner?.name
+        this.sharedByDisplayName = this.sharedItem.owner?.displayName
+        if (this.sharedItem.owner?.additionalInfo) {
+          this.sharedByDisplayName += ' (' + this.sharedItem.owner.additionalInfo + ')'
+        }
+        this.sharedTime = this.sharedItem.stime
+        this.sharedParentDir = sharePathParentOrCurrent
+      },
+      immediate: true
     }
   },
   mounted() {
     this.loadData()
-    this.refreshShareDetailsTree()
   },
   asyncComputed: {
     preview: {
@@ -405,16 +406,8 @@ export default defineComponent({
     }
   },
   methods: {
-    ...mapActions('Files', ['loadPreview', 'loadVersions', 'loadSharesTree']),
-    async refreshShareDetailsTree() {
-      await this.loadSharesTree({
-        client: this.$client,
-        path: this.file.path,
-        $gettext: this.$gettext,
-        storageId: this.file.fileId
-      })
-      this.shareIndicators = getIndicators(this.file, this.sharesTree)
-    },
+    ...mapActions('Files', ['loadPreview', 'loadVersions']),
+
     getParentSharePath(childPath, shares) {
       let currentPath = childPath
       if (!currentPath) {

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
@@ -217,13 +217,6 @@ export default defineComponent({
       return this.$gettextInterpolate(translated, context)
     },
 
-    shareExpirationText() {
-      const translated = this.$gettext('Expires %{ expiryDateRelative }')
-      return this.$gettextInterpolate(translated, {
-        expiryDateRelative: this.expirationDateRelative
-      })
-    },
-
     screenreaderShareExpiration() {
       const translated = this.$gettext('Share expires %{ expiryDateRelative } (%{ expiryDate })')
       return this.$gettextInterpolate(translated, {
@@ -258,11 +251,6 @@ export default defineComponent({
       return this.sharedParentRoute?.params?.item.split('/').pop()
     },
 
-    shareDetailsHelperContent() {
-      return {
-        text: this.$gettext('Invite persons or groups to access this file or folder.')
-      }
-    },
     editDropDownToggleId() {
       return uuid.v4()
     },

diff --git a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/RoleDropdown.vue b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/RoleDropdown.vue
@@ -122,6 +122,7 @@ import { PropType } from '@vue/composition-api'
 export default defineComponent({
   name: 'RoleDropdown',
   components: { RoleItem },
+  inject: ['incomingParentShare'],
   props: {
     resource: {
       type: Object,
@@ -181,10 +182,6 @@ export default defineComponent({
     resourceIsSharable() {
       return this.allowSharePermission && this.resource.canShare()
     },
-    share() {
-      // the root share has an empty key in the shares tree. That's the reason why we retrieve the share by an empty key here
-      return this.sharesTree['']?.find((s) => s.incoming)
-    },
     allowCustomSharing() {
       return this.capabilities?.files_sharing?.allow_custom
     },
@@ -193,9 +190,9 @@ export default defineComponent({
         return SpacePeopleShareRoles.list()
       }
 
-      if (this.share?.incoming && this.resourceIsSharable) {
+      if (this.incomingParentShare.value && this.resourceIsSharable) {
         return PeopleShareRoles.filterByBitmask(
-          parseInt(this.share.permissions),
+          parseInt(this.incomingParentShare.value.permissions),
           this.resource.isFolder,
           this.allowSharePermission,
           this.allowCustomSharing !== false
@@ -205,8 +202,10 @@ export default defineComponent({
       return PeopleShareRoles.list(this.resource.isFolder, this.allowCustomSharing !== false)
     },
     availablePermissions() {
-      if (this.share?.incoming && this.resourceIsSharable) {
-        return SharePermissions.bitmaskToPermissions(parseInt(this.share.permissions))
+      if (this.incomingParentShare.value && this.resourceIsSharable) {
+        return SharePermissions.bitmaskToPermissions(
+          parseInt(this.incomingParentShare.value.permissions)
+        )
       }
       return this.customPermissionsRole.permissions(this.allowSharePermission)
     },

diff --git a/packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue b/packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue
@@ -137,6 +137,7 @@ export default defineComponent({
     DetailsAndEdit,
     NameAndCopy
   },
+  inject: ['incomingParentShare'],
   setup() {
     const store = useStore()
 
@@ -182,11 +183,6 @@ export default defineComponent({
       return this.currentFileOutgoingLinks.find((link) => link.quicklink === true)
     },
 
-    share() {
-      // the root share has an empty key in the shares tree. That's the reason why we retrieve the share by an empty key here
-      return this.sharesTree['']?.find((s) => s.incoming)
-    },
-
     expirationDate() {
       const expireDate = this.capabilities.files_sharing.public.expire_date
 
@@ -218,9 +214,9 @@ export default defineComponent({
     },
 
     availableRoleOptions() {
-      if (this.share?.incoming && this.canCreatePublicLinks) {
+      if (this.incomingParentShare.value && this.canCreatePublicLinks) {
         return LinkShareRoles.filterByBitmask(
-          parseInt(this.share.permissions),
+          parseInt(this.incomingParentShare.value.permissions),
           this.highlightedFile.isFolder,
           this.hasPublicLinkEditing,
           this.hasPublicLinkAliasSupport
@@ -323,9 +319,6 @@ export default defineComponent({
         return []
       }
 
-      // remove root entry
-      parentPaths.pop()
-
       parentPaths.forEach((parentPath) => {
         const shares = cloneStateObject(this.sharesTree[parentPath])
         if (shares) {

diff --git a/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue b/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue
@@ -188,38 +188,6 @@ export default {
       return this.collaborators.length > 0
     },
 
-    /**
-     * Returns all incoming shares, direct and indirect
-     *
-     * @return {Array.<Object>} list of incoming shares
-     */
-    $_allIncomingShares() {
-      // direct incoming shares
-      const allShares = [...this.incomingShares]
-
-      // indirect incoming shares
-      const parentPaths = getParentPaths(this.highlightedFile.path, true)
-      if (parentPaths.length === 0) {
-        return []
-      }
-
-      // remove root entry
-      parentPaths.pop()
-
-      parentPaths.forEach((parentPath) => {
-        const shares = this.sharesTree[parentPath]
-        if (shares) {
-          shares.forEach((share) => {
-            if (share.incoming) {
-              allShares.push(share)
-            }
-          })
-        }
-      })
-
-      return allShares
-    },
-
     collaborators() {
       return [...this.currentFileOutgoingCollaborators, ...this.indirectOutgoingShares]
         .sort(this.collaboratorsComparator)
@@ -260,9 +228,6 @@ export default {
         return []
       }
 
-      // remove root entry
-      parentPaths.pop()
-
       parentPaths.forEach((parentPath) => {
         const shares = this.sharesTree[parentPath]
         if (shares) {
@@ -294,20 +259,6 @@ export default {
       const translatedFolder = this.$gettext("You don't have permission to share this folder.")
       return this.highlightedFile.type === 'file' ? translatedFile : translatedFolder
     },
-
-    currentUsersPermissions() {
-      if (this.$_allIncomingShares.length > 0) {
-        let permissions = 0
-
-        for (const share of this.$_allIncomingShares) {
-          permissions |= share.permissions
-        }
-
-        return permissions
-      }
-
-      return null
-    },
     currentUserIsMemberOfSpace() {
       return this.currentSpace?.spaceMemberIds?.includes(this.user.uuid)
     },