Skip to content

Commit c7014a7

Browse files
committed
fix(files_trashbin): correctly sort custom columns in trashbin view
1. Refactor to make code better testable (move columns and view source to `files_views` folder) 2. Fix deletion time fallback (JS Date vs unix timestamp for "delted"-column) 3. Correctly sort `deletedBy` and `originalLocation` columns to use natural sort like any other column 4. Add unit tests for columns and views Signed-off-by: Ferdinand Thiessen <[email protected]>
1 parent a91cd62 commit c7014a7

File tree

7 files changed

+387
-80
lines changed

7 files changed

+387
-80
lines changed

__tests__/setup-global.js

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: CC0-1.0
4+
*/
5+
export function setup() {
6+
process.env.TZ = 'UTC'
7+
}

apps/files_trashbin/src/files-init.ts

+2-18
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55

6+
import { trashbinView } from './files_views/trashbinView.ts'
67
import './trashbin.scss'
78

89
import { translate as t } from '@nextcloud/l10n'
@@ -18,23 +19,6 @@ import './actions/restoreAction'
1819
import { emptyTrashAction } from './fileListActions/emptyTrashAction.ts'
1920

2021
const Navigation = getNavigation()
21-
Navigation.register(new View({
22-
id: 'trashbin',
23-
name: t('files_trashbin', 'Deleted files'),
24-
caption: t('files_trashbin', 'List of files that have been deleted.'),
25-
26-
emptyTitle: t('files_trashbin', 'No deleted files'),
27-
emptyCaption: t('files_trashbin', 'Files and folders you have deleted will show up here'),
28-
29-
icon: DeleteSvg,
30-
order: 50,
31-
sticky: true,
32-
33-
defaultSortKey: 'deleted',
34-
35-
columns,
36-
37-
getContents,
38-
}))
22+
Navigation.register(trashbinView)
3923

