Skip to content

Commit

Permalink
Add rules and validation for value source in calculations (#146)
Browse files Browse the repository at this point in the history
  • Loading branch information
petechd authored Jul 13, 2022
1 parent 918d254 commit e2d7e97
Show file tree
Hide file tree
Showing 10 changed files with 240 additions and 32 deletions.
8 changes: 6 additions & 2 deletions app/validators/questions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,13 @@
from app.validators.questions.question_validator import QuestionValidator


def get_question_validator(question):
def get_question_validator(question, schema):
if question["type"] == "Calculated":
validators = {
"Calculated": CalculatedQuestionValidator,
}
return validators.get(question["type"], QuestionValidator)(question, schema)
validators = {
"Calculated": CalculatedQuestionValidator,
"DateRange": DateRangeQuestionValidator,
"MutuallyExclusive": MutuallyExclusiveQuestionValidator,
}
Expand Down
31 changes: 31 additions & 0 deletions app/validators/questions/calculated_question_validator.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
from app.validators.questions.question_validator import QuestionValidator
from app.validators.routing.types import ANSWER_TYPE_TO_JSON_TYPE, TYPE_NUMBER


class CalculatedQuestionValidator(QuestionValidator):
ANSWER_NOT_IN_QUESTION = "Answer does not exist within this question"
ANSWER_TYPE_FOR_CALCULATION_TYPE_INVALID = "Expected the answer type for calculation to be type 'number' but got type '{answer_type}'"

def validate(self):
super().validate()
self.validate_calculations()
self.validate_calculations_value_source_is_numeric()
return self.errors

def validate_calculations(self):
Expand All @@ -19,3 +22,31 @@ def validate_calculations(self):
for answer_id in calculation["answers_to_calculate"]:
if answer_id not in answer_ids:
self.add_error(self.ANSWER_NOT_IN_QUESTION, answer_id=answer_id)

def _validate_answer_is_numeric(self, answer_id):
answer_type = self.schema.get_answer_type(answer_id)

if ANSWER_TYPE_TO_JSON_TYPE[answer_type.value] != TYPE_NUMBER:
self.add_error(
self.ANSWER_TYPE_FOR_CALCULATION_TYPE_INVALID.format(
answer_type=ANSWER_TYPE_TO_JSON_TYPE[answer_type.value],
),
referenced_answer=answer_id,
)

def validate_calculations_value_source_is_numeric(self):
"""
Validates that source answer is of number type
"""
for calculation in self.question.get("calculations"):
value = calculation.get("value")

if answer_id := calculation.get("answer_id"):
self._validate_answer_is_numeric(answer_id)

elif value and value.get("source"):
answer_id = value.get("identifier")
# Calculated summary value source is validated elsewhere and must be of a number type

if value.get("source") == "answers":
self._validate_answer_is_numeric(answer_id)
3 changes: 2 additions & 1 deletion app/validators/questions/question_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ class QuestionValidator(Validator):
)
question = {}

def __init__(self, schema_element):
def __init__(self, schema_element, schema=None):
super().__init__(schema_element)
self.question = schema_element
self.answers = self.question.get("answers", [])
self.context["question_id"] = schema_element["id"]
self.schema = schema

def validate(self):
if self.question["type"] != "MutuallyExclusive":
Expand Down
4 changes: 3 additions & 1 deletion app/validators/sections/section_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ def validate_question(self, block_or_variant):
question = block_or_variant.get("question")

if question:
question_validator = get_question_validator(question)
question_validator = get_question_validator(
question, self.questionnaire_schema
)

self.errors += question_validator.validate()

Expand Down
12 changes: 10 additions & 2 deletions schemas/questions/types/calculated.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,16 @@
"description": "The id of an answer from which to obtain the total to validate against"
},
"value": {
"type": "integer",
"description": "A hard coded total to validate against"
"oneOf": [
{
"type": "number",
"description": "A hard coded total to validate against"
},
{
"$ref": "https://eq.ons.gov.uk/value_sources.json#/value_source_for_calculations",
"description": "A value source to validate against that resolves to a number."
}
]
},
"answers_to_calculate": {
"type": "array",
Expand Down
12 changes: 12 additions & 0 deletions schemas/value_sources.json
Original file line number Diff line number Diff line change
Expand Up @@ -161,5 +161,17 @@
"$ref": "https://eq.ons.gov.uk/value_sources.json#/list_value_source"
}
]
},
"value_source_for_calculations": {
"description": "Only answer and calculated summary value source are supported in current implementation",
"anyOf": [
{
"$ref": "https://eq.ons.gov.uk/value_sources.json#/answer_value_source",
"description": "Only answer types that return numeric answers are valid"
},
{
"$ref": "https://eq.ons.gov.uk/value_sources.json#/calculated_summary_value_source"
}
]
}
}
109 changes: 109 additions & 0 deletions tests/schemas/invalid/test_invalid_calculations_value_source.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
{
"mime_type": "application/json/ons/eq",
"language": "en",
"schema_version": "0.0.1",
"data_version": "0.0.3",
"survey_id": "0",
"title": "Calculated question with value sources test survey",
"theme": "default",
"description": "A survey that tests validation against value sources, answer value source is not one of Number types which is expected for calculations",
"metadata": [
{
"name": "user_id",
"type": "string"
},
{
"name": "period_id",
"type": "string"
},
{
"name": "ru_name",
"type": "string"
}
],
"questionnaire_flow": {
"type": "Linear",
"options": {
"summary": {
"collapsible": false
}
}
},
"sections": [
{
"id": "default-section",
"groups": [
{
"id": "group",
"title": "Validate sum against answer, calculated summary source",
"blocks": [
{
"type": "Question",
"id": "total-block",
"question": {
"id": "total-question",
"title": "Total",
"description": [
"Enter a number to breakdown in subsequent questions and calculated summary."
],
"type": "General",
"answers": [
{
"id": "total-answer",
"label": "Total",
"mandatory": true,
"type": "TextField"
}
]
}
},
{
"type": "Question",
"id": "breakdown-block",
"question": {
"id": "breakdown-question",
"title": "Breakdown validated against an answer value source",
"description": [
"This is a breakdown of the total number from the previous question."
],
"type": "Calculated",
"calculations": [
{
"calculation_type": "sum",
"value": {
"source": "answers",
"identifier": "total-answer"
},
"answers_to_calculate": [
"breakdown-1",
"breakdown-2",
"breakdown-3",
"breakdown-4"
],
"conditions": ["equals"]
}
],
"answers": [
{
"id": "breakdown-1",
"label": "Breakdown 1",
"mandatory": false,
"decimal_places": 2,
"type": "Number"
},
{
"id": "breakdown-2",
"label": "Breakdown 2",
"mandatory": false,
"decimal_places": 2,
"type": "Number"
}
]
}
}
]
}
]
}
]
}
25 changes: 25 additions & 0 deletions tests/schemas/valid/test_question_validation_messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,31 @@
"groups": [
{
"blocks": [
{
"type": "Question",
"id": "total-block",
"question": {
"id": "total-question",
"title": "Total",
"description": [
"Enter a number to breakdown in subsequent questions and calculated summary."
],
"type": "General",
"answers": [
{
"id": "total-answer",
"label": "Total",
"mandatory": true,
"type": "Number",
"decimal_places": 2,
"minimum": {
"value": 0,
"exclusive": true
}
}
]
}
},
{
"type": "Question",
"id": "date-block",
Expand Down
59 changes: 37 additions & 22 deletions tests/validators/blocks/test_calculated_question_validator.py
Original file line number Diff line number Diff line change
@@ -1,42 +1,50 @@
from app.validators.questions.calculated_question_validator import (
CalculatedQuestionValidator,
)
from app.validators.questionnaire_schema import QuestionnaireSchema
from app.validators.questions import get_question_validator
from tests.test_questionnaire_validator import _open_and_load_schema_file


def test_invalid_id_in_answers_to_calculate():
filename = "schemas/invalid/test_invalid_calculations_value_source.json"
schema = QuestionnaireSchema(_open_and_load_schema_file(filename))
question = {
"answers": [
{
"decimal_places": 2,
"id": "breakdown-1",
"label": "Breakdown 1",
"type": "Number",
},
{
"decimal_places": 2,
"id": "breakdown-2",
"label": "Breakdown 2",
"type": "Number",
},
"id": "breakdown-question",
"title": "Breakdown validated against an answer value source",
"description": [
"This is a breakdown of the total number from the previous question."
],
"type": "Calculated",
"calculations": [
{
"answer_id": "total-answer",
"calculation_type": "sum",
"value": {"source": "answers", "identifier": "total-answer"},
"answers_to_calculate": [
"breakdown-1",
"breakdown-2",
"breakdown-3",
"breakdown-4",
],
"calculation_type": "sum",
"conditions": ["equals"],
}
],
"id": "breakdown-question",
"title": "Breakdown",
"type": "Calculated",
"answers": [
{
"id": "breakdown-1",
"label": "Breakdown 1",
"mandatory": False,
"decimal_places": 2,
"type": "Number",
},
{
"id": "breakdown-2",
"label": "Breakdown 2",
"mandatory": False,
"decimal_places": 2,
"type": "Number",
},
],
}
validator = CalculatedQuestionValidator(question)

validator = get_question_validator(question, schema)
validator.validate()

expected_error_messages = [
Expand All @@ -50,6 +58,13 @@ def test_invalid_id_in_answers_to_calculate():
"question_id": "breakdown-question",
"answer_id": "breakdown-4",
},
{
"message": validator.ANSWER_TYPE_FOR_CALCULATION_TYPE_INVALID.format(
answer_type="string"
),
"referenced_answer": "total-answer",
"question_id": "breakdown-question",
},
]

assert expected_error_messages == validator.errors
9 changes: 5 additions & 4 deletions tests/validators/questions/test_question_validator.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from app.validators.questions import get_question_validator
from tests.conftest import get_mock_schema


def test_no_answer_label_single_answer():
Expand All @@ -9,7 +10,7 @@ def test_no_answer_label_single_answer():
"answers": [{"id": "number-1", "mandatory": False, "type": "Number"}],
}

validator = get_question_validator(question)
validator = get_question_validator(question, schema=get_mock_schema())
validator.validate()

assert not validator.errors
Expand All @@ -31,7 +32,7 @@ def test_no_answer_label_multiple_answers():
],
}

validator = get_question_validator(question)
validator = get_question_validator(question, schema=get_mock_schema())
validator.validate()

expected_error_messages = [
Expand Down Expand Up @@ -74,7 +75,7 @@ def test_no_answer_label_mutually_exclusive():
],
}

validator = get_question_validator(question)
validator = get_question_validator(question, schema=get_mock_schema())
validator.validate()

assert not validator.errors
Expand Down Expand Up @@ -106,7 +107,7 @@ def test_no_answer_label_two_answers_last_answer_single_checkbox():
],
}

validator = get_question_validator(question)
validator = get_question_validator(question, schema=get_mock_schema())
validator.validate()

assert not validator.errors

0 comments on commit e2d7e97

Please sign in to comment.