Skip to content

Rename processed? to processed_at?#6637

Open
misaka wants to merge 76 commits intonextfrom
rename-processed-to-processed-at
Open

Rename processed? to processed_at?#6637
misaka wants to merge 76 commits intonextfrom
rename-processed-to-processed-at

Conversation

@misaka
Copy link
Copy Markdown
Collaborator

@misaka misaka commented Apr 20, 2026

We already have a status in CSVImportable called processed so it's helper method processed? was being masked by this method. We also don't have to define a processed_at? as Rails has automatically generated one for use.

Release version is tbd.

MAV-6746

thomasleese and others added 30 commits April 16, 2026 13:52
I've renamed this to have a prefix of `pds` as we're going to be adding
two more feature flags both related to PDS so it makes sense to group
them together.

Jira-Issue: MAV-2354
This replaces the `enqueue_bulk_updates` setting on `Settings.pds` with
a feature flag which makes it easier to be enable/disable the feature
without needing an infrastructure deploy.

Jira-Issue: MAV-2354
This replaces `Settings.pds.enabled` with a feature flag that we can use
to quickly enable or disable any PDS related functionality without
needing to re-deploy the application.

Jira-Issue: MAV-2354
This reduces the risk of supply chain attacks where the tag points to a
different commit implicitly without us being aware.

Jira-Issue: MAV-5897
This refactors how the options for the "who" step of the draft consent
form are determined to simplify the code, and allow for a more complex
set up around self-consent.

Jira-Issue: MAV-5915
This adds a method on the `Patient` model that can be used to determine
if a child can self-consent, following a competent Gillick assessment.

Jira-Issue: MAV-5915
Gillick assessments are only considered valid for the day that they were
recorded on. Outside of that, the child needs to be re-assessed before
they can self-consent.

Jira-Issue: MAV-5915
The national reporting teams uses the same vaccination table but requires more details on vaccination records like
location and source, whereas full-fat Mavis doesn't need so much information. The new `show_details` arg controls
whether to show or hide this info.
Shows the programme status (heading with colour, contextual detail text, vaccination outcomes table, and action link) for a patient on the session patient show page. This card  links to the respective programme tab in the child’s record. Eventually this will be the only card on the page with a coloured heading so various statuses (consent, triage, etc) do not compete for attention and the programme status for the child is reflected right at the top.

MAV-4846
When a triage has been performed, the programme card now shows a human-readable summary of the decision (e.g. "Joy, Nurse decided that Oliver is safe to vaccinate.") using the shared triage_summary helper. The delay vaccination case is updated to include the "until" date when present.
…undle-link-mismatch

Suppress warning for `immunization.target` mismatch
…tions

Improve date validations on imports
Only consider today’s Gillick assessments as valid
We've seen some evidence that the batching queries generated by
`PatientStatusUpdater#update_programme_statuses!` are (at least sometimes) slow
in production, potentially because of a bunch of sequential scans relating to
JOINs generated by the call to `ActiveRecord::QueryMethods#includes`.

The call to `#includes` is necessary in order to trigger eager loading of
associations to avoid N+1 queries when calling
`Patient::ProgrammeStatus#assign`. However, this is only relevant for the batch
queries which make the model instances available within each batch; not for the
batching queries which just work out which model IDs to include in a given
batch.

Thus we can simplify the batching queries without changing the overall
behaviour by using `ActiveRecord::Batches#in_batches` [1] instead of
`#find_in_batches` and moving the call to `#includes` onto the batch relation.

Note that as per the `ActiveRecord::QueryMethods#includes` docs [2]:

> A separate query is performed for each association, unless a join is required
> by conditions.

This means that the JOINs were only being generated on the batching queries
when the patient scope included a condition, e.g. when associated with an
`ImmunisationImport`. Given the instance of `PatientStatusUpdaterJob` scheduled
overnight [3] uses the default arguments, its batching query is *unscoped* and
thus it may not directly benefit from any performance improvements made by the
changes in this commit.

