mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-27 01:43:22 +00:00
Refactor Voucher to get rid of CalculatedAdjustments
Voucher adjustment have complicated calculation that prevent us from using Spree::Calculator, and we need to override CalculatedAdjustments method, so no point using it. We still keep the same methods to stay as consitent as possible in regards to adjustments
This commit is contained in:
committed by
Maikel Linke
parent
3f37554ff9
commit
56e981d300
@@ -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
|
||||
|
||||
@@ -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) }
|
||||
|
||||
|
||||
Reference in New Issue
Block a user