Skip to content

Commit

Permalink
Create audit trail when SLC upload runs
Browse files Browse the repository at this point in the history
When the SLC data upload runs we want to use the existing amendments
mechanism to record any changes to the claim. As amending a claim
requires a user to be associated with the amendment we need to pass who
triggered the SLC upload down to the `ClaimStudentLoanDetailsUpdater`.

We've had to slightly change the validation on
`StudentLoans::Eligibility` to check if the repayment amount has changed
as there's one test (`spec/features/admin/upload_slc_data_spec.rb`)
where:
* No SLC data is found for a claim
* The claim has a student_loan_repayment_amount of `0`
* The claim's plan is expected to change from `not_applicable` to `nil`
* The claim's `has_student_loan` is expected to change from `false` to
  `nil`
Without changing the validation on the eligibility we can't pass this
test as amending the claim errors due to the `0` student loan repayment
amount.
  • Loading branch information
rjlynch committed Jan 16, 2025
1 parent 44c20da commit 651af99
Show file tree
Hide file tree
Showing 12 changed files with 546 additions and 79 deletions.
9 changes: 3 additions & 6 deletions app/jobs/file_importer_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ def perform(file_upload_id)
raise ActiveRecord::RecordNotFound unless FileUpload.exists?(id: file_upload_id)

self.file_upload_id = file_upload_id
self.uploaded_by_email = find_user_email
uploaded_by = FileUpload.find(file_upload_id).uploaded_by
self.uploaded_by_email = uploaded_by.email

ingest!
send_success_email if uploaded_by_email
post_import_block&.call
post_import_block&.call(uploaded_by)
rescue => e
Rollbar.error(e)
rescue_with_lambda&.call
Expand All @@ -59,10 +60,6 @@ def ingest!
FileUpload.delete(file_upload_id)
end

def find_user_email
FileUpload.select(:uploaded_by_id).find_by(id: file_upload_id)&.uploaded_by&.email
end

def send_success_email
return unless notify_with_mailer && success_mailer_method

Expand Down
6 changes: 3 additions & 3 deletions app/jobs/import_student_loans_data_job.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
class ImportStudentLoansDataJob < FileImporterJob
import_with StudentLoansDataImporter do
import_with StudentLoansDataImporter do |uploaded_by|
Rails.logger.info "SLC data imported; student loan verifiers will re-run where necessary"

StudentLoanAmountCheckJob.perform_later
StudentLoanPlanCheckJob.perform_later
StudentLoanAmountCheckJob.perform_later(uploaded_by)
StudentLoanPlanCheckJob.perform_later(uploaded_by)
end
rescue_with -> do
StudentLoansData.delete_all
Expand Down
4 changes: 2 additions & 2 deletions app/jobs/student_loan_amount_check_job.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
class StudentLoanAmountCheckJob < ApplicationJob
def perform
def perform(admin)
delete_no_data_student_loan_amount_tasks
claims = current_year_tslr_claims_awaiting_decision.awaiting_task("student_loan_amount")

claims.each do |claim|
ClaimStudentLoanDetailsUpdater.call(claim)
ClaimStudentLoanDetailsUpdater.call(claim, admin)
AutomatedChecks::ClaimVerifiers::StudentLoanAmount.new(claim:).perform
rescue => e
# If something goes wrong, log the error and continue
Expand Down
4 changes: 2 additions & 2 deletions app/jobs/student_loan_plan_check_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ class StudentLoanPlanCheckJob < ApplicationJob
Policies::EarlyYearsPayments
].freeze

def perform
def perform(admin)
delete_no_data_student_loan_plan_tasks
claims = current_year_claims_awaiting_decision.awaiting_task("student_loan_plan")
claims.each do |claim|
ClaimStudentLoanDetailsUpdater.call(claim)
ClaimStudentLoanDetailsUpdater.call(claim, admin)
AutomatedChecks::ClaimVerifiers::StudentLoanPlan.new(claim:).perform
rescue => e
# If something goes wrong, log the error and continue
Expand Down
1 change: 1 addition & 0 deletions app/models/claim.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class Claim < ApplicationRecord
national_insurance_number
date_of_birth
student_loan_plan
has_student_loan
bank_sort_code
bank_account_number
building_society_roll_number
Expand Down
56 changes: 48 additions & 8 deletions app/models/claim_student_loan_details_updater.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,38 @@
class ClaimStudentLoanDetailsUpdater
def self.call(claim)
new(claim).update_claim_with_latest_data
class StudentLoanUpdateError < StandardError; end

def self.call(claim, admin)
new(claim, admin).update_claim_with_latest_data
end

def initialize(claim)
def initialize(claim, admin)
@claim = claim
@admin = admin
end

def update_claim_with_latest_data
claim.transaction do
eligibility.update!(eligibility_student_loan_attributes) if claim.has_tslr_policy?
claim_changes = {}

if claim.has_student_loan != student_loans_data.has_student_loan?
claim_changes[:has_student_loan] = student_loans_data.has_student_loan?
end

claim.assign_attributes(claim_student_loan_attributes)
if claim.student_loan_plan != student_loans_data.student_loan_plan
claim_changes[:student_loan_plan] = student_loans_data.student_loan_plan
end

claim.save!(context: :"student-loan")
if student_loan_repayment_amount_changed?
claim_changes[:eligibility_attributes] = {
student_loan_repayment_amount: student_loans_data.total_repayment_amount
}
end

amend_claim(claim_changes) if claim_changes.present?
end

private

attr_reader :claim
attr_reader :claim, :admin

