MIG-109: split getStudyManifest function as per requirement #372
Conversation
| @@ -136,8 +142,18 @@ class StudyManifestTools { | |||
| allEntities = [study, ...await this.getStudyManifestEntities(org, study, orgReferenceProps)], | |||
| { outputEntities, removedEntities } = this.validateReferences(allEntities, orgReferenceProps), | |||
| manifest = this.createManifest(outputEntities), | |||
| mappingScript = await getMappingScript(org) | |||
| mappingScript = await getMappingScript(org), | |||
| ingestTransform = fs.readFileSync('../packageScripts/ingestTransform.js') | |||
There was a problem hiding this comment.
This requires the __dirname prefix to make sure it is reading relative to this directory otherwise it will fail when run from the mdctl
Getting orac__forms
Getting orac__form_questions
Getting orac__events
Validating Internal References
{
object: 'fault',
name: 'error',
errCode: 'mdctl.error.unspecified',
code: 'kError',
message: "ENOENT: no such file or directory, open '../packageScripts/ingestTransform.js'",
status: 500,
trace: null,
path: undefined,
resource: undefined,
reason: '../packageScripts/ingestTransform.js'
}
There was a problem hiding this comment.
can we add some tests for this change please? there are some examples in the __tests__ folder
There was a problem hiding this comment.
Hi @nahueld, apologies I think I've accidentally deleted the __dirname at some stage, but it has been fixed now. I've also created a unit test for this ticket.
Thank you
| @@ -125,6 +125,12 @@ class StudyManifestTools { | |||
|
|
|||
| async getStudyManifest() { | |||
There was a problem hiding this comment.
Also noticed that we have some code duplication for the other mdctl tasks, check
- getTasksManifest
- getConsentsManifest
For both cases we could split the logic too (that is have writing logic and processing logic decoupled), this will be really convenient for the next release that we are going to be using those other more granular exports
There was a problem hiding this comment.
Hi @nahueld is it ok for you if I create another Jira to keep track of this as I guess we probably don't want to overlap with the original requirement?
There was a problem hiding this comment.
ok let's update the ticket to reflect this, given that this is an internal task we are ok adjusting the requirements
davideaquaro
changed the title
davideaquaro
changed the title
Requirements
Notes