Change sort order for collaborators and public links

Adjusted order of collaborators

Sort public links and disable edit options for indirect shares

apps/files/src/components/FileLinkSidebar.vue

@@ -32,28 +32,16 @@
                             :leave-active-class="$_transitionGroupLeave"
                             name="custom-classes-transition"
                             tag="ul">
-            <li v-for="link in $_directLinks" :key="link.key">
+            <li v-for="link in links" :key="link.key">
               <public-link-list-item :link="link"
-                                     :modifiable="true"
+                                     :modifiable="!link.outgoing"
                                      :indirect="false"
                                      :linksCopied="linksCopied"
                                      @onCopy="$_clipboardSuccessHandler"
                                      @onDelete="$_removePublicLink"
                                      @onEdit="$_editPublicLink" />
             </li>
           </transition-group>
-          <hr class="uk-margin-small-top uk-margin-small-bottom" v-if="$_directLinks.length > 0 && $_indirectLinks.length > 0"/>
-        </section>
-        <section v-if="$_indirectLinks.length > 0">
-          <ul class="uk-list uk-list-divider uk-overflow-hidden uk-margin-remove">
-            <li v-for="link in $_indirectLinks" :key="'li-' + link.id">
-              <public-link-list-item :link="link"
-                                     :modifiable="false"
-                                     :indirect="true"
-                                     :linksCopied="linksCopied"
-                                     @onCopy="$_clipboardSuccessHandler" />
-            </li>
-          </ul>
         </section>
         <div v-if="$_noPublicLinks" key="oc-file-links-no-results">
           <translate>No public links</translate>
@@ -128,16 +116,16 @@ export default {
       return this.transitionGroupActive ? 'uk-animation-slide-right-medium uk-animation-reverse' : ''
     },
 
-    $_directLinks () {
-      return [...this.currentFileOutgoingLinks]
-        .sort(this.$_linksComparator)
+    links () {
+      return [...this.currentFileOutgoingLinks, ...this.indirectLinks]
+        .sort(this.linksComparator)
         .map(share => {
           share.key = 'direct-link-' + share.id
           return share
         })
     },
 
-    $_indirectLinks () {
+    indirectLinks () {
       const allShares = []
       const parentPaths = getParentPaths(this.highlightedFile.path, true)
       if (parentPaths.length === 0) {
@@ -159,11 +147,11 @@ export default {
         }
       })
 
-      return allShares.sort(this.$_linksComparator)
+      return allShares.sort(this.linksComparator)
     },
 
     $_noPublicLinks () {
-      return (this.$_directLinks.length + this.$_indirectLinks.length) === 0
+      return this.links.length === 0
     },
 
     $_expirationDate () {
@@ -254,15 +242,22 @@ export default {
         $gettext: this.$gettext
       })
     },
-    $_linksComparator (l1, l2) {
+    linksComparator (l1, l2) {
+      // sorting priority 1: display name (lower case, ascending), 2: creation time (descending)
       const name1 = l1.name.toLowerCase().trim()
       const name2 = l2.name.toLowerCase().trim()
-      // sorting priority 1: display name (lower case, ascending), 2: id (ascending)
-      if (name1 === name2) {
-        return textUtils.naturalSortCompare(l1.id + '', l2.id + '')
-      } else {
+      const l1Direct = !l1.outgoing
+      const l2Direct = !l2.outgoing
+
+      if (l1Direct === l2Direct) {
+        if (name1 === name2) {
+          return textUtils.naturalSortCompare(l1.id + '', l2.id + '')
+        }
+
         return textUtils.naturalSortCompare(name1, name2)
       }
+
+      return l1Direct ? -1 : 1
     }
   }
 }

apps/files/src/components/FileSharingSidebar.vue

@@ -22,7 +22,7 @@
               <collaborator :collaborator="$_currentUserAsCollaborator"/>
             </li>
           </ul>
-          <hr class="uk-margin-small-top uk-margin-small-bottom" v-if="$_directOutgoingShares.length > 0 || $_indirectOutgoingShares.length > 0" />
+          <hr class="uk-margin-small-top uk-margin-small-bottom" v-if="collaborators.length > 0" />
         </section>
         <section>
           <transition-group id="files-collaborators-list"
@@ -31,18 +31,10 @@
                             :leave-active-class="$_transitionGroupLeave"
                             name="custom-classes-transition"
                             tag="ul">
-            <li v-for="collaborator in $_directOutgoingShares" :key="collaborator.key">
-              <collaborator :collaborator="collaborator" :modifiable="true" @onDelete="$_ocCollaborators_deleteShare" @onEdit="$_ocCollaborators_editShare"/>
+            <li v-for="collaborator in collaborators" :key="collaborator.key">
+              <collaborator :collaborator="collaborator" :modifiable="!collaborator.outgoing" @onDelete="$_ocCollaborators_deleteShare" @onEdit="$_ocCollaborators_editShare"/>
             </li>
           </transition-group>