delegate :eligibility, to: :claim
delegate :national_insurance_number, :date_of_birth, to: :claim
Expand All @@ -41,4 +54,31 @@ def student_loans_data
date_of_birth: date_of_birth
)
end

def student_loan_repayment_amount_changed?
return false unless claim.has_tslr_policy?

claim.eligibility.student_loan_repayment_amount != student_loans_data.total_repayment_amount
end

def amend_claim(claim_changes)
amendment = Amendment.amend_claim(
claim,
claim_changes,
{
notes: "Student loan details updated from SLC data",
created_by: admin
}
)

if amendment.errors.any?
msg = [
"Failed to update claim #{claim.id} student loan data.",
"amendment_error: \"#{amendment.errors.full_messages.to_sentence}\"",
"SLC data: #{claim_changes}"
].join(" ")

raise StudentLoanUpdateError, msg
end
end
end
2 changes: 1 addition & 1 deletion app/models/policies/student_loans/eligibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class Eligibility < ApplicationRecord
validates :had_leadership_position, on: [:submit], inclusion: {in: [true, false], message: "Select yes if you were employed in a leadership position"}
validates :mostly_performed_leadership_duties, on: [:submit], inclusion: {in: [true, false], message: "Select yes if you spent more than half your working hours on leadership duties"}, if: :had_leadership_position?
validates_numericality_of :student_loan_repayment_amount, message: "Enter a valid monetary amount", allow_nil: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 99999
validates :student_loan_repayment_amount, on: :amendment, award_range: {max: 5_000}
validates :student_loan_repayment_amount, on: :amendment, award_range: {max: 5_000}, if: :student_loan_repayment_amount_changed?
validates :teacher_reference_number, on: :amendment, presence: {message: "Enter your teacher reference number"}
validate :validate_teacher_reference_number_length

Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/amendments/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<% admin_amendment_details(amendment).each do |details| %>
<h3 class="hmcts-timeline__title"><%= details[0] %></h3>
<% if details.count > 1 %>
<p class="hmcts-timeline__by">changed from <%= details[1] %> to <%= details[2] %></p>
<p class="hmcts-timeline__by">changed from <%= details[1].presence || "[no details]" %> to <%= details[2].presence || "[no details]" %></p>
<% else %>
<p class="hmcts-timeline__by">changed</p>
<% end %>
Expand Down
11 changes: 6 additions & 5 deletions spec/jobs/student_loan_amount_check_job_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
require "rails_helper"

RSpec.describe StudentLoanAmountCheckJob do
subject(:perform_job) { described_class.new.perform }
let(:admin) { create(:dfe_signin_user) }
subject(:perform_job) { described_class.new.perform(admin) }

let!(:claim) { create(:claim, claim_status, academic_year:, policy: Policies::StudentLoans) }
let(:claim_status) { :submitted }
Expand All @@ -21,7 +22,7 @@
end

it "excludes the claim from the check", :aggregate_failures do
expect(ClaimStudentLoanDetailsUpdater).not_to receive(:call).with(claim)
expect(ClaimStudentLoanDetailsUpdater).not_to receive(:call).with(claim, admin)
expect(AutomatedChecks::ClaimVerifiers::StudentLoanAmount).not_to receive(:new).with(claim: claim)
perform_job
end
Expand Down Expand Up @@ -49,7 +50,7 @@

context "when the student loan amount check did not run before" do
it "updates the student loan details" do
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim, admin)
perform_job
end

Expand All @@ -64,7 +65,7 @@
let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: nil, manual: false) }

it "updates the student loan details" do
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim, admin)
perform_job
end

Expand Down Expand Up @@ -119,7 +120,7 @@
nino: claim.national_insurance_number,
date_of_birth: claim.date_of_birth
)
allow_any_instance_of(Claim).to receive(:save!) { raise(exception) }
allow_any_instance_of(Claim).to receive(:save) { raise(exception) }
allow(Rollbar).to receive(:error)
end

Expand Down
16 changes: 9 additions & 7 deletions spec/jobs/student_loan_plan_check_job_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
require "rails_helper"

RSpec.describe StudentLoanPlanCheckJob do
subject(:perform_job) { described_class.new.perform }
let(:admin) { create(:dfe_signin_user) }
subject(:perform_job) { described_class.new.perform(admin) }

before do
create(:journey_configuration, :further_education_payments)
Expand Down Expand Up @@ -59,7 +60,7 @@
end

it "updates the student loan details" do
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim, admin)
perform_job
end

Expand All @@ -78,7 +79,7 @@
let(:claim) { create(:claim, claim_status, academic_year:, policy: Policies::FurtherEducationPayments) }

it "updates the student loan details" do
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim, admin)
perform_job
end

Expand All @@ -90,7 +91,7 @@

context "when the student loan plan check did not run before" do
it "updates the student loan details" do
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim, admin)
perform_job
end

Expand All @@ -104,7 +105,7 @@
let!(:previous_task) { create(:task, claim: claim, name: "student_loan_plan", claim_verifier_match: nil, manual: false) }

it "updates the student loan details" do
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim, admin)
perform_job
end

Expand Down Expand Up @@ -134,9 +135,10 @@
:student_loans_data,
claim_reference: claim.reference,
nino: claim.national_insurance_number,
date_of_birth: claim.date_of_birth
date_of_birth: claim.date_of_birth,
plan_type_of_deduction: 2
)
allow_any_instance_of(Claim).to receive(:save!) { raise(exception) }
allow_any_instance_of(Claim).to receive(:save) { raise(exception) }
allow(Rollbar).to receive(:error)
end

Expand Down
Loading

0 comments on commit 651af99

Please sign in to comment.