From 3c2081f2805d8bc25b908bdfb351b83ef8d626de Mon Sep 17 00:00:00 2001 From: cosa65 Date: Mon, 20 Jun 2022 13:27:50 -0300 Subject: [PATCH 01/29] Add get example experiments endpoint --- .../controllers/experimentController.js | 23 ++++++ src/api.v2/helpers/roles.js | 4 +- src/api.v2/model/Experiment.js | 19 +++-- src/api.v2/routes/experiment.js | 13 +++- src/specs/api.v2.yaml | 76 ++++++++++++++++++- ...iments.v2.yaml => ExperimentsList.v2.yaml} | 0 6 files changed, 123 insertions(+), 12 deletions(-) rename src/specs/models/experiment-bodies/{GetAllExperiments.v2.yaml => ExperimentsList.v2.yaml} (100%) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 37d66863d..6e3791dee 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -19,6 +19,12 @@ const getAllExperiments = async (req, res) => { res.json(data); }; +const getAllExampleExperiments = async (req, res) => { + const data = await new Experiment().getAllExampleExperiments(); + + res.json(data); +}; + const getExperiment = async (req, res) => { const { params: { experimentId } } = req; @@ -129,11 +135,27 @@ const downloadData = async (req, res) => { logger.log(`Providing download link for download ${downloadType} for experiment ${experimentId}`); const downloadLink = await new Experiment().getDownloadLink(experimentId, downloadType); + + logger.log(`Finished providing download link for download ${downloadType} for experiment ${experimentId}`); res.json(downloadLink); }; +const cloneExperiment = async (req, res) => { + const { experimentId } = req.params; + + logger.log(`Cloning experiment ${experimentId}`); + + const cloneExperimentId = await new Experiment().createClone(experimentId); + // const downloadLink = await new SampleFile().getDownloadLink(experimentId, downloadType); + + logger.log(`Finished cloning experiment ${experimentId}, new expeirment's id is ${cloneExperimentId}`); + + res.json(cloneExperimentId); +}; + module.exports = { getAllExperiments, + getAllExampleExperiments, getExperiment, createExperiment, updateProcessingConfig, @@ -143,4 +165,5 @@ module.exports = { getProcessingConfig, getBackendStatus, downloadData, + cloneExperiment, }; diff --git a/src/api.v2/helpers/roles.js b/src/api.v2/helpers/roles.js index 0223105c1..084dd2f25 100644 --- a/src/api.v2/helpers/roles.js +++ b/src/api.v2/helpers/roles.js @@ -19,7 +19,9 @@ const allowedResources = { '/experiments/(?.*)/plots-tables/(?.*)', '/experiments/(?.*)/cellSets', ], - [VIEWER]: [], + [VIEWER]: [ + '/experiments/(?.*)/clone', + ], }; const isRoleAuthorized = (role, resource, method) => { // if no valid role is provided, return not authorized diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 139322ec2..107a93626 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -43,20 +43,27 @@ class Experiment extends BasicModel { ]; const aliasedExperimentFields = fields.map((field) => `e.${field}`); - function mainQuery() { - this.select([...aliasedExperimentFields, 'm.key']) + + const result = await collapseKeyIntoArray( + this.sql.select([...aliasedExperimentFields, 'm.key']) .from(tableNames.USER_ACCESS) .where('user_id', userId) .join(`${tableNames.EXPERIMENT} as e`, 'e.id', `${tableNames.USER_ACCESS}.experiment_id`) .leftJoin(`${tableNames.METADATA_TRACK} as m`, 'e.id', 'm.experiment_id') - .as('mainQuery'); - } - - const result = await collapseKeyIntoArray(mainQuery, [...fields], 'key', 'metadataKeys', this.sql); + .as('mainQuery'), [...fields], + 'key', + 'metadataKeys', + this.sql, + ); return result; } + async getAllExampleExperiments() { + const noOwnerExperiments = '00000000-0000-0000-0000-000000000000'; + return this.getAllExperiments(noOwnerExperiments); + } + async getExperimentData(experimentId) { function mainQuery() { this.select('*') diff --git a/src/api.v2/routes/experiment.js b/src/api.v2/routes/experiment.js index c0696a75f..c2a99d07c 100644 --- a/src/api.v2/routes/experiment.js +++ b/src/api.v2/routes/experiment.js @@ -1,7 +1,8 @@ const { - createExperiment, getExperiment, patchExperiment, deleteExperiment, - updateSamplePosition, getAllExperiments, + getAllExperiments, getAllExampleExperiments, + createExperiment, getExperiment, patchExperiment, deleteExperiment, cloneExperiment, getProcessingConfig, updateProcessingConfig, + updateSamplePosition, getBackendStatus, downloadData, } = require('../controllers/experimentController'); @@ -12,6 +13,10 @@ module.exports = { expressAuthenticationOnlyMiddleware, (req, res, next) => getAllExperiments(req, res).catch(next), ], + 'experiment#getAllExampleExperiments': [ + expressAuthenticationOnlyMiddleware, + (req, res, next) => getAllExampleExperiments(req, res).catch(next), + ], 'experiment#getExperiment': [ expressAuthorizationMiddleware, (req, res, next) => getExperiment(req, res).catch(next), @@ -48,4 +53,8 @@ module.exports = { expressAuthorizationMiddleware, (req, res, next) => downloadData(req, res).catch(next), ], + 'experiment#clone': [ + expressAuthorizationMiddleware, + (req, res, next) => cloneExperiment(req, res).catch(next), + ], }; diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 1c6f26fac..7559bc94b 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -229,7 +229,39 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/GetAllExperiments' + $ref: '#/components/schemas/ExperimentsList' + '400': + description: Bad Request + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '401': + description: The request lacks authentication credentials. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '404': + description: Not found error. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '/experiments/examples': + get: + summary: Get all example experiments + description: Get all experiments that are for demonstration + operationId: getAllExampleExperiments + x-eov-operation-id: experiment#getAllExampleExperiments + x-eov-operation-handler: routes/experiment + responses: + '200': + description: All experiments that the user can acess + content: + application/json: + schema: + $ref: '#/components/schemas/ExperimentsList' '400': description: Bad Request content: @@ -939,6 +971,44 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPError' + '/experiments/{experimentId}/clone': + post: + summary: Clone an experiment + description: Clone an experiment, returns the id of the new experiment + operationId: cloneExperiment + x-eov-operation-id: experiment#clone + x-eov-operation-handler: routes/experiment + responses: + '200': + description: Clone experiment + content: + application/json: + schema: + type: string + '400': + description: Bad Request + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '401': + description: The request lacks authentication credentials. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '403': + description: Forbidden request for this user. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '404': + description: Not found error. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' '/access/{experimentId}': get: summary: Get the users with access to an experiment @@ -1460,8 +1530,8 @@ components: $ref: './models/experiment-bodies/PatchExperiment.v2.yaml' ProcessingConfig: $ref: './models/experiment-bodies/ProcessingConfig.v2.yaml' - GetAllExperiments: - $ref: './models/experiment-bodies/GetAllExperiments.v2.yaml' + ExperimentsList: + $ref: './models/experiment-bodies/ExperimentsList.v2.yaml' CreateSample: $ref: './models/samples-bodies/CreateSample.v2.yaml' GetSamples: diff --git a/src/specs/models/experiment-bodies/GetAllExperiments.v2.yaml b/src/specs/models/experiment-bodies/ExperimentsList.v2.yaml similarity index 100% rename from src/specs/models/experiment-bodies/GetAllExperiments.v2.yaml rename to src/specs/models/experiment-bodies/ExperimentsList.v2.yaml From 73953b610d1f482a818cf590be1e447d1fe4d890 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Tue, 21 Jun 2022 09:21:14 -0300 Subject: [PATCH 02/29] Add return of sample file ids in Sample.getSamples --- src/api.v2/model/Sample.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/api.v2/model/Sample.js b/src/api.v2/model/Sample.js index 48a2c5428..47d9bc955 100644 --- a/src/api.v2/model/Sample.js +++ b/src/api.v2/model/Sample.js @@ -38,6 +38,11 @@ class Sample extends BasicModel { const sampleFileFields = ['sample_file_type', 'size', 'upload_status', 's3_path']; const sampleFileFieldsWithAlias = sampleFileFields.map((field) => `sf.${field}`); const fileObjectFormatted = sampleFileFields.map((field) => [`'${field}'`, field]); + + // Add sample file id (needs to go separate to avoid conflict with sample id) + sampleFileFieldsWithAlias.push('sf.id as sf_id'); + fileObjectFormatted.push(['\'id\'', 'sf_id']); + const sampleFileObject = `jsonb_object_agg(sample_file_type,json_build_object(${fileObjectFormatted})) as files`; const fileNamesQuery = sql.select(['id', sql.raw(sampleFileObject)]) .from(sql.select([...sampleFileFieldsWithAlias, 's.id']) From b98719486ea2a2491dc712a279acbad0f290b7c9 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Tue, 21 Jun 2022 15:32:52 -0300 Subject: [PATCH 03/29] Add experiment clone endpoint --- .../controllers/experimentController.js | 26 ++++++-- src/api.v2/helpers/roles.js | 3 +- src/api.v2/model/Experiment.js | 4 +- src/api.v2/model/Sample.js | 65 +++++++++++++++++++ src/api.v2/model/UserAccess.js | 10 ++- src/specs/api.v2.yaml | 2 +- 6 files changed, 98 insertions(+), 12 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 6e3791dee..5494b5364 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -8,6 +8,8 @@ const { OK, NotFoundError } = require('../../utils/responses'); const sqlClient = require('../../sql/sqlClient'); const getExperimentBackendStatus = require('../helpers/backendStatus/getExperimentBackendStatus'); +const Sample = require('../model/Sample'); +const MetadataTrack = require('../model/MetadataTrack'); const logger = getLogger('[ExperimentController] - '); @@ -141,16 +143,28 @@ const downloadData = async (req, res) => { }; const cloneExperiment = async (req, res) => { - const { experimentId } = req.params; + const { + params: { experimentId: fromExperimentId, toExperimentId }, + } = req; + + logger.log(`Cloning experiment ${fromExperimentId} into ${toExperimentId}`); - logger.log(`Cloning experiment ${experimentId}`); + // toExperiment might have metadata tracks, + // we want to start from an empty one so clear them out + await new MetadataTrack().delete({ experiment_id: toExperimentId }); - const cloneExperimentId = await new Experiment().createClone(experimentId); - // const downloadLink = await new SampleFile().getDownloadLink(experimentId, downloadType); + const { samplesOrder } = await new Experiment().findById(fromExperimentId).first(); - logger.log(`Finished cloning experiment ${experimentId}, new expeirment's id is ${cloneExperimentId}`); + const newSampleIds = await new Sample().copyTo(fromExperimentId, toExperimentId, samplesOrder); - res.json(cloneExperimentId); + await new Experiment().updateById( + toExperimentId, + { samples_order: JSON.stringify(newSampleIds) }, + ); + + logger.log(`Finished cloning experiment ${fromExperimentId}, new expeirment's id is ${toExperimentId}`); + + res.json(OK()); }; module.exports = { diff --git a/src/api.v2/helpers/roles.js b/src/api.v2/helpers/roles.js index 084dd2f25..653caadb4 100644 --- a/src/api.v2/helpers/roles.js +++ b/src/api.v2/helpers/roles.js @@ -18,9 +18,10 @@ const allowedResources = { 'socket', '/experiments/(?.*)/plots-tables/(?.*)', '/experiments/(?.*)/cellSets', + '/experiments/(?.*)/clone/.*', ], [VIEWER]: [ - '/experiments/(?.*)/clone', + '/experiments/(?.*)/clone/.*', ], }; const isRoleAuthorized = (role, resource, method) => { diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 107a93626..3ea4cfed3 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -60,8 +60,8 @@ class Experiment extends BasicModel { } async getAllExampleExperiments() { - const noOwnerExperiments = '00000000-0000-0000-0000-000000000000'; - return this.getAllExperiments(noOwnerExperiments); + const publicAccessId = '00000000-0000-0000-0000-000000000000'; + return this.getAllExperiments(publicAccessId); } async getExperimentData(experimentId) { diff --git a/src/api.v2/model/Sample.js b/src/api.v2/model/Sample.js index 47d9bc955..dec7d534f 100644 --- a/src/api.v2/model/Sample.js +++ b/src/api.v2/model/Sample.js @@ -1,3 +1,7 @@ +const _ = require('lodash'); + +const { v4: uuidv4 } = require('uuid'); + const BasicModel = require('./BasicModel'); const sqlClient = require('../../sql/sqlClient'); @@ -86,6 +90,67 @@ class Sample extends BasicModel { ); }); } + + /** + * Creates copy samples from one experiment to another one + * + * @param {*} fromExperimentId + * @param {*} toExperimentId + */ + async copyTo(fromExperimentId, toExperimentId, samplesOrder) { + const result = await this.getSamples(fromExperimentId); + + const newSampleIds = []; + + const metadataTrackKeys = Object.keys(result[0].metadata); + + const sampleRows = []; + const sampleFileMapRows = []; + const metadataValueMapRows = []; + + await this.sql.transaction(async (trx) => { + const metadataTracks = await trx(tableNames.METADATA_TRACK) + .insert(metadataTrackKeys.map((key) => ({ experiment_id: toExperimentId, key }))) + .returning(['id', 'key']); + + // Copy each sample in order so + // the new samples we create follow the same order + samplesOrder.forEach((fromSampleId) => { + const sample = result.find(({ id }) => id === fromSampleId); + + const toSampleId = uuidv4(); + + newSampleIds.push(toSampleId); + + sampleRows.push({ + id: toSampleId, + experiment_id: toExperimentId, + name: sample.name, + sample_technology: sample.sampleTechnology, + }); + + Object.entries(sample.files).forEach(([, file]) => { + sampleFileMapRows.push({ + sample_id: toSampleId, + sample_file_id: file.id, + }); + }); + + Object.entries(sample.metadata).forEach(([key, value]) => { + console.log('valueDebug'); + console.log(value); + const { id } = _.find(metadataTracks, ({ key: currentKey }) => currentKey === key); + metadataValueMapRows.push({ metadata_track_id: id, sample_id: toSampleId, value }); + }); + }); + + await trx(tableNames.SAMPLE).insert(sampleRows); + await trx(tableNames.SAMPLE_IN_METADATA_TRACK_MAP).insert(metadataValueMapRows); + await trx(tableNames.SAMPLE_TO_SAMPLE_FILE_MAP).insert(sampleFileMapRows); + }); + + return newSampleIds; + } } module.exports = Sample; diff --git a/src/api.v2/model/UserAccess.js b/src/api.v2/model/UserAccess.js index 26b977b2b..e7d866f79 100644 --- a/src/api.v2/model/UserAccess.js +++ b/src/api.v2/model/UserAccess.js @@ -91,11 +91,17 @@ class UserAccess extends BasicModel { await this.grantAccess(userId, experimentId, AccessRole.OWNER); } + async canAccessExperiment(userId, experimentId, url, method) { + const publicAccessId = '00000000-0000-0000-0000-000000000000'; + const result = await this.sql(tableNames.USER_ACCESS) - .first() + // Check if user has access .where({ experiment_id: experimentId, user_id: userId }) - .from(tableNames.USER_ACCESS); + // Or if it is a public access experiment + .orWhere({ experiment_id: experimentId, user_id: publicAccessId }) + .from(tableNames.USER_ACCESS) + .first(); // If there is no entry for this user and role, then user definitely doesn't have access if (_.isNil(result)) { diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 7559bc94b..7e6081686 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -971,7 +971,7 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPError' - '/experiments/{experimentId}/clone': + '/experiments/{experimentId}/clone/{toExperimentId}': post: summary: Clone an experiment description: Clone an experiment, returns the id of the new experiment From ffb177a4c8aaac9e51db5339325226e9538ab171 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Tue, 21 Jun 2022 15:49:53 -0300 Subject: [PATCH 04/29] Cleanup --- src/api.v2/helpers/worker/getWorkResults.js | 1 - src/api.v2/model/Sample.js | 2 -- src/specs/api.v2.yaml | 2 +- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/api.v2/helpers/worker/getWorkResults.js b/src/api.v2/helpers/worker/getWorkResults.js index 0c1da07f5..20b7e3721 100644 --- a/src/api.v2/helpers/worker/getWorkResults.js +++ b/src/api.v2/helpers/worker/getWorkResults.js @@ -52,7 +52,6 @@ const getWorkResults = async (experimentId, ETag) => { const formattedExperimentId = formatExperimentId(experimentId); await validateTagMatching(formattedExperimentId, params); - console.log('GETTING WORK RESULTS IWTH PARAMS ', params); logger.log(`Found worker results for experiment: ${experimentId}, Etag: ${ETag}`); const signedUrl = getSignedUrl('getObject', params); diff --git a/src/api.v2/model/Sample.js b/src/api.v2/model/Sample.js index dec7d534f..fb79303a9 100644 --- a/src/api.v2/model/Sample.js +++ b/src/api.v2/model/Sample.js @@ -137,8 +137,6 @@ class Sample extends BasicModel { }); Object.entries(sample.metadata).forEach(([key, value]) => { - console.log('valueDebug'); - console.log(value); const { id } = _.find(metadataTracks, ({ key: currentKey }) => currentKey === key); metadataValueMapRows.push({ metadata_track_id: id, sample_id: toSampleId, value }); }); diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 7e6081686..438c1e63c 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -984,7 +984,7 @@ paths: content: application/json: schema: - type: string + $ref: '#/components/schemas/HTTPSuccess' '400': description: Bad Request content: From f61c436618317d0af166c85f3f40e7954983cc46 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 09:48:43 -0300 Subject: [PATCH 05/29] Minor readbilitity improvement --- src/api.v2/controllers/experimentController.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 5494b5364..09a814077 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -153,13 +153,15 @@ const cloneExperiment = async (req, res) => { // we want to start from an empty one so clear them out await new MetadataTrack().delete({ experiment_id: toExperimentId }); + // Get samples order so we preserve the relative order of the original samples in the cloned ones const { samplesOrder } = await new Experiment().findById(fromExperimentId).first(); - const newSampleIds = await new Sample().copyTo(fromExperimentId, toExperimentId, samplesOrder); + const cloneSamplesOrder = await new Sample() + .copyTo(fromExperimentId, toExperimentId, samplesOrder); await new Experiment().updateById( toExperimentId, - { samples_order: JSON.stringify(newSampleIds) }, + { samples_order: JSON.stringify(cloneSamplesOrder) }, ); logger.log(`Finished cloning experiment ${fromExperimentId}, new expeirment's id is ${toExperimentId}`); From 79f7c3fbac7649c737c5afa4856fbbad16d9ab65 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 12:03:58 -0300 Subject: [PATCH 06/29] Update snapshots --- tests/api.v2/model/__snapshots__/Sample.test.js.snap | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/api.v2/model/__snapshots__/Sample.test.js.snap b/tests/api.v2/model/__snapshots__/Sample.test.js.snap index 848e3a747..53141e68e 100644 --- a/tests/api.v2/model/__snapshots__/Sample.test.js.snap +++ b/tests/api.v2/model/__snapshots__/Sample.test.js.snap @@ -74,6 +74,7 @@ Array [ "sf.size", "sf.upload_status", "sf.s3_path", + "sf.id as sf_id", "s.id", ], ], @@ -127,7 +128,7 @@ Array [ "undefined as metadata", ], Array [ - "jsonb_object_agg(sample_file_type,json_build_object('sample_file_type',sample_file_type,'size',size,'upload_status',upload_status,'s3_path',s3_path)) as files", + "jsonb_object_agg(sample_file_type,json_build_object('sample_file_type',sample_file_type,'size',size,'upload_status',upload_status,'s3_path',s3_path,'id',sf_id)) as files", ], ] `; From 010d4a1c3af497c1f94f4d445451f85e34b3299d Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 13:17:28 -0300 Subject: [PATCH 07/29] Fix tests and minor refactoreing --- src/api.v2/model/Experiment.js | 4 ++-- src/api.v2/model/UserAccess.js | 5 ++--- src/utils/constants.js | 3 +++ tests/api.v2/mocks/getMockSqlClient.js | 1 + tests/api.v2/model/UserAccess.test.js | 20 +++++++++++++++----- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 3ea4cfed3..c6f7dc339 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -12,6 +12,7 @@ const config = require('../../config'); const getLogger = require('../../utils/getLogger'); const bucketNames = require('../helpers/s3/bucketNames'); const { getSignedUrl } = require('../../utils/aws/s3'); +const constants = require('../../utils/constants'); const logger = getLogger('[ExperimentModel] - '); @@ -60,8 +61,7 @@ class Experiment extends BasicModel { } async getAllExampleExperiments() { - const publicAccessId = '00000000-0000-0000-0000-000000000000'; - return this.getAllExperiments(publicAccessId); + return this.getAllExperiments(constants.PUBLIC_ACCESS_ID); } async getExperimentData(experimentId) { diff --git a/src/api.v2/model/UserAccess.js b/src/api.v2/model/UserAccess.js index e7d866f79..7c04cc586 100644 --- a/src/api.v2/model/UserAccess.js +++ b/src/api.v2/model/UserAccess.js @@ -19,6 +19,7 @@ const selectableProps = [ ]; const getLogger = require('../../utils/getLogger'); +const constants = require('../../utils/constants'); const logger = getLogger('[UserAccessModel] - '); @@ -93,13 +94,11 @@ class UserAccess extends BasicModel { async canAccessExperiment(userId, experimentId, url, method) { - const publicAccessId = '00000000-0000-0000-0000-000000000000'; - const result = await this.sql(tableNames.USER_ACCESS) // Check if user has access .where({ experiment_id: experimentId, user_id: userId }) // Or if it is a public access experiment - .orWhere({ experiment_id: experimentId, user_id: publicAccessId }) + .orWhere({ experiment_id: experimentId, user_id: constants.PUBLIC_ACCESS_ID }) .from(tableNames.USER_ACCESS) .first(); diff --git a/src/utils/constants.js b/src/utils/constants.js index 62032e0c4..681852d9b 100644 --- a/src/utils/constants.js +++ b/src/utils/constants.js @@ -23,6 +23,8 @@ const NOT_CREATED = 'NOT_CREATED'; const EXECUTION_DOES_NOT_EXIST = 'ExecutionDoesNotExist'; const ACCESS_DENIED = 'AccessDeniedException'; +const PUBLIC_ACCESS_ID = '00000000-0000-0000-0000-000000000000'; + module.exports = { QC_PROCESS_NAME, GEM2S_PROCESS_NAME, @@ -36,4 +38,5 @@ module.exports = { EXECUTION_DOES_NOT_EXIST, ACCESS_DENIED, ASSIGN_POD_TO_PIPELINE, + PUBLIC_ACCESS_ID, }; diff --git a/tests/api.v2/mocks/getMockSqlClient.js b/tests/api.v2/mocks/getMockSqlClient.js index 7251b08e3..5f84c904d 100644 --- a/tests/api.v2/mocks/getMockSqlClient.js +++ b/tests/api.v2/mocks/getMockSqlClient.js @@ -20,6 +20,7 @@ module.exports = () => { into: jest.fn().mockReturnThis(), timeout: jest.fn().mockReturnThis(), andWhere: jest.fn().mockReturnThis(), + orWhere: jest.fn().mockReturnThis(), whereExists: jest.fn().mockReturnThis(), whereIn: jest.fn().mockReturnThis(), queryContext: jest.fn().mockReturnThis(), diff --git a/tests/api.v2/model/UserAccess.test.js b/tests/api.v2/model/UserAccess.test.js index 5c4d9d213..c80d046db 100644 --- a/tests/api.v2/model/UserAccess.test.js +++ b/tests/api.v2/model/UserAccess.test.js @@ -22,6 +22,7 @@ jest.mock('../../../src/utils/aws/user', () => ({ const BasicModel = require('../../../src/api.v2/model/BasicModel'); const UserAccess = require('../../../src/api.v2/model/UserAccess'); const AccessRole = require('../../../src/utils/enums/AccessRole'); +const constants = require('../../../src/utils/constants'); const mockAdminUserId = testConfig.adminSub; const mockUserId = '1234-5678-9012-1234'; @@ -200,18 +201,21 @@ describe('model/userAccess', () => { const url = 'url'; const method = 'method'; - mockSqlClient.from.mockImplementationOnce(() => ({ accessRole: 'roleThatIsOk' })); + mockSqlClient.first.mockImplementationOnce(() => ({ accessRole: 'roleThatIsOk' })); roles.isRoleAuthorized.mockImplementationOnce(() => true); - const result = await new UserAccess().canAccessExperiment(mockUserId, mockExperimentId, url, method); - + const result = await new UserAccess() + .canAccessExperiment(mockUserId, mockExperimentId, url, method); expect(mockSqlClient.first).toHaveBeenCalled(); expect(mockSqlClient.from).toHaveBeenCalledWith('user_access'); expect(mockSqlClient.where).toHaveBeenCalledWith( { experiment_id: mockExperimentId, user_id: mockUserId }, ); + expect(mockSqlClient.orWhere).toHaveBeenCalledWith( + { experiment_id: mockExperimentId, user_id: constants.PUBLIC_ACCESS_ID }, + ); expect(roles.isRoleAuthorized).toHaveBeenCalledWith('roleThatIsOk', url, method); expect(result).toEqual(true); @@ -221,7 +225,7 @@ describe('model/userAccess', () => { const url = 'url'; const method = 'method'; - mockSqlClient.from.mockImplementationOnce(() => undefined); + mockSqlClient.first.mockImplementationOnce(() => undefined); const result = await new UserAccess().canAccessExperiment(mockUserId, mockExperimentId, url, method); @@ -230,6 +234,9 @@ describe('model/userAccess', () => { expect(mockSqlClient.where).toHaveBeenCalledWith( { experiment_id: mockExperimentId, user_id: mockUserId }, ); + expect(mockSqlClient.orWhere).toHaveBeenCalledWith( + { experiment_id: mockExperimentId, user_id: constants.PUBLIC_ACCESS_ID }, + ); expect(roles.isRoleAuthorized).not.toHaveBeenCalled(); @@ -240,7 +247,7 @@ describe('model/userAccess', () => { const url = 'url'; const method = 'method'; - mockSqlClient.from.mockImplementationOnce(() => ({ accessRole: 'roleThatIsNotOk' })); + mockSqlClient.first.mockImplementationOnce(() => ({ accessRole: 'roleThatIsNotOk' })); roles.isRoleAuthorized.mockImplementationOnce(() => false); @@ -251,6 +258,9 @@ describe('model/userAccess', () => { expect(mockSqlClient.where).toHaveBeenCalledWith( { experiment_id: mockExperimentId, user_id: mockUserId }, ); + expect(mockSqlClient.orWhere).toHaveBeenCalledWith( + { experiment_id: mockExperimentId, user_id: constants.PUBLIC_ACCESS_ID }, + ); expect(roles.isRoleAuthorized).toHaveBeenCalledWith('roleThatIsNotOk', url, method); From 6c573c483e4f4d4774e8557d6bc3b91302c488f2 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 13:22:13 -0300 Subject: [PATCH 08/29] Update model/experiment tests --- tests/api.v2/model/Experiment.test.js | 15 +------------- .../__snapshots__/Experiment.test.js.snap | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/tests/api.v2/model/Experiment.test.js b/tests/api.v2/model/Experiment.test.js index d1abfea52..3dfaee51b 100644 --- a/tests/api.v2/model/Experiment.test.js +++ b/tests/api.v2/model/Experiment.test.js @@ -47,20 +47,7 @@ describe('model/Experiment', () => { expect(queryResult).toEqual(expectedResult); expect(sqlClient.get).toHaveBeenCalled(); - expect(helpers.collapseKeyIntoArray).toHaveBeenCalledWith( - expect.any(Function), - ['id', 'name', 'description', 'samples_order', 'notify_by_email', 'created_at', 'updated_at'], - 'key', - 'metadataKeys', - mockSqlClient, - ); - - - // Check that mainQuery is correct - const mainQuery = helpers.collapseKeyIntoArray.mock.calls[0][0]; - - jest.clearAllMocks(); - await mainQuery.bind(mockSqlClient)(); + expect(helpers.collapseKeyIntoArray.mock.calls).toMatchSnapshot(); expect(mockSqlClient.select).toHaveBeenCalledWith( ['e.id', 'e.name', 'e.description', 'e.samples_order', 'e.notify_by_email', 'e.created_at', 'e.updated_at', 'm.key'], diff --git a/tests/api.v2/model/__snapshots__/Experiment.test.js.snap b/tests/api.v2/model/__snapshots__/Experiment.test.js.snap index 60e0c5761..078a132db 100644 --- a/tests/api.v2/model/__snapshots__/Experiment.test.js.snap +++ b/tests/api.v2/model/__snapshots__/Experiment.test.js.snap @@ -1,5 +1,25 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`model/Experiment getAllExperiments works correctly 1`] = ` +Array [ + Array [ + [MockFunction], + Array [ + "id", + "name", + "description", + "samples_order", + "notify_by_email", + "created_at", + "updated_at", + ], + "key", + "metadataKeys", + [MockFunction], + ], +] +`; + exports[`model/Experiment getExperimentData works correctly 1`] = ` Array [ "COALESCE( From 3934bd75388322702fb2d1b86b2378f08c9675ff Mon Sep 17 00:00:00 2001 From: Martin Fosco Date: Wed, 22 Jun 2022 13:50:33 -0300 Subject: [PATCH 09/29] Update experimentController.js --- src/api.v2/controllers/experimentController.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 09a814077..4b8071cbb 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -149,8 +149,8 @@ const cloneExperiment = async (req, res) => { logger.log(`Cloning experiment ${fromExperimentId} into ${toExperimentId}`); - // toExperiment might have metadata tracks, - // we want to start from an empty one so clear them out + // toExperiment might have pre existing metadata tracks, + // we want to start without any old ones so we clear them out await new MetadataTrack().delete({ experiment_id: toExperimentId }); // Get samples order so we preserve the relative order of the original samples in the cloned ones From fb5da33fa36950f2387600b10404ee7aa216010f Mon Sep 17 00:00:00 2001 From: Martin Fosco Date: Wed, 22 Jun 2022 13:56:54 -0300 Subject: [PATCH 10/29] Update experimentController.js --- src/api.v2/controllers/experimentController.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 4b8071cbb..2d46d342f 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -149,8 +149,8 @@ const cloneExperiment = async (req, res) => { logger.log(`Cloning experiment ${fromExperimentId} into ${toExperimentId}`); - // toExperiment might have pre existing metadata tracks, - // we want to start without any old ones so we clear them out + // toExperiment might be an old experiment with metadata tracks, + // we want to start without any old tracks so clear them out await new MetadataTrack().delete({ experiment_id: toExperimentId }); // Get samples order so we preserve the relative order of the original samples in the cloned ones From d02bd4de70557c4f14ba61ef66c08b88fad55769 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 14:14:30 -0300 Subject: [PATCH 11/29] Add samplesSubsetIds optional body in clone endpoint --- src/api.v2/controllers/experimentController.js | 12 +++++++++--- src/specs/api.v2.yaml | 12 ++++++++++++ tests/api.v2/model/Sample.test.js | 1 + 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 2d46d342f..611e5306b 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -145,6 +145,7 @@ const downloadData = async (req, res) => { const cloneExperiment = async (req, res) => { const { params: { experimentId: fromExperimentId, toExperimentId }, + body: { samplesSubsetIds = null }, } = req; logger.log(`Cloning experiment ${fromExperimentId} into ${toExperimentId}`); @@ -153,11 +154,16 @@ const cloneExperiment = async (req, res) => { // we want to start without any old tracks so clear them out await new MetadataTrack().delete({ experiment_id: toExperimentId }); - // Get samples order so we preserve the relative order of the original samples in the cloned ones - const { samplesOrder } = await new Experiment().findById(fromExperimentId).first(); + let samplesToCloneIds = samplesSubsetIds; + if (!samplesToCloneIds) { + // Get samples order so we preserve + // the relative order of the original samples in the cloned ones + const { samplesOrder } = await new Experiment().findById(fromExperimentId).first(); + samplesToCloneIds = samplesOrder; + } const cloneSamplesOrder = await new Sample() - .copyTo(fromExperimentId, toExperimentId, samplesOrder); + .copyTo(fromExperimentId, toExperimentId, samplesToCloneIds); await new Experiment().updateById( toExperimentId, diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index ea910b6db..47e1c8105 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1193,6 +1193,18 @@ paths: operationId: cloneExperiment x-eov-operation-id: experiment#clone x-eov-operation-handler: routes/experiment + requestBody: + content: + application/json: + schema: + description: If samplesSubsetIds is sent then the cloned experiment only contains the subset samples instead of all of them + type: object + properties: + samplesSubsetIds: + type: array + items: + type: string + additionalProperties: false responses: '200': description: Clone experiment diff --git a/tests/api.v2/model/Sample.test.js b/tests/api.v2/model/Sample.test.js index 3c9d4e1e8..722db48c9 100644 --- a/tests/api.v2/model/Sample.test.js +++ b/tests/api.v2/model/Sample.test.js @@ -1,3 +1,4 @@ +// @ts-nocheck const { mockSqlClient, mockTrx } = require('../mocks/getMockSqlClient')(); const Sample = require('../../../src/api.v2/model/Sample'); From 8dd4956aecfb1c08a7e4b5332c84571d4d335c6e Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 14:18:26 -0300 Subject: [PATCH 12/29] Rename --- src/api.v2/controllers/experimentController.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 611e5306b..dee7e78f5 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -162,12 +162,12 @@ const cloneExperiment = async (req, res) => { samplesToCloneIds = samplesOrder; } - const cloneSamplesOrder = await new Sample() + const clonedSamplesOrder = await new Sample() .copyTo(fromExperimentId, toExperimentId, samplesToCloneIds); await new Experiment().updateById( toExperimentId, - { samples_order: JSON.stringify(cloneSamplesOrder) }, + { samples_order: JSON.stringify(clonedSamplesOrder) }, ); logger.log(`Finished cloning experiment ${fromExperimentId}, new expeirment's id is ${toExperimentId}`); From 9796c2a0d28107500e15432c496c5ceb15681d93 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 14:32:58 -0300 Subject: [PATCH 13/29] Refactor way in which we get the sample ids --- src/api.v2/controllers/experimentController.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index dee7e78f5..dbb50998b 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -142,10 +142,16 @@ const downloadData = async (req, res) => { res.json(downloadLink); }; + const cloneExperiment = async (req, res) => { + const getAllSampleIds = async (experimentId) => { + const { samplesOrder } = await new Experiment().findById(experimentId).first(); + return samplesOrder; + }; + const { params: { experimentId: fromExperimentId, toExperimentId }, - body: { samplesSubsetIds = null }, + body: { samplesSubsetIds = await getAllSampleIds(fromExperimentId) }, } = req; logger.log(`Cloning experiment ${fromExperimentId} into ${toExperimentId}`); @@ -154,16 +160,8 @@ const cloneExperiment = async (req, res) => { // we want to start without any old tracks so clear them out await new MetadataTrack().delete({ experiment_id: toExperimentId }); - let samplesToCloneIds = samplesSubsetIds; - if (!samplesToCloneIds) { - // Get samples order so we preserve - // the relative order of the original samples in the cloned ones - const { samplesOrder } = await new Experiment().findById(fromExperimentId).first(); - samplesToCloneIds = samplesOrder; - } - const clonedSamplesOrder = await new Sample() - .copyTo(fromExperimentId, toExperimentId, samplesToCloneIds); + .copyTo(fromExperimentId, toExperimentId, samplesSubsetIds); await new Experiment().updateById( toExperimentId, From 5d3390d0ff34c0eb426e532c1f144f53bfd406d5 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 14:53:38 -0300 Subject: [PATCH 14/29] Add model/Sample copyTo tests --- .../api.v2/mocks/data/getSamplesResponse.json | 70 ++++++++++++++++ tests/api.v2/model/Sample.test.js | 27 ++++++ .../model/__snapshots__/Sample.test.js.snap | 82 +++++++++++++++++++ 3 files changed, 179 insertions(+) create mode 100644 tests/api.v2/mocks/data/getSamplesResponse.json diff --git a/tests/api.v2/mocks/data/getSamplesResponse.json b/tests/api.v2/mocks/data/getSamplesResponse.json new file mode 100644 index 000000000..3d2aeaddf --- /dev/null +++ b/tests/api.v2/mocks/data/getSamplesResponse.json @@ -0,0 +1,70 @@ +[ + { + "id": "4058acff-3dd4-4a0b-9bff-bb4c1b9a69e5", + "experimentId": "d3031abeeac4aaf94f05a9b543779e09", + "name": "KO", + "sampleTechnology": "10x", + "createdAt": "2022-06-22 17:35:40.427554+00", + "updatedAt": "2022-06-22 17:35:40.427554+00", + "metadata": { + "Track0": "N.A.1" + }, + "files": { + "matrix10x": { + "id": "2afe253f-5542-4a90-9fc9-17acdc3ecaf1", + "size": 25581681, + "s3Path": "2afe253f-5542-4a90-9fc9-17acdc3ecaf1", + "uploadStatus": "uploaded", + "sampleFileType": "matrix10x" + }, + "barcodes10x": { + "id": "59b8221d-dd3f-4bb9-ad9d-1166471e7143", + "size": 21715, + "s3Path": "59b8221d-dd3f-4bb9-ad9d-1166471e7143", + "uploadStatus": "uploaded", + "sampleFileType": "barcodes10x" + }, + "features10x": { + "id": "415323cb-72dc-4f42-b452-c12422ea042f", + "size": 279361, + "s3Path": "415323cb-72dc-4f42-b452-c12422ea042f", + "uploadStatus": "uploaded", + "sampleFileType": "features10x" + } + } + }, + { + "id": "908b7d7a-e1fe-4285-82b0-47c4cf7cc2bf", + "experimentId": "d3031abeeac4aaf94f05a9b543779e09", + "name": "WT1", + "sampleTechnology": "10x", + "createdAt": "2022-06-22 17:44:02.155432+00", + "updatedAt": "2022-06-22 17:44:02.155432+00", + "metadata": { + "Track0": "N.A." + }, + "files": { + "matrix10x": { + "id": "d5c957b9-a6cb-4828-8ee6-1f9da8cacb6a", + "size": 5079737, + "s3Path": "d5c957b9-a6cb-4828-8ee6-1f9da8cacb6a", + "uploadStatus": "uploaded", + "sampleFileType": "matrix10x" + }, + "barcodes10x": { + "id": "d52a84a8-4669-41b3-a8d1-38e16ddbf522", + "size": 5331, + "s3Path": "d52a84a8-4669-41b3-a8d1-38e16ddbf522", + "uploadStatus": "uploaded", + "sampleFileType": "barcodes10x" + }, + "features10x": { + "id": "5b8ec36c-0f06-4a65-b924-da5189c89b7b", + "size": 279361, + "s3Path": "5b8ec36c-0f06-4a65-b924-da5189c89b7b", + "uploadStatus": "uploaded", + "sampleFileType": "features10x" + } + } + } +] \ No newline at end of file diff --git a/tests/api.v2/model/Sample.test.js b/tests/api.v2/model/Sample.test.js index 722db48c9..c351a74b3 100644 --- a/tests/api.v2/model/Sample.test.js +++ b/tests/api.v2/model/Sample.test.js @@ -3,6 +3,8 @@ const { mockSqlClient, mockTrx } = require('../mocks/getMockSqlClient')(); const Sample = require('../../../src/api.v2/model/Sample'); +const getSamplesResponse = require('../mocks/data/getSamplesResponse.json'); + const mockExperimentId = 'mockExperimentId'; // @ts-nocheck // Disabled ts because it doesn't recognize jest mocks @@ -65,4 +67,29 @@ describe('model/Sample', () => { sample_file_id: mockSampleFileId, }); }); + + it('copyTo works correctly if valid params are passed', async () => { + const fromExperimentId = 'fromExperimentIdMock'; + const toExperimentId = 'toExperimentIdMock'; + const samplesOrder = Object.values(getSamplesResponse).map((sample) => sample.id); + + mockTrx.returning.mockImplementationOnce(() => Promise.resolve([{ id: 0, key: 'Track0' }])); + + const getSamplesSpy = jest.spyOn(Sample.prototype, 'getSamples') + .mockImplementationOnce(() => Promise.resolve(getSamplesResponse)); + + + await new Sample().copyTo(fromExperimentId, toExperimentId, samplesOrder); + + + expect(getSamplesSpy).toHaveBeenCalledWith(fromExperimentId); + + expect(mockTrx).toHaveBeenCalledWith(tableNames.METADATA_TRACK); + expect(mockTrx).toHaveBeenCalledWith(tableNames.SAMPLE); + expect(mockTrx).toHaveBeenCalledWith(tableNames.SAMPLE_IN_METADATA_TRACK_MAP); + expect(mockTrx).toHaveBeenCalledWith(tableNames.SAMPLE_TO_SAMPLE_FILE_MAP); + + expect(mockTrx.insert.mock.calls).toMatchSnapshot(); + expect(mockTrx.returning.mock.calls).toMatchSnapshot(); + }); }); diff --git a/tests/api.v2/model/__snapshots__/Sample.test.js.snap b/tests/api.v2/model/__snapshots__/Sample.test.js.snap index 53141e68e..e435474a2 100644 --- a/tests/api.v2/model/__snapshots__/Sample.test.js.snap +++ b/tests/api.v2/model/__snapshots__/Sample.test.js.snap @@ -149,3 +149,85 @@ Array [ ], ] `; + +exports[`model/Sample copyTo works correctly if valid params are passed 1`] = ` +Array [ + Array [ + Array [ + Object { + "experiment_id": "toExperimentIdMock", + "key": "Track0", + }, + ], + ], + Array [ + Array [ + Object { + "experiment_id": "toExperimentIdMock", + "id": "mock-uuid", + "name": "KO", + "sample_technology": "10x", + }, + Object { + "experiment_id": "toExperimentIdMock", + "id": "mock-uuid", + "name": "WT1", + "sample_technology": "10x", + }, + ], + ], + Array [ + Array [ + Object { + "metadata_track_id": 0, + "sample_id": "mock-uuid", + "value": "N.A.1", + }, + Object { + "metadata_track_id": 0, + "sample_id": "mock-uuid", + "value": "N.A.", + }, + ], + ], + Array [ + Array [ + Object { + "sample_file_id": "2afe253f-5542-4a90-9fc9-17acdc3ecaf1", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "59b8221d-dd3f-4bb9-ad9d-1166471e7143", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "415323cb-72dc-4f42-b452-c12422ea042f", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "d5c957b9-a6cb-4828-8ee6-1f9da8cacb6a", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "d52a84a8-4669-41b3-a8d1-38e16ddbf522", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "5b8ec36c-0f06-4a65-b924-da5189c89b7b", + "sample_id": "mock-uuid", + }, + ], + ], +] +`; + +exports[`model/Sample copyTo works correctly if valid params are passed 2`] = ` +Array [ + Array [ + Array [ + "id", + "key", + ], + ], +] +`; From 1c0ca31ded9d72ebab5f60b123f18a7928ca1b10 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 14:57:48 -0300 Subject: [PATCH 15/29] Add getAllExampleExperiments test --- tests/api.v2/model/Experiment.test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/api.v2/model/Experiment.test.js b/tests/api.v2/model/Experiment.test.js index 3dfaee51b..1a0722b04 100644 --- a/tests/api.v2/model/Experiment.test.js +++ b/tests/api.v2/model/Experiment.test.js @@ -27,6 +27,7 @@ jest.mock('../../../src/sql/helpers', () => ({ })); const Experiment = require('../../../src/api.v2/model/Experiment'); +const constants = require('../../../src/utils/constants'); const mockExperimentId = 'mockExperimentId'; const mockSampleId = 'mockSampleId'; @@ -59,6 +60,18 @@ describe('model/Experiment', () => { expect(mockSqlClient.as).toHaveBeenCalledWith('mainQuery'); }); + it('getAllExampleExperiments works correctly', async () => { + const expectedResult = { isMockResult: true }; + + const getAllExperimentsSpy = jest.spyOn(Experiment.prototype, 'getAllExperiments') + .mockImplementationOnce(() => Promise.resolve(expectedResult)); + + const result = await new Experiment().getAllExampleExperiments('mockUserId'); + + expect(result).toBe(expectedResult); + expect(getAllExperimentsSpy).toHaveBeenCalledWith(constants.PUBLIC_ACCESS_ID); + }); + it('getExperimentData works correctly', async () => { const experimentFields = [ 'id', 'name', 'description', From 0985cbb33b0034e385cbbfccfbc12e83325bba62 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 15:06:52 -0300 Subject: [PATCH 16/29] Add route tests --- tests/api.v2/routes/experiment.test.js | 95 ++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/tests/api.v2/routes/experiment.test.js b/tests/api.v2/routes/experiment.test.js index b4d08c828..0f7cb4629 100644 --- a/tests/api.v2/routes/experiment.test.js +++ b/tests/api.v2/routes/experiment.test.js @@ -1,6 +1,7 @@ // @ts-nocheck const express = require('express'); const request = require('supertest'); +const { send } = require('process'); const expressLoader = require('../../../src/loaders/express'); const { OK } = require('../../../src/utils/responses'); @@ -21,6 +22,8 @@ jest.mock('../../../src/api.v2/controllers/experimentController', () => ({ getProcessingConfig: jest.fn(), updateProcessingConfig: jest.fn(), downloadData: jest.fn(), + getAllExampleExperiments: jest.fn(), + cloneExperiment: jest.fn(), })); jest.mock('../../../src/api.v2/middlewares/authMiddlewares'); @@ -312,4 +315,96 @@ describe('tests for experiment route', () => { return done(); }); }); + + it('getAllExampleExperiments results in a successful response', async (done) => { + experimentController.getAllExampleExperiments.mockImplementationOnce((req, res) => { + res.json(getExperimentResponse); + return Promise.resolve(); + }); + + request(app) + .get('/v2/experiments/examples') + .expect(200) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); + + it('cloneExperiment results in a successful response without a body', async (done) => { + const experimentId = 'fromExperimentId'; + const toExperimentId = 'toExperimentId'; + + experimentController.cloneExperiment.mockImplementationOnce((req, res) => { + res.json(OK()); + return Promise.resolve(); + }); + + request(app) + .post(`/v2/experiments/${experimentId}/clone/${toExperimentId}`) + .set({ 'Content-Type': 'application/json' }) + .expect(200) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); + + it('cloneExperiment results in a successful response with a valid body', async (done) => { + const experimentId = 'fromExperimentId'; + const toExperimentId = 'toExperimentId'; + + const body = { samplesSubsetIds: ['sampleId1', 'sampleId2'] }; + + experimentController.cloneExperiment.mockImplementationOnce((req, res) => { + res.json(OK()); + return Promise.resolve(); + }); + + request(app) + .post(`/v2/experiments/${experimentId}/clone/${toExperimentId}`) + .send(body) + .expect(200) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); + + it('cloneExperiment results in a 400 with an invalid body', async (done) => { + const experimentId = 'fromExperimentId'; + const toExperimentId = 'toExperimentId'; + + const body = { samplesSubsetIdsInvalid: ['sampleId1', 'sampleId2'] }; + + experimentController.cloneExperiment.mockImplementationOnce((req, res) => { + res.json(OK()); + return Promise.resolve(); + }); + + request(app) + .post(`/v2/experiments/${experimentId}/clone/${toExperimentId}`) + .send(body) + .expect(400) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); }); From 10cd9f7f935026b938cdd544053ea631d4a4083a Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 15:10:04 -0300 Subject: [PATCH 17/29] Add some logs --- src/api.v2/controllers/experimentController.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index dbb50998b..6a0f83ae7 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -15,34 +15,36 @@ const logger = getLogger('[ExperimentController] - '); const getAllExperiments = async (req, res) => { const { user: { sub: userId } } = req; + console.log(`Getting all experiments for user: ${userId}`); const data = await new Experiment().getAllExperiments(userId); + console.log(`Finished getting all experiments for user: ${userId}, length: ${data.length}`); res.json(data); }; const getAllExampleExperiments = async (req, res) => { + console.log('Getting example experiments'); + const data = await new Experiment().getAllExampleExperiments(); + console.log(`Finished getting example experiments, length: ${data.length}`); res.json(data); }; const getExperiment = async (req, res) => { const { params: { experimentId } } = req; - logger.log(`Getting experiment ${experimentId}`); const data = await new Experiment().getExperimentData(experimentId); logger.log(`Finished getting experiment ${experimentId}`); - res.json(data); }; const createExperiment = async (req, res) => { const { params: { experimentId }, user, body } = req; const { name, description } = body; - logger.log('Creating experiment'); await sqlClient.get().transaction(async (trx) => { @@ -76,7 +78,6 @@ const deleteExperiment = async (req, res) => { throw new NotFoundError(`Experiment ${experimentId} not found`); } - logger.log(`Finished deleting experiment ${experimentId}`); res.json(OK()); }; From dd3157eb16decaa8411da1fcc9bdde29e4b6f5e1 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 15:12:32 -0300 Subject: [PATCH 18/29] Add tests for experimentController --- src/api.v2/model/__mocks__/Experiment.js | 1 + src/api.v2/model/__mocks__/Sample.js | 1 + .../api.v2/controllers/experimentController.test.js | 13 +++++++++++++ 3 files changed, 15 insertions(+) diff --git a/src/api.v2/model/__mocks__/Experiment.js b/src/api.v2/model/__mocks__/Experiment.js index 0ac3d61e9..2df9fe5aa 100644 --- a/src/api.v2/model/__mocks__/Experiment.js +++ b/src/api.v2/model/__mocks__/Experiment.js @@ -9,6 +9,7 @@ const stub = { addSample: jest.fn(), deleteSample: jest.fn(), getDownloadLink: jest.fn(), + getAllExampleExperiments: jest.fn(), ...BasicModel, }; diff --git a/src/api.v2/model/__mocks__/Sample.js b/src/api.v2/model/__mocks__/Sample.js index a8b8e1688..637a79bcd 100644 --- a/src/api.v2/model/__mocks__/Sample.js +++ b/src/api.v2/model/__mocks__/Sample.js @@ -3,6 +3,7 @@ const BasicModel = require('./BasicModel')(); const stub = { getSamples: jest.fn(), setNewFile: jest.fn(), + copyTo: jest.fn(), ...BasicModel, }; diff --git a/tests/api.v2/controllers/experimentController.test.js b/tests/api.v2/controllers/experimentController.test.js index 0b0feebaa..486181f40 100644 --- a/tests/api.v2/controllers/experimentController.test.js +++ b/tests/api.v2/controllers/experimentController.test.js @@ -70,6 +70,19 @@ describe('experimentController', () => { expect(mockRes.json).toHaveBeenCalledWith(getAllExperimentsResponse); }); + it('getAllExperiments works correctly', async () => { + const mockReq = { user: { sub: 'mockUserId' } }; + + experimentInstance.getAllExampleExperiments.mockImplementationOnce( + () => Promise.resolve(getAllExperimentsResponse), + ); + + await experimentController.getAllExampleExperiments(mockReq, mockRes); + + expect(experimentInstance.getAllExampleExperiments).toHaveBeenCalled(); + expect(mockRes.json).toHaveBeenCalledWith(getAllExperimentsResponse); + }); + it('getExperiment works correctly', async () => { const mockReq = { params: { experimentId: getExperimentResponse.id } }; From 252bf91656ca5f3d5d45ab856eddd9971a8a405b Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 15:33:25 -0300 Subject: [PATCH 19/29] Add experimentController.cloneExperiment tests --- .../controllers/experimentController.test.js | 76 ++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/tests/api.v2/controllers/experimentController.test.js b/tests/api.v2/controllers/experimentController.test.js index 486181f40..674abcef6 100644 --- a/tests/api.v2/controllers/experimentController.test.js +++ b/tests/api.v2/controllers/experimentController.test.js @@ -1,6 +1,8 @@ // @ts-nocheck const Experiment = require('../../../src/api.v2/model/Experiment'); +const Sample = require('../../../src/api.v2/model/Sample'); const UserAccess = require('../../../src/api.v2/model/UserAccess'); +const MetadataTrack = require('../../../src/api.v2/model/MetadataTrack'); const { mockSqlClient, mockTrx } = require('../mocks/getMockSqlClient')(); const getPipelineStatus = require('../../../src/api.v2/helpers/pipeline/getPipelineStatus'); @@ -9,6 +11,8 @@ const getWorkerStatus = require('../../../src/api.v2/helpers/worker/getWorkerSta const bucketNames = require('../../../src/api.v2/helpers/s3/bucketNames'); const experimentInstance = Experiment(); +const sampleTrackInstance = Sample(); +const metadataTrackInstance = MetadataTrack(); const userAccessInstance = UserAccess(); const mockExperiment = { @@ -22,7 +26,10 @@ const mockExperiment = { }; jest.mock('../../../src/api.v2/model/Experiment'); +jest.mock('../../../src/api.v2/model/Sample'); jest.mock('../../../src/api.v2/model/UserAccess'); +jest.mock('../../../src/api.v2/model/MetadataTrack'); + jest.mock('../../../src/sql/sqlClient', () => ({ get: jest.fn(() => mockSqlClient), })); @@ -70,7 +77,7 @@ describe('experimentController', () => { expect(mockRes.json).toHaveBeenCalledWith(getAllExperimentsResponse); }); - it('getAllExperiments works correctly', async () => { + it('getAllExampleExperiments works correctly', async () => { const mockReq = { user: { sub: 'mockUserId' } }; experimentInstance.getAllExampleExperiments.mockImplementationOnce( @@ -279,4 +286,71 @@ describe('experimentController', () => { expect(experimentInstance.getDownloadLink) .toHaveBeenCalledWith(mockExperiment.id, bucketNames.PROCESSED_MATRIX); }); + + it('cloneExperiment works correctly when samplesSubsetIds is provided', async () => { + const toExperimentId = 'toExperimentIdMock'; + const samplesSubsetIds = ['mockSample2', 'mockSample3']; + const clonedSamplesSubsetIds = ['mockClonedSample2', 'mockClonedSample3']; + + const mockReq = { + params: { experimentId: mockExperiment.id, toExperimentId }, + body: { samplesSubsetIds }, + }; + + metadataTrackInstance.delete.mockImplementationOnce(() => Promise.resolve()); + sampleTrackInstance.copyTo.mockImplementationOnce( + () => Promise.resolve(clonedSamplesSubsetIds), + ); + experimentInstance.updateById.mockImplementationOnce(() => Promise.resolve()); + + await experimentController.cloneExperiment(mockReq, mockRes); + + expect(metadataTrackInstance.delete).toHaveBeenCalledWith({ experiment_id: toExperimentId }); + + expect(sampleTrackInstance.copyTo) + .toHaveBeenCalledWith(mockExperiment.id, toExperimentId, samplesSubsetIds); + + expect(experimentInstance.updateById).toHaveBeenCalledWith( + toExperimentId, + { samples_order: JSON.stringify(clonedSamplesSubsetIds) }, + ); + + expect(mockRes.json).toHaveBeenCalledWith(OK()); + }); + + it('cloneExperiment works correctly when samplesSubsetIds is NOT provided', async () => { + const toExperimentId = 'toExperimentIdMock'; + const allSampleIds = ['mockSample1', 'mockSample2', 'mockSample3', 'mockSample4']; + const clonedSamplesIds = ['mockClonedSample1', 'mockClonedSample2', 'mockClonedSample3', 'mockClonedSample4']; + + const mockReq = { + params: { experimentId: mockExperiment.id, toExperimentId }, + body: {}, + }; + + experimentInstance.findById.mockReturnValueOnce( + { first: () => Promise.resolve({ samplesOrder: allSampleIds }) }, + ); + metadataTrackInstance.delete.mockImplementationOnce(() => Promise.resolve()); + sampleTrackInstance.copyTo.mockImplementationOnce( + () => Promise.resolve(clonedSamplesIds), + ); + experimentInstance.updateById.mockImplementationOnce(() => Promise.resolve()); + + await experimentController.cloneExperiment(mockReq, mockRes); + + expect(experimentInstance.findById).toHaveBeenCalledWith(mockExperiment.id); + + expect(metadataTrackInstance.delete).toHaveBeenCalledWith({ experiment_id: toExperimentId }); + + expect(sampleTrackInstance.copyTo) + .toHaveBeenCalledWith(mockExperiment.id, toExperimentId, allSampleIds); + + expect(experimentInstance.updateById).toHaveBeenCalledWith( + toExperimentId, + { samples_order: JSON.stringify(clonedSamplesIds) }, + ); + + expect(mockRes.json).toHaveBeenCalledWith(OK()); + }); }); From b31ecbc6df0e29c58ea55f0d58a0f9ebd03bdcf2 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 16:20:35 -0300 Subject: [PATCH 20/29] Add check on any metadata tracks existing before doing queries based on them --- src/api.v2/model/Sample.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/api.v2/model/Sample.js b/src/api.v2/model/Sample.js index fb79303a9..65cb1dd54 100644 --- a/src/api.v2/model/Sample.js +++ b/src/api.v2/model/Sample.js @@ -109,9 +109,12 @@ class Sample extends BasicModel { const metadataValueMapRows = []; await this.sql.transaction(async (trx) => { - const metadataTracks = await trx(tableNames.METADATA_TRACK) - .insert(metadataTrackKeys.map((key) => ({ experiment_id: toExperimentId, key }))) - .returning(['id', 'key']); + let metadataTracks = []; + if (metadataTrackKeys.length > 0) { + metadataTracks = await trx(tableNames.METADATA_TRACK) + .insert(metadataTrackKeys.map((key) => ({ experiment_id: toExperimentId, key }))) + .returning(['id', 'key']); + } // Copy each sample in order so // the new samples we create follow the same order @@ -143,8 +146,11 @@ class Sample extends BasicModel { }); await trx(tableNames.SAMPLE).insert(sampleRows); - await trx(tableNames.SAMPLE_IN_METADATA_TRACK_MAP).insert(metadataValueMapRows); await trx(tableNames.SAMPLE_TO_SAMPLE_FILE_MAP).insert(sampleFileMapRows); + + if (metadataValueMapRows.length > 0) { + await trx(tableNames.SAMPLE_IN_METADATA_TRACK_MAP).insert(metadataValueMapRows); + } }); return newSampleIds; From 6aa021b9bf75886b01779aaa43b02d5e86594bee Mon Sep 17 00:00:00 2001 From: cosa65 Date: Wed, 22 Jun 2022 16:24:47 -0300 Subject: [PATCH 21/29] Update tests --- tests/api.v2/model/Sample.test.js | 36 ++++++++- .../model/__snapshots__/Sample.test.js.snap | 73 ++++++++++++++++--- 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/tests/api.v2/model/Sample.test.js b/tests/api.v2/model/Sample.test.js index c351a74b3..b32a21c20 100644 --- a/tests/api.v2/model/Sample.test.js +++ b/tests/api.v2/model/Sample.test.js @@ -1,4 +1,6 @@ // @ts-nocheck +const _ = require('lodash'); + const { mockSqlClient, mockTrx } = require('../mocks/getMockSqlClient')(); const Sample = require('../../../src/api.v2/model/Sample'); @@ -68,7 +70,7 @@ describe('model/Sample', () => { }); }); - it('copyTo works correctly if valid params are passed', async () => { + it('copyTo works correctly', async () => { const fromExperimentId = 'fromExperimentIdMock'; const toExperimentId = 'toExperimentIdMock'; const samplesOrder = Object.values(getSamplesResponse).map((sample) => sample.id); @@ -92,4 +94,36 @@ describe('model/Sample', () => { expect(mockTrx.insert.mock.calls).toMatchSnapshot(); expect(mockTrx.returning.mock.calls).toMatchSnapshot(); }); + + it('copyTo works correctly if experiment has no metadata tracks', async () => { + const fromExperimentId = 'fromExperimentIdMock'; + const toExperimentId = 'toExperimentIdMock'; + + const noMetadataGetSamplesResponse = _.cloneDeep(getSamplesResponse); + noMetadataGetSamplesResponse.forEach((sample) => { + // eslint-disable-next-line no-param-reassign + sample.metadata = {}; + }); + + const samplesOrder = Object.values(noMetadataGetSamplesResponse).map((sample) => sample.id); + + mockTrx.returning.mockImplementationOnce(() => Promise.resolve([{ id: 0, key: 'Track0' }])); + + const getSamplesSpy = jest.spyOn(Sample.prototype, 'getSamples') + .mockImplementationOnce(() => Promise.resolve(noMetadataGetSamplesResponse)); + + + await new Sample().copyTo(fromExperimentId, toExperimentId, samplesOrder); + + + expect(getSamplesSpy).toHaveBeenCalledWith(fromExperimentId); + + expect(mockTrx).toHaveBeenCalledWith(tableNames.SAMPLE); + expect(mockTrx).toHaveBeenCalledWith(tableNames.SAMPLE_TO_SAMPLE_FILE_MAP); + expect(mockTrx).not.toHaveBeenCalledWith(tableNames.METADATA_TRACK); + expect(mockTrx).not.toHaveBeenCalledWith(tableNames.SAMPLE_IN_METADATA_TRACK_MAP); + + expect(mockTrx.insert.mock.calls).toMatchSnapshot(); + expect(mockTrx.returning.mock.calls).toMatchSnapshot(); + }); }); diff --git a/tests/api.v2/model/__snapshots__/Sample.test.js.snap b/tests/api.v2/model/__snapshots__/Sample.test.js.snap index e435474a2..07bf150f8 100644 --- a/tests/api.v2/model/__snapshots__/Sample.test.js.snap +++ b/tests/api.v2/model/__snapshots__/Sample.test.js.snap @@ -150,7 +150,7 @@ Array [ ] `; -exports[`model/Sample copyTo works correctly if valid params are passed 1`] = ` +exports[`model/Sample copyTo works correctly 1`] = ` Array [ Array [ Array [ @@ -176,6 +176,34 @@ Array [ }, ], ], + Array [ + Array [ + Object { + "sample_file_id": "2afe253f-5542-4a90-9fc9-17acdc3ecaf1", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "59b8221d-dd3f-4bb9-ad9d-1166471e7143", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "415323cb-72dc-4f42-b452-c12422ea042f", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "d5c957b9-a6cb-4828-8ee6-1f9da8cacb6a", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "d52a84a8-4669-41b3-a8d1-38e16ddbf522", + "sample_id": "mock-uuid", + }, + Object { + "sample_file_id": "5b8ec36c-0f06-4a65-b924-da5189c89b7b", + "sample_id": "mock-uuid", + }, + ], + ], Array [ Array [ Object { @@ -190,6 +218,38 @@ Array [ }, ], ], +] +`; + +exports[`model/Sample copyTo works correctly 2`] = ` +Array [ + Array [ + Array [ + "id", + "key", + ], + ], +] +`; + +exports[`model/Sample copyTo works correctly if experiment has no metadata tracks 1`] = ` +Array [ + Array [ + Array [ + Object { + "experiment_id": "toExperimentIdMock", + "id": "mock-uuid", + "name": "KO", + "sample_technology": "10x", + }, + Object { + "experiment_id": "toExperimentIdMock", + "id": "mock-uuid", + "name": "WT1", + "sample_technology": "10x", + }, + ], + ], Array [ Array [ Object { @@ -221,13 +281,4 @@ Array [ ] `; -exports[`model/Sample copyTo works correctly if valid params are passed 2`] = ` -Array [ - Array [ - Array [ - "id", - "key", - ], - ], -] -`; +exports[`model/Sample copyTo works correctly if experiment has no metadata tracks 2`] = `Array []`; From 617410493cc44af9ae76089de1da8a8a56bf236e Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 23 Jun 2022 11:09:15 -0300 Subject: [PATCH 22/29] Change endpoints based on pols comments --- .../controllers/experimentController.js | 27 +++++++++++-------- src/api.v2/helpers/roles.js | 4 +-- src/api.v2/model/Experiment.js | 21 +++++++++++++++ src/specs/api.v2.yaml | 4 +-- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 6a0f83ae7..30a771db9 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -9,26 +9,25 @@ const sqlClient = require('../../sql/sqlClient'); const getExperimentBackendStatus = require('../helpers/backendStatus/getExperimentBackendStatus'); const Sample = require('../model/Sample'); -const MetadataTrack = require('../model/MetadataTrack'); const logger = getLogger('[ExperimentController] - '); const getAllExperiments = async (req, res) => { const { user: { sub: userId } } = req; - console.log(`Getting all experiments for user: ${userId}`); + logger.log(`Getting all experiments for user: ${userId}`); const data = await new Experiment().getAllExperiments(userId); - console.log(`Finished getting all experiments for user: ${userId}, length: ${data.length}`); + logger.log(`Finished getting all experiments for user: ${userId}, length: ${data.length}`); res.json(data); }; const getAllExampleExperiments = async (req, res) => { - console.log('Getting example experiments'); + logger.log('Getting example experiments'); const data = await new Experiment().getAllExampleExperiments(); - console.log(`Finished getting example experiments, length: ${data.length}`); + logger.log(`Finished getting example experiments, length: ${data.length}`); res.json(data); }; @@ -151,15 +150,21 @@ const cloneExperiment = async (req, res) => { }; const { - params: { experimentId: fromExperimentId, toExperimentId }, + params: { experimentId: fromExperimentId }, body: { samplesSubsetIds = await getAllSampleIds(fromExperimentId) }, + user: { sub: userId }, } = req; - logger.log(`Cloning experiment ${fromExperimentId} into ${toExperimentId}`); + logger.log(`Creating experiment to clone ${fromExperimentId} to`); + + let toExperimentId; - // toExperiment might be an old experiment with metadata tracks, - // we want to start without any old tracks so clear them out - await new MetadataTrack().delete({ experiment_id: toExperimentId }); + await sqlClient.get().transaction(async (trx) => { + toExperimentId = await new Experiment(trx).copyFrom(fromExperimentId); + await new UserAccess(trx).createNewExperimentPermissions(userId, toExperimentId); + }); + + logger.log(`Cloning experiment ${fromExperimentId} into ${toExperimentId}`); const clonedSamplesOrder = await new Sample() .copyTo(fromExperimentId, toExperimentId, samplesSubsetIds); @@ -171,7 +176,7 @@ const cloneExperiment = async (req, res) => { logger.log(`Finished cloning experiment ${fromExperimentId}, new expeirment's id is ${toExperimentId}`); - res.json(OK()); + res.json(toExperimentId); }; module.exports = { diff --git a/src/api.v2/helpers/roles.js b/src/api.v2/helpers/roles.js index 653caadb4..d3aa051b0 100644 --- a/src/api.v2/helpers/roles.js +++ b/src/api.v2/helpers/roles.js @@ -18,10 +18,10 @@ const allowedResources = { 'socket', '/experiments/(?.*)/plots-tables/(?.*)', '/experiments/(?.*)/cellSets', - '/experiments/(?.*)/clone/.*', + '/experiments/(?.*)/clone', ], [VIEWER]: [ - '/experiments/(?.*)/clone/.*', + '/experiments/(?.*)/clone', ], }; const isRoleAuthorized = (role, resource, method) => { diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index c6f7dc339..4fde73e0d 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -1,4 +1,5 @@ const _ = require('lodash'); +const { v4: uuidv4 } = require('uuid'); const BasicModel = require('./BasicModel'); const sqlClient = require('../../sql/sqlClient'); @@ -105,6 +106,26 @@ class Experiment extends BasicModel { return result; } + async copyFrom(fromExperimentId) { + const toExperimentId = uuidv4().replace(/-/g, ''); + + const { sql } = this; + + await sql + .insert( + sql(tableNames.EXPERIMENT) + .select( + sql.raw('? as id', [toExperimentId]), + 'name', + 'description', + ) + .where({ id: fromExperimentId }), + ) + .into(sql.raw(`${tableNames.EXPERIMENT} (id, name, description)`)); + + return toExperimentId; + } + // Sets samples_order as an array that has the sample in oldPosition moved to newPosition async updateSamplePosition( experimentId, oldPosition, newPosition, diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 47e1c8105..dcda7e684 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1186,7 +1186,7 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPError' - '/experiments/{experimentId}/clone/{toExperimentId}': + '/experiments/{experimentId}/clone': post: summary: Clone an experiment description: Clone an experiment, returns the id of the new experiment @@ -1211,7 +1211,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/HTTPSuccess' + type: string '400': description: Bad Request content: From 5d6c8419d35872bd1e14b11253c8fb8686ef4129 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 23 Jun 2022 11:22:07 -0300 Subject: [PATCH 23/29] Update tests --- .../controllers/experimentController.js | 2 +- src/api.v2/model/Experiment.js | 2 +- src/api.v2/model/__mocks__/Experiment.js | 1 + .../controllers/experimentController.test.js | 34 +++++++++++++------ tests/api.v2/routes/experiment.test.js | 9 ++--- 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 30a771db9..653b98c99 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -160,7 +160,7 @@ const cloneExperiment = async (req, res) => { let toExperimentId; await sqlClient.get().transaction(async (trx) => { - toExperimentId = await new Experiment(trx).copyFrom(fromExperimentId); + toExperimentId = await new Experiment(trx).createCopy(fromExperimentId); await new UserAccess(trx).createNewExperimentPermissions(userId, toExperimentId); }); diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 4fde73e0d..9ad08172e 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -106,7 +106,7 @@ class Experiment extends BasicModel { return result; } - async copyFrom(fromExperimentId) { + async createCopy(fromExperimentId) { const toExperimentId = uuidv4().replace(/-/g, ''); const { sql } = this; diff --git a/src/api.v2/model/__mocks__/Experiment.js b/src/api.v2/model/__mocks__/Experiment.js index 2df9fe5aa..38245efc9 100644 --- a/src/api.v2/model/__mocks__/Experiment.js +++ b/src/api.v2/model/__mocks__/Experiment.js @@ -6,6 +6,7 @@ const stub = { updateSamplePosition: jest.fn(), updateProcessingConfig: jest.fn(), getProcessingConfig: jest.fn(), + createCopy: jest.fn(), addSample: jest.fn(), deleteSample: jest.fn(), getDownloadLink: jest.fn(), diff --git a/tests/api.v2/controllers/experimentController.test.js b/tests/api.v2/controllers/experimentController.test.js index 674abcef6..f7e432da3 100644 --- a/tests/api.v2/controllers/experimentController.test.js +++ b/tests/api.v2/controllers/experimentController.test.js @@ -288,16 +288,18 @@ describe('experimentController', () => { }); it('cloneExperiment works correctly when samplesSubsetIds is provided', async () => { - const toExperimentId = 'toExperimentIdMock'; const samplesSubsetIds = ['mockSample2', 'mockSample3']; const clonedSamplesSubsetIds = ['mockClonedSample2', 'mockClonedSample3']; + const userId = 'mockUserId'; + const toExperimentId = 'mockToExperimentId'; const mockReq = { - params: { experimentId: mockExperiment.id, toExperimentId }, + params: { experimentId: mockExperiment.id }, body: { samplesSubsetIds }, + user: { sub: userId }, }; - metadataTrackInstance.delete.mockImplementationOnce(() => Promise.resolve()); + experimentInstance.createCopy.mockImplementationOnce(() => Promise.resolve(toExperimentId)); sampleTrackInstance.copyTo.mockImplementationOnce( () => Promise.resolve(clonedSamplesSubsetIds), ); @@ -305,33 +307,40 @@ describe('experimentController', () => { await experimentController.cloneExperiment(mockReq, mockRes); - expect(metadataTrackInstance.delete).toHaveBeenCalledWith({ experiment_id: toExperimentId }); + // Creates new experiment + expect(experimentInstance.createCopy).toHaveBeenCalledWith(mockExperiment.id); + expect(userAccessInstance.createNewExperimentPermissions) + .toHaveBeenCalledWith(userId, toExperimentId); + // Creates copy samples for new experiment expect(sampleTrackInstance.copyTo) .toHaveBeenCalledWith(mockExperiment.id, toExperimentId, samplesSubsetIds); + // Sets created sample in experiment expect(experimentInstance.updateById).toHaveBeenCalledWith( toExperimentId, { samples_order: JSON.stringify(clonedSamplesSubsetIds) }, ); - expect(mockRes.json).toHaveBeenCalledWith(OK()); + expect(mockRes.json).toHaveBeenCalledWith(toExperimentId); }); it('cloneExperiment works correctly when samplesSubsetIds is NOT provided', async () => { - const toExperimentId = 'toExperimentIdMock'; const allSampleIds = ['mockSample1', 'mockSample2', 'mockSample3', 'mockSample4']; const clonedSamplesIds = ['mockClonedSample1', 'mockClonedSample2', 'mockClonedSample3', 'mockClonedSample4']; + const userId = 'mockUserId'; + const toExperimentId = 'mockToExperimentId'; const mockReq = { - params: { experimentId: mockExperiment.id, toExperimentId }, + params: { experimentId: mockExperiment.id }, body: {}, + user: { sub: userId }, }; + experimentInstance.createCopy.mockImplementationOnce(() => Promise.resolve(toExperimentId)); experimentInstance.findById.mockReturnValueOnce( { first: () => Promise.resolve({ samplesOrder: allSampleIds }) }, ); - metadataTrackInstance.delete.mockImplementationOnce(() => Promise.resolve()); sampleTrackInstance.copyTo.mockImplementationOnce( () => Promise.resolve(clonedSamplesIds), ); @@ -341,16 +350,21 @@ describe('experimentController', () => { expect(experimentInstance.findById).toHaveBeenCalledWith(mockExperiment.id); - expect(metadataTrackInstance.delete).toHaveBeenCalledWith({ experiment_id: toExperimentId }); + // Creates new experiment + expect(experimentInstance.createCopy).toHaveBeenCalledWith(mockExperiment.id); + expect(userAccessInstance.createNewExperimentPermissions) + .toHaveBeenCalledWith(userId, toExperimentId); + // Creates copy samples for new experiment expect(sampleTrackInstance.copyTo) .toHaveBeenCalledWith(mockExperiment.id, toExperimentId, allSampleIds); + // Sets created sample in experiment expect(experimentInstance.updateById).toHaveBeenCalledWith( toExperimentId, { samples_order: JSON.stringify(clonedSamplesIds) }, ); - expect(mockRes.json).toHaveBeenCalledWith(OK()); + expect(mockRes.json).toHaveBeenCalledWith(toExperimentId); }); }); diff --git a/tests/api.v2/routes/experiment.test.js b/tests/api.v2/routes/experiment.test.js index 0f7cb4629..90d0db22a 100644 --- a/tests/api.v2/routes/experiment.test.js +++ b/tests/api.v2/routes/experiment.test.js @@ -337,7 +337,6 @@ describe('tests for experiment route', () => { it('cloneExperiment results in a successful response without a body', async (done) => { const experimentId = 'fromExperimentId'; - const toExperimentId = 'toExperimentId'; experimentController.cloneExperiment.mockImplementationOnce((req, res) => { res.json(OK()); @@ -345,7 +344,7 @@ describe('tests for experiment route', () => { }); request(app) - .post(`/v2/experiments/${experimentId}/clone/${toExperimentId}`) + .post(`/v2/experiments/${experimentId}/clone`) .set({ 'Content-Type': 'application/json' }) .expect(200) .end((err) => { @@ -360,7 +359,6 @@ describe('tests for experiment route', () => { it('cloneExperiment results in a successful response with a valid body', async (done) => { const experimentId = 'fromExperimentId'; - const toExperimentId = 'toExperimentId'; const body = { samplesSubsetIds: ['sampleId1', 'sampleId2'] }; @@ -370,7 +368,7 @@ describe('tests for experiment route', () => { }); request(app) - .post(`/v2/experiments/${experimentId}/clone/${toExperimentId}`) + .post(`/v2/experiments/${experimentId}/clone`) .send(body) .expect(200) .end((err) => { @@ -385,7 +383,6 @@ describe('tests for experiment route', () => { it('cloneExperiment results in a 400 with an invalid body', async (done) => { const experimentId = 'fromExperimentId'; - const toExperimentId = 'toExperimentId'; const body = { samplesSubsetIdsInvalid: ['sampleId1', 'sampleId2'] }; @@ -395,7 +392,7 @@ describe('tests for experiment route', () => { }); request(app) - .post(`/v2/experiments/${experimentId}/clone/${toExperimentId}`) + .post(`/v2/experiments/${experimentId}/clone`) .send(body) .expect(400) .end((err) => { From 61f91ea74b5173ef83d714c6b374642b698a05e7 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 23 Jun 2022 13:01:55 -0300 Subject: [PATCH 24/29] Rename to fromSamples --- src/api.v2/model/Sample.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/api.v2/model/Sample.js b/src/api.v2/model/Sample.js index 65cb1dd54..58f5c9dfc 100644 --- a/src/api.v2/model/Sample.js +++ b/src/api.v2/model/Sample.js @@ -98,11 +98,11 @@ class Sample extends BasicModel { * @param {*} toExperimentId */ async copyTo(fromExperimentId, toExperimentId, samplesOrder) { - const result = await this.getSamples(fromExperimentId); + const fromSamples = await this.getSamples(fromExperimentId); const newSampleIds = []; - const metadataTrackKeys = Object.keys(result[0].metadata); + const metadataTrackKeys = Object.keys(fromSamples[0].metadata); const sampleRows = []; const sampleFileMapRows = []; @@ -119,7 +119,7 @@ class Sample extends BasicModel { // Copy each sample in order so // the new samples we create follow the same order samplesOrder.forEach((fromSampleId) => { - const sample = result.find(({ id }) => id === fromSampleId); + const sample = fromSamples.find(({ id }) => id === fromSampleId); const toSampleId = uuidv4(); From 63b282dba65c5f2ecb16e53a4cb691094f8a956b Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 23 Jun 2022 13:05:04 -0300 Subject: [PATCH 25/29] Rename --- src/api.v2/controllers/experimentController.js | 6 +++--- src/api.v2/model/Experiment.js | 2 +- src/api.v2/model/__mocks__/Experiment.js | 2 +- src/api.v2/routes/experiment.js | 6 +++--- src/specs/api.v2.yaml | 4 ++-- tests/api.v2/controllers/experimentController.test.js | 8 ++++---- tests/api.v2/model/Experiment.test.js | 4 ++-- tests/api.v2/routes/experiment.test.js | 6 +++--- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 653b98c99..8c78822fe 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -22,10 +22,10 @@ const getAllExperiments = async (req, res) => { res.json(data); }; -const getAllExampleExperiments = async (req, res) => { +const getExampleExperiments = async (req, res) => { logger.log('Getting example experiments'); - const data = await new Experiment().getAllExampleExperiments(); + const data = await new Experiment().getExampleExperiments(); logger.log(`Finished getting example experiments, length: ${data.length}`); res.json(data); @@ -181,7 +181,7 @@ const cloneExperiment = async (req, res) => { module.exports = { getAllExperiments, - getAllExampleExperiments, + getExampleExperiments, getExperiment, createExperiment, updateProcessingConfig, diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 9ad08172e..16b9d9bc9 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -61,7 +61,7 @@ class Experiment extends BasicModel { return result; } - async getAllExampleExperiments() { + async getExampleExperiments() { return this.getAllExperiments(constants.PUBLIC_ACCESS_ID); } diff --git a/src/api.v2/model/__mocks__/Experiment.js b/src/api.v2/model/__mocks__/Experiment.js index 38245efc9..50707c8d3 100644 --- a/src/api.v2/model/__mocks__/Experiment.js +++ b/src/api.v2/model/__mocks__/Experiment.js @@ -10,7 +10,7 @@ const stub = { addSample: jest.fn(), deleteSample: jest.fn(), getDownloadLink: jest.fn(), - getAllExampleExperiments: jest.fn(), + getExampleExperiments: jest.fn(), ...BasicModel, }; diff --git a/src/api.v2/routes/experiment.js b/src/api.v2/routes/experiment.js index c2a99d07c..eb484f280 100644 --- a/src/api.v2/routes/experiment.js +++ b/src/api.v2/routes/experiment.js @@ -1,5 +1,5 @@ const { - getAllExperiments, getAllExampleExperiments, + getAllExperiments, getExampleExperiments, createExperiment, getExperiment, patchExperiment, deleteExperiment, cloneExperiment, getProcessingConfig, updateProcessingConfig, updateSamplePosition, @@ -13,9 +13,9 @@ module.exports = { expressAuthenticationOnlyMiddleware, (req, res, next) => getAllExperiments(req, res).catch(next), ], - 'experiment#getAllExampleExperiments': [ + 'experiment#getExampleExperiments': [ expressAuthenticationOnlyMiddleware, - (req, res, next) => getAllExampleExperiments(req, res).catch(next), + (req, res, next) => getExampleExperiments(req, res).catch(next), ], 'experiment#getExperiment': [ expressAuthorizationMiddleware, diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index dcda7e684..b3f657812 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -266,8 +266,8 @@ paths: get: summary: Get all example experiments description: Get all experiments that are for demonstration - operationId: getAllExampleExperiments - x-eov-operation-id: experiment#getAllExampleExperiments + operationId: getExampleExperiments + x-eov-operation-id: experiment#getExampleExperiments x-eov-operation-handler: routes/experiment responses: '200': diff --git a/tests/api.v2/controllers/experimentController.test.js b/tests/api.v2/controllers/experimentController.test.js index f7e432da3..df442630e 100644 --- a/tests/api.v2/controllers/experimentController.test.js +++ b/tests/api.v2/controllers/experimentController.test.js @@ -77,16 +77,16 @@ describe('experimentController', () => { expect(mockRes.json).toHaveBeenCalledWith(getAllExperimentsResponse); }); - it('getAllExampleExperiments works correctly', async () => { + it('getExampleExperiments works correctly', async () => { const mockReq = { user: { sub: 'mockUserId' } }; - experimentInstance.getAllExampleExperiments.mockImplementationOnce( + experimentInstance.getExampleExperiments.mockImplementationOnce( () => Promise.resolve(getAllExperimentsResponse), ); - await experimentController.getAllExampleExperiments(mockReq, mockRes); + await experimentController.getExampleExperiments(mockReq, mockRes); - expect(experimentInstance.getAllExampleExperiments).toHaveBeenCalled(); + expect(experimentInstance.getExampleExperiments).toHaveBeenCalled(); expect(mockRes.json).toHaveBeenCalledWith(getAllExperimentsResponse); }); diff --git a/tests/api.v2/model/Experiment.test.js b/tests/api.v2/model/Experiment.test.js index 1a0722b04..4831531ee 100644 --- a/tests/api.v2/model/Experiment.test.js +++ b/tests/api.v2/model/Experiment.test.js @@ -60,13 +60,13 @@ describe('model/Experiment', () => { expect(mockSqlClient.as).toHaveBeenCalledWith('mainQuery'); }); - it('getAllExampleExperiments works correctly', async () => { + it('getExampleExperiments works correctly', async () => { const expectedResult = { isMockResult: true }; const getAllExperimentsSpy = jest.spyOn(Experiment.prototype, 'getAllExperiments') .mockImplementationOnce(() => Promise.resolve(expectedResult)); - const result = await new Experiment().getAllExampleExperiments('mockUserId'); + const result = await new Experiment().getExampleExperiments('mockUserId'); expect(result).toBe(expectedResult); expect(getAllExperimentsSpy).toHaveBeenCalledWith(constants.PUBLIC_ACCESS_ID); diff --git a/tests/api.v2/routes/experiment.test.js b/tests/api.v2/routes/experiment.test.js index 90d0db22a..a2ef84ae7 100644 --- a/tests/api.v2/routes/experiment.test.js +++ b/tests/api.v2/routes/experiment.test.js @@ -22,7 +22,7 @@ jest.mock('../../../src/api.v2/controllers/experimentController', () => ({ getProcessingConfig: jest.fn(), updateProcessingConfig: jest.fn(), downloadData: jest.fn(), - getAllExampleExperiments: jest.fn(), + getExampleExperiments: jest.fn(), cloneExperiment: jest.fn(), })); @@ -316,8 +316,8 @@ describe('tests for experiment route', () => { }); }); - it('getAllExampleExperiments results in a successful response', async (done) => { - experimentController.getAllExampleExperiments.mockImplementationOnce((req, res) => { + it('getExampleExperiments results in a successful response', async (done) => { + experimentController.getExampleExperiments.mockImplementationOnce((req, res) => { res.json(getExperimentResponse); return Promise.resolve(); }); From efd9f6cee9b42f90f24cca8a16361a090735cb08 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 23 Jun 2022 13:05:26 -0300 Subject: [PATCH 26/29] Fix comment --- src/api.v2/model/Sample.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api.v2/model/Sample.js b/src/api.v2/model/Sample.js index 58f5c9dfc..986ba65be 100644 --- a/src/api.v2/model/Sample.js +++ b/src/api.v2/model/Sample.js @@ -92,7 +92,7 @@ class Sample extends BasicModel { } /** - * Creates copy samples from one experiment to another one + * Copies samples from one experiment to another one * * @param {*} fromExperimentId * @param {*} toExperimentId From c9426d4b9bd38120343dfe0d66c09dddbe89ff41 Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 23 Jun 2022 13:06:18 -0300 Subject: [PATCH 27/29] Refactor --- src/api.v2/model/Experiment.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 16b9d9bc9..2fef75145 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -46,13 +46,17 @@ class Experiment extends BasicModel { const aliasedExperimentFields = fields.map((field) => `e.${field}`); + const mainQuery = this.sql + .select([...aliasedExperimentFields, 'm.key']) + .from(tableNames.USER_ACCESS) + .where('user_id', userId) + .join(`${tableNames.EXPERIMENT} as e`, 'e.id', `${tableNames.USER_ACCESS}.experiment_id`) + .leftJoin(`${tableNames.METADATA_TRACK} as m`, 'e.id', 'm.experiment_id') + .as('mainQuery'); + const result = await collapseKeyIntoArray( - this.sql.select([...aliasedExperimentFields, 'm.key']) - .from(tableNames.USER_ACCESS) - .where('user_id', userId) - .join(`${tableNames.EXPERIMENT} as e`, 'e.id', `${tableNames.USER_ACCESS}.experiment_id`) - .leftJoin(`${tableNames.METADATA_TRACK} as m`, 'e.id', 'm.experiment_id') - .as('mainQuery'), [...fields], + mainQuery, + [...fields], 'key', 'metadataKeys', this.sql, From c2b71d2dfb62f9a27fea977e05a99bee66afeabf Mon Sep 17 00:00:00 2001 From: cosa65 Date: Thu, 23 Jun 2022 13:07:29 -0300 Subject: [PATCH 28/29] Rename --- src/api.v2/controllers/experimentController.js | 4 ++-- src/specs/api.v2.yaml | 4 ++-- tests/api.v2/controllers/experimentController.test.js | 10 +++++----- tests/api.v2/routes/experiment.test.js | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 8c78822fe..5fab7c08e 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -151,7 +151,7 @@ const cloneExperiment = async (req, res) => { const { params: { experimentId: fromExperimentId }, - body: { samplesSubsetIds = await getAllSampleIds(fromExperimentId) }, + body: { samplesToCloneIds = await getAllSampleIds(fromExperimentId) }, user: { sub: userId }, } = req; @@ -167,7 +167,7 @@ const cloneExperiment = async (req, res) => { logger.log(`Cloning experiment ${fromExperimentId} into ${toExperimentId}`); const clonedSamplesOrder = await new Sample() - .copyTo(fromExperimentId, toExperimentId, samplesSubsetIds); + .copyTo(fromExperimentId, toExperimentId, samplesToCloneIds); await new Experiment().updateById( toExperimentId, diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index b3f657812..1f39116eb 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1197,10 +1197,10 @@ paths: content: application/json: schema: - description: If samplesSubsetIds is sent then the cloned experiment only contains the subset samples instead of all of them + description: If samplesToCloneIds is sent then the cloned experiment only contains the subset samples instead of all of them type: object properties: - samplesSubsetIds: + samplesToCloneIds: type: array items: type: string diff --git a/tests/api.v2/controllers/experimentController.test.js b/tests/api.v2/controllers/experimentController.test.js index df442630e..7e696a138 100644 --- a/tests/api.v2/controllers/experimentController.test.js +++ b/tests/api.v2/controllers/experimentController.test.js @@ -287,15 +287,15 @@ describe('experimentController', () => { .toHaveBeenCalledWith(mockExperiment.id, bucketNames.PROCESSED_MATRIX); }); - it('cloneExperiment works correctly when samplesSubsetIds is provided', async () => { - const samplesSubsetIds = ['mockSample2', 'mockSample3']; + it('cloneExperiment works correctly when samplesToCloneIds is provided', async () => { + const samplesToCloneIds = ['mockSample2', 'mockSample3']; const clonedSamplesSubsetIds = ['mockClonedSample2', 'mockClonedSample3']; const userId = 'mockUserId'; const toExperimentId = 'mockToExperimentId'; const mockReq = { params: { experimentId: mockExperiment.id }, - body: { samplesSubsetIds }, + body: { samplesToCloneIds }, user: { sub: userId }, }; @@ -314,7 +314,7 @@ describe('experimentController', () => { // Creates copy samples for new experiment expect(sampleTrackInstance.copyTo) - .toHaveBeenCalledWith(mockExperiment.id, toExperimentId, samplesSubsetIds); + .toHaveBeenCalledWith(mockExperiment.id, toExperimentId, samplesToCloneIds); // Sets created sample in experiment expect(experimentInstance.updateById).toHaveBeenCalledWith( @@ -325,7 +325,7 @@ describe('experimentController', () => { expect(mockRes.json).toHaveBeenCalledWith(toExperimentId); }); - it('cloneExperiment works correctly when samplesSubsetIds is NOT provided', async () => { + it('cloneExperiment works correctly when samplesToCloneIds is NOT provided', async () => { const allSampleIds = ['mockSample1', 'mockSample2', 'mockSample3', 'mockSample4']; const clonedSamplesIds = ['mockClonedSample1', 'mockClonedSample2', 'mockClonedSample3', 'mockClonedSample4']; const userId = 'mockUserId'; diff --git a/tests/api.v2/routes/experiment.test.js b/tests/api.v2/routes/experiment.test.js index a2ef84ae7..6551d62c2 100644 --- a/tests/api.v2/routes/experiment.test.js +++ b/tests/api.v2/routes/experiment.test.js @@ -360,7 +360,7 @@ describe('tests for experiment route', () => { it('cloneExperiment results in a successful response with a valid body', async (done) => { const experimentId = 'fromExperimentId'; - const body = { samplesSubsetIds: ['sampleId1', 'sampleId2'] }; + const body = { samplesToCloneIds: ['sampleId1', 'sampleId2'] }; experimentController.cloneExperiment.mockImplementationOnce((req, res) => { res.json(OK()); From 9040c9a34d843ab3649bbfd1a29aa3f265dae2ea Mon Sep 17 00:00:00 2001 From: Martin Fosco Date: Thu, 23 Jun 2022 14:10:47 -0300 Subject: [PATCH 29/29] Update experimentController.js --- src/api.v2/controllers/experimentController.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 5fab7c08e..62882a7e0 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -174,7 +174,7 @@ const cloneExperiment = async (req, res) => { { samples_order: JSON.stringify(clonedSamplesOrder) }, ); - logger.log(`Finished cloning experiment ${fromExperimentId}, new expeirment's id is ${toExperimentId}`); + logger.log(`Finished cloning experiment ${fromExperimentId}, new experiment's id is ${toExperimentId}`); res.json(toExperimentId); };