Update validator to support floats in min/max and disallow string typ…

…es (#230)

* Ensure validator checks that min/max values are not strings

* Add validation to ensure that numbers and floats in min_value or max_value are not declared as strings and create test cases

* Update test case definition

* Format python files

* Fix test

* Update validation logic

* Add Number type to JSON definitions for Validator to accept floats

* Format python tests

* Update description

* Update number validator to account for value sources and update types.py to check for the types of Metadata value sources

* Format python files and fix broken tests

* Reformat python to pass linting

* Add boolean to types accepted for metadata values

* Format python

* Add additional boolean valid type for operators

* Rename error message variable and add Decimal into values accepted for min/max

* Format files

* Update definitions to allow any_value type to be booleans and update tests

* Parametrize tests, add additional test cases and refactor types.py and number_answer_validator.py

* Refactor method in types.py

* Refactor number_answer_validator and add type hints to types.py

* Add metadata of types url/uuid in test_valid_metadata.json and add new test cases to reflect that these types are allowed

* Format files

* Update when rule to use a different source rather than answers

app/validators/answers/number_answer_validator.py

@@ -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
@@ -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)
@@ -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()
@@ -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)

app/validators/routing/types.py

@@ -1,3 +1,5 @@
+from app.validators.questionnaire_schema import QuestionnaireSchema
+
 TYPE_STRING = "string"
 TYPE_NUMBER = "number"
 TYPE_ARRAY = "array"
@@ -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"]
+    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]

app/validators/rules/rule_validator.py

@@ -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,

schemas/common_definitions.json

@@ -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"

schemas/rules/definitions.json

@@ -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"
@@ -122,7 +122,7 @@
     "description": "Any value",
     "oneOf": [
       {
-        "type": ["number", "string", "null"]
+        "type": ["number", "string", "null", "boolean"]
       },
       {
         "$ref": "../value_sources.json#/any_value_source"

tests/schemas/rules/boolean/comparison/equal.json

@@ -34,5 +34,17 @@
         null
       ]
     }
+  },
+  {
+    "description": "Compare metadata to a boolean",
+    "rule": {
+      "==": [
+        {
+          "source": "metadata",
+          "identifier": "test"
+        },
+        true
+      ]
+    }
   }
 ]

tests/schemas/valid/test_valid_metadata.json

@@ -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": {
@@ -41,6 +57,17 @@
           "id": "group3",
           "blocks": [
             {
+              "skip_conditions": {
+                "when": {
+                  "==": [
+                    {
+                      "identifier": "flag_1",
+                      "source": "metadata"
+                    },
+                    true
+                  ]
+                }
+              },
               "id": "block4",
               "type": "Question",
               "question": {

tests/validators/answers/test_number_answer_validator.py

@@ -1,3 +1,5 @@
+import pytest
+
 from app.validators.answers import NumberAnswerValidator
 from app.validators.questionnaire_schema import (
     MAX_NUMBER,
@@ -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()
+
+    assert len(validator.errors) == 0
+
+
 def test_invalid_range():
     answer = {
         "id": "answer-3",

tests/validators/routing/test_types.py

@@ -98,6 +98,28 @@ def test_resolve_grand_calculated_summary_value_source_json_type():
     )
 
 
+@pytest.mark.parametrize(
+    "metadata_value_source, json_type",
+    [
+        ({"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",
     [

tests/validators/routing/test_when_rule_validator.py

@@ -2,6 +2,7 @@
 
 from app.validators.routing.types import (
     TYPE_ARRAY,
+    TYPE_BOOLEAN,
     TYPE_DATE,
     TYPE_NULL,
     TYPE_NUMBER,
@@ -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]