Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 44 additions & 0 deletions manage_breast_screening/assets/sass/main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
@forward "components/secondary-navigation";
@forward "components/special-appointment-banner";

$color_app-dark-blue: #00386e;

h1 {
@extend %nhsuk-heading-l;
}
Expand Down Expand Up @@ -80,3 +82,45 @@ a[href="#"] {
}
}
}

.app-status-bar {
background-color: $color_app-dark-blue;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you check the scss from the prototype, this was background-color: nhsuk-shade($nhsuk-brand-colour, 40%); which is probably clearer for how we chose this number / where it came from.

color: $color_nhsuk-white;
padding: nhsuk-spacing(2) 0;
border-bottom: 1px solid nhsuk-colour("grey-4");
}

.app-status-bar a {
color: $color_nhsuk-white;
}
.app-status-bar a:hover,
.app-status-bar a:active {
color: $color_nhsuk-white;
}
.app-status-bar a:focus {
color: $color_nhsuk-black;
}

.app-status-bar__row {
display: flex;
flex-wrap: wrap;
gap: nhsuk-spacing(4);
align-items: center;
}

.app-status-bar__row + .app-status-bar__row {
margin-top: nhsuk-spacing(2);
padding-top: nhsuk-spacing(2);
border-top: 1px solid rgba($color_nhsuk-white, 0.2);
}

.app-status-bar__item {
display: flex;
gap: nhsuk-spacing(2);
align-items: center;
}

.app-status-bar__key {
@include nhsuk-typography-weight-bold(true);
opacity: 0.9;
}
Comment on lines +86 to +126
Copy link
Contributor

Choose a reason for hiding this comment

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

I imagine it might be good to put these in their own named file inside a folder like _components?

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{% from 'nhsuk/components/tag/macro.jinja' import tag %}

{% macro appointment_status_bar(user, presented_status_bar) %}
{% if presented_status_bar and presented_status_bar.show_status_bar_for(user) %}
<div class="app-status-bar app-status-bar--rows-2">
<div class="nhsuk-width-container">
<div class="app-status-bar__row">
<div class="app-status-bar__item">
<span class="app-status-bar__key">Appointment:</span>
<span>
{{ presented_status_bar.clinic_slot.clinic_date_and_slot_time }}
{% if presented_status_bar.appointment.is_special_appointment %}
{{ tag(presented_status_bar.tag_properties) }}
{% endif %}
</span>
Comment on lines +9 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

I somewhat wonder if these and other items within the summary list should use a definition list. @colinrotherham any thoughts?

</div>
</div>
<div class="app-status-bar__row">
<div class="app-status-bar__item">
<strong>{{ presented_status_bar.participant.full_name }}</strong>
</div>
<div class="app-status-bar__item">
<span class="app-status-bar__key" aria-label="Date of birth">DOB:</span>
<span>{{ presented_status_bar.participant.date_of_birth }} ({{ presented_status_bar.participant.age }})</span>
</div>
<div class="app-status-bar__item">
<span class="app-status-bar__key">NHS:</span>
<span>{{ presented_status_bar.participant.nhs_number }}</span>
</div>
</div>
</div>
</div>
{% endif %}
{% endmacro %}
7 changes: 6 additions & 1 deletion manage_breast_screening/core/jinja2/layout-app.jinja
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{% from 'nhsuk/components/header/macro.jinja' import header %}
{% from "notification-banner/macro.jinja" import notificationBanner %}
{% from 'components/appointment-status-bar.jinja' import appointment_status_bar %}

{% set assetPath = STATIC_URL ~ "/assets" %}
{% extends "nhsuk/template.jinja" %}
Expand Down Expand Up @@ -33,6 +34,10 @@
]
}
}) }}

{{
Copy link
Contributor

Choose a reason for hiding this comment

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

Will having this in the header mean it's skipped by the skip link? This doesn't seem right to me for something that we want users to perceive for clinical safety reasons.

Copy link
Contributor Author

@swebberuk swebberuk Dec 1, 2025

Choose a reason for hiding this comment

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

Have asked about skip link in screening-manage-ucd Slack channel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

appointment_status_bar(request.user, presented_appointment.status_bar)
}}
{% endblock %}

{% block content %}
Expand Down Expand Up @@ -71,4 +76,4 @@

{% block bodyEnd %}
<script type="module" src="{{ static('js/bundle.js' )}}"></script>
{% endblock %}}
{% endblock %}
4 changes: 2 additions & 2 deletions manage_breast_screening/core/utils/string_formatting.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ def format_age(value: int) -> str:
Format an age in years as a string

