New validation logging#136
Merged
Merged
Conversation
Contributor
|
How do you feel about updating the README section that talks about date filtering here? I know it doesn't related to this PR specifically, but I want to reflect the conversation we had at stand up earlier this week. In the "Extraction Date Range" section, I updated the second line and added a third to result in the section reading like this: Let me know if you think this is overstepping this PR or if you don't like how this reads. |
jafeltra
reviewed
Jun 28, 2021
jafeltra
left a comment
Contributor
There was a problem hiding this comment.
This looks good to me! I think the warning/debug messages make sense. I had two really small comments inline. Let me know what you think.
dmendelowitz
left a comment
Contributor
There was a problem hiding this comment.
I agree with Julia's comments. but this looks good to me otherwise!
25b507d to
c26a86c
Compare
Co-authored-by: Julia Afeltra <30803904+jafeltra@users.noreply.github.com>
Contributor
Author
|
Added some test cases! |
dmendelowitz
approved these changes
Jun 28, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This pull requests adds more detailed log information when a resource within the extracted bundle fails validation.
It introduces use of the
compilefunction in AJV to generate a validator object that comes witherrorswe can filter and display in a more detailed manner.We also now use the
subSchemamotif to pul off refs to specific resources in the schema that we wish to validate from within the loop, allowing us to collect all the error messages present for that resource rather than short circuit after one.New behavior
New log messages to the console when a resource fails validation. E.g.:
The default behavior now logs the resource ID and any attributes on that resource that failed validation. When debug is enabled, the specific error message will be displayed too for every error.
Code changes
fhirUtilsto useajv compileand the sub schemaBaseClientto join all errors and resources failing during log warn and debug statementsTesting guidance
To cause some validation errors, I would recommend making some changes in our templates. Some test cases that I tried included changing properties that were supposed to be dates and making them just strings or booleans or something, and also changing attributes that are enums (e.g. gender) to have a value that doesn't exist in that enum.
Feel free to experiment with other cases that you might expect to fail validation smartly.
Run without debug to see just the basic warning message that lists the resources that are failing and where they are failing. Run with debug to see the detailed error messages.