From 81ddb5afab987f4e89ff43a0bb6eef4ba8afd3dd Mon Sep 17 00:00:00 2001 From: Ross Oliver Date: Wed, 22 May 2024 10:15:35 +0100 Subject: [PATCH] Relax validation on ChangeSchedule service for certain participants We want to allow ECTs and mentors to change schedule/cohort even if they have billable declarations under certain scenarios. The participants eligible are identified by `eligible_to_change_cohort_and_continue_training` when passed the cohort they intend to transfer to. By default the service will not allow eligible participants to transfer; it needs to be called explicitly with `attempt_to_transfer_leaving_billable_declarations` set to `true`. This is to prevent lead providers performing these changes for now. Participants that change cohort using this mechanism should be able to change back to their original cohort providing no billable declarations were created against the new cohort. --- app/services/change_schedule.rb | 22 ++++- spec/services/change_schedule_spec.rb | 131 +++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 3 deletions(-) diff --git a/app/services/change_schedule.rb b/app/services/change_schedule.rb index 15a4ee7af13..99d467a5939 100644 --- a/app/services/change_schedule.rb +++ b/app/services/change_schedule.rb @@ -9,6 +9,7 @@ class ChangeSchedule attribute :course_identifier attribute :schedule_identifier attribute :cohort, :integer + attribute :attempt_to_change_cohort_leaving_billable_declarations, :boolean, default: false delegate :participant_profile_state, to: :participant_profile, allow_nil: true delegate :lead_provider, to: :cpd_lead_provider, allow_nil: true @@ -80,6 +81,17 @@ def cohort private + def can_change_cohort_leaving_billable_declarations? + return false unless attempt_to_change_cohort_leaving_billable_declarations && participant_profile.ecf? + + return true if participant_profile.eligible_to_change_cohort_and_continue_training? + + # Allow participants to change back to their original cohort + # if they have no billable declarations in the new cohort. + billable_declarations_in_current_cohort = applicable_declarations.where(cohort: participant_profile.cohort) + participant_profile.cohort_changed_after_payments_frozen? && billable_declarations_in_current_cohort.none? + end + def user @user ||= participant_identity&.user end @@ -111,6 +123,12 @@ def update_participant_profile_schedule_references! end def update_school_cohort_and_schedule! + changing_cohort = new_schedule.cohort != participant_profile.schedule.cohort + + if changing_cohort && can_change_cohort_leaving_billable_declarations? + participant_profile.toggle!(:cohort_changed_after_payments_frozen) + end + participant_profile.update!(school_cohort: target_school_cohort, schedule: new_schedule) end @@ -165,6 +183,7 @@ def not_already_withdrawn def validate_new_schedule_valid_with_existing_declarations return if user.blank? || participant_profile.blank? return unless new_schedule + return if can_change_cohort_leaving_billable_declarations? applicable_declarations.each do |declaration| milestone = new_schedule.milestones.find_by!(declaration_type: declaration.declaration_type) @@ -194,6 +213,7 @@ def validate_permitted_schedule_for_course def validate_cannot_change_cohort_ecf return unless participant_profile&.ecf? + return if can_change_cohort_leaving_billable_declarations? if applicable_declarations.any? && relevant_induction_record && @@ -212,7 +232,7 @@ def validate_cannot_change_cohort_npq def change_with_a_different_schedule return unless new_schedule && participant_profile && new_schedule == participant_profile.schedule - + return if relevant_induction_record_has_different_schedule errors.add(:schedule_identifier, I18n.t(:schedule_already_on_the_profile)) diff --git a/spec/services/change_schedule_spec.rb b/spec/services/change_schedule_spec.rb index 830a628138d..00a2512051d 100644 --- a/spec/services/change_schedule_spec.rb +++ b/spec/services/change_schedule_spec.rb @@ -64,6 +64,46 @@ end end +RSpec.shared_examples "reversing a change of schedule/cohort" do + it "allows a change of schedule to be reversed" do + original_cohort = participant_profile.schedule.cohort + + first_change_of_schedule = described_class.new(params) + expect(first_change_of_schedule).to be_valid + expect { first_change_of_schedule.call }.to change { ParticipantProfileSchedule.count } + + second_change_of_schedule = described_class.new(params.merge({ + cohort: original_cohort.start_year, + })) + expect(second_change_of_schedule).to be_valid + expect { second_change_of_schedule.call }.to change { ParticipantProfileSchedule.count } + end + + it "does not allow a change of schedule to be reversed if there are billable declarations in the new cohort" do + original_cohort = participant_profile.schedule.cohort + + first_change_of_schedule = described_class.new(params) + expect(first_change_of_schedule).to be_valid + expect { first_change_of_schedule.call }.to change { ParticipantProfileSchedule.count } + + second_change_of_schedule = described_class.new(params.merge({ + cohort: original_cohort.start_year, + })) + + create(:participant_declaration, + participant_profile: participant_profile.reload, + declaration_type: "retained-1", + state: :payable, + course_identifier:, + cpd_lead_provider:, + cohort: new_cohort, + ) + + expect(second_change_of_schedule).not_to be_valid + expect(second_change_of_schedule.errors.messages_for(:cohort)).to include("The property '#/cohort' cannot be changed") + end +end + RSpec.shared_examples "validating a participant is not already withdrawn for a change schedule" do it "is invalid and returns an error message" do is_expected.to be_invalid @@ -122,6 +162,7 @@ participant_id:, course_identifier:, schedule_identifier:, + attempt_to_change_cohort_leaving_billable_declarations:, } end let(:participant_identity) { create(:participant_identity) } @@ -136,6 +177,7 @@ let(:participant_profile) { create(:ect, lead_provider: cpd_lead_provider.lead_provider, user:) } let(:schedule_identifier) { "ecf-standard-september" } let(:course_identifier) { "ecf-induction" } + let(:attempt_to_change_cohort_leaving_billable_declarations) { false } let!(:schedule) { Finance::Schedule::ECF.find_by(schedule_identifier: "ecf-standard-september") } let(:new_cohort) { Cohort.previous } let!(:new_schedule) { create(:ecf_schedule, cohort: new_cohort, schedule_identifier: "ecf-replacement-april") } @@ -149,7 +191,7 @@ context "when the cohort is changing" do let!(:new_schedule) { Finance::Schedule::ECF.find_by(schedule_identifier:, cohort: new_cohort) } - let!(:new_school_cohort) { create(:school_cohort, :cip, :with_induction_programme, cohort: new_cohort, lead_provider: cpd_lead_provider.lead_provider, school: participant_profile.school) } + let!(:new_school_cohort) { create(:school_cohort, :fip, :with_induction_programme, cohort: new_cohort, lead_provider: cpd_lead_provider.lead_provider, school: participant_profile.school) } let(:params) do { cpd_lead_provider:, @@ -157,9 +199,12 @@ course_identifier:, schedule_identifier:, cohort: new_cohort.start_year, + attempt_to_change_cohort_leaving_billable_declarations:, } end + it_behaves_like "reversing a change of schedule/cohort" + %i[submitted eligible payable paid].each do |state| context "when there are #{state} declarations" do before { create(:participant_declaration, participant_profile:, state:, course_identifier:, cpd_lead_provider:) } @@ -174,6 +219,35 @@ end end + %i[eligible payable paid].each do |state| + context "when there are #{state} declarations" do + before { create(:participant_declaration, participant_profile:, state:, course_identifier:, cpd_lead_provider:) } + + context "attempt_to_change_cohort_leaving_billable_declarations is true" do + let(:attempt_to_change_cohort_leaving_billable_declarations) { true } + + context "when the participant is not eligible to transfer and continue training" do + it { is_expected.not_to be_valid } + end + + context "when the participant is eligible to transfer and continue training" do + before { participant_profile.schedule.cohort.update!(payments_frozen_at: Time.zone.now) } + + it_behaves_like "changing the schedule of a participant" + it_behaves_like "reversing a change of schedule/cohort" + + it { is_expected.to be_valid } + + context "when attempt_to_change_cohort_leaving_billable_declarations is false and the participant is eligible to transfer and continue training" do + let(:attempt_to_change_cohort_leaving_billable_declarations) { false } + + it { is_expected.not_to be_valid } + end + end + end + end + end + context "when there are no submitted/eligible/payable/paid declarations" do context "when changing to another cohort" do describe ".call" do @@ -298,6 +372,7 @@ participant_id:, course_identifier:, schedule_identifier:, + attempt_to_change_cohort_leaving_billable_declarations:, cohort: new_cohort.start_year, } end @@ -336,6 +411,7 @@ participant_id:, course_identifier:, schedule_identifier: new_schedule.schedule_identifier, + attempt_to_change_cohort_leaving_billable_declarations:, cohort:, } end @@ -384,6 +460,7 @@ let!(:participant_profile) { create(:mentor, lead_provider: cpd_lead_provider.lead_provider, user:) } let(:schedule_identifier) { "ecf-extended-april" } let(:course_identifier) { "ecf-mentor" } + let(:attempt_to_change_cohort_leaving_billable_declarations) { false } let(:new_cohort) { Cohort.previous } let!(:schedule) { create(:ecf_mentor_schedule, schedule_identifier: "ecf-extended-april") } @@ -395,7 +472,7 @@ end context "when the cohort is changing" do - let!(:new_school_cohort) { create(:school_cohort, :cip, :with_induction_programme, cohort: new_cohort, lead_provider: cpd_lead_provider.lead_provider, school: participant_profile.school) } + let!(:new_school_cohort) { create(:school_cohort, :fip, :with_induction_programme, cohort: new_cohort, lead_provider: cpd_lead_provider.lead_provider, school: participant_profile.school) } let!(:new_schedule) { create(:ecf_mentor_schedule, schedule_identifier:, cohort: new_cohort) } let(:params) do { @@ -403,10 +480,13 @@ participant_id:, course_identifier:, schedule_identifier:, + attempt_to_change_cohort_leaving_billable_declarations:, cohort: new_cohort.start_year, } end + it_behaves_like "reversing a change of schedule/cohort" + %i[submitted eligible payable paid].each do |state| context "when there are #{state} declarations" do before { create(:participant_declaration, participant_profile:, state:, course_identifier:, cpd_lead_provider:) } @@ -421,6 +501,35 @@ end end + %i[eligible payable paid].each do |state| + context "when there are #{state} declarations" do + before { create(:participant_declaration, participant_profile:, state:, course_identifier:, cpd_lead_provider:) } + + context "attempt_to_change_cohort_leaving_billable_declarations is true" do + let(:attempt_to_change_cohort_leaving_billable_declarations) { true } + + context "when the participant is not eligible to transfer and continue training" do + it { is_expected.not_to be_valid } + end + + context "when the participant is eligible to transfer and continue training" do + before { participant_profile.schedule.cohort.update!(payments_frozen_at: Time.zone.now) } + + it_behaves_like "changing the schedule of a participant" + it_behaves_like "reversing a change of schedule/cohort" + + it { is_expected.to be_valid } + end + + context "when attempt_to_change_cohort_leaving_billable_declarations is false and the participant is eligible to transfer and continue training" do + let(:attempt_to_change_cohort_leaving_billable_declarations) { false } + + it { is_expected.not_to be_valid } + end + end + end + end + context "when there are no submitted/eligible/payable/paid declarations" do context "when changing to another cohort" do describe ".call" do @@ -543,6 +652,7 @@ let(:participant_profile) { create(:npq_participant_profile, npq_lead_provider:, npq_course:, schedule:, user:) } let(:course_identifier) { npq_course.identifier } let(:schedule_identifier) { new_schedule.schedule_identifier } + let(:attempt_to_change_cohort_leaving_billable_declarations) { false } let(:new_cohort) { Cohort.previous } let(:new_schedule) { Finance::Schedule::NPQ.find_by(cohort: new_cohort, schedule_identifier: "npq-leadership-spring") } let!(:npq_contract) { create(:npq_contract, :npq_senior_leadership, npq_lead_provider:, npq_course:) } @@ -564,9 +674,12 @@ course_identifier:, schedule_identifier:, cohort: new_cohort.start_year, + attempt_to_change_cohort_leaving_billable_declarations:, } end + it_behaves_like "reversing a change of schedule/cohort" + %i[submitted eligible payable paid].each do |state| context "when there are #{state} declarations" do before { create(:participant_declaration, participant_profile:, state:, course_identifier:, cpd_lead_provider:) } @@ -583,6 +696,20 @@ end end + %i[eligible payable paid].each do |state| + context "when there are #{state} declarations" do + before { create(:participant_declaration, participant_profile:, state:, course_identifier:, cpd_lead_provider:) } + + context "attempt_to_change_cohort_leaving_billable_declarations is true" do + let(:attempt_to_change_cohort_leaving_billable_declarations) { true } + + before { participant_profile.schedule.cohort.update!(payments_frozen_at: Time.zone.now) } + + it { is_expected.not_to be_valid } + end + end + end + context "when there are no submitted/eligible/payable/paid declarations" do context "when changing to another cohort" do let(:new_cohort) { Cohort.previous }