Disallow offline spreadsheets from editing records received from the Imms API #4650
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alistair-white-horne-tng
added
feature
thomasleese
reviewed
Sep 17, 2025
There was a problem hiding this comment.
The functionality looks good, I think we should try and make it more generic, so fewer classes need to know about the Immunisations API and the logic around it. I think if we can keep that limited to ImportDuplicateForm, and then the views and controllers use methods from the form that would be ideal.
alistair-white-horne-tng
force-pushed
the
disallow-offline-sheet-edits-to-api-records
branch
4 times, most recently
from
8bfd712 to
46802f3
Compare
|
@thomasleese I've done as you described in your comment. I don't think it's reasonable to keep this logic out of the |
alistair-white-horne-tng
force-pushed
the
disallow-offline-sheet-edits-to-api-records
branch
from
46802f3 to
d6399b3
Compare
|
@misaka I just changed the logic to make it much more generic, like we discussed this morning. cc @thomasleese hopefully this makes you happy |
alistair-white-horne-tng
force-pushed
the
disallow-offline-sheet-edits-to-api-records
branch
2 times, most recently
from
4016b4f to
3ecde1a
Compare
Vaccination records which are received from the Imms API are considered immutable in Mavis, because we are not the vaccination record's primary source. As such, users should not be allowed to edit the records by any means, including an offline spreadsheet upload. We (Thomas, Mike, Alistair) considered exposing this as a validation error on the import, but that approach was discarded. This is because we would have needed to duplicate a lot of the import logic to decide whether it was appropriate to throw a validation error. This led to circular dependencies in the validation logic. Instead this approach was taken, where an adjusted import issue page is shown, forcing the user to discard the incoming changes. Manually set `apply_changes` in the `.html` Instead of depending on custom logic in the form.
alistair-white-horne-tng
force-pushed
the
disallow-offline-sheet-edits-to-api-records
branch
from
3ecde1a to
ab91ef1
Compare
thomasleese
reviewed
Sep 19, 2025
There are a number of issues which cause unedited Imms API records to flag as duplicates when uploading an offline spreadsheet. These are listed below. ## Round time objects when comparing in `stage_changes` There was an issue with detecting import duplicates on `VaccinationRecord`s; the value in the offline spreadsheet was rounded to the nearest second, whereas the value in the database was accurate to the nearest millisecond. This was causing the times to show as a duplicate to review unnecessarily. ## ODS code The offline spreadsheet used to always use the session's ODS code, not the existing `VaccinationRecord`'s. ## Performer's names I see no reason to overwrite these if they're `nil` in the upload. ## Protocol Should always be `nil` for an Imms API record. ## Location This can't be properly encoded in the offline spreadsheet, so instead have ignored it on import if importing an Imms API record.
This is a nonsensical case which causes an error. As such it should be validated out.
Tests the case where no edits are made to the vaccination record between download and upload, for Imms API records.
This test file now includes a flu session, so describing it as "HPV" is no longer accurate.
alistair-white-horne-tng
force-pushed
the
disallow-offline-sheet-edits-to-api-records
branch
from
ab91ef1 to
535ebd2
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Vaccination records which are received from the Imms API are considered
immutable in Mavis, because we are not the vaccination record's primary
source. As such, users should not be allowed to edit the records by any
means, including an offline spreadsheet upload.
We (Thomas, Mike, Alistair) considered exposing this as a validation
error on the import, but that approach was discarded. This is because we
would have needed to duplicate a lot of the import logic to decide
whether it was appropriate to throw a validation error. This led to
circular dependencies in the validation logic.
Instead this approach was taken, where an adjusted import issue page is
shown, forcing the user to discard the incoming changes.
Off the back of this change, another issue needed to be resolved;
unedited vaccination records from the API were being flagged for review.
This required a bit of a rewrite of the import logic, as well as the offline
spreadsheet logic.
MAV-1781