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' diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index c4485e9905..b710056062 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 @@ -50,9 +51,9 @@ 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' } + adjustments.reject! { |a| a.originator.type == 'Spree::TaxRate' } if exclude.include? :line_item adjustments.reject! { |a| @@ -68,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:), percentage: number_to_percentage(tax_rate.amount * 100, precision: 1), rate_amount: tax_rate.amount, } @@ -83,8 +83,20 @@ class Invoice I18n.l(invoice_date.to_date, format: :long) 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' } + all_eligible_adjustments.select { |a| a.originator.type == 'Spree::TaxRate' } end def paid? diff --git a/app/models/invoice/data_presenter/adjustable.rb b/app/models/invoice/data_presenter/adjustable.rb new file mode 100644 index 0000000000..da8f4ad7b3 --- /dev/null +++ b/app/models/invoice/data_presenter/adjustable.rb @@ -0,0 +1,20 @@ +# 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:) + I18n.t(:tax_amount_included, amount:) + elsif additional_tax_total.positive? + Spree::Money.new(additional_tax_total, currency:) + elsif display_zero + Spree::Money.new(0.00, currency:) + end + end + end + end +end diff --git a/app/models/invoice/data_presenter/adjustment.rb b/app/models/invoice/data_presenter/adjustment.rb index c3dc5c61c2..1ac58e8467 100644 --- a/app/models/invoice/data_presenter/adjustment.rb +++ b/app/models/invoice/data_presenter/adjustment.rb @@ -4,7 +4,10 @@ class Invoice class DataPresenter class Adjustment < Invoice::DataPresenter::Base 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' + attributes_with_presenter :adjustable invoice_generation_attributes :additional_tax_total, :adjustable_type, :amount, :included_tax_total invoice_update_attributes :label @@ -23,6 +26,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/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/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 32e5a3e0c8..0e091a0655 100644 --- a/app/models/invoice/data_presenter/line_item.rb +++ b/app/models/invoice/data_presenter/line_item.rb @@ -6,6 +6,7 @@ class Invoice 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 +29,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/models/invoice/data_presenter_attributes.rb b/app/models/invoice/data_presenter_attributes.rb index 476e9dbbf8..0609549cba 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.presence || attribute.to_s.classify ).new(data&.[](attribute))) end end 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_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 4ca16b3c8d..daf46294cf 100644 --- a/app/serializers/invoice/adjustment_serializer.rb +++ b/app/serializers/invoice/adjustment_serializer.rb @@ -4,5 +4,12 @@ class Invoice class AdjustmentSerializer < ActiveModel::Serializer 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 + TaxRateFinder.tax_rates_of(object) + 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_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 diff --git a/app/views/spree/admin/orders/_invoice_table4.html.haml b/app/views/spree/admin/orders/_invoice_table4.html.haml index 8107d772e3..e322b826bb 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 @@ -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"} diff --git a/spec/system/admin/invoice_print_spec.rb b/spec/system/admin/invoice_print_spec.rb index 76a1b35b42..d5d66774cf 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' @@ -76,13 +86,15 @@ describe ' 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) + 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,37 @@ 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 - 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 +151,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 +163,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 +179,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 +208,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 +232,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 +257,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 +276,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 +316,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 +331,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 +399,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 +450,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