Always set a line item's currency to order currency

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

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

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

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