From 35ec5a3313537cd5c8868634f8f3770acdf2e3f9 Mon Sep 17 00:00:00 2001 From: Chandima H Date: Fri, 13 Dec 2024 19:25:24 -0500 Subject: [PATCH] Fix compare_age crash (#31) * Fix compare_age crash * Update compare_age tests * Update changelog --- CHANGELOG.md | 1 + nacc_form_validator/nacc_validator.py | 12 ++++- tests/test_rules_compare_age.py | 72 +++++++++++++++++++++------ 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1a7aa..f104c79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Documentation of release versions of `nacc-form-validator` * Refactors the tests to be more modularized so that they're more manageable * Refactors logic for `compare_values` by moving it to its own utility method and allows comparing to null values * Fixes issue where `datastore` was not being set for the temp validator in `_check_subschema_valid`, causing nested conditions with previous records to not evaluate correctly +* Overrides `_validate_nullable` method to drop custom rule definitions that cannot be evaluated for null values ## 0.3.0 diff --git a/nacc_form_validator/nacc_validator.py b/nacc_form_validator/nacc_validator.py index 6952a34..34e6d94 100755 --- a/nacc_form_validator/nacc_validator.py +++ b/nacc_form_validator/nacc_validator.py @@ -320,6 +320,16 @@ def _validate_formatting(self, formatting: str, field: str, value: object): self.__add_system_error(field, err_msg) raise ValidationException(err_msg) + def _validate_nullable(self, nullable, field, value): + """Override nullable rule to drop custom defined rules. + + The rule's arguments are validated against this schema: + {'type': 'boolean'} + """ + super()._validate_nullable(nullable, field, value) + if value is None: + super()._drop_remaining_rules('compare_age') + def _validate_max(self, max_value: object, field: str, value: object): """Override max rule to support validations wrt current date/year. @@ -1088,7 +1098,7 @@ def _validate_compare_age(self, comparison: Dict[str, Any], field: str, try: value = utils.convert_to_date(value) - except parser.ParserError as error: + except (ValueError, TypeError, parser.ParserError) as error: self._error(field, ErrorDefs.DATE_CONVERSION, value, error) return diff --git a/tests/test_rules_compare_age.py b/tests/test_rules_compare_age.py index b09ef6a..6df91fd 100644 --- a/tests/test_rules_compare_age.py +++ b/tests/test_rules_compare_age.py @@ -1,6 +1,8 @@ """ Tests the custom compare_with_date rule (_validate_compare_with_date). """ + + def test_compare_age(date_constraint, create_nacc_validator): """ Tests compare_age, case where compare_to is another field""" schema = { @@ -32,14 +34,18 @@ def test_compare_age(date_constraint, create_nacc_validator): nv = create_nacc_validator(schema) # valid cases - assert nv.validate({'frmdate': '2024/02/02', 'birthmo': 6, 'birthyr': 1950, 'behage': 50}) - assert nv.validate({'frmdate': '2024/02/02', 'birthmo': 2, 'birthyr': 2024, 'behage': 0}) + assert nv.validate( + {'frmdate': '2024/02/02', 'birthmo': 6, 'birthyr': 1950, 'behage': 50}) + assert nv.validate( + {'frmdate': '2024/02/02', 'birthmo': 2, 'birthyr': 2024, 'behage': 0}) # invalid cases - assert not nv.validate({'frmdate': '2024/02/02', 'birthmo': 1, 'birthyr': 2024, 'behage': 50}) + assert not nv.validate( + {'frmdate': '2024/02/02', 'birthmo': 1, 'birthyr': 2024, 'behage': 50}) assert nv.errors == {'frmdate': [ - "input value behage doesn't satisfy the condition: age at frmdate >= behage" - ]} + "input value behage doesn't satisfy the condition: age at frmdate >= behage" + ]} + def test_compare_age_list(date_constraint, create_nacc_validator): """ Tests compare_age, case where compare_to is a list """ @@ -85,15 +91,19 @@ def test_compare_age_list(date_constraint, create_nacc_validator): nv = create_nacc_validator(schema) # valid cases - assert nv.validate({'frmdate': '2024/02/02', 'birthmo': 6, 'birthyr': 1950, 'behage': 50, 'cogage': 40, 'perchage': 70}) - assert nv.validate({'frmdate': '2024/02/02', 'birthmo': 2, 'birthyr': 2024, 'behage': 0, 'cogage': 0, 'perchage': -2}) + assert nv.validate({'frmdate': '2024/02/02', 'birthmo': 6, + 'birthyr': 1950, 'behage': 50, 'cogage': 40, 'perchage': 70}) + assert nv.validate({'frmdate': '2024/02/02', 'birthmo': 2, + 'birthyr': 2024, 'behage': 0, 'cogage': 0, 'perchage': -2}) # invalid cases - assert not nv.validate({'frmdate': '2024/02/02', 'birthmo': 1, 'birthyr': 2024, 'behage': 50, 'cogage': 0, 'perchage': 60}) + assert not nv.validate({'frmdate': '2024/02/02', 'birthmo': 1, + 'birthyr': 2024, 'behage': 50, 'cogage': 0, 'perchage': 60}) assert nv.errors == {'frmdate': [ - "input value perchage doesn't satisfy the condition: age at frmdate >= behage, cogage, perchage, 0", - "input value behage doesn't satisfy the condition: age at frmdate >= behage, cogage, perchage, 0" - ]} + "input value perchage doesn't satisfy the condition: age at frmdate >= behage, cogage, perchage, 0", + "input value behage doesn't satisfy the condition: age at frmdate >= behage, cogage, perchage, 0" + ]} + def test_compare_age_invalid_field(date_constraint, create_nacc_validator): """ Test case where invalid age to compare is provided """ @@ -118,8 +128,11 @@ def test_compare_age_invalid_field(date_constraint, create_nacc_validator): } nv = create_nacc_validator(schema) - assert not nv.validate({'frmdate': '2024/02/02', 'birthyr': 2024, 'behage': "dummy_str"}) - assert nv.errors == {'frmdate': ["Error in comparing behage to age at frmdate (0.08761122518822724): '<=' not supported between instances of 'float' and 'str'"]} + assert not nv.validate( + {'frmdate': '2024/02/02', 'birthyr': 2024, 'behage': "dummy_str"}) + assert nv.errors == {'frmdate': [ + "Error in comparing behage to age at frmdate (0.08761122518822724): '<=' not supported between instances of 'float' and 'str'"]} + def test_compare_age_invalid_base(create_nacc_validator): """ Test case where base_date is invalid """ @@ -142,5 +155,34 @@ def test_compare_age_invalid_base(create_nacc_validator): } nv = create_nacc_validator(schema) - assert not nv.validate({'frmdate': 'hello world', 'birthyr': 2024, 'behage': 50}) - assert nv.errors == {'frmdate': ['failed to convert value hello world to a date: Unknown string format: hello world']} + assert not nv.validate( + {'frmdate': 'hello world', 'birthyr': 2024, 'behage': 50}) + assert nv.errors == {'frmdate': [ + 'failed to convert value hello world to a date: Unknown string format: hello world']} + + +def test_compare_age_null_base(create_nacc_validator): + """ Test case where base_date is null """ + + schema = { + "frmdate": { + "type": "string", + "required": True, + "compare_age": { + "comparator": "<=", + "birth_year": "birthyr", + "compare_to": "behage", + } + }, + "birthyr": { + "type": "integer" + }, + "behage": { + "type": "integer" + } + } + + nv = create_nacc_validator(schema) + assert not nv.validate( + nv.cast_record({'frmdate': '', 'birthyr': 2024, 'behage': 50})) + assert nv.errors == {'frmdate': ['null value not allowed']}