Skip to content
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

Update validator to support floats in min/max and disallow string types #230

Merged
merged 27 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
eb137b4
Ensure validator checks that min/max values are not strings
VirajP1002 Dec 4, 2023
d62e6dd
Add validation to ensure that numbers and floats in min_value or max_…
VirajP1002 Dec 5, 2023
4abdcf2
Update test case definition
VirajP1002 Dec 5, 2023
dd592d7
Format python files
VirajP1002 Dec 5, 2023
8db4545
Fix test
VirajP1002 Dec 7, 2023
7e2d7ae
Update validation logic
VirajP1002 Dec 7, 2023
fada702
Add Number type to JSON definitions for Validator to accept floats
VirajP1002 Dec 11, 2023
8d767c5
Format python tests
VirajP1002 Dec 11, 2023
e8ff612
Update description
VirajP1002 Dec 11, 2023
bb48b37
Merge branch 'main' into support-float-max-or-min
VirajP1002 Dec 18, 2023
94afa2c
Merge branch 'main' into support-float-max-or-min
VirajP1002 Dec 19, 2023
e1103d3
Update number validator to account for value sources and update types…
VirajP1002 Dec 19, 2023
5aad464
Format python files and fix broken tests
VirajP1002 Dec 20, 2023
1141ae3
Reformat python to pass linting
VirajP1002 Dec 20, 2023
b9d25d9
Add boolean to types accepted for metadata values
VirajP1002 Dec 20, 2023
23dab79
Format python
VirajP1002 Dec 20, 2023
fa3ffa8
Add additional boolean valid type for operators
VirajP1002 Dec 20, 2023
06b4792
Merge branch 'main' into support-float-max-or-min
VirajP1002 Dec 20, 2023
26b20e0
Rename error message variable and add Decimal into values accepted fo…
VirajP1002 Dec 20, 2023
9de61b9
Format files
VirajP1002 Dec 20, 2023
62feaed
Update definitions to allow any_value type to be booleans and update …
VirajP1002 Dec 20, 2023
4c09270
Parametrize tests, add additional test cases and refactor types.py an…
VirajP1002 Dec 22, 2023
2f34f14
Refactor method in types.py
VirajP1002 Dec 22, 2023
19543f2
Refactor number_answer_validator and add type hints to types.py
VirajP1002 Jan 2, 2024
1a2d715
Add metadata of types url/uuid in test_valid_metadata.json and add ne…
VirajP1002 Jan 2, 2024
da6cc52
Format files
VirajP1002 Jan 2, 2024
d0bb128
Update when rule to use a different source rather than answers
VirajP1002 Jan 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions app/validators/answers/number_answer_validator.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
from decimal import Decimal

from app.validators.answers.answer_validator import AnswerValidator
from app.validators.routing.types import TYPE_NUMBER, resolve_value_source_json_type

MAX_NUMBER = 999_999_999_999_999
MIN_NUMBER = -999_999_999_999_999
Expand All @@ -21,6 +24,9 @@ class NumberAnswerValidator(AnswerValidator):
GREATER_DECIMALS_ON_ANSWER_REFERENCE = (
"The referenced answer has a greater number of decimal places than answer"
)
MIN_OR_MAX_IS_NOT_NUMERIC = (
"The minimum or maximum value is not a float or an integer"
)

def __init__(self, schema_element, questionnaire_schema):
super().__init__(schema_element, questionnaire_schema)
Expand All @@ -29,6 +35,12 @@ def __init__(self, schema_element, questionnaire_schema):
def validate(self):
super().validate()

self.validate_min_max_is_number()

# Prevent other validation methods that requires calculations running into errors due to types
if self.errors:
return self.errors

self.validate_decimal_places()
self.validate_mandatory_has_no_default()
self.validate_decimals()
Expand Down Expand Up @@ -58,6 +70,18 @@ def validate_mandatory_has_no_default(self):
if self.answer.get("mandatory") and self.answer.get("default") is not None:
self.add_error(self.DEFAULT_ON_MANDATORY)

