From 98634bc6366dd02ca59773a6c09752f15ad1998a Mon Sep 17 00:00:00 2001 From: Kenneth Lee Date: Tue, 10 Sep 2024 15:52:56 +0100 Subject: [PATCH 1/2] LUPEYALPHA-1015 - FE claims with no TRN aren't considered matching other claims with no TRN * TRN is optional on FE claims. * The matching logic didn't exclude fields if they are blank, so all FE claims with a blank TRN are considered a duplicate. * This excludes querying attributes if the source claim value for that attribute is blank. --- app/models/claim/matching_attribute_finder.rb | 15 ++++- .../claim/matching_attribute_finder_spec.rb | 56 +++++++++++++++++++ ...dmin_view_claim_feature_shared_examples.rb | 8 ++- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/app/models/claim/matching_attribute_finder.rb b/app/models/claim/matching_attribute_finder.rb index 710601bb92..185c269a4f 100644 --- a/app/models/claim/matching_attribute_finder.rb +++ b/app/models/claim/matching_attribute_finder.rb @@ -38,13 +38,16 @@ def matching_claims # Eligibility attributes eligibility_ids = eligibility_attributes_groups_to_match.map { |attributes| - vals = values_for_attributes(@source_claim.eligibility, attributes) - concatenated_columns = "CONCAT(#{attributes.join(",")})" + present_attributes = attributes_for(@source_claim.eligibility, attributes).reject { |_, v| v.blank? }.map(&:first) + next if present_attributes.empty? + + vals = values_for_attributes(@source_claim.eligibility, present_attributes) + concatenated_columns = "CONCAT(#{present_attributes.join(",")})" policies_to_find_matches.map { |policy| policy::Eligibility.where("LOWER(#{concatenated_columns}) = LOWER(?)", vals.join) } - }.flatten.map(&:id) + }.compact.flatten.map(&:id) eligibility_match_query = Claim.where(eligibility_id: eligibility_ids) match_queries = match_queries.or(eligibility_match_query) @@ -78,5 +81,11 @@ def values_for_attributes(object, attributes) object.read_attribute(attribute) }.reject(&:blank?) end + + def attributes_for(object, attributes) + attributes.map { |attribute| + [attribute, object.read_attribute(attribute)] + } + end end end diff --git a/spec/models/claim/matching_attribute_finder_spec.rb b/spec/models/claim/matching_attribute_finder_spec.rb index c1e868a0dd..7105f7ef6b 100644 --- a/spec/models/claim/matching_attribute_finder_spec.rb +++ b/spec/models/claim/matching_attribute_finder_spec.rb @@ -173,4 +173,60 @@ expect(matching_claims).to be_empty end end + + describe "matching_claims - blank trn" do + let(:policy) { Policies::FurtherEducationPayments } + + let!(:source_claim) { + eligibility = create(:further_education_payments_eligibility, :eligible) + create( + :claim, + :submitted, + policy: policy, + eligibility: eligibility + ) + } + + let!(:other_claim) { + eligibility = create(:further_education_payments_eligibility, :eligible) + create( + :claim, + :submitted, + policy: policy, + eligibility: eligibility + ) + } + + subject(:matching_claims) { Claim::MatchingAttributeFinder.new(source_claim).matching_claims } + + it { is_expected.to be_empty } + end + + describe "matching_claims - same trn" do + let(:policy) { Policies::FurtherEducationPayments } + + let!(:source_claim) { + eligibility = create(:further_education_payments_eligibility, :eligible, :with_trn) + create( + :claim, + :submitted, + policy: policy, + eligibility: eligibility + ) + } + + let!(:other_claim) { + eligibility = create(:further_education_payments_eligibility, :eligible, teacher_reference_number: source_claim.eligibility.teacher_reference_number) + create( + :claim, + :submitted, + policy: policy, + eligibility: eligibility + ) + } + + subject(:matching_claims) { Claim::MatchingAttributeFinder.new(source_claim).matching_claims } + + it { is_expected.to eq [other_claim] } + end end diff --git a/spec/support/admin_view_claim_feature_shared_examples.rb b/spec/support/admin_view_claim_feature_shared_examples.rb index 555dad3a50..261f477709 100644 --- a/spec/support/admin_view_claim_feature_shared_examples.rb +++ b/spec/support/admin_view_claim_feature_shared_examples.rb @@ -13,7 +13,11 @@ } let!(:multiple_claim) { - eligibility = create(:"#{policy.to_s.underscore}_eligibility", :eligible) + eligibility = if policy == Policies::FurtherEducationPayments + create(:"#{policy.to_s.underscore}_eligibility", :eligible, :with_trn) + else + create(:"#{policy.to_s.underscore}_eligibility", :eligible) + end create( :claim, :submitted, @@ -158,7 +162,7 @@ def expect_page_to_have_policy_sections(policy) when Policies::InternationalRelocationPayments ["Identity confirmation", "Visa", "Arrival date", "Employment", "Employment contract", "Employment start", "Subject", "Teaching hours", "Decision"] when Policies::FurtherEducationPayments - ["Identity confirmation", "Provider verification", "Student loan plan", "Payroll details", "Matching details", "Decision"] + ["Identity confirmation", "Provider verification", "Student loan plan", "Payroll details", "Decision"] else raise "Unimplemented policy: #{policy}" end From 23849178b897ccbc3499301b3004fc01e56c3255 Mon Sep 17 00:00:00 2001 From: Kenneth Lee Date: Wed, 11 Sep 2024 14:53:50 +0100 Subject: [PATCH 2/2] No need to selectively filter blank fields --- app/models/claim/matching_attribute_finder.rb | 15 ++------- .../claim/matching_attribute_finder_spec.rb | 32 +++++++++++++++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/app/models/claim/matching_attribute_finder.rb b/app/models/claim/matching_attribute_finder.rb index 185c269a4f..400a08f559 100644 --- a/app/models/claim/matching_attribute_finder.rb +++ b/app/models/claim/matching_attribute_finder.rb @@ -27,7 +27,6 @@ def matching_claims # Claim attributes CLAIM_ATTRIBUTE_GROUPS_TO_MATCH.each do |attributes| vals = values_for_attributes(@source_claim, attributes) - next if vals.blank? concatenated_columns = "CONCAT(#{attributes.join(",")})" @@ -38,12 +37,10 @@ def matching_claims # Eligibility attributes eligibility_ids = eligibility_attributes_groups_to_match.map { |attributes| - present_attributes = attributes_for(@source_claim.eligibility, attributes).reject { |_, v| v.blank? }.map(&:first) - next if present_attributes.empty? - - vals = values_for_attributes(@source_claim.eligibility, present_attributes) - concatenated_columns = "CONCAT(#{present_attributes.join(",")})" + vals = values_for_attributes(@source_claim.eligibility, attributes) + next if vals.blank? + concatenated_columns = "CONCAT(#{attributes.join(",")})" policies_to_find_matches.map { |policy| policy::Eligibility.where("LOWER(#{concatenated_columns}) = LOWER(?)", vals.join) } @@ -81,11 +78,5 @@ def values_for_attributes(object, attributes) object.read_attribute(attribute) }.reject(&:blank?) end - - def attributes_for(object, attributes) - attributes.map { |attribute| - [attribute, object.read_attribute(attribute)] - } - end end end diff --git a/spec/models/claim/matching_attribute_finder_spec.rb b/spec/models/claim/matching_attribute_finder_spec.rb index 7105f7ef6b..0cd1426e42 100644 --- a/spec/models/claim/matching_attribute_finder_spec.rb +++ b/spec/models/claim/matching_attribute_finder_spec.rb @@ -229,4 +229,36 @@ it { is_expected.to eq [other_claim] } end + + describe "matching_claims - blank trn, another field group with same contract type, blank provision_search" do + before do + stub_const("Policies::FurtherEducationPayments::ELIGIBILITY_MATCHING_ATTRIBUTES", [["teacher_reference_number"], ["provision_search", "contract_type"]]) + end + + let(:policy) { Policies::FurtherEducationPayments } + + let!(:source_claim) { + eligibility = create(:further_education_payments_eligibility, :eligible, contract_type: "permanent", provision_search: nil) + create( + :claim, + :submitted, + policy: policy, + eligibility: eligibility + ) + } + + let!(:other_claim) { + eligibility = create(:further_education_payments_eligibility, :eligible, contract_type: "permanent", provision_search: nil) + create( + :claim, + :submitted, + policy: policy, + eligibility: eligibility + ) + } + + subject(:matching_claims) { Claim::MatchingAttributeFinder.new(source_claim).matching_claims } + + it { is_expected.to eq [other_claim] } + end end