From bf912ae4d349553843aec4be64a1873f0f553b55 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 1 Jun 2023 18:28:18 +0100 Subject: [PATCH 01/16] Remove angular loading element in darkwarm layout --- app/views/layouts/darkswarm.html.haml | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/layouts/darkswarm.html.haml b/app/views/layouts/darkswarm.html.haml index 472f37b74f6..e28fb78bffe 100644 --- a/app/views/layouts/darkswarm.html.haml +++ b/app/views/layouts/darkswarm.html.haml @@ -46,7 +46,6 @@ = yield #footer - %loading = yield :scripts = inject_current_hub From 074eb4b5923c832686ac35db2b414f673a952e1b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 23:19:26 +0100 Subject: [PATCH 02/16] Move tax charge logic out of checkout controller and update payment transition --- app/controllers/split_checkout_controller.rb | 8 +------- app/models/spree/order/checkout.rb | 5 ++++- spec/controllers/spree/admin/orders_controller_spec.rb | 2 +- spec/system/consumer/split_checkout_tax_not_incl_spec.rb | 8 ++++---- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index db900e7f0d1..aa27cfbfcb3 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -24,7 +24,7 @@ class SplitCheckoutController < ::BaseController def edit redirect_to_step_based_on_order unless params[:step] check_step if params[:step] - recalculate_tax if params[:step] == "summary" + apply_voucher if @order.voucher_adjustments.present? flash_error_when_no_shipping_method_available if available_shipping_methods.none? end @@ -309,12 +309,6 @@ def check_step end end - def recalculate_tax - @order.create_tax_charge! - @order.update_order! - - apply_voucher if @order.voucher_adjustments.present? - end def apply_voucher VoucherAdjustmentsService.calculate(@order) diff --git a/app/models/spree/order/checkout.rb b/app/models/spree/order/checkout.rb index 9d0bc4c54cf..6a6bee68c48 100644 --- a/app/models/spree/order/checkout.rb +++ b/app/models/spree/order/checkout.rb @@ -77,7 +77,10 @@ def self.define_state_machine! before_transition to: :delivery, do: :ensure_available_shipping_rates before_transition to: :confirmation, do: :validate_payment_method! - after_transition to: :payment, do: :create_tax_charge! + after_transition to: :payment do |order| + order.create_tax_charge! + order.update_totals_and_states + end after_transition to: :complete, do: :finalize! after_transition to: :resumed, do: :after_resume after_transition to: :canceled, do: :after_cancel diff --git a/spec/controllers/spree/admin/orders_controller_spec.rb b/spec/controllers/spree/admin/orders_controller_spec.rb index 5664d9e3a56..cca61f44d38 100644 --- a/spec/controllers/spree/admin/orders_controller_spec.rb +++ b/spec/controllers/spree/admin/orders_controller_spec.rb @@ -57,7 +57,7 @@ it "updates fees and taxes and redirects to order details page" do expect(order).to receive(:recreate_all_fees!) - expect(order).to receive(:create_tax_charge!) + expect(order).to receive(:create_tax_charge!).at_least :once spree_put :update, params diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb index b899f5d01de..21e14f83719 100644 --- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb @@ -99,12 +99,12 @@ choose "Delivery" click_button "Next - Payment method" - click_on "Next - Order summary" - click_on "Complete order" # DB checks - order_within_zone.reload - expect(order_within_zone.additional_tax_total).to eq(1.3) + expect(order_within_zone.reload.additional_tax_total).to eq(1.3) + + click_on "Next - Order summary" + click_on "Complete order" # UI checks expect(page).to have_content("Confirmed") From a0d0f8fd2f8c1d0b62bd68ea3a563533ea6b726a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 11:59:45 +0100 Subject: [PATCH 03/16] Tidy up checkout tax spec --- .../consumer/split_checkout_tax_not_incl_spec.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb index 21e14f83719..d4a7ddda411 100644 --- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb @@ -10,8 +10,12 @@ include AuthenticationHelper include WebHelper - let!(:address_within_zone) { create(:address, state: Spree::State.first) } - let!(:address_outside_zone) { create(:address, state: Spree::State.second) } + let!(:state) { create(:state, name: "Victoria") } + let!(:zone) { create(:zone_with_state_member, default_tax: false, member: state) } + let!(:address_within_zone) { create(:address, state: state) } + let!(:address_outside_zone) { + create(:address, state: create(:state, name: "Timbuktu", country: state.country)) + } let(:user_within_zone) { create(:user, bill_address: address_within_zone, ship_address: address_within_zone) @@ -20,8 +24,7 @@ create(:user, bill_address: address_outside_zone, ship_address: address_outside_zone) } - let!(:zone) { create(:zone_with_state_member, name: 'Victoria', default_tax: false) } - let!(:tax_category) { create(:tax_category, name: "Veggies", is_default: "f") } + let!(:tax_category) { create(:tax_category, name: "Veggies", is_default: false) } let!(:tax_rate) { create(:tax_rate, name: "Tax rate - included or not", amount: 0.13, zone: zone, tax_category: tax_category, included_in_price: false) @@ -62,7 +65,7 @@ Spree::Config.set(tax_using_ship_address: true) end - pending "a not-included tax" do + describe "a not-included tax" do before do zone.update!(default_tax: false) tax_rate.update!(included_in_price: false) @@ -112,7 +115,7 @@ expect(page).to have_selector('#tax-row', text: with_currency(1.30)) end - context "when using a voucher" do + pending "when using a voucher" do let!(:voucher) do create(:voucher, code: 'some_code', enterprise: distributor, amount: 10) end From 65b6a75c9bcc0e81745f77736170cfad2d4b548e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 12:16:20 +0100 Subject: [PATCH 04/16] Reorder callback definitions --- app/models/spree/order.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index d180a4b8333..62717fe3285 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -86,15 +86,6 @@ def states delegate :create_line_item_fees!, :create_order_fees!, :update_order_fees!, :update_line_item_fees!, :recreate_all_fees!, to: :fee_handler - # Needs to happen before save_permalink is called - before_validation :set_currency - before_validation :generate_order_number, if: :new_record? - before_validation :clone_billing_address, if: :use_billing? - before_validation :ensure_customer - - before_create :link_by_email - after_create :create_tax_charge! - validates :customer, presence: true, if: :require_customer? validate :products_available_from_new_distribution, if: lambda { distributor_id_changed? || order_cycle_id_changed? @@ -107,16 +98,24 @@ def states validates :order_cycle, presence: true, on: :require_distribution validates :distributor, presence: true, on: :require_distribution - make_permalink field: :number + before_validation :set_currency + before_validation :generate_order_number, if: :new_record? + before_validation :clone_billing_address, if: :use_billing? + before_validation :ensure_customer + before_create :link_by_email before_save :update_shipping_fees!, if: :complete? before_save :update_payment_fees!, if: :complete? + after_create :create_tax_charge! + after_save_commit DefaultAddressUpdater + make_permalink field: :number + attribute :send_cancellation_email, type: :boolean, default: true attribute :restock_items, type: :boolean, default: true - # -- Scopes + scope :not_empty, -> { left_outer_joins(:line_items).where.not(spree_line_items: { id: nil }) } From 13a22c56f4bcd873dadaffc989ae468ea4bd23b8 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 14:13:56 +0100 Subject: [PATCH 05/16] Move taxing of admin adjustments out of customer details controller --- .../spree/admin/orders/customer_details_controller.rb | 2 -- app/models/spree/order.rb | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/orders/customer_details_controller.rb b/app/controllers/spree/admin/orders/customer_details_controller.rb index ed13ae4e523..53d08434f51 100644 --- a/app/controllers/spree/admin/orders/customer_details_controller.rb +++ b/app/controllers/spree/admin/orders/customer_details_controller.rb @@ -56,8 +56,6 @@ def recalculate_taxes # If the order's address has been changed, the tax zone could be different, # which means a different set of tax rates might be applicable. @order.create_tax_charge! - Spree::TaxRate.adjust(@order, @order.adjustments.admin) - @order.update_totals_and_states end diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 62717fe3285..e7e87415d19 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -322,6 +322,7 @@ def create_tax_charge! Spree::TaxRate.adjust(self, line_items) Spree::TaxRate.adjust(self, shipments) if shipments.any? + Spree::TaxRate.adjust(self, adjustments.admin) if adjustments.admin.any? fee_handler.tax_enterprise_fees! end From a0f23fc510e45dffbd7a41881893cf329b975230 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 13 Jun 2023 18:16:09 +0100 Subject: [PATCH 06/16] Extract before_payment_state? method --- app/models/spree/order.rb | 6 +++++- app/services/order_workflow.rb | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index e7e87415d19..aa920e602da 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -316,7 +316,7 @@ def ship_total # Creates new tax charges if there are any applicable rates. If prices already # include taxes then price adjustments are created instead. def create_tax_charge! - return if state.in?(["cart", "address", "delivery"]) + return if before_payment_state? clear_legacy_taxes! @@ -574,6 +574,10 @@ def sorted_line_items end end + def before_payment_state? + state.in?(["cart", "address", "delivery"]) + end + private def deliver_order_confirmation_email diff --git a/app/services/order_workflow.rb b/app/services/order_workflow.rb index 53d7813e57e..815f3e1ee4c 100644 --- a/app/services/order_workflow.rb +++ b/app/services/order_workflow.rb @@ -24,13 +24,13 @@ def next(options = {}) end def advance_to_payment - return unless order.state.in? ["cart", "address", "delivery"] + return unless order.before_payment_state? advance_to_state("payment", advance_order_options) end def advance_checkout(options = {}) - advance_to = order.state.in?(["cart", "address", "delivery"]) ? "payment" : "confirmation" + advance_to = order.before_payment_state? ? "payment" : "confirmation" advance_to_state(advance_to, advance_order_options.merge(options)) end From c5dfecbb6922eb9dc99b0d050b477243d7956a71 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 13:12:07 +0100 Subject: [PATCH 07/16] Reapply taxes at model level if order address changes This should be done at the model level --- .../spree/admin/orders/customer_details_controller.rb | 8 -------- app/models/spree/order.rb | 9 +++++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/controllers/spree/admin/orders/customer_details_controller.rb b/app/controllers/spree/admin/orders/customer_details_controller.rb index 53d08434f51..cc6d6f679fd 100644 --- a/app/controllers/spree/admin/orders/customer_details_controller.rb +++ b/app/controllers/spree/admin/orders/customer_details_controller.rb @@ -24,7 +24,6 @@ def update end refresh_shipment_rates - recalculate_taxes OrderWorkflow.new(@order).advance_to_payment flash[:success] = Spree.t('customer_details_updated') @@ -52,13 +51,6 @@ def refresh_shipment_rates @order.shipments.map(&:refresh_rates) end - def recalculate_taxes - # If the order's address has been changed, the tax zone could be different, - # which means a different set of tax rates might be applicable. - @order.create_tax_charge! - @order.update_totals_and_states - end - def order_params params.require(:order).permit( :email, diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index aa920e602da..13d65cea257 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -108,6 +108,7 @@ def states before_save :update_payment_fees!, if: :complete? after_create :create_tax_charge! + after_save :reapply_tax_on_changed_address after_save_commit DefaultAddressUpdater @@ -580,6 +581,14 @@ def before_payment_state? private + def reapply_tax_on_changed_address + return if before_payment_state? + return unless tax_address&.saved_changes? + + create_tax_charge! + update_totals_and_states + end + def deliver_order_confirmation_email return if subscription.present? From 06c9697d0d539b9767224bcecabe6099f9699001 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 22:53:33 +0100 Subject: [PATCH 08/16] Don't try to cancel shipments that might already be cancelled --- app/models/spree/order/checkout.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/order/checkout.rb b/app/models/spree/order/checkout.rb index 6a6bee68c48..3575dc1ae0b 100644 --- a/app/models/spree/order/checkout.rb +++ b/app/models/spree/order/checkout.rb @@ -147,7 +147,7 @@ def state_changed(name) private def after_cancel - shipments.each(&:cancel!) + shipments.reject(&:canceled?).each(&:cancel!) payments.checkout.each(&:void!) OrderMailer.cancel_email(id).deliver_later if send_cancellation_email From 837e581c29851be86940818af373a6b414b07bae Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 8 Jun 2023 17:49:20 +0100 Subject: [PATCH 09/16] Reload order after cancelling it in tests that check shipment states --- spec/models/proxy_order_spec.rb | 1 + spec/models/spree/order_spec.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/spec/models/proxy_order_spec.rb b/spec/models/proxy_order_spec.rb index 8f6c9973aaa..17078eb5d2d 100644 --- a/spec/models/proxy_order_spec.rb +++ b/spec/models/proxy_order_spec.rb @@ -118,6 +118,7 @@ } break unless order.next! while !order.completed? order.cancel + order.reload end it "returns true, clears canceled_at and resumes the order" do diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index adaaf97f4ec..526ebb71b06 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -336,6 +336,7 @@ before do order.cancel! + order.reload order.resume! end From 6fa381197a53d4e98f5730e127ba477367a4bec9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 15:02:41 +0100 Subject: [PATCH 10/16] Don't tax fees before payment state --- app/services/order_fees_handler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/order_fees_handler.rb b/app/services/order_fees_handler.rb index f05351aec7e..47be2c99035 100644 --- a/app/services/order_fees_handler.rb +++ b/app/services/order_fees_handler.rb @@ -21,7 +21,7 @@ def recreate_all_fees! create_order_fees! end - tax_enterprise_fees! + tax_enterprise_fees! unless order.before_payment_state? order.update_order! end From ef09492883fea5d3d9511dd9238db6ecc66f8706 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 15:14:20 +0100 Subject: [PATCH 11/16] Update adjustment spec to use orders past delivery state --- spec/models/spree/adjustment_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 35a6ba5779a..f1b30805b55 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -250,6 +250,7 @@ module Spree context "when the shipment has an inclusive tax rate" do it "calculates the shipment tax from the tax rate" do order.shipments = [shipment] + order.state = "payment" order.create_tax_charge! order.update_totals @@ -274,6 +275,7 @@ module Spree it "records the tax on the shipment's adjustments" do order.shipments = [shipment] + order.state = "payment" order.create_tax_charge! order.update_totals @@ -355,6 +357,7 @@ module Spree context "when enterprise fees have a fixed tax_category" do before do + order.update(state: "payment") order.recreate_all_fees! end @@ -440,6 +443,11 @@ module Spree calculator: ::Calculator::FlatRate.new(preferred_amount: 50.0)) } + before do + order.update(state: "payment") + order.create_tax_charge! + end + describe "when the tax rate includes the tax in the price" do it "records no tax on the enterprise fee adjustments" do # EnterpriseFee tax category is nil and inheritance only applies to per item fees @@ -471,6 +479,11 @@ module Spree calculator: ::Calculator::PerItem.new(preferred_amount: 50.0)) } + before do + order.update(state: "payment") + order.create_tax_charge! + end + describe "when the tax rate includes the tax in the price" do it "records the correct amount in a tax adjustment" do # Applying product tax rate of 0.2 to enterprise fee of $50 @@ -522,6 +535,8 @@ module Spree tax_category.tax_rates << tax_rate allow(order).to receive(:tax_zone) { zone } order.line_items << create(:line_item, variant: variant, quantity: 5) + order.update(state: "payment") + order.create_tax_charge! end context "with included taxes" do From 11382a518eb15bf735ae345a8140984cfd77a39d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 5 Jun 2023 19:01:01 +0100 Subject: [PATCH 12/16] Fix test setup in BOM spec --- spec/system/admin/bulk_order_management_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/system/admin/bulk_order_management_spec.rb b/spec/system/admin/bulk_order_management_spec.rb index 269e06ccb0a..ce2921bf9ad 100644 --- a/spec/system/admin/bulk_order_management_spec.rb +++ b/spec/system/admin/bulk_order_management_spec.rb @@ -945,6 +945,9 @@ let!(:li2) { create(:line_item_with_shipment, order: o2 ) } before :each do + o1.update_totals_and_states + o2.update_totals_and_states + visit_bulk_order_management end From 52806a35eedf7fdc56bbf1994a9ff6ecb8e29908 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 13 Jun 2023 18:51:57 +0100 Subject: [PATCH 13/16] Fix Rubocop complaint --- app/controllers/split_checkout_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index aa27cfbfcb3..c7396b39dd5 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -226,7 +226,7 @@ def add_voucher adjustment = voucher.create_adjustment(voucher.code, @order) - if !adjustment.valid? + unless adjustment.valid? @order.errors.add(:voucher, I18n.t('split_checkout.errors.add_voucher_error')) adjustment.errors.each { |error| @order.errors.import(error) } return false From 93df70c0a7e6a3df1587e00c56ce405d968c76d0 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 13 Jun 2023 18:56:56 +0100 Subject: [PATCH 14/16] Use order total excluding discounts in voucher calculations --- app/models/spree/adjustment.rb | 2 ++ app/models/spree/order.rb | 5 +++++ app/models/voucher.rb | 2 +- spec/services/voucher_adjustments_service_spec.rb | 4 +--- spec/system/consumer/split_checkout_spec.rb | 5 +---- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb index 57ef3d549c7..7ebacacafdf 100644 --- a/app/models/spree/adjustment.rb +++ b/app/models/spree/adjustment.rb @@ -67,6 +67,8 @@ class Adjustment < ApplicationRecord scope :charge, -> { where('amount >= 0') } scope :credit, -> { where('amount < 0') } scope :return_authorization, -> { where(originator_type: "Spree::ReturnAuthorization") } + scope :voucher, -> { where(originator_type: "Voucher") } + scope :non_voucher, -> { where.not(originator_type: "Voucher") } scope :inclusive, -> { where(included: true) } scope :additional, -> { where(included: false) } scope :legacy_tax, -> { additional.tax.where(adjustable_type: "Spree::Order") } diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 13d65cea257..027bff09821 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -171,6 +171,11 @@ def amount line_items.inject(0.0) { |sum, li| sum + li.amount } end + # Order total without any applied discounts from vouchers + def pre_discount_total + item_total + all_adjustments.additional.eligible.non_voucher.sum(:amount) + end + def currency self[:currency] || Spree::Config[:currency] end diff --git a/app/models/voucher.rb b/app/models/voucher.rb index 94743fc7316..1199ad0820f 100644 --- a/app/models/voucher.rb +++ b/app/models/voucher.rb @@ -41,6 +41,6 @@ def create_adjustment(label, order) # We limit adjustment to the maximum amount needed to cover the order, ie if the voucher # covers more than the order.total we only need to create an adjustment covering the order.total def compute_amount(order) - -amount.clamp(0, order.total) + -amount.clamp(0, order.pre_discount_total) end end diff --git a/spec/services/voucher_adjustments_service_spec.rb b/spec/services/voucher_adjustments_service_spec.rb index 0fc08d7260d..fa2c1bb9516 100644 --- a/spec/services/voucher_adjustments_service_spec.rb +++ b/spec/services/voucher_adjustments_service_spec.rb @@ -14,9 +14,7 @@ it 'updates the adjustment amount to -order.total' do voucher.create_adjustment(voucher.code, order) - - order.total = 6 - order.save! + order.update_columns(item_total: 6) VoucherAdjustmentsService.calculate(order) diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb index f2939bb53e7..2bcc9f1cd0f 100644 --- a/spec/system/consumer/split_checkout_spec.rb +++ b/spec/system/consumer/split_checkout_spec.rb @@ -1114,11 +1114,8 @@ let(:voucher) { create(:voucher, code: 'some_code', enterprise: distributor, amount: 6) } before do - # Add voucher to the order voucher.create_adjustment(voucher.code, order) - - # Update order so voucher adjustment is properly taken into account - order.update_order! + order.update_totals VoucherAdjustmentsService.calculate(order) visit checkout_step_path(:summary) From 1a744de37a38f57aeade682a14de231588fb3fdb Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 13 Jun 2023 18:59:25 +0100 Subject: [PATCH 15/16] Improve voucher #calculate tests --- spec/models/voucher_spec.rb | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/spec/models/voucher_spec.rb b/spec/models/voucher_spec.rb index c5bbcdabe54..a41ec65812f 100644 --- a/spec/models/voucher_spec.rb +++ b/spec/models/voucher_spec.rb @@ -20,20 +20,21 @@ end describe '#compute_amount' do - subject { create(:voucher, code: 'new_code', enterprise: enterprise, amount: 10) } - let(:order) { create(:order_with_totals) } - it 'returns -10' do - expect(subject.compute_amount(order).to_f).to eq(-10) + context 'when order total is more than the voucher' do + subject { create(:voucher, code: 'new_code', enterprise: enterprise, amount: 5) } + + it 'uses the voucher total' do + expect(subject.compute_amount(order).to_f).to eq(-5) + end end - context 'when order total is smaller than 10' do - it 'returns minus the order total' do - order.total = 6 - order.save! + context 'when order total is less than the voucher' do + subject { create(:voucher, code: 'new_code', enterprise: enterprise, amount: 20) } - expect(subject.compute_amount(order).to_f).to eq(-6) + it 'matches the order total' do + expect(subject.compute_amount(order).to_f).to eq(-10) end end end From 202b0041d174a5e0cface44903411e426b1508be Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 13 Jun 2023 18:59:57 +0100 Subject: [PATCH 16/16] Remove voucher processing from checkout edit action --- app/controllers/split_checkout_controller.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index c7396b39dd5..31f2368b695 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -24,7 +24,6 @@ class SplitCheckoutController < ::BaseController def edit redirect_to_step_based_on_order unless params[:step] check_step if params[:step] - apply_voucher if @order.voucher_adjustments.present? flash_error_when_no_shipping_method_available if available_shipping_methods.none? end @@ -204,6 +203,7 @@ def shipping_method_ship_address_not_required? def process_voucher if add_voucher + VoucherAdjustmentsService.calculate(@order) render_voucher_section_or_redirect elsif @order.errors.present? render_error @@ -308,12 +308,4 @@ def check_step redirect_to checkout_step_path(:payment) if params[:step] == "summary" end end - - - def apply_voucher - VoucherAdjustmentsService.calculate(@order) - - # update order to take into account the voucher we applied - @order.update_order! - end end