From 6aab0532b0ebacbe28f825c4a73bf6285d5c5a42 Mon Sep 17 00:00:00 2001 From: Pol Alvarez Date: Thu, 21 Jul 2022 16:37:24 +0200 Subject: [PATCH 1/2] moved sample validation --- src/redux/actions/samples/createSample.js | 19 ++++++++++++++----- src/redux/actions/samples/createSampleFile.js | 2 +- src/utils/upload/processUpload.js | 11 +---------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/redux/actions/samples/createSample.js b/src/redux/actions/samples/createSample.js index 0b6dbcd548..27e9a05581 100644 --- a/src/redux/actions/samples/createSample.js +++ b/src/redux/actions/samples/createSample.js @@ -13,10 +13,13 @@ import { METADATA_DEFAULT_VALUE } from 'redux/reducers/experiments/initialState' import { sampleTemplate } from 'redux/reducers/samples/initialState'; import UploadStatus from 'utils/upload/UploadStatus'; +import pushNotificationMessage from 'utils/pushNotificationMessage'; +import validate from 'utils/upload/sampleValidator'; const createSample = ( experimentId, name, + sample, type, filesToUpload, ) => async (dispatch, getState) => { @@ -52,11 +55,22 @@ const createSample = ( } else { throw new Error(`Sample technology ${type} is not recognized`); } + const errors = await validate(sample); + if (errors && errors.length > 0) { + const errorMessage = `Error uploading sample ${name}.\n${errors.join('\n')}`; + pushNotificationMessage('error', errorMessage, 15); + throw new Error(errorMessage); + } filesToUpload.forEach((fileName) => { newSample.files[fileName] = { upload: { status: UploadStatus.UPLOADING } }; }); + await dispatch({ + type: SAMPLES_CREATE, + payload: { sample: newSample, experimentId }, + }); + try { await fetchAPI( url, @@ -69,11 +83,6 @@ const createSample = ( }, ); - await dispatch({ - type: SAMPLES_CREATE, - payload: { sample: newSample, experimentId }, - }); - await dispatch({ type: SAMPLES_SAVED, payload: { sample: newSample, experimentId }, diff --git a/src/redux/actions/samples/createSampleFile.js b/src/redux/actions/samples/createSampleFile.js index 5fe6d26a26..a60cac8cac 100644 --- a/src/redux/actions/samples/createSampleFile.js +++ b/src/redux/actions/samples/createSampleFile.js @@ -53,7 +53,7 @@ const createSampleFile = ( return signedUrl; } catch (e) { - // Can't update the upload status becuase we didn't even get to create the sample file + // Can't update the upload status because we didn't even get to create the sample file handleError(e, endUserMessages.ERROR_BEGIN_SAMPLE_FILE_UPLOAD); throw e; diff --git a/src/utils/upload/processUpload.js b/src/utils/upload/processUpload.js index db69640209..ddf056e008 100644 --- a/src/utils/upload/processUpload.js +++ b/src/utils/upload/processUpload.js @@ -4,14 +4,12 @@ import _ from 'lodash'; import axios from 'axios'; import { createSample, createSampleFile, updateSampleFileUpload } from 'redux/actions/samples'; -import validate from 'utils/upload/sampleValidator'; import UploadStatus from 'utils/upload/UploadStatus'; import loadAndCompressIfNecessary from 'utils/upload/loadAndCompressIfNecessary'; import { inspectFile, Verdict } from 'utils/upload/fileInspector'; import getFileTypeV2 from 'utils/getFileTypeV2'; -import pushNotificationMessage from 'utils/pushNotificationMessage'; const putInS3 = async (loadedFileData, signedUrl, onUploadProgress) => ( await axios.request({ @@ -131,22 +129,15 @@ const processUpload = async (filesList, sampleType, samples, experimentId, dispa }, {}); Object.entries(samplesMap).forEach(async ([name, sample]) => { - const errors = await validate(sample); - const filesToUploadForSample = Object.keys(sample.files); - if (errors && errors.length > 0) { - const errorMessage = errors.join('\n'); - pushNotificationMessage('error', `Error uploading sample ${name}.\n${errorMessage}`, 15); - return; - } - // Create sample if not exists. try { sample.uuid ??= await dispatch( createSample( experimentId, name, + sample, sampleType, filesToUploadForSample, ), From 595b385768b3ec1b126afd0460c90ba5c65630e1 Mon Sep 17 00:00:00 2001 From: Pol Alvarez Date: Thu, 21 Jul 2022 16:51:09 +0200 Subject: [PATCH 2/2] fixed tests --- .../redux/actions/samples/createSample.test.js | 11 +++++++---- src/redux/actions/samples/createSample.js | 10 +++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/__test__/redux/actions/samples/createSample.test.js b/src/__test__/redux/actions/samples/createSample.test.js index db00081996..32827d030a 100644 --- a/src/__test__/redux/actions/samples/createSample.test.js +++ b/src/__test__/redux/actions/samples/createSample.test.js @@ -7,6 +7,7 @@ import { v4 as uuidv4 } from 'uuid'; import createSample from 'redux/actions/samples/createSample'; import initialSampleState from 'redux/reducers/samples/initialState'; import initialExperimentState, { experimentTemplate } from 'redux/reducers/experiments/initialState'; +import 'utils/upload/sampleValidator'; import { SAMPLES_CREATE, SAMPLES_SAVING, SAMPLES_ERROR, SAMPLES_SAVED, @@ -15,6 +16,7 @@ import { import endUserMessages from 'utils/endUserMessages'; import pushNotificationMessage from 'utils/pushNotificationMessage'; +jest.mock('utils/upload/sampleValidator'); pushNotificationMessage.mockImplementation(() => async () => { }); enableFetchMocks(); @@ -26,6 +28,7 @@ const sampleUuid = 'abc123'; uuidv4.mockImplementation(() => sampleUuid); const sampleName = 'test sample'; +const sample = {}; describe('createSample action', () => { const experimentId = 'exp234'; @@ -62,7 +65,7 @@ describe('createSample action', () => { it('Works correctly with one file being uploaded', async () => { fetchMock.mockResponse(JSON.stringify({}), { url: 'mockedUrl', status: 200 }); - const newUuid = await store.dispatch(createSample(experimentId, sampleName, mockType, ['matrix.tsv.gz'])); + const newUuid = await store.dispatch(createSample(experimentId, sampleName, sample, mockType, ['matrix.tsv.gz'])); // Returns a new sampleUuid expect(newUuid).toEqual(sampleUuid); @@ -85,7 +88,7 @@ describe('createSample action', () => { it('Works correctly with many files being uploaded', async () => { fetchMock.mockResponse(JSON.stringify({}), { url: 'mockedUrl', status: 200 }); - const newUuid = await store.dispatch(createSample(experimentId, sampleName, mockType, ['matrix.tsv.gz', 'features.tsv.gz', 'barcodes.tsv.gz'])); + const newUuid = await store.dispatch(createSample(experimentId, sampleName, sample, mockType, ['matrix.tsv.gz', 'features.tsv.gz', 'barcodes.tsv.gz'])); // Returns a new sampleUuid expect(newUuid).toEqual(sampleUuid); @@ -110,7 +113,7 @@ describe('createSample action', () => { await expect( store.dispatch( - createSample(experimentId, sampleName, mockType, ['matrix.tsv.gz']), + createSample(experimentId, sampleName, sample, mockType, ['matrix.tsv.gz']), ), ).rejects.toThrow(endUserMessages.ERROR_CREATING_SAMPLE); @@ -125,7 +128,7 @@ describe('createSample action', () => { await expect( store.dispatch( - createSample(experimentId, sampleName, 'unrecognizable type', ['matrix.tsv.gz', 'features.tsv.gz', 'barcodes.tsv.gz']), + createSample(experimentId, sampleName, sample, 'unrecognizable type', ['matrix.tsv.gz', 'features.tsv.gz', 'barcodes.tsv.gz']), ), ).rejects.toThrow('Sample technology unrecognizable type is not recognized'); }); diff --git a/src/redux/actions/samples/createSample.js b/src/redux/actions/samples/createSample.js index 27e9a05581..c465ad26dc 100644 --- a/src/redux/actions/samples/createSample.js +++ b/src/redux/actions/samples/createSample.js @@ -66,11 +66,6 @@ const createSample = ( newSample.files[fileName] = { upload: { status: UploadStatus.UPLOADING } }; }); - await dispatch({ - type: SAMPLES_CREATE, - payload: { sample: newSample, experimentId }, - }); - try { await fetchAPI( url, @@ -83,6 +78,11 @@ const createSample = ( }, ); + await dispatch({ + type: SAMPLES_CREATE, + payload: { sample: newSample, experimentId }, + }); + await dispatch({ type: SAMPLES_SAVED, payload: { sample: newSample, experimentId },