From a05f2aabd56582506493c406c7bd9be3f910c5a1 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Fri, 18 Mar 2016 18:00:00 +0100 Subject: [PATCH] Assign a line items currency as an attribute, not an option When extracting pricing to service objects I came across a number of ways of setting a line item's currency. In `OrderContents`, the currency, although an attribute of the line item, is passed to the line item through an options Hash designed for an extension point introduced around Spree 2.2/2.3 while it could just be assigned directly like any other attribute. This PR refactors that bit of weird code, hopefully relieving us of the `options=` method on line item entirely in the future. --- core/app/models/spree/line_item.rb | 11 +++-------- core/app/models/spree/order_contents.rb | 12 +++++++----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/core/app/models/spree/line_item.rb b/core/app/models/spree/line_item.rb index f0af132b90a..c9cb07e7d7a 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.except(:currency) end private diff --git a/core/app/models/spree/order_contents.rb b/core/app/models/spree/order_contents.rb index 7cbb934c4ad..a6ad22c4dc2 100644 --- a/core/app/models/spree/order_contents.rb +++ b/core/app/models/spree/order_contents.rb @@ -81,12 +81,14 @@ def add_to_line_item(variant, quantity, 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) + permitted_controller_parameters = ActionController::Parameters.new(options).permit(PermittedAttributes.line_item_attributes).to_h + line_item = order.line_items.new( + quantity: quantity, + variant: variant, + currency: order.currency, + options: permitted_controller_parameters + ) create_order_stock_locations(line_item, options[:stock_location_quantities]) end line_item.target_shipment = options[:shipment] if options.key? :shipment