From 08647f18cebda64aba497bcd285b622870a307e0 Mon Sep 17 00:00:00 2001 From: Kenneth Lee Date: Mon, 7 Oct 2024 13:33:25 +0100 Subject: [PATCH 01/10] Add reply-to-id to FE policy --- .../policies/further_education_payments.rb | 3 +-- .../models/further_education_payments_spec.rb | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 spec/models/further_education_payments_spec.rb diff --git a/app/models/policies/further_education_payments.rb b/app/models/policies/further_education_payments.rb index 31f0d1056d..2d76bd7e19 100644 --- a/app/models/policies/further_education_payments.rb +++ b/app/models/policies/further_education_payments.rb @@ -70,9 +70,8 @@ module FurtherEducationPayments # NOOP as PERSONAL_DATA_ATTRIBUTES_TO_RETAIN_FOR_EXTENDED_PERIOD is empty EXTENDED_PERIOD_END_DATE = ->(start_of_academic_year) {} - # TODO: This is needed once the reply-to email address has been added to Gov Notify def notify_reply_to_id - nil + "89939786-7078-4267-b197-ee505dfad8ae" end def verification_due_date_for_claim(claim) diff --git a/spec/models/further_education_payments_spec.rb b/spec/models/further_education_payments_spec.rb new file mode 100644 index 0000000000..cc1b15704f --- /dev/null +++ b/spec/models/further_education_payments_spec.rb @@ -0,0 +1,20 @@ +require "rails_helper" + +RSpec.describe Policies::FurtherEducationPayments, type: :model do + it { is_expected.to include(BasePolicy) } + + it do + expect(subject::VERIFIERS).to eq([ + AutomatedChecks::ClaimVerifiers::Identity, + AutomatedChecks::ClaimVerifiers::ProviderVerification, + AutomatedChecks::ClaimVerifiers::Employment, + AutomatedChecks::ClaimVerifiers::StudentLoanPlan + ]) + end + + specify { + expect(subject).to have_attributes( + notify_reply_to_id: "89939786-7078-4267-b197-ee505dfad8ae" + ) + } +end From 5362b3a8929ec0b1f7f62b73a073dbf41d5f7c0f Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Thu, 10 Oct 2024 13:06:30 +0100 Subject: [PATCH 02/10] status of rejected trumps provider verification --- app/helpers/admin/claims_helper.rb | 6 +++--- spec/factories/claims.rb | 8 ++++++++ spec/helpers/admin/claims_helper_spec.rb | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/app/helpers/admin/claims_helper.rb b/app/helpers/admin/claims_helper.rb index 49e98b2c2b..1142a130e3 100644 --- a/app/helpers/admin/claims_helper.rb +++ b/app/helpers/admin/claims_helper.rb @@ -172,9 +172,7 @@ def claim_summary_heading(claim) end def status(claim) - if claim.awaiting_provider_verification? - "Awaiting provider verification" - elsif claim.all_payrolled? + if claim.all_payrolled? "Payrolled" elsif claim.latest_decision&.approved? && claim.awaiting_qa? && !claim.held? "Approved awaiting QA" @@ -182,6 +180,8 @@ def status(claim) "Approved awaiting payroll" elsif claim.latest_decision&.rejected? "Rejected" + elsif claim.awaiting_provider_verification? + "Awaiting provider verification" elsif claim.held? "Awaiting decision - on hold" else diff --git a/spec/factories/claims.rb b/spec/factories/claims.rb index cc17bae42a..0680f2c5bb 100644 --- a/spec/factories/claims.rb +++ b/spec/factories/claims.rb @@ -309,6 +309,14 @@ end end + trait :awaiting_provider_verification do + eligibility_trait { :not_verified } + + after(:create) do |claim, _| + create(:note, claim:, label: "provider_verification") + end + end + trait :with_dqt_teacher_status do dqt_teacher_status do { diff --git a/spec/helpers/admin/claims_helper_spec.rb b/spec/helpers/admin/claims_helper_spec.rb index 43f7303c1c..a3978e08ad 100644 --- a/spec/helpers/admin/claims_helper_spec.rb +++ b/spec/helpers/admin/claims_helper_spec.rb @@ -540,6 +540,23 @@ end end end + + context "rejected claim whilst awaiting provider verification" do + let!(:claim) do + create( + :claim, + :rejected, + :awaiting_provider_verification, + policy: Policies::FurtherEducationPayments, + ) + end + + it "returns rejected" do + freeze_time do + expect(status(claim)).to eql "Rejected" + end + end + end end describe "#index_status_filter" do From d594d82fe2f659f6734c06b5d2e3e0b8c3f88d9d Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Thu, 10 Oct 2024 13:29:15 +0100 Subject: [PATCH 03/10] filtering awaiting_provider_verification - must now also be awaiting a decision - this prevents approved and rejected claims when filtering for this --- app/forms/admin/claims_filter_form.rb | 2 +- spec/forms/admin/claims_filter_form_spec.rb | 25 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 spec/forms/admin/claims_filter_form_spec.rb diff --git a/app/forms/admin/claims_filter_form.rb b/app/forms/admin/claims_filter_form.rb index 9e37bb314e..01c7bf0220 100644 --- a/app/forms/admin/claims_filter_form.rb +++ b/app/forms/admin/claims_filter_form.rb @@ -57,7 +57,7 @@ def claims when "failed_bank_validation" Claim.includes(:decisions).failed_bank_validation.awaiting_decision when "awaiting_provider_verification" - Claim.by_policy(Policies::FurtherEducationPayments).awaiting_further_education_provider_verification + Claim.by_policy(Policies::FurtherEducationPayments).awaiting_further_education_provider_verification.awaiting_decision else Claim.includes(:decisions).not_held.awaiting_decision.not_awaiting_further_education_provider_verification end diff --git a/spec/forms/admin/claims_filter_form_spec.rb b/spec/forms/admin/claims_filter_form_spec.rb new file mode 100644 index 0000000000..a842414b2f --- /dev/null +++ b/spec/forms/admin/claims_filter_form_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe Admin::ClaimsFilterForm, type: :model do + describe "#claims" do + context "when rejected whilst awaiting provider verification" do + let!(:claim) do + create( + :claim, + :rejected, + :awaiting_provider_verification, + policy: Policies::FurtherEducationPayments, + ) + end + + let(:session) { {} } + let(:filters) { { status: "awaiting_provider_verification" } } + + subject { described_class.new(filters:, session:) } + + it "filtering by status awaiting provider verification excludes them" do + expect(subject.claims).not_to include(claim) + end + end + end +end From 3f14648f650122424c77386b35cdac62d8551dfd Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Thu, 10 Oct 2024 13:47:57 +0100 Subject: [PATCH 04/10] fix linting --- spec/forms/admin/claims_filter_form_spec.rb | 4 ++-- spec/helpers/admin/claims_helper_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/forms/admin/claims_filter_form_spec.rb b/spec/forms/admin/claims_filter_form_spec.rb index a842414b2f..57e7ac4b2f 100644 --- a/spec/forms/admin/claims_filter_form_spec.rb +++ b/spec/forms/admin/claims_filter_form_spec.rb @@ -8,12 +8,12 @@ :claim, :rejected, :awaiting_provider_verification, - policy: Policies::FurtherEducationPayments, + policy: Policies::FurtherEducationPayments ) end let(:session) { {} } - let(:filters) { { status: "awaiting_provider_verification" } } + let(:filters) { {status: "awaiting_provider_verification"} } subject { described_class.new(filters:, session:) } diff --git a/spec/helpers/admin/claims_helper_spec.rb b/spec/helpers/admin/claims_helper_spec.rb index a3978e08ad..a37a92d689 100644 --- a/spec/helpers/admin/claims_helper_spec.rb +++ b/spec/helpers/admin/claims_helper_spec.rb @@ -547,7 +547,7 @@ :claim, :rejected, :awaiting_provider_verification, - policy: Policies::FurtherEducationPayments, + policy: Policies::FurtherEducationPayments ) end From 107ec8603327b32654de611e1dc362bc8ae2fa8c Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 10 Oct 2024 13:25:18 +0100 Subject: [PATCH 05/10] Fix duplciate claims in list The awaiting provider verification scope joins to notes, claims can have more than one note, if they do duplicate claims are returned from the query. To resolve this we call distinct on the claim id. Claims have a couple of JSON columns so we can't use the `distinct` method directly, instead we have to explicitly use distinct with the claim.id. We also need to wrap the whole query in a where clause so when we chain on a `count` rails generates the correct SQL rather than something like `SELECT COUNT(count_column) FROM (SELECT DISTINCT ON (claims.id), claims.id)` Additionally we have to make sure to include `submitted_at` in the distinct clause as some of the scopes used to build the query include order by submitted_at clauses. --- app/forms/admin/claims_filter_form.rb | 6 ++- spec/forms/admin/claims_filter_form_spec.rb | 59 +++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/app/forms/admin/claims_filter_form.rb b/app/forms/admin/claims_filter_form.rb index 01c7bf0220..35ac401989 100644 --- a/app/forms/admin/claims_filter_form.rb +++ b/app/forms/admin/claims_filter_form.rb @@ -67,8 +67,12 @@ def claims @claims = @claims.unassigned if unassigned? @claims = @claims.includes(:tasks, eligibility: [:claim_school, :current_school]) - @claims = @claims.order(:submitted_at) + @claims = @claims.select("DISTINCT ON (claims.id, claims.submitted_at) claims.id") + + @claims = Claim.where(id: @claims) + + @claims = @claims.order(:submitted_at) @claims end diff --git a/spec/forms/admin/claims_filter_form_spec.rb b/spec/forms/admin/claims_filter_form_spec.rb index 57e7ac4b2f..5364b7f2b2 100644 --- a/spec/forms/admin/claims_filter_form_spec.rb +++ b/spec/forms/admin/claims_filter_form_spec.rb @@ -21,5 +21,64 @@ expect(subject.claims).not_to include(claim) end end + + context "when the status is awaiting_provider_verification" do + it "returns the expected claims" do + claim_awaiting_provider_verification_1 = build( + :claim, + :submitted + ) + + create( + :further_education_payments_eligibility, + claim: claim_awaiting_provider_verification_1, + flagged_as_duplicate: false + ) + + claim_awaiting_provider_verification_2 = build( + :claim, + :submitted + ) + + create( + :further_education_payments_eligibility, + claim: claim_awaiting_provider_verification_2, + flagged_as_duplicate: true + ) + + create( + :note, + claim: claim_awaiting_provider_verification_2, + label: "provider_verification" + ) + + create( + :note, + claim: claim_awaiting_provider_verification_2, + label: "provider_verification" + ) + + _claim_not_awating_provider_verification = build(:claim, :submitted) + + create( + :further_education_payments_eligibility, + :verified + ) + + form = described_class.new( + session: {}, + filters: { + status: "awaiting_provider_verification" + } + ) + + expect(form.claims).to match_array( + [ + claim_awaiting_provider_verification_1, + claim_awaiting_provider_verification_2 + ] + ) + end + end end end From 256ae4dd5c1aeec90bce7c2580e35075d996d1a7 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 10 Oct 2024 14:34:32 +0100 Subject: [PATCH 06/10] Remove order by from model scope We move the order clause to where it's needed rather than having it on every call to `payrollable`, this lets us simplify the distinct clause as we no longer need to include the `submitted_at` in the distinct. --- app/controllers/admin/payroll_runs_controller.rb | 2 +- app/forms/admin/claims_filter_form.rb | 2 +- app/models/claim.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/payroll_runs_controller.rb b/app/controllers/admin/payroll_runs_controller.rb index 1604cd1a3a..d1feac8886 100644 --- a/app/controllers/admin/payroll_runs_controller.rb +++ b/app/controllers/admin/payroll_runs_controller.rb @@ -9,7 +9,7 @@ def index end def new - @claims = Claim.payrollable + @claims = Claim.payrollable.order(submitted_at: :asc) # Due to limitations with the current payroll software we need a temporary # cap on the number of claims that can enter payroll, especially expecting diff --git a/app/forms/admin/claims_filter_form.rb b/app/forms/admin/claims_filter_form.rb index 35ac401989..650ff6eed7 100644 --- a/app/forms/admin/claims_filter_form.rb +++ b/app/forms/admin/claims_filter_form.rb @@ -68,7 +68,7 @@ def claims @claims = @claims.includes(:tasks, eligibility: [:claim_school, :current_school]) - @claims = @claims.select("DISTINCT ON (claims.id, claims.submitted_at) claims.id") + @claims = @claims.select("DISTINCT ON (claims.id) claims.id") @claims = Claim.where(id: @claims) diff --git a/app/models/claim.rb b/app/models/claim.rb index ba0c5d1e9d..9006adb23c 100644 --- a/app/models/claim.rb +++ b/app/models/claim.rb @@ -205,7 +205,7 @@ class Claim < ApplicationRecord delegate :award_amount, to: :eligibility - scope :payrollable, -> { approved.not_awaiting_qa.left_joins(:payments).where(payments: nil).order(submitted_at: :asc) } + scope :payrollable, -> { approved.not_awaiting_qa.left_joins(:payments).where(payments: nil) } scope :not_awaiting_qa, -> { approved.where("qa_required = false OR (qa_required = true AND qa_completed_at IS NOT NULL)") } scope :awaiting_qa, -> { approved.qa_required.where(qa_completed_at: nil) } scope :qa_required, -> { where(qa_required: true) } From db7e3f03af32440c7a9aa8010d632dcd8011c8f4 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 10 Oct 2024 15:25:43 +0100 Subject: [PATCH 07/10] Make sure we eager load the other models Need to eager load the other models AFTER we dedup the clams in the query --- app/forms/admin/claims_filter_form.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/forms/admin/claims_filter_form.rb b/app/forms/admin/claims_filter_form.rb index 650ff6eed7..c80a57b1fe 100644 --- a/app/forms/admin/claims_filter_form.rb +++ b/app/forms/admin/claims_filter_form.rb @@ -66,11 +66,9 @@ def claims @claims = @claims.by_claims_team_member(selected_team_member, status) if selected_team_member @claims = @claims.unassigned if unassigned? - @claims = @claims.includes(:tasks, eligibility: [:claim_school, :current_school]) - - @claims = @claims.select("DISTINCT ON (claims.id) claims.id") + @claims = Claim.where(id: @claims.select("DISTINCT ON (claims.id) claims.id")) - @claims = Claim.where(id: @claims) + @claims = @claims.includes(:tasks, eligibility: [:claim_school, :current_school]) @claims = @claims.order(:submitted_at) @claims From 28317f4f606e28063654a046ef6df2d7bc364c72 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 9 Oct 2024 10:45:06 +0100 Subject: [PATCH 08/10] back office: add missing FE employment answer --- .../admin_tasks_presenter.rb | 4 +++- .../admin_tasks_presenter_spec.rb | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 spec/models/policies/further_education_payments/admin_tasks_presenter_spec.rb diff --git a/app/models/policies/further_education_payments/admin_tasks_presenter.rb b/app/models/policies/further_education_payments/admin_tasks_presenter.rb index 9294ddfd9b..a6101ff37b 100644 --- a/app/models/policies/further_education_payments/admin_tasks_presenter.rb +++ b/app/models/policies/further_education_payments/admin_tasks_presenter.rb @@ -27,7 +27,9 @@ def qualifications end def employment - [] + [ + ["Current provider", display_school(claim.eligibility.current_school)] + ] end def identity_confirmation diff --git a/spec/models/policies/further_education_payments/admin_tasks_presenter_spec.rb b/spec/models/policies/further_education_payments/admin_tasks_presenter_spec.rb new file mode 100644 index 0000000000..b0b1f3b88e --- /dev/null +++ b/spec/models/policies/further_education_payments/admin_tasks_presenter_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +RSpec.describe Policies::FurtherEducationPayments::AdminTasksPresenter do + describe "#employment" do + let(:claim) { create(:claim, :submitted) } + + subject { described_class.new(claim) } + + it "displays answer with link" do + expect(subject.employment[0][0]).to eql("Current provider") + expect(subject.employment[0][1]).to include("href") + expect(subject.employment[0][1]).to include(claim.school.dfe_number) + end + end +end From c58594c804e45cc31bec68a0e16e3c7d0ce3146e Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 9 Oct 2024 10:45:54 +0100 Subject: [PATCH 09/10] add missing back office translation --- config/locales/en.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index b9c21703e3..95d037dd77 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -936,6 +936,8 @@ en: claimant_answers: true: "Yes" false: "No" + employment: + title: Does the claimant’s place of work match the above information on their claim? forms: ineligible: courses: From f732e2a88d29984d46aa6b1f416c237df8c36011 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 9 Oct 2024 14:24:10 +0100 Subject: [PATCH 10/10] back office can filter for auto approved claims --- app/forms/admin/claims_filter_form.rb | 3 +++ spec/factories/claims.rb | 4 ++++ spec/forms/admin/claims_filter_form_spec.rb | 16 ++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/app/forms/admin/claims_filter_form.rb b/app/forms/admin/claims_filter_form.rb index c80a57b1fe..0b4ac548c6 100644 --- a/app/forms/admin/claims_filter_form.rb +++ b/app/forms/admin/claims_filter_form.rb @@ -48,6 +48,8 @@ def claims Claim.approved.awaiting_qa when "approved_awaiting_payroll" approved_awaiting_payroll + when "automatically_approved" + Claim.current_academic_year.auto_approved when "automatically_approved_awaiting_payroll" Claim.current_academic_year.payrollable.auto_approved when "rejected" @@ -94,6 +96,7 @@ def status_select_options ["Awaiting decision - failed bank details", "failed_bank_validation"], ["Approved awaiting QA", "approved_awaiting_qa"], ["Approved awaiting payroll", "approved_awaiting_payroll"], + ["Automatically approved", "automatically_approved"], ["Automatically approved awaiting payroll", "automatically_approved_awaiting_payroll"], ["Approved", "approved"], ["Rejected", "rejected"] diff --git a/spec/factories/claims.rb b/spec/factories/claims.rb index 0680f2c5bb..f83470ca17 100644 --- a/spec/factories/claims.rb +++ b/spec/factories/claims.rb @@ -41,6 +41,10 @@ claim.academic_year = claim_academic_year unless claim.academic_year_before_type_cast end + trait :current_academic_year do + academic_year { AcademicYear.current } + end + trait :with_onelogin_idv_data do identity_confirmed_with_onelogin { true } onelogin_uid { SecureRandom.uuid } diff --git a/spec/forms/admin/claims_filter_form_spec.rb b/spec/forms/admin/claims_filter_form_spec.rb index 5364b7f2b2..8edf7a8473 100644 --- a/spec/forms/admin/claims_filter_form_spec.rb +++ b/spec/forms/admin/claims_filter_form_spec.rb @@ -80,5 +80,21 @@ ) end end + + context "filtering for unassigned + auto approved claims" do + subject { described_class.new(session:, filters: {team_member:, status:}) } + + let(:team_member) { "unassigned" } + let(:status) { "automatically_approved" } + let(:session) { {} } + + before do + create(:claim, :submitted, :auto_approved, :current_academic_year) + end + + it "works" do + expect(subject.claims.count).to eql(1) + end + end end end