>>> format_age(64)
'64 years old'
'64 years'
"""
return f"{value} years old"
return f"{value} years"


def format_phone_number(value: str) -> str:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,32 @@ def note(self):
except AppointmentNote.DoesNotExist:
return None

@cached_property
def status_bar(self):
return StatusBarPresenter(self)


class StatusBarPresenter:
def __init__(self, appointment):
self.appointment = appointment
self.clinic_slot = appointment.clinic_slot
self.participant = appointment.participant

def show_status_bar_for(self, user):
# The appointment status bar should only display if the current user is the one that has the appointment 'in progress'
current_status = self.appointment._appointment.current_status
return (
current_status.state == AppointmentStatus.IN_PROGRESS
and user.nhs_uid == current_status.created_by.nhs_uid
)

@property
def tag_properties(self):
return {
"classes": "nhsuk-tag--yellow nhsuk-u-margin-left-1",
"text": "Special appointment",
}


class ClinicSlotPresenter:
def __init__(self, clinic_slot):
Expand All @@ -143,6 +169,15 @@ def slot_time_and_clinic_date(self):

return f"{format_time(clinic_slot.starts_at)} ({clinic_slot.duration_in_minutes} minutes) - {format_date(clinic.starts_at)} ({format_relative_date(clinic.starts_at)})"

@cached_property
def clinic_date_and_slot_time(self):
clinic_slot = self._clinic_slot
clinic = self._clinic

return (
f"{format_date(clinic.starts_at)} at {format_time(clinic_slot.starts_at)}"
)

@cached_property
def starts_at(self):
return format_time(self._clinic_slot.starts_at)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,68 @@ def test_status_attribution(
assert presenter.status_attribution == expected_result


class TestStatusBarPresenter:
@pytest.fixture
def mock_appointment(self):
mock = MagicMock(spec=Appointment)
mock.screening_episode.participant.nhs_number = "99900900829"
mock.screening_episode.participant.pk = uuid4()
return mock

@pytest.fixture
def mock_user(self):
return MagicMock(spec=User)

def test_show_status_bar_when_in_progress_and_user_is_owner(
self, mock_appointment, mock_user
):
mock_appointment.current_status.state = AppointmentStatus.IN_PROGRESS
mock_user.nhs_uid = "user-123"
mock_appointment.current_status.created_by.nhs_uid = "user-123"
presenter = AppointmentPresenter(mock_appointment)
assert presenter.status_bar.show_status_bar_for(mock_user)

def test_show_status_bar_when_user_is_not_owner(self, mock_appointment, mock_user):
mock_appointment.current_status.state = AppointmentStatus.IN_PROGRESS
mock_user.nhs_uid = "user-123"
mock_appointment.current_status.created_by.nhs_uid = "user-456"
presenter = AppointmentPresenter(mock_appointment)
assert not presenter.status_bar.show_status_bar_for(mock_user)

@pytest.mark.parametrize(
"current_state",
[
AppointmentStatus.CONFIRMED,
AppointmentStatus.CHECKED_IN,
AppointmentStatus.DID_NOT_ATTEND,
AppointmentStatus.SCREENED,
AppointmentStatus.PARTIALLY_SCREENED,
AppointmentStatus.ATTENDED_NOT_SCREENED,
],
)
def test_dont_show_status_bar_when_not_in_progress(
self, mock_appointment, mock_user, current_state
):
mock_appointment.current_status.state = current_state
mock_user.nhs_uid = "user-123"
mock_appointment.current_status.created_by.nhs_uid = "user-123"
presenter = AppointmentPresenter(mock_appointment)
assert not presenter.status_bar.show_status_bar_for(mock_user)

def test_tag_properties(self, mock_appointment):
presenter = AppointmentPresenter(mock_appointment)
assert presenter.status_bar.tag_properties == {
"classes": "nhsuk-tag--yellow nhsuk-u-margin-left-1",
"text": "Special appointment",
}

def test_attributes(self, mock_appointment):
presenter = AppointmentPresenter(mock_appointment)
assert presenter.status_bar.appointment == presenter
assert presenter.status_bar.clinic_slot == presenter.clinic_slot
assert presenter.status_bar.participant == presenter.participant


class TestClinicSlotPresenter:
@pytest.fixture
def clinic_slot_mock(self):
Expand Down Expand Up @@ -229,6 +291,17 @@ def test_slot_time_and_clinic_date(self, clinic_slot_mock):
== "9:30am (30 minutes) - 2 January 2025 (4 months, 17 days ago)"
)

@time_machine.travel(datetime(2025, 5, 19, tzinfo=tz.utc))
def test_clinic_date_and_slot_time(self, clinic_slot_mock):
clinic_slot_mock.starts_at = datetime(2025, 1, 2, 9, 30)
clinic_slot_mock.duration_in_minutes = 30
clinic_slot_mock.clinic.starts_at = date(2025, 1, 2)

assert (
ClinicSlotPresenter(clinic_slot_mock).clinic_date_and_slot_time
== "2 January 2025 at 9:30am"
)


class TestSpecialAppointmentPresenter:
def test_change_url(self):
Expand Down
22 changes: 14 additions & 8 deletions manage_breast_screening/mammograms/views/appointment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,22 @@ class ConfirmIdentity(InProgressAppointmentMixin, TemplateView):
template_name = "mammograms/confirm_identity.jinja"

def get_context_data(self, pk, **kwargs):
context = super().get_context_data()

participant = self.appointment.participant

return {
"heading": "Confirm identity",
"page_title": "Confirm identity",
"presented_participant": ParticipantPresenter(participant),
"appointment_cannot_proceed_href": reverse(
"mammograms:appointment_cannot_go_ahead", kwargs={"pk": pk}
),
}
context.update(
{
"heading": "Confirm identity",
"page_title": "Confirm identity",
"presented_participant": ParticipantPresenter(participant),
"appointment_cannot_proceed_href": reverse(
"mammograms:appointment_cannot_go_ahead", kwargs={"pk": pk}
),
},
)

return context

def post(self, request, pk):
return redirect("mammograms:ask_for_medical_information", pk=pk)
Expand Down
14 changes: 14 additions & 0 deletions manage_breast_screening/mammograms/views/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
from rules.contrib.views import PermissionRequiredMixin

from manage_breast_screening.auth.models import Permission
from manage_breast_screening.mammograms.presenters.appointment_presenters import (
AppointmentPresenter,
)
from manage_breast_screening.participants.models import Appointment


Expand Down Expand Up @@ -33,6 +36,17 @@ def appointment(self):
def participant(self):
return self.appointment.participant

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)

context.update(
{
"presented_appointment": AppointmentPresenter(self.appointment),
},
)

return context


class InProgressAppointmentMixin(PermissionRequiredMixin, AppointmentMixin):
"""
Expand Down
8 changes: 8 additions & 0 deletions manage_breast_screening/participants/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,14 @@ def current_status(obj, create, extracted, **kwargs):
AppointmentStatusFactory.create(state=extracted, appointment=obj)
)