def validate_min_max_is_number(self):
for min_max in ["minimum", "maximum"]:
if value := self.answer.get(min_max, {}).get("value", 0):
if isinstance(value, dict):
if (
resolve_value_source_json_type(value, self.questionnaire_schema)
!= TYPE_NUMBER
):
self.add_error(self.MIN_OR_MAX_IS_NOT_NUMERIC)
elif not isinstance(value, int | float | Decimal):
self.add_error(self.MIN_OR_MAX_IS_NOT_NUMERIC)

def validate_value_in_limits(self):
min_value = self.answer.get("minimum", {}).get("value", 0)
max_value = self.answer.get("maximum", {}).get("value", 0)
Expand Down
82 changes: 63 additions & 19 deletions app/validators/routing/types.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from app.validators.questionnaire_schema import QuestionnaireSchema

TYPE_STRING = "string"
TYPE_NUMBER = "number"
TYPE_ARRAY = "array"
Expand Down Expand Up @@ -42,36 +44,78 @@
"NoneType": TYPE_NULL,
}

METADATA_TYPE_TO_JSON_TYPE = {
"string": TYPE_STRING,
"date": TYPE_STRING,
"boolean": TYPE_BOOLEAN,
"uuid": TYPE_STRING,
"url": TYPE_STRING,
}


def resolve_answer_source_json_type(answer_id: str, schema: QuestionnaireSchema) -> str:
answer_type = schema.answers_with_context[answer_id]["answer"]["type"]
return ANSWER_TYPE_TO_JSON_TYPE[answer_type]


def resolve_calculated_summary_source_json_type(
block_id: str, schema: QuestionnaireSchema
) -> str:
block = schema.get_block(block_id)
if block["calculation"].get("answers_to_calculate"):
answer_id = block["calculation"]["answers_to_calculate"][0]
else:
answer_value_source = block["calculation"]["operation"]["+"][0]
answer_id = answer_value_source["identifier"]
answer_type = schema.answers_with_context[answer_id]["answer"]["type"]
return ANSWER_TYPE_TO_JSON_TYPE[answer_type]


def resolve_grand_calculated_summary_source_json_type(
block_id: str, schema: QuestionnaireSchema
) -> str:
block = schema.get_block(block_id)
first_calculated_summary_source = block["calculation"]["operation"]["+"][0]
return resolve_value_source_json_type(first_calculated_summary_source, schema)

def resolve_value_source_json_type(value_source, schema):

def resolve_metadata_source_json_type(
identifier: str | None, schema: QuestionnaireSchema
) -> str:
if identifier:
for values in schema.schema.get("metadata", []):
if values.get("name") == identifier:
return METADATA_TYPE_TO_JSON_TYPE[values.get("type")]
return TYPE_STRING


def resolve_list_source_json_type(selector: str | None) -> str:
return LIST_SELECTOR_TO_JSON_TYPE[selector] if selector else TYPE_ARRAY


def resolve_value_source_json_type(
value_source: dict[str, str], schema: QuestionnaireSchema
) -> str:
source = value_source["source"]
katie-gardner marked this conversation as resolved.
Show resolved Hide resolved
identifier = value_source.get("identifier")
selector = value_source.get("selector")
if source == "answers":
answer_id = value_source["identifier"]
answer_type = schema.answers_with_context[answer_id]["answer"]["type"]
return ANSWER_TYPE_TO_JSON_TYPE[answer_type]
return resolve_answer_source_json_type(identifier, schema)

if source == "calculated_summary":
block = schema.get_block(value_source["identifier"])
if block["calculation"].get("answers_to_calculate"):
answer_id = block["calculation"]["answers_to_calculate"][0]
else:
answer_value_source = block["calculation"]["operation"]["+"][0]
answer_id = answer_value_source["identifier"]
answer_type = schema.answers_with_context[answer_id]["answer"]["type"]
return ANSWER_TYPE_TO_JSON_TYPE[answer_type]
return resolve_calculated_summary_source_json_type(identifier, schema)

