Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax validation on ChangeSchedule service for certain participants #4838

Merged
merged 5 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions app/models/participant_profile/ecf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ def self.ransackable_associations(_auth_object = nil)
%w[cohort participant_identity school user teacher_profile induction_records]
end

def self.eligible_to_change_cohort_and_continue_training(restrict_to_participant_ids: [])
def self.eligible_to_change_cohort_and_continue_training(cohort:, restrict_to_participant_ids: [])
billable_states = %w[eligible payable paid].freeze

return none unless cohort == Cohort.active_registration_cohort

completed_billable_declarations = ParticipantDeclaration.billable.for_declaration(:completed)
completed_billable_declarations = completed_billable_declarations.where(participant_profile_id: restrict_to_participant_ids) if restrict_to_participant_ids.any?

Expand All @@ -70,6 +72,14 @@ def self.eligible_to_change_cohort_and_continue_training(restrict_to_participant
query
end

def eligible_to_change_cohort_back_to_their_payments_frozen_original?(cohort:, current_cohort:)
return false unless cohort_changed_after_payments_frozen?
return false unless cohort.payments_frozen?
return false if participant_declarations.billable_or_changeable.where(cohort: current_cohort).exists?

participant_declarations.billable.where(cohort:).exists?
end

def self.archivable(restrict_to_participant_ids: [])
unbillable_states = %i[ineligible voided submitted].freeze

Expand All @@ -92,8 +102,8 @@ def self.archivable(restrict_to_participant_ids: [])
end

# Instance Methods
def eligible_to_change_cohort_and_continue_training?
self.class.eligible_to_change_cohort_and_continue_training(restrict_to_participant_ids: [id]).exists?
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?
end

def archivable?
Expand Down
4 changes: 2 additions & 2 deletions app/models/participant_profile/ect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def role
"Early career teacher"
end

def self.eligible_to_change_cohort_and_continue_training(restrict_to_participant_ids: [])
super(restrict_to_participant_ids:).where(induction_completion_date: nil)
def self.eligible_to_change_cohort_and_continue_training(cohort:, restrict_to_participant_ids: [])
super(cohort:, restrict_to_participant_ids:).where(induction_completion_date: nil)
end
end
4 changes: 2 additions & 2 deletions app/models/participant_profile/mentor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def role
"Mentor"
end

def self.eligible_to_change_cohort_and_continue_training(restrict_to_participant_ids: [])
super(restrict_to_participant_ids:).where(mentor_completion_date: nil)
def self.eligible_to_change_cohort_and_continue_training(cohort:, restrict_to_participant_ids: [])
super(cohort:, restrict_to_participant_ids:).where(mentor_completion_date: nil)
end
end
26 changes: 25 additions & 1 deletion app/services/change_schedule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class ChangeSchedule
attribute :course_identifier
attribute :schedule_identifier
attribute :cohort, :integer
attribute :allow_change_to_from_frozen_cohort, :boolean, default: false

delegate :participant_profile_state, to: :participant_profile, allow_nil: true
delegate :lead_provider, to: :cpd_lead_provider, allow_nil: true
Expand Down Expand Up @@ -80,6 +81,14 @@ def cohort

private

def changing_cohort_due_to_payments_frozen?
return false unless participant_profile.ecf?
return false unless allow_change_to_from_frozen_cohort
return true if participant_profile.eligible_to_change_cohort_and_continue_training?(cohort:)

participant_profile.eligible_to_change_cohort_back_to_their_payments_frozen_original?(cohort:, current_cohort:)
end

def user
@user ||= participant_identity&.user
end
Expand Down Expand Up @@ -114,6 +123,18 @@ def update_school_cohort_and_schedule!
participant_profile.update!(school_cohort: target_school_cohort, schedule: new_schedule)
end

def update_cohort_changed_after_payments_frozen!
changing_cohort = current_cohort.start_year != cohort&.start_year

if changing_cohort && changing_cohort_due_to_payments_frozen?
participant_profile.toggle(:cohort_changed_after_payments_frozen).save!
end
end

def current_cohort
relevant_induction_record.schedule.cohort
end

def update_induction_records!
return unless relevant_induction_record

Expand All @@ -129,6 +150,7 @@ def update_induction_records!
def update_ecf_records!
return unless participant_profile&.ecf?

update_cohort_changed_after_payments_frozen!
update_school_cohort_and_schedule!
update_induction_records!
end
Expand Down Expand Up @@ -191,6 +213,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 changing_cohort_due_to_payments_frozen?

applicable_declarations.each do |declaration|
milestone = new_schedule.milestones.find_by!(declaration_type: declaration.declaration_type)
Expand Down Expand Up @@ -220,10 +243,11 @@ def validate_permitted_schedule_for_course

def validate_cannot_change_cohort_ecf
return unless participant_profile&.ecf?
return if changing_cohort_due_to_payments_frozen?

if applicable_declarations.any? &&
relevant_induction_record &&
relevant_induction_record.schedule.cohort.start_year != cohort&.start_year
current_cohort.start_year != cohort&.start_year
errors.add(:cohort, I18n.t("cannot_change_cohort"))
end
end
Expand Down
143 changes: 135 additions & 8 deletions spec/services/change_schedule_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,90 @@
end
end

RSpec.shared_examples "changing cohort and continuing training" do
%i[eligible payable paid].each do |state|
context "when there are #{state} declarations" do
let(:cohort) { new_cohort.previous }
let(:new_cohort) { Cohort.active_registration_cohort }

before { create(:participant_declaration, participant_profile:, cohort:, state:, course_identifier:, cpd_lead_provider:) }

it { expect(new_cohort).not_to eq(cohort) }

context "allow_change_to_from_frozen_cohort is true" do
let(:allow_change_to_from_frozen_cohort) { true }

context "when the participant is not eligible to transfer and continue training" do
it { is_expected.to be_invalid }
end

context "when the participant is eligible to transfer and continue training" do
before { 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 }
it { expect { service.call }.to change { participant_profile.reload.cohort_changed_after_payments_frozen }.to(true) }
end
end

context "when allow_change_to_from_frozen_cohort is false" do
let(:allow_change_to_from_frozen_cohort) { false }

context "when the participant is eligible to transfer and continue training" do
before { cohort.update!(payments_frozen_at: Time.zone.now) }

it { is_expected.to be_invalid }
end
end
end
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 }

expect(participant_profile.reload).not_to be_cohort_changed_after_payments_frozen
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,
}))

travel_to(Date.new(new_cohort.start_year).end_of_year) do
create(:participant_declaration,
participant_profile: participant_profile.reload,
declaration_type: "retained-1",
state: :payable,
course_identifier:,
cpd_lead_provider:,
cohort: new_cohort)
end

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
Expand Down Expand Up @@ -122,6 +206,7 @@
participant_id:,
course_identifier:,
schedule_identifier:,
allow_change_to_from_frozen_cohort:,
}
end
let(:participant_identity) { create(:participant_identity) }
Expand All @@ -133,10 +218,12 @@

context "ECT participant profile" do
let(:cpd_lead_provider) { create(:cpd_lead_provider, :with_lead_provider) }
let(:participant_profile) { create(:ect, lead_provider: cpd_lead_provider.lead_provider, user:) }
let(:cohort) { Cohort.current }
let(:participant_profile) { create(:ect, lead_provider: cpd_lead_provider.lead_provider, user:, cohort:) }
let(:schedule_identifier) { "ecf-standard-september" }
let(:course_identifier) { "ecf-induction" }
let!(:schedule) { Finance::Schedule::ECF.find_by(schedule_identifier: "ecf-standard-september") }
let(:allow_change_to_from_frozen_cohort) { false }
let!(:schedule) { Finance::Schedule::ECF.find_by(schedule_identifier: "ecf-standard-september", cohort:) }
let(:new_cohort) { Cohort.previous }
let!(:new_schedule) { create(:ecf_schedule, cohort: new_cohort, schedule_identifier: "ecf-replacement-april") }

Expand All @@ -149,17 +236,20 @@

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:,
participant_id:,
course_identifier:,
schedule_identifier:,
cohort: new_cohort.start_year,
allow_change_to_from_frozen_cohort:,
}
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:) }
Expand All @@ -174,6 +264,10 @@
end
end

