Skip to content

[wip] Allow symptoms to be highlighted to image readers #1385

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.

Sign up for GitHub

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

Closed
swebberuk wants to merge 1 commit into from
Closed

[wip] Allow symptoms to be highlighted to image readers #1385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions manage_breast_screening/mammograms/forms/symptom_forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from manage_breast_screening.nhsuk_forms.forms import FormWithConditionalFields
from manage_breast_screening.nhsuk_forms.utils import YesNo, yes_no, yes_no_field
from manage_breast_screening.participants.models.symptom import (
HighlightToReaderChoices,
NippleChangeChoices,
RelativeDateChoices,
SkinChangeChoices,
Expand Down Expand Up @@ -80,6 +81,13 @@ class CommonFields:
widget=Textarea(attrs={"rows": 5}),
error_messages={"required": "Enter details of any investigations"},
)
highlight_to_readers = ChoiceField(
choices=HighlightToReaderChoices,
label="Highlight this symptom to readers?",
error_messages={
"required": "Select whether this symptom should be highlighted to image readers"
},
)
additional_information = CharField(
required=False,
label="Additional info (optional)",
Expand Down Expand Up @@ -151,6 +159,9 @@ def initial_values(self, instance):
"when_resolved": instance.when_resolved,
"investigated": yes_no(instance.investigated),
"investigation_details": instance.investigation_details,
"highlight_to_readers": HighlightToReaderChoices.YES
if instance.highlight_to_readers
else HighlightToReaderChoices.NO,
"additional_information": instance.additional_information,
}

Expand Down Expand Up @@ -184,6 +195,10 @@ def model_values(self):
intermittent = self.cleaned_data.get("intermittent", False)
recently_resolved = self.cleaned_data.get("recently_resolved", False)
when_resolved = self.cleaned_data.get("when_resolved", "")
highlight_to_readers = (
self.cleaned_data.get("highlight_to_readers", HighlightToReaderChoices.YES)
== HighlightToReaderChoices.YES
)
additional_information = self.cleaned_data.get("additional_information", "")

return dict(
Expand All @@ -199,6 +214,7 @@ def model_values(self):
intermittent=intermittent,
recently_resolved=recently_resolved,
when_resolved=when_resolved,
highlight_to_readers=highlight_to_readers,
additional_information=additional_information,
)

Expand Down Expand Up @@ -470,6 +486,7 @@ class OtherSymptomForm(SymptomForm):
when_resolved = CommonFields.when_resolved
investigated = CommonFields.investigated
investigation_details = CommonFields.investigation_details
highlight_to_readers = CommonFields.highlight_to_readers
additional_information = CommonFields.additional_information

def __init__(self, instance=None, **kwargs):
Expand Down Expand Up @@ -514,6 +531,7 @@ class BreastPainForm(SymptomForm):
when_resolved = CommonFields.when_resolved
investigated = CommonFields.investigated
investigation_details = CommonFields.investigation_details
highlight_to_readers = CommonFields.highlight_to_readers
additional_information = CommonFields.additional_information

def __init__(self, instance=None, **kwargs):
Expand Down
2 changes: 2 additions & 0 deletions ...reening/mammograms/jinja2/mammograms/medical_information/symptoms/forms/breast_pain.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@

{{ form.investigated.as_field_group() }}

{{ form.highlight_to_readers.as_field_group() }}

{{ form.additional_information.as_field_group() }}

<div class="nhsuk-button-group">
Expand Down
2 changes: 2 additions & 0 deletions ...ast_screening/mammograms/jinja2/mammograms/medical_information/symptoms/forms/other.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

{{ form.investigated.as_field_group() }}

{{ form.highlight_to_readers.as_field_group() }}

{{ form.additional_information.as_field_group() }}

<div class="nhsuk-button-group">
Expand Down
13 changes: 11 additions & 2 deletions manage_breast_screening/mammograms/presenters/symptom_presenter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from django.urls import reverse
from django.utils.html import escape
from django.utils.html import escape, format_html
from django.utils.safestring import mark_safe

