Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(files_sharing): ensure downloaded file has the correct filename #51152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(files_sharing): ensure downloaded file has the correct filename #51152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 21 additions & 2 deletions apps/files/src/actions/downloadAction.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe('Download action execute tests', () => {

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

// Silent action
expect(exec).toStrictEqual([null])
expect(link.download).toEqual('')
expect(link.download).toEqual('foobar.txt')
expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
expect(link.click).toHaveBeenCalledTimes(1)
})

test('Download single file with displayname set', async () => {
const file = new File({
id: 1,
source: 'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
owner: 'admin',
mime: 'text/plain',
displayname: 'baz.txt',
permissions: Permission.READ,
})

const exec = await action.execBatch!([file], view, '/')

// Silent action
expect(exec).toStrictEqual([null])
expect(link.download).toEqual('baz.txt')
expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
expect(link.click).toHaveBeenCalledTimes(1)
})
Expand Down
12 changes: 9 additions & 3 deletions apps/files/src/actions/downloadAction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ import { isDownloadable } from '../utils/permissions'

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

const triggerDownload = function(url: string) {
/**
* Trigger downloading a file.
*
* @param url The url of the asset to download
* @param name Optionally the recommended name of the download (browsers might ignore it)
*/
function triggerDownload(url: string, name?: string) {
const hiddenElement = document.createElement('a')
hiddenElement.download = ''
hiddenElement.download = name ?? ''
hiddenElement.href = url
hiddenElement.click()
}
Expand Down Expand Up @@ -42,7 +48,7 @@ const downloadNodes = function(nodes: Node[]) {

if (nodes.length === 1) {
if (nodes[0].type === FileType.File) {
return triggerDownload(nodes[0].encodedSource)
return triggerDownload(nodes[0].encodedSource, nodes[0].displayname)
} else {
url = new URL(nodes[0].encodedSource)
url.searchParams.append('accept', 'zip')
Expand Down
239 changes: 136 additions & 103 deletions cypress/e2e/files_sharing/public-share/download-files.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,136 +4,169 @@
*/
// @ts-expect-error The package is currently broken - but works...
import { deleteDownloadsFolderBeforeEach } from 'cypress-delete-downloads-folder'

import { createShare, getShareUrl, setupPublicShare, type ShareContext } from './setup-public-share.ts'
import { getRowForFile, getRowForFileId, triggerActionForFile, triggerActionForFileId } from '../../files/FilesUtils.ts'
import { zipFileContains } from '../../../support/utils/assertions.ts'
import { getRowForFile, triggerActionForFile } from '../../files/FilesUtils.ts'
import { getShareUrl, setupPublicShare } from './setup-public-share.ts'

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

before(() => setupPublicShare())

deleteDownloadsFolderBeforeEach()

beforeEach(() => {
cy.logout()
cy.visit(getShareUrl())
})

it('Can download all files', () => {
getRowForFile('foo.txt').should('be.visible')

cy.get('[data-cy-files-list]').within(() => {
cy.findByRole('checkbox', { name: /Toggle selection for all files/i })
.should('exist')
.check({ force: true })

// see that two files are selected
cy.contains('2 selected').should('be.visible')
// in general there is no difference except downloading
// as file shares have the source of the share token but a different displayname
describe('file share', () => {
let fileId: number

before(() => {
cy.createRandomUser().then((user) => {
const context: ShareContext = { user }
cy.uploadContent(user, new Blob(['<content>foo</content>']), 'text/plain', '/file.txt')
.then(({ headers }) => { fileId = Number.parseInt(headers['oc-fileid']) })
cy.login(user)
createShare(context, 'file.txt')
.then(() => cy.logout())
.then(() => cy.visit(context.url!))
})
})

// click download
cy.findByRole('button', { name: 'Download (selected)' })
it('can download the file', () => {
getRowForFileId(fileId)
.should('be.visible')
.click()

// check a file is downloaded
.find('[data-cy-files-list-row-name]')
.should((el) => expect(el.text()).to.match(/file\s*\.txt/)) // extension is sparated so there might be a space between
triggerActionForFileId(fileId, 'download')
// check a file is downloaded with the correct name
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/download.zip`, null, { timeout: 15000 })
cy.readFile(`${downloadsFolder}/file.txt`, 'utf-8', { timeout: 15000 })
.should('exist')
.and('have.length.gt', 30)
// Check all files are included
.and(zipFileContains([
'foo.txt',
'subfolder/',
'subfolder/bar.txt',
]))
.and('have.length.gt', 5)
.and('contain', '<content>foo</content>')
})
})

it('Can download selected files', () => {
getRowForFile('subfolder')
.should('be.visible')
describe('folder share', () => {
before(() => setupPublicShare())

cy.get('[data-cy-files-list]').within(() => {
getRowForFile('subfolder')
.findByRole('checkbox')
.check({ force: true })
deleteDownloadsFolderBeforeEach()

beforeEach(() => {
cy.logout()
cy.visit(getShareUrl())
})

// see that two files are selected
cy.contains('1 selected').should('be.visible')
it('Can download all files', () => {
getRowForFile('foo.txt').should('be.visible')

cy.get('[data-cy-files-list]').within(() => {
cy.findByRole('checkbox', { name: /Toggle selection for all files/i })
.should('exist')
.check({ force: true })

// see that two files are selected
cy.contains('2 selected').should('be.visible')

// click download
cy.findByRole('button', { name: 'Download (selected)' })
.should('be.visible')
.click()

// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/download.zip`, null, { timeout: 15000 })
.should('exist')
.and('have.length.gt', 30)
// Check all files are included
.and(zipFileContains([
'foo.txt',
'subfolder/',
'subfolder/bar.txt',
]))
})
})

// click download
cy.findByRole('button', { name: 'Download (selected)' })
it('Can download selected files', () => {
getRowForFile('subfolder')
.should('be.visible')
.click()

// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/subfolder.zip`, null, { timeout: 15000 })
.should('exist')
.and('have.length.gt', 30)
// Check all files are included
.and(zipFileContains([
'subfolder/',
'subfolder/bar.txt',
]))
cy.get('[data-cy-files-list]').within(() => {
getRowForFile('subfolder')
.findByRole('checkbox')
.check({ force: true })

// see that two files are selected
cy.contains('1 selected').should('be.visible')

// click download
cy.findByRole('button', { name: 'Download (selected)' })
.should('be.visible')
.click()

// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/subfolder.zip`, null, { timeout: 15000 })
.should('exist')
.and('have.length.gt', 30)
// Check all files are included
.and(zipFileContains([
'subfolder/',
'subfolder/bar.txt',
]))
})
})
})

it('Can download folder by action', () => {
getRowForFile('subfolder')
.should('be.visible')

cy.get('[data-cy-files-list]').within(() => {
triggerActionForFile('subfolder', 'download')
it('Can download folder by action', () => {
getRowForFile('subfolder')
.should('be.visible')

// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/subfolder.zip`, null, { timeout: 15000 })
.should('exist')
.and('have.length.gt', 30)
// Check all files are included
.and(zipFileContains([
'subfolder/',
'subfolder/bar.txt',
]))
cy.get('[data-cy-files-list]').within(() => {
triggerActionForFile('subfolder', 'download')

// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/subfolder.zip`, null, { timeout: 15000 })
.should('exist')
.and('have.length.gt', 30)
// Check all files are included
.and(zipFileContains([
'subfolder/',
'subfolder/bar.txt',
]))
})
})
})

it('Can download file by action', () => {
getRowForFile('foo.txt')
.should('be.visible')
it('Can download file by action', () => {
getRowForFile('foo.txt')
.should('be.visible')

cy.get('[data-cy-files-list]').within(() => {
triggerActionForFile('foo.txt', 'download')
cy.get('[data-cy-files-list]').within(() => {
triggerActionForFile('foo.txt', 'download')

// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/foo.txt`, 'utf-8', { timeout: 15000 })
.should('exist')
.and('have.length.gt', 5)
.and('contain', '<content>foo</content>')
// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/foo.txt`, 'utf-8', { timeout: 15000 })
.should('exist')
.and('have.length.gt', 5)
.and('contain', '<content>foo</content>')
})
})
})

it('Can download file by selection', () => {
getRowForFile('foo.txt')
.should('be.visible')

cy.get('[data-cy-files-list]').within(() => {
it('Can download file by selection', () => {
getRowForFile('foo.txt')
.findByRole('checkbox')
.check({ force: true })

cy.findByRole('button', { name: 'Download (selected)' })
.click()
.should('be.visible')

// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/foo.txt`, 'utf-8', { timeout: 15000 })
.should('exist')
.and('have.length.gt', 5)
.and('contain', '<content>foo</content>')
cy.get('[data-cy-files-list]').within(() => {
getRowForFile('foo.txt')
.findByRole('checkbox')
.check({ force: true })

cy.findByRole('button', { name: 'Download (selected)' })
.click()

// check a file is downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(`${downloadsFolder}/foo.txt`, 'utf-8', { timeout: 15000 })
.should('exist')
.and('have.length.gt', 5)
.and('contain', '<content>foo</content>')
})
})
})
})
6 changes: 5 additions & 1 deletion cypress/e2e/files_sharing/public-share/setup-public-share.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,13 @@ function checkExpirationDateState(enforced: boolean, hasDefault: boolean) {
cy.get('input[data-cy-files-sharing-expiration-date-input]')
.invoke('val')
.then((val) => {
// eslint-disable-next-line no-unused-expressions
expect(val).to.not.be.undefined

const inputDate = new Date(typeof val === 'number' ? val : String(val))
const expectedDate = new Date()
expectedDate.setDate(expectedDate.getDate() + 2)
expect(new Date(val).toDateString()).to.eq(expectedDate.toDateString())
expect(inputDate.toDateString()).to.eq(expectedDate.toDateString())
})

}
Expand Down
Loading