Skip to content

Add new appointment statuses for mammogram workflow #864

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

Jump to bottom
Merged
malcolmbaig merged 7 commits into from
Jan 7, 2026
Merged

Add new appointment statuses for mammogram workflow #864

malcolmbaig merged 7 commits into from
Jan 7, 2026

Conversation

@malcolmbaig
Copy link
Contributor

@malcolmbaig malcolmbaig commented Dec 31, 2025 •
edited
Loading

Description

  • Rename AppointmentStatus#state to AppointmentStatus#name, so we can avoid code that reads status.state in favour of status.name.
  • Add several new statuses to support upcoming mammogram workflow changes
  • Treat all of these statuses as 'In progress' when displaying the status tag in the UI
Screenshot 2025-12-31 at 11 56 13

Jira link

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

Review notes

  • The Jira ticket suggests changing the model name to AppointmentEvent. I've elected not to rename the model on the basis that:
    • It's a fairly widespread change with a knock-on effect of requiring dependent function names to change as well (current_status -> most_recent_event).
    • The 'event' nomenclature isn't clearer or more readable, and arguably slightly obfuscates what we're trying to model here.
    • If anything, there might be a case to rename AppointmentStatus to AppointmentState as some point, so we can consistently use the language of 'states' in our codebase.
  • The clinic show page filters currently have a SQL bug — appointments will be displayed if a given status appears anywhere in the status history, rather than matching on the most recent status. I've started work on a fix but will raise that in a separate PR.

Review checklist

  • Check database queries are correctly scoped to current_provider

@malcolmbaig malcolmbaig requested a review from a team as a code owner December 31, 2025 11:58
colinrotherham
colinrotherham reviewed Dec 31, 2025
View reviewed changes
manage_breast_screening/participants/models/appointment.py Outdated
Comment on lines 211 to 216
return self.name in [
self.STARTED,
self.IDENTITY_CONFIRMED,
self.MEDICAL_INFORMATION_REVIEWED,
self.IMAGES_TAKEN,
]
Copy link
Contributor

@colinrotherham colinrotherham Dec 31, 2025

Choose a reason for hiding this comment

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

Is there a better way to do this, should we add new statuses and forget to update this?

E.g. If the index is >= the STARTED status, but not cancelled etc?

Copy link
Contributor Author

@malcolmbaig malcolmbaig Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I've refactored this by:

  • Specifying the YET_TO_BEGIN, IN_PROGRESS, and FINAL status groups in the AppointmentStatus model.
  • Using these groups in other functions, where applicable.

The specification of the groups is still pretty manual but this should be less error prone on the whole.

Copy link
Contributor

@colinrotherham colinrotherham Jan 5, 2026

Choose a reason for hiding this comment

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

Is the YET_TO_* status the only way?

As in, since the others don't need YET_TO_* counterparts:

  • YET_TO_HAVE_IDENTITY_CONFIRMED
  • YET_TO_HAVE_IMAGES_TAKEN

Have we accidentally picked a series of statuses that don't quite fit?

Copy link
Contributor

@colinrotherham colinrotherham Jan 5, 2026

Choose a reason for hiding this comment

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

Work in 385011e is a nice way around it though

Copy link
Contributor Author

@malcolmbaig malcolmbaig Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The name could probably do with some refinement, but YET_TO_BEGIN_STATUSES is the group name, under which sit CONFIRMED and CHECKED_IN. The idea is that this group contains any status prior to STARTED.

@steventux steventux mentioned this pull request Jan 5, 2026
Merged
1 task
@malcolmbaig malcolmbaig requested review from MatMoore and colinrotherham and removed request for colinrotherham January 5, 2026 13:18
@malcolmbaig
Rename AppointmentStatus state field to name
39468a3 
Using variations of 'status.state' in the codebase isn't as clear to
read as it could be. In the first instance, rename the state field to
name.
@malcolmbaig
Reword AppointmentStatus#state to AppointmentStatus#name in codebase
913ef1e 
- Update all references to this field
- Avoid slipping into usage of 'state' when we're referring to instances
  of the AppointmentStatus model
@malcolmbaig
Rename IN_PROGRESS appointment status to STARTED
074da26 
We're going to add some additional statuses, which will be represented
by the 'In progress' tag on the frontend. To begin with, simply rename
the existing IN_PROGRESS status to STARTED.
@malcolmbaig
Add several new statuses for Appointments
08aaa10 
To support upcoming workflow changes for appointments, add the following
statuses:

IDENTITY_CONFIRMED
MEDICAL_INFORMATION_REVIEWED
IMAGES_TAKEN

We also amend presenter behaviour to represent these statuses (and
STARTED) with an 'In progress' tag on the frontend.
@malcolmbaig
Update seed demo data with new Appointment statuses
105e81e
@malcolmbaig
Rework show_status_bar logic and tests
0c211cd 
We only show the status bar for an appointment if the current user is
the one who has the appointment in progress.

Update the StatusBarPresenter to reflect this and amend the tests
accordingly.
@malcolmbaig
Refactor appointment statuses
636fefa 
- Specify groups of statuses in the AppointmentStatus model
- Use these groups in various functions instead of explicit lists of
  statuses.
@malcolmbaig malcolmbaig force-pushed the 11594-mammogram-workflow-states branch from ae8a645 to 636fefa Compare January 5, 2026 14:25
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 5, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

@malcolmbaig malcolmbaig requested review from gpeng and swebberuk January 6, 2026 14:44
swebberuk
swebberuk approved these changes Jan 7, 2026
View reviewed changes
Copy link
Contributor

@swebberuk swebberuk left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me.

@malcolmbaig malcolmbaig merged commit e52c99e into main Jan 7, 2026
14 checks passed
@malcolmbaig malcolmbaig deleted the 11594-mammogram-workflow-states branch January 7, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colinrotherham colinrotherham colinrotherham left review comments

@swebberuk swebberuk swebberuk approved these changes

@MatMoore MatMoore Awaiting requested review from MatMoore

@gpeng gpeng Awaiting requested review from gpeng

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@malcolmbaig @colinrotherham @swebberuk