From 33826631a37fef56007da1a9429c5b4126802d1c Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 14:39:14 +0100 Subject: [PATCH 01/13] reimplement the display_line_item_tax_rates helper on the line item presenter 1. reimplment the helper on the LintItem presenter 2. add the tax rate associated to the line items to the list of objects to serialize during the order serialization. 3. implement a presenter for the introducer tax rates. --- app/models/invoice/data_presenter/line_item.rb | 6 ++++++ app/models/invoice/data_presenter/tax_rate.rb | 9 +++++++++ app/serializers/invoice/line_item_serializer.rb | 1 + app/serializers/invoice/tax_rate_serializer.rb | 7 +++++++ app/views/spree/admin/orders/_invoice_table4.html.haml | 2 +- 5 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 app/models/invoice/data_presenter/tax_rate.rb create mode 100644 app/serializers/invoice/tax_rate_serializer.rb diff --git a/app/models/invoice/data_presenter/line_item.rb b/app/models/invoice/data_presenter/line_item.rb index 32e5a3e0c8..0e929c3d23 100644 --- a/app/models/invoice/data_presenter/line_item.rb +++ b/app/models/invoice/data_presenter/line_item.rb @@ -3,9 +3,11 @@ class Invoice class DataPresenter class LineItem < Invoice::DataPresenter::Base + include ::ActionView::Helpers::NumberHelper attributes :added_tax, :currency, :included_tax, :price_with_adjustments, :quantity, :variant_id attributes_with_presenter :variant + array_attribute :tax_rates, class_name: 'TaxRate' invoice_generation_attributes :added_tax, :included_tax, :price_with_adjustments, :quantity, :variant_id @@ -28,6 +30,10 @@ class Invoice Spree::Money.new(0.00, currency:) end end + + def display_line_item_tax_rates + tax_rates.map { |tr| number_to_percentage(tr.amount * 100, precision: 1) }.join(", ") + end end end end diff --git a/app/models/invoice/data_presenter/tax_rate.rb b/app/models/invoice/data_presenter/tax_rate.rb new file mode 100644 index 0000000000..a9c13b2258 --- /dev/null +++ b/app/models/invoice/data_presenter/tax_rate.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: false + +class Invoice + class DataPresenter + class TaxRate < Invoice::DataPresenter::Base + attributes :id, :amount + end + end +end diff --git a/app/serializers/invoice/line_item_serializer.rb b/app/serializers/invoice/line_item_serializer.rb index 6b215298b8..fe037b4d16 100644 --- a/app/serializers/invoice/line_item_serializer.rb +++ b/app/serializers/invoice/line_item_serializer.rb @@ -5,5 +5,6 @@ class Invoice attributes :id, :added_tax, :currency, :included_tax, :price_with_adjustments, :quantity, :variant_id has_one :variant, serializer: Invoice::VariantSerializer + has_many :tax_rates, serializer: Invoice::TaxRateSerializer end end diff --git a/app/serializers/invoice/tax_rate_serializer.rb b/app/serializers/invoice/tax_rate_serializer.rb new file mode 100644 index 0000000000..2862f0c12e --- /dev/null +++ b/app/serializers/invoice/tax_rate_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: false + +class Invoice + class TaxRateSerializer < ActiveModel::Serializer + attributes :id, :amount + end +end diff --git a/app/views/spree/admin/orders/_invoice_table4.html.haml b/app/views/spree/admin/orders/_invoice_table4.html.haml index 8107d772e3..b9c280cb22 100644 --- a/app/views/spree/admin/orders/_invoice_table4.html.haml +++ b/app/views/spree/admin/orders/_invoice_table4.html.haml @@ -28,7 +28,7 @@ = item.display_amount_with_adjustments - if @order.total_tax > 0 %td{:align => "right"} - = display_line_item_tax_rates(item) + = item.display_line_item_tax_rates - @order.checkout_adjustments(exclude: [:line_item]).reverse_each do |adjustment| %tr From 68a6fb132ad639aaf5c4d54828fbb12db678b5c8 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 15:30:56 +0100 Subject: [PATCH 02/13] implement display_adjustment_tax_rates helper on the adjustment presenter For every adjustment a list of tax rates will be created using TaxRateFinder.tax_rates_of. This will prevent running the queries to find the tax rates during the invoice generation. --- app/models/invoice/data_presenter/adjustment.rb | 6 ++++++ app/serializers/invoice/adjustment_serializer.rb | 5 +++++ app/views/spree/admin/orders/_invoice_table4.html.haml | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/models/invoice/data_presenter/adjustment.rb b/app/models/invoice/data_presenter/adjustment.rb index c3dc5c61c2..ad6f49f6de 100644 --- a/app/models/invoice/data_presenter/adjustment.rb +++ b/app/models/invoice/data_presenter/adjustment.rb @@ -3,8 +3,10 @@ class Invoice class DataPresenter class Adjustment < Invoice::DataPresenter::Base + include ::ActionView::Helpers::NumberHelper attributes :additional_tax_total, :adjustable_type, :amount, :currency, :included_tax_total, :label, :originator_type + array_attribute :tax_rates, class_name: 'TaxRate' invoice_generation_attributes :additional_tax_total, :adjustable_type, :amount, :included_tax_total invoice_update_attributes :label @@ -23,6 +25,10 @@ class Invoice Spree::Money.new(0.00, currency:) end end + + def display_adjustment_tax_rates + tax_rates.map { |tr| number_to_percentage(tr.amount * 100, precision: 1) }.join(", ") + end end end end diff --git a/app/serializers/invoice/adjustment_serializer.rb b/app/serializers/invoice/adjustment_serializer.rb index 4ca16b3c8d..1ec87f0ec4 100644 --- a/app/serializers/invoice/adjustment_serializer.rb +++ b/app/serializers/invoice/adjustment_serializer.rb @@ -4,5 +4,10 @@ class Invoice class AdjustmentSerializer < ActiveModel::Serializer attributes :adjustable_type, :label, :included_tax_total, :additional_tax_total, :amount, :currency + has_many :tax_rates, serializer: Invoice::TaxRateSerializer + + def tax_rates + TaxRateFinder.tax_rates_of(object) + end end end diff --git a/app/views/spree/admin/orders/_invoice_table4.html.haml b/app/views/spree/admin/orders/_invoice_table4.html.haml index b9c280cb22..e322b826bb 100644 --- a/app/views/spree/admin/orders/_invoice_table4.html.haml +++ b/app/views/spree/admin/orders/_invoice_table4.html.haml @@ -40,7 +40,7 @@ = adjustment.display_amount - if @order.total_tax > 0 %td{:align => "right"} - = display_adjustment_tax_rates(adjustment) + = adjustment.display_adjustment_tax_rates %tfoot %tr %td{:align => "right", :colspan => "3"} From b027387bee64dfe66c48549e6782aabe7381d511 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 17:30:52 +0100 Subject: [PATCH 03/13] add :originator_type to Invoice::AdjustmentSerializer --- app/serializers/invoice/adjustment_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/invoice/adjustment_serializer.rb b/app/serializers/invoice/adjustment_serializer.rb index 1ec87f0ec4..8eb0a6a13f 100644 --- a/app/serializers/invoice/adjustment_serializer.rb +++ b/app/serializers/invoice/adjustment_serializer.rb @@ -3,7 +3,7 @@ class Invoice class AdjustmentSerializer < ActiveModel::Serializer attributes :adjustable_type, :label, :included_tax_total, :additional_tax_total, :amount, - :currency + :currency, :originator_type has_many :tax_rates, serializer: Invoice::TaxRateSerializer def tax_rates From a2b06fa20047915f5f9c015d72ec5f00c71fbfb2 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 17:31:27 +0100 Subject: [PATCH 04/13] fix cloning all_eligible_adjustments array on DataPresenter#checkout_adjustments --- app/models/invoice/data_presenter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index c4485e9905..b90b9444d6 100644 --- a/app/models/invoice/data_presenter.rb +++ b/app/models/invoice/data_presenter.rb @@ -50,7 +50,7 @@ class Invoice end def checkout_adjustments(exclude: [], reject_zero_amount: true) - adjustments = all_eligible_adjustments + adjustments = all_eligible_adjustments.map(&:clone) adjustments.reject! { |a| a.originator_type == 'Spree::TaxRate' } From d40338bed5a83aa890c2c001b3e33ce067c31a47 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 19:10:53 +0100 Subject: [PATCH 05/13] add adjustment originators to the order serialization for some helpers like display_checkout_taxes_hash, it's needed to access the tax rates through the adjument's originator. The adjustment's originator will store the minimal details: id, type and amount --- app/models/invoice/data_presenter.rb | 2 +- app/models/invoice/data_presenter/adjustment.rb | 3 ++- .../data_presenter/adjustment_originator.rb | 9 +++++++++ app/models/invoice/data_presenter_attributes.rb | 4 ++-- .../invoice/adjustment_originator_serializer.rb | 16 ++++++++++++++++ app/serializers/invoice/adjustment_serializer.rb | 3 ++- 6 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 app/models/invoice/data_presenter/adjustment_originator.rb create mode 100644 app/serializers/invoice/adjustment_originator_serializer.rb diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index b90b9444d6..19c137eda2 100644 --- a/app/models/invoice/data_presenter.rb +++ b/app/models/invoice/data_presenter.rb @@ -52,7 +52,7 @@ class Invoice def checkout_adjustments(exclude: [], reject_zero_amount: true) adjustments = all_eligible_adjustments.map(&:clone) - adjustments.reject! { |a| a.originator_type == 'Spree::TaxRate' } + adjustments.reject! { |a| a.originator.type == 'Spree::TaxRate' } if exclude.include? :line_item adjustments.reject! { |a| diff --git a/app/models/invoice/data_presenter/adjustment.rb b/app/models/invoice/data_presenter/adjustment.rb index ad6f49f6de..ad5db4bded 100644 --- a/app/models/invoice/data_presenter/adjustment.rb +++ b/app/models/invoice/data_presenter/adjustment.rb @@ -5,8 +5,9 @@ class Invoice class Adjustment < Invoice::DataPresenter::Base include ::ActionView::Helpers::NumberHelper attributes :additional_tax_total, :adjustable_type, :amount, :currency, :included_tax_total, - :label, :originator_type + :label array_attribute :tax_rates, class_name: 'TaxRate' + attributes_with_presenter :originator, class_name: :AdjustmentOriginator invoice_generation_attributes :additional_tax_total, :adjustable_type, :amount, :included_tax_total invoice_update_attributes :label diff --git a/app/models/invoice/data_presenter/adjustment_originator.rb b/app/models/invoice/data_presenter/adjustment_originator.rb new file mode 100644 index 0000000000..920ee947b5 --- /dev/null +++ b/app/models/invoice/data_presenter/adjustment_originator.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: false + +class Invoice + class DataPresenter + class AdjustmentOriginator < Invoice::DataPresenter::Base + attributes :id, :type, :amount + end + end +end diff --git a/app/models/invoice/data_presenter_attributes.rb b/app/models/invoice/data_presenter_attributes.rb index 476e9dbbf8..bc2b3b7c75 100644 --- a/app/models/invoice/data_presenter_attributes.rb +++ b/app/models/invoice/data_presenter_attributes.rb @@ -12,7 +12,7 @@ class Invoice end end - def attributes_with_presenter(*attributes) + def attributes_with_presenter(*attributes, class_name: nil) attributes.each do |attribute| define_method(attribute) do instance_variable = instance_variable_get("@#{attribute}") @@ -20,7 +20,7 @@ class Invoice instance_variable_set("@#{attribute}", Invoice::DataPresenter.const_get( - attribute.to_s.classify + class_name.present? ? class_name : attribute.to_s.classify ).new(data&.[](attribute))) end end diff --git a/app/serializers/invoice/adjustment_originator_serializer.rb b/app/serializers/invoice/adjustment_originator_serializer.rb new file mode 100644 index 0000000000..4c98757777 --- /dev/null +++ b/app/serializers/invoice/adjustment_originator_serializer.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: false + +class Invoice + class AdjustmentOriginatorSerializer < ActiveModel::Serializer + attributes :id, :type, :amount + def type + object.class.name + end + + def amount + return nil unless object.respond_to?(:amount) + + object.amount.to_f + end + end +end diff --git a/app/serializers/invoice/adjustment_serializer.rb b/app/serializers/invoice/adjustment_serializer.rb index 8eb0a6a13f..dd5f8efe6b 100644 --- a/app/serializers/invoice/adjustment_serializer.rb +++ b/app/serializers/invoice/adjustment_serializer.rb @@ -3,7 +3,8 @@ class Invoice class AdjustmentSerializer < ActiveModel::Serializer attributes :adjustable_type, :label, :included_tax_total, :additional_tax_total, :amount, - :currency, :originator_type + :currency + has_one :originator, serializer: Invoice::AdjustmentOriginatorSerializer has_many :tax_rates, serializer: Invoice::TaxRateSerializer def tax_rates From fc519da83bed672ad932ca44203d08a33ad12b17 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 20:13:52 +0100 Subject: [PATCH 06/13] update the invoice system tests to run with invoice feature (enabled/disabled) --- spec/system/admin/invoice_print_spec.rb | 115 ++++++++++++++++++------ 1 file changed, 89 insertions(+), 26 deletions(-) diff --git a/spec/system/admin/invoice_print_spec.rb b/spec/system/admin/invoice_print_spec.rb index 76a1b35b42..eea468e046 100644 --- a/spec/system/admin/invoice_print_spec.rb +++ b/spec/system/admin/invoice_print_spec.rb @@ -35,7 +35,15 @@ describe ' Capybara.use_default_driver end - describe "that contains right Payment Description at Checkout information" do + shared_examples "contains right Payment Description at Checkout information" do + let(:url_params) { + if OpenFoodNetwork::FeatureToggle.enabled?(:invoices) + { invoice_id: order.invoices.first.id } + else + {} + end + } + let!(:payment_method1) do create(:stripe_sca_payment_method, distributors: [distributor], description: "description1") end @@ -45,8 +53,9 @@ describe ' context "with no payment" do it "do not display the payment description information" do + order.invoices.create! login_as_admin - visit spree.print_admin_order_path(order) + visit spree.print_admin_order_path(order, params: url_params) convert_pdf_to_page expect(page).to have_no_content 'Payment Description at Checkout' end @@ -58,11 +67,12 @@ describe ' end before do order.save! + order.invoices.create! end it "display the payment description section" do login_as_admin - visit spree.print_admin_order_path(order) + visit spree.print_admin_order_path(order, params: url_params) convert_pdf_to_page expect(page).to have_content 'Payment Description at Checkout' expect(page).to have_content 'description1' @@ -75,14 +85,16 @@ describe ' order.payments << create(:payment, :completed, order:, payment_method: payment_method1, created_at: 1.day.ago) - order.payments << create(:payment, order:, state: 'failed', - payment_method: payment_method2, created_at: 2.days.ago) + order.payments << create(:payment, order: order, state: 'failed', + payment_method: payment_method2, + created_at: 2.days.ago) order.save! + order.invoices.create! end it "display the payment description section and use the one from the completed payment" do login_as_admin - visit spree.print_admin_order_path(order) + visit spree.print_admin_order_path(order, params: url_params) convert_pdf_to_page expect(page).to have_content 'Payment Description at Checkout' expect(page).to have_content 'description1' @@ -99,29 +111,38 @@ describe ' payment_method: payment_method2, created_at: 1.day.ago) order.save! + order.invoices.create! end it "display the payment description section and use the one from the last payment" do login_as_admin - visit spree.print_admin_order_path(order) + visit spree.print_admin_order_path(order, params: url_params) convert_pdf_to_page expect(page).to have_content 'Payment Description at Checkout' expect(page).to have_content 'description2' end end - end + end shared_examples "Check display on each invoice: legacy and alternative" do |alternative_invoice| let!(:completed_order) do create(:completed_order_with_fees, distributor:, order_cycle:, user: create(:user, email: "xxxxxx@example.com"), bill_address: create(:address, phone: '1234567890')) end + let(:url_params) { + if OpenFoodNetwork::FeatureToggle.enabled?(:invoices) + { invoice_id: completed_order.invoices.first.id } + else + {} + end + } before do + completed_order.invoices.create! allow(Spree::Config).to receive(:invoice_style2?).and_return(alternative_invoice) login_as_admin - visit spree.print_admin_order_path(completed_order) + visit spree.print_admin_order_path(completed_order, params: url_params) convert_pdf_to_page end @@ -131,10 +152,7 @@ describe ' end end - it_behaves_like "Check display on each invoice: legacy and alternative", false - it_behaves_like "Check display on each invoice: legacy and alternative", true - - describe "an order with taxes" do + shared_examples "order with tax" do let(:user1) { create(:user, enterprises: [distributor]) } let!(:zone) { create(:zone_with_member) } let(:address) { create(:address) } @@ -146,8 +164,12 @@ describe ' let(:enterprise_fee_rate_included) { create(:tax_rate, amount: 0.15, included_in_price: true, zone:) } - let(:shipping_tax_category) { create(:tax_category, tax_rates: [shipping_tax_rate_included]) } - let(:fee_tax_category) { create(:tax_category, tax_rates: [enterprise_fee_rate_included]) } + let(:shipping_tax_category) { + create(:tax_category, tax_rates: [shipping_tax_rate_included]) + } + let(:fee_tax_category) { + create(:tax_category, tax_rates: [enterprise_fee_rate_included]) + } let!(:shipping_method) { create(:shipping_method_with, :expensive_name, distributors: [distributor], tax_category: shipping_tax_category) @@ -158,9 +180,11 @@ describe ' calculator: Calculator::FlatRate.new(preferred_amount: 120.0)) } let!(:order_cycle) { - create(:simple_order_cycle, coordinator: distributor, - coordinator_fees: [enterprise_fee], distributors: [distributor], - variants: [product1.variants.first, product2.variants.first]) + create(:simple_order_cycle, + coordinator: distributor, + coordinator_fees: [enterprise_fee], + distributors: [distributor], + variants: [product1.variants.first, product2.variants.first]) } let!(:order1) { @@ -185,6 +209,14 @@ describe ' order: order1) } + let(:url_params) { + if OpenFoodNetwork::FeatureToggle.enabled?(:invoices) + { invoice_id: order1.invoices.first.id } + else + {} + end + } + before do order1.reload while !order1.delivery? @@ -201,13 +233,14 @@ describe ' while !order1.complete? break if !order1.next! end + order1.invoices.create! end context "legacy invoice" do before do allow(Spree::Config).to receive(:invoice_style2?).and_return(false) login_as_admin - visit spree.print_admin_order_path(order1) + visit spree.print_admin_order_path(order1, params: url_params) convert_pdf_to_page end @@ -225,8 +258,12 @@ describe ' expect(page).to have_content "(1g)" # display as expect(page).to have_content "3 $250.08 $1,500.45" # Enterprise fee - expect(page).to have_content "Whole order - #{enterprise_fee.name} fee by coordinator " \ - "#{user1.enterprises.first.name} 1 $15.65 (included) $120.00" + expect(page).to have_content "Whole order - #{ + enterprise_fee.name + } fee by coordinator " \ + "#{ + user1.enterprises.first.name + } 1 $15.65 (included) $120.00" # Shipping expect(page).to have_content "Shipping 1 $9.14 (included) $100.55" # Order Totals @@ -240,7 +277,7 @@ describe ' before do allow(Spree::Config).to receive(:invoice_style2?).and_return(true) login_as_admin - visit spree.print_admin_order_path(order1) + visit spree.print_admin_order_path(order1, params: url_params) convert_pdf_to_page end @@ -280,7 +317,9 @@ describe ' let(:enterprise_fee_rate_added) { create(:tax_rate, amount: 0.15, included_in_price: false, zone:) } - let(:shipping_tax_category) { create(:tax_category, tax_rates: [shipping_tax_rate_added]) } + let(:shipping_tax_category) { + create(:tax_category, tax_rates: [shipping_tax_rate_added]) + } let(:fee_tax_category) { create(:tax_category, tax_rates: [enterprise_fee_rate_added]) } let!(:shipping_method) { create(:shipping_method_with, :expensive_name, distributors: [distributor], @@ -293,7 +332,8 @@ describe ' } let(:order_cycle2) { create(:simple_order_cycle, coordinator: distributor, - coordinator_fees: [enterprise_fee], distributors: [distributor], + coordinator_fees: [enterprise_fee], + distributors: [distributor], variants: [product3.variants.first, product4.variants.first]) } @@ -360,8 +400,12 @@ describe ' # header expect(page).to have_content "Item Qty GST Price" # Enterprise fee - expect(page).to have_content "Whole order - #{enterprise_fee.name} fee by coordinator " \ - "#{user1.enterprises.first.name} 1 $18.00 $120.00" + expect(page).to have_content "Whole order - #{ + enterprise_fee.name + } fee by coordinator " \ + "#{ + user1.enterprises.first.name + } 1 $18.00 $120.00" # Shipping expect(page).to have_content "Shipping 1 $10.06 $100.55" # Order Totals @@ -407,6 +451,25 @@ describe ' end end end + + context "when invoice feature is not enabled" do + before do + Flipper.disable(:invoices) + end + it_behaves_like "contains right Payment Description at Checkout information" + it_behaves_like "Check display on each invoice: legacy and alternative", false + it_behaves_like "Check display on each invoice: legacy and alternative", true + it_behaves_like "order with tax" + end + context "when invoice feature is enabled" do + before do + Flipper.enable(:invoice) + end + it_behaves_like "contains right Payment Description at Checkout information" + it_behaves_like "Check display on each invoice: legacy and alternative", false + it_behaves_like "Check display on each invoice: legacy and alternative", true + it_behaves_like "order with tax" + end end def convert_pdf_to_page From aa4798d35c8ba3e516c209574d022a0491e318b4 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 23:51:40 +0100 Subject: [PATCH 07/13] add adjustables to the list of objects to serialize when creating an invoice --- .../invoice/data_presenter/adjustable.rb | 21 +++++++++++++++++++ .../invoice/data_presenter/adjustment.rb | 3 ++- .../invoice/adjustable_serializer.rb | 18 ++++++++++++++++ .../invoice/adjustment_serializer.rb | 1 + .../admin/orders/_invoice_table3.html.haml | 4 ++-- 5 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 app/models/invoice/data_presenter/adjustable.rb create mode 100644 app/serializers/invoice/adjustable_serializer.rb diff --git a/app/models/invoice/data_presenter/adjustable.rb b/app/models/invoice/data_presenter/adjustable.rb new file mode 100644 index 0000000000..299732d77f --- /dev/null +++ b/app/models/invoice/data_presenter/adjustable.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: false + +class Invoice + class DataPresenter + class Adjustable < Invoice::DataPresenter::Base + attributes :id, :type, :currency, :included_tax_total, :additional_tax_total, :amount + + def display_taxes(display_zero: false) + if included_tax_total.positive? + amount = Spree::Money.new(included_tax_total, currency: currency) + I18n.t(:tax_amount_included, amount: amount) + elsif additional_tax_total.positive? + Spree::Money.new(additional_tax_total, currency: currency) + elsif display_zero + Spree::Money.new(0.00, currency: currency) + end + end + + end + end +end \ No newline at end of file diff --git a/app/models/invoice/data_presenter/adjustment.rb b/app/models/invoice/data_presenter/adjustment.rb index ad5db4bded..8ec4ad414c 100644 --- a/app/models/invoice/data_presenter/adjustment.rb +++ b/app/models/invoice/data_presenter/adjustment.rb @@ -7,7 +7,8 @@ class Invoice attributes :additional_tax_total, :adjustable_type, :amount, :currency, :included_tax_total, :label array_attribute :tax_rates, class_name: 'TaxRate' - attributes_with_presenter :originator, class_name: :AdjustmentOriginator + attributes_with_presenter :originator, class_name: 'AdjustmentOriginator' + attributes_with_presenter :adjustable invoice_generation_attributes :additional_tax_total, :adjustable_type, :amount, :included_tax_total invoice_update_attributes :label diff --git a/app/serializers/invoice/adjustable_serializer.rb b/app/serializers/invoice/adjustable_serializer.rb new file mode 100644 index 0000000000..fd9d5c255a --- /dev/null +++ b/app/serializers/invoice/adjustable_serializer.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: false + +class Invoice + class AdjustableSerializer < ActiveModel::Serializer + attributes :id, :type, :currency, :included_tax_total, :additional_tax_total, :amount + def type + object.class.name + end + + [:currency, :included_tax_total, :additional_tax_total, :amount].each do |method| + define_method method do + return nil unless object.respond_to?(method) + + object.public_send(method).to_f + end + end + end +end diff --git a/app/serializers/invoice/adjustment_serializer.rb b/app/serializers/invoice/adjustment_serializer.rb index dd5f8efe6b..daf46294cf 100644 --- a/app/serializers/invoice/adjustment_serializer.rb +++ b/app/serializers/invoice/adjustment_serializer.rb @@ -5,6 +5,7 @@ class Invoice attributes :adjustable_type, :label, :included_tax_total, :additional_tax_total, :amount, :currency has_one :originator, serializer: Invoice::AdjustmentOriginatorSerializer + has_one :adjustable, serializer: Invoice::AdjustableSerializer has_many :tax_rates, serializer: Invoice::TaxRateSerializer def tax_rates diff --git a/app/views/spree/admin/orders/_invoice_table3.html.haml b/app/views/spree/admin/orders/_invoice_table3.html.haml index 77dfc3f292..299539d964 100644 --- a/app/views/spree/admin/orders/_invoice_table3.html.haml +++ b/app/views/spree/admin/orders/_invoice_table3.html.haml @@ -25,14 +25,14 @@ = item.display_amount_with_adjustments - @order.checkout_adjustments(exclude: [:line_item]).reverse_each do |adjustment| - - taxable = adjustment#.adjustable_type == "Spree::Shipment" ? adjustment.adjustable : adjustment + - taxable = adjustment.adjustable_type == "Spree::Shipment" ? adjustment.adjustable : adjustment %tr %td %strong= "#{raw(adjustment.label)}" %td{:align => "right"} 1 %td{:align => "right"} - = adjustment.display_taxes + = taxable.display_taxes %td{:align => "right"} = adjustment.display_amount %tfoot From 7eabb9a1705d2274708a5903d3743dce4fa3e0fe Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 23:52:57 +0100 Subject: [PATCH 08/13] move include ::ActionView::Helpers::NumberHelper to Invoice::DataPresenter::Base --- app/models/invoice/data_presenter.rb | 1 + app/models/invoice/data_presenter/adjustment.rb | 1 - app/models/invoice/data_presenter/base.rb | 1 + app/models/invoice/data_presenter/line_item.rb | 1 - 4 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index 19c137eda2..255f0ee410 100644 --- a/app/models/invoice/data_presenter.rb +++ b/app/models/invoice/data_presenter.rb @@ -2,6 +2,7 @@ class Invoice class DataPresenter + include ::ActionView::Helpers::NumberHelper attr_reader :invoice delegate :data, to: :invoice diff --git a/app/models/invoice/data_presenter/adjustment.rb b/app/models/invoice/data_presenter/adjustment.rb index 8ec4ad414c..1ac58e8467 100644 --- a/app/models/invoice/data_presenter/adjustment.rb +++ b/app/models/invoice/data_presenter/adjustment.rb @@ -3,7 +3,6 @@ class Invoice class DataPresenter class Adjustment < Invoice::DataPresenter::Base - include ::ActionView::Helpers::NumberHelper attributes :additional_tax_total, :adjustable_type, :amount, :currency, :included_tax_total, :label array_attribute :tax_rates, class_name: 'TaxRate' diff --git a/app/models/invoice/data_presenter/base.rb b/app/models/invoice/data_presenter/base.rb index b27976fbe0..c653573cf6 100644 --- a/app/models/invoice/data_presenter/base.rb +++ b/app/models/invoice/data_presenter/base.rb @@ -3,6 +3,7 @@ class Invoice class DataPresenter class Base + include ::ActionView::Helpers::NumberHelper attr_reader :data def initialize(data) diff --git a/app/models/invoice/data_presenter/line_item.rb b/app/models/invoice/data_presenter/line_item.rb index 0e929c3d23..0e091a0655 100644 --- a/app/models/invoice/data_presenter/line_item.rb +++ b/app/models/invoice/data_presenter/line_item.rb @@ -3,7 +3,6 @@ class Invoice class DataPresenter class LineItem < Invoice::DataPresenter::Base - include ::ActionView::Helpers::NumberHelper attributes :added_tax, :currency, :included_tax, :price_with_adjustments, :quantity, :variant_id attributes_with_presenter :variant From 9950dd90f78f2030ae4036d95d6023130780d952 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 23:55:02 +0100 Subject: [PATCH 09/13] fix display_checkout_taxes_hash --- app/models/invoice/data_presenter.rb | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index 255f0ee410..4484ed3b37 100644 --- a/app/models/invoice/data_presenter.rb +++ b/app/models/invoice/data_presenter.rb @@ -69,11 +69,10 @@ class Invoice end def display_checkout_taxes_hash - totals = OrderTaxAdjustmentsFetcher.new(nil).totals(all_tax_adjustments) - - totals.map do |tax_rate, tax_amount| + tax_adjustment_totals.map do |tax_rate_id, tax_amount| + tax_rate = tax_rate_by_id[tax_rate_id] { - amount: Spree::Money.new(tax_amount, currency: order.currency), + amount: Spree::Money.new(tax_amount, currency: currency), percentage: number_to_percentage(tax_rate.amount * 100, precision: 1), rate_amount: tax_rate.amount, } @@ -84,6 +83,22 @@ class Invoice I18n.l(invoice_date.to_date, format: :long) end + def all_tax_adjustments + all_eligible_adjustments.select { |a| a.originator.type == 'Spree::TaxRate' } + end + + def tax_adjustment_totals + all_tax_adjustments.each_with_object(Hash.new(0)) do |adjustment, totals| + totals[adjustment.originator.id] += adjustment.amount + end + end + + def tax_rate_by_id + all_tax_adjustments.each_with_object({}) do |adjustment, tax_rates| + tax_rates[adjustment.originator.id] = adjustment.originator + end + end + def all_tax_adjustments all_eligible_adjustments.select { |a| a.originator_type == 'Spree::TaxRate' } end From 4a74396bcc25fccd85fb04188eacf687c1976559 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 23:55:26 +0100 Subject: [PATCH 10/13] add 'app/models/invoice/data_presenter.rb' to Metrics/ClassLength:Exclude --- .rubocop_todo.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 31339ee76b..5b58d28b91 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -210,6 +210,7 @@ Metrics/ClassLength: - 'app/controllers/spree/admin/users_controller.rb' - 'app/controllers/spree/orders_controller.rb' - 'app/models/enterprise.rb' + - 'app/models/invoice/data_presenter.rb' - 'app/models/order_cycle.rb' - 'app/models/product_import/entry_processor.rb' - 'app/models/product_import/entry_validator.rb' From 7cb200e21e598c2ccf4dfe22b88c1ed8d476268f Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Sat, 15 Jul 2023 09:47:41 +0100 Subject: [PATCH 11/13] fix linting errors --- app/models/invoice/data_presenter/adjustable.rb | 13 ++++++------- app/models/invoice/data_presenter_attributes.rb | 2 +- spec/system/admin/invoice_print_spec.rb | 1 - 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/models/invoice/data_presenter/adjustable.rb b/app/models/invoice/data_presenter/adjustable.rb index 299732d77f..da8f4ad7b3 100644 --- a/app/models/invoice/data_presenter/adjustable.rb +++ b/app/models/invoice/data_presenter/adjustable.rb @@ -2,20 +2,19 @@ class Invoice class DataPresenter - class Adjustable < Invoice::DataPresenter::Base + class Adjustable < Invoice::DataPresenter::Base attributes :id, :type, :currency, :included_tax_total, :additional_tax_total, :amount def display_taxes(display_zero: false) if included_tax_total.positive? - amount = Spree::Money.new(included_tax_total, currency: currency) - I18n.t(:tax_amount_included, amount: amount) + amount = Spree::Money.new(included_tax_total, currency:) + I18n.t(:tax_amount_included, amount:) elsif additional_tax_total.positive? - Spree::Money.new(additional_tax_total, currency: currency) + Spree::Money.new(additional_tax_total, currency:) elsif display_zero - Spree::Money.new(0.00, currency: currency) + Spree::Money.new(0.00, currency:) end end - end end -end \ No newline at end of file +end diff --git a/app/models/invoice/data_presenter_attributes.rb b/app/models/invoice/data_presenter_attributes.rb index bc2b3b7c75..0609549cba 100644 --- a/app/models/invoice/data_presenter_attributes.rb +++ b/app/models/invoice/data_presenter_attributes.rb @@ -20,7 +20,7 @@ class Invoice instance_variable_set("@#{attribute}", Invoice::DataPresenter.const_get( - class_name.present? ? class_name : attribute.to_s.classify + class_name.presence || attribute.to_s.classify ).new(data&.[](attribute))) end end diff --git a/spec/system/admin/invoice_print_spec.rb b/spec/system/admin/invoice_print_spec.rb index eea468e046..f323eee21a 100644 --- a/spec/system/admin/invoice_print_spec.rb +++ b/spec/system/admin/invoice_print_spec.rb @@ -122,7 +122,6 @@ describe ' expect(page).to have_content 'description2' end end - end shared_examples "Check display on each invoice: legacy and alternative" do |alternative_invoice| let!(:completed_order) do From c750602b521127a504f283477cf1d924aa3a7a54 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Wed, 11 Oct 2023 10:38:45 +0100 Subject: [PATCH 12/13] remove duplicated code --- app/models/invoice/data_presenter.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index 4484ed3b37..3ad7d80626 100644 --- a/app/models/invoice/data_presenter.rb +++ b/app/models/invoice/data_presenter.rb @@ -83,10 +83,6 @@ class Invoice I18n.l(invoice_date.to_date, format: :long) end - def all_tax_adjustments - all_eligible_adjustments.select { |a| a.originator.type == 'Spree::TaxRate' } - end - def tax_adjustment_totals all_tax_adjustments.each_with_object(Hash.new(0)) do |adjustment, totals| totals[adjustment.originator.id] += adjustment.amount @@ -100,7 +96,7 @@ class Invoice end def all_tax_adjustments - all_eligible_adjustments.select { |a| a.originator_type == 'Spree::TaxRate' } + all_eligible_adjustments.select { |a| a.originator.type == 'Spree::TaxRate' } end def paid? From 1c2aa1c370cf8a970fda2d6f874c315db2edcdb7 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Wed, 11 Oct 2023 10:55:14 +0100 Subject: [PATCH 13/13] fix linter issue --- app/models/invoice/data_presenter.rb | 2 +- spec/system/admin/invoice_print_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index 3ad7d80626..b710056062 100644 --- a/app/models/invoice/data_presenter.rb +++ b/app/models/invoice/data_presenter.rb @@ -72,7 +72,7 @@ class Invoice tax_adjustment_totals.map do |tax_rate_id, tax_amount| tax_rate = tax_rate_by_id[tax_rate_id] { - amount: Spree::Money.new(tax_amount, currency: currency), + amount: Spree::Money.new(tax_amount, currency:), percentage: number_to_percentage(tax_rate.amount * 100, precision: 1), rate_amount: tax_rate.amount, } diff --git a/spec/system/admin/invoice_print_spec.rb b/spec/system/admin/invoice_print_spec.rb index f323eee21a..d5d66774cf 100644 --- a/spec/system/admin/invoice_print_spec.rb +++ b/spec/system/admin/invoice_print_spec.rb @@ -85,7 +85,7 @@ describe ' order.payments << create(:payment, :completed, order:, payment_method: payment_method1, created_at: 1.day.ago) - order.payments << create(:payment, order: order, state: 'failed', + order.payments << create(:payment, order:, state: 'failed', payment_method: payment_method2, created_at: 2.days.ago) order.save!