Include records for patients with newly added nhs numbers in automated careplus reports

[jf-x-dev]

From the ticket:

For a given team, if a patient goes from having no NHS number to having an NHS number, then we want to include all of the records for that patient in the next automated careplus export. However, we only want to include the records that were created after the automated careplus integration was enabled for the team - record prior to this will have been sent manually.

We accomplish this by adding a nhs_number_first_added_at field to Patient and a careplus_integration_enabled_at field to Team. This allows us to extend the scope of the automated careplus reporter.

Jira Issue - MAV-7091

[thomasleese]

I don't think this will always work, particularly if we're importing patients in bulk via Patient.import (where callbacks won't typically be called).

I think we might need to add something to

manage-vaccinations-in-schools/app/models/immunisation_import_row.rb

Lines 553 to 564 in ea73635

	def new_patient_attributes
	{
	address_postcode: patient_postcode&.to_postcode,
	date_of_birth: patient_date_of_birth&.to_date,
	birth_academic_year: patient_date_of_birth&.to_date&.academic_year,
	family_name: patient_last_name.to_s,
	given_name: patient_first_name.to_s,
	gender_code: patient_gender_code_value,
	nhs_number: patient_nhs_number_value,
	school: nil
	}.compact
	end

and https://github.com/NHSDigital/manage-vaccinations-in-schools/blob/main/app/models/patient_import_row.rb.

In general we've tried to avoid callbacks where possible due to the fact that they don't always execute and instead set the values explicitly.

[jf-x-dev]

I think I've added unit tests for each place where the nhs number can change. It looks like the callback works in almost all cases, but as you said it doesn't trigger during imports.

I was struggling to find a nice way to explicitly set nhs_number_added_at in all cases, since we'd need to do this for real changes, staged changes and bulk imports which would all be slightly different (plus we'd need to do this for any future mechanisms that set nhs numbers).

The callback does work in most cases, so any "standard" future mechanism that changes nhs numbers should be covered by it. If there are any new mechanisms that don't trigger callbacks, ideally we'd investigate and see that we need to update the nhs_number_first_added_at field as a part of the mechanism.

For now I've kept the callback and handled the bulk import case manually, but would be interested to hear what you think about it

[thomasleese]

Can we test how long this takes on data replication, as I believe it'll lock the table, particularly adding the index.

[jf-x-dev]

Good point, we should definitely test this. I don't have DR access but I can find someone to test this with

[jf-x-dev]

I've had another look and it seems we should be a bit more careful about this, so I've split the migrations up and moved the backfill column part to data - which will now be handled in batches and outside of a transaction

[thomasleese]

Is this change necessary because the callbacks don't execute without it?

[jf-x-dev]

Yes, the callbacks didn't execute otherwise