Skip to content

Commit 580f147

Browse files
committed
Fix start appointment visibility and state guard
The start appointment link is currently appearing for clinical users even if an appointment has already been started by another user. We also don't confirm appointment state before attempting to update it in the start_appointment view. - Template delegates start link visibility to `can_be_started_by(user)` on the presented appointment, instead of checking a permission directly. - View now guards against starting an already-in-progress appointment: if the state machine can't transition, it redirects to the in-progress tab with a warning naming who started it.
1 parent 5e6a9a1 commit 580f147

File tree

4 files changed

+61
-9
lines changed

4 files changed

+61
-9
lines changed

manage_breast_screening/core/jinja2/components/start-appointment/template.jinja

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{{ raise('start_appointment_url is required') }}
33
{% endif %}
44

5-
{% if user.has_perm('mammograms.do_mammogram_appointment') %}
5+
{% if presented_appointment.can_be_started_by(user) %}
66
<div data-module="app-start-appointment" data-appointment-id="{{ presented_appointment.pk }}" {% if presented_appointment.can_be_checked_in %}hidden{% endif %}>
77
<form action="{{ start_appointment_url }}" method="post" novalidate>
88
<p>

manage_breast_screening/core/tests/jinja2/test_start_appointment.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ def presented_appointment():
1717
return mock
1818

1919

20-
def render(template, user_has_perm, presented_appointment):
20+
def render(template, can_be_started_by, presented_appointment):
2121
user = MagicMock()
22-
user.has_perm.return_value = user_has_perm
22+
presented_appointment.can_be_started_by.return_value = can_be_started_by
2323
return template.render(
2424
{
2525
"user": user,
@@ -30,24 +30,28 @@ def render(template, user_has_perm, presented_appointment):
3030
)
3131

3232

33-
def test_renders_start_button_for_clinical_user(template, presented_appointment):
33+
def test_renders_start_button_when_appointment_can_be_started(
34+
template, presented_appointment
35+
):
3436
html = render(
35-
template, user_has_perm=True, presented_appointment=presented_appointment
37+
template, can_be_started_by=True, presented_appointment=presented_appointment
3638
)
3739
assert 'data-module="app-start-appointment"' in html
3840

3941

40-
def test_does_not_render_start_button_for_admin_user(template, presented_appointment):
42+
def test_does_not_render_start_button_when_appointment_cannot_be_started(
43+
template, presented_appointment
44+
):
4145
html = render(
42-
template, user_has_perm=False, presented_appointment=presented_appointment
46+
template, can_be_started_by=False, presented_appointment=presented_appointment
4347
)
4448
assert 'data-module="app-start-appointment"' not in html
4549

4650

4751
def test_renders_button_hidden_before_checkin(template, presented_appointment):
4852
presented_appointment.can_be_checked_in = True
4953
html = render(
50-
template, user_has_perm=True, presented_appointment=presented_appointment
54+
template, can_be_started_by=True, presented_appointment=presented_appointment
5155
)
5256
assert 'data-module="app-start-appointment"' in html
5357
assert 'data-appointment-id="abc-123" hidden' in html
@@ -56,7 +60,7 @@ def test_renders_button_hidden_before_checkin(template, presented_appointment):
5660
def test_renders_button_visible_after_checkin(template, presented_appointment):
5761
presented_appointment.can_be_checked_in = False
5862
html = render(
59-
template, user_has_perm=True, presented_appointment=presented_appointment
63+
template, can_be_started_by=True, presented_appointment=presented_appointment
6064
)
6165
assert 'data-module="app-start-appointment"' in html
6266
assert 'data-appointment-id="abc-123" hidden' not in html

manage_breast_screening/mammograms/tests/views/test_appointment_workflow_views.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
import django
44
import pytest
55
import statemachine
6+
from django.contrib import messages
67
from django.urls import reverse
78
from pytest_django.asserts import (
89
assertContains,
910
assertInHTML,
11+
assertMessages,
1012
assertNotContains,
1113
assertQuerySetEqual,
1214
assertRedirects,
@@ -534,6 +536,38 @@ def test_user_not_permitted(self, administrative_user_client):
534536
response = administrative_user_client.http.post(url)
535537
assert response.status_code == 403
536538

539+
def test_redirects_to_in_progress_tab_with_warning_when_already_started(
540+
self, clinical_user_client
541+
):
542+
other_user = UserFactory.create(first_name="Alice", last_name="Smith")
543+
appointment = AppointmentFactory.create(
544+
clinic_slot__clinic__setting__provider=clinical_user_client.current_provider,
545+
current_status_params={
546+
"name": AppointmentStatusNames.IN_PROGRESS,
547+
"created_by": other_user,
548+
},
549+
)
550+
response = clinical_user_client.http.post(
551+
reverse("mammograms:start_appointment", kwargs={"pk": appointment.pk})
552+
)
553+
assertRedirects(
554+
response,
555+
reverse(
556+
"clinics:list_clinic_appointments_in_progress",
557+
kwargs={"pk": appointment.clinic_slot.clinic.pk},
558+
),
559+
)
560+
patient_name = appointment.participant.full_name
561+
assertMessages(
562+
response,
563+
[
564+
messages.Message(
565+
level=messages.WARNING,
566+
message=f"Appointment for {patient_name} has already been started by {other_user.get_short_name()}.",
567+
)
568+
],
569+
)
570+
537571

538572
@pytest.mark.django_db
539573
class TestResumeAppointment:

manage_breast_screening/mammograms/views/appointment_workflow_views.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
ParticipantReportedMammogram,
4444
)
4545
from manage_breast_screening.participants.models.appointment import (
46+
AppointmentMachine,
4647
AppointmentWorkflowStepCompletion,
4748
)
4849
from manage_breast_screening.participants.presenters import ParticipantPresenter
@@ -309,6 +310,19 @@ def start_appointment(request, pk):
309310
except Appointment.DoesNotExist:
310311
raise Http404("Appointment not found")
311312

313+
if not AppointmentMachine.from_appointment(appointment).can("start"):
314+
patient_name = appointment.participant.full_name
315+
started_by = appointment.current_status.created_by.get_short_name()
316+
messages.add_message(
317+
request,
318+
messages.WARNING,
319+
f"Appointment for {patient_name} has already been started by {started_by}.",
320+
)
321+
return redirect(
322+
"clinics:list_clinic_appointments_in_progress",
323+
pk=appointment.clinic_slot.clinic.pk,
324+
)
325+
312326
AppointmentStatusUpdater(appointment=appointment, current_user=request.user).start()
313327

314328
return redirect("mammograms:confirm_identity", pk=pk)

0 commit comments

Comments
 (0)