it_behaves_like "changing cohort and continuing training" do
let!(:new_schedule) { Finance::Schedule::ECF.find_by(cohort: new_cohort, schedule_identifier:) }
end

context "when there are no submitted/eligible/payable/paid declarations" do
context "when changing to another cohort" do
describe ".call" do
Expand Down Expand Up @@ -298,6 +392,7 @@
participant_id:,
course_identifier:,
schedule_identifier:,
allow_change_to_from_frozen_cohort:,
cohort: new_cohort.start_year,
}
end
Expand Down Expand Up @@ -336,6 +431,7 @@
participant_id:,
course_identifier:,
schedule_identifier: new_schedule.schedule_identifier,
allow_change_to_from_frozen_cohort:,
cohort:,
}
end
Expand Down Expand Up @@ -381,11 +477,13 @@

context "Mentor participant profile" do
let(:cpd_lead_provider) { create(:cpd_lead_provider, :with_lead_provider) }
let!(:participant_profile) { create(:mentor, lead_provider: cpd_lead_provider.lead_provider, user:) }
let(:cohort) { Cohort.current }
let!(:participant_profile) { create(:mentor, lead_provider: cpd_lead_provider.lead_provider, user:, cohort:) }
let(:schedule_identifier) { "ecf-extended-april" }
let(:course_identifier) { "ecf-mentor" }
let(:allow_change_to_from_frozen_cohort) { false }
let(:new_cohort) { Cohort.previous }
let!(:schedule) { create(:ecf_mentor_schedule, schedule_identifier: "ecf-extended-april") }
let!(:schedule) { create(:ecf_mentor_schedule, cohort:, schedule_identifier: "ecf-extended-april") }

describe "validations" do
it_behaves_like "validating a participant for a change schedule"
Expand All @@ -395,18 +493,21 @@
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
{
cpd_lead_provider:,
participant_id:,
course_identifier:,
schedule_identifier:,
allow_change_to_from_frozen_cohort:,
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:) }
Expand All @@ -421,6 +522,10 @@
end
end

it_behaves_like "changing cohort and continuing training" do
let!(:new_schedule) { create(:ecf_mentor_schedule, cohort: new_cohort, schedule_identifier:) }
end

context "when there are no submitted/eligible/payable/paid declarations" do
context "when changing to another cohort" do
describe ".call" do
Expand Down Expand Up @@ -539,10 +644,12 @@
let(:cpd_lead_provider) { create(:cpd_lead_provider, :with_npq_lead_provider) }
let(:npq_lead_provider) { cpd_lead_provider.npq_lead_provider }
let(:npq_course) { create(:npq_course, identifier: "npq-senior-leadership") }
let(:schedule) { Finance::Schedule::NPQ.find_by(cohort: Cohort.current, schedule_identifier: "npq-specialist-spring") }
let(:participant_profile) { create(:npq_participant_profile, npq_lead_provider:, npq_course:, schedule:, user:) }
let(:schedule) { Finance::Schedule::NPQ.find_by(cohort:, schedule_identifier: "npq-specialist-spring") }
let(:cohort) { Cohort.current }
let(:participant_profile) { create(:npq_participant_profile, npq_lead_provider:, npq_course:, schedule:, user:, cohort:) }
let(:course_identifier) { npq_course.identifier }
let(:schedule_identifier) { new_schedule.schedule_identifier }
let(:allow_change_to_from_frozen_cohort) { 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:) }
Expand All @@ -564,9 +671,12 @@
course_identifier:,
schedule_identifier:,
cohort: new_cohort.start_year,
allow_change_to_from_frozen_cohort:,
}
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:) }
Expand All @@ -583,6 +693,23 @@
end
end

%i[eligible payable paid].each do |state|
context "when there are #{state} declarations" do
let(:new_cohort) { Cohort.active_registration_cohort }
let!(:new_schedule) { create(:npq_leadership_schedule, cohort: new_cohort) }

before { create(:participant_declaration, participant_profile:, state:, course_identifier:, cpd_lead_provider:) }

context "allow_change_to_from_frozen_cohort is true" do
let(:allow_change_to_from_frozen_cohort) { 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 }
Expand Down
Loading
Loading