From 16ea190fe49ba20d4f51c5de439f326292342416 Mon Sep 17 00:00:00 2001 From: Lorenzo Tello Date: Wed, 16 Oct 2024 18:57:37 +0100 Subject: [PATCH 1/6] bug fix: do not archive participants in a closing cohort with archivable and non-archivable declarations --- app/models/participant_profile/ecf.rb | 21 ---------- app/models/participant_profile/ect.rb | 34 ++++++++++++++-- app/models/participant_profile/mentor.rb | 40 +++++++++++++++++-- .../shared_examples/archivable_support.rb | 16 ++++++-- 4 files changed, 81 insertions(+), 30 deletions(-) diff --git a/app/models/participant_profile/ecf.rb b/app/models/participant_profile/ecf.rb index 2a8677f80a..b8ae1f1162 100644 --- a/app/models/participant_profile/ecf.rb +++ b/app/models/participant_profile/ecf.rb @@ -80,27 +80,6 @@ def eligible_to_change_cohort_back_to_their_payments_frozen_original?(cohort:, c participant_declarations.billable.where(cohort:).exists? end - def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) - unbillable_states = %i[ineligible voided submitted].freeze - - # Find all participants that have no FIP induction records (as finding those with only FIP is more complicated). - not_fip_induction_records = InductionRecord.left_joins(:induction_programme).where.not(induction_programme: { training_programme: :full_induction_programme }) - not_fip_induction_records = not_fip_induction_records.where(participant_profile_id: restrict_to_participant_ids) if restrict_to_participant_ids.any? - - query = left_joins(:participant_declarations, schedule: :cohort) - # Exclude participants that have any induction records that are not FIP. - .where.not(id: not_fip_induction_records.select(:participant_profile_id)) - .where.not(cohorts: { payments_frozen_at: nil }) - # Exclude participants that have any billable/submitted declarations, but - # retain participants that have no declarations at all. - .where.not("participant_declarations.id IS NOT NULL AND participant_declarations.state NOT IN (?)", unbillable_states) - .distinct - - query = query.where(id: restrict_to_participant_ids) if restrict_to_participant_ids.any? - - query - end - # Instance Methods def eligible_to_change_cohort_and_continue_training?(cohort:) self.class.eligible_to_change_cohort_and_continue_training(cohort:, restrict_to_participant_ids: [id]).exists? diff --git a/app/models/participant_profile/ect.rb b/app/models/participant_profile/ect.rb index 684e236cdc..4dc889a698 100644 --- a/app/models/participant_profile/ect.rb +++ b/app/models/participant_profile/ect.rb @@ -11,9 +11,37 @@ class ParticipantProfile::ECT < ParticipantProfile::ECF } def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) - super(restrict_to_participant_ids:) - .where(induction_completion_date: nil) - .where("induction_start_date IS NULL OR induction_start_date < make_date(cohorts.start_year, 9, 1)") + archivable_states = %i[ineligible voided submitted].freeze + + # ECTs that have no FIP induction records + not_fip = InductionRecord.joins(:induction_programme, participant_profile: { schedule: :cohort }) + .where.not(cohorts: { payments_frozen_at: nil }) + .where(participant_profiles: { induction_completion_date: nil }) + .where(participant_profiles: { type: "ParticipantProfile::ECT" }) + .where("participant_profiles.induction_start_date IS NULL OR participant_profiles.induction_start_date < make_date(cohorts.start_year, 9, 1)") + .where.not(induction_programme: { training_programme: :full_induction_programme }) + not_fip = not_fip.where(participant_profile_id: restrict_to_participant_ids) if restrict_to_participant_ids.any? + not_fip_ids = not_fip.pluck(:participant_profile_id).uniq + + # ECTs that have at least one non-archivable declaration + with_unarchivable_declaration = joins(:participant_declarations, schedule: :cohort) + .where.not(cohorts: { payments_frozen_at: nil }) + .where(induction_completion_date: nil) + .where("induction_start_date IS NULL OR induction_start_date < make_date(cohorts.start_year, 9, 1)") + .where.not(id: not_fip_ids) + .where.not(participant_declarations: { state: archivable_states }) + .distinct + with_unarchivable_declaration = with_unarchivable_declaration.where(id: restrict_to_participant_ids) if restrict_to_participant_ids.any? + with_unarchivable_declaration_ids = with_unarchivable_declaration.pluck(:id) + + query = joins(schedule: :cohort) + .where.not(cohorts: { payments_frozen_at: nil }) + .where(induction_completion_date: nil) + .where("induction_start_date IS NULL OR induction_start_date < make_date(cohorts.start_year, 9, 1)") + .where.not(id: not_fip_ids + with_unarchivable_declaration_ids) + query = query.where(id: restrict_to_participant_ids) if restrict_to_participant_ids.any? + + query end def completed_training? diff --git a/app/models/participant_profile/mentor.rb b/app/models/participant_profile/mentor.rb index 86e2ab21e2..db70330aed 100644 --- a/app/models/participant_profile/mentor.rb +++ b/app/models/participant_profile/mentor.rb @@ -22,9 +22,43 @@ class ParticipantProfile::Mentor < ParticipantProfile::ECF } def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) - super(restrict_to_participant_ids:) - .where(mentor_completion_date: nil) - .where.not(id: InductionRecord.where.not(mentor_profile_id: nil).select(:mentor_profile_id).distinct) + archivable_states = %i[ineligible voided submitted].freeze + + # ECTs that have no FIP induction records + not_fip = InductionRecord.joins(:induction_programme, participant_profile: { schedule: :cohort }) + .where.not(cohorts: { payments_frozen_at: nil }) + .where(participant_profiles: { mentor_completion_date: nil }) + .where(participant_profiles: { type: "ParticipantProfile::Mentor" }) + .where.not(induction_programme: { training_programme: :full_induction_programme }) + not_fip = not_fip.where(participant_profile_id: restrict_to_participant_ids) if restrict_to_participant_ids.any? + not_fip_ids = not_fip.pluck(:participant_profile_id).uniq + + # Mentors that have at least one non-archivable declaration + with_unarchivable_declaration = joins(:participant_declarations, schedule: :cohort) + .where.not(cohorts: { payments_frozen_at: nil }) + .where(mentor_completion_date: nil) + .where.not(id: not_fip_ids) + .where.not(participant_declarations: { state: archivable_states }) + .distinct + with_unarchivable_declaration = with_unarchivable_declaration.where(id: restrict_to_participant_ids) if restrict_to_participant_ids.any? + with_unarchivable_declaration_ids = with_unarchivable_declaration.pluck(:id) + + # Mentors ever assigned an ECT + with_mentee = InductionRecord.joins(:induction_programme, :participant_profile) + .where(participant_profiles: { type: "ParticipantProfile::ECT" }) + .where.not(mentor_profile_id: nil) + .where.not(mentor_profile_id: not_fip_ids + with_unarchivable_declaration_ids) + with_mentee = with_mentee.where(mentor_profile_id: restrict_to_participant_ids) if restrict_to_participant_ids.any? + with_mentee_ids = with_mentee.pluck(:mentor_profile_id).uniq + + # Mentors in a payments frozen cohort with no completion date + query = joins(schedule: :cohort) + .where.not(cohorts: { payments_frozen_at: nil }) + .where(mentor_completion_date: nil) + .where.not(id: not_fip_ids + with_unarchivable_declaration_ids + with_mentee_ids) + query = query.where(id: restrict_to_participant_ids) if restrict_to_participant_ids.any? + + query end def complete_training!(completion_date:, completion_reason:) diff --git a/spec/support/shared_examples/archivable_support.rb b/spec/support/shared_examples/archivable_support.rb index 49379f9dd3..09fb999155 100644 --- a/spec/support/shared_examples/archivable_support.rb +++ b/spec/support/shared_examples/archivable_support.rb @@ -70,9 +70,19 @@ def build_declaration(attrs = {}) expect(participant_profile).not_to be_archivable_from_frozen_cohort end - it "returns false if the participant has billable declarations" do - participant_profile = build_declaration(state: :paid, cohort: eligible_cohort).participant_profile - expect(participant_profile).not_to be_archivable_from_frozen_cohort + it "returns false if the participant has not archivable declarations" do + paid_declaration = build_declaration(state: :paid, cohort: eligible_cohort) + participant_profile = paid_declaration.participant_profile + declaration_date = participant_profile.schedule.milestones.find_by(declaration_type: "retained-2").milestone_date - 1.day + travel_to declaration_date do + build_declaration(state: :voided, + cohort: eligible_cohort, + participant_profile:, + cpd_lead_provider: paid_declaration.cpd_lead_provider, + declaration_date:, + declaration_type: "retained-2") + expect(participant_profile).not_to be_archivable_from_frozen_cohort + end end it "returns false if the participant has a CIP induction record" do From 48b40e4b684d17dbe51dd0432a6ab94ba92663ba Mon Sep 17 00:00:00 2001 From: Lorenzo Tello Date: Wed, 16 Oct 2024 19:12:18 +0100 Subject: [PATCH 2/6] Update app/models/participant_profile/mentor.rb --- app/models/participant_profile/mentor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/participant_profile/mentor.rb b/app/models/participant_profile/mentor.rb index db70330aed..5fb6816344 100644 --- a/app/models/participant_profile/mentor.rb +++ b/app/models/participant_profile/mentor.rb @@ -51,7 +51,7 @@ def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) with_mentee = with_mentee.where(mentor_profile_id: restrict_to_participant_ids) if restrict_to_participant_ids.any? with_mentee_ids = with_mentee.pluck(:mentor_profile_id).uniq - # Mentors in a payments frozen cohort with no completion date + # Mentors in a payments frozen cohort with no completion date excluding the ones above query = joins(schedule: :cohort) .where.not(cohorts: { payments_frozen_at: nil }) .where(mentor_completion_date: nil) From 56e2e75447acd4c268d8fa412aa6dd81943e8d9d Mon Sep 17 00:00:00 2001 From: Lorenzo Tello Date: Wed, 16 Oct 2024 19:12:30 +0100 Subject: [PATCH 3/6] Update app/models/participant_profile/mentor.rb --- app/models/participant_profile/mentor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/participant_profile/mentor.rb b/app/models/participant_profile/mentor.rb index 5fb6816344..96ece86e52 100644 --- a/app/models/participant_profile/mentor.rb +++ b/app/models/participant_profile/mentor.rb @@ -24,7 +24,7 @@ class ParticipantProfile::Mentor < ParticipantProfile::ECF def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) archivable_states = %i[ineligible voided submitted].freeze - # ECTs that have no FIP induction records + # Mentors that have no FIP induction records not_fip = InductionRecord.joins(:induction_programme, participant_profile: { schedule: :cohort }) .where.not(cohorts: { payments_frozen_at: nil }) .where(participant_profiles: { mentor_completion_date: nil }) From b1c56896d5fbdbd1df77217e6aecc9b4b97fa706 Mon Sep 17 00:00:00 2001 From: Lorenzo Tello Date: Wed, 16 Oct 2024 19:12:37 +0100 Subject: [PATCH 4/6] Update app/models/participant_profile/ect.rb --- app/models/participant_profile/ect.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/participant_profile/ect.rb b/app/models/participant_profile/ect.rb index 4dc889a698..ee0a1008b3 100644 --- a/app/models/participant_profile/ect.rb +++ b/app/models/participant_profile/ect.rb @@ -34,6 +34,7 @@ def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) with_unarchivable_declaration = with_unarchivable_declaration.where(id: restrict_to_participant_ids) if restrict_to_participant_ids.any? with_unarchivable_declaration_ids = with_unarchivable_declaration.pluck(:id) + # ECTs in a payments-frozen cohort with no induction start date or prior to Sept 2021 excluding the ones above query = joins(schedule: :cohort) .where.not(cohorts: { payments_frozen_at: nil }) .where(induction_completion_date: nil) From 9231258b3807f0b1c91708910c20a90f4cf3a5b7 Mon Sep 17 00:00:00 2001 From: Lorenzo Tello Date: Wed, 16 Oct 2024 20:48:17 +0100 Subject: [PATCH 5/6] restrict with_mentee query to speed up the general archiving query --- app/models/participant_profile/mentor.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/participant_profile/mentor.rb b/app/models/participant_profile/mentor.rb index 96ece86e52..42c11fd1c1 100644 --- a/app/models/participant_profile/mentor.rb +++ b/app/models/participant_profile/mentor.rb @@ -44,10 +44,11 @@ def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) with_unarchivable_declaration_ids = with_unarchivable_declaration.pluck(:id) # Mentors ever assigned an ECT - with_mentee = InductionRecord.joins(:induction_programme, :participant_profile) - .where(participant_profiles: { type: "ParticipantProfile::ECT" }) + with_mentee = InductionRecord.joins(:induction_programme, mentor_profile: { schedule: :cohort }) .where.not(mentor_profile_id: nil) .where.not(mentor_profile_id: not_fip_ids + with_unarchivable_declaration_ids) + .where.not(cohorts: { payments_frozen_at: nil }) + .where(participant_profiles: { mentor_completion_date: nil }) with_mentee = with_mentee.where(mentor_profile_id: restrict_to_participant_ids) if restrict_to_participant_ids.any? with_mentee_ids = with_mentee.pluck(:mentor_profile_id).uniq From 49239aa6b6718fc5cd275c21ccd5ef78e534664a Mon Sep 17 00:00:00 2001 From: Lorenzo Tello Date: Thu, 17 Oct 2024 18:25:43 +0100 Subject: [PATCH 6/6] refactor --- app/models/participant_declaration.rb | 6 +++++ app/models/participant_profile/ect.rb | 4 +-- app/models/participant_profile/mentor.rb | 4 +-- .../shared_examples/archivable_support.rb | 26 ++++++++++--------- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/app/models/participant_declaration.rb b/app/models/participant_declaration.rb index d6e673f80a..15b8558fac 100644 --- a/app/models/participant_declaration.rb +++ b/app/models/participant_declaration.rb @@ -3,6 +3,8 @@ class ParticipantDeclaration < ApplicationRecord self.ignored_columns = %w[statement_type statement_id voided_at] + ARCHIVABLE_STATES = %w[ineligible voided submitted].freeze + belongs_to :cpd_lead_provider belongs_to :user belongs_to :cohort @@ -112,6 +114,10 @@ class ParticipantDeclaration < ApplicationRecord before_create :build_initial_declaration_state + def self.non_archivable_states + states.keys.excluding(ARCHIVABLE_STATES) + end + def voidable? %w[submitted eligible payable ineligible].include?(state) end diff --git a/app/models/participant_profile/ect.rb b/app/models/participant_profile/ect.rb index ee0a1008b3..5490b269a7 100644 --- a/app/models/participant_profile/ect.rb +++ b/app/models/participant_profile/ect.rb @@ -11,8 +11,6 @@ class ParticipantProfile::ECT < ParticipantProfile::ECF } def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) - archivable_states = %i[ineligible voided submitted].freeze - # ECTs that have no FIP induction records not_fip = InductionRecord.joins(:induction_programme, participant_profile: { schedule: :cohort }) .where.not(cohorts: { payments_frozen_at: nil }) @@ -29,7 +27,7 @@ def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) .where(induction_completion_date: nil) .where("induction_start_date IS NULL OR induction_start_date < make_date(cohorts.start_year, 9, 1)") .where.not(id: not_fip_ids) - .where.not(participant_declarations: { state: archivable_states }) + .where(participant_declarations: { state: ParticipantDeclaration.non_archivable_states }) .distinct with_unarchivable_declaration = with_unarchivable_declaration.where(id: restrict_to_participant_ids) if restrict_to_participant_ids.any? with_unarchivable_declaration_ids = with_unarchivable_declaration.pluck(:id) diff --git a/app/models/participant_profile/mentor.rb b/app/models/participant_profile/mentor.rb index 42c11fd1c1..8e2270a1ca 100644 --- a/app/models/participant_profile/mentor.rb +++ b/app/models/participant_profile/mentor.rb @@ -22,8 +22,6 @@ class ParticipantProfile::Mentor < ParticipantProfile::ECF } def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) - archivable_states = %i[ineligible voided submitted].freeze - # Mentors that have no FIP induction records not_fip = InductionRecord.joins(:induction_programme, participant_profile: { schedule: :cohort }) .where.not(cohorts: { payments_frozen_at: nil }) @@ -38,7 +36,7 @@ def self.archivable_from_frozen_cohort(restrict_to_participant_ids: []) .where.not(cohorts: { payments_frozen_at: nil }) .where(mentor_completion_date: nil) .where.not(id: not_fip_ids) - .where.not(participant_declarations: { state: archivable_states }) + .where(participant_declarations: { state: ParticipantDeclaration.non_archivable_states }) .distinct with_unarchivable_declaration = with_unarchivable_declaration.where(id: restrict_to_participant_ids) if restrict_to_participant_ids.any? with_unarchivable_declaration_ids = with_unarchivable_declaration.pluck(:id) diff --git a/spec/support/shared_examples/archivable_support.rb b/spec/support/shared_examples/archivable_support.rb index 09fb999155..a9aef821af 100644 --- a/spec/support/shared_examples/archivable_support.rb +++ b/spec/support/shared_examples/archivable_support.rb @@ -70,18 +70,20 @@ def build_declaration(attrs = {}) expect(participant_profile).not_to be_archivable_from_frozen_cohort end - it "returns false if the participant has not archivable declarations" do - paid_declaration = build_declaration(state: :paid, cohort: eligible_cohort) - participant_profile = paid_declaration.participant_profile - declaration_date = participant_profile.schedule.milestones.find_by(declaration_type: "retained-2").milestone_date - 1.day - travel_to declaration_date do - build_declaration(state: :voided, - cohort: eligible_cohort, - participant_profile:, - cpd_lead_provider: paid_declaration.cpd_lead_provider, - declaration_date:, - declaration_type: "retained-2") - expect(participant_profile).not_to be_archivable_from_frozen_cohort + ParticipantDeclaration.non_archivable_states.each do |declaration_state| + it "returns false if the participant has #{declaration_state} declarations" do + paid_declaration = build_declaration(state: declaration_state, cohort: eligible_cohort) + participant_profile = paid_declaration.participant_profile + declaration_date = participant_profile.schedule.milestones.find_by(declaration_type: "retained-2").milestone_date - 1.day + travel_to declaration_date do + build_declaration(state: :voided, + cohort: eligible_cohort, + participant_profile:, + cpd_lead_provider: paid_declaration.cpd_lead_provider, + declaration_date:, + declaration_type: "retained-2") + expect(participant_profile).not_to be_archivable_from_frozen_cohort + end end end