diff --git a/CHANGELOG.md b/CHANGELOG.md index f8d17fe4345..0c365e4c320 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ ## Solidus 1.3.0 (unreleased) +* Deprecate setting a line item's currency by hand + + Previously, a line item's currency could be set directly, and differently from the line item's + order's currency. This would result in an error. It still does, but is also now explicitly + deprecated. In the future, we might delete the line item's `currency` column and just delegate + to the line item's order. + * Taxes for carts now configurable via the `Spree::Store` object In VAT countries, carts (orders without addresses) have to be shown with diff --git a/core/app/models/spree/line_item.rb b/core/app/models/spree/line_item.rb index f0af132b90a..db63644c186 100644 --- a/core/app/models/spree/line_item.rb +++ b/core/app/models/spree/line_item.rb @@ -104,15 +104,10 @@ def insufficient_stock? def options=(options = {}) return unless options.present? - opts = options.dup # we will be deleting from the hash, so leave the caller's copy intact + self.price = variant.price_in(currency).amount + + variant.price_modifier_amount_in(currency, options) - currency = opts.delete(:currency) || order.currency - - self.currency = currency - self.price = variant.price_in(currency).amount + - variant.price_modifier_amount_in(currency, opts) - - assign_attributes opts + assign_attributes options end private @@ -136,7 +131,7 @@ def set_pricing_attributes # If the legacy method #copy_price has been overridden, handle that gracefully return handle_copy_price_override if respond_to?(:copy_price) - self.currency ||= variant.currency + self.currency ||= order.currency self.cost_price ||= variant.cost_price self.price ||= variant.price end @@ -175,7 +170,10 @@ def update_tax_charge end def ensure_proper_currency - unless currency == order.currency + if currency != order.currency + Spree::Deprecation.warn "The line items currency is different from it's order currency. " \ + "This behavior is not supported anymore and will be deleted soon.", + caller errors.add(:currency, :must_match_order_currency) end end diff --git a/core/app/models/spree/order_contents.rb b/core/app/models/spree/order_contents.rb index 7cbb934c4ad..83faad38122 100644 --- a/core/app/models/spree/order_contents.rb +++ b/core/app/models/spree/order_contents.rb @@ -1,6 +1,6 @@ module Spree class OrderContents - attr_accessor :order, :currency + attr_accessor :order def initialize(order) @order = order @@ -79,17 +79,20 @@ def reload_totals def add_to_line_item(variant, quantity, options = {}) line_item = grab_line_item_by_variant(variant, false, options) - if line_item - line_item.quantity += quantity.to_i - line_item.currency = currency unless currency.nil? - else - opts = { currency: order.currency }.merge ActionController::Parameters.new(options).permit(PermittedAttributes.line_item_attributes).to_h - line_item = order.line_items.new(quantity: quantity, - variant: variant, - options: opts) + line_item ||= order.line_items.new( + quantity: 0, + variant: variant, + currency: order.currency + ) + + line_item.quantity += quantity.to_i + line_item.options = ActionController::Parameters.new(options).permit(PermittedAttributes.line_item_attributes) + + if line_item.new_record? create_order_stock_locations(line_item, options[:stock_location_quantities]) end - line_item.target_shipment = options[:shipment] if options.key? :shipment + + line_item.target_shipment = options[:shipment] line_item.save! line_item end diff --git a/core/spec/models/spree/line_item_spec.rb b/core/spec/models/spree/line_item_spec.rb index 858f49e6d43..ad84996596a 100644 --- a/core/spec/models/spree/line_item_spec.rb +++ b/core/spec/models/spree/line_item_spec.rb @@ -106,8 +106,8 @@ expect(line_item.cost_price).to eq(variant.cost_price) end - it 'copies the variants currency' do - expect(line_item.currency).to eq(variant.currency) + it "copies the order's currency" do + expect(line_item.currency).to eq(order.currency) end # Test for https://github.com/spree/spree/issues/3481 @@ -164,12 +164,6 @@ def copy_price end end - describe '.currency' do - it 'returns the globally configured currency' do - line_item.currency == 'USD' - end - end - describe ".money" do before do line_item.price = 3.50 @@ -188,23 +182,27 @@ def copy_price end end - context "currency same as order.currency" do - it "is a valid line item" do - line_item = order.line_items.first - line_item.currency = order.currency - line_item.valid? + context 'setting the currency' do + let(:line_item) { order.line_items.first } + + context "currency same as order.currency" do + it "is a valid line item" do + line_item.currency = order.currency + line_item.valid? - expect(line_item.error_on(:currency).size).to eq(0) + expect(line_item.error_on(:currency).size).to eq(0) + end end - end - context "currency different than order.currency" do - it "is not a valid line item" do - line_item = order.line_items.first - line_item.currency = "no currency" - line_item.valid? + context "currency different than order.currency" do + it "is not a valid line item" do + expect(Spree::Deprecation).to receive(:warn).at_least(:once) - expect(line_item.error_on(:currency).size).to eq(1) + line_item.currency = "no currency" + line_item.valid? + + expect(line_item.error_on(:currency).size).to eq(1) + end end end