Skip to content

Commit

Permalink
remove Claim.submitted scope
Browse files Browse the repository at this point in the history
  • Loading branch information
alkesh committed Oct 24, 2024
1 parent cb38b3d commit 7736fbd
Show file tree
Hide file tree
Showing 15 changed files with 52 additions and 83 deletions.
4 changes: 2 additions & 2 deletions app/controllers/admin/page_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ class PageController < BaseAdminController
def index
flash[:notice] = "There is currently no School Workforce Census data present" if SchoolWorkforceCensus.all.size.zero?

@claims_received = Claim.current_academic_year.submitted.count
@claims_received = Claim.current_academic_year.count
@claims_approved = Claim.current_academic_year.approved.count
@claims_rejected = Claim.current_academic_year.rejected.count

@total_claims_received = Claim.submitted.count
@total_claims_received = Claim.count
@claims_approaching_deadline = Claim.approaching_decision_deadline.count
@claims_passed_decision_deadline = Claim.passed_decision_deadline.count

Expand Down
4 changes: 1 addition & 3 deletions app/models/claim.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,10 @@ class Claim < ApplicationRecord
before_save :normalise_first_name, if: %i[first_name first_name_changed?]
before_save :normalise_surname, if: %i[surname surname_changed?]

scope :submitted, -> { where.not(submitted_at: nil) }
scope :held, -> { where(held: true) }
scope :not_held, -> { where(held: false) }
scope :awaiting_decision, -> do
submitted
.joins("LEFT OUTER JOIN decisions ON decisions.claim_id = claims.id AND decisions.undone = false")
joins("LEFT OUTER JOIN decisions ON decisions.claim_id = claims.id AND decisions.undone = false")
.where(decisions: {claim_id: nil})
end
scope :awaiting_task, ->(task_name) { awaiting_decision.joins(sanitize_sql(["LEFT OUTER JOIN tasks ON tasks.claim_id = claims.id AND tasks.name = ?", task_name])).where(tasks: {claim_id: nil}) }
Expand Down
2 changes: 1 addition & 1 deletion app/models/claim/matching_attribute_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def policies_to_find_matches_eligibility_types
end

def claims_to_compare
Claim.submitted
Claim
.by_academic_year(@source_claim.academic_year)
.by_policies(policies_to_find_matches)
.where.not(id: @source_claim.id)
Expand Down
8 changes: 4 additions & 4 deletions app/models/school_workforce_census.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,17 @@ def grouped_census_subjects_taught_totals
end

def any_match_count
return 0.0 if Claim.submitted.count.zero?
return 0.0 if Claim.count.zero?

any_match_count = grouped_census_subjects_taught_totals["any"].to_i ||= 0
((any_match_count / Claim.submitted.count.to_f) * 100).round(1)
((any_match_count / Claim.count.to_f) * 100).round(1)
end

def no_data_census_subjects_taught_count
return 0.0 if Claim.submitted.count.zero?
return 0.0 if Claim.count.zero?

count = grouped_census_subjects_taught_totals[nil].to_i ||= 0
((count / Claim.submitted.count.to_f) * 100).round(1)
((count / Claim.count.to_f) * 100).round(1)
end
end
end
56 changes: 28 additions & 28 deletions config/brakeman.ignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,37 +23,14 @@
],
"note": ""
},
{
"warning_type": "SQL Injection",
"warning_code": 0,
"fingerprint": "0ac253105d2af7d0dd324537b4be4a392ded73c0f3df8ca4d32db09cac7b586d",
"check_name": "SQL",
"message": "Possible SQL injection",
"file": "app/models/claim/search.rb",
"line": 45,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "Claim.submitted.where(\"LOWER(#{attribute}) = LOWER(?)\", search_term)",
"render_path": null,
"location": {
"type": "method",
"class": "Search",
"method": "search_by"
},
"user_input": "attribute",
"confidence": "Medium",
"cwe_id": [
89
],
"note": ""
},
{
"warning_type": "Dynamic Render Path",
"warning_code": 15,
"fingerprint": "23e1b8f42ab87b9f17f883de7ac8b7b11261ec3dd8f8ad11f06aaae2a3b6fc51",
"check_name": "Render",
"message": "Render path contains parameter value",
"file": "app/controllers/admin/tasks_controller.rb",
"line": 49,
"line": 47,
"link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
"code": "render(action => Claim.includes(:tasks).find(params[:claim_id]).tasks.where(:name => params[:name]).first.name, {})",
"render_path": null,
Expand All @@ -76,7 +53,7 @@
"check_name": "Render",
"message": "Render path contains parameter value",
"file": "app/controllers/admin/tasks_controller.rb",
"line": 22,
"line": 20,
"link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
"code": "render(action => Claim.includes(:tasks).find(params[:claim_id]).tasks.find_or_initialize_by(:name => params[:name]).name, {})",
"render_path": null,
Expand All @@ -92,6 +69,29 @@
],
"note": "Constrained to valid input by routes"
},
{
"warning_type": "SQL Injection",
"warning_code": 0,
"fingerprint": "46bfd0a9d4a19eb048a883184b501b060aa4d6006accc3c76bbfc00722b44dbf",
"check_name": "SQL",
"message": "Possible SQL injection",
"file": "app/models/claim/search.rb",
"line": 35,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "Claim.where(\"LOWER(#{attribute}) = LOWER(?)\", search_term)",
"render_path": null,
"location": {
"type": "method",
"class": "Search",
"method": "search_by"
},
"user_input": "attribute",
"confidence": "Medium",
"cwe_id": [
89
],
"note": ""
},
{
"warning_type": "File Access",
"warning_code": 16,
Expand Down Expand Up @@ -122,7 +122,7 @@
"check_name": "SQL",
"message": "Possible SQL injection",
"file": "app/models/claim/matching_attribute_finder.rb",
"line": 54,
"line": 34,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "Claim.where(\"LOWER(#{\"CONCAT(#{attributes.join(\",\")})\"}) = LOWER(?)\", values_for_attributes(source_claim, attributes).join)",
"render_path": null,
Expand All @@ -139,6 +139,6 @@
"note": ""
}
],
"updated": "2024-06-20 16:06:05 +0100",
"brakeman_version": "6.1.2"
"updated": "2024-10-23 16:53:59 +0100",
"brakeman_version": "6.2.1"
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@
end

trait :with_trn do
eligible
teacher_reference_number { generate(:teacher_reference_number) }
end
end
Expand Down
10 changes: 0 additions & 10 deletions spec/features/admin/admin_search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

let!(:claim1) { create(:claim, :submitted, surname: "Wayne") }
let!(:claim2) { create(:claim, :submitted, surname: "Wayne") }
let!(:unsubmitted_claim) { create(:claim, surname: "Smith") }

scenario "redirects a service operator to the claim if there is only one match" do
visit search_admin_claims_path
Expand Down Expand Up @@ -38,13 +37,4 @@
expect(page).to have_content(claim1.eligibility.teacher_reference_number)
expect(page).to have_content(claim1.policy.short_name)
end

scenario "unsubmitted claims filtered from search results" do
visit search_admin_claims_path

fill_in :reference, with: "Smith"
click_button "Search"

expect(page).to have_content("Cannot find a claim for query \"Smith\"")
end
end
1 change: 0 additions & 1 deletion spec/features/admin/admin_stats_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
let!(:school_workforce_census_task_no_data) { create(:task, claim: submitted_claims.third, name: "census_subjects_taught") }
before do
@approved_claims = create_list(:claim, 3, :approved, submitted_at: 10.weeks.ago)
@unfinished_claims = create_list(:claim, 1, :submittable)
@rejected_claims = create_list(:claim, 1, :rejected)
@claims_approaching_deadline = create_list(:claim, 2, :submitted, submitted_at: (Claim::DECISION_DEADLINE - 1.week).ago)
@claims_passed_deadline = create_list(:claim, 1, :submitted, submitted_at: (Claim::DECISION_DEADLINE + 1.week).ago)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

RSpec.feature "Admin view claim for FurtherEducationPayments" do
let!(:journey_configuration) { create(:journey_configuration, "further_education_payments") }
let(:eligibility_with_trn) { create(:further_education_payments_eligibility, :eligible, :with_trn) }
let!(:claim) {
create(
:claim,
Expand All @@ -16,7 +15,7 @@
:claim,
:submitted,
policy: Policies::FurtherEducationPayments,
eligibility: eligibility_with_trn
eligibility_trait: :with_trn
)
}
let!(:claim_not_verified) {
Expand Down Expand Up @@ -58,8 +57,7 @@
end

scenario "view claim summary for claim with no TRN" do
visit admin_claims_path
find("a[href='#{admin_claim_tasks_path(claim)}']").click
visit admin_claim_tasks_path(claim)
expect(page).not_to have_content("Claim route")
expect(page).not_to have_content("Not signed in with DfE Identity")
expect(page).to have_content("Not provided")
Expand All @@ -68,8 +66,7 @@
end

scenario "view claim summary for claim with TRN" do
visit admin_claims_path
find("a[href='#{admin_claim_tasks_path(claim_with_trn)}']").click
visit admin_claim_tasks_path(claim_with_trn)
expect(page).not_to have_content("Not provided")
expect(page).to have_content(claim_with_trn.eligibility.teacher_reference_number)
expect(page).to have_content("UK Provider Reference Number (UKPRN)")
Expand Down
2 changes: 1 addition & 1 deletion spec/features/student_loans_claim_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def fill_in_remaining_personal_details_and_submit

fill_in_remaining_personal_details_and_submit

claim = Claim.submitted.order(:created_at).last
claim = Claim.order(:created_at).last
expect(claim.submitted_using_slc_data).to eql(true)
end

Expand Down
4 changes: 0 additions & 4 deletions spec/mailers/previews/claim_mailer_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ def update_after_three_weeks_lup
ClaimMailer.update_after_three_weeks(claim_for(Claim.approved.by_policy(Policies::LevellingUpPremiumPayments)))
end

def email_verification
ClaimMailer.email_verification(claim_for(Claim.unsubmitted), OneTimePassword::Generator.new.code)
end

private

def claim_for(scope)
Expand Down
6 changes: 0 additions & 6 deletions spec/models/claim/matching_attribute_finder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@
expect(matching_claims).to be_empty
end

it "does not include unsubmitted claims with matching attributes" do
create(:claim, :submittable, eligibility_attributes: {teacher_reference_number: source_claim.eligibility.teacher_reference_number})

expect(matching_claims).to be_empty
end

it "does not include claims that match, but have a different policy" do
student_loans_claim = create(:claim, :submitted, eligibility_attributes: {teacher_reference_number: source_claim.eligibility.teacher_reference_number}, policy: Policies::StudentLoans)

Expand Down
1 change: 0 additions & 1 deletion spec/models/claim/school_check_email_data_export_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
create(:claim, :submitted, eligibility: eligibility, first_name: "John", middle_name: "Herbert", surname: "Adams")
end
let!(:submitted_claims) { [submitted_student_loans_claim] }
let!(:submittable_claims) { create_list(:claim, 4, :submittable) }
let!(:excluded_claims) { create_list(:claim, 3, :submitted) }
let!(:approved_claims) { create_list(:claim, 2, :approved) }
let!(:rejected_claims) { create_list(:claim, 2, :rejected) }
Expand Down
1 change: 0 additions & 1 deletion spec/models/claim_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@

describe "scopes" do
let!(:submitted_claims) { create_list(:claim, 5, :submitted) }
let!(:unsubmitted_claims) { create_list(:claim, 2, :submittable) }
let!(:approved_claims) { create_list(:claim, 5, :approved) }
let!(:rejected_claims) { create_list(:claim, 5, :rejected) }

Expand Down
26 changes: 11 additions & 15 deletions spec/support/admin_view_claim_feature_shared_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,27 @@
let(:academic_year) { journey_configuration.current_academic_year }

let!(:claim) {
eligibility = create(:"#{policy.to_s.underscore}_eligibility", :eligible)
create(
:claim,
:submitted,
policy: policy,
eligibility: eligibility,
first_name: Faker::Name.first_name,
surname: Faker::Name.last_name
surname: Faker::Name.last_name,
eligibility_trait: :eligible
)
}

let!(:multiple_claim) {
eligibility = if policy == Policies::FurtherEducationPayments
create(:"#{policy.to_s.underscore}_eligibility", :eligible, :with_trn)
eligibility_trait = if policy == Policies::FurtherEducationPayments
:with_trn
else
create(:"#{policy.to_s.underscore}_eligibility", :eligible)
:eligible
end
create(
:claim,
:submitted,
policy: policy,
eligibility: eligibility
eligibility_trait: eligibility_trait
)
}

Expand All @@ -34,42 +33,39 @@
else
{teacher_reference_number: multiple_claim.eligibility.teacher_reference_number}
end
eligibility = create(:"#{policy.to_s.underscore}_eligibility", :eligible, duplicate_attribute)
create(
:claim,
:submitted,
policy: policy,
eligibility: eligibility
eligibility_trait: :eligible,
eligibility_attributes: duplicate_attribute
)
}

let!(:approved_awaiting_payroll_claim) {
eligibility = create(:"#{policy.to_s.underscore}_eligibility", :eligible)
create(
:claim,
:payrollable,
policy: policy,
eligibility: eligibility
eligibility_trait: :eligible
)
}

let!(:approved_paid_claim) {
eligibility = create(:"#{policy.to_s.underscore}_eligibility", :eligible)
create(
:claim,
:approved,
policy: policy,
eligibility: eligibility
eligibility_trait: :eligible
)
}

let!(:rejected_claim) {
eligibility = create(:"#{policy.to_s.underscore}_eligibility", :eligible)
create(
:claim,
:rejected,
policy: policy,
eligibility: eligibility
eligibility_trait: :eligible
)
}

Expand Down

0 comments on commit 7736fbd

Please sign in to comment.