Adding implementation of extractor dependencies

[dmendelowitz]

Summary

Adds new sortExtractors() function called within MCODEClient which sorts extractors based on a predefined order (defined in MCODEClient.js) and will throw an error detailing any missing dependencies. Since there was some uncertainty over which method of implementing extractor dependencies was best, I would appreciate people's further thoughts after seeing this method in action and I wouldn't be against going back and trying another method if people think this isn't quite what we wanted.

Also, This new functionality makes a previous test fail (in mcodeExtraction.test.js). I didn't want to just remove it because I feel like we could modify the test to work a different way, I just wasn't sure how, so I would appreciate some help with that

New behavior

If extractors are put out of order in the config file (e.g. Patient is put after extractors that require Patient), they will be reordered automatically before running the client and if any extractor's dependencies are missing an error will be thrown.

Code changes

• dependencyUtils.js is a new file containing the sortExtractors() function that sorts extractors and checks for missing dependencies
• dependencyUtils.test.js contains tests for this new function
• MCODEClient.js now contains information about the order and dependencies of extractors and calls the sortExtractors function to sort based on that order and check those dependencies

Testing guidance

• Put the patient extractor somewhere other than the start of your config file and see that the client still runs properly
• Remove the patient extractor entirely and see that you receive an error telling you that it is missing and which extractors require it
• Ensure that all tests pass (see note in summary about the one that doesn't) and check that the new tests cover enough cases

[julianxcarter]

The actual config sorting is working well and the approach looks sound to me. Great work!

In regards to the failing test case, given that the purpose of that test is to test extraction on non-fatal errors my first thought is to actually add the PatientExtractor config object to the example config shown here, as non-fatal errors would still occur down the line in extraction since there's no clinicalSiteID on the CTI config. I also think we should probably add another test that doesn't run extraction, but just tests that an error is thrown when the MCODEClient is initialized with a config that's out of order. Happy to discuss this more if you have other ideas you'd like to kick around!

[Dtphelan1]

The actual config sorting is working well and the approach looks sound to me. Great work!

In regards to the failing test case, given that the purpose of that test is to test extraction on non-fatal errors my first thought is to actually add the PatientExtractor config object to the example config shown here, as non-fatal errors would still occur down the line in extraction since there's no clinicalSiteID on the CTI config. I also think we should probably add another test that doesn't run extraction, but just tests that an error is thrown when the MCODEClient is initialized with a config that's out of order. Happy to discuss this more if you have other ideas you'd like to kick around!

Re: An additional test, I think that's a good idea!

Re: Fixing the current test, I think Julian's suggestion sounds great. Another approach, if we wanted to make our test a little less fragile, could be explicitly Mocking a response from an extractor get function that just fails. We do something similar at the level of the client in our second test should fail to execute if a fatal error is produced in the extracting data. Up to you which way you want to go.

[dmendelowitz]

I think I addressed all comments. I went with @julianxcarter's suggestion for fixing the broken test. Give it another look and let me know if anything else needs changing!

[julianxcarter]

Looks good to me!

[Dtphelan1]

Looks good Dylan – I'll leave the privilege of a merge to you 😄