diff --git a/manage_breast_screening/mammograms/jinja2/mammograms/add_previous_mammogram.jinja b/manage_breast_screening/mammograms/jinja2/mammograms/add_previous_mammogram.jinja index d1dc41bed..4540ecbc4 100644 --- a/manage_breast_screening/mammograms/jinja2/mammograms/add_previous_mammogram.jinja +++ b/manage_breast_screening/mammograms/jinja2/mammograms/add_previous_mammogram.jinja @@ -19,4 +19,12 @@ "text": "Save" }) }} + + {% if delete_link %} +

+ + {{ delete_link.text }} + +

+ {% endif %} {% endblock %} diff --git a/manage_breast_screening/mammograms/presenters/last_known_mammogram_presenter.py b/manage_breast_screening/mammograms/presenters/last_known_mammogram_presenter.py index 0bb0b2094..f68747d24 100644 --- a/manage_breast_screening/mammograms/presenters/last_known_mammogram_presenter.py +++ b/manage_breast_screening/mammograms/presenters/last_known_mammogram_presenter.py @@ -17,12 +17,18 @@ def __init__(self, last_known_mammograms, appointment_pk, current_url): @cached_property def last_known_mammograms(self): result = [] - for mammogram in self._last_known_mammograms: - result.append(self._present_mammogram(mammogram)) + + if len(self._last_known_mammograms) == 1: + result.append(self._present_mammogram(self._last_known_mammograms[0], None)) + else: + for item_index, mammogram in enumerate( + self._last_known_mammograms, start=1 + ): + result.append(self._present_mammogram(mammogram, item_index)) return result - def _present_mammogram(self, mammogram): + def _present_mammogram(self, mammogram, item_index): location = ( mammogram.provider.name if mammogram.provider @@ -54,12 +60,30 @@ def _present_mammogram(self, mammogram): else: date = {"value": "Date unknown"} + href = ( + reverse( + "mammograms:change_previous_mammogram", + kwargs={ + "pk": self.appointment_pk, + "participant_reported_mammogram_pk": mammogram.pk, + }, + ) + + f"?return_url={self.current_url}" + ) + return { "date_added": format_relative_date(mammogram.created_at), "location": location, "date": date, "different_name": mammogram.different_name, "additional_information": mammogram.additional_information, + "change_link": { + "href": href, + "text": "Change", + "visually_hidden_text": ( + f" mammogram item {item_index}" if item_index else " mammogram item" + ), + }, } @cached_property diff --git a/manage_breast_screening/mammograms/tests/presenters/test_last_known_mammogram_presenter.py b/manage_breast_screening/mammograms/tests/presenters/test_last_known_mammogram_presenter.py index 80f193544..4634fb14e 100644 --- a/manage_breast_screening/mammograms/tests/presenters/test_last_known_mammogram_presenter.py +++ b/manage_breast_screening/mammograms/tests/presenters/test_last_known_mammogram_presenter.py @@ -43,9 +43,10 @@ def test_no_last_known_mammograms(self, reported_today): @time_machine.travel(datetime(2025, 1, 1, tzinfo=tz.utc)) def test_last_known_mammograms_single(self, reported_today): + appointment_pk = uuid4() result = LastKnownMammogramPresenter( [reported_today], - appointment_pk=uuid4(), + appointment_pk=appointment_pk, current_url="/mammograms/abc", ) @@ -60,14 +61,20 @@ def test_last_known_mammograms_single(self, reported_today): }, "different_name": "", "location": "In the UK: Somewhere", + "change_link": { + "href": f"/mammograms/{appointment_pk}/previous-mammograms/{reported_today.pk}?return_url=/mammograms/abc", + "text": "Change", + "visually_hidden_text": " mammogram item", + }, }, ] @time_machine.travel(datetime(2025, 1, 1, tzinfo=tz.utc)) def test_last_known_mammograms_multiple(self, reported_today, reported_earlier): + appointment_pk = uuid4() result = LastKnownMammogramPresenter( [reported_today, reported_earlier], - appointment_pk=uuid4(), + appointment_pk=appointment_pk, current_url="/mammograms/abc", ) @@ -82,6 +89,11 @@ def test_last_known_mammograms_multiple(self, reported_today, reported_earlier): }, "different_name": "", "location": "In the UK: Somewhere", + "change_link": { + "href": f"/mammograms/{appointment_pk}/previous-mammograms/{reported_today.pk}?return_url=/mammograms/abc", + "text": "Change", + "visually_hidden_text": " mammogram item 1", + }, }, { "date_added": "3 years ago", @@ -91,6 +103,11 @@ def test_last_known_mammograms_multiple(self, reported_today, reported_earlier): }, "different_name": "Janet Williams", "location": "West of London BSS", + "change_link": { + "href": f"/mammograms/{appointment_pk}/previous-mammograms/{reported_earlier.pk}?return_url=/mammograms/abc", + "text": "Change", + "visually_hidden_text": " mammogram item 2", + }, }, ] diff --git a/manage_breast_screening/mammograms/tests/views/conftest.py b/manage_breast_screening/mammograms/tests/views/conftest.py index 69941a63f..86f01b757 100644 --- a/manage_breast_screening/mammograms/tests/views/conftest.py +++ b/manage_breast_screening/mammograms/tests/views/conftest.py @@ -5,8 +5,10 @@ @pytest.fixture -def appointment(): - return AppointmentFactory.create() +def appointment(clinical_user_client): + return AppointmentFactory.create( + clinic_slot__clinic__setting__provider=clinical_user_client.current_provider + ) @pytest.fixture diff --git a/manage_breast_screening/mammograms/tests/views/test_participant_reported_mammogram_views.py b/manage_breast_screening/mammograms/tests/views/test_participant_reported_mammogram_views.py index d13afda44..20a7705af 100644 --- a/manage_breast_screening/mammograms/tests/views/test_participant_reported_mammogram_views.py +++ b/manage_breast_screening/mammograms/tests/views/test_participant_reported_mammogram_views.py @@ -15,6 +15,14 @@ ) +@pytest.fixture +def participant_reported_mammogram(appointment): + return ParticipantReportedMammogramFactory.create( + participant=appointment.participant, + location_type=ParticipantReportedMammogram.LocationType.NHS_BREAST_SCREENING_UNIT, + ) + + @pytest.mark.django_db class TestAddParticipantReportedMammogram: def test_renders_response(self, clinical_user_client): @@ -206,19 +214,6 @@ def test_post_exact_date_within_last_six_months( @pytest.mark.django_db class TestChangeParticipantReportedMammogram: - @pytest.fixture - def appointment(self, clinical_user_client): - return AppointmentFactory.create( - clinic_slot__clinic__setting__provider=clinical_user_client.current_provider - ) - - @pytest.fixture - def participant_reported_mammogram(self, appointment): - return ParticipantReportedMammogramFactory.create( - participant=appointment.participant, - location_type=ParticipantReportedMammogram.LocationType.NHS_BREAST_SCREENING_UNIT, - ) - def test_renders_response( self, clinical_user_client, appointment, participant_reported_mammogram ): @@ -423,3 +418,30 @@ def test_post_exact_date_within_last_six_months( assert ( appointment.current_status.name == AppointmentStatus.ATTENDED_NOT_SCREENED ) + + +@pytest.mark.django_db +class TestDeleteParticipantReportedMammogram: + def test_delete_previous_mammogram( + self, + clinical_user_client, + appointment, + participant_reported_mammogram, + ): + assert ParticipantReportedMammogram.objects.filter( + pk=participant_reported_mammogram.pk + ).exists() + + clinical_user_client.http.post( + reverse( + "mammograms:delete_previous_mammogram", + kwargs={ + "pk": appointment.pk, + "participant_reported_mammogram_pk": participant_reported_mammogram.pk, + }, + ) + ) + + assert not ParticipantReportedMammogram.objects.filter( + pk=participant_reported_mammogram.pk + ).exists() diff --git a/manage_breast_screening/mammograms/urls.py b/manage_breast_screening/mammograms/urls.py index 97e6ca72a..5698c6941 100644 --- a/manage_breast_screening/mammograms/urls.py +++ b/manage_breast_screening/mammograms/urls.py @@ -237,6 +237,11 @@ participant_reported_mammogram_views.UpdateParticipantReportedMammogramView.as_view(), name="change_previous_mammogram", ), + path( + "/previous-mammograms//delete/", + participant_reported_mammogram_views.DeleteParticipantReportedMammogramView.as_view(), + name="delete_previous_mammogram", + ), path( "/appointment-should-not-proceed/", mammogram_views.appointment_should_not_proceed, diff --git a/manage_breast_screening/mammograms/views/participant_reported_mammogram_views.py b/manage_breast_screening/mammograms/views/participant_reported_mammogram_views.py index 5fe4fdcf4..b73072fe4 100644 --- a/manage_breast_screening/mammograms/views/participant_reported_mammogram_views.py +++ b/manage_breast_screening/mammograms/views/participant_reported_mammogram_views.py @@ -6,6 +6,7 @@ from manage_breast_screening.core.views.generic import ( AddWithAuditView, + DeleteWithAuditView, UpdateWithAuditView, ) from manage_breast_screening.participants.forms import ParticipantReportedMammogramForm @@ -99,6 +100,9 @@ class UpdateParticipantReportedMammogramView( def update_title(self, thing_name): return f"Edit details of {thing_name}" + def confirm_delete_link_text(self, thing_name): + return "Delete this item" + def get_object(self): try: return ParticipantReportedMammogram.objects.get( @@ -110,3 +114,43 @@ def get_object(self): self.kwargs, ) return None + + def get_delete_url(self): + return ( + reverse( + "mammograms:delete_previous_mammogram", + kwargs={ + "pk": self.kwargs["pk"], + "participant_reported_mammogram_pk": self.kwargs[ + "participant_reported_mammogram_pk" + ], + }, + ) + + f"?return_url={self.request.GET.get('return_url', '')}" + ) + + +class DeleteParticipantReportedMammogramView( + InProgressAppointmentMixin, DeleteWithAuditView +): + def get_thing_name(self, object): + return "item" + + def get_success_message_content(self, object): + return "Deleted other procedure" + + def get_object(self): + provider = self.request.user.current_provider + appointment = provider.appointments.get(pk=self.kwargs["pk"]) + return appointment.participant.reported_mammograms.get( + pk=self.kwargs["participant_reported_mammogram_pk"] + ) + + def get_success_url(self) -> str: + return parse_return_url( + self.request, + default=reverse( + "mammograms:record_medical_information", + kwargs={"pk": self.appointment.pk}, + ), + ) diff --git a/manage_breast_screening/participants/jinja2/components/participant-details/summary_list_rows.jinja b/manage_breast_screening/participants/jinja2/components/participant-details/summary_list_rows.jinja index 5deef592b..7a897c72d 100644 --- a/manage_breast_screening/participants/jinja2/components/participant-details/summary_list_rows.jinja +++ b/manage_breast_screening/participants/jinja2/components/participant-details/summary_list_rows.jinja @@ -39,6 +39,10 @@ {% if mammograms_by_date_added %} {% for mammogram in mammograms_by_date_added %}

+ + {{ mammogram.change_link.text }}{{ mammogram.change_link.visually_hidden_text }} + + Added {{ mammogram.date_added }}
{% if mammogram.date.is_exact %}{{ mammogram.date.absolute }} ({{ mammogram.date.relative }}){% else %}{{ mammogram.date.value }}{% endif %}
{{ mammogram.location }} diff --git a/manage_breast_screening/participants/views.py b/manage_breast_screening/participants/views.py index b74f7ed1d..3d094bb1a 100644 --- a/manage_breast_screening/participants/views.py +++ b/manage_breast_screening/participants/views.py @@ -17,11 +17,7 @@ def parse_return_url(request, default: str) -> str: Parse the return_url from the request, with a fallback, and validating that the URL is not external to the service. """ - return_url = ( - request.POST.get("return_url") - if request.method == "POST" - else request.GET.get("return_url") - ) + return_url = request.POST.get("return_url") or request.GET.get("return_url") if not return_url or urlparse(return_url).netloc: return default diff --git a/manage_breast_screening/tests/system/clinical/test_adding_previous_mammograms.py b/manage_breast_screening/tests/system/clinical/test_adding_previous_mammograms.py index e75f4ea3e..389f77bba 100644 --- a/manage_breast_screening/tests/system/clinical/test_adding_previous_mammograms.py +++ b/manage_breast_screening/tests/system/clinical/test_adding_previous_mammograms.py @@ -16,7 +16,7 @@ class TestAddingPreviousMammograms(SystemTestCase): - def test_adding_a_mammogram_at_the_same_provider(self): + def test_adding_and_updating_a_mammogram_at_the_same_provider(self): """ If a mammogram was taken at the same provider, but there is an error in the system, the participant can report that it was taken. """ @@ -36,7 +36,18 @@ def test_adding_a_mammogram_at_the_same_provider(self): self.then_i_should_be_back_on_the_appointment() self.and_i_should_see_the_mammogram_with_the_same_provider() - def test_adding_a_mammogram_taken_elsewhere_with_a_different_name(self): + self.when_i_click_change() + self.then_i_should_be_on_the_edit_previous_mammogram_form() + + self.when_i_enter_another_location_in_the_uk() + self.and_i_enter_an_approximate_date() + self.and_i_enter_a_different_name() + self.and_i_enter_additional_information() + self.and_i_click_save() + self.then_i_should_be_back_on_the_appointment() + self.and_i_should_see_the_mammogram_with_the_other_provider_and_name() + + def test_adding_and_deleting_mammogram_taken_elsewhere_with_a_different_name(self): """ If a mammogram was taken at another BSU, or elsewhere in the UK, the participant can report that it was taken If the mammogram was taken under a different name, the mammographer can record that name. @@ -57,6 +68,12 @@ def test_adding_a_mammogram_taken_elsewhere_with_a_different_name(self): self.then_i_should_be_back_on_the_appointment() self.and_i_should_see_the_mammogram_with_the_other_provider_and_name() + self.when_i_click_change() + self.and_i_click_delete_this_item() + self.and_i_click_delete_item() + self.then_i_should_be_back_on_the_appointment() + self.and_the_previous_mammogram_is_gone() + def test_adding_a_mammogram_within_last_six_months(self): """ If has had a mammogram within the last six months, they should be shown a page advising them not to proceed. @@ -147,12 +164,6 @@ def then_i_should_be_on_the_add_previous_mammogram_form(self): expect(self.page).to_have_url(re.compile(path)) self.assert_page_title_contains("Add details of a previous mammogram") - def then_i_should_be_on_the_edit_previous_mammogram_form(self): - expect( - self.page.get_by_text("Edit details of a previous mammogram") - ).to_be_visible() - self.assert_page_title_contains("Edit details of a previous mammogram") - def then_i_should_be_back_on_the_appointment(self): path = reverse( "mammograms:participant_details", @@ -263,3 +274,28 @@ def then_the_appointment_is_attended_not_screened(self): expect( row.locator(".nhsuk-tag").filter(has_text="Attended not screened") ).to_be_visible() + + def when_i_click_change(self): + self.page.get_by_text("Change mammogram item").click() + + def then_i_should_be_on_the_edit_previous_mammogram_form(self): + expect( + self.page.get_by_text("Edit details of a previous mammogram") + ).to_be_visible() + self.assert_page_title_contains("Edit details of a previous mammogram") + + def and_i_click_delete_this_item(self): + self.page.get_by_text("Delete this item").click() + + def and_i_click_delete_item(self): + self.page.get_by_text("Delete item").click() + + def and_the_previous_mammogram_is_gone(self): + expect(self.page.get_by_test_id("mammograms")).to_contain_text( + "Not known", + use_inner_text=True, + ) + expect(self.page.get_by_test_id("mammograms")).not_to_contain_text( + "Added today", + use_inner_text=True, + )