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.

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

Add more information for users on invalid files #987

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
76 changes: 76 additions & 0 deletions src/__test__/components/data-management/FileUploadModal.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
35 changes: 35 additions & 0 deletions src/components/Expandable.jsx
Original file line number Diff line number Diff line change
@@ -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
<span
onClick={() => {
if (!isExpanded) {
setIsExpanded(true);
}
}}
>
{!isExpanded ? collapsedContent
: (
<>
{expandedContent}
</>
)}
</span>
);
};

Expandable.propTypes = {
expandedContent: PropTypes.node.isRequired,
collapsedContent: PropTypes.node.isRequired,
};

Expandable.defaultProps = {};

export default Expandable;
93 changes: 77 additions & 16 deletions src/components/data-management/FileUploadModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -54,6 +59,8 @@ const extraHelpText = {
[sampleTech.PARSE]: () => <></>,
};

const emptyFiles = { valid: [], invalid: [] };

const FileUploadModal = (props) => {
const { onUpload, onCancel, currentSelectedTech } = props;

Expand All @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are always obtained and updated together, so it seemed to make sense to put invalid in the same useState


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
Expand All @@ -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);
}
Expand All @@ -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];
Expand Down Expand Up @@ -165,8 +178,8 @@ const FileUploadModal = (props) => {
block
disabled={!canUpload}
onClick={() => {
onUpload(filesList, selectedTech);
setFilesList([]);
onUpload(files.valid, selectedTech);
setFiles(emptyFiles);
}}
>
Upload
Expand Down Expand Up @@ -267,12 +280,11 @@ const FileUploadModal = (props) => {
<Row>
<Col span={24}>
{/* eslint-enable react/jsx-props-no-spreading */}

{filesList.length ? (
{files.valid.length ? (
<>
<Divider orientation='center'>To upload</Divider>
<List
dataSource={filesList}
dataSource={files.valid}
size='small'
itemLayout='horizontal'
grid='{column: 4}'
Expand Down Expand Up @@ -305,6 +317,55 @@ const FileUploadModal = (props) => {
/>
</>
) : ''}
{files.invalid.length > 0 && (
<Expandable
style={{ width: '100%' }}
expandedContent={(
<>
<Divider orientation='center' style={{ color: 'red', marginBottom: '0' }}>Ignored files</Divider>
<List
dataSource={files.invalid}
size='small'
itemLayout='horizontal'
pagination
renderItem={(file) => (
<List.Item key={file.path} style={{ height: '100%', width: '100%' }}>
<Space style={{ width: 200, justifyContent: 'center' }}>
<CloseCircleTwoTone twoToneColor='#f5222d' />
<div style={{ width: 200 }}>
<Text
ellipsis={{ tooltip: _.trim(file.path, '/') }}
>
{_.trim(file.path, '/')}
</Text>
</div>
</Space>
<Text style={{ width: '100%', marginLeft: '50px' }}>{file.rejectReason}</Text>
</List.Item>
)}
/>
</>
)}
collapsedContent={(
<center style={{ cursor: 'pointer' }}>
<Divider orientation='center' style={{ color: 'red' }} />
<Text type='danger'>
{' '}
<WarningOutlined />
{' '}
</Text>
<Text>
{files.invalid.length}
{' '}
file
{files.invalid.length > 1 ? 's were' : ' was'}
{' '}
ignored, click to display
</Text>
</center>
)}
/>
)}
</Col>
</Row>
</Modal>
Expand Down
Loading
Loading