Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
fix(files): Implement SortingService to fix sorting of files
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 <[email protected]>
  • Loading branch information
susnux committed May 28, 2024
commit 1d811291a5fc96b629680a6ca08007c7be01101a
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'])
})
})
59 changes: 59 additions & 0 deletions apps/files/src/services/SortingService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* 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'

/**
* Helper to create string representation
* @param value Value to stringify
*/
function stringify(value: unknown) {
// The default representation of Date is not sortable because of the weekday names in front of it
if (value instanceof Date) {
return value.toISOString()
}
return String(value)
}

/**
* 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(stringify(identifier(a)), stringify(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 @@ -125,7 +125,6 @@ import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
import { Folder, Node, Permission } from '@nextcloud/files'
import { getCapabilities } from '@nextcloud/capabilities'
import { join, dirname } from 'path'
import { orderBy } from 'natural-orderby'
import { Parser } from 'xml2js'
import { showError } from '@nextcloud/dialogs'
import { translate, translatePlural } from '@nextcloud/l10n'
Expand All @@ -152,6 +151,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
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@
"marked": "^9.1.5",
"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