diff --git a/src/__test__/components/data-management/FileUploadModal.test.jsx b/src/__test__/components/data-management/FileUploadModal.test.jsx index 58b99cd8c..e28ab8aeb 100644 --- a/src/__test__/components/data-management/FileUploadModal.test.jsx +++ b/src/__test__/components/data-management/FileUploadModal.test.jsx @@ -428,4 +428,80 @@ describe('FileUploadModal', () => { // Upload is enabled because the file is considered valid expect(uploadButton).not.toBeDisabled(); }); + + it('drag and drop works with Parse file, invalid files are shown in warning', async () => { + await renderFileUploadModal(initialStore); + + // Switch file upload to Parse + selectTech(sampleTech.PARSE); + + const uploadInput = document.querySelector( + `[data-test-id="${integrationTestConstants.ids.FILE_UPLOAD_INPUT}"]`, + ); + + // create an all genes file (features) + const files = [ + // 4 valid files + mockFile('all_genes.csv.gz', '/WT13/DGE_unfiltered'), + mockFile('all_genes.csv.gz', '/WT14/DGE_filtered'), + mockFile('all_genes.csv.gz', '/WT15'), + // This one should be ignored, /WT13/DGE_unfiltered takes precedence + mockFile('all_genes.csv.gz', '/WT13/DGE_filtered'), + // 1 invalid file + mockFile('all_genes.csv.gz', '/all-sample'), + ]; + + // drop it into drop-zone + await act(async () => { + Object.defineProperty(uploadInput, 'files', { + value: files, + configurable: true, + }); + + fireEvent.drop(uploadInput); + }); + + // Valid files show up + expect(await screen.findByText(/To upload/)).toBeInTheDocument(); + expect(await screen.findAllByText('WT13/DGE_unfiltered/all_genes.csv.gz')).toHaveLength(2); + expect(await screen.findByText('WT14/DGE_filtered/all_genes.csv.gz')).toBeInTheDocument(); + expect(await screen.findByText('WT15/all_genes.csv.gz')).toBeInTheDocument(); + + // WT13/DGE_filtered doesn't because it is valid but superseded by WT13/DGE_unfiltered + expect(screen.queryByText('WT13/DGE_filtered/all_genes.csv.gz')).not.toBeInTheDocument(); + + // Warning shows up with offer to expand + expect(screen.getByText(/1 file was ignored, click to display/i)).toBeInTheDocument(); + + // Add a new invalid file + await act(async () => { + Object.defineProperty(uploadInput, 'files', { + value: [mockFile('invalidName.csv.gz', 'KO/DGE_unfiltered')], + configurable: true, + }); + + fireEvent.drop(uploadInput); + }); + + // Now warning shows plural version + expect(screen.getByText(/2 files were ignored, click to display/i)).toBeInTheDocument(); + + // But invalid files don't show up yet + expect(screen.queryByText('all-sample/all_genes.csv.gz')).not.toBeInTheDocument(); + expect(screen.queryByText('KO/DGE_unfiltered/invalidName.csv.gz')).not.toBeInTheDocument(); + + await act(() => { + fireEvent.click(screen.getByText(/2 files were ignored, click to display/i)); + }); + + // All invalid files are shown here + expect(screen.getByText('all-sample/all_genes.csv.gz')).toBeInTheDocument(); + expect(screen.getByText('KO/DGE_unfiltered/invalidName.csv.gz')).toBeInTheDocument(); + + const uploadButtonText = screen.getAllByText(/Upload/i).pop(); + const uploadButton = uploadButtonText.closest('button'); + + // Upload is enabled because the file is considered valid + expect(uploadButton).not.toBeDisabled(); + }); }); diff --git a/src/components/Expandable.jsx b/src/components/Expandable.jsx new file mode 100644 index 000000000..c330ba8ac --- /dev/null +++ b/src/components/Expandable.jsx @@ -0,0 +1,35 @@ +import React, { useState } from 'react'; +import PropTypes from 'prop-types'; + +const Expandable = (props) => { + const { expandedContent, collapsedContent } = props; + + const [isExpanded, setIsExpanded] = useState(false); + + return ( + // eslint-disable-next-line jsx-a11y/click-events-have-key-events + { + if (!isExpanded) { + setIsExpanded(true); + } + }} + > + {!isExpanded ? collapsedContent + : ( + <> + {expandedContent} + + )} + + ); +}; + +Expandable.propTypes = { + expandedContent: PropTypes.node.isRequired, + collapsedContent: PropTypes.node.isRequired, +}; + +Expandable.defaultProps = {}; + +export default Expandable; diff --git a/src/components/data-management/FileUploadModal.jsx b/src/components/data-management/FileUploadModal.jsx index 401bf62ed..7b9562bff 100644 --- a/src/components/data-management/FileUploadModal.jsx +++ b/src/components/data-management/FileUploadModal.jsx @@ -14,11 +14,16 @@ import { List, Tooltip, } from 'antd'; -import { CheckCircleTwoTone, CloseCircleTwoTone, DeleteOutlined } from '@ant-design/icons'; +import { + CheckCircleTwoTone, CloseCircleTwoTone, DeleteOutlined, WarningOutlined, +} from '@ant-design/icons'; import Dropzone from 'react-dropzone'; import { useSelector } from 'react-redux'; import config from 'config'; + +import Expandable from 'components/Expandable'; + import { sampleTech } from 'utils/constants'; import fileUploadUtils, { techNamesToDisplay } from 'utils/upload/fileUploadUtils'; import handleError from 'utils/http/handleError'; @@ -54,6 +59,8 @@ const extraHelpText = { [sampleTech.PARSE]: () => <>, }; +const emptyFiles = { valid: [], invalid: [] }; + const FileUploadModal = (props) => { const { onUpload, onCancel, currentSelectedTech } = props; @@ -64,14 +71,14 @@ const FileUploadModal = (props) => { const [selectedTech, setSelectedTech] = useState(currentSelectedTech ?? sampleTech['10X']); const [canUpload, setCanUpload] = useState(false); - const [filesList, setFilesList] = useState([]); + const [files, setFiles] = useState(emptyFiles); useEffect(() => { - setCanUpload(filesList.length && filesList.every((file) => !file.errors)); - }, [filesList]); + setCanUpload(files.valid.length && files.valid.every((file) => !file.errors)); + }, [files]); useEffect(() => { - setFilesList([]); + setFiles(emptyFiles); }, [selectedTech]); // Handle on Drop @@ -92,7 +99,7 @@ const FileUploadModal = (props) => { return; } - const allFiles = [...filesList, ...newFiles]; + const allFiles = [...files.valid, ...newFiles]; if (allFiles.length > 1) { handleError('error', endUserMessages.ERROR_SEURAT_MULTIPLE_FILES); } @@ -103,20 +110,26 @@ const FileUploadModal = (props) => { return; } - setFilesList([seuratFile]); + setFiles({ valid: [seuratFile], invalid: [] }); } else { - const newFiles = await fileUploadUtils[selectedTech].filterFiles(filteredFiles); + const { + valid: newFiles, + invalid, + } = await fileUploadUtils[selectedTech].filterFiles(filteredFiles); - setFilesList([...filesList, ...newFiles]); + setFiles({ + valid: [...files.valid, ...newFiles], + invalid: [...files.invalid, ...invalid], + }); } }; const removeFile = (fileName) => { - const newArray = _.cloneDeep(filesList); + const newArray = _.cloneDeep(files.valid); const fileIdx = newArray.findIndex((file) => file.name === fileName); newArray.splice(fileIdx, 1); - setFilesList(newArray); + setFiles({ valid: newArray, invalid: files.invalid }); }; const { fileUploadParagraphs, dropzoneText, webkitdirectory } = fileUploadUtils[selectedTech]; @@ -165,8 +178,8 @@ const FileUploadModal = (props) => { block disabled={!canUpload} onClick={() => { - onUpload(filesList, selectedTech); - setFilesList([]); + onUpload(files.valid, selectedTech); + setFiles(emptyFiles); }} > Upload @@ -267,12 +280,11 @@ const FileUploadModal = (props) => { {/* eslint-enable react/jsx-props-no-spreading */} - - {filesList.length ? ( + {files.valid.length ? ( <> To upload { /> ) : ''} + {files.invalid.length > 0 && ( + + Ignored files + ( + + + +
+ + {_.trim(file.path, '/')} + +
+ + {file.rejectReason} + + )} + /> + + )} + collapsedContent={( +
+ + + {' '} + + {' '} + + + {files.invalid.length} + {' '} + file + {files.invalid.length > 1 ? 's were' : ' was'} + {' '} + ignored, click to display + +
+ )} + /> + )} diff --git a/src/utils/upload/fileUploadUtils.js b/src/utils/upload/fileUploadUtils.js index 46d04d80d..d8cb8010b 100644 --- a/src/utils/upload/fileUploadUtils.js +++ b/src/utils/upload/fileUploadUtils.js @@ -41,9 +41,15 @@ const filterFilesDefaultConstructor = (selectedTech) => async (files) => { handleError('error', endUserMessages.ERROR_FILES_FOLDER); } - return await Promise.all(filteredFiles.map((file) => ( - fileObjectToFileRecord(file, selectedTech) - ))); + const invalidFiles = _.difference(files, filteredFiles) + .map((file) => ({ path: file.path, rejectReason: 'Invalid file path. Check the instructions in the modal for more information' })); + + return { + valid: await Promise.all(filteredFiles.map((file) => ( + fileObjectToFileRecord(file, selectedTech) + ))), + invalid: invalidFiles, + }; }; const getFilePathToDisplayDefaultConstructor = (selectedTech) => (filePath) => ( @@ -217,12 +223,12 @@ const fileUploadUtils = { // Returns the same list of files // The valid ones are in a dictionary ordered by their sample names // The invalid ones are in a list - const getFilesMatching = (dirNameDGE, filesToFilter) => { + const getFilesMatching = (middlePath, filesToFilter) => { const validFiles = {}; const invalidFiles = []; const regexes = Array.from(parseUtils.acceptedFiles).map((validFileName) => ( - new RegExp(`${sampleNameMatcher}/${dirNameDGE}/${validFileName}$`) + new RegExp(`${sampleNameMatcher}/${middlePath}${validFileName}$`) )); filesToFilter.forEach((fileObject) => { @@ -247,30 +253,74 @@ const fileUploadUtils = { return { valid: validFiles, invalid: invalidFiles }; }; - const { valid: DGEUnfilteredFiles, invalid } = getFilesMatching('DGE_unfiltered', files); - const { valid: DGEFilteredFiles } = getFilesMatching('DGE_filtered', invalid); + const dgeUnfilteredFiles = getFilesMatching('DGE_unfiltered/', files); + const dgeFilteredFiles = getFilesMatching('DGE_filtered/', dgeUnfilteredFiles.invalid); + const noMiddlePathFiles = getFilesMatching('', dgeFilteredFiles.invalid); - const filesToUpload = _.uniq([...Object.keys(DGEFilteredFiles), ...Object.keys(DGEUnfilteredFiles)]) + // These are the ones that didn't match any of the 3 accepted shapes + const invalidFiles = noMiddlePathFiles.invalid.map((file) => ({ + path: file.path, + rejectReason: 'Invalid file path. It should be in the form "sample/DGE_unfiltered/file", "sample/DGE_filtered/file" or "sample/file".', + })); + + const filesToUpload = _.uniq([ + ...Object.entries(dgeFilteredFiles.valid), + ...Object.entries(dgeUnfilteredFiles.valid), + ...Object.entries(noMiddlePathFiles.valid)]) // Only allow sample-specific files, not all samples in one files - .filter((sampleName) => !['all-sample', 'all-well', 'All Wells'].includes(sampleName)) - // if unfiltered files are present, pick them. Otherwise, pick the filtered files - .flatMap((sampleName) => ( - DGEUnfilteredFiles[sampleName] ?? DGEFilteredFiles[sampleName] + .filter(([sampleName, sampleFiles]) => { + const accepted = !['all-sample', 'all-well', 'All Wells'].includes(sampleName); + + if (!accepted) { + invalidFiles.push( + ...sampleFiles.map((file) => ({ + path: file.path, + rejectReason: 'Uploading files in "all-sample", "all-well" and "All Wells" is not supported currently.', + })), + ); + } + + return accepted; + }) + .flatMap(([sampleName]) => ( + // By order of priority + dgeUnfilteredFiles.valid[sampleName] ?? dgeFilteredFiles.valid[sampleName] ?? noMiddlePathFiles.valid[sampleName] )); - return await Promise.all(filesToUpload.map((file) => ( - fileObjectToFileRecord(file, sampleTech.PARSE) - ))); + return { + valid: await Promise.all(filesToUpload.map((file) => ( + fileObjectToFileRecord(file, sampleTech.PARSE) + ))), + invalid: invalidFiles, + }; }, getFilePathToDisplay: (filePath) => { - const [sample, filteredState, name] = _.takeRight(_.trim(filePath, '/').split('/'), 3); + const { sample, filteredState, name } = fileUploadUtils[sampleTech.PARSE].getFileSampleAndName(filePath); + + if (filteredState) { + return [sample, filteredState, name].join('/'); + } - return [sample, filteredState, name].join('/'); + return [sample, name].join('/'); }, getFileSampleAndName: (filePath) => { - const [sample, , name] = _.takeRight(filePath.split('/'), 3); - - return { sample, name }; + const splitFilePath = _.takeRight(_.trim(filePath, '/').split('/'), 3); + + let sample; + let filteredState; + let name; + + // Path can take one of two accepted shapes: + // - sample//file + // - sample/file + if (['DGE_unfiltered', 'DGE_filtered'].includes(splitFilePath[1])) { + [sample, filteredState, name] = splitFilePath; + } else { + // splitFilePath might be length 2 or 3, so use takeRight + [sample, name] = _.takeRight(splitFilePath, 2); + } + + return { sample, filteredState, name }; }, }, };