From 66f233153fe16d6ef2796d90c1d5b7b887ccbe4c Mon Sep 17 00:00:00 2001 From: MellyGray Date: Thu, 14 Dec 2023 18:05:57 +0100 Subject: [PATCH] feat(IntegrationMultipleFileDownload): do not zip if only 1 file is selected --- src/files/domain/models/File.ts | 8 +-- .../domain/repositories/FileRepository.ts | 5 +- .../useCases/getMultipleFileDownloadUrl.ts | 5 ++ .../FileJSDataverseRepository.ts | 19 ++++-- .../access-dataset-menu/AccessDatasetMenu.tsx | 18 +++--- .../download-files/DownloadFilesButton.tsx | 3 +- src/stories/files/FileMockRepository.ts | 9 ++- .../dataset/domain/models/DatasetMother.ts | 12 ++-- .../AccessDatasetMenu.spec.tsx | 4 +- .../DownloadFilesButton.spec.tsx | 60 ++++++++++++++++--- .../e2e/sections/dataset/Dataset.spec.tsx | 2 +- 11 files changed, 100 insertions(+), 45 deletions(-) diff --git a/src/files/domain/models/File.ts b/src/files/domain/models/File.ts index 173ae1e0f..ad09ce9f1 100644 --- a/src/files/domain/models/File.ts +++ b/src/files/domain/models/File.ts @@ -63,17 +63,11 @@ export class FileSize { } } -export enum FileDownloadSizeMode { - ALL = 'All', - ORIGINAL = 'Original', - ARCHIVAL = 'Archival' -} - export class FileDownloadSize extends FileSize { constructor( readonly value: number, readonly unit: FileSizeUnit, - readonly mode: FileDownloadSizeMode + readonly mode: FileDownloadMode ) { super(value, unit) } diff --git a/src/files/domain/repositories/FileRepository.ts b/src/files/domain/repositories/FileRepository.ts index 2be3fcd19..9b861084d 100644 --- a/src/files/domain/repositories/FileRepository.ts +++ b/src/files/domain/repositories/FileRepository.ts @@ -1,4 +1,4 @@ -import { File } from '../models/File' +import { File, FileDownloadMode } from '../models/File' import { FileCriteria } from '../models/FileCriteria' import { FilesCountInfo } from '../models/FilesCountInfo' import { FilePaginationInfo } from '../models/FilePaginationInfo' @@ -23,5 +23,6 @@ export interface FileRepository { criteria?: FileCriteria ) => Promise getUserPermissionsById: (id: number) => Promise - getMultipleFileDownloadUrl: (ids: number[], downloadMode: string) => string + getMultipleFileDownloadUrl: (ids: number[], downloadMode: FileDownloadMode) => string + getFileDownloadUrl: (id: number, downloadMode: FileDownloadMode) => string } diff --git a/src/files/domain/useCases/getMultipleFileDownloadUrl.ts b/src/files/domain/useCases/getMultipleFileDownloadUrl.ts index ae6e8015f..fac6afcf3 100644 --- a/src/files/domain/useCases/getMultipleFileDownloadUrl.ts +++ b/src/files/domain/useCases/getMultipleFileDownloadUrl.ts @@ -1,10 +1,15 @@ import { FileRepository } from '../repositories/FileRepository' import { FileDownloadMode } from '../models/File' +const ONLY_ONE_FILE = 1 export function getMultipleFileDownloadUrl( fileRepository: FileRepository, ids: number[], downloadMode: FileDownloadMode ): string { + if (ids.length === ONLY_ONE_FILE) { + return fileRepository.getFileDownloadUrl(ids[0], downloadMode) + } + return fileRepository.getMultipleFileDownloadUrl(ids, downloadMode) } diff --git a/src/files/infrastructure/FileJSDataverseRepository.ts b/src/files/infrastructure/FileJSDataverseRepository.ts index 54b8bb285..96affa553 100644 --- a/src/files/infrastructure/FileJSDataverseRepository.ts +++ b/src/files/infrastructure/FileJSDataverseRepository.ts @@ -1,19 +1,19 @@ import { FileRepository } from '../domain/repositories/FileRepository' -import { File } from '../domain/models/File' +import { File, FileDownloadMode } from '../domain/models/File' import { FilesCountInfo } from '../domain/models/FilesCountInfo' import { FilePaginationInfo } from '../domain/models/FilePaginationInfo' import { FileUserPermissions } from '../domain/models/FileUserPermissions' import { + File as JSFile, + FileDataTable as JSFileTabularData, FileDownloadSizeMode, getDatasetFileCounts, getDatasetFiles, getDatasetFilesTotalDownloadSize, + getFileDataTables, getFileDownloadCount, getFileUserPermissions, - ReadError, - File as JSFile, - getFileDataTables, - FileDataTable as JSFileTabularData + ReadError } from '@iqss/dataverse-client-javascript' import { FileCriteria } from '../domain/models/FileCriteria' import { DomainFileMapper } from './mappers/DomainFileMapper' @@ -156,7 +156,14 @@ export class FileJSDataverseRepository implements FileRepository { }) } - getMultipleFileDownloadUrl(ids: number[], downloadMode: string): string { + getMultipleFileDownloadUrl(ids: number[], downloadMode: FileDownloadMode): string { return `/api/access/datafiles/${ids.join(',')}?format=${downloadMode}` } + + getFileDownloadUrl(id: number, downloadMode: FileDownloadMode): string { + if (downloadMode === FileDownloadMode.ORIGINAL) { + return `/api/access/datafile/${id}?format=${downloadMode}` + } + return `/api/access/datafile/${id}` + } } diff --git a/src/sections/dataset/dataset-action-buttons/access-dataset-menu/AccessDatasetMenu.tsx b/src/sections/dataset/dataset-action-buttons/access-dataset-menu/AccessDatasetMenu.tsx index 23a52d5a5..1b3591596 100644 --- a/src/sections/dataset/dataset-action-buttons/access-dataset-menu/AccessDatasetMenu.tsx +++ b/src/sections/dataset/dataset-action-buttons/access-dataset-menu/AccessDatasetMenu.tsx @@ -6,7 +6,7 @@ import { import { DropdownButton, DropdownButtonItem, DropdownHeader } from '@iqss/dataverse-design-system' import { useTranslation } from 'react-i18next' -import { FileDownloadSize, FileDownloadSizeMode } from '../../../../files/domain/models/File' +import { FileDownloadMode, FileDownloadSize } from '../../../../files/domain/models/File' import { Download } from 'react-bootstrap-icons' interface AccessDatasetMenuProps { @@ -30,12 +30,12 @@ export function AccessDatasetMenu({ return <> } - function getFormattedFileSize(mode: FileDownloadSizeMode): string { + function getFormattedFileSize(mode: FileDownloadMode): string { const foundSize = fileDownloadSizes && fileDownloadSizes.find((size) => size.mode === mode) return foundSize ? foundSize.toString() : '' } - const handleDownload = (type: FileDownloadSizeMode) => { + const handleDownload = (type: FileDownloadMode) => { //TODO: implement download feature console.log('downloading file ' + type) } @@ -47,19 +47,19 @@ export function AccessDatasetMenu({ const DatasetDownloadOptions = ({ datasetContainsTabularFiles }: DatasetDownloadOptionsProps) => { return datasetContainsTabularFiles ? ( <> - handleDownload(FileDownloadSizeMode.ORIGINAL)}> + handleDownload(FileDownloadMode.ORIGINAL)}> {t('datasetActionButtons.accessDataset.downloadOriginalZip')} ( - {getFormattedFileSize(FileDownloadSizeMode.ORIGINAL)}) + {getFormattedFileSize(FileDownloadMode.ORIGINAL)}) - handleDownload(FileDownloadSizeMode.ARCHIVAL)}> + handleDownload(FileDownloadMode.ARCHIVAL)}> {t('datasetActionButtons.accessDataset.downloadArchiveZip')} ( - {getFormattedFileSize(FileDownloadSizeMode.ARCHIVAL)}) + {getFormattedFileSize(FileDownloadMode.ARCHIVAL)}) ) : ( - handleDownload(FileDownloadSizeMode.ORIGINAL)}> + handleDownload(FileDownloadMode.ORIGINAL)}> {t('datasetActionButtons.accessDataset.downloadZip')} ( - {getFormattedFileSize(FileDownloadSizeMode.ORIGINAL)}) + {getFormattedFileSize(FileDownloadMode.ORIGINAL)}) ) } diff --git a/src/sections/dataset/dataset-files/files-table/file-actions/download-files/DownloadFilesButton.tsx b/src/sections/dataset/dataset-files/files-table/file-actions/download-files/DownloadFilesButton.tsx index 83a547672..67e1596ff 100644 --- a/src/sections/dataset/dataset-files/files-table/file-actions/download-files/DownloadFilesButton.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-actions/download-files/DownloadFilesButton.tsx @@ -22,8 +22,9 @@ export function DownloadFilesButton({ files, fileSelection }: DownloadFilesButto const { dataset } = useDataset() const [showNoFilesSelectedModal, setShowNoFilesSelectedModal] = useState(false) const { getMultipleFileDownloadUrl } = useMultipleFileDownload() + const fileSelectionCount = Object.keys(fileSelection).length const onClick = (event: MouseEvent) => { - if (Object.keys(fileSelection).length === SELECTED_FILES_EMPTY) { + if (fileSelectionCount === SELECTED_FILES_EMPTY) { event.preventDefault() setShowNoFilesSelectedModal(true) } diff --git a/src/stories/files/FileMockRepository.ts b/src/stories/files/FileMockRepository.ts index 3086827c5..317facbc4 100644 --- a/src/stories/files/FileMockRepository.ts +++ b/src/stories/files/FileMockRepository.ts @@ -1,6 +1,6 @@ import { FileRepository } from '../../files/domain/repositories/FileRepository' import { FilesMockData } from './FileMockData' -import { File } from '../../files/domain/models/File' +import { File, FileDownloadMode } from '../../files/domain/models/File' import { FilesCountInfo } from '../../files/domain/models/FilesCountInfo' import { FilesCountInfoMother } from '../../../tests/component/files/domain/models/FilesCountInfoMother' import { FilePaginationInfo } from '../../files/domain/models/FilePaginationInfo' @@ -62,7 +62,12 @@ export class FileMockRepository implements FileRepository { } // eslint-disable-next-line unused-imports/no-unused-vars - getMultipleFileDownloadUrl(ids: number[], downloadMode: string): string { + getMultipleFileDownloadUrl(ids: number[], downloadMode: FileDownloadMode): string { + return FileMother.createDownloadUrl() + } + + // eslint-disable-next-line unused-imports/no-unused-vars + getFileDownloadUrl(id: number, downloadMode: FileDownloadMode): string { return FileMother.createDownloadUrl() } } diff --git a/tests/component/dataset/domain/models/DatasetMother.ts b/tests/component/dataset/domain/models/DatasetMother.ts index 09099953d..73d7b18f7 100644 --- a/tests/component/dataset/domain/models/DatasetMother.ts +++ b/tests/component/dataset/domain/models/DatasetMother.ts @@ -13,8 +13,8 @@ import { MetadataBlockName } from '../../../../../src/dataset/domain/models/Dataset' import { + FileDownloadMode, FileDownloadSize, - FileDownloadSizeMode, FileSizeUnit } from '../../../../../src/files/domain/models/File' @@ -199,16 +199,16 @@ export class DatasetFileDownloadSizeMother { return new FileDownloadSize( props?.value ?? faker.datatype.number(), props?.unit ?? faker.helpers.arrayElement(Object.values(FileSizeUnit)), - props?.mode ?? faker.helpers.arrayElement(Object.values(FileDownloadSizeMode)) + props?.mode ?? faker.helpers.arrayElement(Object.values(FileDownloadMode)) ) } static createArchival(): FileDownloadSize { - return this.create({ mode: FileDownloadSizeMode.ARCHIVAL }) + return this.create({ mode: FileDownloadMode.ARCHIVAL }) } static createOriginal(): FileDownloadSize { - return this.create({ mode: FileDownloadSizeMode.ORIGINAL }) + return this.create({ mode: FileDownloadMode.ORIGINAL }) } } @@ -482,8 +482,8 @@ export class DatasetMother { hasValidTermsOfAccess: true, hasOneTabularFileAtLeast: true, fileDownloadSizes: [ - new FileDownloadSize(21.98, FileSizeUnit.KILOBYTES, FileDownloadSizeMode.ORIGINAL), - new FileDownloadSize(21.98, FileSizeUnit.KILOBYTES, FileDownloadSizeMode.ARCHIVAL) + new FileDownloadSize(21.98, FileSizeUnit.KILOBYTES, FileDownloadMode.ORIGINAL), + new FileDownloadSize(21.98, FileSizeUnit.KILOBYTES, FileDownloadMode.ARCHIVAL) ], isValid: true, ...props diff --git a/tests/component/sections/dataset/dataset-action-buttons/access-dataset-menu/AccessDatasetMenu.spec.tsx b/tests/component/sections/dataset/dataset-action-buttons/access-dataset-menu/AccessDatasetMenu.spec.tsx index f085f51d2..9a3ff1e0d 100644 --- a/tests/component/sections/dataset/dataset-action-buttons/access-dataset-menu/AccessDatasetMenu.spec.tsx +++ b/tests/component/sections/dataset/dataset-action-buttons/access-dataset-menu/AccessDatasetMenu.spec.tsx @@ -48,10 +48,10 @@ describe('AccessDatasetMenu', () => { cy.findByRole('button', { name: 'Access Dataset' }).should('exist') cy.findByRole('button', { name: 'Access Dataset' }).click() cy.contains('Original Format ZIP').click() - cy.get('@consoleLog').should('have.been.calledWith', 'downloading file Original') + cy.get('@consoleLog').should('have.been.calledWith', 'downloading file original') cy.findByRole('button', { name: 'Access Dataset' }).click() cy.contains('Archive Format').click() - cy.get('@consoleLog').should('have.been.calledWith', 'downloading file Archival') + cy.get('@consoleLog').should('have.been.calledWith', 'downloading file archival') }) it('renders the AccessDatasetMenu if the user has download files permissions and the dataset is not deaccessioned', () => { diff --git a/tests/component/sections/dataset/dataset-files/files-table/file-actions/download-files/DownloadFilesButton.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/file-actions/download-files/DownloadFilesButton.spec.tsx index 7d8974614..b1b197d1b 100644 --- a/tests/component/sections/dataset/dataset-files/files-table/file-actions/download-files/DownloadFilesButton.spec.tsx +++ b/tests/component/sections/dataset/dataset-files/files-table/file-actions/download-files/DownloadFilesButton.spec.tsx @@ -27,6 +27,12 @@ describe('DownloadFilesButton', () => { ) } + beforeEach(() => { + fileRepository.getMultipleFileDownloadUrl = cy + .stub() + .returns('https://multiple-file-download-url') + }) + it('renders the Download Files button if there is more than 1 file in the dataset and the user has download files permission', () => { const datasetWithDownloadFilesPermission = DatasetMother.create({ permissions: DatasetPermissionsMother.createWithFilesDownloadAllowed() @@ -141,7 +147,6 @@ describe('DownloadFilesButton', () => { 'some-file-id': files[0], 'some-other-file-id': files[1] } - fileRepository.getMultipleFileDownloadUrl = cy.stub().returns('https://some-download-url') cy.mountAuthenticated( {withDataset( @@ -153,7 +158,7 @@ describe('DownloadFilesButton', () => { cy.findByRole('button', { name: 'Download' }) .parent('a') - .should('have.attr', 'href', 'https://some-download-url') + .should('have.attr', 'href', 'https://multiple-file-download-url') }) it('renders the download url for the selected files when some files are selected and there are tabular files', () => { @@ -172,7 +177,6 @@ describe('DownloadFilesButton', () => { 'some-file-id': files[0], 'some-other-file-id': files[1] } - fileRepository.getMultipleFileDownloadUrl = cy.stub().returns('https://some-download-url') cy.mountAuthenticated( {withDataset( @@ -186,12 +190,12 @@ describe('DownloadFilesButton', () => { cy.findByRole('link', { name: 'Original Format' }).should( 'have.attr', 'href', - 'https://some-download-url' + 'https://multiple-file-download-url' ) cy.findByRole('link', { name: 'Archival Format (.tab)' }).should( 'have.attr', 'href', - 'https://some-download-url' + 'https://multiple-file-download-url' ) }) @@ -200,8 +204,8 @@ describe('DownloadFilesButton', () => { permissions: DatasetPermissionsMother.createWithFilesDownloadAllowed(), hasOneTabularFileAtLeast: true, downloadUrls: { - original: 'https://some-download-url', - archival: 'https://some-download-url' + original: 'https://dataset-download-url-original', + archival: 'https://dataset-download-url-archival' } }) const files = FileMother.createMany(2, { @@ -226,12 +230,50 @@ describe('DownloadFilesButton', () => { cy.findByRole('link', { name: 'Original Format' }).should( 'have.attr', 'href', - 'https://some-download-url' + 'https://dataset-download-url-original' + ) + cy.findByRole('link', { name: 'Archival Format (.tab)' }).should( + 'have.attr', + 'href', + 'https://dataset-download-url-archival' + ) + }) + + it('renders the dataset download url with the single file download url when one file is selected', () => { + const datasetWithDownloadFilesPermission = DatasetMother.create({ + permissions: DatasetPermissionsMother.createWithFilesDownloadAllowed(), + hasOneTabularFileAtLeast: true + }) + const files = FileMother.createMany(2, { + tabularData: { + variablesCount: 2, + observationsCount: 3, + unf: 'some-unf' + } + }) + const fileSelection = { + 'some-file-id': files[0] + } + fileRepository.getFileDownloadUrl = cy.stub().returns('https://single-file-download-url') + cy.mountAuthenticated( + + {withDataset( + , + datasetWithDownloadFilesPermission + )} + + ) + + cy.findByRole('button', { name: 'Download' }).click() + cy.findByRole('link', { name: 'Original Format' }).should( + 'have.attr', + 'href', + 'https://single-file-download-url' ) cy.findByRole('link', { name: 'Archival Format (.tab)' }).should( 'have.attr', 'href', - 'https://some-download-url' + 'https://single-file-download-url' ) }) }) diff --git a/tests/e2e-integration/e2e/sections/dataset/Dataset.spec.tsx b/tests/e2e-integration/e2e/sections/dataset/Dataset.spec.tsx index 327a655af..c7bc50488 100644 --- a/tests/e2e-integration/e2e/sections/dataset/Dataset.spec.tsx +++ b/tests/e2e-integration/e2e/sections/dataset/Dataset.spec.tsx @@ -372,7 +372,7 @@ describe('Dataset', () => { }) }) - it.only('applies filters to the Files Table in the correct order', () => { + it('applies filters to the Files Table in the correct order', () => { const files = [ FileHelper.create('csv', { description: 'Some description',