[wip] Allow symptoms to be highlighted to image readers by swebberuk · Pull Request #1385 · NHSDigital/dtos-manage-breast-screening

swebberuk · 2026-04-28T15:03:12Z

Description

Add option for breast pain and 'other' symptoms to be highlighted to readers.

Display info text for symptoms that are always highlighted to readers.

Add highlight_to_readers column to symptoms table.

Use 'Highlight to image readers' tag on medical info page.

Jira link

https://nhsd-jira.digital.nhs.uk/browse/DTOSS-12818

swebberuk · 2026-04-28T15:07:18Z


+    def _get_heading_description(self):
+        suffix = " a recognised symptom of breast cancer. Information recorded here will be highlighted during image reading."
+        if self.symptom_type_name == "lump":


Feels awkward that BaseSymptomFormView has logic about lumps.

Could override get_context_data for lumps, but then would have to duplicate in AddSymptomLumpView and UpdateSymptomLumpView.

Copilot

Pull request overview

This PR introduces a new highlight_to_readers flag on symptoms so that breast pain and “other” symptoms can optionally be highlighted to image readers, and updates the medical information UI/presenters to display a “Highlight to image readers” tag when appropriate.

Changes:

Added highlight_to_readers boolean field to participants.Symptom plus a schema migration and updated factories.
Added a “Highlight this symptom to readers?” choice field to Breast pain and Other symptom forms/templates, and updated view tests accordingly.
Updated symptom presenters (and related tests/system tests) to render a “Highlight to image readers” tag in summary lists.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
manage_breast_screening/participants/models/symptom.py	Adds `HighlightToReaderChoices` and `highlight_to_readers` model field.
manage_breast_screening/participants/migrations/0068_symptom_highlight_to_readers.py	Adds DB column for `highlight_to_readers`.
manage_breast_screening/participants/migrations/max_migration.txt	Bumps max migration pointer to 0068.
manage_breast_screening/participants/tests/factories.py	Updates `SymptomFactory` to set `highlight_to_readers`.
manage_breast_screening/mammograms/forms/symptom_forms.py	Adds `highlight_to_readers` ChoiceField and maps it into model create/update for Breast pain & Other.
manage_breast_screening/mammograms/jinja2/.../forms/breast_pain.jinja	Renders the new `highlight_to_readers` field.
manage_breast_screening/mammograms/jinja2/.../forms/other.jinja	Renders the new `highlight_to_readers` field.
manage_breast_screening/mammograms/views/symptom_views.py	Adds/centralises heading description text for symptom forms.
manage_breast_screening/mammograms/presenters/symptom_presenter.py	Appends “Highlight to image readers” tag to summary-list keys when flagged.
manage_breast_screening/mammograms/tests/forms/test_symptom_forms.py	Updates form tests to require/accept `highlight_to_readers` for Breast pain & Other.
manage_breast_screening/mammograms/tests/views/test_symptom_views.py	Updates view POST tests to include `highlight_to_readers` for Breast pain & Other.
manage_breast_screening/mammograms/tests/presenters/test_symptom_presenter.py	Updates presenter expectations for tagged key rendering.
manage_breast_screening/mammograms/tests/presenters/test_medical_information_presenter.py	Updates medical-information presenter expectations for tagged key rendering.
manage_breast_screening/tests/system/clinical/test_recording_symptoms.py	Adjusts Playwright locators to cope with key text changing due to the appended tag.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-28T15:15:39Z

    def and_the_lump_on_the_right_breast_is_listed(self):
        key = self.page.locator(
-            ".nhsuk-summary-list__key", has=self.page.get_by_text("Lump", exact=True)
+            ".nhsuk-summary-list__key", has=self.page.get_by_text("Lump")
        )


This file updates some summary-list lookups to avoid exact=True (needed now that a tag is appended), but and_the_lump_is_no_longer_listed() further down still searches for get_by_text("Lump", exact=True). With the new tag, that exact-text assertion is likely to become brittle/fail; consider updating that locator to be non-exact (or to target the key element more directly).

Copilot · 2026-04-28T15:15:40Z

                ],
            },
            "key": {
-                "text": "Lump",
+                "html": 'Lump<strong class="nhsuk-tag app-nowrap nhsuk-tag--yellow">Highlight to image readers</strong>',
            },


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 · 2026-04-28T15:15:40Z

