diff --git a/app/models/voucher.rb b/app/models/voucher.rb index 94674cb7d3..bb65c33d21 100644 --- a/app/models/voucher.rb +++ b/app/models/voucher.rb @@ -3,8 +3,6 @@ class Voucher < ApplicationRecord acts_as_paranoid - include CalculatedAdjustments - belongs_to :enterprise has_many :adjustments, @@ -15,8 +13,6 @@ class Voucher < ApplicationRecord validates :code, presence: true, uniqueness: { scope: :enterprise_id } - before_validation :add_calculator - def self.adjust!(order) return if order.nil? @@ -29,6 +25,10 @@ class Voucher < ApplicationRecord # Recalculate value amount = adjustment.originator.compute_amount(order) + # It is quite possible to have an order with both tax included in and tax excluded from price. + # We should be able to caculate the relevant amount apply the current calculation. + # + # For now we just assume it is either all tax included in price or all tax excluded from price. if order.additional_tax_total.positive? handle_tax_excluded_from_price(order, amount) else @@ -42,7 +42,6 @@ class Voucher < ApplicationRecord def self.handle_tax_excluded_from_price(order, amount) voucher_rate = amount / order.total - # TODO: might need to use VoucherTax has originator (sub class of Voucher) # Adding the voucher tax part tax_amount = voucher_rate * order.additional_tax_total @@ -92,12 +91,12 @@ class Voucher < ApplicationRecord Spree::Money.new(value) end - # override the one from CalculatedAdjustments - # Create an "open" adjustment which will be updated later once tax and other fees have - # been applied to the order + # Ideally we would use `include CalculatedAdjustments` to be consistent with other adjustments, + # but vouchers have complicated calculation so we can't easily use Spree::Calculator. We keep + # the same method to stay as consistent as possible. # - # rubocop:disable Style/OptionalBooleanParameter - def create_adjustment(label, order, mandatory = false, _state = "open", tax_category = nil) + # Creates a new voucher adjustment for the given order + def create_adjustment(label, order) amount = compute_amount(order) adjustment_attributes = { @@ -105,32 +104,17 @@ class Voucher < ApplicationRecord originator: self, order: order, label: label, - mandatory: mandatory, + mandatory: false, state: "open", - tax_category: tax_category + tax_category: nil } order.adjustments.create(adjustment_attributes) end - # rubocop:enable Style/OptionalBooleanParameter - # override the one from CalculatedAdjustments so 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 - # Doesn't work with taxes for now - # TODO move this to a calculator + # 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 = calculator.compute(order) - - return -order.total if amount.abs > order.total - - amount - end - - private - - # For now voucher are only flat rate of 10 - def add_calculator - self.calculator = Calculator::FlatRate.new(preferred_amount: -value) + -value.clamp(0, order.total) end end diff --git a/spec/models/voucher_spec.rb b/spec/models/voucher_spec.rb index e01a3a0935..cbec1c7626 100644 --- a/spec/models/voucher_spec.rb +++ b/spec/models/voucher_spec.rb @@ -17,18 +17,6 @@ describe Voucher do it { is_expected.to validate_uniqueness_of(:code).scoped_to(:enterprise_id) } end - describe 'after_save' do - subject { Voucher.create(code: 'new_code', enterprise: enterprise) } - - it 'adds a FlateRate calculator' do - expect(subject.calculator.instance_of?(Calculator::FlatRate)).to be(true) - end - - it 'has a preferred_amount of -10' do - expect(subject.calculator.preferred_amount.to_f).to eq(-10) - end - end - describe '.adjust!' do let(:voucher) { Voucher.create(code: 'new_code', enterprise: enterprise) }