Skip to content

Commit

Permalink
fix(files): Implement SortingService to fix sorting of files
Browse files Browse the repository at this point in the history
The previously used library was parsing strings to try to detect dates,
but for filenames it makes no sense to parse them as dates.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
susnux committed May 21, 2024
1 parent 5a6e48e commit 531197e
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 11 deletions.
100 changes: 100 additions & 0 deletions apps/files/src/services/SortingService.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import { describe, expect } from '@jest/globals'
import { orderBy } from './SortingService'

describe('SortingService', () => {
test('By default the identify and ascending order is used', () => {
const array = ['a', 'z', 'b']
expect(orderBy(array)).toEqual(['a', 'b', 'z'])
})

test('Use identifiy but descending', () => {
const array = ['a', 'z', 'b']
expect(orderBy(array, undefined, ['desc'])).toEqual(['z', 'b', 'a'])
})

test('Can set identifier function', () => {
const array = [
{ text: 'a', order: 2 },
{ text: 'z', order: 1 },
{ text: 'b', order: 3 },
] as const
expect(orderBy(array, [(v) => v.order]).map((v) => v.text)).toEqual(['z', 'a', 'b'])
})

test('Can set multiple identifier functions', () => {
const array = [
{ text: 'a', order: 2, secondOrder: 2 },
{ text: 'z', order: 1, secondOrder: 3 },
{ text: 'b', order: 2, secondOrder: 1 },
] as const
expect(orderBy(array, [(v) => v.order, (v) => v.secondOrder]).map((v) => v.text)).toEqual(['z', 'b', 'a'])
})

test('Can set order partially', () => {
const array = [
{ text: 'a', order: 2, secondOrder: 2 },
{ text: 'z', order: 1, secondOrder: 3 },
{ text: 'b', order: 2, secondOrder: 1 },
] as const

expect(
orderBy(
array,
[(v) => v.order, (v) => v.secondOrder],
['desc'],
).map((v) => v.text),
).toEqual(['b', 'a', 'z'])
})

test('Can set order array', () => {
const array = [
{ text: 'a', order: 2, secondOrder: 2 },
{ text: 'z', order: 1, secondOrder: 3 },
{ text: 'b', order: 2, secondOrder: 1 },
] as const

expect(
orderBy(
array,
[(v) => v.order, (v) => v.secondOrder],
['desc', 'desc'],
).map((v) => v.text),
).toEqual(['a', 'b', 'z'])
})

test('Numbers are handled correctly', () => {
const array = [
{ text: '2.3' },
{ text: '2.10' },
{ text: '2.0' },
{ text: '2.2' },
] as const

expect(
orderBy(
array,
[(v) => v.text],
).map((v) => v.text),
).toEqual(['2.0', '2.2', '2.3', '2.10'])
})

test('Numbers with suffixes are handled correctly', () => {
const array = [
{ text: '2024-01-05' },
{ text: '2024-05-01' },
{ text: '2024-01-10' },
{ text: '2024-01-05 Foo' },
] as const

expect(
orderBy(
array,
[(v) => v.text],
).map((v) => v.text),
).toEqual(['2024-01-05', '2024-01-05 Foo', '2024-01-10', '2024-05-01'])
})
})
47 changes: 47 additions & 0 deletions apps/files/src/services/SortingService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import { getCanonicalLocale, getLanguage } from '@nextcloud/l10n'

type IdentifierFn<T> = (v: T) => unknown
type SortingOrder = 'asc'|'desc'

/**
* Natural order a collection
* You can define identifiers as callback functions, that get the element and return the value to sort.
*
* @param collection The collection to order
* @param identifiers An array of identifiers to use, by default the identity of the element is used
* @param orders Array of orders, by default all identifiers are sorted ascening
*/
export function orderBy<T>(collection: readonly T[], identifiers?: IdentifierFn<T>[], orders?: SortingOrder[]): T[] {
// If not identifiers are set we use the identity of the value
identifiers = identifiers ?? [(value) => value]
// By default sort the collection ascending
orders = orders ?? []
const sorting = identifiers.map((_, index) => (orders[index] ?? 'asc') === 'asc' ? 1 : -1)

const collator = Intl.Collator(
[getLanguage(), getCanonicalLocale()],
{
// handle 10 as ten and not as one-zero
numeric: true,
usage: 'sort',
},
)

return [...collection].sort((a, b) => {
for (const [index, identifier] of identifiers.entries()) {
// Get the local compare of stringified value a and b
const value = collator.compare(String(identifier(a)), String(identifier(b)))
// If they do not match return the order
if (value !== 0) {
return value * sorting[index]
}
// If they match we need to continue with the next identifier
}
// If all are equal we need to return equality
return 0
})
}
2 changes: 1 addition & 1 deletion apps/files/src/views/FilesList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
import { Folder, Node, Permission } from '@nextcloud/files'
import { translate as t } from '@nextcloud/l10n'
import { join, dirname } from 'path'
import { orderBy } from 'natural-orderby'
import { showError } from '@nextcloud/dialogs'
import { Type } from '@nextcloud/sharing'
import { UploadPicker } from '@nextcloud/upload'
Expand All @@ -151,6 +150,7 @@ import { useSelectionStore } from '../store/selection.ts'
import { useUploaderStore } from '../store/uploader.ts'
import { useUserConfigStore } from '../store/userconfig.ts'
import { useViewConfigStore } from '../store/viewConfig.ts'
import { orderBy } from '../services/SortingService.ts'
import BreadCrumbs from '../components/BreadCrumbs.vue'
import FilesListVirtual from '../components/FilesListVirtual.vue'
import filesListWidthMixin from '../mixins/filesListWidth.ts'
Expand Down
9 changes: 0 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
"marked": "^11.2.0",
"moment": "^2.30.1",
"moment-timezone": "^0.5.45",
"natural-orderby": "^3.0.2",
"nextcloud-vue-collections": "^0.12.0",
"node-vibrant": "^3.1.6",
"p-limit": "^4.0.0",
Expand Down

0 comments on commit 531197e

Please sign in to comment.