-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Json conditional check #10
Conversation
…ere is a standard dollar charge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a few things that might be useful to re-examine.
src/versions/2.0/json.ts
Outdated
if: { | ||
properties: { | ||
methodology: { | ||
const: "other", | ||
}, | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be useful to include required: [ "methodology" ]
as part of this subschema. That way, if methodology
is not in the document, the subschema will fail, and the then
subschema won't be applied. Adding in this requirement gets rid of the two extra errors that appear in the test for sample-errors.json
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Updated.
const result = await validateJson( | ||
loadFixtureStream("/2.0/sample-conditional-errors.json"), | ||
"v2.0" | ||
) | ||
t.is(result.valid, false) | ||
t.deepEqual(result.errors.length, 2) | ||
t.deepEqual(result.errors, [ | ||
{ | ||
path: "/standard_charges/3/payers_information/2", | ||
field: "2", | ||
message: "must have required property 'additional_payer_notes'", | ||
}, | ||
{ | ||
path: "/standard_charges/3/payers_information/2", | ||
field: "2", | ||
message: 'must match "then" schema', | ||
}, | ||
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it is identical to the tests on lines 71-90.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated and removed lines 71-90.
path: "/standard_charges/0", | ||
field: "0", | ||
message: "must have required property 'discounted_cash'", | ||
}, | ||
{ | ||
path: "/standard_charges/1/payers_information/0", | ||
field: "0", | ||
message: "must have required property 'standard_charge_dollar'", | ||
}, | ||
{ | ||
path: "/standard_charges/1/payers_information/0", | ||
field: "0", | ||
message: "must have required property 'standard_charge_algorithm'", | ||
}, | ||
{ | ||
path: "/standard_charges/1/payers_information/0", | ||
field: "0", | ||
message: "must have required property 'standard_charge_percentage'", | ||
}, | ||
{ | ||
path: "/standard_charges/1/payers_information/0", | ||
field: "0", | ||
message: "must match a schema in anyOf", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The numeric indices in the error paths look a little off. These all apply to the same object, so shouldn't the index for standard_charges
be the same for all of them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Captured this in Issue #12
Adding conditional checks for JSON files.
The first four have been added to the schema and need to incorporated into the data dictionary.