diff --git a/app/models/application.rb b/app/models/application.rb index e67464c252..14a26a28b6 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -72,6 +72,13 @@ class Application < ApplicationRecord withdrawn: "withdrawn", } + validates :funded_place, + inclusion: { in: [true, false] }, + if: :validate_funded_place?, + on: :npq_separation + validate :eligible_for_funded_place, on: :npq_separation + validate :validate_permitted_schedule_for_course, on: :npq_separation + # `eligible_for_dfe_funding?` takes into consideration what we know # about user eligibility plus if it has been previously funded. We need # to keep this method in place to keep consistency during the split between @@ -182,4 +189,26 @@ def funding_eligibility(with_funded_place:) def schedule_cohort_matches errors.add(:schedule, :cohort_mismatch) if schedule && schedule.cohort != cohort end + + def validate_funded_place? + accepted? && errors.blank? && cohort&.funding_cap? + end + + def eligible_for_funded_place + return if errors.any? + return unless cohort&.funding_cap? + + if funded_place && !eligible_for_funding + errors.add(:funded_place, :not_eligible) + end + end + + def validate_permitted_schedule_for_course + return if errors.any? + return unless accepted? && schedule && course + + unless schedule.course_group.courses.include?(course) + errors.add(:schedule, :invalid_for_course) + end + end end diff --git a/app/models/declaration.rb b/app/models/declaration.rb index 5e129751d9..405b804b62 100644 --- a/app/models/declaration.rb +++ b/app/models/declaration.rb @@ -58,6 +58,8 @@ class Declaration < ApplicationRecord }, _suffix: true validates :declaration_date, :declaration_type, presence: true + validate :validate_declaration_date_within_schedule + validate :validate_declaration_date_not_in_the_future def billable_statement statement_items.find(&:billable?)&.statement @@ -108,4 +110,19 @@ def duplicate_declarations application: { course: application.course.rebranded_alternative_courses }, ) end + +private + + def validate_declaration_date_within_schedule + return unless application&.schedule + return unless declaration_date + + if declaration_date < application.schedule.applies_from + errors.add(:declaration_date, :declaration_before_schedule_start) + end + end + + def validate_declaration_date_not_in_the_future + errors.add(:declaration_date, :future_declaration_date) if declaration_date&.future? + end end diff --git a/app/services/applications/accept.rb b/app/services/applications/accept.rb index 853b72156f..5939a68c39 100644 --- a/app/services/applications/accept.rb +++ b/app/services/applications/accept.rb @@ -24,7 +24,6 @@ def accept accept_application! create_application_state! reject_other_applications_in_same_cohort! - set_funded_place_on_npq_application! end true @@ -52,10 +51,16 @@ def other_accepted_applications_with_same_course? end def accept_application! - application.update!( + opts = { lead_provider_approval_status: "accepted", schedule:, - ) + } + + if cohort&.funding_cap? + opts[:funded_place] = funded_place + end + + application.update!(opts) end def reject_other_applications_in_same_cohort! @@ -92,12 +97,6 @@ def same_trn_users .where.not(id: user.id) end - def set_funded_place_on_npq_application! - return unless cohort&.funding_cap? - - application.update!(funded_place:) - end - def eligible_for_funded_place return if errors.any? return unless cohort&.funding_cap? diff --git a/app/services/valid_test_data_generators/applications_populater.rb b/app/services/valid_test_data_generators/applications_populater.rb index ac14eb3343..efba2034eb 100644 --- a/app/services/valid_test_data_generators/applications_populater.rb +++ b/app/services/valid_test_data_generators/applications_populater.rb @@ -39,9 +39,7 @@ def initialize(lead_provider:, cohort:, number_of_participants:, logger: Rails.l def create_participants! number_of_participants.times do - travel_to(rand(3.years.ago..Time.zone.now)) do - create_participant(school: School.open.order("RANDOM()").first) - end + create_participant(school: School.open.order("RANDOM()").first) end end @@ -55,32 +53,37 @@ def prepare_cohort! def create_participant(school:) course = courses.sample user = create_user - application = create_application(user, school, course) + schedule = Schedule.where(cohort:, course_group: course.course_group).sample + application = create_application(user, school, course, schedule) return if Faker::Boolean.boolean(true_ratio: 0.3) - accept_application(application) - create_participant_id_change(application) - create_declarations(application) - create_outcomes(application) - void_completed_declaration_for(application) + current_date = schedule.applies_from + rand(1..100) - return if Faker::Boolean.boolean(true_ratio: 0.3) + travel_to(current_date) do + accept_application(application) + create_participant_id_change(application) + create_declarations(application) + create_outcomes(application) + void_completed_declaration_for(application) - if Faker::Boolean.boolean(true_ratio: 0.5) - defer_application(application) - else - withdrawn_application(application) - end + return if Faker::Boolean.boolean(true_ratio: 0.3) - return if Faker::Boolean.boolean(true_ratio: 0.3) + if Faker::Boolean.boolean(true_ratio: 0.5) + defer_application(application) + else + withdrawn_application(application) + end - course = courses.reject { |c| c.identifier == course.identifier }.sample - application = create_application(user, school, course) + return if Faker::Boolean.boolean(true_ratio: 0.3) - return if Faker::Boolean.boolean(true_ratio: 0.3) + course = courses.reject { |c| c.identifier == course.identifier }.sample + application = create_application(user, school, course, schedule) - reject_application(application) + return if Faker::Boolean.boolean(true_ratio: 0.3) + + reject_application(application) + end end def create_participant_id_change(application) @@ -100,7 +103,7 @@ def create_user trn_auto_verified: true) end - def create_application(user, school, course) + def create_application(user, school, course, schedule) FactoryBot.create(:application, :pending, :with_random_work_setting, @@ -109,6 +112,7 @@ def create_application(user, school, course) user:, cohort:, course:, + schedule:, headteacher_status: Application.headteacher_statuses.keys.sample, eligible_for_funding: Faker::Boolean.boolean, itt_provider: IttProvider.currently_approved.order("RANDOM()").first) @@ -119,7 +123,11 @@ def accept_application(application) application.eligible_for_funding? ? Faker::Boolean.boolean(true_ratio: 0.7) : false end - Applications::Accept.new(application:, funded_place:).accept + Applications::Accept.new( + application:, + funded_place:, + schedule_identifier: application.schedule.identifier, + ).accept application.reload end diff --git a/config/locales/en.yml b/config/locales/en.yml index 30b7fe888c..dea11115af 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -38,9 +38,11 @@ en: schedule: &schedule cohort_mismatch: The schedule cohort must match the application cohort + invalid_for_course: Selected schedule is not valid for the course funded_place: &funded_place inclusion: Set '#/%{parameterized_attribute}' to true or false. + not_eligible: "The participant is not eligible for funding, so '#/funded_place' cannot be set to true." participant_id: &participant_id blank: "The property '#/participant_id' must be present" @@ -102,6 +104,7 @@ en: time: formats: admin: "%R on %d/%m/%Y" + activerecord: errors: models: @@ -109,6 +112,19 @@ en: attributes: schedule: <<: *schedule + funded_place: + <<: *funded_place + declaration: + attributes: + declaration_date: + *declaration_date + declaration_type: + *declaration_type + participant_outcome: + attributes: + completion_date: + *completion_date + activemodel: attributes: questionnaires/funding_your_npq: diff --git a/db/seeds/base/add_declarations.rb b/db/seeds/base/add_declarations.rb index d4994ae8f3..943071cfac 100644 --- a/db/seeds/base/add_declarations.rb +++ b/db/seeds/base/add_declarations.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +helpers = Class.new { include ActiveSupport::Testing::TimeHelpers }.new + LeadProvider.find_each do |lead_provider| quantity = { "review" => 4, "development" => 1 }.fetch(Rails.env, 0) @@ -15,12 +17,14 @@ cohort: Cohort.all.sample, ) %w[started].each do |declaration_type| - FactoryBot.create( - :declaration, - :submitted_or_eligible, - application: application1, - declaration_type:, - ) + helpers.travel_to application1.schedule.applies_from do + FactoryBot.create( + :declaration, + :submitted_or_eligible, + application: application1, + declaration_type:, + ) + end end # Application with a started and retained-1 declaration @@ -34,12 +38,14 @@ cohort: Cohort.all.sample, ) %w[started retained-1].each do |declaration_type| - FactoryBot.create( - :declaration, - :submitted_or_eligible, - application: application2, - declaration_type:, - ) + helpers.travel_to application2.schedule.applies_from do + FactoryBot.create( + :declaration, + :submitted_or_eligible, + application: application2, + declaration_type:, + ) + end end # Application with a started, retained-1 and retained-2 declaration @@ -53,12 +59,14 @@ cohort: Cohort.all.sample, ) %w[started retained-1 retained-2].each do |declaration_type| - FactoryBot.create( - :declaration, - :submitted_or_eligible, - application: application3, - declaration_type:, - ) + helpers.travel_to application3.schedule.applies_from do + FactoryBot.create( + :declaration, + :submitted_or_eligible, + application: application3, + declaration_type:, + ) + end end # Application with a started, retained-1, retained-2 and completed declaration @@ -72,20 +80,24 @@ cohort: Cohort.all.sample, ) %w[started retained-1 retained-2 completed].each do |declaration_type| - declaration = FactoryBot.create( - :declaration, - :submitted_or_eligible, - application: application4, - declaration_type:, - ) + declaration = helpers.travel_to(application4.schedule.applies_from) do + FactoryBot.create( + :declaration, + :submitted_or_eligible, + application: application4, + declaration_type:, + ) + end next unless declaration_type == "completed" ParticipantOutcomes::Create::STATES.reverse.each do |state| - FactoryBot.create(:participant_outcome, - declaration:, - state:, - completion_date: declaration.declaration_date.to_s) + helpers.travel_to declaration.declaration_date do + FactoryBot.create(:participant_outcome, + declaration:, + state:, + completion_date: declaration.declaration_date.to_s) + end break if Faker::Boolean.boolean(true_ratio: 0.3) end diff --git a/spec/factories/applications.rb b/spec/factories/applications.rb index 9037b5762a..fccebb80b0 100644 --- a/spec/factories/applications.rb +++ b/spec/factories/applications.rb @@ -5,12 +5,12 @@ with_school user - course + course { create(Course::IDENTIFIERS.sample.to_sym) } lead_provider { LeadProvider.all.sample } headteacher_status { "no" } lead_provider_approval_status { :pending } ecf_id { SecureRandom.uuid } - cohort + cohort { create(:cohort, :current) } teacher_catchment { "england" } teacher_catchment_country { "United Kingdom of Great Britain and Northern Ireland" } teacher_catchment_iso_country_code { "GBR" } @@ -57,6 +57,7 @@ trait :accepted do lead_provider_approval_status { :accepted } schedule { Schedule.find_by(cohort:, course_group: course.course_group) || create(:schedule, course_group: course.course_group, cohort:) } + funded_place { !!eligible_for_funding } end trait :rejected do @@ -74,7 +75,6 @@ trait :eligible_for_funded_place do accepted eligible_for_funding - funded_place { true } after(:create) do |application| application.cohort.update!(funding_cap: true) diff --git a/spec/factories/schedules.rb b/spec/factories/schedules.rb index 3cf85c3daa..f6b7b3df7b 100644 --- a/spec/factories/schedules.rb +++ b/spec/factories/schedules.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :schedule do course_group - cohort + cohort { create(:cohort, :current) } sequence(:name) { |n| "Schedule #{n}" } sequence(:identifier) { |n| "schedule-#{n}" } diff --git a/spec/models/application_spec.rb b/spec/models/application_spec.rb index b09bd39de5..2952e40d20 100644 --- a/spec/models/application_spec.rb +++ b/spec/models/application_spec.rb @@ -26,6 +26,39 @@ it { is_expected.to have_error(:schedule, :cohort_mismatch, "The schedule cohort must match the application cohort") } end + + context "when accepted" do + let(:cohort) { create(:cohort, :current, :with_funding_cap) } + + context "when funding_cap" do + subject { build(:application, :accepted, cohort:) } + + it "validates funded_place boolean" do + subject.funded_place = nil + + expect(subject).to have_error(:funded_place, :inclusion, "Set '#/funded_place' to true or false.", :npq_separation) + end + + it "validates funded_place eligibility" do + subject.funded_place = true + subject.eligible_for_funding = false + + expect(subject).to have_error(:funded_place, :not_eligible, "The participant is not eligible for funding, so '#/funded_place' cannot be set to true.", :npq_separation) + end + end + + context "when changing to wrong schedule" do + let(:new_schedule) { create(:schedule, cohort:) } + + subject { create(:application, :accepted, cohort:) } + + it "returns validation error" do + subject.schedule = new_schedule + + expect(subject).to have_error(:schedule, :invalid_for_course, "Selected schedule is not valid for the course", :npq_separation) + end + end + end end describe "enums" do @@ -240,7 +273,10 @@ end context "when the application has not been previously funded (previous application is not for a rebranded alternative course)" do - before { previous_application.update!(course: Course.npqeyl) } + before do + previous_application.schedule.course_group.courses << Course.npqeyl + previous_application.update!(course: Course.npqeyl) + end it { is_expected.not_to be_previously_funded } end diff --git a/spec/models/declaration_spec.rb b/spec/models/declaration_spec.rb index fb73b2d620..c6aac0162c 100644 --- a/spec/models/declaration_spec.rb +++ b/spec/models/declaration_spec.rb @@ -16,6 +16,39 @@ describe "validations" do it { is_expected.to validate_presence_of(:declaration_type) } it { is_expected.to validate_presence_of(:declaration_date) } + + context "when the declaration_date is in the future" do + before { subject.declaration_date = 1.day.from_now } + + it "has an error on create" do + expect(subject.save).to be_falsey + expect(subject).to have_error(:declaration_date, :future_declaration_date, "The '#/declaration_date' value cannot be a future date. Check the date and try again.") + end + + it "has an error on update" do + subject.declaration_date = 1.day.ago + expect(subject.save).to be_truthy + + subject.declaration_date = 1.day.from_now + expect(subject.save).to be_falsey + expect(subject).to have_error(:declaration_date, :future_declaration_date, "The '#/declaration_date' value cannot be a future date. Check the date and try again.") + end + end + + context "when declaration_date is before the schedule start" do + before { subject.declaration_date = subject.application.schedule.applies_from.prev_week } + + it "has a meaningful error" do + expect(subject).to be_invalid + expect(subject).to have_error(:declaration_date, :declaration_before_schedule_start, "Enter a '#/declaration_date' that's on or after the schedule start.") + end + end + + context "when declaration_date is at the schedule start" do + before { subject.declaration_date = subject.application.schedule.applies_from } + + it { is_expected.to be_valid } + end end describe "delegations" do @@ -296,7 +329,7 @@ before do Course::IDENTIFIERS.excluding(course_identifier).each do |identifier| - create(:declaration).application.update!(course: Course.find_by(identifier:)) + create(:declaration, course: Course.find_by(identifier:)) end end diff --git a/spec/requests/api/docs/v3/applications_spec.rb b/spec/requests/api/docs/v3/applications_spec.rb index 29d9a6210b..75aa1e589f 100644 --- a/spec/requests/api/docs/v3/applications_spec.rb +++ b/spec/requests/api/docs/v3/applications_spec.rb @@ -80,7 +80,7 @@ "The NPQ application after changing the funded place", "#/components/schemas/ApplicationResponse", "#/components/schemas/ApplicationChangeFundedPlaceRequest" do - let(:application) { create(:application, :eligible_for_funded_place, lead_provider:, schedule:, cohort:) } + let(:application) { create(:application, :eligible_for_funded_place, lead_provider:, schedule:, cohort:, course:) } let(:resource) { application } let(:type) { "npq-application-change-funded-place" } let(:attributes) { { funded_place: true } } diff --git a/spec/services/applications/change_funded_place_spec.rb b/spec/services/applications/change_funded_place_spec.rb index 183dd2ab7d..d8ce444998 100644 --- a/spec/services/applications/change_funded_place_spec.rb +++ b/spec/services/applications/change_funded_place_spec.rb @@ -63,7 +63,7 @@ end it "is invalid if the application is not eligible for funding" do - application.update!(eligible_for_funding: false) + application.update!(eligible_for_funding: false, funded_place: false) service.change diff --git a/spec/services/participant_outcomes/create_spec.rb b/spec/services/participant_outcomes/create_spec.rb index 18ee4ca047..2c2faa7d05 100644 --- a/spec/services/participant_outcomes/create_spec.rb +++ b/spec/services/participant_outcomes/create_spec.rb @@ -46,7 +46,10 @@ context "when the participant has completed declarations with a different course identifier" do let(:other_course) { Course.find_by(identifier: described_class::PERMITTED_COURSES.excluding(course_identifier).sample) } - before { completed_declaration.application.update!(course: other_course) } + before do + other_course.update!(course_group: course.course_group) + completed_declaration.application.update!(course: other_course) + end it { is_expected.to have_error(:base, :no_completed_declarations, "The participant has not had a 'completed' declaration submitted for them. Therefore you cannot update their outcome.") } end diff --git a/spec/services/participants/change_schedule_spec.rb b/spec/services/participants/change_schedule_spec.rb index e2cee54e6a..a1ce949d7d 100644 --- a/spec/services/participants/change_schedule_spec.rb +++ b/spec/services/participants/change_schedule_spec.rb @@ -223,6 +223,7 @@ context "when existing declarations is not valid for new_schedule" do before do + schedule.update!(applies_from: declaration_date.prev_week) create( :declaration, application:, diff --git a/spec/support/matchers/error_matcher.rb b/spec/support/matchers/error_matcher.rb index 3cd6a40a8a..19c117e450 100644 --- a/spec/support/matchers/error_matcher.rb +++ b/spec/support/matchers/error_matcher.rb @@ -1,6 +1,6 @@ -RSpec::Matchers.define :have_error do |attribute, type, message| +RSpec::Matchers.define :have_error do |attribute, type, message, context| match do |actual| - actual.invalid? && actual.errors.any? do |error| + actual.invalid?(context) && actual.errors.any? do |error| error.attribute == attribute && error.type == type && error.message == message end end