# Allow passing an explicit status and created_by user
# e.g. `current_status_params={'state': AppointmentStatus.IN_PROGRESS, 'created_by': self.current_user}`
@post_generation
def current_status_params(obj, create, extracted, **kwargs):
if not create or not extracted:
return
obj.statuses.add(AppointmentStatusFactory.create(appointment=obj, **extracted))


class AppointmentNoteFactory(DjangoModelFactory):
class Meta:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def test_presented_values(self, participant):
assert result.phone == "07700 900000"
assert result.nhs_number == "999 009 00829"
assert result.date_of_birth == "1 January 1955"
assert result.age == "70 years old"
assert result.age == "70 years"
assert result.risk_level == ""
assert result.url == f"/participants/{participant.pk}/"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from django.urls import reverse
from playwright.sync_api import expect

from manage_breast_screening.core.utils.string_formatting import format_nhs_number
from manage_breast_screening.participants.models import AppointmentStatus
from manage_breast_screening.participants.models.medical_history.benign_lump_history_item import (
BenignLumpHistoryItem,
)
Expand All @@ -21,6 +23,7 @@ def test_adding_a_benign_lump_history_item(self):
self.and_i_am_on_the_record_medical_information_page()
self.when_i_click_on_benign_lumps()
self.then_i_see_the_add_benign_lump_history_form()
self.and_i_see_the_appointment_status_bar()
self.when_i_try_to_save_without_entering_benign_lump_details()
self.then_i_see_validation_errors_for_missing_benign_lump_details()

Expand Down Expand Up @@ -63,6 +66,10 @@ def and_there_is_an_appointment(self):
self.appointment = AppointmentFactory(
screening_episode=self.screening_episode,
clinic_slot__clinic__setting__provider=self.current_provider,
current_status_params={
"state": AppointmentStatus.IN_PROGRESS,
"created_by": self.current_user,
},
)

def and_i_am_on_the_record_medical_information_page(self):
Expand All @@ -74,6 +81,13 @@ def and_i_am_on_the_record_medical_information_page(self):
)
)

def and_i_see_the_appointment_status_bar(self):
status_bar = self.page.locator("div.app-status-bar")
expect(status_bar).to_contain_text(
format_nhs_number(self.participant.nhs_number)
)
expect(status_bar).to_contain_text(self.participant.full_name)

def when_i_click_on_benign_lumps(self):
self.page.get_by_role("button").filter(has_text="Benign lumps").click()

Expand Down
Loading
Loading