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

Improve ux parse upload #972

Merged
merged 2 commits into from
Jan 17, 2024
Merged

Improve ux parse upload #972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8cf1a3
Rename fileUploadSpecifications to be more accurate
cosa65 Jan 16, 2024
ca1eaf8
Improve ux of parse upload
cosa65 Jan 16, 2024
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
118 changes: 31 additions & 87 deletions src/__test__/components/data-management/FileUploadModal.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import {
render, screen, waitFor, fireEvent,
render, screen, waitFor, fireEvent, within,
} from '@testing-library/react';
import '@testing-library/jest-dom';
import { act } from 'react-dom/test-utils';
Expand All @@ -11,7 +11,7 @@ import configureMockStore from 'redux-mock-store';
import handleError from 'utils/http/handleError';
import endUserMessages from 'utils/endUserMessages';

import techOptions, { techNamesToDisplay } from 'utils/upload/fileUploadSpecifications';
import techOptions, { techNamesToDisplay } from 'utils/upload/fileUploadUtils';
import { sampleTech } from 'utils/constants';

import mockFile from '__test__/test-utils/mockFile';
Expand Down Expand Up @@ -64,15 +64,30 @@ const renderFileUploadModal = async (store, currentSelectedTech = null) => {
)));
};

const chromiumTech = techNamesToDisplay[sampleTech['10X']];
const seuratTech = techNamesToDisplay[sampleTech.SEURAT];

const selectTech = (selectedTech) => {
const displayedName = techNamesToDisplay[selectedTech];

const techSelection = screen.getByRole('combobox', { name: 'sampleTechnologySelect' });

act(() => {
fireEvent.change(techSelection, { target: { value: selectedTech } });
});

// The 2nd option is the selection
const option = screen.getByText(displayedName, { selector: '.ant-select-item-option-content' });

act(() => {
fireEvent.click(option);
});
};

describe('FileUploadModal', () => {
it('contains required components for Chromium 10X', async () => {
await renderFileUploadModal(initialStore);

// It has a select button to select technology
expect(screen.getByText(chromiumTech)).toBeInTheDocument();
selectTech(sampleTech['10X']);

// It contains instructions on what files can be uploaded
expect(screen.getByText(/For each sample, upload a folder containing/i)).toBeInTheDocument();
Expand Down Expand Up @@ -107,26 +122,11 @@ describe('FileUploadModal', () => {
it('contains required components for Seurat', async () => {
await renderFileUploadModal(initialStore);

// It has default chromium selected
expect(screen.queryAllByText(chromiumTech).length).toBe(1);
// It has default 10x selected
expect(screen.queryAllByText(techNamesToDisplay[sampleTech['10X']]).length).toBe(1);
expect(screen.queryAllByText(seuratTech).length).toBe(0);

// Switch file upload to Seurat
const chosenTech = sampleTech.SEURAT;
const displayedName = techNamesToDisplay[chosenTech];

const techSelection = screen.getByRole('combobox', { name: 'sampleTechnologySelect' });

act(() => {
fireEvent.change(techSelection, { target: { value: sampleTech.SEURAT } });
});

// The 2nd option is the selection
const option = screen.getByText(displayedName);

act(() => {
fireEvent.click(option);
});
selectTech(sampleTech.SEURAT);

// Lists the requirements of the Seurat object
expect(screen.getByText(/The Seurat object must contain the following slots and metadata:/i)).toBeInTheDocument();
Expand Down Expand Up @@ -167,6 +167,8 @@ describe('FileUploadModal', () => {
it('drag and drop works with valid 10X Chromium file', async () => {
await renderFileUploadModal(initialStore);

selectTech(sampleTech['10X']);

expect(await screen.queryByText(/To upload/)).not.toBeInTheDocument();
// It has a select button to select technology
const uploadInput = document.querySelector(
Expand Down Expand Up @@ -201,22 +203,7 @@ describe('FileUploadModal', () => {
it('drag and drop works with valid Seurat file', async () => {
await renderFileUploadModal(initialStore);

// Switch file upload to Seurat
const chosenTech = sampleTech.SEURAT;
const displayedName = techNamesToDisplay[chosenTech];

const techSelection = screen.getByRole('combobox', { name: 'sampleTechnologySelect' });

act(() => {
fireEvent.change(techSelection, { target: { value: sampleTech.SEURAT } });
});

// The 2nd option is the selection
const option = screen.getByText(displayedName);

act(() => {
fireEvent.click(option);
});
selectTech(sampleTech.SEURAT);

expect(await screen.queryByText(/To upload/)).not.toBeInTheDocument();

Expand Down Expand Up @@ -253,22 +240,7 @@ describe('FileUploadModal', () => {
it('drag and drop works with valid Seurat file when different experiment has valid uploaded Seurat object', async () => {
await renderFileUploadModal(prevUpDiffExpStore);

// Switch file upload to Seurat
const chosenTech = sampleTech.SEURAT;
const displayedName = techNamesToDisplay[chosenTech];

const techSelection = screen.getByRole('combobox', { name: 'sampleTechnologySelect' });

act(() => {
fireEvent.change(techSelection, { target: { value: sampleTech.SEURAT } });
});

// The 2nd option is the selection
const option = screen.getByText(displayedName);

act(() => {
fireEvent.click(option);
});
selectTech(sampleTech.SEURAT);

expect(await screen.queryByText(/To upload/)).not.toBeInTheDocument();

Expand Down Expand Up @@ -312,22 +284,7 @@ describe('FileUploadModal', () => {

expect(techInput).toBeEnabled();

// Switch file upload to Seurat
const chosenTech = sampleTech.SEURAT;
const displayedName = techNamesToDisplay[chosenTech];

const techSelection = screen.getByRole('combobox', { name: 'sampleTechnologySelect' });

act(() => {
fireEvent.change(techSelection, { target: { value: sampleTech.SEURAT } });
});

// The 2nd option is the selection
const option = screen.getByText(displayedName);

act(() => {
fireEvent.click(option);
});
selectTech(sampleTech.SEURAT);

expect(await screen.queryByText(/To upload/)).not.toBeInTheDocument();

Expand Down Expand Up @@ -436,34 +393,21 @@ describe('FileUploadModal', () => {
await renderFileUploadModal(initialStore);

// Switch file upload to Parse
const displayedName = techNamesToDisplay[sampleTech.PARSE];

const techSelection = screen.getByRole('combobox', { name: 'sampleTechnologySelect' });

act(() => {
fireEvent.change(techSelection, { target: { value: sampleTech.PARSE } });
});

// The 2nd option is the selection
const option = screen.getByText(displayedName);

act(() => {
fireEvent.click(option);
});
selectTech(sampleTech.PARSE);

// It mentions the files that can be uploaded
techOptions[sampleTech.PARSE].acceptedFiles.forEach((filename) => {
expect(screen.getByText(filename)).toBeInTheDocument();
});

expect(await screen.queryByText(/To upload/)).not.toBeInTheDocument();
// It has a select button to select technology

const uploadInput = document.querySelector(
`[data-test-id="${integrationTestConstants.ids.FILE_UPLOAD_INPUT}"]`,
);

// create an all genes file (features)
const file = mockFile('all_genes.csv.gz', '/WT13');
const file = mockFile('all_genes.csv.gz', '/WT13/DGE_unfiltered');

// drop it into drop-zone
await act(async () => {
Expand Down
18 changes: 15 additions & 3 deletions src/__test__/utils/upload/processUpload.10XAndParse.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,38 +32,50 @@ const getValidFiles = (selectedSampleTech, optionals = {}) => {
let barcodesFileName;
let matrixFileName;

let featuresPathPrefix;
let barcodesPathPrefix;
let matrixPathPrefix;

if (selectedSampleTech === sampleTech['10X']) {
featuresFileName = cellrangerVersion === 'v2' ? 'genes.tsv.gz' : 'features.tsv.gz';
barcodesFileName = 'barcodes.tsv.gz';
matrixFileName = 'matrix.mtx.gz';

featuresPathPrefix = 'WT13';
barcodesPathPrefix = 'WT13';
matrixPathPrefix = 'WT13';
} else if (selectedSampleTech === sampleTech.PARSE) {
featuresFileName = 'all_genes.csv.gz';
barcodesFileName = 'cell_metadata.csv.gz';
matrixFileName = 'DGE.mtx.gz';

featuresPathPrefix = 'WT13/DGE_unfiltered';
barcodesPathPrefix = 'WT13/DGE_filtered';
matrixPathPrefix = 'WT13/DGE_unfiltered';
} else {
throw new Error(`${selectedSampleTech} not implemented`);
}

let fileList = [
{
name: `${featuresFileName}`,
fileObject: mockFile(featuresFileName, 'WT13'),
fileObject: mockFile(featuresFileName, featuresPathPrefix),
upload: { status: UploadStatus.UPLOADING },
errors: '',
compressed,
valid: true,
},
{
name: barcodesFileName,
fileObject: mockFile(barcodesFileName, 'WT13'),
fileObject: mockFile(barcodesFileName, barcodesPathPrefix),
upload: { status: UploadStatus.UPLOADING },
errors: '',
compressed,
valid: true,
},
{
name: matrixFileName,
fileObject: mockFile(matrixFileName, 'WT13'),
fileObject: mockFile(matrixFileName, matrixPathPrefix),
upload: { status: UploadStatus.UPLOADING },
errors: '',
compressed,
Expand Down
43 changes: 10 additions & 33 deletions src/components/data-management/FileUploadModal.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import { useSelector } from 'react-redux';

import config from 'config';
import { sampleTech } from 'utils/constants';
import techOptions, { techNamesToDisplay } from 'utils/upload/fileUploadSpecifications';
import fileUploadUtils, { techNamesToDisplay } from 'utils/upload/fileUploadUtils';
import handleError from 'utils/http/handleError';
import { fileObjectToFileRecord, getFileSampleAndName } from 'utils/upload/processUpload';
import { fileObjectToFileRecord } from 'utils/upload/processUpload';
import integrationTestConstants from 'utils/integrationTestConstants';
import endUserMessages from 'utils/endUserMessages';

Expand Down Expand Up @@ -78,10 +78,12 @@ const FileUploadModal = (props) => {
// Handle on Drop
const onDrop = async (acceptedFiles) => {
// Remove all hidden files
let filteredFiles = acceptedFiles
const filteredFiles = acceptedFiles
.filter((file) => !file.name.startsWith('.') && !file.name.startsWith('__MACOSX'));

if (selectedTech === sampleTech.SEURAT) {
// TODO1 this needs to be further refactored before it is moved into
// fileUploadUtils as a filterFiles call, right now it's a bit unnecessarily complicated
const newFiles = await Promise.all(filteredFiles.map((file) => (
fileObjectToFileRecord(file, selectedTech)
)));
Expand All @@ -104,27 +106,7 @@ const FileUploadModal = (props) => {

setFilesList([seuratFile]);
} else {
let filesNotInFolder = false;

filteredFiles = filteredFiles
// Remove all files that aren't in a folder
.filter((fileObject) => {
const inFolder = fileObject.path.includes('/');

filesNotInFolder ||= !inFolder;

return inFolder;
})
// Remove all files that don't fit the current technology's valid names
.filter((file) => techOptions[selectedTech].isNameValid(file.name));

if (filesNotInFolder) {
handleError('error', endUserMessages.ERROR_FILES_FOLDER);
}

const newFiles = await Promise.all(filteredFiles.map((file) => (
fileObjectToFileRecord(file, selectedTech)
)));
const newFiles = await fileUploadUtils[selectedTech].filterFiles(filteredFiles);

setFilesList([...filesList, ...newFiles]);
}
Expand All @@ -138,7 +120,7 @@ const FileUploadModal = (props) => {
setFilesList(newArray);
};

const { fileUploadParagraphs, dropzoneText, webkitdirectory } = techOptions[selectedTech];
const { fileUploadParagraphs, dropzoneText, webkitdirectory } = fileUploadUtils[selectedTech];

const renderHelpText = () => (
<>
Expand All @@ -151,7 +133,7 @@ const FileUploadModal = (props) => {
))
}
<List
dataSource={techOptions[selectedTech].inputInfo}
dataSource={fileUploadUtils[selectedTech].inputInfo}
size='small'
itemLayout='vertical'
bordered
Expand All @@ -170,8 +152,6 @@ const FileUploadModal = (props) => {
</>
);

const getFilePathToDisplay = (fileObject) => _.trim(Object.values(getFileSampleAndName(fileObject.path)).join('/'), '/');

return (
<Modal
title=''
Expand Down Expand Up @@ -298,7 +278,6 @@ const FileUploadModal = (props) => {
itemLayout='horizontal'
grid='{column: 4}'
renderItem={(file) => (

<List.Item
key={file.name}
style={{ width: '100%' }}
Expand All @@ -315,16 +294,14 @@ const FileUploadModal = (props) => {
</>
)}
<Text
ellipsis={{ tooltip: file.name }}
ellipsis={{ tooltip: fileUploadUtils[selectedTech].getFilePathToDisplay(file.fileObject.path) }}
style={{ width: '200px' }}
>
{getFilePathToDisplay(file.fileObject)}

{fileUploadUtils[selectedTech].getFilePathToDisplay(file.fileObject.path)}
</Text>
<DeleteOutlined style={{ color: 'crimson' }} onClick={() => { removeFile(file.name); }} />
</Space>
</List.Item>

)}
/>
</>
Expand Down
6 changes: 3 additions & 3 deletions src/components/data-management/SamplesTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import DraggableBodyRow from 'components/data-management/DraggableBodyRow';
import { metadataNameToKey, metadataKeyToName, temporaryMetadataKey } from 'utils/data-management/metadataUtils';
import integrationTestConstants from 'utils/integrationTestConstants';
import useConditionalEffect from 'utils/customHooks/useConditionalEffect';
import fileUploadSpecifications from 'utils/upload/fileUploadSpecifications';
import fileUploadUtils from 'utils/upload/fileUploadUtils';
import { sampleTech } from 'utils/constants';
import { fileTypeToDisplay } from 'utils/sampleFileType';

Expand Down Expand Up @@ -73,7 +73,7 @@ const SamplesTable = forwardRef((props, ref) => {

const initialTableColumns = useMemo(() => {
const filesColumns = !_.isNil(selectedTech)
? fileUploadSpecifications[selectedTech].requiredFiles.map(
? fileUploadUtils[selectedTech].requiredFiles.map(
(requiredFile, indx) => ({
index: 2 + indx,
title: <center>{fileTypeToDisplay[requiredFile]}</center>,
Expand Down Expand Up @@ -236,7 +236,7 @@ const SamplesTable = forwardRef((props, ref) => {
};

const generateDataForItem = useCallback((sampleUuid) => {
const sampleFileTypes = fileUploadSpecifications[selectedTech]?.requiredFiles
const sampleFileTypes = fileUploadUtils[selectedTech]?.requiredFiles
.map((requiredFile) => ([requiredFile, { sampleUuid }]));

return {
Expand Down
2 changes: 1 addition & 1 deletion src/components/data-management/UploadDetailsModal.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const UploadDetailsModal = (props) => {
upload, size, lastModified, fileObject = undefined,
} = data;

const { progress, status } = upload;
const { progress, status } = upload ?? {};

const isSuccessModal = status === UploadStatus.UPLOADED;
const isNotUploadedModal = status === UploadStatus.FILE_NOT_FOUND;
Expand Down
Loading
Loading