Merge pull request #2375 from ministryofjustice/check-your-claim-page…

…-fixes

"Check your claim" page fixes

.rubocop.yml

@@ -38,6 +38,7 @@ Metrics/BlockLength:
 
 Metrics/ModuleLength:
   Exclude:
+    - 'app/models/claims/calculations.rb'
     - 'app/models/claims/state_machine.rb'
     - 'app/helpers/application_helper.rb'
 

app/controllers/external_users/litigators/interim_claims_controller.rb

@@ -7,7 +7,6 @@ class ExternalUsers::Litigators::InterimClaimsController < ExternalUsers::Claims
 
   def build_nested_resources
     @claim.build_interim_fee if @claim.interim_fee.nil?
-    @claim.build_warrant_fee if @claim.warrant_fee.nil?
 
     %i[disbursements expenses].each do |association|
       build_nested_resource(@claim, association)

app/models/claim/interim_claim.rb

@@ -71,10 +71,8 @@ class InterimClaim < BaseClaim
     validates_with ::Claim::InterimClaimSubModelValidator
 
     has_one :interim_fee, foreign_key: :claim_id, class_name: 'Fee::InterimFee', dependent: :destroy, inverse_of: :claim
-    has_one :warrant_fee, foreign_key: :claim_id, class_name: 'Fee::WarrantFee', dependent: :destroy, inverse_of: :claim
 
     accepts_nested_attributes_for :interim_fee, reject_if: :all_blank, allow_destroy: false
-    accepts_nested_attributes_for :warrant_fee, reject_if: :all_blank, allow_destroy: false
 
     before_validation do
       assign_total_attrs
@@ -151,15 +149,10 @@ def provider_delegator
     end
 
     def destroy_all_invalid_fee_types
-      return unless interim_fee
-
-      if interim_fee.is_interim_warrant?
-        disbursements.destroy_all
-        self.disbursements = []
-      else
-        warrant_fee.try(:destroy)
-        self.warrant_fee = nil
-      end
+      return unless interim_fee&.is_interim_warrant?
+
+      disbursements.destroy_all
+      self.disbursements = []
     end
 
     def assign_total_attrs
@@ -169,17 +162,22 @@ def assign_total_attrs
       return if from_api?
       assign_fees_total(%i[interim_fee]) if interim_fee_changed?
       assign_expenses_total if expenses_changed?
+      assign_disbursements_total if disbursements_changed?
       return unless total_changes_required?
       assign_total
       assign_vat
     end
 
     def total_changes_required?
-      interim_fee_changed? || expenses_changed?
+      interim_fee_changed? || expenses_changed? || disbursements_changed?
     end
 
     def interim_fee_changed?
       interim_fee&.changed?
     end
+
+    def disbursements_changed?
+      disbursements.any?(&:changed?)
+    end
   end
 end

app/models/claims/calculations.rb

@@ -38,6 +38,17 @@ def totalize_for_claim(klass, claim_id, net_attribute, vat_attribute)
     { vat: values.map { |v| v.first || BigDecimal.new(0.0, 8) }.sum, net: values.map(&:last).sum }
   end
 
+  # NOTE: This is meant to reproduce the same behaviour as totalize_for_claim
+  # but doing all the operations in memory without hitting the DB
+  def calculate_association_total(association_name, net_attribute, vat_attribute)
+    records = public_send(association_name)
+    records.each_with_object(vat: 0, net: 0) do |record, memo|
+      next if record.marked_for_destruction? || record.public_send(net_attribute).nil?
+      memo[:vat] += (record.public_send(vat_attribute) || BigDecimal.new(0.0, 8))
+      memo[:net] += record.public_send(net_attribute)
+    end
+  end
+
   def attribute_is_null_to_s(net_attribute)
     "#{net_attribute} IS NOT NULL"
   end
@@ -77,6 +88,13 @@ def assign_expenses_total
                       value_band_id: Claims::ValueBands.band_id_for_value(totals[:net] + totals[:vat]))
   end
 
+  def assign_disbursements_total
+    totals = calculate_association_total(:disbursements, :net_amount, :vat_amount)
+    assign_attributes(disbursements_vat: totals[:vat],
+                      disbursements_total: totals[:net],
+                      value_band_id: Claims::ValueBands.band_id_for_value(totals[:net] + totals[:vat]))
+  end
+
   def update_expenses_total
     totals = totalize_for_claim(Expense, id, :amount, :vat_amount)
     update_columns(expenses_vat: totals[:vat],

app/presenters/claim/interim_claim_presenter.rb

@@ -1,5 +1,5 @@
 class Claim::InterimClaimPresenter < Claim::BaseClaimPresenter
-  present_with_currency :interim_fees_total, :warrant_fees_total
+  present_with_currency :interim_fees_total
 
   # NOTE: this shows we should probably refactor the template naming
   # to bring some consistency between claim steps and their associated
@@ -9,6 +9,7 @@ class Claim::InterimClaimPresenter < Claim::BaseClaimPresenter
     defendants: :defendants,
     offence_details: :offence_details,
     interim_fee: :interim_fees,
+    disbursements: :interim_fees,
     expenses: :travel_expenses,
     supporting_evidence: :supporting_evidence,
     additional_information: :supporting_evidence
@@ -42,10 +43,6 @@ def raw_interim_fees_total
     claim.interim_fee&.amount || 0
   end
 
-  def raw_warrant_fees_total
-    claim.warrant_fee&.amount || 0
-  end
-
   def summary_sections
     SUMMARY_SECTIONS
   end

app/views/external_users/claims/_summary_claims_content.html.haml

@@ -16,4 +16,4 @@
 
 - claim.summary_sections.each do |section, associated_step|
   - if claim.accessible_step?(associated_step)
-    = render partial: "external_users/claims/#{section}/summary", locals: { claim: claim, section: section, editable: claim.editable_step?(associated_step) }
+    = render partial: "external_users/claims/#{section}/summary", locals: { claim: claim, step: associated_step, section: section, editable: claim.editable_step?(associated_step) }

app/views/external_users/claims/disbursements/_summary.html.haml

@@ -1,12 +1,13 @@
+- step = local_assigns[:step] ? local_assigns[:step] : :disbursements
 .form-section
   %h3.heading-medium
     = t('external_users.claims.disbursements.summary.header')
     - if local_assigns[:editable]
-      = link_to 'Change', edit_polymorphic_path(claim, step: :disbursements, referrer: :summary), class: 'link-change'
+      = link_to 'Change', edit_polymorphic_path(claim, step: step, referrer: :summary), class: 'link-change'
 
   - if claim.disbursements.empty?
     - if local_assigns.has_key?(:editable) && !local_assigns[:editable]
-      = render partial: 'external_users/claims/summary/section_status', locals: { claim: claim, section: section, step: :disbursements }
+      = render partial: 'external_users/claims/summary/section_status', locals: { claim: claim, section: section, step: step }
     - else
       %p
         = t('shared.summary.no_values.disbursements')

app/views/external_users/claims/offence_details/_default_summary.html.haml

@@ -1,11 +1,10 @@
 - claim_fields = %i[external_users claims case_details fields]
 
-- if claim.agfs?
-  %tr
-    %th{ scope: 'row', class: 'bold' }
-      = t("offence_category", scope: claim_fields)
-    %td
-      = claim.offence ? claim.offence.description : t("general.not_provided")
+%tr
+  %th{ scope: 'row', class: 'bold' }
+    = t("offence_category", scope: claim_fields)
+  %td
+    = claim.offence ? claim.offence.description : t("general.not_provided")
 
 %tr
   %th{ scope: 'row', class: 'bold' }

spec/presenters/claim/interim_claim_presenter_spec.rb

@@ -59,37 +59,14 @@
     end
   end
 
-  describe '#raw_warrant_fees_total' do
-    context 'when the warrant fee is nil' do
-      let(:claim) { build(:interim_claim, warrant_fee: nil) }
-
-      specify { expect(presenter.raw_warrant_fees_total).to eq(0) }
-    end
-
-    context 'when the warrant fee is set' do
-      let(:claim) { build(:interim_claim, warrant_fee: warrant_fee) }
-
-      context 'but amount is not set' do
-        let(:warrant_fee) { build(:warrant_fee, amount: nil) }
-
-        specify { expect(presenter.raw_warrant_fees_total).to eq(0) }
-      end
-
-      context 'and amount is set' do
-        let(:warrant_fee) { build(:warrant_fee, amount: 42.5) }
-
-        specify { expect(presenter.raw_warrant_fees_total).to eq(42.5) }
-      end
-    end
-  end
-
   describe '#summary_sections' do
     specify {
       expect(presenter.summary_sections).to eq({
         case_details: :case_details,
         defendants: :defendants,
         offence_details: :offence_details,
         interim_fee: :interim_fees,
+        disbursements: :interim_fees,
         expenses: :travel_expenses,
         supporting_evidence: :supporting_evidence,
         additional_information: :supporting_evidence