4024
registerFileListAction(emptyTrashAction)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import { File } from '@nextcloud/files'
7+
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
8+
import { deleted, deletedBy, originalLocation } from './columns.ts'
9+
import { trashbinView } from './trashbinView.ts'
10+
import * as ncAuth from '@nextcloud/auth'
11+
12+
describe('files_trashbin: file list columns', () => {
13+
14+
describe('column: original location', () => {
15+
it('has id set', () => {
16+
expect(originalLocation.id).toBe('files_trashbin--original-location')
17+
})
18+
19+
it('has title set', () => {
20+
expect(originalLocation.title).toBe('Original location')
21+
})
22+
23+
it('correctly sorts nodes by original location', () => {
24+
const nodeA = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-original-location': 'z-folder/a.txt' } })
25+
const nodeB = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/b.txt', mime: 'text/plain', attributes: { 'trashbin-original-location': 'folder/b.txt' } })
26+
27+
expect(originalLocation.sort).toBeTypeOf('function')
28+
expect(originalLocation.sort!(nodeA, nodeB)).toBeGreaterThan(0)
29+
expect(originalLocation.sort!(nodeB, nodeA)).toBeLessThan(0)
30+
})
31+
32+
it('renders a node with original location', () => {
33+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-original-location': 'folder/a.txt' } })
34+
const el: HTMLElement = originalLocation.render(node, trashbinView)
35+
expect(el).toBeInstanceOf(HTMLElement)
36+
expect(el.textContent).toBe('folder')
37+
expect(el.title).toBe('folder')
38+
})
39+
40+
it('renders a node when original location is missing', () => {
41+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain' })
42+
const el: HTMLElement = originalLocation.render(node, trashbinView)
43+
expect(el).toBeInstanceOf(HTMLElement)
44+
expect(el.textContent).toBe('Unknown')
45+
expect(el.title).toBe('Unknown')
46+
})
47+
48+
it('renders a node when original location is the root', () => {
49+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-original-location': 'a.txt' } })
50+
const el: HTMLElement = originalLocation.render(node, trashbinView)
51+
expect(el).toBeInstanceOf(HTMLElement)
52+
expect(el.textContent).toBe('All files')
53+
expect(el.title).toBe('All files')
54+
})
55+
})
56+
57+
describe('column: deleted time', () => {
58+
it('has id set', () => {
59+
expect(deleted.id).toBe('files_trashbin--deleted')
60+
})
61+
62+
it('has title set', () => {
63+
expect(deleted.title).toBe('Deleted')
64+
})
65+
66+
it('correctly sorts nodes by deleted time', () => {
67+
const nodeA = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deletion-time': 1741684522 } })
68+
const nodeB = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/b.txt', mime: 'text/plain', attributes: { 'trashbin-deletion-time': 1741684422 } })
69+
70+
expect(deleted.sort).toBeTypeOf('function')
71+
expect(deleted.sort!(nodeA, nodeB)).toBeLessThan(0)
72+
expect(deleted.sort!(nodeB, nodeA)).toBeGreaterThan(0)
73+
})
74+
75+
it('correctly sorts nodes by deleted time and falls back to mtime', () => {
76+
const nodeA = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deletion-time': 1741684522 } })
77+
const nodeB = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/b.txt', mime: 'text/plain', mtime: new Date(1741684422000) })
78+
79+
expect(deleted.sort).toBeTypeOf('function')
80+
expect(deleted.sort!(nodeA, nodeB)).toBeLessThan(0)
81+
expect(deleted.sort!(nodeB, nodeA)).toBeGreaterThan(0)
82+
})
83+
84+
it('correctly sorts nodes even if no deletion date is provided', () => {
85+
const nodeA = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain' })
86+
const nodeB = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/b.txt', mime: 'text/plain', mtime: new Date(1741684422000) })
87+
88+
expect(deleted.sort).toBeTypeOf('function')
89+
expect(deleted.sort!(nodeA, nodeB)).toBeGreaterThan(0)
90+
expect(deleted.sort!(nodeB, nodeA)).toBeLessThan(0)
91+
})
92+
93+
describe('rendering', () => {
94+
afterAll(() => {
95+
vi.useRealTimers()
96+
})
97+
98+
beforeEach(() => {
99+
vi.useFakeTimers({ now: 1741684582000 })
100+
})
101+
102+
it('renders a node with deletion date', () => {
103+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deletion-time': 1741684522 } })
104+
const el: HTMLElement = deleted.render(node, trashbinView)
105+
expect(el).toBeInstanceOf(HTMLElement)
106+
expect(el.textContent).toBe('a minute ago')
107+
expect(el.title).toBe('March 11, 2025 9:15 AM')
108+
})
109+
110+
it('renders a node when deletion date is missing and falls back to mtime', () => {
111+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', mtime: new Date(1741684522000) })
112+
const el: HTMLElement = deleted.render(node, trashbinView)
113+
expect(el).toBeInstanceOf(HTMLElement)
114+
expect(el.textContent).toBe('a minute ago')
115+
expect(el.title).toBe('March 11, 2025 9:15 AM')
116+
})
117+
118+
it('renders a node when deletion date is missing', () => {
119+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain' })
120+
const el: HTMLElement = deleted.render(node, trashbinView)
121+
expect(el).toBeInstanceOf(HTMLElement)
122+
expect(el.textContent).toBe('A long time ago')
123+
})
124+
})
125+
126+
describe('column: deleted by', () => {
127+
it('has id set', () => {
128+
expect(deletedBy.id).toBe('files_trashbin--deleted-by')
129+
})
130+
131+
it('has title set', () => {
132+
expect(deletedBy.title).toBe('Deleted by')
133+
})
134+
135+
it('correctly sorts nodes by user-id of deleting user', () => {
136+
const nodeA = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-id': 'zzz' } })
137+
const nodeB = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/b.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-id': 'aaa' } })
138+
139+
expect(deletedBy.sort).toBeTypeOf('function')
140+
expect(deletedBy.sort!(nodeA, nodeB)).toBeGreaterThan(0)
141+
expect(deletedBy.sort!(nodeB, nodeA)).toBeLessThan(0)
142+
})
143+
144+
it('correctly sorts nodes by display name of deleting user', () => {
145+
const nodeA = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-display-name': 'zzz' } })
146+
const nodeB = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/b.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-display-name': 'aaa' } })
147+
148+
expect(deletedBy.sort).toBeTypeOf('function')
149+
expect(deletedBy.sort!(nodeA, nodeB)).toBeGreaterThan(0)
150+
expect(deletedBy.sort!(nodeB, nodeA)).toBeLessThan(0)
151+
})
152+
153+
it('correctly sorts nodes by display name of deleting user before user id', () => {
154+
const nodeA = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-display-name': '000', 'trashbin-deleted-by-id': 'zzz' } })
155+
const nodeB = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/b.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-display-name': 'aaa', 'trashbin-deleted-by-id': '999' } })
156+
157+
expect(deletedBy.sort).toBeTypeOf('function')
158+
expect(deletedBy.sort!(nodeA, nodeB)).toBeLessThan(0)
159+
expect(deletedBy.sort!(nodeB, nodeA)).toBeGreaterThan(0)
160+
})
161+
162+
it('correctly sorts nodes even when one is missing', () => {
163+
const nodeA = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-id': 'aaa' } })
164+
const nodeB = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-id': 'zzz' } })
165+
const nodeC = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/b.txt', mime: 'text/plain' })
166+
167+
expect(deletedBy.sort).toBeTypeOf('function')
168+
// aaa is less then "Unknown"
169+
expect(deletedBy.sort!(nodeA, nodeC)).toBeLessThan(0)
170+
// zzz is greater than "Unknown"
171+
expect(deletedBy.sort!(nodeB, nodeC)).toBeGreaterThan(0)
172+
})
173+
174+
it('renders a node with deleting user', () => {
175+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-id': 'user-id' } })
176+
const el: HTMLElement = deletedBy.render(node, trashbinView)
177+
expect(el).toBeInstanceOf(HTMLElement)
178+
expect(el.textContent).toMatch(/\suser-id\s/)
179+
})
180+
181+
it('renders a node with deleting user display name', () => {
182+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-display-name': 'user-name', 'trashbin-deleted-by-id': 'user-id' } })
183+
const el: HTMLElement = deletedBy.render(node, trashbinView)
184+
expect(el).toBeInstanceOf(HTMLElement)
185+
expect(el.textContent).toMatch(/\suser-name\s/)
186+
})
187+
188+
it('renders a node even when information is missing', () => {
189+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain' })
190+
const el: HTMLElement = deletedBy.render(node, trashbinView)
191+
expect(el).toBeInstanceOf(HTMLElement)
192+
expect(el.textContent).toBe('Unknown')
193+
})
194+
195+
it('renders a node when current user is the deleting user', () => {
196+
vi.spyOn(ncAuth, 'getCurrentUser').mockImplementationOnce(() => ({
197+
uid: 'user-id',
198+
displayName: 'user-display-name',
199+
isAdmin: false,
200+
}))
201+
202+
const node = new File({ owner: 'test', source: 'https://example.com/remote.php/dav/files/test/a.txt', mime: 'text/plain', attributes: { 'trashbin-deleted-by-id': 'user-id' } })
203+
const el: HTMLElement = deletedBy.render(node, trashbinView)
204+
expect(el).toBeInstanceOf(HTMLElement)
205+
expect(el.textContent).toBe('You')
206+
})
207+
})
208+
209+
})
210+
211+
})

0 commit comments

Comments
 (0)