Skip to content

Add series_fingerprint to multiple images information form#981

Merged
malcolmbaig merged 2 commits intomainfrom
11827-multi-image-info--stale-form
Feb 10, 2026
Merged

Add series_fingerprint to multiple images information form#981
malcolmbaig merged 2 commits intomainfrom
11827-multi-image-info--stale-form

Conversation

@malcolmbaig
Copy link
Copy Markdown
Contributor

@malcolmbaig malcolmbaig commented Feb 3, 2026

Description

If the multiple images information form is stale (eg—changes have been made to the series in a different tab), we need some way of detecting a mismatch between the series data persisted in the database, and the fields present on the multiple images form.

In this PR:

  • A hidden series_fingerprint field is embedded in the form on render
  • On POST the view compares it against current DB state to detect if the underlying series have changed
  • If stale, the user is redirected to add image details
  • Refactor the multiple images information form to simply take a study instance rather than passing in the series list explicitly.

Jira link

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

Review notes

Review checklist

  • Check database queries are correctly scoped to current_provider

@malcolmbaig malcolmbaig force-pushed the 11827-multi-image-info--stale-form branch from af01281 to 2346496 Compare February 3, 2026 17:19
@malcolmbaig malcolmbaig force-pushed the 11827-add-image-info-for-repeats branch 3 times, most recently from 5965ee4 to 9dad496 Compare February 6, 2026 13:18
Base automatically changed from 11827-add-image-info-for-repeats to main February 6, 2026 14:31
@malcolmbaig malcolmbaig force-pushed the 11827-multi-image-info--stale-form branch from 2346496 to 3082848 Compare February 6, 2026 14:32
- A hidden series_fingerprint field is embedded in the form on render
- On POST the view compares it against current DB state to detect if
  the underlying series have changed
- If stale, the user is redirected to add image details
@malcolmbaig malcolmbaig force-pushed the 11827-multi-image-info--stale-form branch from 3082848 to 9e7052f Compare February 6, 2026 16:35
Refactor the form by removing the explicit series_list parameter from
the form constructor. The form now derives it from
instance.series_with_multiple_images(), where instance is the study.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Feb 6, 2026

Copy link
Copy Markdown
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. 👍

return redirect(self.get_success_url())
return super().get(request, *args, **kwargs)

def post(self, request, *args, **kwargs):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have a mix of FormView control flow and our own here... would it be cleaner to get rid of FormView and just depend on View for this one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think there's still some benefit to using it here in terms of what it handles for us, plus we can see at a glance what our custom behaviour is and otherwise safely assume it just works like our various other FormViews.

@malcolmbaig malcolmbaig merged commit efd7d59 into main Feb 10, 2026
14 checks passed
@malcolmbaig malcolmbaig deleted the 11827-multi-image-info--stale-form branch February 10, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants