From 963bf79a164139e612f42f68ee0ba767be0b6fc2 Mon Sep 17 00:00:00 2001 From: Sergio Marques Date: Tue, 22 May 2018 13:26:41 +0100 Subject: [PATCH 1/4] Ensure disbursements are added to the claim totals if changed For interim claims, if disbursements only are supplied then update totals with disbursements totals before validations are performed so the validations work as expected and the user does not get blocked on the page with a total value claimed error when they have submitted the correct information. --- .rubocop.yml | 1 + app/models/claim/interim_claim.rb | 7 ++++++- app/models/claims/calculations.rb | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) 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/models/claim/interim_claim.rb b/app/models/claim/interim_claim.rb index 74d5b228e0..e0e4745aa5 100644 --- a/app/models/claim/interim_claim.rb +++ b/app/models/claim/interim_claim.rb @@ -169,17 +169,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], From 28e4219f83898aeb6fe957f7edcdc7637deb4302 Mon Sep 17 00:00:00 2001 From: Sergio Marques Date: Tue, 22 May 2018 14:22:02 +0100 Subject: [PATCH 2/4] Remove reference to warrant_fee association for LGFS interim claims LGFS interim claim do not have a warrant fee association but instead can have an interim fee which is of type warrant. --- .../litigators/interim_claims_controller.rb | 1 - app/models/claim/interim_claim.rb | 15 ++++-------- .../claim/interim_claim_presenter.rb | 6 +---- .../claim/interim_claim_presenter_spec.rb | 24 ------------------- 4 files changed, 5 insertions(+), 41 deletions(-) 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 e0e4745aa5..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 diff --git a/app/presenters/claim/interim_claim_presenter.rb b/app/presenters/claim/interim_claim_presenter.rb index 676f9e66b1..ea9b758637 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 @@ -42,10 +42,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/spec/presenters/claim/interim_claim_presenter_spec.rb b/spec/presenters/claim/interim_claim_presenter_spec.rb index 582d0b3552..accb8265c0 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({ From c28b9d300e22167fbaf6e9ead0277292bbc2affd Mon Sep 17 00:00:00 2001 From: Sergio Marques Date: Tue, 22 May 2018 16:21:05 +0100 Subject: [PATCH 3/4] Display disbursement section for LGFS interim claims disbursement section is actual part of the misc fees steps, so if it needs to be edited, the user will see the misc fees step. --- app/presenters/claim/interim_claim_presenter.rb | 1 + .../external_users/claims/_summary_claims_content.html.haml | 2 +- .../external_users/claims/disbursements/_summary.html.haml | 5 +++-- spec/presenters/claim/interim_claim_presenter_spec.rb | 1 + 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/presenters/claim/interim_claim_presenter.rb b/app/presenters/claim/interim_claim_presenter.rb index ea9b758637..c2d7b3c138 100644 --- a/app/presenters/claim/interim_claim_presenter.rb +++ b/app/presenters/claim/interim_claim_presenter.rb @@ -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 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/spec/presenters/claim/interim_claim_presenter_spec.rb b/spec/presenters/claim/interim_claim_presenter_spec.rb index accb8265c0..924613ae13 100644 --- a/spec/presenters/claim/interim_claim_presenter_spec.rb +++ b/spec/presenters/claim/interim_claim_presenter_spec.rb @@ -66,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 From 2224907bf02a4f22872528493a7a1cb892d55929 Mon Sep 17 00:00:00 2001 From: Sergio Marques Date: Tue, 22 May 2018 16:26:46 +0100 Subject: [PATCH 4/4] Display offence category for LGFS claims in the CYC page CYC (Check Your Claim) --- .../claims/offence_details/_default_summary.html.haml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) 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' }