Merge pull request hms-dbmi-cellenics#512 from biomage-org/65-restric…

…t-upload-status-changes Restrict upload status changes
biomage-org · Nov 30, 2023 · 1921729 · 1921729
2 parents 377d9d3 + 2d45781
commit 1921729
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 39 deletions.
diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js
@@ -65,14 +65,14 @@ const beginUpload = async (req, res) => {
 
 const patchFile = async (req, res) => {
   const {
-    params: { experimentId, sampleId, sampleFileType },
+    params: { experimentId, sampleFileId },
     body: { uploadStatus },
   } = req;
-  logger.log(`Patching file ${sampleFileType} for sample ${sampleId} in experiment ${experimentId}`);
+  logger.log(`Patching file ${sampleFileId} in experiment ${experimentId}`);
 
-  await new SampleFile().updateUploadStatus(sampleId, sampleFileType, uploadStatus);
+  await new SampleFile().updateUploadStatus(sampleFileId, uploadStatus);
 
-  logger.log(`Finished patching sample file for experiment ${experimentId}, sample ${sampleId}, sampleFileType ${sampleFileType}`);
+  logger.log(`Finished patching sample file for experiment ${experimentId}, file: ${sampleFileId}`);
   res.json(OK());
 };
 

diff --git a/src/api.v2/model/SampleFile.js b/src/api.v2/model/SampleFile.js
@@ -33,16 +33,10 @@ class SampleFile extends BasicModel {
     return files;
   }
 
-  async updateUploadStatus(sampleId, sampleFileType, uploadStatus) {
-    await this.sql({ sf: tableNames.SAMPLE_FILE })
+  async updateUploadStatus(sampleFileId, uploadStatus) {
+    return this.sql({ sf: tableNames.SAMPLE_FILE })
       .update({ upload_status: uploadStatus })
-      .whereExists(
-        this.sql({ sf_map: tableNames.SAMPLE_TO_SAMPLE_FILE_MAP })
-          .select(['sample_file_id'])
-          .where('sf_map.sample_file_id', '=', this.sql.ref('sf.id'))
-          .where('sf_map.sample_id', '=', sampleId),
-      )
-      .andWhere({ sample_file_type: sampleFileType });
+      .where({ id: sampleFileId });
   }
 }
 

diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml
@@ -1574,9 +1574,9 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/HTTPError"
+  "/experiments/{experimentId}/sampleFiles/{sampleFileId}":
     patch:
       summary: Update a sample files property
-      description: Update property of a sample file
       operationId: patchFile
       x-eov-operation-id: sampleFile#patch
       x-eov-operation-handler: routes/sampleFile
@@ -1587,13 +1587,7 @@ paths:
           description: ID of the experiment.
           schema:
             type: string
-        - name: sampleId
-          in: path
-          description: ID of the sample.
-          required: true
-          schema:
-            type: string
-        - name: sampleFileType
+        - name: sampleFileId
           in: path
           description: Type of the file.
           required: true
@@ -1641,6 +1635,7 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/HTTPError"
+
   "/experiments/{experimentId}/sampleFiles/{sampleFileId}/beginUpload":
     post:
       summary: Begins the multipart upload of a sample file

diff --git a/tests/api.v2/controllers/sampleFileController.test.js b/tests/api.v2/controllers/sampleFileController.test.js
@@ -163,13 +163,12 @@ describe('sampleFileController', () => {
 
   it('patchFile works correctly', async () => {
     const experimentId = 'experimentId';
-    const sampleId = 'sampleId';
-    const sampleFileType = 'features10x';
+    const sampleFileId = 'sampleFileId';
 
     const uploadStatus = 'uploaded';
 
     const mockReq = {
-      params: { experimentId, sampleId, sampleFileType },
+      params: { experimentId, sampleFileId },
       body: { uploadStatus },
     };
 
@@ -178,7 +177,7 @@ describe('sampleFileController', () => {
     // Used with normal client
     expect(SampleFile).toHaveBeenCalled();
 
-    expect(sampleFileInstance.updateUploadStatus).toHaveBeenCalledWith('sampleId', 'features10x', uploadStatus);
+    expect(sampleFileInstance.updateUploadStatus).toHaveBeenCalledWith(sampleFileId, uploadStatus);
 
     // Response is generated signed url
     expect(mockRes.json).toHaveBeenCalledWith(OK());

diff --git a/tests/api.v2/model/SampleFile.test.js b/tests/api.v2/model/SampleFile.test.js
@@ -53,20 +53,15 @@ describe('model/Sample', () => {
 
   it('updateUploadStatus works correctly if valid params are passed', async () => {
     const mockUploadStatus = 'fileNotFound';
+    const mockSampleFileId = 'fileId1';
 
     const sfIdRef = 'sf.idRef';
     mockSqlClient.ref.mockReturnValueOnce(sfIdRef);
 
-    await new SampleFile().updateUploadStatus(mockSampleId, mockSampleFileType, mockUploadStatus);
+    await new SampleFile().updateUploadStatus(mockSampleFileId, mockUploadStatus);
 
     expect(mockSqlClient).toHaveBeenCalledWith({ sf: tableNames.SAMPLE_FILE });
     expect(mockSqlClient.update).toHaveBeenCalledWith({ upload_status: mockUploadStatus });
-    expect(mockSqlClient.whereExists).toHaveBeenCalled();
-
-    expect(mockSqlClient).toHaveBeenCalledWith({ sf_map: tableNames.SAMPLE_TO_SAMPLE_FILE_MAP });
-    expect(mockSqlClient.select).toHaveBeenCalledWith(['sample_file_id']);
-    expect(mockSqlClient.where).toHaveBeenCalledWith('sf_map.sample_file_id', '=', sfIdRef);
-    expect(mockSqlClient.where).toHaveBeenCalledWith('sf_map.sample_id', '=', mockSampleId);
-    expect(mockSqlClient.andWhere).toHaveBeenCalledWith({ sample_file_type: mockSampleFileType });
+    expect(mockSqlClient.where.mock.calls).toMatchSnapshot();
   });
 });
diff --git a/tests/api.v2/model/__snapshots__/SampleFile.test.js.snap b/tests/api.v2/model/__snapshots__/SampleFile.test.js.snap
@@ -0,0 +1,11 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`model/Sample updateUploadStatus works correctly if valid params are passed 1`] = `
+Array [
+  Array [
+    Object {
+      "id": "fileId1",
+    },
+  ],
+]
+`;
diff --git a/tests/api.v2/routes/sampleFile.test.js b/tests/api.v2/routes/sampleFile.test.js
@@ -98,15 +98,14 @@ describe('tests for experiment route', () => {
     });
 
     const experimentId = 'experiment-id';
-    const sampleId = 'sample-id';
-    const sampleFileType = 'features.tsv.gz';
+    const sampleFileId = 'sample-file-id';
 
     const patchFileBody = {
       uploadStatus: 'uploading',
     };
 
     request(app)
-      .patch(`/v2/experiments/${experimentId}/samples/${sampleId}/sampleFiles/${sampleFileType}`)
+      .patch(`/v2/experiments/${experimentId}/sampleFiles/${sampleFileId}`)
       .send(patchFileBody)
       .expect(200)
       .end((err) => {
@@ -126,15 +125,14 @@ describe('tests for experiment route', () => {
     });
 
     const experimentId = 'experiment-id';
-    const sampleId = 'sample-id';
-    const sampleFileType = 'features.tsv.gz';
+    const sampleFileId = 'sample-file-id';
 
     const patchFileBody = {
       s3Path: 'features.tsv.gz',
     };
 
     request(app)
-      .patch(`/v2/experiments/${experimentId}/samples/${sampleId}/sampleFiles/${sampleFileType}`)
+      .patch(`/v2/experiments/${experimentId}/sampleFiles/${sampleFileId}`)
       .send(patchFileBody)
       .expect(400)
       .end((err) => {