[1]: https://api.rubyonrails.org/v8.1.3/classes/ActiveRecord/Batches.html#method-i-in_batches
[2]: https://api.rubyonrails.org/v8.1.3/classes/ActiveRecord/QueryMethods.html#method-i-includes
[3]: https://github.com/NHSDigital/manage-vaccinations-in-schools/blob/6895ee514a018aaa507a460e332a32b3d9e8b6a1/config/sidekiq.yml#L70-L73

Co-authored-by: Chris Lowis <chris.lowis@gofreerange.com>
Co-authored-by: Chris Roos <chris.roos@gofreerange.com>
…atient-status-updater

Simplify batching query in PatientStatusUpdater
Pin all GitHub Actions to commit SHAs for security
misaka and others added 25 commits April 17, 2026 14:42
This adds the logic where if a child is eligible for the programme and
hasn't been sent any consent requests and has parent contact details:
- If part of a session where consent requests are scheduled to go out in
  the future, the programme status becomes `needs_consent_request_scheduled`.
- If not assigned to a session with a request scheduled, the programme
  status becomes `needs_consent_request_not_scheduled`.

Jira-Issue: MAV-5882
This allows us to run the end_to_end stack while running development locally.
The convention is capitalised variable names are intended for environment
variables that are exported and passed between processes. This change makes it
clear that only RAILS_ENV and REDIS_URL are, in fact, env vars.
…cheduled-not-scheduled-attempt-2

Implement logic needs consent status request scheduled/not scheduled
This helps prevent the E2E tests from being flaky.

This has already been implemented for the national reporting `Vaccine`s.
Sort `Vaccine`s alphabetically in upload help test
Bumps [prettier](https://github.com/prettier/prettier) from 3.8.1 to 3.8.2.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md)
- [Commits](prettier/prettier@3.8.1...3.8.2)

---
updated-dependencies:
- dependency-name: prettier
  dependency-version: 3.8.2
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…/prettier-3.8.2

Bump prettier from 3.8.1 to 3.8.2
Bumps the bundler group with 1 update in the / directory: [yard](https://yardoc.org).


Updates `yard` from 0.9.38 to 0.9.42

---
updated-dependencies:
- dependency-name: yard
  dependency-version: 0.9.42
  dependency-type: indirect
  dependency-group: bundler
...

Signed-off-by: dependabot[bot] <support@github.com>
…134ee445d

Bump yard from 0.9.38 to 0.9.42 in the bundler group across 1 directory
This rotates the API key we use in production as the old key was created
18 months ago and it's good practice to rotate regularly.

Jira-Issue: MAV-6545
Since they are shared behaviour ...

Jira-Issue: MAV-6062
Import::CSVData will be used to abstract certain operations out of
CSVImportable, such as loading CSV data from user params.

Jira-Issue: MAV-6063
It is already tested as part of CSVParser and the setup is confusing in these
this tests. It needs to know that the encoding is either detected as part of
`csv=` through the bom (which we don't use here), or in CSVParser, which is
where this test really belongs anyway.

Jira-Issue: MAV-6062
Recent changes rewired specs to test import by setting `csv_data` instead of
`csv` to isolate what's being tested, but that left `csv=` largely being untested.

Jira-Issue: MAV-6063
This changes import specs to not make test `subject` an action. This is
stylistic, it can work either way, but considering how hard it's been to reason
about the imports I thought it would be better to return to what's considered
common convention to try to make the tests easier to understand.

This became particularly apparent when a call to `parse_rows!` had to be added
to tests (coming in a later change), meaning the `subject` would need a call to
`parse_rows!`, or possibly `parse_rows!` added to the `before` block, but either
way it was becoming un-idiomatic.

Jira-Issue: MAV-6063
This is to make it easier to reason about the code: when parse_rows! is explicit
it's clear what stage of the process is executing when.

Jira-Issue: MAV-6063
The intention is to make the import processing simpler to understand. Since
nearly all the processing happens in the sub-classes we just call that directly
from the ProcessImportJob.

Jira-Issue: MAV-6063
@misaka misaka requested a review from a team as a code owner April 20, 2026 10:37
@misaka misaka added the refactor Improving maintainability label Apr 20, 2026
@misaka misaka added this to the v8.2.0 milestone Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Improving maintainability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants