Skip to content

Commit

Permalink
Extract policy specific claim data scrubber
Browse files Browse the repository at this point in the history
Different policies will have different rules around what data we need to
retain. This commit adds policy specific subclasses for handling
removing personal data. Subclasses can implement policy specific
behaviour by redefining the PERSONAL_DATA_ATTRIBUTES_TO_DELETE constant
or overwriting the `scrub_completed_claims` method.
  • Loading branch information
rjlynch committed Jul 2, 2024
1 parent 3ba8cbd commit 973bfb6
Show file tree
Hide file tree
Showing 21 changed files with 193 additions and 47 deletions.
4 changes: 3 additions & 1 deletion app/jobs/delete_personal_data_from_old_claims_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ class DeletePersonalDataFromOldClaimsJob < CronJob

def perform
Rails.logger.info "Deleting personal data from old claims which have been rejected or paid"
Claim::PersonalDataScrubber.new.scrub_completed_claims
Policies::POLICIES.each do |policy|
policy::ClaimPersonalDataScrubber.new.scrub_completed_claims
end
end
end
21 changes: 16 additions & 5 deletions app/models/claim/personal_data_scrubber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
# was scheduled to be paid more than two months ago.
#
# Attributes are set to nil, and personal_data_removed_at is set to the current timestamp.
#
# Inherit policy specifc data scrubbers from this class
# `app/models/policy/{some_policy}/claim_personal_data_scrubber.rb`

class Claim
class PersonalDataScrubber
Expand All @@ -29,14 +32,22 @@ class PersonalDataScrubber
]

def scrub_completed_claims
Claim.transaction do
ApplicationRecord.transaction do
scrub_claims(old_rejected_claims)
scrub_claims(old_paid_claims)
end
end

private

def policy
self.class.module_parent
end

def claim_scope
Claim.by_policy(policy)
end

def scrub_claims(claims)
claims.includes(:amendments).each do |claim|
scrub_amendments_personal_data(claim)
Expand All @@ -46,13 +57,13 @@ def scrub_claims(claims)
end

def attribute_values_to_set
PERSONAL_DATA_ATTRIBUTES_TO_DELETE.map { |attr| [attr, nil] }.to_h.merge(
self.class::PERSONAL_DATA_ATTRIBUTES_TO_DELETE.map { |attr| [attr, nil] }.to_h.merge(
personal_data_removed_at: Time.zone.now
)
end

def old_rejected_claims
Claim.joins(:decisions)
claim_scope.joins(:decisions)
.where(personal_data_removed_at: nil)
.where(
"(decisions.undone = false AND decisions.result = :rejected AND decisions.created_at < :minimum_time)",
Expand All @@ -65,7 +76,7 @@ def old_paid_claims
claim_ids_with_payrollable_topups = Topup.payrollable.pluck(:claim_id)
claim_ids_with_payrolled_topups_without_payment_confirmation = Topup.joins(payment: [:payroll_run]).where(payments: {scheduled_payment_date: nil}).pluck(:claim_id)

Claim.approved.joins(payments: [:payroll_run])
claim_scope.approved.joins(payments: [:payroll_run])
.where(personal_data_removed_at: nil)
.where.not(id: claim_ids_with_payrollable_topups + claim_ids_with_payrolled_topups_without_payment_confirmation)
.where("payments.scheduled_payment_date < :minimum_time", minimum_time: minimum_time)
Expand All @@ -86,7 +97,7 @@ def scrub_amendments_personal_data(claim)
end

def scrub_amendment_personal_data(amendment)
attributes_to_scrub = PERSONAL_DATA_ATTRIBUTES_TO_DELETE.map(&:to_s) & amendment.claim_changes.keys
attributes_to_scrub = self.class::PERSONAL_DATA_ATTRIBUTES_TO_DELETE.map(&:to_s) & amendment.claim_changes.keys
personal_data_mask = attributes_to_scrub.to_h { |attribute| [attribute, nil] }
amendment.claim_changes.merge!(personal_data_mask)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module Policies
module EarlyCareerPayments
class ClaimPersonalDataScrubber < Claim::PersonalDataScrubber
end
end
end
2 changes: 2 additions & 0 deletions app/models/policies/further_education_payments.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module Policies
module FurtherEducationPayments
include BasePolicy
extend self
# Percentage of claims to QA
MIN_QA_THRESHOLD = 10
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module Policies
module FurtherEducationPayments
class ClaimPersonalDataScrubber < Claim::PersonalDataScrubber
def scrub_completed_claims
# FIXME RL: temp NOOP until business decsion around whether TRN is
# required in the payment claim matching has been resolved
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module Policies
module InternationalRelocationPayments
class ClaimPersonalDataScrubber < Claim::PersonalDataScrubber
def scrub_completed_claims
# FIXME RL: temp NOOP until business decsion around whether TRN is
# required in the payment claim matching has been resolved
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module Policies
module LevellingUpPremiumPayments
class ClaimPersonalDataScrubber < Claim::PersonalDataScrubber
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module Policies
module StudentLoans
class ClaimPersonalDataScrubber < Claim::PersonalDataScrubber
end
end
end
2 changes: 2 additions & 0 deletions config/analytics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ shared:
- date_of_entry
- nationality
- current_school_id
- award_amount
:schools:
- id
- urn
Expand Down Expand Up @@ -270,3 +271,4 @@ shared:
- id
- created_at
- updated_at
- award_amount
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddAwardAmountToInternationalRelocationPaymentsEligibilities < ActiveRecord::Migration[7.0]
def change
add_column :international_relocation_payments_eligibilities, :award_amount, :decimal, precision: 7, scale: 2
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddAwardAmountToFurtherEducationPaymentsEligibilities < ActiveRecord::Migration[7.0]
def change
add_column :further_education_payments_eligibilities, :award_amount, :decimal, precision: 7, scale: 2
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2024_06_27_145713) do
ActiveRecord::Schema[7.0].define(version: 2024_07_02_105328) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
enable_extension "pgcrypto"
Expand Down Expand Up @@ -185,6 +185,7 @@
create_table "further_education_payments_eligibilities", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.decimal "award_amount", precision: 7, scale: 2
end

create_table "international_relocation_payments_eligibilities", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
Expand All @@ -201,6 +202,7 @@
t.string "passport_number"
t.string "school_headteacher_name"
t.uuid "current_school_id"
t.decimal "award_amount", precision: 7, scale: 2
t.index ["current_school_id"], name: "index_irb_eligibilities_on_current_school_id"
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FactoryBot.define do
factory :further_education_payments_eligibility, class: "Policies::FurtherEducationPayments::Eligibility" do
trait :eligible do
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
trait :eligible do
eligible_home_office
eligible_school

application_route { "teacher" }
state_funded_secondary_school { true }
one_year { true }
start_date { Date.tomorrow }
subject { "physics" }
visa_type { "British National (Overseas) visa" }
date_of_entry { start_date - 1.week }
end

trait :eligible_school do
Expand Down
8 changes: 0 additions & 8 deletions spec/models/claim/personal_data_scrubber_spec.rb

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
require "rails_helper"

RSpec.describe Policies::EarlyCareerPayments::ClaimPersonalDataScrubber do
it_behaves_like(
"a claim personal data scrubber",
Policies::EarlyCareerPayments
)
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
require "rails_helper"

RSpec.describe Policies::FurtherEducationPayments::ClaimPersonalDataScrubber do
# FIXME RL: temp disabled until business decsion around whether TRN is
# required in the payment claim matching has been resolved
# it_behaves_like(
# "a claim personal data scrubber",
# Policies::FurtherEducationPayments
# )
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
require "rails_helper"

RSpec.describe Policies::InternationalRelocationPayments::ClaimPersonalDataScrubber do
# FIXME RL: temp disabled until business decsion around whether TRN is
# required in the payment claim matching has been resolved
# it_behaves_like(
# "a claim personal data scrubber",
# Policies::InternationalRelocationPayments
# )
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
require "rails_helper"

RSpec.describe Policies::LevellingUpPremiumPayments::ClaimPersonalDataScrubber do
it_behaves_like(
"a claim personal data scrubber",
Policies::LevellingUpPremiumPayments
)

subject(:personal_data_scrubber) { described_class.new.scrub_completed_claims }
let!(:journey_configuration) { create(:journey_configuration, :additional_payments) }
let(:current_academic_year) { AcademicYear.current }
let(:last_academic_year) { Time.zone.local(current_academic_year.start_year, 8, 1) }
let(:user) { create(:dfe_signin_user) }

it "does not delete details from a claim that has a payment, but has a payrollable topup" do
eligibility = create(:levelling_up_premium_payments_eligibility, :eligible, award_amount: 1500.0)

claim = create(:claim, :approved, policy: Policies::LevellingUpPremiumPayments, eligibility: eligibility)

create(:payment, :confirmed, :with_figures, claims: [claim], scheduled_payment_date: last_academic_year)
create(:topup, payment: nil, claim: claim, award_amount: 500, created_by: user)

expect { personal_data_scrubber }.not_to change { claim.reload.attributes }
end

it "does not delete details from a claim that has a payment, but has a payrolled topup without payment confirmation" do
claim = nil

travel_to 2.months.ago do
eligibility = create(:levelling_up_premium_payments_eligibility, :eligible, award_amount: 1500.0)
claim = create(:claim, :approved, policy: Policies::LevellingUpPremiumPayments, eligibility: eligibility)
create(:payment, :confirmed, :with_figures, claims: [claim], scheduled_payment_date: last_academic_year)
end

payment2 = create(:payment, :with_figures, claims: [claim], scheduled_payment_date: nil)
create(:topup, payment: payment2, claim: claim, award_amount: 500, created_by: user)

expect { personal_data_scrubber }.not_to change { claim.reload.attributes }
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
require "rails_helper"

RSpec.describe Policies::StudentLoans::ClaimPersonalDataScrubber do
it_behaves_like(
"a claim personal data scrubber",
Policies::StudentLoans
)
end
61 changes: 29 additions & 32 deletions spec/support/claim_personal_data_scrubber_shared_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,45 +75,37 @@
expect { personal_data_scrubber }.not_to change { claim.reload.attributes }
end

it "does not delete details from a claim that has a payment, but has a payrollable topup" do
eligibility = create(eligibility_factory, :eligible, award_amount: 1500.0)

claim = create(:claim, :approved, policy: policy, eligibility: eligibility)

create(:payment, :confirmed, :with_figures, claims: [claim], scheduled_payment_date: last_academic_year)
create(:topup, payment: nil, claim: claim, award_amount: 500, created_by: user)

expect { personal_data_scrubber }.not_to change { claim.reload.attributes }
end

it "does not delete details from a claim that has a payment, but has a payrolled topup without payment confirmation" do
it "deletes expected details from a claim with multiple payments all of which have been confirmed" do
claim = nil

travel_to 2.months.ago do
eligibility = create(eligibility_factory, :eligible, award_amount: 1500.0)
eligibility = build(eligibility_factory, :eligible)
# Student loans don't have a settable award amount
if eligibility.has_attribute?(:award_amount)
eligibility.award_amount = 1500.0
end
eligibility.save!
claim = create(:claim, :approved, policy: policy, eligibility: eligibility)
create(:payment, :confirmed, :with_figures, claims: [claim], scheduled_payment_date: last_academic_year)
end

payment2 = create(:payment, :with_figures, claims: [claim], scheduled_payment_date: nil)
create(:topup, payment: payment2, claim: claim, award_amount: 500, created_by: user)
payment2 = create(:payment, :confirmed, :with_figures, claims: [claim], scheduled_payment_date: last_academic_year)

expect { personal_data_scrubber }.not_to change { claim.reload.attributes }
end
if claim.topupable?
create(:topup, payment: payment2, claim: claim, award_amount: 500, created_by: user)
end

it "deletes expected details from a claim with multiple payments all of which have been confirmed" do
claim = nil
expect { personal_data_scrubber }.to change { claim.reload.attributes }
end

travel_to 2.months.ago do
eligibility = create(eligibility_factory, :eligible, award_amount: 1500.0)
claim = create(:claim, :approved, policy: policy, eligibility: eligibility)
create(:payment, :confirmed, :with_figures, claims: [claim], scheduled_payment_date: last_academic_year)
end
it "does not delete expected details from a claim for a different policy" do
other_policy = Policies::POLICIES.detect { |p| p != policy }

payment2 = create(:payment, :confirmed, :with_figures, claims: [claim], scheduled_payment_date: last_academic_year)
create(:topup, payment: payment2, claim: claim, award_amount: 500, created_by: user)
claim = create(:claim, :submitted, policy: other_policy)
create(:decision, :rejected, claim: claim, created_at: last_academic_year)
claim.update_attribute :hmrc_bank_validation_responses, ["test"]

expect { personal_data_scrubber }.to change { claim.reload.attributes }
expect { personal_data_scrubber }.not_to change { claim.reload.attributes }
end

it "deletes expected details from an old rejected claim, setting a personal_data_removed_at timestamp" do
Expand Down Expand Up @@ -182,13 +174,13 @@

it "only scrubs claims from the previous academic year" do
# Initialise the scrubber, and create a claim
scrubber = Claim::PersonalDataScrubber.new
scrubber = described_class.new

claim = create(:claim, :submitted, policy: policy)
create(:decision, :rejected, claim: claim)

travel_to(last_academic_year) do
claim = create(:claim, :submitted)
claim = create(:claim, :submitted, policy: policy)
create(:decision, :rejected, claim: claim)
end

Expand All @@ -204,15 +196,20 @@
claim, amendment = nil
travel_to last_academic_year - 1.week do
claim = create(:claim, :submitted, policy: policy)
amendment = create(:amendment, claim: claim, claim_changes: {
"teacher_reference_number" => [generate(:teacher_reference_number).to_s, claim.eligibility.teacher_reference_number],
claim_changes = {
"payroll_gender" => ["male", claim.payroll_gender],
"date_of_birth" => [25.years.ago.to_date, claim.date_of_birth],
"student_loan_plan" => ["plan_1", claim.student_loan_plan],
"bank_sort_code" => ["457288", claim.bank_sort_code],
"bank_account_number" => ["84818482", claim.bank_account_number],
"building_society_roll_number" => ["123456789/ABCD", claim.building_society_roll_number]
})
}

if claim.eligibility.has_attribute?(:teacher_reference_number)
claim_changes["teacher_reference_number"] = [generate(:teacher_reference_number).to_s, claim.eligibility.teacher_reference_number]
end

amendment = create(:amendment, claim: claim, claim_changes: claim_changes)
create(:decision, :approved, claim: claim, created_at: last_academic_year)
create(:payment, :confirmed, :with_figures, claims: [claim], scheduled_payment_date: last_academic_year)
end
Expand Down

0 comments on commit 973bfb6

Please sign in to comment.