diff --git a/lung_cancer_screening/core/tests/acceptance/helpers/test_helpers.py b/lung_cancer_screening/core/tests/acceptance/helpers/test_helpers.py new file mode 100644 index 00000000..ab4ff8ec --- /dev/null +++ b/lung_cancer_screening/core/tests/acceptance/helpers/test_helpers.py @@ -0,0 +1,15 @@ +def check_labels(page, answers): + if answers is None: + return + + if isinstance(answers, str): + page.get_by_label(answers, exact=True).check() + return + + if isinstance(answers, (list, tuple)): + for answer in answers: + page.get_by_label(answer, exact=True).check() + return + + raise TypeError("answers must be a string, list, or tuple") + diff --git a/lung_cancer_screening/core/tests/acceptance/helpers/user_interaction_helpers.py b/lung_cancer_screening/core/tests/acceptance/helpers/user_interaction_helpers.py index cb52e652..7af0b1f2 100644 --- a/lung_cancer_screening/core/tests/acceptance/helpers/user_interaction_helpers.py +++ b/lung_cancer_screening/core/tests/acceptance/helpers/user_interaction_helpers.py @@ -1,4 +1,5 @@ from playwright.sync_api import expect +from .test_helpers import check_labels def setup_participant(page, live_server_url): participant_id = 'abc123' @@ -89,3 +90,12 @@ def fill_in_and_submit_asbestos_exposure(page, answer): page.get_by_label(answer, exact=True).check() page.click("text=Continue") + +def fill_in_and_submit_respiratory_conditions(page, answer): + expect(page.locator("legend")).to_have_text( + "Have you ever been diagnosed with any of the following respiratory conditions?") + + check_labels(page, answer) + + page.click("text=Continue") + diff --git a/lung_cancer_screening/core/tests/acceptance/test_cannot_change_answers_after_submission.py b/lung_cancer_screening/core/tests/acceptance/test_cannot_change_answers_after_submission.py index ced1e554..5f00b621 100644 --- a/lung_cancer_screening/core/tests/acceptance/test_cannot_change_answers_after_submission.py +++ b/lung_cancer_screening/core/tests/acceptance/test_cannot_change_answers_after_submission.py @@ -13,7 +13,8 @@ fill_in_and_submit_sex_at_birth, fill_in_and_submit_gender, fill_in_and_submit_ethnicity, - fill_in_and_submit_asbestos_exposure + fill_in_and_submit_asbestos_exposure, + fill_in_and_submit_respiratory_conditions ) class TestQuestionnaire(StaticLiveServerTestCase): @@ -48,7 +49,7 @@ def test_cannot_change_responses_once_checked_and_submitted(self): fill_in_and_submit_gender(page, "Male") fill_in_and_submit_ethnicity(page, "White") page.click("text=Continue") # education - page.click("text=Continue") # respiratory conditions + fill_in_and_submit_respiratory_conditions(page, "No, I have not had any of these respiratory conditions") fill_in_and_submit_asbestos_exposure(page, "No") page.click("text=Continue") # cancer diagnosis page.click("text=Continue") # family history diff --git a/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py b/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py index 50bfe6b9..f6bbbaf4 100644 --- a/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py +++ b/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py @@ -15,7 +15,8 @@ fill_in_and_submit_sex_at_birth, fill_in_and_submit_gender, fill_in_and_submit_ethnicity, - fill_in_and_submit_asbestos_exposure + fill_in_and_submit_asbestos_exposure, + fill_in_and_submit_respiratory_conditions ) from .helpers.assertion_helpers import expect_back_link_to_have_url @@ -96,7 +97,7 @@ def test_full_questionnaire_user_journey(self): expect(page).to_have_url(f"{self.live_server_url}/respiratory-conditions") expect_back_link_to_have_url(page, "/education") - page.click("text=Continue") + fill_in_and_submit_respiratory_conditions(page, "No, I have not had any of these respiratory conditions") expect(page).to_have_url(f"{self.live_server_url}/asbestos-exposure") expect_back_link_to_have_url(page, "/respiratory-conditions") @@ -122,7 +123,40 @@ def test_full_questionnaire_user_journey(self): expect(responses).to_contain_text("Which of these best describes you? Male") expect(responses).to_contain_text("What is your ethnic background? White") expect(responses).to_contain_text("Have you ever worked in a job where you might have been exposed to asbestos? No") + expect(responses).to_contain_text("Have you ever been diagnosed with any of the following respiratory conditions? No, I have not had any of these respiratory conditions") page.click("text=Submit") expect(page).to_have_url(f"{self.live_server_url}/your-results") + + def test_can_select_multiple_respiratory_conditions(self): + """Test that users can select multiple respiratory conditions in the UI""" + participant_id = '456' + smoking_status = 'Yes, I currently smoke' + age = datetime.now() - relativedelta(years=60) + + page = self.browser.new_page() + page.goto(f"{self.live_server_url}/start") + + fill_in_and_submit_participant_id(page, participant_id) + fill_in_and_submit_smoking_eligibility(page, smoking_status) + fill_in_and_submit_date_of_birth(page, age) + fill_in_and_submit_height_metric(page, "170") + fill_in_and_submit_weight_metric(page, "70") + fill_in_and_submit_sex_at_birth(page, "Female") + fill_in_and_submit_gender(page, "Female") + fill_in_and_submit_ethnicity(page, "White") + page.click("text=Continue") # education + + # Select multiple respiratory conditions + expect(page).to_have_url(f"{self.live_server_url}/respiratory-conditions") + fill_in_and_submit_respiratory_conditions(page, ["Pneumonia", "Emphysema"]) + + fill_in_and_submit_asbestos_exposure(page, "No") + page.click("text=Continue") # cancer diagnosis + page.click("text=Continue") # family history + + # Verify both conditions appear on the responses page + expect(page).to_have_url(f"{self.live_server_url}/responses") + responses = page.locator(".responses") + expect(responses).to_contain_text("Have you ever been diagnosed with any of the following respiratory conditions? Pneumonia, Emphysema") diff --git a/lung_cancer_screening/core/tests/acceptance/test_questionnaire_validation_errors.py b/lung_cancer_screening/core/tests/acceptance/test_questionnaire_validation_errors.py index c874a273..efc35f90 100644 --- a/lung_cancer_screening/core/tests/acceptance/test_questionnaire_validation_errors.py +++ b/lung_cancer_screening/core/tests/acceptance/test_questionnaire_validation_errors.py @@ -115,3 +115,37 @@ def test_ethnicity_validation_errors(self): expect(page.locator(".nhsuk-error-message")).to_contain_text( "Select your ethnic background." ) + + def test_respiratory_conditions_validation_errors(self): + participant_id = '123' + + page = self.browser.new_page() + page.goto(f"{self.live_server_url}/start") + fill_in_and_submit_participant_id(page, participant_id) + page.goto(f"{self.live_server_url}/respiratory-conditions") + + page.click("text=Continue") + expect(page.locator(".nhsuk-error-message")).to_contain_text( + "Select if you have had any respiratory conditions" + ) + + # Select one respiratory condition + page.get_by_label("Bronchitis").click() + + # Select None option + page.get_by_label("No, I have not had any of these respiratory conditions").click() + expect(page.locator(".nhsuk-error-message")).to_contain_text( + "Select if you have had any respiratory conditions" + ) + + # Continue + page.click("text=Continue") + + # Assert error is shown + expect(page.locator(".nhsuk-error-message")).to_contain_text( + "Select if you have had any respiratory conditions, or select 'No, I have not had any of these respiratory conditions'" + ) + + expect(page).to_have_url(f"{self.live_server_url}/respiratory-conditions") + + diff --git a/lung_cancer_screening/nhsuk_forms/choice_field.py b/lung_cancer_screening/nhsuk_forms/choice_field.py index 9ed574cb..a95ab21b 100644 --- a/lung_cancer_screening/nhsuk_forms/choice_field.py +++ b/lung_cancer_screening/nhsuk_forms/choice_field.py @@ -20,6 +20,7 @@ def __init__(self, form: forms.Form, field: "ChoiceField", name: str): self._conditional_html = {} self.dividers = {} + self.choice_hints = {} def add_conditional_html(self, value, html): if isinstance(self.field.widget, widgets.Select): @@ -36,6 +37,12 @@ def add_divider_after(self, previous, divider): def get_divider_after(self, previous): return self.dividers.get(previous) + def add_hint_for_choice(self, value, hint): + self.choice_hints[value] = hint + + def get_hint_for_choice(self, value): + return self.choice_hints.get(value) + class ChoiceField(forms.ChoiceField): """ diff --git a/lung_cancer_screening/nhsuk_forms/jinja2/checkboxes.jinja b/lung_cancer_screening/nhsuk_forms/jinja2/checkboxes.jinja new file mode 100644 index 00000000..5588d6f5 --- /dev/null +++ b/lung_cancer_screening/nhsuk_forms/jinja2/checkboxes.jinja @@ -0,0 +1,38 @@ +{% from 'nhsuk/components/checkboxes/macro.jinja' import checkboxes %} +{% set unbound_field = field.field %} +{% if field.errors %} + {% set error_message = {"text": field.errors | first} %} +{% endif %} +{% set ns = namespace(items=[]) %} +{% for value, text in unbound_field.choices %} + {% set hint_text = field.get_hint_for_choice(value) %} + {% set ns.items = ns.items + [{ + "id": field.auto_id ~ '_' ~ loop.index0, + "value": value, + "text": text, + "checked": value in (field.value() or []), + "hint": { + "text": hint_text + } if hint_text else undefined + }] %} + {% set divider = field.get_divider_after(value) %} + {% if divider %} + {% set ns.items = ns.items + [{"divider": divider}] %} + {% endif %} +{% endfor %} +{{ checkboxes({ + "name": field.html_name, + "idPrefix": field.auto_id, + "fieldset": { + "legend": { + "text": field.label, + "classes": unbound_field.label_classes + } + } if field.use_fieldset else none, + "errorMessage": error_message, + "classes": unbound_field.classes if unbound_field.classes, + "hint": { + "html": unbound_field.hint|e + } if unbound_field.hint, + "items": ns.items +}) }} diff --git a/lung_cancer_screening/nhsuk_forms/jinja2/radios.jinja b/lung_cancer_screening/nhsuk_forms/jinja2/radios.jinja index 40215935..3538584a 100644 --- a/lung_cancer_screening/nhsuk_forms/jinja2/radios.jinja +++ b/lung_cancer_screening/nhsuk_forms/jinja2/radios.jinja @@ -6,11 +6,15 @@ {% set ns = namespace(items=[]) %} {% for value, text in unbound_field.choices %} {% set conditional_html = field.conditional_html(value) %} + {% set hint_text = field.get_hint_for_choice(value) %} {% set ns.items = ns.items + [{ "id": field.auto_id if loop.first, "value": value, "text": text, "checked": field.value() == value, + "hint": { + "text": hint_text + } if hint_text else undefined, "conditional": { "html": conditional_html } if conditional_html else undefined diff --git a/lung_cancer_screening/nhsuk_forms/tests/unit/test_choice_field.py b/lung_cancer_screening/nhsuk_forms/tests/unit/test_choice_field.py index 7992b36c..ce5a651e 100644 --- a/lung_cancer_screening/nhsuk_forms/tests/unit/test_choice_field.py +++ b/lung_cancer_screening/nhsuk_forms/tests/unit/test_choice_field.py @@ -1,7 +1,7 @@ from django.test import TestCase from django.forms import Form -from ...choice_field import ChoiceField +from ...choice_field import ChoiceField, MultipleChoiceField class TestForm(Form): field = ChoiceField( @@ -11,6 +11,13 @@ class TestForm(Form): hint="Pick either one", ) +class TestMultipleChoiceForm(Form): + field = MultipleChoiceField( + label="Select options", + choices=(("a", "Option A"), ("b", "Option B"), ("c", "Option C")), + hint="Select all that apply", + ) + class TestChoiceField(TestCase): def test_renders_nhs_radios(self): self.assertHTMLEqual( @@ -74,3 +81,72 @@ class TestForm(Form): """, ) + + def test_checkbox_field_with_choice_hints(self): + """Test that choice hints are rendered correctly for checkbox fields""" + form = TestMultipleChoiceForm() + bound_field = form["field"] + + # Add hints for specific choices + bound_field.add_hint_for_choice("a", "This is hint for option A") + bound_field.add_hint_for_choice("b", "This is hint for option B") + + rendered_html = bound_field.as_field_group() + + # Verify the hints are rendered + self.assertIn('This is hint for option A', rendered_html) + self.assertIn('This is hint for option B', rendered_html) + self.assertIn('aria-describedby="id_field_0-item-hint"', rendered_html) + self.assertIn('aria-describedby="id_field_1-item-hint"', rendered_html) + + def test_get_hint_for_choice_returns_correct_hint(self): + """Test that get_hint_for_choice returns the correct hint text""" + form = TestMultipleChoiceForm() + bound_field = form["field"] + + # Add a hint + bound_field.add_hint_for_choice("a", "Hint for A") + + # Verify the hint can be retrieved + self.assertEqual(bound_field.get_hint_for_choice("a"), "Hint for A") + + def test_get_hint_for_choice_returns_none_when_no_hint(self): + """Test that get_hint_for_choice returns None when no hint is set""" + form = TestMultipleChoiceForm() + bound_field = form["field"] + + # No hint added, should return None + self.assertIsNone(bound_field.get_hint_for_choice("a")) + + def test_checkbox_field_renders_without_hints_when_none_added(self): + """Test that checkbox field renders correctly when no hints are added""" + form = TestMultipleChoiceForm() + rendered_html = form["field"].as_field_group() + + # Should render without hint elements + self.assertNotIn('nhsuk-checkboxes__hint', rendered_html) + + def test_radio_field_with_choice_hints(self): + """Test that choice hints are rendered correctly for radio fields""" + form = TestForm() + bound_field = form["field"] + + # Add hints for specific choices + bound_field.add_hint_for_choice("a", "This is hint for option A") + bound_field.add_hint_for_choice("b", "This is hint for option B") + + rendered_html = bound_field.as_field_group() + + # Verify the hints are rendered + self.assertIn('This is hint for option A', rendered_html) + self.assertIn('This is hint for option B', rendered_html) + self.assertIn('aria-describedby="id_field-item-hint"', rendered_html) + self.assertIn('aria-describedby="id_field-2-item-hint"', rendered_html) + + def test_radio_field_renders_without_hints_when_none_added(self): + """Test that radio field renders correctly when no hints are added""" + form = TestForm() + rendered_html = form["field"].as_field_group() + + # Should render without hint elements for individual items + self.assertNotIn('nhsuk-radios__hint', rendered_html) diff --git a/lung_cancer_screening/questions/forms/respiratory_conditions_form.py b/lung_cancer_screening/questions/forms/respiratory_conditions_form.py new file mode 100644 index 00000000..57911f05 --- /dev/null +++ b/lung_cancer_screening/questions/forms/respiratory_conditions_form.py @@ -0,0 +1,57 @@ +from django import forms + +from ...nhsuk_forms.choice_field import MultipleChoiceField +from ..models.response_set import ResponseSet, RespiratoryConditionValues + + +class RespiratoryConditionsForm(forms.ModelForm): + + def __init__(self, *args, **kwargs): + self.participant = kwargs.pop('participant') + super().__init__(*args, **kwargs) + self.instance.participant = self.participant + + self.fields["respiratory_conditions"] = MultipleChoiceField( + choices=RespiratoryConditionValues.choices, + widget=forms.CheckboxSelectMultiple, + label="Have you ever been diagnosed with any of the following respiratory conditions?", + label_classes="nhsuk-fieldset__legend--l", + hint="Select all that apply", + error_messages={ + 'required': 'Select if you have had any respiratory conditions', + 'singleton_option': 'Select if you have had any respiratory conditions, or select \'No, I have not had any of these respiratory conditions\'' + } + ) + + # Add hints for each choice + respiratory_conditions_field = self["respiratory_conditions"] + respiratory_conditions_field.add_hint_for_choice( + RespiratoryConditionValues.PNEUMONIA, + "An infection of the lungs, usually diagnosed by a chest x-ray" + ) + respiratory_conditions_field.add_hint_for_choice( + RespiratoryConditionValues.EMPHYSEMA, + "Damage to the air sacs in the lungs" + ) + respiratory_conditions_field.add_hint_for_choice( + RespiratoryConditionValues.BRONCHITIS, + "An ongoing inflammation of the airways in the lungs that is usually caused by an infection" + ) + respiratory_conditions_field.add_hint_for_choice( + RespiratoryConditionValues.TUBERCULOSIS, + "An infection that usually affects the lungs, but can affect any part of the body" + ) + respiratory_conditions_field.add_hint_for_choice( + RespiratoryConditionValues.COPD, + "A group of lung conditions that cause breathing difficulties" + ) + + # Add divider before "None of the above" + respiratory_conditions_field.add_divider_after( + RespiratoryConditionValues.COPD, + "or" + ) + + class Meta: + model = ResponseSet + fields = ['respiratory_conditions'] diff --git a/lung_cancer_screening/questions/jinja2/responses.jinja b/lung_cancer_screening/questions/jinja2/responses.jinja index 9e168c6d..234d581f 100644 --- a/lung_cancer_screening/questions/jinja2/responses.jinja +++ b/lung_cancer_screening/questions/jinja2/responses.jinja @@ -26,11 +26,10 @@