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

Single file shares use the share token as source name, so we need to use
the displayname. To do so we need to set the download attribute to the
displayname of the file to download.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

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

@@ -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)
 	})
@@ -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)
 	})

apps/files/src/actions/downloadAction.ts

@@ -9,9 +9,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()
 }
@@ -43,7 +49,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')

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

@@ -4,136 +4,170 @@
  */
 // @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())
+	// 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
 
-	deleteDownloadsFolderBeforeEach()
+		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!))
+			})
+		})
 
-	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')
-
-			// click download
-			cy.findByRole('button', { name: 'Download (selected)' })
+		it('can download the file', () => {
+			getRowForFileId(fileId)
 				.should('be.visible')
-				.click()
-
-			// check a file is downloaded
+			getRowForFileId(fileId)
+				.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(() => {
+		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')
+
+				// 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',
+					]))
+			})
+		})
+
+		it('Can download selected files', () => {
 			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',
-				]))
+			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')
+		it('Can download folder by action', () => {
+			getRowForFile('subfolder')
+				.should('be.visible')
 
-		cy.get('[data-cy-files-list]').within(() => {
-			triggerActionForFile('subfolder', 'download')
+			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',
-				]))
+				// 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')
-
-		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>')
-		})
-	})
-
-	it('Can download file by selection', () => {
-		getRowForFile('foo.txt')
-			.should('be.visible')
-
-		cy.get('[data-cy-files-list]').within(() => {
+		it('Can download file by action', () => {
 			getRowForFile('foo.txt')
-				.findByRole('checkbox')
-				.check({ force: true })
+				.should('be.visible')
 
-			cy.findByRole('button', { name: 'Download (selected)' })
-				.click()
+			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(() => {
+				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>')
+			})
 		})
 	})
 })

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

@@ -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())
 		})
 
 }