From 1e36529e7ae4cf2e9bab60739ec00994f1991cbd Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Fri, 28 Jun 2024 15:37:27 +0100 Subject: [PATCH 1/3] Make below_min_qa_threshold an instance method This method is only used in one place, in an instance method on Claim, so it doesn't need to be a class method. By moving this method to be an instance method it will be easier to overwrite the MIN_QA_THRESHOLD on a per policy basis. --- app/models/claim.rb | 52 +++++++++++++++++++-------------------- spec/models/claim_spec.rb | 8 +++--- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/app/models/claim.rb b/app/models/claim.rb index 625b7b2f22..304e4bce68 100644 --- a/app/models/claim.rb +++ b/app/models/claim.rb @@ -181,31 +181,6 @@ class Claim < ApplicationRecord scope :awaiting_qa, -> { approved.qa_required.where(qa_completed_at: nil) } scope :qa_required, -> { where(qa_required: true) } - # This method's intention is to help make a decision on whether a claim should - # be flagged for QA or not. These criteria need to be met for each academic year: - # - # 1. the first claim to be approved should always be flagged for QA - # 2. subsequently approved claims should be flagged for QA, 1 in 100/MIN_QA_THRESHOLD. - # - # This method should be used every time a new approval decision is being made; - # when used retrospectively, i.e. when several claims have been approved, - # the method returns: - # - # 1. `true` if none of then claims have been flagged for QA - # 2. `true` if some claims have been flagged for QA using a lower MIN_QA_THRESHOLD - # 3. `false` if some claims have been flagged for QA using a higher MIN_QA_THRESHOLD - # - # Newly approved claims should not be flagged for QA for as long as the method - # returns `false`; they should be flagged for QA otherwise. - def self.below_min_qa_threshold? - return false if MIN_QA_THRESHOLD.zero? - - claims_approved_so_far = current_academic_year.approved.count - return true if claims_approved_so_far.zero? - - (current_academic_year.approved.qa_required.count.to_f / claims_approved_so_far) * 100 <= MIN_QA_THRESHOLD - end - def hold!(reason:, user:) if holdable? && !held? self.class.transaction do @@ -245,7 +220,32 @@ def holdable? end def flaggable_for_qa? - decision_made? && latest_decision.approved? && Claim.below_min_qa_threshold? && !awaiting_qa? && !qa_completed? + decision_made? && latest_decision.approved? && below_min_qa_threshold? && !awaiting_qa? && !qa_completed? + end + + # This method's intention is to help make a decision on whether a claim should + # be flagged for QA or not. These criteria need to be met for each academic year: + # + # 1. the first claim to be approved should always be flagged for QA + # 2. subsequently approved claims should be flagged for QA, 1 in 100/MIN_QA_THRESHOLD. + # + # This method should be used every time a new approval decision is being made; + # when used retrospectively, i.e. when several claims have been approved, + # the method returns: + # + # 1. `true` if none of then claims have been flagged for QA + # 2. `true` if some claims have been flagged for QA using a lower MIN_QA_THRESHOLD + # 3. `false` if some claims have been flagged for QA using a higher MIN_QA_THRESHOLD + # + # Newly approved claims should not be flagged for QA for as long as the method + # returns `false`; they should be flagged for QA otherwise. + def below_min_qa_threshold? + return false if MIN_QA_THRESHOLD.zero? + + claims_approved_so_far = Claim.current_academic_year.approved.count + return true if claims_approved_so_far.zero? + + (Claim.current_academic_year.approved.qa_required.count.to_f / claims_approved_so_far) * 100 <= MIN_QA_THRESHOLD end def qa_completed? diff --git a/spec/models/claim_spec.rb b/spec/models/claim_spec.rb index ef5c807cab..75897302b5 100644 --- a/spec/models/claim_spec.rb +++ b/spec/models/claim_spec.rb @@ -371,7 +371,7 @@ context "when above the min QA threshold" do before do - allow(Claim).to receive(:below_min_qa_threshold?).and_return(false) + stub_const("Claim::MIN_QA_THRESHOLD", 0) end it { is_expected.to eq(false) } @@ -379,7 +379,7 @@ context "when below the min QA threshold" do before do - allow(Claim).to receive(:below_min_qa_threshold?).and_return(true) + stub_const("Claim::MIN_QA_THRESHOLD", 100) end it { is_expected.to eq(true) } @@ -597,8 +597,8 @@ end end - describe ".below_min_qa_threshold?" do - subject { described_class.below_min_qa_threshold? } + describe "#below_min_qa_threshold?" do + subject { described_class.new.below_min_qa_threshold? } context "when the MIN_QA_THRESHOLD is set to zero" do before do From af1611b128c2f317f131a525a63731dd24470ade Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Fri, 28 Jun 2024 17:07:13 +0100 Subject: [PATCH 2/3] Set QA threshold per policy Previously we were QAing 10% of all claims when what we want to do is QA 10% of claims for each policy. This commit moves the MIN_QA_THRESHOLD constant into the policy such that it's configurable on a per policy basis. --- app/models/claim.rb | 7 +- app/models/policies/early_career_payments.rb | 3 + .../policies/further_education_payments.rb | 2 + .../international_relocation_payments.rb | 3 + .../policies/levelling_up_premium_payments.rb | 3 + app/models/policies/student_loans.rb | 3 + spec/models/claim_spec.rb | 178 ++++++++++++++++-- spec/support/stubbing_helpers.rb | 4 +- 8 files changed, 182 insertions(+), 21 deletions(-) diff --git a/app/models/claim.rb b/app/models/claim.rb index 304e4bce68..41517119b2 100644 --- a/app/models/claim.rb +++ b/app/models/claim.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class Claim < ApplicationRecord - MIN_QA_THRESHOLD = 10 NO_STUDENT_LOAN = "not_applicable" STUDENT_LOAN_PLAN_OPTIONS = StudentLoan::PLANS.dup << NO_STUDENT_LOAN ADDRESS_ATTRIBUTES = %w[address_line_1 address_line_2 address_line_3 address_line_4 postcode].freeze @@ -240,12 +239,12 @@ def flaggable_for_qa? # Newly approved claims should not be flagged for QA for as long as the method # returns `false`; they should be flagged for QA otherwise. def below_min_qa_threshold? - return false if MIN_QA_THRESHOLD.zero? + return false if policy::MIN_QA_THRESHOLD.zero? - claims_approved_so_far = Claim.current_academic_year.approved.count + claims_approved_so_far = Claim.by_policy(policy).current_academic_year.approved.count return true if claims_approved_so_far.zero? - (Claim.current_academic_year.approved.qa_required.count.to_f / claims_approved_so_far) * 100 <= MIN_QA_THRESHOLD + (Claim.by_policy(policy).current_academic_year.approved.qa_required.count.to_f / claims_approved_so_far) * 100 <= policy::MIN_QA_THRESHOLD end def qa_completed? diff --git a/app/models/policies/early_career_payments.rb b/app/models/policies/early_career_payments.rb index 8d6bffbe94..1f6569aa95 100644 --- a/app/models/policies/early_career_payments.rb +++ b/app/models/policies/early_career_payments.rb @@ -37,6 +37,9 @@ module EarlyCareerPayments SEARCHABLE_ELIGIBILITY_ATTRIBUTES = %w[teacher_reference_number].freeze + # Percentage of claims to QA + MIN_QA_THRESHOLD = 10 + def eligibility_page_url "https://www.gov.uk/guidance/early-career-payments-guidance-for-teachers-and-schools" end diff --git a/app/models/policies/further_education_payments.rb b/app/models/policies/further_education_payments.rb index 7aa3fb3edf..2e526c6f07 100644 --- a/app/models/policies/further_education_payments.rb +++ b/app/models/policies/further_education_payments.rb @@ -1,4 +1,6 @@ module Policies module FurtherEducationPayments + # Percentage of claims to QA + MIN_QA_THRESHOLD = 10 end end diff --git a/app/models/policies/international_relocation_payments.rb b/app/models/policies/international_relocation_payments.rb index d66ed2c747..e0acd520bb 100644 --- a/app/models/policies/international_relocation_payments.rb +++ b/app/models/policies/international_relocation_payments.rb @@ -6,6 +6,9 @@ module InternationalRelocationPayments ELIGIBILITY_MATCHING_ATTRIBUTES = [["passport_number"]].freeze OTHER_CLAIMABLE_POLICIES = [] + # Percentage of claims to QA + MIN_QA_THRESHOLD = 10 + # NOTE RL: currently IRP only has a single reply to address, so notify # doesn't show the address id def notify_reply_to_id diff --git a/app/models/policies/levelling_up_premium_payments.rb b/app/models/policies/levelling_up_premium_payments.rb index 052fa437bb..69837d6308 100644 --- a/app/models/policies/levelling_up_premium_payments.rb +++ b/app/models/policies/levelling_up_premium_payments.rb @@ -27,6 +27,9 @@ module LevellingUpPremiumPayments POLICY_START_YEAR = AcademicYear.new(2022).freeze POLICY_END_YEAR = AcademicYear.new(2024).freeze + # Percentage of claims to QA + MIN_QA_THRESHOLD = 10 + def notify_reply_to_id "03ece7eb-2a5b-461b-9c91-6630d0051aa6" end diff --git a/app/models/policies/student_loans.rb b/app/models/policies/student_loans.rb index a39eda968a..71006ed7d9 100644 --- a/app/models/policies/student_loans.rb +++ b/app/models/policies/student_loans.rb @@ -37,6 +37,9 @@ module StudentLoans SEARCHABLE_ELIGIBILITY_ATTRIBUTES = %w[teacher_reference_number].freeze + # Percentage of claims to QA + MIN_QA_THRESHOLD = 10 + def eligibility_page_url "https://www.gov.uk/guidance/teachers-claim-back-your-student-loan-repayments" end diff --git a/spec/models/claim_spec.rb b/spec/models/claim_spec.rb index 75897302b5..ba2eca6132 100644 --- a/spec/models/claim_spec.rb +++ b/spec/models/claim_spec.rb @@ -371,7 +371,7 @@ context "when above the min QA threshold" do before do - stub_const("Claim::MIN_QA_THRESHOLD", 0) + stub_const("Policies::#{claim.policy}::MIN_QA_THRESHOLD", 0) end it { is_expected.to eq(false) } @@ -379,7 +379,7 @@ context "when below the min QA threshold" do before do - stub_const("Claim::MIN_QA_THRESHOLD", 100) + stub_const("Policies::#{claim.policy}::MIN_QA_THRESHOLD", 100) end it { is_expected.to eq(true) } @@ -598,11 +598,14 @@ end describe "#below_min_qa_threshold?" do - subject { described_class.new.below_min_qa_threshold? } + let(:policy) { Policies::EarlyCareerPayments } + let(:other_policy) { Policies::POLICIES.detect { |p| p != policy } } + + subject { build(:claim, policy: policy).below_min_qa_threshold? } context "when the MIN_QA_THRESHOLD is set to zero" do before do - stub_const("Claim::MIN_QA_THRESHOLD", 0) + stub_const("Policies::#{policy}::MIN_QA_THRESHOLD", 0) end it { is_expected.to eq(false) } @@ -610,50 +613,193 @@ context "when the MIN_QA_THRESHOLD is set to 10" do before do - stub_const("Claim::MIN_QA_THRESHOLD", 10) unless described_class::MIN_QA_THRESHOLD == 10 + stub_const("Policies::#{policy}::MIN_QA_THRESHOLD", 10) end context "with no previously approved claims" do + let!(:claims_for_other_policy) do + create_list( + :claim, + 1, + :approved, + :flagged_for_qa, + policy: other_policy, + academic_year: AcademicYear.current + ) + end it { is_expected.to eq(true) } end context "with 1 previously approved claim (1 flagged for QA)" do - let!(:claims_flagged_for_qa) { create_list(:claim, 1, :approved, :flagged_for_qa, academic_year: AcademicYear.current) } + let!(:claims_flagged_for_qa) do + create_list( + :claim, + 1, + :approved, + :flagged_for_qa, + policy: policy, + academic_year: AcademicYear.current + ) + end it { is_expected.to eq(false) } end context "with 2 previously approved claims (1 flagged for QA)" do - let!(:claims_flagged_for_qa) { create_list(:claim, 1, :approved, :flagged_for_qa, academic_year: AcademicYear.current) } - let!(:claims_not_flagged_for_qa) { create_list(:claim, 1, :approved, academic_year: AcademicYear.current) } + let!(:claims_flagged_for_qa) do + create_list( + :claim, + 1, + :approved, + :flagged_for_qa, + policy: policy, + academic_year: AcademicYear.current + ) + end + let!(:claims_not_flagged_for_qa) do + create_list( + :claim, + 1, + :approved, + policy: policy, + academic_year: AcademicYear.current + ) + end it { is_expected.to eq(false) } end context "with 9 previously approved claims (1 flagged for QA)" do - let!(:claims_flagged_for_qa) { create_list(:claim, 1, :approved, :flagged_for_qa, academic_year: AcademicYear.current) } - let!(:claims_not_flagged_for_qa) { create_list(:claim, 8, :approved, academic_year: AcademicYear.current) } + let!(:claims_flagged_for_qa) do + create_list( + :claim, + 1, + :approved, + :flagged_for_qa, + policy: policy, + academic_year: AcademicYear.current + ) + end + + let!(:claims_not_flagged_for_qa) do + create_list( + :claim, + 8, + :approved, + policy: policy, + academic_year: AcademicYear.current + ) + end + let!(:claims_for_other_policy) do + create_list( + :claim, + 1, + :approved, + :flagged_for_qa, + policy: other_policy, + academic_year: AcademicYear.current + ) + end it { is_expected.to eq(false) } end context "with 10 previously approved claims (1 flagged for QA)" do - let!(:claims_flagged_for_qa) { create_list(:claim, 1, :approved, :flagged_for_qa, academic_year: AcademicYear.current) } - let!(:claims_not_flagged_for_qa) { create_list(:claim, 9, :approved, academic_year: AcademicYear.current) } + let!(:claims_flagged_for_qa) do + create_list( + :claim, + 1, + :approved, + :flagged_for_qa, + policy: policy, + academic_year: AcademicYear.current + ) + end + let!(:claims_not_flagged_for_qa) do + create_list( + :claim, + 9, + :approved, + policy: policy, + academic_year: AcademicYear.current + ) + end + let!(:claims_for_other_policy) do + create_list( + :claim, + 1, + :approved, + :flagged_for_qa, + policy: other_policy, + academic_year: AcademicYear.current + ) + end it { is_expected.to eq(true) } end context "with 11 previously approved claims (2 flagged for QA)" do - let!(:claims_flagged_for_qa) { create_list(:claim, 2, :approved, :flagged_for_qa, academic_year: AcademicYear.current) } - let!(:claims_not_flagged_for_qa) { create_list(:claim, 10, :approved, academic_year: AcademicYear.current) } + let!(:claims_flagged_for_qa) do + create_list( + :claim, + 2, + :approved, + :flagged_for_qa, + policy: policy, + academic_year: AcademicYear.current + ) + end + let!(:claims_not_flagged_for_qa) do + create_list( + :claim, + 10, + :approved, + policy: policy, + academic_year: AcademicYear.current + ) + end + let!(:claims_for_other_policy) do + create_list( + :claim, + 2, + :approved, + policy: other_policy, + academic_year: AcademicYear.current + ) + end it { is_expected.to eq(false) } end context "with 21 previously approved claims (2 flagged for QA)" do - let!(:claims_flagged_for_qa) { create_list(:claim, 2, :approved, :flagged_for_qa, academic_year: AcademicYear.current) } - let!(:claims_not_flagged_for_qa) { create_list(:claim, 19, :approved, academic_year: AcademicYear.current) } + let!(:claims_flagged_for_qa) do + create_list( + :claim, + 2, + :approved, + :flagged_for_qa, + policy: policy, + academic_year: AcademicYear.current + ) + end + let!(:claims_not_flagged_for_qa) do + create_list( + :claim, + 19, + :approved, + policy: policy, + academic_year: AcademicYear.current + ) + end + let!(:claims_for_other_policy) do + create_list( + :claim, + 19, + :approved, + policy: other_policy, + academic_year: AcademicYear.current + ) + end it { is_expected.to eq(true) } end diff --git a/spec/support/stubbing_helpers.rb b/spec/support/stubbing_helpers.rb index 386e8b5be2..b0a9c68f44 100644 --- a/spec/support/stubbing_helpers.rb +++ b/spec/support/stubbing_helpers.rb @@ -1,6 +1,8 @@ module StubbingHelpers def disable_claim_qa_flagging - stub_const("Claim::MIN_QA_THRESHOLD", 0) + Policies::POLICIES.each do |policy| + stub_const("Policies::#{policy}::MIN_QA_THRESHOLD", 0) + end end def stub_otp_verification(otp_code: "123456", valid: true) From 1f53e01656b9af0f9d93f51d49139186819000be Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Mon, 1 Jul 2024 11:19:51 +0100 Subject: [PATCH 3/3] QA 100% of IRP claims A business requirement is for us to QA 100% of the claims for IRP --- app/models/policies/international_relocation_payments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/policies/international_relocation_payments.rb b/app/models/policies/international_relocation_payments.rb index e0acd520bb..b6d75d5aad 100644 --- a/app/models/policies/international_relocation_payments.rb +++ b/app/models/policies/international_relocation_payments.rb @@ -7,7 +7,7 @@ module InternationalRelocationPayments OTHER_CLAIMABLE_POLICIES = [] # Percentage of claims to QA - MIN_QA_THRESHOLD = 10 + MIN_QA_THRESHOLD = 100 # NOTE RL: currently IRP only has a single reply to address, so notify # doesn't show the address id