From f14b0aa0849d669de1a28d5f6f8b5fb1d7cb4864 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Tue, 28 Nov 2023 11:27:24 -0300 Subject: [PATCH 1/5] Send update upload status to new endpoint shape using sampleFileId Signed-off-by: cosa65 --- src/redux/actions/samples/createSampleFile.js | 8 ++++++-- src/redux/actions/samples/updateSampleFileUpload.js | 4 ++-- src/utils/upload/processUpload.js | 10 ++++++---- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/redux/actions/samples/createSampleFile.js b/src/redux/actions/samples/createSampleFile.js index ec91f235a0..7b5c52fb5c 100644 --- a/src/redux/actions/samples/createSampleFile.js +++ b/src/redux/actions/samples/createSampleFile.js @@ -16,10 +16,12 @@ const createSampleFile = ( ) => async (dispatch) => { const updatedAt = dayjs().toISOString(); + const sampleFileId = uuidv4(); + try { const url = `/v2/experiments/${experimentId}/samples/${sampleId}/sampleFiles/${type}`; const body = { - sampleFileId: uuidv4(), + sampleFileId, size: fileForApiV1.size, }; @@ -49,7 +51,9 @@ const createSampleFile = ( return body.sampleFileId; } catch (e) { - dispatch(updateSampleFileUpload(experimentId, sampleId, type, UploadStatus.UPLOAD_ERROR)); + dispatch(updateSampleFileUpload( + experimentId, sampleId, sampleFileId, type, UploadStatus.UPLOAD_ERROR, + )); throw e; } diff --git a/src/redux/actions/samples/updateSampleFileUpload.js b/src/redux/actions/samples/updateSampleFileUpload.js index d31553dae5..c67d613434 100644 --- a/src/redux/actions/samples/updateSampleFileUpload.js +++ b/src/redux/actions/samples/updateSampleFileUpload.js @@ -8,14 +8,14 @@ import UploadStatus from 'utils/upload/UploadStatus'; import fileNameForApiV1 from 'utils/upload/fileNameForApiV1'; const updateSampleFileUpload = ( - experimentId, sampleId, type, uploadStatus, uploadProgress, + experimentId, sampleId, sampleFileId, type, uploadStatus, uploadProgress, ) => async (dispatch) => { const updatedAt = dayjs().toISOString(); // Don't send an api update whenever the progress bar is updated, only for uploadStatus changes // TODO: move progress to not even be a part of redux, manage it in a different way if (uploadStatus !== UploadStatus.UPLOADING) { - const url = `/v2/experiments/${experimentId}/samples/${sampleId}/sampleFiles/${type}`; + const url = `/v2/experiments/${experimentId}/sampleFiles/${sampleFileId}`; const body = { uploadStatus }; try { diff --git a/src/utils/upload/processUpload.js b/src/utils/upload/processUpload.js index 607be41d30..068278162f 100644 --- a/src/utils/upload/processUpload.js +++ b/src/utils/upload/processUpload.js @@ -84,7 +84,7 @@ const createAndUploadSampleFile = async (file, experimentId, sampleId, dispatch, try { file.fileObject = await loadAndCompressIfNecessary(file, () => { dispatch(updateSampleFileUpload( - experimentId, sampleId, fileType, UploadStatus.COMPRESSING, + experimentId, sampleId, sampleFileId, fileType, UploadStatus.COMPRESSING, )); }); @@ -94,7 +94,9 @@ const createAndUploadSampleFile = async (file, experimentId, sampleId, dispatch, ? UploadStatus.FILE_READ_ABORTED : UploadStatus.FILE_READ_ERROR; - dispatch(updateSampleFileUpload(experimentId, sampleId, fileType, fileErrorStatus)); + dispatch(updateSampleFileUpload( + experimentId, sampleId, sampleFileId, fileType, fileErrorStatus, + )); return; } } @@ -109,7 +111,7 @@ const createAndUploadSampleFile = async (file, experimentId, sampleId, dispatch, const updateSampleFileUploadProgress = (status, percentProgress = 0) => dispatch( updateSampleFileUpload( - experimentId, sampleId, fileType, status, percentProgress, + experimentId, sampleId, sampleFileId, fileType, status, percentProgress, ), ); @@ -118,7 +120,7 @@ const createAndUploadSampleFile = async (file, experimentId, sampleId, dispatch, await prepareAndUploadFileToS3(file, uploadUrlParams, 'sample', updateSampleFileUploadProgress); } catch (e) { dispatch(updateSampleFileUpload( - experimentId, sampleId, fileType, UploadStatus.UPLOAD_ERROR, + experimentId, sampleId, sampleFileId, fileType, UploadStatus.UPLOAD_ERROR, )); } }; From 40a8c5871261ffd0c7847d9892b135079c5aab33 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Tue, 28 Nov 2023 11:28:25 -0300 Subject: [PATCH 2/5] Update process10XUpload.test.js Signed-off-by: cosa65 --- .../process10XUpload.test.js.snap | 24 +++++++++---------- .../utils/upload/process10XUpload.test.js | 6 +++++ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/__test__/utils/upload/__snapshots__/process10XUpload.test.js.snap b/src/__test__/utils/upload/__snapshots__/process10XUpload.test.js.snap index 8d69fa1852..0df49e3c7b 100644 --- a/src/__test__/utils/upload/__snapshots__/process10XUpload.test.js.snap +++ b/src/__test__/utils/upload/__snapshots__/process10XUpload.test.js.snap @@ -73,7 +73,7 @@ exports[`processUpload Should not upload files if there are errors beginning the }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/features10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploadError"}", "headers": { @@ -83,7 +83,7 @@ exports[`processUpload Should not upload files if there are errors beginning the }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/barcodes10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploadError"}", "headers": { @@ -93,7 +93,7 @@ exports[`processUpload Should not upload files if there are errors beginning the }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/matrix10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploadError"}", "headers": { @@ -193,7 +193,7 @@ exports[`processUpload Updates redux correctly when there are file upload errors }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/features10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploadError"}", "headers": { @@ -203,7 +203,7 @@ exports[`processUpload Updates redux correctly when there are file upload errors }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/barcodes10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploadError"}", "headers": { @@ -213,7 +213,7 @@ exports[`processUpload Updates redux correctly when there are file upload errors }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/matrix10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploadError"}", "headers": { @@ -360,7 +360,7 @@ exports[`processUpload Uploads and updates redux correctly when there are no err }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/features10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploaded"}", "headers": { @@ -370,7 +370,7 @@ exports[`processUpload Uploads and updates redux correctly when there are no err }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/barcodes10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploaded"}", "headers": { @@ -380,7 +380,7 @@ exports[`processUpload Uploads and updates redux correctly when there are no err }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/matrix10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploaded"}", "headers": { @@ -527,7 +527,7 @@ exports[`processUpload Uploads and updates redux correctly when there are no err }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/features10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploaded"}", "headers": { @@ -537,7 +537,7 @@ exports[`processUpload Uploads and updates redux correctly when there are no err }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/barcodes10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploaded"}", "headers": { @@ -547,7 +547,7 @@ exports[`processUpload Uploads and updates redux correctly when there are no err }, ], [ - "http://localhost:3000/v2/experiments/project-uuid/samples/mockSampleId/sampleFiles/matrix10x", + "http://localhost:3000/v2/experiments/project-uuid/sampleFiles/mockSampleFileId", { "body": "{"uploadStatus":"uploaded"}", "headers": { diff --git a/src/__test__/utils/upload/process10XUpload.test.js b/src/__test__/utils/upload/process10XUpload.test.js index 3678541502..7fa6b982b7 100644 --- a/src/__test__/utils/upload/process10XUpload.test.js +++ b/src/__test__/utils/upload/process10XUpload.test.js @@ -140,10 +140,16 @@ const mockProcessUploadCalls = () => { result = { status: 200, body: JSON.stringify({ WT13: sampleId }) }; } + // Create sample file if (new RegExp(`/v2/experiments/${mockExperimentId}/samples/.*/sampleFiles/.*`).test(url)) { result = { status: 200, body: JSON.stringify({}) }; } + // Update sample file status + if (new RegExp(`/v2/experiments/${mockExperimentId}/sampleFiles/.*`).test(url)) { + result = { status: 200, body: JSON.stringify({}) }; + } + if (url.endsWith(`/v2/experiments/${mockExperimentId}/sampleFiles/${sampleFileId}/beginUpload`)) { result = { status: 200, body: JSON.stringify(mockUploadUrlParams) }; } From f84a61c6089287e5c47ad391a3fbb436e67fbc92 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Tue, 28 Nov 2023 11:56:41 -0300 Subject: [PATCH 3/5] Update tests Signed-off-by: cosa65 --- .../data-management/SamplesTableCells.test.jsx | 10 +++++++--- src/__test__/test-utils/constants.js | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/__test__/components/data-management/SamplesTableCells.test.jsx b/src/__test__/components/data-management/SamplesTableCells.test.jsx index cbfe20785d..b0df1fc6e0 100644 --- a/src/__test__/components/data-management/SamplesTableCells.test.jsx +++ b/src/__test__/components/data-management/SamplesTableCells.test.jsx @@ -13,7 +13,7 @@ import { } from 'components/data-management/SamplesTableCells'; import UploadStatus, { messageForStatus } from 'utils/upload/UploadStatus'; import { makeStore } from 'redux/store'; -import mockAPI, { generateDefaultMockAPIResponses } from '__test__/test-utils/mockAPI'; +import mockAPI, { generateDefaultMockAPIResponses, statusResponse } from '__test__/test-utils/mockAPI'; import { loadSamples, updateSampleFileUpload } from 'redux/actions/samples'; import fake from '__test__/test-utils/constants'; @@ -40,6 +40,7 @@ jest.mock('swr', () => () => ({ const experimentId = `${fake.EXPERIMENT_ID}-0`; const sampleId = `${fake.SAMPLE_ID}-0`; +const sampleFileId = `${fake.SAMPLE_FILE_ID}`; enableFetchMocks(); @@ -50,7 +51,10 @@ describe('UploadCell', () => { beforeEach(async () => { fetchMock.mockClear(); - fetchMock.mockIf(/.*/, mockAPI(generateDefaultMockAPIResponses(experimentId))); + fetchMock.mockIf(/.*/, mockAPI({ + [`experiments/${experimentId}/sampleFiles/${sampleFileId}$`]: () => statusResponse(200, 'OK'), + ...generateDefaultMockAPIResponses(experimentId), + })); storeState = makeStore(); @@ -73,7 +77,7 @@ describe('UploadCell', () => { await storeState.dispatch( updateSampleFileUpload( - experimentId, sampleId, 'features10x', UploadStatus.UPLOADING, percentUploaded, + experimentId, sampleId, sampleFileId, 'features10x', UploadStatus.UPLOADING, percentUploaded, ), ); diff --git a/src/__test__/test-utils/constants.js b/src/__test__/test-utils/constants.js index bdc75a642c..5c47eec565 100644 --- a/src/__test__/test-utils/constants.js +++ b/src/__test__/test-utils/constants.js @@ -6,6 +6,7 @@ export default { EXPERIMENT_NAME: 'Test Experiment', PROJECT_ID: 'test24b6-8600-test-mock-63822d2fmock', SAMPLE_ID: 'test9188-d682-test-mock-cb6d644cmock', + SAMPLE_FILE_ID: 'test4321-1234-test-mock-sampfilemock', API_ENDPOINT: /^http?:\/\/localhost:3000.*$/, S3_ENDPOINT: 'http://mock.s3.amazonaws.com', // pragma: allowlist secret From ab401e1382058795576b6888cd5855a1a2a0eb41 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 30 Nov 2023 11:25:41 -0300 Subject: [PATCH 4/5] Add optional chaining to abortController.signal Signed-off-by: cosa65 --- src/utils/upload/processMultipartUpload.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/upload/processMultipartUpload.js b/src/utils/upload/processMultipartUpload.js index c1c0e3d97e..6a85da268e 100644 --- a/src/utils/upload/processMultipartUpload.js +++ b/src/utils/upload/processMultipartUpload.js @@ -11,7 +11,7 @@ const putPartInS3 = async ( method: 'put', data: blob, url: signedUrl, - signal: abortController.signal, + signal: abortController?.signal, headers: { 'Content-Type': 'application/octet-stream', }, From 67dab906df97a47a9c8f4ba74902879ca6c6bb98 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 30 Nov 2023 11:42:31 -0300 Subject: [PATCH 5/5] Optional chaining doesnt work, so instead make the abort controller non-optional Signed-off-by: cosa65 --- .../data-management/metadata/AddMetadataButton.jsx | 2 +- src/utils/upload/processMultipartUpload.js | 10 ++++++---- src/utils/upload/processUpload.js | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/data-management/metadata/AddMetadataButton.jsx b/src/components/data-management/metadata/AddMetadataButton.jsx index 50895afec1..004a4b7501 100644 --- a/src/components/data-management/metadata/AddMetadataButton.jsx +++ b/src/components/data-management/metadata/AddMetadataButton.jsx @@ -59,7 +59,7 @@ const AddMetadataButton = ({ samplesTableRef }) => { } try { - await prepareAndUploadFileToS3(file, uploadUrlParams, 'cellLevelMeta', onUpdateUploadStatus); + await prepareAndUploadFileToS3(file, uploadUrlParams, 'cellLevelMeta', new AbortController(), onUpdateUploadStatus); } catch (e) { pushNotificationMessage('error', 'Something went wrong while uploading your metadata file.'); console.log(e); diff --git a/src/utils/upload/processMultipartUpload.js b/src/utils/upload/processMultipartUpload.js index 6a85da268e..fa5493044c 100644 --- a/src/utils/upload/processMultipartUpload.js +++ b/src/utils/upload/processMultipartUpload.js @@ -4,14 +4,14 @@ const FILE_CHUNK_SIZE = 10000000; const MAX_RETRIES = 2; const putPartInS3 = async ( - blob, signedUrl, onUploadProgress, currentRetry = 0, abortController = null, + blob, signedUrl, onUploadProgress, abortController, currentRetry = 0, ) => { try { return await axios.request({ method: 'put', data: blob, url: signedUrl, - signal: abortController?.signal, + signal: abortController.signal, headers: { 'Content-Type': 'application/octet-stream', }, @@ -19,7 +19,9 @@ const putPartInS3 = async ( }); } catch (e) { if (currentRetry < MAX_RETRIES) { - return await putPartInS3(blob, signedUrl, onUploadProgress, currentRetry + 1); + return await putPartInS3( + blob, signedUrl, onUploadProgress, abortController, currentRetry + 1, + ); } throw e; @@ -42,8 +44,8 @@ const processMultipartUpload = async ( blob, signedUrl, createOnUploadProgressForPart(index), - 0, abortController, + 0, ); promises.push(req); diff --git a/src/utils/upload/processUpload.js b/src/utils/upload/processUpload.js index 5668e98e87..85b0924c7f 100644 --- a/src/utils/upload/processUpload.js +++ b/src/utils/upload/processUpload.js @@ -18,7 +18,7 @@ import endUserMessages from 'utils/endUserMessages'; import pushNotificationMessage from 'utils/pushNotificationMessage'; const prepareAndUploadFileToS3 = async ( - file, uploadUrlParams, type, onStatusUpdate = () => { }, abortController = null, + file, uploadUrlParams, type, abortController, onStatusUpdate = () => { }, ) => { let parts = null; const { signedUrls, uploadId, fileId } = uploadUrlParams; @@ -121,7 +121,7 @@ const createAndUploadSampleFile = async (file, experimentId, sampleId, dispatch, ); const uploadUrlParams = { signedUrls, uploadId, fileId: sampleFileId }; - await prepareAndUploadFileToS3(file, uploadUrlParams, 'sample', updateSampleFileUploadProgress, abortController); + await prepareAndUploadFileToS3(file, uploadUrlParams, 'sample', abortController, updateSampleFileUploadProgress); } catch (e) { dispatch(updateSampleFileUpload( experimentId, sampleId, sampleFileId, fileType, UploadStatus.UPLOAD_ERROR,