Do not allow vaccination of absent children #3743
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
from
a33d38a to
2269452
Compare
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
from
2269452 to
cd02163
Compare
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
from
cd02163 to
631deac
Compare
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
from
631deac to
e114b7b
Compare
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
from
e114b7b to
45d62dd
Compare
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
from
45d62dd to
67f0b83
Compare
|
| joins(:patient_session).where(patient_session: { patient:, session: }) | ||
| end | ||
|
|
||
| scope :allows_recording_vaccination, -> { attending.or(completed) } |
There was a problem hiding this comment.
This is the same logic as:
I wonder if there's a way we can refactor this so the same logic is used in both places, perhaps something like this:
STATUSES_FOR_RECORDING_VACCINATION = %w[attending completed]
scope :allows_recording_vaccination, -> { where(status: STATUSES_FOR_RECORDING_VACCINATION) }
.has_registration_status(STATUSES_FOR_RECORDING_VACCINATION)There was a problem hiding this comment.
I did this refactor. There is a third place that used .has_registration_status(%w[attending completed]), namely the record controller, but in this case the purpose is to check that they are attending, not to check they can receive a vaccination. I have chosen for now to not replace that instance.
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
from
67f0b83 to
6bbe91e
Compare
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
8 times, most recently
from
317d83b to
380eb8b
Compare
In the next commit we will enforce that a child is registered as attending. We therefore fix the tests so that the children are attending when this is appropriate and required.
Ideally we would get the error message to be a direct link to the child on the registration page, see x-govuk/govuk-form-builder#566 for why this is not yet possible. I could not find a way to test this feature with an e2e test as our testing library uses a mocked browser that does not actually allow you to open a second tab in which to modify the attendance while submitting the vaccination.
MartinVanIJcken
force-pushed
the
do-not-allow-vaccination-of-absent-children
branch
from
380eb8b to
775c80e
Compare
|
| def attending?(session:) | ||
| patient_sessions | ||
| .includes(:registration_status) | ||
| .find_by(session:) | ||
| &.registration_status | ||
| &.attending? | ||
| end | ||
|
|
||
| def completed?(session:) | ||
| patient_sessions | ||
| .includes(:registration_status) | ||
| .find_by(session:) | ||
| &.registration_status | ||
| &.completed? | ||
| end | ||
|
|
There was a problem hiding this comment.
I can't see where these methods are used.
| .exists? | ||
| date = performed_at.to_date | ||
| errors.add( | ||
| :session_id, |
There was a problem hiding this comment.
I'm wondering if it makes sense for this error to be attached to :session_id, I wonder if :base is more appropriate (since it's more like a general validation error for the model rather than a specific attribute).
| patient_full_name: patient.full_name, | ||
| display_date: | ||
| date.today? ? "Today (#{date.to_fs(:long)})" : date.to_fs(:long), | ||
| location_name: |
There was a problem hiding this comment.
location_name is an optional value that's only set when recording a vaccination in a clinic session. I think you'll want to use the logic from the vaccination_record_location helper.
This PR provides a fix for https://nhsd-jira.digital.nhs.uk/browse/MAV-1212