from manage_breast_screening.core.template_helpers import (
Expand Down Expand Up @@ -139,8 +139,17 @@ def build_summary_list_row(self, include_actions=True):
]
)

if self._symptom.highlight_to_readers:
key = {
"html": format_html(
'{}<br><strong class="nhsuk-tag app-nowrap nhsuk-tag--yellow">Highlight to image readers</strong>',
self._symptom.symptom_type.name,
)
}
else:
Comment on lines +142 to +149
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The summary-list key HTML concatenates the symptom name directly with the tag with no separator. Unless CSS is adding spacing, this will render as a single joined string (e.g. LumpHighlight to image readers). Consider inserting whitespace and/or a margin utility class between the label and tag, and using format_html() instead of mark_safe() to avoid hand-building safe HTML.

Copilot uses AI. Check for mistakes.
key = {"text": self._symptom.symptom_type.name}
result = {
"key": {"text": self._symptom.symptom_type.name},
"key": key,
"value": {"html": html},
}

Expand Down
13 changes: 13 additions & 0 deletions manage_breast_screening/mammograms/tests/forms/test_symptom_forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
)
from manage_breast_screening.nhsuk_forms.choices import YesNo
from manage_breast_screening.participants.models.symptom import (
HighlightToReaderChoices,
NippleChangeChoices,
SkinChangeChoices,
SymptomAreas,
Expand Down Expand Up @@ -588,6 +589,7 @@ def test_valid_form(self):
"symptom_sub_type": SkinChangeChoices.COLOUR_CHANGE,
"when_started": RelativeDateChoices.LESS_THAN_THREE_MONTHS,
"investigated": YesNo.NO,
"highlight_to_readers": HighlightToReaderChoices.YES,
}
)
)
Expand All @@ -603,6 +605,9 @@ def test_missing_required_fields(self):
"symptom_sub_type_details": ["Enter a description of the symptom"],
"investigated": ["Select whether the symptom has been investigated or not"],
"area": ["Select the location of the symptom"],
"highlight_to_readers": [
"Select whether this symptom should be highlighted to image readers"
],
}

def test_missing_conditionally_required_fields(self):
Expand All @@ -614,6 +619,7 @@ def test_missing_conditionally_required_fields(self):
"symptom_sub_type_details": "abc symptom",
"when_started": RelativeDateChoices.SINCE_A_SPECIFIC_DATE,
"investigated": YesNo.YES,
"highlight_to_readers": HighlightToReaderChoices.YES,
}
)
)
Expand Down Expand Up @@ -641,6 +647,7 @@ def test_valid_form_with_conditionally_required_fields(self):
"specific_date_0": "2",
"specific_date_1": "2025",
"investigation_details": "def",
"highlight_to_readers": HighlightToReaderChoices.YES,
}
)
)
Expand All @@ -658,6 +665,7 @@ def test_valid_form(self):
"area_description_left_breast": "uoq",
"when_started": RelativeDateChoices.LESS_THAN_THREE_MONTHS,
"investigated": YesNo.NO,
"highlight_to_readers": HighlightToReaderChoices.YES,
}
)
)
Expand All @@ -672,6 +680,9 @@ def test_missing_required_fields(self):
"when_started": ["Select how long the symptom has existed"],
"investigated": ["Select whether the symptom has been investigated or not"],
"area": ["Select the location of the pain"],
"highlight_to_readers": [
"Select whether this symptom should be highlighted to image readers"
],
}

