Merge pull request #984 from nextcloud-libraries/fix/files-sorting-ex…

…tension

fix: When sorting by filename the extension should only be considered if the basename is equal

__tests__/utils/fileSorting.spec.ts

@@ -35,8 +35,8 @@ const sortNodes = (...args: ArgumentsType<typeof originalSortNodes>) => original
 describe('sortNodes', () => {
 	test('By default files are sorted by name', () => {
 		const array = [
-			file('a', 500, 100),
 			file('b', 100, 100),
+			file('a', 500, 100),
 			file('c', 100, 500),
 		]
 
@@ -224,4 +224,71 @@ describe('sortNodes', () => {
 
 		expect(sortNodes(array, { sortingMode: FilesSortingMode.Modified, sortingOrder: 'desc' })).toEqual(['a', 'b', 'c'])
 	})
+
+	/**
+	 * Regression testing for https://github.com/nextcloud/server/issues/45829
+	 */
+	test('Names with underscores are sorted correctly', () => {
+		const array = [
+			file('file_1.txt'),
+			file('file_3.txt'),
+			file('file.txt'),
+			file('file_2.txt'),
+		] as const
+
+		expect(
+			sortNodes(
+				array,
+				{
+					sortingMode: FilesSortingMode.Name,
+					sortingOrder: 'asc',
+				},
+			),
+		).toEqual(['file.txt', 'file_1.txt', 'file_2.txt', 'file_3.txt'])
+	})
+
+	/**
+	 * Ensure that also without extensions the files are sorted correctly
+	 * Regression testing for https://github.com/nextcloud/server/issues/45829
+	 */
+	test('Names with underscores without extensions are sorted correctly', () => {
+		const array = [
+			file('file_1'),
+			file('file_3'),
+			file('file'),
+			file('file_2'),
+		] as const
+
+		expect(
+			sortNodes(
+				array,
+				{
+					sortingMode: FilesSortingMode.Name,
+					sortingOrder: 'asc',
+				},
+			),
+		).toEqual(['file', 'file_1', 'file_2', 'file_3'])
+	})
+
+	/**
+	 * Ensure that files with same name but different extensions are sorted by the full name incl. extension
+	 */
+	test('Names are sorted correctly by extension', () => {
+		const array = [
+			file('file.b'),
+			file('file.c'),
+			file('file.a'),
+			file('file.d'),
+		] as const
+
+		expect(
+			sortNodes(
+				array,
+				{
+					sortingMode: FilesSortingMode.Name,
+					sortingOrder: 'asc',
+				},
+			),
+		).toEqual(['file.a', 'file.b', 'file.c', 'file.d'])
+	})
 })

lib/utils/fileSorting.ts

@@ -41,7 +41,7 @@ export interface FilesSortingOptions {
  * @param nodes Nodes to sort
  * @param options Sorting options
  */
-export function sortNodes(nodes: Node[], options: FilesSortingOptions = {}): Node[] {
+export function sortNodes(nodes: readonly Node[], options: FilesSortingOptions = {}): Node[] {
 	const sortingOptions = {
 		// Default to sort by name
 		sortingMode: FilesSortingMode.Name,
@@ -50,6 +50,12 @@ export function sortNodes(nodes: Node[], options: FilesSortingOptions = {}): Nod
 		...options,
 	}
 
+	/**
+	 * Get the basename without any extension
+	 * @param name The filename to extract the basename from
+	 */
+	const basename = (name: string) => name.lastIndexOf('.') > 0 ? name.slice(0, name.lastIndexOf('.')) : name
+
 	const identifiers = [
 		// 1: Sort favorites first if enabled
 		...(sortingOptions.sortFavoritesFirst ? [(v: Node) => v.attributes?.favorite !== 1] : []),
@@ -58,7 +64,7 @@ export function sortNodes(nodes: Node[], options: FilesSortingOptions = {}): Nod
 		// 3: Use sorting mode if NOT basename (to be able to use displayName too)
 		...(sortingOptions.sortingMode !== FilesSortingMode.Name ? [(v: Node) => v[sortingOptions.sortingMode]] : []),
 		// 4: Use displayName if available, fallback to name
-		(v: Node) => v.attributes?.displayName || v.basename,
+		(v: Node) => basename(v.attributes?.displayName || v.basename),
 		// 5: Finally, use basename if all previous sorting methods failed
 		(v: Node) => v.basename,
 	]