if source == "grand_calculated_summary":
block = schema.get_block(value_source["identifier"])
first_calculated_summary_source = block["calculation"]["operation"]["+"][0]
return resolve_value_source_json_type(first_calculated_summary_source, schema)
return resolve_grand_calculated_summary_source_json_type(identifier, schema)

if source == "metadata":
return resolve_metadata_source_json_type(identifier, schema)

if source == "list":
if selector := value_source.get("selector"):
return LIST_SELECTOR_TO_JSON_TYPE[selector]
return TYPE_ARRAY
return resolve_list_source_json_type(selector)

return TYPE_STRING


def python_type_to_json_type(python_type):
def python_type_to_json_type(python_type: str) -> str:
return PYTHON_TYPE_TO_JSON_TYPE[python_type]
9 changes: 8 additions & 1 deletion app/validators/rules/rule_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,14 @@ def _validate_comparison_operator_argument_types(
@staticmethod
def _get_valid_types_for_operator(operator_name, argument_position):
if operator_name in [Operator.EQUAL, Operator.NOT_EQUAL]:
return [TYPE_DATE, TYPE_NUMBER, TYPE_STRING, TYPE_NULL, TYPE_ARRAY]
return [
TYPE_DATE,
TYPE_NUMBER,
TYPE_STRING,
TYPE_NULL,
TYPE_ARRAY,
TYPE_BOOLEAN,
]

if operator_name in [
Operator.LESS_THAN,
Expand Down
4 changes: 2 additions & 2 deletions schemas/common_definitions.json
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@
"value_reference": {
"oneOf": [
{
"description": "A string or integer value",
"type": ["integer", "string"]
"description": "A string or number value",
"type": ["string", "number"]
},
{
"$ref": "value_sources.json#/answer_value_source"
Expand Down
4 changes: 2 additions & 2 deletions schemas/rules/definitions.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
"description": "Any value",
"oneOf": [
{
"type": ["number", "string", "array", "null"]
"type": ["number", "string", "array", "null", "boolean"]
},
{
"$ref": "../value_sources.json#/any_value_source"
Expand All @@ -122,7 +122,7 @@
"description": "Any value",
"oneOf": [
{
"type": ["number", "string", "null"]
"type": ["number", "string", "null", "boolean"]
},
{
"$ref": "../value_sources.json#/any_value_source"
Expand Down
12 changes: 12 additions & 0 deletions tests/schemas/rules/boolean/comparison/equal.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,17 @@
null
]
}
},
{
"description": "Compare metadata to a boolean",
"rule": {
"==": [
{
"source": "metadata",
"identifier": "test"
},
true
MebinAbraham marked this conversation as resolved.
Show resolved Hide resolved
]
}
}
]
27 changes: 27 additions & 0 deletions tests/schemas/valid/test_valid_metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,22 @@
{
"name": "test_metadata",
"type": "string"
},
{
"name": "flag_1",
"type": "boolean"
},
{
"name": "ref_p_start_date",
"type": "date"
},
{
"name": "case_id",
"type": "uuid"
},
{
"name": "test_url",
"type": "url"
}
],
"questionnaire_flow": {
Expand All @@ -41,6 +57,17 @@
"id": "group3",
"blocks": [
{
"skip_conditions": {
"when": {
"==": [
{
"identifier": "flag_1",
"source": "metadata"
},
true
]
}
},
"id": "block4",
"type": "Question",
"question": {
Expand Down
66 changes: 66 additions & 0 deletions tests/validators/answers/test_number_answer_validator.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import pytest

from app.validators.answers import NumberAnswerValidator
from app.validators.questionnaire_schema import (
MAX_NUMBER,
Expand Down Expand Up @@ -80,6 +82,70 @@ def test_are_decimal_places_valid():
}


@pytest.mark.parametrize(
"bounds, error_count",
[
({"minimum": {"value": "100"}}, 1),
({"maximum": {"value": "0"}}, 1),
({"maximum": {"value": "100"}, "minimum": {"value": "0"}}, 2),
],
)
def test_invalid_min_or_max_is_string(bounds, error_count):
answer = {
"calculated": True,
"description": "The total percentages should be 100%",
"id": "total-percentage",
"label": "Total",
"mandatory": False,
"q_code": "10002",
"type": "Percentage",
**bounds,
}

validator = NumberAnswerValidator(
answer, get_mock_schema_with_data_version("0.0.3")
)
validator.validate_min_max_is_number()

assert validator.errors[0] == {
"message": validator.MIN_OR_MAX_IS_NOT_NUMERIC,
"answer_id": "total-percentage",
}
assert len(validator.errors) == error_count


def test_valid_minimum_value_is_float():
answer = {
"id": "answer-2",
"mandatory": True,
"type": "Currency",
"label": "Money spent on vegetables",
"description": "Enter the full value",
"minimum": {"value": 0.00, "exclusive": True},
}

validator = NumberAnswerValidator(
answer, get_mock_schema_with_data_version("0.0.3")
)

validator.validate_min_max_is_number()

assert len(validator.errors) == 0


def test_valid_max_if_numeric_value_source():
filename = "schemas/valid/test_calculated_summary.json"
schema = QuestionnaireSchema(_open_and_load_schema_file(filename))

answer = schema.get_answer("set-maximum-answer")

validator = NumberAnswerValidator(answer, schema)

validator.validate_min_max_is_number()

katie-gardner marked this conversation as resolved.
Show resolved Hide resolved
assert len(validator.errors) == 0


def test_invalid_range():
answer = {
"id": "answer-3",
Expand Down
22 changes: 22 additions & 0 deletions tests/validators/routing/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,28 @@ def test_resolve_grand_calculated_summary_value_source_json_type():
)


@pytest.mark.parametrize(
"metadata_value_source, json_type",
berroar marked this conversation as resolved.
Show resolved Hide resolved
[
({"source": "metadata", "identifier": "ru_name"}, TYPE_STRING),
({"identifier": "flag_1", "source": "metadata"}, TYPE_BOOLEAN),
({"identifier": "ref_p_start_date", "source": "metadata"}, TYPE_STRING),
({"identifier": "case_id", "source": "metadata"}, TYPE_STRING),
({"identifier": "test_url", "source": "metadata"}, TYPE_STRING),
],
)
def test_resolve_metadata_summary_value_source_json_type(
metadata_value_source, json_type
):
filename = "schemas/valid/test_valid_metadata.json"
questionnaire_schema = QuestionnaireSchema(_open_and_load_schema_file(filename))

assert (
resolve_value_source_json_type(metadata_value_source, questionnaire_schema)
== json_type
)


@pytest.mark.parametrize(
"source, selector, json_type",
[
Expand Down
10 changes: 9 additions & 1 deletion tests/validators/routing/test_when_rule_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from app.validators.routing.types import (
TYPE_ARRAY,
TYPE_BOOLEAN,
TYPE_DATE,
TYPE_NULL,
TYPE_NUMBER,
Expand Down Expand Up @@ -152,7 +153,14 @@ def test_comparison_operator_invalid_argument_types(operator_name):
validator.validate()

if operator_name in ["==", "!="]:
valid_types = [TYPE_DATE, TYPE_NUMBER, TYPE_STRING, TYPE_NULL, TYPE_ARRAY]
valid_types = [
TYPE_DATE,
TYPE_NUMBER,
TYPE_STRING,
TYPE_NULL,
TYPE_ARRAY,
TYPE_BOOLEAN,
]
else:
valid_types = [TYPE_DATE, TYPE_NUMBER]

Expand Down