Skip to content

Commit 293984d

Browse files
authored
Add question error panel (#445)
* Add question error panel
1 parent 6320d73 commit 293984d

File tree

5 files changed

+49
-7
lines changed

5 files changed

+49
-7
lines changed

app/forms/questionnaire_form.py

+6-3
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,10 @@ def map_errors(self):
299299

300300
if self.question["id"] in self.question_errors:
301301
ordered_errors += [
302-
(self.question["id"], self.question_errors[self.question["id"]])
302+
(
303+
_get_error_id(self.question["id"]),
304+
self.question_errors[self.question["id"]],
305+
)
303306
]
304307

305308
for answer in self.question["answers"]:
@@ -383,8 +386,8 @@ def map_detail_answer_errors(errors, answer_json):
383386
return detail_answer_errors
384387

385388

386-
def _get_error_id(answer_id):
387-
return f"{answer_id}-error"
389+
def _get_error_id(id):
390+
return f"{id}-error"
388391

389392

390393
def _clear_detail_answer_field(form_data, question_schema):

templates/partials/question.html

+23-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
{% from "components/question/_macro.njk" import onsQuestion %}
22
{% from "components/fieldset/_macro.njk" import onsFieldset %}
33
{% from "components/panel/_macro.njk" import onsPanel %}
4-
{% from 'macros/helpers.html' import format_paragraphs %}
4+
{% from "components/error/_macro.njk" import onsError %}
55

6+
{% from 'macros/helpers.html' import format_paragraphs %}
67
{% from 'macros/helpers.html' import interviewer_note %}
78

89
{% set form = content.form %}
@@ -11,6 +12,7 @@
1112
{% set question_title= question.title %}
1213
{% set question_description = format_paragraphs(question.description) %}
1314
{% set question_instruction = format_paragraphs(question.instruction) %}
15+
{% set question_error = form.question_errors[question.id] %}
1416

1517
{% set should_wrap_with_fieldset = should_wrap_with_fieldset(question) %}
1618

@@ -93,6 +95,7 @@
9395
{%- endif -%}
9496
{%- endif -%}
9597
{% endset %}
98+
9699
{% call onsQuestion({
97100
"id": question.id,
98101
"title": title,
@@ -107,5 +110,23 @@
107110
{{ individual_response_guidance }}
108111
{{ question_definition }}
109112
{{ question_guidance }}
110-
{{ question_answers }}
113+
114+
{% if question_error %}
115+
{% set config = {
116+
"text": question_error,
117+
"id": question.id ~ '-error',
118+
"attributes": {
119+
"data-ga": "question-error",
120+
"data-ga-category": "Question error",
121+
"data-ga-action": question_error | striptags,
122+
"data-ga-label": question.id
123+
}
124+
}%}
125+
{% call onsError(config) %}
126+
{{ question_answers }}
127+
{% endcall %}
128+
{% else %}
129+
{{ question_answers }}
130+
{% endif %}
131+
111132
{% endcall %}

tests/functional/generate_pages.py

+15
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@
9999
"""
100100
)
101101

102+
QUESTION_ERROR_PANEL = Template(
103+
r""" ${questionName}ErrorPanel() { return `#${questionId}-error`; }
104+
105+
"""
106+
)
107+
102108
ANSWER_LABEL_GETTER = Template(
103109
r""" ${answerName}Label() {
104110
return `[for=${answerId}]`;
@@ -381,6 +387,15 @@ def process_question(question, page_spec, num_questions, page_name):
381387
for answer in question.get("answers", []):
382388
process_answer(answer, page_spec, long_names, page_name)
383389

390+
if question["type"] in ["DateRange", "MutuallyExclusive"]:
391+
question_name = generate_pascal_case_from_id(question["id"])
392+
question_name = question_name.replace(page_name, "")
393+
question_context = {
394+
"questionName": camel_case(question_name),
395+
"questionId": question["id"],
396+
}
397+
page_spec.write(QUESTION_ERROR_PANEL.substitute(question_context))
398+
384399

385400
def process_calculated_summary(answers, page_spec):
386401
for answer in answers:

tests/functional/spec/components/checkbox/mutually_exclusive/mutually_exclusive_checkbox.spec.js

+1
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ describe("Component: Mutually Exclusive Checkbox With Single Checkbox Override",
124124
// Then
125125
expect($(MandatoryCheckboxPage.errorHeader()).getText()).to.contain("There is a problem with your answer");
126126
expect($(MandatoryCheckboxPage.errorNumber(1)).getText()).to.contain("Select at least one answer");
127+
expect($(MandatoryCheckboxPage.questionErrorPanel()).isExisting()).to.be.true;
127128
});
128129
});
129130
});

tests/functional/spec/dates.spec.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@ describe("Date checks", () => {
6161

6262
$(DateRangePage.submit()).click();
6363

64-
// Then an error message is shown
64+
// Then an error message is shown and the question panel is highlighted
6565
expect($(DateRangePage.errorNumber(1)).getText()).to.contain("Enter a 'period to' date later than the 'period from' date");
66+
expect($(DateRangePage.dateRangeQuestionErrorPanel()).isExisting()).to.be.true;
6667

6768
// Then clicking error should focus on first input field
6869
$(DateRangePage.errorNumber(1)).click();
@@ -81,8 +82,9 @@ describe("Date checks", () => {
8182

8283
$(DateRangePage.submit()).click();
8384

84-
// Then an error message is shown
85+
// Then an error message is shown and the question panel is highlighted
8586
expect($(DateRangePage.errorNumber(1)).getText()).to.contain("Enter a 'period to' date later than the 'period from' date");
87+
expect($(DateRangePage.dateRangeQuestionErrorPanel()).isExisting()).to.be.true;
8688
});
8789

8890
it("Given the test_dates survey is selected when an invalid date is entered in a date range then an error message is shown", () => {

0 commit comments

Comments
 (0)