@@ -84,7 +84,7 @@ def test_formats_symptoms_summary_list(self):
                    ],
                },
                "key": {
-                    "text": "Swelling or shape change",
+                    "html": 'Swelling or shape change<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",


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 · 2026-04-28T15:15:41Z

    recently_resolved = models.BooleanField(null=False, default=False)
    when_resolved = models.CharField(blank=True, null=False)

+    highlight_to_readers = models.BooleanField(null=False, default=True)


highlight_to_readers is being defaulted to True at the model level. Given this PR introduces an option specifically for breast pain/other symptoms, a global default of True risks marking symptoms as highlighted even when no explicit choice was made (e.g. existing rows after migration, or any code path that creates a Symptom without the form). Consider defaulting to False on the model and setting True explicitly only for symptom types that are always highlighted (or via the form logic for those types).

Suggested change

highlight_to_readers = models.BooleanField(null=False, default=True)

highlight_to_readers = models.BooleanField(null=False, default=False)

Copilot · 2026-04-28T15:15:41Z

+        migrations.AddField(
+            model_name='symptom',
+            name='highlight_to_readers',
+            field=models.BooleanField(default=True),


This migration will backfill all existing Symptom rows with highlight_to_readers=True (because of the default=True on AddField). That looks inconsistent with the requirement to add an option for breast pain/“other” symptoms, and could cause existing records to appear/behave as highlighted when they previously weren’t. Consider adding the field with a safe default (e.g. False) and a data migration to set True only for symptom types that should always be highlighted (leaving breast pain/other as False unless explicitly chosen).

Suggested change

field=models.BooleanField(default=True),

field=models.BooleanField(default=False),

Copilot · 2026-04-28T15:15:41Z

+    def _get_heading_description(self):
+        suffix = " a recognised symptom of breast cancer. Information recorded here will be highlighted during image reading."
+        if self.symptom_type_name == "lump":
+            return "Lumps are" + suffix
+        return f"{self.symptom_type_name.capitalize()} is" + suffix


heading_description currently always says the information "will be highlighted during image reading" for every symptom type (including Breast pain / Other where highlighting is optional per the PR description). This is likely misleading. Consider only showing this description for symptom types that are always highlighted, and using different copy (or none) for symptom types where the user can choose whether to highlight.

Copilot · 2026-04-28T15:15:42Z

+        if self._symptom.highlight_to_readers:
+            tag = '<strong class="nhsuk-tag app-nowrap nhsuk-tag--yellow">Highlight to image readers</strong>'
+            key = {"html": mark_safe(f"{escape(self._symptom.symptom_type.name)}{tag}")}
+        else:


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.

Add option for breast pain and 'other' symptoms to be highlighted to readers Display info text for symptoms that are always highlighted to readers Add highlight_to_readers column to participants_symptom Use 'Highlight to image readers' tag on medical info page

sonarqubecloud · 2026-04-28T16:04:48Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

swebberuk · 2026-04-28T16:12:11Z

will raise new version

swebberuk force-pushed the DTOSS-12818-highlight-symptom branch from b48c08a to bc0f9c4 Compare April 28, 2026 15:04

swebberuk commented Apr 28, 2026

View reviewed changes

swebberuk requested a review from Copilot April 28, 2026 15:07

Copilot started reviewing on behalf of swebberuk April 28, 2026 15:08 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

swebberuk force-pushed the DTOSS-12818-highlight-symptom branch from bc0f9c4 to ff01330 Compare April 28, 2026 16:03

swebberuk closed this Apr 28, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

[wip] Allow symptoms to be highlighted to image readers#1385
swebberuk wants to merge 1 commit intomainfrom
DTOSS-12818-highlight-symptom

swebberuk commented Apr 28, 2026

Uh oh!

swebberuk Apr 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

sonarqubecloud Bot commented Apr 28, 2026

Uh oh!

swebberuk commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	highlight_to_readers = models.BooleanField(null=False, default=True)
	highlight_to_readers = models.BooleanField(null=False, default=False)

	field=models.BooleanField(default=True),
	field=models.BooleanField(default=False),

Conversation

swebberuk commented Apr 28, 2026

Description

Jira link

Uh oh!

swebberuk Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 28, 2026

Quality Gate passed

Uh oh!

swebberuk commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants