From 6e4ff6c3f792ef8e9b977818430c2b35f2d23701 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Thu, 18 Sep 2025 14:03:53 +0100 Subject: [PATCH 1/2] Swap associations between consent and consent forms At the moment the association is described as many consent forms have the same consent. Whereas in reality it should be one consent form has many consents (one per programme). Jira-Issue: MAV-1897 --- .../api/testing/teams_controller.rb | 3 +- app/lib/generate/consents.rb | 26 +++++++--------- app/models/consent.rb | 5 ++- app/models/consent_form.rb | 10 +++--- ...p_consent_and_consent_form_associations.rb | 29 +++++++++++++++++ db/schema.rb | 8 ++--- spec/factories/consent_forms.rb | 3 -- spec/factories/consents.rb | 3 ++ spec/models/consent_form_spec.rb | 31 +++++++++---------- spec/models/consent_spec.rb | 7 +++++ 10 files changed, 78 insertions(+), 47 deletions(-) create mode 100644 db/migrate/20250918122640_swap_consent_and_consent_form_associations.rb diff --git a/app/controllers/api/testing/teams_controller.rb b/app/controllers/api/testing/teams_controller.rb index 3041075d73..99aab882eb 100644 --- a/app/controllers/api/testing/teams_controller.rb +++ b/app/controllers/api/testing/teams_controller.rb @@ -52,11 +52,12 @@ def destroy log_destroy(SchoolMoveLogEntry.where(patient_id: patient_ids)) log_destroy(VaccinationRecord.where(patient_id: patient_ids)) + log_destroy(Consent.where(team:)) log_destroy(ConsentForm.where(id: consent_form_ids)) + log_destroy(SessionDate.where(session: sessions)) log_destroy(ArchiveReason.where(team:)) - log_destroy(Consent.where(team:)) log_destroy(Triage.where(team:)) Patient diff --git a/app/lib/generate/consents.rb b/app/lib/generate/consents.rb index 2d7d924c20..792d6a6a9b 100644 --- a/app/lib/generate/consents.rb +++ b/app/lib/generate/consents.rb @@ -90,7 +90,7 @@ def create_consents(response, count) traits = [response] end - consents = + consent_forms = available_patient_sessions.map do |patient, session| school = session.location.school? ? session.location : patient.school @@ -98,23 +98,19 @@ def create_consents(response, count) @updated_sessions << session FactoryBot.build( - :consent, - *traits, - patient:, - programme:, + :consent_form, team:, - consent_form: - FactoryBot.build( - :consent_form, - team:, - programmes: [programme], - session:, - school:, - response: - ) + programmes: [programme], + session:, + school:, + response:, + consents: [ + FactoryBot.build(:consent, *traits, patient:, programme:, team:) + ] ) end - Consent.import!(consents, recursive: true) + + ConsentForm.import!(consent_forms, recursive: true) end def validate_programme_and_session(programme, session) diff --git a/app/models/consent.rb b/app/models/consent.rb index 0fb4b0eca9..0fee7139eb 100644 --- a/app/models/consent.rb +++ b/app/models/consent.rb @@ -19,6 +19,7 @@ # withdrawn_at :datetime # created_at :datetime not null # updated_at :datetime not null +# consent_form_id :bigint # parent_id :bigint # patient_id :bigint not null # programme_id :bigint not null @@ -28,6 +29,7 @@ # Indexes # # index_consents_on_academic_year (academic_year) +# index_consents_on_consent_form_id (consent_form_id) # index_consents_on_parent_id (parent_id) # index_consents_on_patient_id (patient_id) # index_consents_on_programme_id (programme_id) @@ -36,6 +38,7 @@ # # Foreign Keys # +# fk_rails_... (consent_form_id => consent_forms.id) # fk_rails_... (parent_id => parents.id) # fk_rails_... (patient_id => patients.id) # fk_rails_... (programme_id => programmes.id) @@ -55,7 +58,7 @@ class Consent < ApplicationRecord belongs_to :programme belongs_to :team - has_one :consent_form + belongs_to :consent_form, optional: true belongs_to :parent, optional: true belongs_to :recorded_by, class_name: "User", diff --git a/app/models/consent_form.rb b/app/models/consent_form.rb index d5e3224d65..a9e722c572 100644 --- a/app/models/consent_form.rb +++ b/app/models/consent_form.rb @@ -35,7 +35,6 @@ # use_preferred_name :boolean # created_at :datetime not null # updated_at :datetime not null -# consent_id :bigint # location_id :bigint not null # school_id :bigint # team_id :bigint not null @@ -43,7 +42,6 @@ # Indexes # # index_consent_forms_on_academic_year (academic_year) -# index_consent_forms_on_consent_id (consent_id) # index_consent_forms_on_location_id (location_id) # index_consent_forms_on_nhs_number (nhs_number) # index_consent_forms_on_school_id (school_id) @@ -51,7 +49,6 @@ # # Foreign Keys # -# fk_rails_... (consent_id => consents.id) # fk_rails_... (location_id => locations.id) # fk_rails_... (school_id => locations.id) # fk_rails_... (team_id => teams.id) @@ -68,7 +65,7 @@ class ConsentForm < ApplicationRecord before_save :reset_unused_attributes - scope :unmatched, -> { where(consent_id: nil) } + scope :unmatched, -> { where.missing(:consents) } scope :recorded, -> { where.not(recorded_at: nil) } attr_accessor :health_question_number, @@ -77,14 +74,15 @@ class ConsentForm < ApplicationRecord :chosen_programme, :injection_alternative - audited associated_with: :consent + audited associated_with: :team has_associated_audits - belongs_to :consent, optional: true belongs_to :location belongs_to :school, class_name: "Location", optional: true belongs_to :team + has_many :consents + has_many :notify_log_entries has_many :consent_form_programmes, -> { ordered }, diff --git a/db/migrate/20250918122640_swap_consent_and_consent_form_associations.rb b/db/migrate/20250918122640_swap_consent_and_consent_form_associations.rb new file mode 100644 index 0000000000..f99daf4c12 --- /dev/null +++ b/db/migrate/20250918122640_swap_consent_and_consent_form_associations.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class SwapConsentAndConsentFormAssociations < ActiveRecord::Migration[8.0] + def change + add_reference :consents, :consent_form, foreign_key: true + + reversible do |direction| + direction.up do + ConsentForm + .where.not(consent_id: nil) + .find_each do |consent_form| + consent = Consent.find(consent_form.consent_id) + consent.update_column(:consent_form_id, consent_form.id) + end + end + + direction.down do + Consent + .where.not(consent_form_id: nil) + .find_each do |consent| + consent_form = ConsentForm.find(consent.consent_form_id) + consent_form.update_column(:consent_id, consent.id) + end + end + end + + remove_reference :consent_forms, :consent, foreign_key: true + end +end diff --git a/db/schema.rb b/db/schema.rb index bbc4396eff..29b59b8b17 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_09_16_074716) do +ActiveRecord::Schema[8.0].define(version: 2025_09_18_122640) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "pg_trgm" @@ -204,7 +204,6 @@ t.string "address_town" t.string "address_postcode" t.jsonb "health_answers", default: [], null: false - t.bigint "consent_id" t.string "parent_contact_method_other_details" t.string "parent_contact_method_type" t.string "parent_email" @@ -225,7 +224,6 @@ t.text "notes", default: "", null: false t.integer "academic_year", null: false t.index ["academic_year"], name: "index_consent_forms_on_academic_year" - t.index ["consent_id"], name: "index_consent_forms_on_consent_id" t.index ["location_id"], name: "index_consent_forms_on_location_id" t.index ["nhs_number"], name: "index_consent_forms_on_nhs_number" t.index ["school_id"], name: "index_consent_forms_on_school_id" @@ -270,7 +268,9 @@ t.integer "vaccine_methods", default: [], null: false, array: true t.integer "academic_year", null: false t.boolean "notify_parent_on_refusal" + t.bigint "consent_form_id" t.index ["academic_year"], name: "index_consents_on_academic_year" + t.index ["consent_form_id"], name: "index_consents_on_consent_form_id" t.index ["parent_id"], name: "index_consents_on_parent_id" t.index ["patient_id"], name: "index_consents_on_patient_id" t.index ["programme_id"], name: "index_consents_on_programme_id" @@ -992,7 +992,6 @@ add_foreign_key "cohort_imports_patients", "patients" add_foreign_key "consent_form_programmes", "consent_forms" add_foreign_key "consent_form_programmes", "programmes" - add_foreign_key "consent_forms", "consents" add_foreign_key "consent_forms", "locations" add_foreign_key "consent_forms", "locations", column: "school_id" add_foreign_key "consent_forms", "teams" @@ -1001,6 +1000,7 @@ add_foreign_key "consent_notifications", "patients" add_foreign_key "consent_notifications", "sessions" add_foreign_key "consent_notifications", "users", column: "sent_by_user_id" + add_foreign_key "consents", "consent_forms" add_foreign_key "consents", "parents" add_foreign_key "consents", "patients" add_foreign_key "consents", "programmes" diff --git a/spec/factories/consent_forms.rb b/spec/factories/consent_forms.rb index b939267a8f..f6e7d2fa8a 100644 --- a/spec/factories/consent_forms.rb +++ b/spec/factories/consent_forms.rb @@ -35,7 +35,6 @@ # use_preferred_name :boolean # created_at :datetime not null # updated_at :datetime not null -# consent_id :bigint # location_id :bigint not null # school_id :bigint # team_id :bigint not null @@ -43,7 +42,6 @@ # Indexes # # index_consent_forms_on_academic_year (academic_year) -# index_consent_forms_on_consent_id (consent_id) # index_consent_forms_on_location_id (location_id) # index_consent_forms_on_nhs_number (nhs_number) # index_consent_forms_on_school_id (school_id) @@ -51,7 +49,6 @@ # # Foreign Keys # -# fk_rails_... (consent_id => consents.id) # fk_rails_... (location_id => locations.id) # fk_rails_... (school_id => locations.id) # fk_rails_... (team_id => teams.id) diff --git a/spec/factories/consents.rb b/spec/factories/consents.rb index 402dcc84b8..65d0273204 100644 --- a/spec/factories/consents.rb +++ b/spec/factories/consents.rb @@ -19,6 +19,7 @@ # withdrawn_at :datetime # created_at :datetime not null # updated_at :datetime not null +# consent_form_id :bigint # parent_id :bigint # patient_id :bigint not null # programme_id :bigint not null @@ -28,6 +29,7 @@ # Indexes # # index_consents_on_academic_year (academic_year) +# index_consents_on_consent_form_id (consent_form_id) # index_consents_on_parent_id (parent_id) # index_consents_on_patient_id (patient_id) # index_consents_on_programme_id (programme_id) @@ -36,6 +38,7 @@ # # Foreign Keys # +# fk_rails_... (consent_form_id => consent_forms.id) # fk_rails_... (parent_id => parents.id) # fk_rails_... (patient_id => patients.id) # fk_rails_... (programme_id => programmes.id) diff --git a/spec/models/consent_form_spec.rb b/spec/models/consent_form_spec.rb index 0b8f56bb4d..357facee01 100644 --- a/spec/models/consent_form_spec.rb +++ b/spec/models/consent_form_spec.rb @@ -35,7 +35,6 @@ # use_preferred_name :boolean # created_at :datetime not null # updated_at :datetime not null -# consent_id :bigint # location_id :bigint not null # school_id :bigint # team_id :bigint not null @@ -43,7 +42,6 @@ # Indexes # # index_consent_forms_on_academic_year (academic_year) -# index_consent_forms_on_consent_id (consent_id) # index_consent_forms_on_location_id (location_id) # index_consent_forms_on_nhs_number (nhs_number) # index_consent_forms_on_school_id (school_id) @@ -51,7 +49,6 @@ # # Foreign Keys # -# fk_rails_... (consent_id => consents.id) # fk_rails_... (location_id => locations.id) # fk_rails_... (school_id => locations.id) # fk_rails_... (team_id => teams.id) @@ -60,6 +57,10 @@ describe ConsentForm do subject(:consent_form) { build(:consent_form) } + describe "associations" do + it { should have_many(:consents) } + end + describe "validations" do subject(:consent_form) do create( @@ -550,30 +551,26 @@ describe "scope unmatched" do let(:programme) { create(:programme) } let(:session) { create(:session, programmes: [programme]) } - let(:consent) { create(:consent, programme:) } - let(:unmatched_consent_form) do - create(:consent_form, consent: nil, session:) - end - let(:matched_consent_form) { create(:consent_form, consent:, session:) } + let(:unmatched_consent_form) { create(:consent_form, session:) } + let(:matched_consent_form) { create(:consent_form, session:) } + + before { create(:consent, programme:, consent_form: matched_consent_form) } it "returns unmatched consent forms" do - expect(described_class.unmatched).to include unmatched_consent_form - expect(described_class.unmatched).not_to include matched_consent_form + expect(described_class.unmatched).to include(unmatched_consent_form) + expect(described_class.unmatched).not_to include(matched_consent_form) end end describe "scope recorded" do let(:programme) { create(:programme) } let(:session) { create(:session, programmes: [programme]) } - let(:consent) { create(:consent, programme:) } - let(:recorded_consent_form) do - create(:consent_form, :recorded, consent:, session:) - end - let(:draft_consent_form) { create(:consent_form, consent:, session:) } + let(:recorded_consent_form) { create(:consent_form, :recorded, session:) } + let(:draft_consent_form) { create(:consent_form, session:) } it "returns unmatched consent forms" do - expect(described_class.recorded).to include recorded_consent_form - expect(described_class.recorded).not_to include draft_consent_form + expect(described_class.recorded).to include(recorded_consent_form) + expect(described_class.recorded).not_to include(draft_consent_form) end end diff --git a/spec/models/consent_spec.rb b/spec/models/consent_spec.rb index 28dfd1e52f..d10616b3a9 100644 --- a/spec/models/consent_spec.rb +++ b/spec/models/consent_spec.rb @@ -19,6 +19,7 @@ # withdrawn_at :datetime # created_at :datetime not null # updated_at :datetime not null +# consent_form_id :bigint # parent_id :bigint # patient_id :bigint not null # programme_id :bigint not null @@ -28,6 +29,7 @@ # Indexes # # index_consents_on_academic_year (academic_year) +# index_consents_on_consent_form_id (consent_form_id) # index_consents_on_parent_id (parent_id) # index_consents_on_patient_id (patient_id) # index_consents_on_programme_id (programme_id) @@ -36,6 +38,7 @@ # # Foreign Keys # +# fk_rails_... (consent_form_id => consent_forms.id) # fk_rails_... (parent_id => parents.id) # fk_rails_... (patient_id => patients.id) # fk_rails_... (programme_id => programmes.id) @@ -46,6 +49,10 @@ describe Consent do subject(:consent) { build(:consent) } + describe "associations" do + it { should belong_to(:consent_form).optional(true) } + end + describe "validations" do it { should validate_length_of(:notes).is_at_most(1000) } From c87bf6a227449e265cf983dcbbedb848d60a750a Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Thu, 18 Sep 2025 15:19:02 +0100 Subject: [PATCH 2/2] Discard `ConsentFormMatchingJob` once matched This ensures that if a consent form has already been matched, the automated process doesn't run and match consents with patients which might result in duplicate consents. This is necessary as we've moved this job to Sidekiq where it will continue to retry if it fails. For example, it might fail due to a bad request from PDS, and continue to do this for the next 21 days. In that time, the consent form may have been matched manually and therefore we would never want the automatic job to succeed. --- app/jobs/consent_form_matching_job.rb | 4 ++++ app/models/consent_form.rb | 2 ++ spec/jobs/consent_form_matching_job_spec.rb | 24 ++++++++++++++++++++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/app/jobs/consent_form_matching_job.rb b/app/jobs/consent_form_matching_job.rb index 2398552a6e..c2e5f1b5b4 100644 --- a/app/jobs/consent_form_matching_job.rb +++ b/app/jobs/consent_form_matching_job.rb @@ -9,6 +9,8 @@ class ConsentFormMatchingJob < ApplicationJob def perform(consent_form) @consent_form = consent_form + return if already_matched? + # Match if we find a patient with the PDS NHS number return if match_with_exact_nhs_number @@ -47,6 +49,8 @@ def pds_patient @pds_patient ||= PDS::Patient.search(**query) end + def already_matched? = @consent_form.matched? + def match_with_exact_nhs_number return false unless pds_patient diff --git a/app/models/consent_form.rb b/app/models/consent_form.rb index a9e722c572..9935cd541b 100644 --- a/app/models/consent_form.rb +++ b/app/models/consent_form.rb @@ -318,6 +318,8 @@ def wizard_steps def recorded? = recorded_at != nil + def matched? = consents.exists? + def response_given? = consent_form_programmes.any?(&:response_given?) def response_refused? = consent_form_programmes.any?(&:response_refused?) diff --git a/spec/jobs/consent_form_matching_job_spec.rb b/spec/jobs/consent_form_matching_job_spec.rb index b94fcdeaae..77c20bff33 100644 --- a/spec/jobs/consent_form_matching_job_spec.rb +++ b/spec/jobs/consent_form_matching_job_spec.rb @@ -16,7 +16,7 @@ ) end - before do + let!(:stub) do stub_request( :get, "https://sandbox.api.service.nhs.uk/personal-demographics/FHIR/R4/Patient" @@ -34,6 +34,28 @@ it "doesn't create a consent" do expect { perform }.not_to change(Consent, :count) end + + it "makes requests to PDS" do + perform + expect(stub).to have_been_requested.twice + end + end + + context "with a consent form that's already been matched" do + let(:response_file) { "pds/search-patients-no-results-response.json" } + + before do + create(:consent, programme: session.programmes.first, consent_form:) + end + + it "doesn't create a consent" do + expect { perform }.not_to change(Consent, :count) + end + + it "doesn't make a request to PDS" do + perform + expect(stub).not_to have_been_requested + end end context "with one matching patient" do