Skip to content

Commit 158f60b

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #51263 from nextcloud/backport/51152/stable31
[stable31] fix(files_sharing): ensure downloaded file has the correct filename
2 parents 0729301 + db80035 commit 158f60b

6 files changed

Lines changed: 175 additions & 112 deletions

File tree

‎apps/files/src/actions/downloadAction.spec.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ describe('Download action execute tests', () => {
105105

106106
// Silent action
107107
expect(exec).toBe(null)
108-
expect(link.download).toEqual('')
108+
expect(link.download).toBe('foobar.txt')
109109
expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
110110
expect(link.click).toHaveBeenCalledTimes(1)
111111
})
@@ -123,7 +123,26 @@ describe('Download action execute tests', () => {
123123

124124
// Silent action
125125
expect(exec).toStrictEqual([null])
126-
expect(link.download).toEqual('')
126+
expect(link.download).toEqual('foobar.txt')
127+
expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
128+
expect(link.click).toHaveBeenCalledTimes(1)
129+
})
130+
131+
test('Download single file with displayname set', async () => {
132+
const file = new File({
133+
id: 1,
134+
source: 'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
135+
owner: 'admin',
136+
mime: 'text/plain',
137+
displayname: 'baz.txt',
138+
permissions: Permission.READ,
139+
})
140+
141+
const exec = await action.execBatch!([file], view, '/')
142+
143+
// Silent action
144+
expect(exec).toStrictEqual([null])
145+
expect(link.download).toEqual('baz.txt')
127146
expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
128147
expect(link.click).toHaveBeenCalledTimes(1)
129148
})

‎apps/files/src/actions/downloadAction.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,15 @@ import { isDownloadable } from '../utils/permissions'
99

1010
import ArrowDownSvg from '@mdi/svg/svg/arrow-down.svg?raw'
1111

12-
const triggerDownload = function(url: string) {
12+
/**
13+
* Trigger downloading a file.
14+
*
15+
* @param url The url of the asset to download
16+
* @param name Optionally the recommended name of the download (browsers might ignore it)
17+
*/
18+
function triggerDownload(url: string, name?: string) {
1319
const hiddenElement = document.createElement('a')
14-
hiddenElement.download = ''
20+
hiddenElement.download = name ?? ''
1521
hiddenElement.href = url
1622
hiddenElement.click()
1723
}
@@ -43,7 +49,7 @@ const downloadNodes = function(nodes: Node[]) {
4349

4450
if (nodes.length === 1) {
4551
if (nodes[0].type === FileType.File) {
46-
return triggerDownload(nodes[0].encodedSource)
52+
return triggerDownload(nodes[0].encodedSource, nodes[0].displayname)
4753
} else {
4854
url = new URL(nodes[0].encodedSource)
4955
url.searchParams.append('accept', 'zip')

‎cypress/e2e/files_sharing/public-share/download-files.cy.ts‎

Lines changed: 137 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -4,136 +4,170 @@
44
*/
55
// @ts-expect-error The package is currently broken - but works...
66
import { deleteDownloadsFolderBeforeEach } from 'cypress-delete-downloads-folder'
7-
7+
import { createShare, getShareUrl, setupPublicShare, type ShareContext } from './setup-public-share.ts'
8+
import { getRowForFile, getRowForFileId, triggerActionForFile, triggerActionForFileId } from '../../files/FilesUtils.ts'
89
import { zipFileContains } from '../../../support/utils/assertions.ts'
9-
import { getRowForFile, triggerActionForFile } from '../../files/FilesUtils.ts'
10-
import { getShareUrl, setupPublicShare } from './setup-public-share.ts'
1110

1211
describe('files_sharing: Public share - downloading files', { testIsolation: true }, () => {
1312

14-
before(() => setupPublicShare())
15-
16-
deleteDownloadsFolderBeforeEach()
17-
18-
beforeEach(() => {
19-
cy.logout()
20-
cy.visit(getShareUrl())
21-
})
22-
23-
it('Can download all files', () => {
24-
getRowForFile('foo.txt').should('be.visible')
25-
26-
cy.get('[data-cy-files-list]').within(() => {
27-
cy.findByRole('checkbox', { name: /Toggle selection for all files/i })
28-
.should('exist')
29-
.check({ force: true })
30-
31-
// see that two files are selected
32-
cy.contains('2 selected').should('be.visible')
13+
// in general there is no difference except downloading
14+
// as file shares have the source of the share token but a different displayname
15+
describe('file share', () => {
16+
let fileId: number
17+
18+
before(() => {
19+
cy.createRandomUser().then((user) => {
20+
const context: ShareContext = { user }
21+
cy.uploadContent(user, new Blob(['<content>foo</content>']), 'text/plain', '/file.txt')
22+
.then(({ headers }) => { fileId = Number.parseInt(headers['oc-fileid']) })
23+
cy.login(user)
24+
createShare(context, 'file.txt')
25+
.then(() => cy.logout())
26+
.then(() => cy.visit(context.url!))
27+
})
28+
})
3329

34-
// click download
35-
cy.findByRole('button', { name: 'Download (selected)' })
30+
it('can download the file', () => {
31+
getRowForFileId(fileId)
3632
.should('be.visible')
37-
.click()
38-
39-
// check a file is downloaded
33+
getRowForFileId(fileId)
34+
.find('[data-cy-files-list-row-name]')
35+
.should((el) => expect(el.text()).to.match(/file\s*\.txt/)) // extension is sparated so there might be a space between
36+
triggerActionForFileId(fileId, 'download')
37+
// check a file is downloaded with the correct name
4038
const downloadsFolder = Cypress.config('downloadsFolder')
41-
cy.readFile(`${downloadsFolder}/download.zip`, null, { timeout: 15000 })
39+
cy.readFile(`${downloadsFolder}/file.txt`, 'utf-8', { timeout: 15000 })
4240
.should('exist')
43-
.and('have.length.gt', 30)
44-
// Check all files are included
45-
.and(zipFileContains([
46-
'foo.txt',
47-
'subfolder/',
48-
'subfolder/bar.txt',
49-
]))
41+
.and('have.length.gt', 5)
42+
.and('contain', '<content>foo</content>')
5043
})
5144
})
5245

53-
it('Can download selected files', () => {
54-
getRowForFile('subfolder')
55-
.should('be.visible')
46+
describe('folder share', () => {
47+
before(() => setupPublicShare())
5648

57-
cy.get('[data-cy-files-list]').within(() => {
58-
getRowForFile('subfolder')
59-
.findByRole('checkbox')
60-
.check({ force: true })
49+
deleteDownloadsFolderBeforeEach()
50+
51+
beforeEach(() => {
52+
cy.logout()
53+
cy.visit(getShareUrl())
54+
})
6155

62-
// see that two files are selected
63-
cy.contains('1 selected').should('be.visible')
56+
it('Can download all files', () => {
57+
getRowForFile('foo.txt').should('be.visible')
58+
59+
cy.get('[data-cy-files-list]').within(() => {
60+
cy.findByRole('checkbox', { name: /Toggle selection for all files/i })
61+
.should('exist')
62+
.check({ force: true })
63+
64+
// see that two files are selected
65+
cy.contains('2 selected').should('be.visible')
66+
67+
// click download
68+
cy.findByRole('button', { name: 'Download (selected)' })
69+
.should('be.visible')
70+
.click()
71+
72+
// check a file is downloaded
73+
const downloadsFolder = Cypress.config('downloadsFolder')
74+
cy.readFile(`${downloadsFolder}/download.zip`, null, { timeout: 15000 })
75+
.should('exist')
76+
.and('have.length.gt', 30)
77+
// Check all files are included
78+
.and(zipFileContains([
79+
'foo.txt',
80+
'subfolder/',
81+
'subfolder/bar.txt',
82+
]))
83+
})
84+
})
6485

65-
// click download
66-
cy.findByRole('button', { name: 'Download (selected)' })
86+
it('Can download selected files', () => {
87+
getRowForFile('subfolder')
6788
.should('be.visible')
68-
.click()
6989

70-
// check a file is downloaded
71-
const downloadsFolder = Cypress.config('downloadsFolder')
72-
cy.readFile(`${downloadsFolder}/subfolder.zip`, null, { timeout: 15000 })
73-
.should('exist')
74-
.and('have.length.gt', 30)
75-
// Check all files are included
76-
.and(zipFileContains([
77-
'subfolder/',
78-
'subfolder/bar.txt',
79-
]))
90+
cy.get('[data-cy-files-list]').within(() => {
91+
getRowForFile('subfolder')
92+
.findByRole('checkbox')
93+
.check({ force: true })
94+
95+
// see that two files are selected
96+
cy.contains('1 selected').should('be.visible')
97+
98+
// click download
99+
cy.findByRole('button', { name: 'Download (selected)' })
100+
.should('be.visible')
101+
.click()
102+
103+
// check a file is downloaded
104+
const downloadsFolder = Cypress.config('downloadsFolder')
105+
cy.readFile(`${downloadsFolder}/subfolder.zip`, null, { timeout: 15000 })
106+
.should('exist')
107+
.and('have.length.gt', 30)
108+
// Check all files are included
109+
.and(zipFileContains([
110+
'subfolder/',
111+
'subfolder/bar.txt',
112+
]))
113+
})
80114
})
81-
})
82115

83-
it('Can download folder by action', () => {
84-
getRowForFile('subfolder')
85-
.should('be.visible')
86-
87-
cy.get('[data-cy-files-list]').within(() => {
88-
triggerActionForFile('subfolder', 'download')
116+
it('Can download folder by action', () => {
117+
getRowForFile('subfolder')
118+
.should('be.visible')
89119

90-
// check a file is downloaded
91-
const downloadsFolder = Cypress.config('downloadsFolder')
92-
cy.readFile(`${downloadsFolder}/subfolder.zip`, null, { timeout: 15000 })
93-
.should('exist')
94-
.and('have.length.gt', 30)
95-
// Check all files are included
96-
.and(zipFileContains([
97-
'subfolder/',
98-
'subfolder/bar.txt',
99-
]))
120+
cy.get('[data-cy-files-list]').within(() => {
121+
triggerActionForFile('subfolder', 'download')
122+
123+
// check a file is downloaded
124+
const downloadsFolder = Cypress.config('downloadsFolder')
125+
cy.readFile(`${downloadsFolder}/subfolder.zip`, null, { timeout: 15000 })
126+
.should('exist')
127+
.and('have.length.gt', 30)
128+
// Check all files are included
129+
.and(zipFileContains([
130+
'subfolder/',
131+
'subfolder/bar.txt',
132+
]))
133+
})
100134
})
101-
})
102135

103-
it('Can download file by action', () => {
104-
getRowForFile('foo.txt')
105-
.should('be.visible')
136+
it('Can download file by action', () => {
137+
getRowForFile('foo.txt')
138+
.should('be.visible')
106139

107-
cy.get('[data-cy-files-list]').within(() => {
108-
triggerActionForFile('foo.txt', 'download')
140+
cy.get('[data-cy-files-list]').within(() => {
141+
triggerActionForFile('foo.txt', 'download')
109142

110-
// check a file is downloaded
111-
const downloadsFolder = Cypress.config('downloadsFolder')
112-
cy.readFile(`${downloadsFolder}/foo.txt`, 'utf-8', { timeout: 15000 })
113-
.should('exist')
114-
.and('have.length.gt', 5)
115-
.and('contain', '<content>foo</content>')
143+
// check a file is downloaded
144+
const downloadsFolder = Cypress.config('downloadsFolder')
145+
cy.readFile(`${downloadsFolder}/foo.txt`, 'utf-8', { timeout: 15000 })
146+
.should('exist')
147+
.and('have.length.gt', 5)
148+
.and('contain', '<content>foo</content>')
149+
})
116150
})
117-
})
118151

119-
it('Can download file by selection', () => {
120-
getRowForFile('foo.txt')
121-
.should('be.visible')
122-
123-
cy.get('[data-cy-files-list]').within(() => {
152+
it('Can download file by selection', () => {
124153
getRowForFile('foo.txt')
125-
.findByRole('checkbox')
126-
.check({ force: true })
127-
128-
cy.findByRole('button', { name: 'Download (selected)' })
129-
.click()
154+
.should('be.visible')
130155

131-
// check a file is downloaded
132-
const downloadsFolder = Cypress.config('downloadsFolder')
133-
cy.readFile(`${downloadsFolder}/foo.txt`, 'utf-8', { timeout: 15000 })
134-
.should('exist')
135-
.and('have.length.gt', 5)
136-
.and('contain', '<content>foo</content>')
156+
cy.get('[data-cy-files-list]').within(() => {
157+
getRowForFile('foo.txt')
158+
.findByRole('checkbox')
159+
.check({ force: true })
160+
161+
cy.findByRole('button', { name: 'Download (selected)' })
162+
.click()
163+
164+
// check a file is downloaded
165+
const downloadsFolder = Cypress.config('downloadsFolder')
166+
cy.readFile(`${downloadsFolder}/foo.txt`, 'utf-8', { timeout: 15000 })
167+
.should('exist')
168+
.and('have.length.gt', 5)
169+
.and('contain', '<content>foo</content>')
170+
})
137171
})
138172
})
139173
})

‎cypress/e2e/files_sharing/public-share/setup-public-share.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,13 @@ function checkExpirationDateState(enforced: boolean, hasDefault: boolean) {
7979
cy.get('input[data-cy-files-sharing-expiration-date-input]')
8080
.invoke('val')
8181
.then((val) => {
82+
// eslint-disable-next-line no-unused-expressions
83+
expect(val).to.not.be.undefined
84+
85+
const inputDate = new Date(typeof val === 'number' ? val : String(val))
8286
const expectedDate = new Date()
8387
expectedDate.setDate(expectedDate.getDate() + 2)
84-
expect(new Date(val).toDateString()).to.eq(expectedDate.toDateString())
88+
expect(inputDate.toDateString()).to.eq(expectedDate.toDateString())
8589
})
8690

8791
}

‎dist/files-init.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/files-init.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)