Skip to content

Add "Significant symptoms reported" to session read page#1392

Open
swebberuk wants to merge 2 commits intoDTOSS-12818-highlight-symptomfrom
DTOSS-12827-significant-symptoms
Open

Add "Significant symptoms reported" to session read page#1392
swebberuk wants to merge 2 commits intoDTOSS-12818-highlight-symptomfrom
DTOSS-12827-significant-symptoms

Conversation

@swebberuk
Copy link
Copy Markdown
Contributor

@swebberuk swebberuk commented Apr 29, 2026

Description

Add "Significant symptoms reported" section to the session read page.

These changes build on changes made in #1386, which is awaiting approval.

image

Jira link

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

@swebberuk swebberuk changed the base branch from main to DTOSS-12818-highlight-symptom April 29, 2026 12:17
"caption": "Review images",
"images": images,
"presented_medical_information": CheckMedicalInformationPresenter(
"check_medical_information_presenter": CheckMedicalInformationPresenter(
Copy link
Copy Markdown
Contributor Author

@swebberuk swebberuk Apr 29, 2026

Choose a reason for hiding this comment

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

Template uses CheckMedicalInformationPresenter for "Medical summary" and MedicalInformationPresenter for "Significant symptoms reported". Renamed presented_medical_information to check_medical_information_presenter to make clearer.

@swebberuk swebberuk force-pushed the DTOSS-12827-significant-symptoms branch from 214fb41 to ff38833 Compare April 29, 2026 13:55
@swebberuk swebberuk changed the title [wip] Dtoss 12827 significant symptoms Add "Significant symptoms reported" to session read page Apr 29, 2026
@swebberuk swebberuk force-pushed the DTOSS-12827-significant-symptoms branch from ff38833 to f4f94c2 Compare April 29, 2026 14:38
def test_formats_symptoms_summary_list(self):
appointment = AppointmentFactory.create()

def _create_default_symptoms(self, appointment):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added this function so could share between tests of test_symptom_rows, test_read_only_symptom_rows and test_significant_symptom_rows.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If they don't depend on each other, you could make each one a fixture. Then pull them in as needed. It's a bit more concise that way.

@swebberuk swebberuk marked this pull request as ready for review April 29, 2026 14:46
Copy link
Copy Markdown
Contributor

@MatMoore MatMoore left a comment

Choose a reason for hiding this comment

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

LGTM, just a suggestion regarding the macros (not blocking)

{% macro symptoms_content(presented_medical_info, read_only=false) %}
{% set symptom_rows = presented_medical_info.read_only_symptom_rows if read_only else presented_medical_info.symptom_rows %}
<p {%- if not read_only and not symptom_rows %} class="nhsuk-u-margin-bottom-3"{% endif %}>Any problems or symptoms, including lumps, swelling, rashes or nipple changes</p>
{% macro symptoms_content(presented_medical_info, read_only=false, significant_only=false) %}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like we're crossing the streams here by reusing the macro from medical information. There's a lot of conditionals here that make it hard to read.

Could we create a separate macro instead?

Copy link
Copy Markdown
Contributor Author

@swebberuk swebberuk Apr 29, 2026

Choose a reason for hiding this comment

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

Good idea. Thanks.

I've reverted this change. For significant symptoms all this macro did was call summaryList - so now do directly from read_image.jinja instead.

def test_formats_symptoms_summary_list(self):
appointment = AppointmentFactory.create()

def _create_default_symptoms(self, appointment):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If they don't depend on each other, you could make each one a fixture. Then pull them in as needed. It's a bit more concise that way.

No longer use symptoms_content from read_image.jinja
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants