Rewrite query with active record

Adds a new scope to claim  `with_award_amount` allowing for querying
claims in the db by their award amount.

app/models/claim.rb

@@ -214,6 +214,28 @@ class Claim < ApplicationRecord
       .where("further_education_payments_eligibilities.verification = '{}'")
   end
 
+  scope :with_award_amounts, -> do
+    joins(
+      <<~SQL
+        JOIN (
+          #{
+            Policies::POLICIES.map do |policy|
+              "
+                SELECT
+                id,
+                #{policy.award_amount_column} AS award_amount,
+                '#{policy::Eligibility}' AS eligibility_type
+                FROM #{policy::Eligibility.table_name}
+              "
+            end.join(" UNION ALL ")
+          }
+        ) AS eligibilities
+        ON claims.eligibility_id = eligibilities.id
+        AND claims.eligibility_type = eligibilities.eligibility_type
+      SQL
+    )
+  end
+
   def onelogin_idv_full_name
     "#{onelogin_idv_first_name} #{onelogin_idv_last_name}"
   end

app/models/payroll_run.rb

@@ -84,48 +84,34 @@ def payments_count
     @payments_count ||= payments.count
   end
 
+  class LineItem < Struct.new(:id, :award_amount, keyword_init: true); end
+
   def line_items(policy, filter: :all)
-    eligibilities_cte = "WITH eligibilities AS("
-    eligibilities_cte += Policies::POLICIES.map do |policy|
-      <<~SQL
-        SELECT
-        id,
-        #{policy.award_amount_column} AS award_amount,
-        '#{policy::Eligibility}' AS eligibility_type
-        FROM #{policy::Eligibility.table_name}
-      SQL
-    end.join(" UNION ALL ")
-    eligibilities_cte += ")"
-
-    sql = <<~SQL
-      #{eligibilities_cte}
-      SELECT
-        /* A topup is always paid in different payment/payroll_run than the main claim was */
+    scope = Claim
+      .select(
+        "
+        DISTINCT(claims.id),
         COALESCE(topups.award_amount, eligibilities.award_amount) AS award_amount
-      FROM payments
-      JOIN claim_payments ON claim_payments.payment_id = payments.id
-      JOIN claims ON claims.id = claim_payments.claim_id
-      JOIN eligibilities
-        ON claims.eligibility_id = eligibilities.id
-        AND claims.eligibility_type = eligibilities.eligibility_type
-      LEFT JOIN topups ON topups.claim_id = claims.id
-      WHERE payments.payroll_run_id = '#{id}'
-    SQL
-
-    unless policy == :all
-      sql += "\nAND claims.eligibility_type = 'Policies::#{policy}::Eligibility'"
-    end
+        "
+      )
+      .with_award_amounts
+      .left_joins(:topups)
+      .joins(payments: :payroll_run)
+      .where(payroll_runs: {id: id})
+
+    scope = scope.by_policy(policy) unless policy == :all
 
     case filter
-    when :all
-      sql
     when :claims
-      sql += "\nAND topups.id IS NULL"
+      scope = scope.where(topups: {id: nil})
     when :topups
-      sql += "\nAND topups.id IS NOT NULL"
+      scope = scope.where.not(topups: {id: nil})
     end
 
-    ActiveRecord::Base.connection.execute(sql).map(&OpenStruct.method(:new))
+    # Claim delegates it's award amount to eligibility, so we want to return
+    # a non claim object ensuring the award amount is from the topup if there
+    # is one
+    ActiveRecord::Base.connection.execute(scope.to_sql).map(&LineItem.method(:new))
   end
 
   def ensure_no_payroll_run_this_month

config/brakeman.ignore

@@ -30,7 +30,7 @@
       "check_name": "SQL",
       "message": "Possible SQL injection",
       "file": "app/models/claim/search.rb",
-      "line": 45,
+      "line": 35,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
       "code": "Claim.submitted.where(\"LOWER(#{attribute}) = LOWER(?)\", search_term)",
       "render_path": null,
@@ -115,14 +115,37 @@
       ],
       "note": ""
     },
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "cddc34b96e07d19510d574438a1ce61b69d9c1f28c0d4e5eee631d17a9b38ba6",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/models/claim.rb",
+      "line": 222,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "joins(\"JOIN (\\n  #{Policies::POLICIES.map do\n \"\\n                SELECT\\n                id,\\n                #{policy.award_amount_column} AS award_amount,\\n                '#{policy::Eligibility}' AS eligibility_type\\n                FROM #{policy::Eligibility.table_name}\\n              \"\n end.join(\" UNION ALL \")}\\n) AS eligibilities\\nON claims.eligibility_id = eligibilities.id\\nAND claims.eligibility_type = eligibilities.eligibility_type\\n\")",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Claim",
+        "method": "with_award_amounts"
+      },
+      "user_input": "Policies::POLICIES.map do\n \"\\n                SELECT\\n                id,\\n                #{policy.award_amount_column} AS award_amount,\\n                '#{policy::Eligibility}' AS eligibility_type\\n                FROM #{policy::Eligibility.table_name}\\n              \"\n end.join(\" UNION ALL \")",
+      "confidence": "Medium",
+      "cwe_id": [
+        89
+      ],
+      "note": "Non of the columns used in generating the join are user provided"
+    },
     {
       "warning_type": "SQL Injection",
       "warning_code": 0,
       "fingerprint": "f83635b54e1ce0088178d8082ffe632ab8aa81b10fce1026caf87f286cb4d81a",
       "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,
@@ -139,6 +162,6 @@
       "note": ""
     }
   ],
-  "updated": "2024-06-20 16:06:05 +0100",
-  "brakeman_version": "6.1.2"
+  "updated": "2024-10-14 12:08:54 +0100",
+  "brakeman_version": "6.2.1"
 }