def test_missing_conditionally_required_fields(self):
Expand All @@ -683,6 +694,7 @@ def test_missing_conditionally_required_fields(self):
"when_started": RelativeDateChoices.SINCE_A_SPECIFIC_DATE,
"investigated": YesNo.YES,
"recently_resolved": True,
"highlight_to_readers": HighlightToReaderChoices.YES,
}
)
)
Expand Down Expand Up @@ -712,6 +724,7 @@ def test_valid_form_with_conditionally_required_fields(self):
"investigation_details": "def",
"recently_resolved": True,
"when_resolved": "3 months ago",
"highlight_to_readers": HighlightToReaderChoices.YES,
}
)
)
Expand Down
55 changes: 53 additions & 2 deletions manage_breast_screening/mammograms/tests/presenters/test_medical_information_presenter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ def test_formats_symptoms_summary_list(self):
area=SymptomAreas.BOTH_BREASTS,
)

symptom3 = SymptomFactory.create(
other=True,
appointment=appointment,
when_started=RelativeDateChoices.LESS_THAN_THREE_MONTHS,
area=SymptomAreas.RIGHT_BREAST,
symptom_sub_type_details="abc",
)

symptom4 = SymptomFactory.create(
other=True,
appointment=appointment,
when_started=RelativeDateChoices.LESS_THAN_THREE_MONTHS,
area=SymptomAreas.LEFT_BREAST,
highlight_to_readers=False,
symptom_sub_type_details="xyz",
)

presenter = MedicalInformationPresenter(appointment)

assert presenter.symptom_rows == [
Expand All @@ -66,12 +83,46 @@ def test_formats_symptoms_summary_list(self):
],
},
"key": {
"text": "Lump",
"html": 'Lump<br><strong class="nhsuk-tag app-nowrap nhsuk-tag--yellow">Highlight to image readers</strong>',
},
"value": {
"html": "Left breast<br>Not sure<br>Symptom is intermittent<br>Stopped: resolved date<br>Not investigated<br>Additional information: abc",
},
},
{
"actions": {
"items": [
{
"text": "Change",
"classes": "nhsuk-link--no-visited-state",
"visuallyHiddenText": "other",
"href": f"/mammograms/{appointment.id}/record-medical-information/other/{symptom3.id}/",
},
],
},
"key": {
"html": 'Other<br><strong class="nhsuk-tag app-nowrap nhsuk-tag--yellow">Highlight to image readers</strong>'
},
"value": {
"html": "Description: abc<br>Right breast<br>Less than 3 months ago<br>Not investigated"
},
},
{
"actions": {
"items": [
{
"text": "Change",
"classes": "nhsuk-link--no-visited-state",
"visuallyHiddenText": "other",
"href": f"/mammograms/{appointment.id}/record-medical-information/other/{symptom4.id}/",
}
]
},
"key": {"text": "Other"},
"value": {
"html": "Description: xyz<br>Left breast<br>Less than 3 months ago<br>Not investigated"
},
},
{
"actions": {
"items": [
Expand All @@ -84,7 +135,7 @@ def test_formats_symptoms_summary_list(self):
],
},
"key": {
"text": "Swelling or shape change",
"html": 'Swelling or shape change<br><strong class="nhsuk-tag app-nowrap nhsuk-tag--yellow">Highlight to image readers</strong>',
},
"value": {
"html": "Both breasts<br>Less than 3 months ago<br>Not investigated",
Comment on lines 83 to 141
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

MedicalInformationPresenter.symptom_rows assertions now expect the highlighted-tag HTML, but there isn’t any coverage for the case where a symptom has highlight_to_readers=False (where the key should remain plain text). Consider adding an additional symptom in this test (or a new test) with highlight_to_readers=False and asserting the tag is omitted.

Copilot uses AI. Check for mistakes.
Expand Down
39 changes: 37 additions & 2 deletions manage_breast_screening/mammograms/tests/presenters/test_symptom_presenter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def test_formats_intermittent_stopped_and_additional_information(self):
assert presenter.intermittent_line == "Symptom is intermittent"
assert presenter.additional_information_line == "Additional information: abc"

def test_formats_for_summary_list(self):
def test_formats_lump_for_summary_list(self):
symptom = SymptomFactory.create(
lump=True,
when_started=RelativeDateChoices.NOT_SURE,
Expand All @@ -159,13 +159,48 @@ def test_formats_for_summary_list(self):
],
},
"key": {
"text": "Lump",
"html": 'Lump<br><strong class="nhsuk-tag app-nowrap nhsuk-tag--yellow">Highlight to image readers</strong>',
},
Comment on lines 159 to 163
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

There’s now branching behaviour in SymptomPresenter.build_summary_list_row() for when highlight_to_readers is False (it should fall back to a plain-text key). The tests only cover the highlighted (True) path. Adding a test case for highlight_to_readers=False would prevent regressions in the non-highlighted rendering.

Copilot uses AI. Check for mistakes.
"value": {
"html": "Left breast<br>Not sure<br>Symptom is intermittent<br>Stopped: resolved date<br>Not investigated<br>Additional information: abc",
},
}