-          <hr class="uk-margin-small-top uk-margin-small-bottom" v-if="$_directOutgoingShares.length > 0 && $_indirectOutgoingShares.length > 0" />
-        </section>
-        <section v-if="$_indirectOutgoingShares.length > 0">
-          <ul class="uk-list uk-list-divider uk-overflow-hidden uk-margin-remove">
-            <li v-for="collaborator in $_indirectOutgoingShares" :key="collaborator.key">
-              <collaborator :collaborator="collaborator"/>
-            </li>
-          </ul>
         </section>
       </template>
     </div>
@@ -175,7 +167,7 @@ export default {
       })
 
       // make them unique then sort
-      ownerAsCollaborator.resharers = Array.from(resharers.values()).sort(this.$_collaboratorsComparator.bind(this))
+      ownerAsCollaborator.resharers = Array.from(resharers.values()).sort(this.collaboratorsComparator.bind(this))
       return ownerAsCollaborator
     },
 
@@ -211,10 +203,9 @@ export default {
       return allShares
     },
 
-    $_directOutgoingShares () {
-      // direct outgoing shares
-      return [...this.currentFileOutgoingCollaborators]
-        .sort(this.$_collaboratorsComparator)
+    collaborators () {
+      return [...this.currentFileOutgoingCollaborators, ...this.indirectOutgoingShares]
+        .sort(this.collaboratorsComparator)
         .map(collaborator => {
           collaborator.key = 'collaborator-' + collaborator.id
           if (collaborator.owner.name !== collaborator.fileOwner.name && collaborator.owner.name !== this.user.id) {
@@ -224,7 +215,7 @@ export default {
         })
     },
 
-    $_indirectOutgoingShares () {
+    indirectOutgoingShares () {
       const allShares = []
       const parentPaths = getParentPaths(this.highlightedFile.path, true)
       if (parentPaths.length === 0) {
@@ -294,25 +285,30 @@ export default {
     $_isCollaboratorShare (collaborator) {
       return userShareTypes.includes(collaborator.shareType)
     },
-    $_collaboratorsComparator (c1, c2) {
-      // resharer entries have displayName on first level,
-      // but shares/collaborators have it under the collaborator attribute
+    collaboratorsComparator (c1, c2) {
+      // Sorted by: type, direct, display name, creation date
       const c1DisplayName = c1.collaborator ? c1.collaborator.displayName : c1.displayName
       const c2DisplayName = c2.collaborator ? c2.collaborator.displayName : c2.displayName
       const name1 = c1DisplayName.toLowerCase().trim()
       const name2 = c2DisplayName.toLowerCase().trim()
-      // sorting priority 1: display name (lower case, ascending), 2: share type (groups first), 3: id (ascending)
-      if (name1 === name2) {
-        const c1GroupShare = c1.shareType === shareTypes.group
-        const c2GroupShare = c2.shareType === shareTypes.group
-        if (c1GroupShare === c2GroupShare) {
-          return textUtils.naturalSortCompare(c1.id + '', c2.id + '')
-        } else {
-          return c1GroupShare ? -1 : 1
+      const c1UserShare = c1.shareType === shareTypes.user || c1.shareType === shareTypes.remote
+      const c2UserShare = c2.shareType === shareTypes.user || c1.shareType === shareTypes.remote
+      const c1DirectShare = !c1.outgoing
+      const c2DirectShare = !c2.outgoing
+
+      if (c1UserShare === c2UserShare) {
+        if (c1DirectShare === c2DirectShare) {
+          if (name1 === name2) {
+            return textUtils.naturalSortCompare(c1.stime + '', c2.stime + '')
+          }
+
+          return textUtils.naturalSortCompare(name1, name2)
         }
-      } else {
-        return textUtils.naturalSortCompare(name1, name2)
+
+        return c1DirectShare ? -1 : 1
       }
+
+      return c1UserShare ? -1 : 1
     },
     $_ocCollaborators_addShare () {
       this.transitionGroupActive = true

changelog/unreleased/3136

@@ -0,0 +1,7 @@
+Change: New sort order for collaborators and public links
+
+We've changed the sort order for collaborators and public links.
+Collaborators are now sorted by: collaborator type, is collaborator direct, display name and creation date.
+Public links are now sorted by: is public link direct, display name and creation date.
+
+https://github.com/owncloud/phoenix/pull/3136

tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature

@@ -22,7 +22,7 @@ Feature: Sharing files and folders with internal groups
     And the user shares folder "simple-folder" with user "User Two" as "Viewer" using the webUI
     And the user shares folder "simple-folder" with group "grp1" as "Viewer" using the webUI
     And the user shares folder "simple-folder" with user "User One" as "Viewer" using the webUI
-    Then the current collaborators list should have order "User Three,grp1,grp11,User One,User Two"
+    Then the current collaborators list should have order "User Three,User One,User Two,grp1,grp11"
 
   Scenario Outline: share a file & folder with another internal user
     Given user "user3" has logged in using the webUI