Skip to content

Conversation

@dmendelowitz
Copy link
Contributor

Summary

The config file is now checked for errors by validating it against the new config schema

New behavior

When there are one or more errors in the config file, the client will fail and tell the user what is wrong with their config file

Code changes

  • configValidator.js contains the new validateConfig() function that performs validation
  • checkInputAndConfig() in app.js now calls the new validateConfig() function instead of manually checking for patientIdsCsvPath
  • Adjusted tests in app.test.js to work with the new validator
  • Added new tests for the config validator in configValidator.test.js

Testing guidance

  • Ensure that when the config has no errors, the client still runs as expected
  • Mess up the config file in more than one way (remove a required field, use a wrong type, use a wrong format) and make sure the error message alerts you of all errors in your config
  • Make sure all tests still pass and new tests make sense


function validateConfig(config) {
const valid = validator.validate('config', config);
const errors = ajv.errorsText(validator.errors, { dataVar: 'config' });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not realize there was a way to access errors from the base Ajv object, good find!

Copy link
Contributor

@julianxcarter julianxcarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works as intended and looks great to me

@mgramigna mgramigna self-assigned this Jun 28, 2021
function checkInputAndConfig(config, fromDate, toDate) {
// Check input args and needed config variables based on client being used
const { patientIdCsvPath } = config;
validateConfig(config);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be useful to have some (likely debug level?) log statements here to indicate that this is happening/has succeeded after the function call

@mgramigna
Copy link
Contributor

Made a tiny comment but everything about the functionality/tests looks good to me

@mgramigna mgramigna merged commit c9ec376 into develop Jun 28, 2021
@mgramigna mgramigna deleted the config-validator branch June 28, 2021 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants