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 error for mutually exclusive #308

Merged
merged 5 commits into from
Sep 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions app/forms/questionnaire_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,17 @@ def validate_mutually_exclusive_question(self, question):
question["validation"].get("messages") if "validation" in question else None
)
answers = (getattr(self, answer["id"]).data for answer in question["answers"])
is_only_checkboxes = all(
answer["type"] == "Checkbox" for answer in question["answers"]
)

validator = MutuallyExclusiveCheck(
messages=messages, question_title=self.question_title
messages=messages,
question_title=self.question_title,
)

try:
validator(answers, is_mandatory)
validator(answers, is_mandatory, is_only_checkboxes)
except validators.ValidationError as e:
self.question_errors[question["id"]] = str(e)

Expand Down
7 changes: 5 additions & 2 deletions app/forms/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,13 +412,16 @@ def __init__(self, question_title, messages=None):
self.messages = {**error_messages, **(messages or {})}
self.question_title = question_title

def __call__(self, answer_values, is_mandatory):
def __call__(self, answer_values, is_mandatory, is_only_checkboxes):
total_answered = sum(1 for value in answer_values if value)
if total_answered > 1:
raise validators.ValidationError(self.messages["MUTUALLY_EXCLUSIVE"])
if is_mandatory and total_answered < 1:
message = format_message_with_title(
self.messages["MANDATORY_QUESTION"], self.question_title
self.messages["MANDATORY_CHECKBOX"]
if is_only_checkboxes
else self.messages["MANDATORY_QUESTION"],
self.question_title,
)
raise validators.ValidationError(message)

Expand Down
42 changes: 42 additions & 0 deletions test_schemas/en/test_mutually_exclusive.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,48 @@
}
]
},
{
"id": "mutually-exclusive-mandatory-date-section",
"title": "Date",
"summary": { "show_on_completion": true },
"groups": [
{
"id": "mutually-exclusive-mandatory-date-group",
"title": "Mutually Exclusive With Single Checkbox Override - Mandatory",
"blocks": [
{
"type": "Question",
"id": "mutually-exclusive-mandatory-date",
"question": {
"id": "mutually-exclusive-mandatory-date-question",
"type": "MutuallyExclusive",
"title": "When did you leave your last paid job?",
"mandatory": true,
"answers": [
{
"id": "mandatory-date-answer",
"label": "Enter a date",
"mandatory": false,
"type": "Date"
},
{
"id": "mandatory-date-exclusive-answer",
"mandatory": false,
"type": "Checkbox",
"options": [
{
"label": "I prefer not to say",
"value": "I prefer not to say"
}
]
}
]
}
}
]
}
]
},
{
"id": "mutually-exclusive-date-section",
"title": "Date",
Expand Down
23 changes: 15 additions & 8 deletions tests/app/forms/test_questionnaire_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@
from werkzeug.datastructures import MultiDict

from app.data_models.answer_store import Answer, AnswerStore
from app.forms import error_messages
from app.forms.questionnaire_form import generate_form
from app.forms.validators import DateRequired, ResponseRequired
from app.forms.validators import (
DateRequired,
ResponseRequired,
format_message_with_title,
)
from app.questionnaire import QuestionnaireSchema
from app.questionnaire.placeholder_renderer import PlaceholderRenderer
from app.utilities.schema import load_schema_from_name
Expand Down Expand Up @@ -1212,7 +1217,7 @@ def test_mandatory_mutually_exclusive_question_raises_error_when_not_answered(se
with self.app_request_context():
schema = load_schema_from_name("test_mutually_exclusive")

question_schema = schema.get_block("mutually-exclusive-checkbox").get(
question_schema = schema.get_block("mutually-exclusive-mandatory-date").get(
"question"
)

Expand All @@ -1226,8 +1231,10 @@ def test_mandatory_mutually_exclusive_question_raises_error_when_not_answered(se
form.validate_mutually_exclusive_question(question_schema)

self.assertEqual(
form.question_errors["mutually-exclusive-checkbox-question"],
"Enter an answer to continue",
form.question_errors["mutually-exclusive-mandatory-date-question"],
format_message_with_title(
error_messages["MANDATORY_QUESTION"], question_schema.get("title")
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test should test the standard error message rather than the checkbox one.

)

def test_mandatory_mutually_exclusive_question_raises_error_with_question_text(
Expand Down Expand Up @@ -1258,9 +1265,9 @@ def test_mandatory_mutually_exclusive_question_raises_error_with_question_text(
form.validate_mutually_exclusive_question(question_schema)
error = form.question_errors["mutually-exclusive-checkbox-question"]

assert (
error
== "Select an answer to ‘Did you really answer ‘Tuna’ to the previous question?’ to continue"
assert error == format_message_with_title(
error_messages["MANDATORY_CHECKBOX"],
"Did you really answer ‘Tuna’ to the previous question?",
)

def test_mutually_exclusive_question_raises_error_when_both_answered(self):
Expand Down Expand Up @@ -1291,7 +1298,7 @@ def test_mutually_exclusive_question_raises_error_when_both_answered(self):

self.assertEqual(
form.question_errors["mutually-exclusive-date-question"],
"Remove an answer to continue",
error_messages["MUTUALLY_EXCLUSIVE"],
)

def test_date_range_form(self):
Expand Down
50 changes: 43 additions & 7 deletions tests/app/forms/validation/test_mutually_exclusive_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,57 @@
from wtforms.validators import ValidationError

from app.forms import error_messages
from app.forms.validators import MutuallyExclusiveCheck
from app.forms.validators import MutuallyExclusiveCheck, format_message_with_title


class TestMutuallyExclusive(unittest.TestCase):
def setUp(self):
self.validator = MutuallyExclusiveCheck(question_title="")
self.question_title = ""
self.validator = MutuallyExclusiveCheck(question_title=self.question_title)

def test_mutually_exclusive_mandatory_checkbox_exception(self):
answer_permutations = [[[], []], [None, []], ["", []]]

for values in answer_permutations:
with self.assertRaises(ValidationError) as ite:
self.validator(
answer_values=iter(values),
is_mandatory=True,
is_only_checkboxes=True,
)

self.assertEqual(
format_message_with_title(
error_messages["MANDATORY_CHECKBOX"], self.question_title
),
str(ite.exception),
)

def test_mutually_exclusive_mandatory_exception(self):
answer_permutations = [[[], []], [None, []], ["", []]]

for values in answer_permutations:
with self.assertRaises(ValidationError) as ite:
self.validator(answer_values=iter(values), is_mandatory=True)
self.validator(
answer_values=iter(values),
is_mandatory=True,
is_only_checkboxes=False,
)

self.assertEqual(error_messages["MANDATORY_QUESTION"], str(ite.exception))
self.assertEqual(
format_message_with_title(
error_messages["MANDATORY_QUESTION"], self.question_title
),
str(ite.exception),
)

def test_mutually_exclusive_passes_when_optional(self):
answer_permutations = [[[], []], [None, []], ["", []]]

for values in answer_permutations:
self.validator(answer_values=iter(values), is_mandatory=False)
self.validator(
answer_values=iter(values), is_mandatory=False, is_only_checkboxes=True
)

def test_mutually_exclusive_exception(self):
answer_permutations = [
Expand All @@ -34,7 +64,11 @@ def test_mutually_exclusive_exception(self):

for values in answer_permutations:
with self.assertRaises(ValidationError) as ite:
self.validator(answer_values=iter(values), is_mandatory=True)
self.validator(
answer_values=iter(values),
is_mandatory=True,
is_only_checkboxes=True,
)

self.assertEqual(error_messages["MUTUALLY_EXCLUSIVE"], str(ite.exception))

Expand All @@ -49,4 +83,6 @@ def test_mutually_exclusive_pass(self):
]

for values in answer_permutations:
self.validator(answer_values=iter(values), is_mandatory=True)
self.validator(
answer_values=iter(values), is_mandatory=True, is_only_checkboxes=True
)
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe("Component: Mutually Exclusive Checkbox With Single Checkbox Override",

// Then
expect($(MandatoryCheckboxPage.errorHeader()).getText()).to.contain("There is a problem with your answer");
expect($(MandatoryCheckboxPage.errorNumber(1)).getText()).to.contain("Enter an answer to continue");
expect($(MandatoryCheckboxPage.errorNumber(1)).getText()).to.contain("Select at least one answer");
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ def test_mandatory_exclusive_question(self):
self.post()

# Then
self.assertInBody("Enter an answer to continue")
self.assertInBody(
'Select at least one answer <span class="u-vh">to ‘What is your nationality?’</span>'
)

def test_invalid_exclusive_answers(self):
# When
Expand Down