Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed sort order for collaborators and public links #3136

Merged
merged 1 commit into from
Mar 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 21 additions & 26 deletions apps/files/src/components/FileLinkSidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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"
:indirect="false"
:modifiable="!link.indirect"
:indirect="link.indirect"
: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>
Expand Down Expand Up @@ -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) {
Expand All @@ -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 () {
Expand Down Expand Up @@ -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.indirect
const l2Direct = !l2.indirect

if (l1Direct === l2Direct) {
if (name1 === name2) {
return textUtils.naturalSortCompare(l1.id + '', l2.id + '')
}

return textUtils.naturalSortCompare(name1, name2)
}

return l1Direct ? -1 : 1
}
}
}
Expand Down
56 changes: 26 additions & 30 deletions apps/files/src/components/FileSharingSidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.indirect" @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>
Expand Down Expand Up @@ -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
},

Expand Down Expand Up @@ -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) {
Expand All @@ -224,7 +215,7 @@ export default {
})
},

$_indirectOutgoingShares () {
indirectOutgoingShares () {
const allShares = []
const parentPaths = getParentPaths(this.highlightedFile.path, true)
if (parentPaths.length === 0) {
Expand Down Expand Up @@ -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.indirect
const c2DirectShare = !c2.indirect

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
Expand Down
4 changes: 2 additions & 2 deletions apps/files/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ export default {
client.shares.getShares(queryPath, { reshares: true })
.then(data => {
data.forEach(element => {
sharesTree[queryPath].push({ ..._buildShare(element.shareInfo, { type: 'folder' }, $gettext), outgoing: true })
sharesTree[queryPath].push({ ..._buildShare(element.shareInfo, { type: 'folder' }, $gettext), outgoing: true, indirect: true })
})
})
.catch(error => {
Expand All @@ -777,7 +777,7 @@ export default {
client.shares.getShares(queryPath, { shared_with_me: true })
.then(data => {
data.forEach(element => {
sharesTree[queryPath].push({ ..._buildCollaboratorShare(element.shareInfo, { type: 'folder' }), incoming: true })
sharesTree[queryPath].push({ ..._buildCollaboratorShare(element.shareInfo, { type: 'folder' }), incoming: true, indirect: true })
})
})
.catch(error => {
Expand Down
7 changes: 7 additions & 0 deletions changelog/unreleased/3136
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,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
Expand Down