From 09dd3e472899e813823b65ed8e6621ae40a5f05d Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Fri, 14 Nov 2025 20:26:06 +0000 Subject: [PATCH 01/12] Add create benign lumps form Introduce the initial template, form object and routing for creating benign lump history items on the medical info screen. --- .../forms/benign_lump_history_item_form.py | 100 ++++++++++++++++++ .../forms/benign_lump_history_item_form.jinja | 45 ++++++++ .../record_medical_information.jinja | 1 + .../medical_information_presenter.py | 12 +++ .../test_medical_information_presenter.py | 10 ++ manage_breast_screening/mammograms/urls.py | 6 ++ .../views/benign_lump_history_item_views.py | 56 ++++++++++ 7 files changed, 230 insertions(+) create mode 100644 manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py create mode 100644 manage_breast_screening/mammograms/jinja2/mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja create mode 100644 manage_breast_screening/mammograms/views/benign_lump_history_item_views.py diff --git a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py new file mode 100644 index 000000000..e82cacd69 --- /dev/null +++ b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py @@ -0,0 +1,100 @@ +from django.forms import Textarea + +from manage_breast_screening.core.services.auditor import Auditor +from manage_breast_screening.nhsuk_forms.fields.char_field import CharField +from manage_breast_screening.nhsuk_forms.fields.choice_fields import ( + ChoiceField, + MultipleChoiceField, +) +from manage_breast_screening.nhsuk_forms.fields.integer_field import IntegerField +from manage_breast_screening.nhsuk_forms.forms import FormWithConditionalFields +from manage_breast_screening.participants.models.benign_lump_history_item import ( + BenignLumpHistoryItem, +) + + +class BenignLumpHistoryItemForm(FormWithConditionalFields): + LOCATION_DETAIL_FIELDS = { + BenignLumpHistoryItem.ProcedureLocation.NHS_HOSPITAL: "nhs_hospital_details", + BenignLumpHistoryItem.ProcedureLocation.PRIVATE_CLINIC_UK: "private_clinic_uk_details", + BenignLumpHistoryItem.ProcedureLocation.OUTSIDE_UK: "outside_uk_details", + BenignLumpHistoryItem.ProcedureLocation.MULTIPLE_LOCATIONS: "multiple_locations_details", + BenignLumpHistoryItem.ProcedureLocation.EXACT_LOCATION_UNKNOWN: "exact_location_unknown_details", + } + + left_breast_procedures = MultipleChoiceField( + label="Left breast", + label_classes="nhsuk-fieldset__legend--s", + choices=BenignLumpHistoryItem.Procedure, + ) + right_breast_procedures = MultipleChoiceField( + label="Right breast", + label_classes="nhsuk-fieldset__legend--s", + choices=BenignLumpHistoryItem.Procedure, + ) + + procedure_year = IntegerField( + label="Year of procedure (optional)", + label_classes="nhsuk-label--m", + classes="nhsuk-input--width-4", + hint="Leave blank if unknown", + required=False, + ) + + procedure_location = ChoiceField( + label="Where were the tests and treatment done?", + choices=BenignLumpHistoryItem.ProcedureLocation, + ) + nhs_hospital_details = CharField(label="Provide details", required=False) + private_clinic_uk_details = CharField(label="Provide details", required=False) + outside_uk_details = CharField(label="Provide details", required=False) + multiple_locations_details = CharField(label="Provide details", required=False) + exact_location_unknown_details = CharField(label="Provide details", required=False) + + additional_details = CharField( + label="Additional details (optional)", + label_classes="nhsuk-label--m", + required=False, + widget=Textarea(attrs={"rows": 3}), + hint="Include any other relevant information about the procedure (optional)", + ) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + for location, detail_field in self.LOCATION_DETAIL_FIELDS.items(): + self.given_field_value("procedure_location", location).require_field( + detail_field + ) + + def create(self, request): + auditor = Auditor.from_request(request) + + benign_lump_history_item = BenignLumpHistoryItem.objects.create( + appointment=self.appointment, + left_breast_procedures=self.cleaned_data.get("left_breast_procedures", []), + right_breast_procedures=self.cleaned_data.get( + "right_breast_procedures", [] + ), + procedure_year=self.cleaned_data.get("procedure_year"), + procedure_location=self.cleaned_data["procedure_location"], + procedure_location_details=self._get_selected_location_details(), + additional_details=self.cleaned_data.get("additional_details", ""), + ) + + auditor.audit_create(benign_lump_history_item) + + return benign_lump_history_item + + @property + def location_detail_fields(self): + return tuple(self.LOCATION_DETAIL_FIELDS.items()) + + def _get_selected_location_details(self): + location = self.cleaned_data.get("procedure_location") + try: + detail_field = self.LOCATION_DETAIL_FIELDS[location] + except KeyError as exc: + msg = f"Unsupported procedure location '{location}'" + raise ValueError(msg) from exc + return self.cleaned_data.get(detail_field, "") diff --git a/manage_breast_screening/mammograms/jinja2/mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja b/manage_breast_screening/mammograms/jinja2/mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja new file mode 100644 index 000000000..11c54fa8d --- /dev/null +++ b/manage_breast_screening/mammograms/jinja2/mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja @@ -0,0 +1,45 @@ +{% extends "layout-form.jinja" %} +{% from "nhsuk/components/button/macro.jinja" import button %} +{% from "nhsuk/components/fieldset/macro.jinja" import fieldset %} + +{% block form %} + + {% do form.left_breast_procedures.add_divider_after("LUMP_REMOVED", "or") %} + {% do form.right_breast_procedures.add_divider_after("LUMP_REMOVED", "or") %} + + {% call fieldset({ + "legend": { + "text": "What benign lump procedures has " ~ participant_first_name ~ " had?", + "classes": "nhsuk-fieldset__legend--m" + } + }) %} +
+
+ {{ form.right_breast_procedures.as_field_group() }} +
+
+ {{ form.left_breast_procedures.as_field_group() }} +
+
+ {% endcall %} + + {{ form.procedure_year.as_field_group() }} + + {% for location_value, detail_field_name in form.location_detail_fields %} + {% set detail_field = form[detail_field_name] %} + {% do form.procedure_location.add_conditional_html( + location_value, + detail_field.as_field_group() + ) %} + {% endfor %} + {{ form.procedure_location.as_field_group() }} + + {{ form.additional_details.as_field_group() }} + +
+ {{ button({ + "text": "Save" + }) }} +
+ +{% endblock %} diff --git a/manage_breast_screening/mammograms/jinja2/mammograms/record_medical_information.jinja b/manage_breast_screening/mammograms/jinja2/mammograms/record_medical_information.jinja index 4123f4904..4c282f7db 100644 --- a/manage_breast_screening/mammograms/jinja2/mammograms/record_medical_information.jinja +++ b/manage_breast_screening/mammograms/jinja2/mammograms/record_medical_information.jinja @@ -79,6 +79,7 @@ {% for presented_item in presenter.benign_lump_history %} {{ summaryList(presented_item.summary_list_params) }} {% endfor %} + {{ presenter.add_benign_lump_history_link.text }}
{% endset %} {% set chest_procedures_link %} Enter other breast or chest procedures diff --git a/manage_breast_screening/mammograms/presenters/medical_information_presenter.py b/manage_breast_screening/mammograms/presenters/medical_information_presenter.py index 32eaa2a70..db7410060 100644 --- a/manage_breast_screening/mammograms/presenters/medical_information_presenter.py +++ b/manage_breast_screening/mammograms/presenters/medical_information_presenter.py @@ -185,3 +185,15 @@ def add_breast_augmentation_history_link(self): "href": url, "text": ("Add breast augmentation history"), } + + @property + def add_benign_lump_history_link(self): + url = reverse( + "mammograms:add_benign_lump_history_item", + kwargs={"pk": self.appointment.pk}, + ) + + return { + "href": url, + "text": "Add benign lump history", + } diff --git a/manage_breast_screening/mammograms/tests/presenters/test_medical_information_presenter.py b/manage_breast_screening/mammograms/tests/presenters/test_medical_information_presenter.py index fef0481c1..b53924ecf 100644 --- a/manage_breast_screening/mammograms/tests/presenters/test_medical_information_presenter.py +++ b/manage_breast_screening/mammograms/tests/presenters/test_medical_information_presenter.py @@ -141,3 +141,13 @@ def test_breast_augmentation_history_link(self): "href": f"/mammograms/{appointment.pk}/record-medical-information/breast-augmentation-history/", "text": "Add breast augmentation history", } + + def test_add_benign_lump_history_link(self): + appointment = AppointmentFactory() + + assert MedicalInformationPresenter( + appointment + ).add_benign_lump_history_link == { + "href": f"/mammograms/{appointment.pk}/record-medical-information/benign-lump-history/", + "text": "Add benign lump history", + } diff --git a/manage_breast_screening/mammograms/urls.py b/manage_breast_screening/mammograms/urls.py index d7460b7d2..713036031 100644 --- a/manage_breast_screening/mammograms/urls.py +++ b/manage_breast_screening/mammograms/urls.py @@ -2,6 +2,7 @@ from .views import ( appointment_views, + benign_lump_history_item_views, breast_augmentation_history_view, cyst_history_view, implanted_medical_device_history_view, @@ -137,4 +138,9 @@ breast_augmentation_history_view.AddBreastAugmentationHistoryView.as_view(), name="add_breast_augmentation_history_item", ), + path( + "/record-medical-information/benign-lump-history/", + benign_lump_history_item_views.AddBenignLumpHistoryItemView.as_view(), + name="add_benign_lump_history_item", + ), ] diff --git a/manage_breast_screening/mammograms/views/benign_lump_history_item_views.py b/manage_breast_screening/mammograms/views/benign_lump_history_item_views.py new file mode 100644 index 000000000..a77ff5759 --- /dev/null +++ b/manage_breast_screening/mammograms/views/benign_lump_history_item_views.py @@ -0,0 +1,56 @@ +from django.contrib import messages +from django.urls import reverse +from django.views.generic import FormView + +from manage_breast_screening.mammograms.forms.benign_lump_history_item_form import ( + BenignLumpHistoryItemForm, +) + +from .mixins import InProgressAppointmentMixin + + +class AddBenignLumpHistoryItemView(InProgressAppointmentMixin, FormView): + form_class = BenignLumpHistoryItemForm + template_name = "mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja" + + def form_valid(self, form): + form.create(appointment=self.appointment, request=self.request) + + messages.add_message( + self.request, + messages.SUCCESS, + "Added benign lump history", + ) + + return super().form_valid(form) + + def get_success_url(self): + return reverse( + "mammograms:record_medical_information", kwargs={"pk": self.appointment.pk} + ) + + def get_back_link_params(self): + return { + "href": reverse( + "mammograms:record_medical_information", + kwargs={"pk": self.appointment_pk}, + ), + "text": "Back", + } + + def get_context_data(self, **kwargs): + context = super().get_context_data() + + participant = self.appointment.participant + + context.update( + { + "back_link_params": self.get_back_link_params(), + "caption": participant.full_name, + "participant_first_name": participant.first_name, + "heading": "Add details of benign lumps", + "page_title": "Add details of benign lumps", + }, + ) + + return context From e068b7df1bdfb375a901355c405af4130c6e7386 Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Mon, 17 Nov 2025 16:36:07 +0000 Subject: [PATCH 02/12] Add YearField to encapsulate year bound validation logic - Introduce YearField as a specific type of IntegerField that validates for a year between 80 years ago and today. - Use this in the benign lump history item form. --- .../forms/benign_lump_history_item_form.py | 4 +-- .../nhsuk_forms/fields/integer_field.py | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py index e82cacd69..0a79b8427 100644 --- a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py +++ b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py @@ -6,7 +6,7 @@ ChoiceField, MultipleChoiceField, ) -from manage_breast_screening.nhsuk_forms.fields.integer_field import IntegerField +from manage_breast_screening.nhsuk_forms.fields.integer_field import YearField from manage_breast_screening.nhsuk_forms.forms import FormWithConditionalFields from manage_breast_screening.participants.models.benign_lump_history_item import ( BenignLumpHistoryItem, @@ -33,7 +33,7 @@ class BenignLumpHistoryItemForm(FormWithConditionalFields): choices=BenignLumpHistoryItem.Procedure, ) - procedure_year = IntegerField( + procedure_year = YearField( label="Year of procedure (optional)", label_classes="nhsuk-label--m", classes="nhsuk-input--width-4", diff --git a/manage_breast_screening/nhsuk_forms/fields/integer_field.py b/manage_breast_screening/nhsuk_forms/fields/integer_field.py index 880ab0381..4be311986 100644 --- a/manage_breast_screening/nhsuk_forms/fields/integer_field.py +++ b/manage_breast_screening/nhsuk_forms/fields/integer_field.py @@ -1,3 +1,5 @@ +from datetime import date + from django import forms @@ -33,3 +35,34 @@ def widget_attrs(self, widget): attrs.pop("step", None) return attrs + + +class YearField(IntegerField): + def __init__( + self, + *args, + hint=None, + label_classes=None, + classes=None, + min_value=None, + max_value=None, + **kwargs, + ): + if min_value is None: + min_value = date.today().year - 80 + if max_value is None: + max_value = date.today().year + + year_bounds_error = f"Year must be between {min_value} and {max_value}" + + if "error_messages" not in kwargs: + kwargs["error_messages"] = {} + + kwargs["error_messages"].setdefault("min_value", year_bounds_error) + kwargs["error_messages"].setdefault("max_value", year_bounds_error) + kwargs["min_value"] = min_value + kwargs["max_value"] = max_value + + super().__init__( + *args, hint=hint, label_classes=label_classes, classes=classes, **kwargs + ) From e969b73bca5cfcf1aa22b9ff209cc3ed32e3ffda Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Tue, 18 Nov 2025 09:04:07 +0000 Subject: [PATCH 03/12] Benign lumps: Make NO_PROCEDURES an exclusive choice --- .../mammograms/forms/benign_lump_history_item_form.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py index 0a79b8427..6ba26e9dc 100644 --- a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py +++ b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py @@ -26,11 +26,13 @@ class BenignLumpHistoryItemForm(FormWithConditionalFields): label="Left breast", label_classes="nhsuk-fieldset__legend--s", choices=BenignLumpHistoryItem.Procedure, + exclusive_choices={"NO_PROCEDURES"}, ) right_breast_procedures = MultipleChoiceField( label="Right breast", label_classes="nhsuk-fieldset__legend--s", choices=BenignLumpHistoryItem.Procedure, + exclusive_choices={"NO_PROCEDURES"}, ) procedure_year = YearField( From 78580bb3ac2f564d5f4e15386dcb52b4c5b4ade4 Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Wed, 19 Nov 2025 10:56:12 +0000 Subject: [PATCH 04/12] Benign lumps: Make procedures legend aria-hidden Matching the choice in the prototype to make this heading aria-hidden=true. --- .../forms/benign_lump_history_item_form.jinja | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/manage_breast_screening/mammograms/jinja2/mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja b/manage_breast_screening/mammograms/jinja2/mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja index 11c54fa8d..1e35d785e 100644 --- a/manage_breast_screening/mammograms/jinja2/mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja +++ b/manage_breast_screening/mammograms/jinja2/mammograms/medical_information/medical_history/forms/benign_lump_history_item_form.jinja @@ -7,21 +7,18 @@ {% do form.left_breast_procedures.add_divider_after("LUMP_REMOVED", "or") %} {% do form.right_breast_procedures.add_divider_after("LUMP_REMOVED", "or") %} - {% call fieldset({ - "legend": { - "text": "What benign lump procedures has " ~ participant_first_name ~ " had?", - "classes": "nhsuk-fieldset__legend--m" - } - }) %} -
-
- {{ form.right_breast_procedures.as_field_group() }} -
-
- {{ form.left_breast_procedures.as_field_group() }} -
+ + +
+
+ {{ form.right_breast_procedures.as_field_group() }} +
+
+ {{ form.left_breast_procedures.as_field_group() }}
- {% endcall %} +
{{ form.procedure_year.as_field_group() }} From fcd0d414a24242add9cdee49a0881c4ce37aeb1f Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Wed, 19 Nov 2025 15:36:02 +0000 Subject: [PATCH 05/12] Benign lumps: Update validation error message wording --- .../forms/benign_lump_history_item_form.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py index 6ba26e9dc..dfa50df15 100644 --- a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py +++ b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py @@ -27,12 +27,18 @@ class BenignLumpHistoryItemForm(FormWithConditionalFields): label_classes="nhsuk-fieldset__legend--s", choices=BenignLumpHistoryItem.Procedure, exclusive_choices={"NO_PROCEDURES"}, + error_messages={ + "required": "Select which procedures they have had in the left breast", + }, ) right_breast_procedures = MultipleChoiceField( label="Right breast", label_classes="nhsuk-fieldset__legend--s", choices=BenignLumpHistoryItem.Procedure, exclusive_choices={"NO_PROCEDURES"}, + error_messages={ + "required": "Select which procedures they have had in the right breast", + }, ) procedure_year = YearField( @@ -46,6 +52,9 @@ class BenignLumpHistoryItemForm(FormWithConditionalFields): procedure_location = ChoiceField( label="Where were the tests and treatment done?", choices=BenignLumpHistoryItem.ProcedureLocation, + error_messages={ + "required": "Select where the tests and treatment were done", + }, ) nhs_hospital_details = CharField(label="Provide details", required=False) private_clinic_uk_details = CharField(label="Provide details", required=False) @@ -69,11 +78,11 @@ def __init__(self, *args, **kwargs): detail_field ) - def create(self, request): + def create(self, appointment, request): auditor = Auditor.from_request(request) benign_lump_history_item = BenignLumpHistoryItem.objects.create( - appointment=self.appointment, + appointment=appointment, left_breast_procedures=self.cleaned_data.get("left_breast_procedures", []), right_breast_procedures=self.cleaned_data.get( "right_breast_procedures", [] From 471a5f18709f7c0753647ba1059b96f86f3cb78b Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Wed, 19 Nov 2025 15:45:27 +0000 Subject: [PATCH 06/12] Benign lumps: Add system test for creating --- .../clinical/test_benign_lump_history.py | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 manage_breast_screening/tests/system/clinical/test_benign_lump_history.py diff --git a/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py b/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py new file mode 100644 index 000000000..b0258ed41 --- /dev/null +++ b/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py @@ -0,0 +1,141 @@ +from django.urls import reverse +from playwright.sync_api import expect + +from manage_breast_screening.participants.tests.factories import ( + AppointmentFactory, + ParticipantFactory, + ScreeningEpisodeFactory, +) + +from ..system_test_setup import SystemTestCase + + +class TestBenignLumpHistory(SystemTestCase): + def test_adding_a_benign_lump_history_item(self): + self.given_i_am_logged_in_as_a_clinical_user() + self.and_there_is_an_appointment() + self.and_i_am_on_the_record_medical_information_page() + self.when_i_click_on_add_benign_lump_history() + self.then_i_see_the_add_benign_lump_history_form() + self.when_i_try_to_save_without_entering_benign_lump_details() + self.then_i_see_validation_errors_for_missing_benign_lump_details() + + self.when_i_select_procedures_for_each_breast() + self.and_i_enter_the_year_and_select_a_location() + self.and_i_enter_additional_details() + self.and_i_click_save_benign_lump_history() + self.then_i_see_a_validation_error_for_missing_location_details() + + self.when_i_enter_the_location_details() + self.and_i_click_save_benign_lump_history() + self.then_i_am_back_on_the_medical_information_page() + self.and_the_benign_lump_history_item_is_listed() + self.and_the_message_says_benign_lump_history_added() + + def test_accessibility(self): + self.given_i_am_logged_in_as_a_clinical_user() + self.and_there_is_an_appointment() + self.and_i_am_on_the_record_medical_information_page() + self.when_i_click_on_add_benign_lump_history() + self.then_the_accessibility_baseline_is_met() + + def and_there_is_an_appointment(self): + self.participant = ParticipantFactory(first_name="Sonia", last_name="Dhillon") + self.screening_episode = ScreeningEpisodeFactory(participant=self.participant) + self.appointment = AppointmentFactory( + screening_episode=self.screening_episode, + clinic_slot__clinic__setting__provider=self.current_provider, + ) + + def and_i_am_on_the_record_medical_information_page(self): + self.page.goto( + self.live_server_url + + reverse( + "mammograms:record_medical_information", + kwargs={"pk": self.appointment.pk}, + ) + ) + + def when_i_click_on_add_benign_lump_history(self): + self.page.get_by_text("Add benign lump history", exact=True).click() + + def then_i_see_the_add_benign_lump_history_form(self): + expect(self.page.get_by_text("Add details of benign lumps")).to_be_visible() + self.assert_page_title_contains("Add details of benign lumps") + + def when_i_try_to_save_without_entering_benign_lump_details(self): + self.page.get_by_role("button", name="Save").click() + + def then_i_see_validation_errors_for_missing_benign_lump_details(self): + self.expect_validation_error( + error_text="Select which procedures they have had in the left breast", + fieldset_legend="Left breast", + field_label="Needle biopsy", + ) + self.expect_validation_error( + error_text="Select which procedures they have had in the right breast", + fieldset_legend="Right breast", + field_label="Needle biopsy", + ) + self.expect_validation_error( + error_text="Select where the tests and treatment were done", + fieldset_legend="Where were the tests and treatment done?", + field_label="At an NHS hospital", + ) + + def when_i_select_procedures_for_each_breast(self): + right_fieldset = self.page.get_by_role("group", name="Right breast", exact=True) + right_fieldset.get_by_label("Needle biopsy", exact=True).check() + right_fieldset.get_by_label("Lump removed", exact=True).check() + + left_fieldset = self.page.get_by_role("group", name="Left breast", exact=True) + left_fieldset.get_by_label("Lump removed", exact=True).check() + + def and_i_enter_the_year_and_select_a_location(self): + self.page.get_by_label("Year of procedure (optional)", exact=True).fill("2022") + self.page.get_by_label("At an NHS hospital", exact=True).click() + + def when_i_enter_the_location_details(self): + self.page.locator("#id_nhs_hospital_details").fill( + "St Thomas' Hospital, London" + ) + + def and_i_enter_additional_details(self): + self.page.get_by_label("Additional details (optional)", exact=True).fill( + "Participant described no complications following any of the procedures." + ) + + def and_i_click_save_benign_lump_history(self): + self.page.get_by_role("button", name="Save").click() + + def then_i_see_a_validation_error_for_missing_location_details(self): + self.expect_validation_error( + error_text="This field is required.", + fieldset_legend="Where were the tests and treatment done?", + field_label="Provide details", + field_name="nhs_hospital_details", + ) + + def then_i_am_back_on_the_medical_information_page(self): + self.expect_url("mammograms:record_medical_information", pk=self.appointment.pk) + + def and_the_benign_lump_history_item_is_listed(self): + key = self.page.locator( + ".nhsuk-summary-list__key", + has=self.page.get_by_text("Benign lump history", exact=True), + ) + row = self.page.locator(".nhsuk-summary-list__row").filter(has=key) + + expect(row).to_contain_text("Right breast: Needle biopsy, Lump removed") + expect(row).to_contain_text("Left breast: Lump removed") + expect(row).to_contain_text("2022") + expect(row).to_contain_text("At an NHS hospital: St Thomas' Hospital, London") + expect(row).to_contain_text( + "Participant described no complications following any of the procedures." + ) + + def and_the_message_says_benign_lump_history_added(self): + alert = self.page.get_by_role("alert") + + expect(alert).to_contain_text("Success") + expect(alert).to_contain_text("Added benign lump history") From f53dc125c796a9c2ccd659221fad15c2e80d5b7d Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Wed, 19 Nov 2025 16:32:54 +0000 Subject: [PATCH 07/12] Benign lumps: Add tests for form object --- .../test_benign_lump_history_item_form.py | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 manage_breast_screening/mammograms/tests/forms/test_benign_lump_history_item_form.py diff --git a/manage_breast_screening/mammograms/tests/forms/test_benign_lump_history_item_form.py b/manage_breast_screening/mammograms/tests/forms/test_benign_lump_history_item_form.py new file mode 100644 index 000000000..40623c30e --- /dev/null +++ b/manage_breast_screening/mammograms/tests/forms/test_benign_lump_history_item_form.py @@ -0,0 +1,128 @@ +import datetime +from urllib.parse import urlencode + +import pytest +from django.http import QueryDict +from django.test import RequestFactory + +from manage_breast_screening.core.models import AuditLog +from manage_breast_screening.mammograms.forms.benign_lump_history_item_form import ( + BenignLumpHistoryItemForm, +) +from manage_breast_screening.participants.models.benign_lump_history_item import ( + BenignLumpHistoryItem, +) +from manage_breast_screening.participants.tests.factories import AppointmentFactory + + +def _form_data(data): + return QueryDict(urlencode(data, doseq=True)) + + +@pytest.mark.django_db +class TestBenignLumpHistoryItemForm: + def test_missing_required_fields(self): + form = BenignLumpHistoryItemForm(_form_data({})) + + assert not form.is_valid() + assert form.errors == { + "left_breast_procedures": [ + "Select which procedures they have had in the left breast" + ], + "right_breast_procedures": [ + "Select which procedures they have had in the right breast" + ], + "procedure_location": ["Select where the tests and treatment were done"], + } + + @pytest.mark.parametrize( + ("location", "detail_field"), + tuple(BenignLumpHistoryItemForm.LOCATION_DETAIL_FIELDS.items()), + ) + def test_requires_location_details_for_selected_location( + self, location, detail_field + ): + form = BenignLumpHistoryItemForm( + _form_data( + [ + ( + "procedure_location", + location, + ), + ] + ) + ) + + assert not form.is_valid() + assert form.errors.get(detail_field) == ["This field is required."] + + @pytest.mark.parametrize( + "procedure_year", + [ + datetime.date.today().year - 80 - 1, + datetime.date.today().year + 1, + ], + ) + def test_procedure_year_must_be_within_range(self, procedure_year): + max_year = datetime.date.today().year + min_year = max_year - 80 + year_outside_range_message = f"Year must be between {min_year} and {max_year}" + + form = BenignLumpHistoryItemForm( + _form_data( + [ + ("procedure_year", procedure_year), + ] + ) + ) + + assert not form.is_valid() + assert form.errors.get("procedure_year") == [year_outside_range_message] + + def test_create_persists_data_and_audits(self, clinical_user): + appointment = AppointmentFactory() + request = RequestFactory().post("/test-form") + request.user = clinical_user + + data = [ + ( + "left_breast_procedures", + BenignLumpHistoryItem.Procedure.NEEDLE_BIOPSY, + ), + ( + "right_breast_procedures", + BenignLumpHistoryItem.Procedure.NO_PROCEDURES, + ), + ("procedure_year", datetime.date.today().year - 1), + ( + "procedure_location", + BenignLumpHistoryItem.ProcedureLocation.NHS_HOSPITAL, + ), + ("nhs_hospital_details", "St Thomas' Hospital"), + ("additional_details", "Additional details."), + ] + form = BenignLumpHistoryItemForm(_form_data(data)) + assert form.is_valid() + + existing_log_count = AuditLog.objects.count() + obj = form.create(appointment=appointment, request=request) + assert AuditLog.objects.count() == existing_log_count + 1 + audit_log = AuditLog.objects.filter( + object_id=obj.pk, operation=AuditLog.Operations.CREATE + ).first() + assert audit_log.actor == clinical_user + + obj.refresh_from_db() + assert obj.appointment == appointment + assert obj.left_breast_procedures == [ + BenignLumpHistoryItem.Procedure.NEEDLE_BIOPSY + ] + assert obj.right_breast_procedures == [ + BenignLumpHistoryItem.Procedure.NO_PROCEDURES + ] + assert obj.procedure_year == data[2][1] + assert obj.procedure_location == ( + BenignLumpHistoryItem.ProcedureLocation.NHS_HOSPITAL + ) + assert obj.procedure_location_details == "St Thomas' Hospital" + assert obj.additional_details == "Additional details." From 156864e610ca472308200745bded34050b76ac8c Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Fri, 21 Nov 2025 14:23:40 +0000 Subject: [PATCH 08/12] Benign lumps: make YearField bounds callable The current YearField validation on min and max value is implemented in a way that means values based on the current year will only be evaluated on module import. An application restart after moving from one year to the next would be needed to ensure current year is correct. This isn't a huge bug necessarily with our current use cases, but it's an inconvenient issue to leave in the codebase. This commit switches to using min_value_callable and max_value_callable to determine the year bounds. These can be overridden, but by default specify a range of 80 years to current year. --- .../nhsuk_forms/fields/__init__.py | 5 +- .../nhsuk_forms/fields/integer_field.py | 59 ++++++++++++++----- .../tests/fields/test_integer_field.py | 26 +++++++- 3 files changed, 71 insertions(+), 19 deletions(-) diff --git a/manage_breast_screening/nhsuk_forms/fields/__init__.py b/manage_breast_screening/nhsuk_forms/fields/__init__.py index f47a0fbe0..972084977 100644 --- a/manage_breast_screening/nhsuk_forms/fields/__init__.py +++ b/manage_breast_screening/nhsuk_forms/fields/__init__.py @@ -1,14 +1,15 @@ from .boolean_field import BooleanField from .char_field import CharField from .choice_fields import ChoiceField, MultipleChoiceField -from .integer_field import IntegerField +from .integer_field import IntegerField, YearField from .split_date_field import SplitDateField __all__ = [ "BooleanField", "CharField", - "IntegerField", "ChoiceField", + "IntegerField", "MultipleChoiceField", "SplitDateField", + "YearField", ] diff --git a/manage_breast_screening/nhsuk_forms/fields/integer_field.py b/manage_breast_screening/nhsuk_forms/fields/integer_field.py index 4be311986..ad9c261d1 100644 --- a/manage_breast_screening/nhsuk_forms/fields/integer_field.py +++ b/manage_breast_screening/nhsuk_forms/fields/integer_field.py @@ -44,25 +44,52 @@ def __init__( hint=None, label_classes=None, classes=None, - min_value=None, - max_value=None, + min_value_callable=None, + max_value_callable=None, **kwargs, ): - if min_value is None: - min_value = date.today().year - 80 - if max_value is None: - max_value = date.today().year - - year_bounds_error = f"Year must be between {min_value} and {max_value}" - - if "error_messages" not in kwargs: - kwargs["error_messages"] = {} - - kwargs["error_messages"].setdefault("min_value", year_bounds_error) - kwargs["error_messages"].setdefault("max_value", year_bounds_error) - kwargs["min_value"] = min_value - kwargs["max_value"] = max_value + self.min_value_callable = min_value_callable + self.max_value_callable = max_value_callable super().__init__( *args, hint=hint, label_classes=label_classes, classes=classes, **kwargs ) + + def _default_min_year(self): + return date.today().year - 80 + + def _default_max_year(self): + return date.today().year + + def validate(self, value): + super().validate(value) + + if value in self.empty_values: + return + + min_value_callable = self.min_value_callable or self._default_min_year + max_value_callable = self.max_value_callable or self._default_max_year + + min_value = min_value_callable() + max_value = max_value_callable() + + if value < min_value: + raise forms.ValidationError( + self.error_messages.get( + "min_value", "Year must be greater than %(min_value)s" + ), + code="min_value", + params={"min_value": min_value}, + ) + + if value > max_value: + raise forms.ValidationError( + self.error_messages.get( + "max_value", "Year must be less than %(max_value)s" + ), + code="max_value", + params={"max_value": max_value}, + ) + + +# End of file diff --git a/manage_breast_screening/nhsuk_forms/tests/fields/test_integer_field.py b/manage_breast_screening/nhsuk_forms/tests/fields/test_integer_field.py index c6433e071..65f51c044 100644 --- a/manage_breast_screening/nhsuk_forms/tests/fields/test_integer_field.py +++ b/manage_breast_screening/nhsuk_forms/tests/fields/test_integer_field.py @@ -2,7 +2,7 @@ from django.forms import Form from pytest_django.asserts import assertHTMLEqual -from ...fields import IntegerField +from ...fields import IntegerField, YearField class TestIntegerField: @@ -30,3 +30,27 @@ def test_renders_nhs_input(self, form_class):
""", ) + + +class TestYearField: + def test_uses_callable_bounds(self): + bounds = {"min": 2000, "max": 2005} + + class TestForm(Form): + field = YearField( + min_value_callable=lambda: bounds["min"], + max_value_callable=lambda: bounds["max"], + ) + + assert TestForm(data={"field": 2003}).is_valid() + + bounds["max"] = 2002 + form = TestForm(data={"field": 2003}) + assert not form.is_valid() + assert form.errors["field"] == ["Year must be less than 2002"] + + bounds["min"] = 2000 + form = TestForm(data={"field": 1999}) + + assert not form.is_valid() + assert form.errors["field"] == ["Year must be greater than 2000"] From 9a4ccbe031ac8f86f05a3e7c05da83ce9415dacd Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Fri, 21 Nov 2025 14:46:07 +0000 Subject: [PATCH 09/12] Benign lumps: rework success flash message warning --- .../test_benign_lump_history_item_form.py | 26 ++++++++++++------- .../views/benign_lump_history_item_views.py | 2 +- .../nhsuk_forms/fields/integer_field.py | 7 ++--- .../tests/fields/test_integer_field.py | 4 +-- .../clinical/test_benign_lump_history.py | 2 +- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/manage_breast_screening/mammograms/tests/forms/test_benign_lump_history_item_form.py b/manage_breast_screening/mammograms/tests/forms/test_benign_lump_history_item_form.py index 40623c30e..439280844 100644 --- a/manage_breast_screening/mammograms/tests/forms/test_benign_lump_history_item_form.py +++ b/manage_breast_screening/mammograms/tests/forms/test_benign_lump_history_item_form.py @@ -54,20 +54,26 @@ def test_requires_location_details_for_selected_location( ) assert not form.is_valid() - assert form.errors.get(detail_field) == ["This field is required."] + assert form.errors.get(detail_field) == [ + "Provide details about where the surgery and treatment took place" + ] @pytest.mark.parametrize( - "procedure_year", + ("procedure_year", "expected_message"), [ - datetime.date.today().year - 80 - 1, - datetime.date.today().year + 1, + ( + datetime.date.today().year - 80 - 1, + f"Year must be {datetime.date.today().year - 80} or later", + ), + ( + datetime.date.today().year + 1, + f"Year must be {datetime.date.today().year} or earlier", + ), ], ) - def test_procedure_year_must_be_within_range(self, procedure_year): - max_year = datetime.date.today().year - min_year = max_year - 80 - year_outside_range_message = f"Year must be between {min_year} and {max_year}" - + def test_procedure_year_must_be_within_range( + self, procedure_year, expected_message + ): form = BenignLumpHistoryItemForm( _form_data( [ @@ -77,7 +83,7 @@ def test_procedure_year_must_be_within_range(self, procedure_year): ) assert not form.is_valid() - assert form.errors.get("procedure_year") == [year_outside_range_message] + assert form.errors.get("procedure_year") == [expected_message] def test_create_persists_data_and_audits(self, clinical_user): appointment = AppointmentFactory() diff --git a/manage_breast_screening/mammograms/views/benign_lump_history_item_views.py b/manage_breast_screening/mammograms/views/benign_lump_history_item_views.py index a77ff5759..bfc27045d 100644 --- a/manage_breast_screening/mammograms/views/benign_lump_history_item_views.py +++ b/manage_breast_screening/mammograms/views/benign_lump_history_item_views.py @@ -19,7 +19,7 @@ def form_valid(self, form): messages.add_message( self.request, messages.SUCCESS, - "Added benign lump history", + "Benign lump history added", ) return super().form_valid(form) diff --git a/manage_breast_screening/nhsuk_forms/fields/integer_field.py b/manage_breast_screening/nhsuk_forms/fields/integer_field.py index ad9c261d1..b58d9f521 100644 --- a/manage_breast_screening/nhsuk_forms/fields/integer_field.py +++ b/manage_breast_screening/nhsuk_forms/fields/integer_field.py @@ -76,7 +76,7 @@ def validate(self, value): if value < min_value: raise forms.ValidationError( self.error_messages.get( - "min_value", "Year must be greater than %(min_value)s" + "min_value", "Year must be %(min_value)s or later" ), code="min_value", params={"min_value": min_value}, @@ -85,11 +85,8 @@ def validate(self, value): if value > max_value: raise forms.ValidationError( self.error_messages.get( - "max_value", "Year must be less than %(max_value)s" + "max_value", "Year must be %(max_value)s or earlier" ), code="max_value", params={"max_value": max_value}, ) - - -# End of file diff --git a/manage_breast_screening/nhsuk_forms/tests/fields/test_integer_field.py b/manage_breast_screening/nhsuk_forms/tests/fields/test_integer_field.py index 65f51c044..2019887d4 100644 --- a/manage_breast_screening/nhsuk_forms/tests/fields/test_integer_field.py +++ b/manage_breast_screening/nhsuk_forms/tests/fields/test_integer_field.py @@ -47,10 +47,10 @@ class TestForm(Form): bounds["max"] = 2002 form = TestForm(data={"field": 2003}) assert not form.is_valid() - assert form.errors["field"] == ["Year must be less than 2002"] + assert form.errors["field"] == ["Year must be 2002 or earlier"] bounds["min"] = 2000 form = TestForm(data={"field": 1999}) assert not form.is_valid() - assert form.errors["field"] == ["Year must be greater than 2000"] + assert form.errors["field"] == ["Year must be 2000 or later"] diff --git a/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py b/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py index b0258ed41..450018dd9 100644 --- a/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py +++ b/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py @@ -138,4 +138,4 @@ def and_the_message_says_benign_lump_history_added(self): alert = self.page.get_by_role("alert") expect(alert).to_contain_text("Success") - expect(alert).to_contain_text("Added benign lump history") + expect(alert).to_contain_text("Benign lump history added") From 9372a8701866efa397427c36d5485b92504614f2 Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Mon, 24 Nov 2025 10:42:26 +0000 Subject: [PATCH 10/12] Benign lumps: add visually hidden text to left/right breast fields --- .../mammograms/forms/benign_lump_history_item_form.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py index dfa50df15..a38c62f62 100644 --- a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py +++ b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py @@ -25,6 +25,8 @@ class BenignLumpHistoryItemForm(FormWithConditionalFields): left_breast_procedures = MultipleChoiceField( label="Left breast", label_classes="nhsuk-fieldset__legend--s", + visually_hidden_label_prefix="What procedure have they had in their ", + visually_hidden_label_suffix="?", choices=BenignLumpHistoryItem.Procedure, exclusive_choices={"NO_PROCEDURES"}, error_messages={ @@ -34,6 +36,8 @@ class BenignLumpHistoryItemForm(FormWithConditionalFields): right_breast_procedures = MultipleChoiceField( label="Right breast", label_classes="nhsuk-fieldset__legend--s", + visually_hidden_label_prefix="What procedure have they had in their ", + visually_hidden_label_suffix="?", choices=BenignLumpHistoryItem.Procedure, exclusive_choices={"NO_PROCEDURES"}, error_messages={ From 43d26fabe96dac259b4636705c8166b589ef14e0 Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Mon, 24 Nov 2025 10:52:15 +0000 Subject: [PATCH 11/12] Benign lumps: add more detailed validation message to location details --- .../forms/benign_lump_history_item_form.py | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py index a38c62f62..00b55b925 100644 --- a/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py +++ b/manage_breast_screening/mammograms/forms/benign_lump_history_item_form.py @@ -60,11 +60,41 @@ class BenignLumpHistoryItemForm(FormWithConditionalFields): "required": "Select where the tests and treatment were done", }, ) - nhs_hospital_details = CharField(label="Provide details", required=False) - private_clinic_uk_details = CharField(label="Provide details", required=False) - outside_uk_details = CharField(label="Provide details", required=False) - multiple_locations_details = CharField(label="Provide details", required=False) - exact_location_unknown_details = CharField(label="Provide details", required=False) + nhs_hospital_details = CharField( + label="Provide details", + required=False, + error_messages={ + "required": "Provide details about where the surgery and treatment took place" + }, + ) + private_clinic_uk_details = CharField( + label="Provide details", + required=False, + error_messages={ + "required": "Provide details about where the surgery and treatment took place" + }, + ) + outside_uk_details = CharField( + label="Provide details", + required=False, + error_messages={ + "required": "Provide details about where the surgery and treatment took place" + }, + ) + multiple_locations_details = CharField( + label="Provide details", + required=False, + error_messages={ + "required": "Provide details about where the surgery and treatment took place" + }, + ) + exact_location_unknown_details = CharField( + label="Provide details", + required=False, + error_messages={ + "required": "Provide details about where the surgery and treatment took place" + }, + ) additional_details = CharField( label="Additional details (optional)", From d5219d05f88e5267e0c6d766d843df16acf0ce0f Mon Sep 17 00:00:00 2001 From: Malcolm Baig Date: Mon, 24 Nov 2025 14:25:42 +0000 Subject: [PATCH 12/12] Benign lumps: update system spec --- .../tests/system/clinical/test_benign_lump_history.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py b/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py index 450018dd9..32e7afc40 100644 --- a/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py +++ b/manage_breast_screening/tests/system/clinical/test_benign_lump_history.py @@ -84,11 +84,13 @@ def then_i_see_validation_errors_for_missing_benign_lump_details(self): ) def when_i_select_procedures_for_each_breast(self): - right_fieldset = self.page.get_by_role("group", name="Right breast", exact=True) + right_fieldset = self.page.get_by_role( + "group", name="Right breast", exact=False + ) right_fieldset.get_by_label("Needle biopsy", exact=True).check() right_fieldset.get_by_label("Lump removed", exact=True).check() - left_fieldset = self.page.get_by_role("group", name="Left breast", exact=True) + left_fieldset = self.page.get_by_role("group", name="Left breast", exact=False) left_fieldset.get_by_label("Lump removed", exact=True).check() def and_i_enter_the_year_and_select_a_location(self): @@ -110,7 +112,7 @@ def and_i_click_save_benign_lump_history(self): def then_i_see_a_validation_error_for_missing_location_details(self): self.expect_validation_error( - error_text="This field is required.", + error_text="Provide details about where the surgery and treatment took place", fieldset_legend="Where were the tests and treatment done?", field_label="Provide details", field_name="nhs_hospital_details",