diff --git a/.rubocop.yml b/.rubocop.yml index 55e245d01a..50165223d6 100644 --- a/.rubocop.yml +++ b/.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' diff --git a/app/controllers/external_users/litigators/interim_claims_controller.rb b/app/controllers/external_users/litigators/interim_claims_controller.rb index 71a8a036e0..69acac7198 100644 --- a/app/controllers/external_users/litigators/interim_claims_controller.rb +++ b/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) diff --git a/app/models/claim/interim_claim.rb b/app/models/claim/interim_claim.rb index 74d5b228e0..b384bbbaa9 100644 --- a/app/models/claim/interim_claim.rb +++ b/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 diff --git a/app/models/claims/calculations.rb b/app/models/claims/calculations.rb index e22b4c7e74..250cdd2d86 100644 --- a/app/models/claims/calculations.rb +++ b/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], diff --git a/app/presenters/claim/interim_claim_presenter.rb b/app/presenters/claim/interim_claim_presenter.rb index 676f9e66b1..c2d7b3c138 100644 --- a/app/presenters/claim/interim_claim_presenter.rb +++ b/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 diff --git a/app/views/external_users/claims/_summary_claims_content.html.haml b/app/views/external_users/claims/_summary_claims_content.html.haml index 4fd7786bce..024bb5f18b 100644 --- a/app/views/external_users/claims/_summary_claims_content.html.haml +++ b/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) } diff --git a/app/views/external_users/claims/disbursements/_summary.html.haml b/app/views/external_users/claims/disbursements/_summary.html.haml index 34aa055b94..17eab976ee 100644 --- a/app/views/external_users/claims/disbursements/_summary.html.haml +++ b/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') diff --git a/app/views/external_users/claims/offence_details/_default_summary.html.haml b/app/views/external_users/claims/offence_details/_default_summary.html.haml index c2cf0585d9..58b5b0ddb2 100644 --- a/app/views/external_users/claims/offence_details/_default_summary.html.haml +++ b/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' } diff --git a/spec/presenters/claim/interim_claim_presenter_spec.rb b/spec/presenters/claim/interim_claim_presenter_spec.rb index 582d0b3552..924613ae13 100644 --- a/spec/presenters/claim/interim_claim_presenter_spec.rb +++ b/spec/presenters/claim/interim_claim_presenter_spec.rb @@ -59,30 +59,6 @@ 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({ @@ -90,6 +66,7 @@ defendants: :defendants, offence_details: :offence_details, interim_fee: :interim_fees, + disbursements: :interim_fees, expenses: :travel_expenses, supporting_evidence: :supporting_evidence, additional_information: :supporting_evidence