[CPDLP-3282] NPQ MVP Prep - Review and add missing validations for se…

…rvices and models (#1559)

* [CPDLP-3282] NPQ MVP Prep - Review and add missing validations for services and models

* [CPDLP-3282] Added validate_permitted_schedule_for_course to application

* [CPDLP-3282] Merge conflict fixes

* [CPDLP-3282] put application validations behind separation environment check

* [CPDLP-3282] use validation context for 'npq_separation'

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

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

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?

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
 

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,13 +104,27 @@ en:
   time:
     formats:
       admin: "%R on %d/%m/%Y"
+
   activerecord:
     errors:
       models:
         application:
           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:

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

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)

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}" }