@pytest.mark.parametrize("highlight_to_readers", [True, False])
def test_formats_other_for_summary_list(self, highlight_to_readers):
symptom = SymptomFactory.create(
breast_pain=True,
when_started=RelativeDateChoices.LESS_THAN_THREE_MONTHS,
area=SymptomAreas.LEFT_BREAST,
highlight_to_readers=highlight_to_readers,
)

presenter = SymptomPresenter(symptom)

if highlight_to_readers:
expected_key = {
"html": 'Breast pain<br><strong class="nhsuk-tag app-nowrap nhsuk-tag--yellow">Highlight to image readers</strong>',
}
else:
expected_key = {"text": "Breast pain"}

assert presenter.summary_list_row == {
"actions": {
"items": [
{
"text": "Change",
"classes": "nhsuk-link--no-visited-state",
"visuallyHiddenText": "breast pain",
"href": f"/mammograms/{symptom.appointment_id}/record-medical-information/breast-pain/{symptom.id}/",
}
]
},
"key": expected_key,
"value": {
"html": "Left breast<br>Less than 3 months ago<br>Not investigated"
},
}

def test_delete_message_html(self):
lump = SymptomFactory.create(lump=True)
presenter = SymptomPresenter(lump)
Expand Down
5 changes: 5 additions & 0 deletions manage_breast_screening/mammograms/tests/views/test_symptom_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from manage_breast_screening.nhsuk_forms.choices import YesNo
from manage_breast_screening.participants.models.symptom import (
HighlightToReaderChoices,
NippleChangeChoices,
RelativeDateChoices,
SkinChangeChoices,
Expand Down Expand Up @@ -409,6 +410,7 @@ def test_valid_post_redirects_to_appointment(
"symptom_sub_type_details": "abc",
"when_started": RelativeDateChoices.LESS_THAN_THREE_MONTHS.value,
"investigated": YesNo.NO.value,
"highlight_to_readers": HighlightToReaderChoices.YES.value,
},
)
assertRedirects(
Expand Down Expand Up @@ -457,6 +459,7 @@ def test_valid_post_redirects_to_appointment(
"symptom_sub_type_details": "abc",
"when_started": RelativeDateChoices.LESS_THAN_THREE_MONTHS.value,
"investigated": YesNo.NO.value,
"highlight_to_readers": HighlightToReaderChoices.YES.value,
},
)
assertRedirects(
Expand Down Expand Up @@ -494,6 +497,7 @@ def test_valid_post_redirects_to_appointment(
"area_description_right_breast": "uiq",
"when_started": RelativeDateChoices.LESS_THAN_THREE_MONTHS.value,
"investigated": YesNo.NO.value,
"highlight_to_readers": HighlightToReaderChoices.YES.value,
},
)
assertRedirects(
Expand Down Expand Up @@ -542,6 +546,7 @@ def test_valid_post_redirects_to_appointment(
"area_description_right_breast": "uiq",
"when_started": RelativeDateChoices.LESS_THAN_THREE_MONTHS.value,
"investigated": YesNo.NO.value,
"highlight_to_readers": HighlightToReaderChoices.YES.value,
},
)
assertRedirects(
Expand Down
Loading
Loading