From 22449155e769b823b11a399ebcd33cfd9ce19041 Mon Sep 17 00:00:00 2001 From: Magnus von Koeller Date: Wed, 23 Sep 2015 16:32:07 -0400 Subject: [PATCH 1/3] Always pass the new order number on unreturned exchange failures Currently, anything that's not specifically a `ChargeFailure` will not include the new order number. That's a pity, because something like an out of stock error shows up as an exception that is not a `ChargeFailure` and then it's really hard to even find the new error. This always makes the new order available to the outside so that we don't have to specifically read it from the exception. --- core/lib/spree/core/unreturned_item_charger.rb | 4 ++-- core/lib/tasks/exchanges.rake | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/lib/spree/core/unreturned_item_charger.rb b/core/lib/spree/core/unreturned_item_charger.rb index 2636c784415..976baaf71ad 100644 --- a/core/lib/spree/core/unreturned_item_charger.rb +++ b/core/lib/spree/core/unreturned_item_charger.rb @@ -53,12 +53,12 @@ def charge_for_items end - private - def new_order @new_order ||= Spree::Order.create!(exchange_order_attributes) end + private + def add_exchange_variants_to_order @return_items.group_by(&:exchange_variant).map do |variant, variant_return_items| variant_inventory_units = variant_return_items.map(&:exchange_inventory_unit) diff --git a/core/lib/tasks/exchanges.rake b/core/lib/tasks/exchanges.rake index 2cc8b24e393..d06b966e3b6 100644 --- a/core/lib/tasks/exchanges.rake +++ b/core/lib/tasks/exchanges.rake @@ -20,12 +20,13 @@ namespace :exchanges do begin item_charger.charge_for_items rescue Spree::UnreturnedItemCharger::ChargeFailure => e - failure = {message: e.message, new_order: e.new_order.try(:number)} + failure = { message: e.message } rescue Exception => e - failure = {message: "#{e.class}: #{e.message}"} + failure = { message: "#{e.class}: #{e.message}" } end if failure + failure[:new_order] = item_charger.new_order.number if item_charger.new_order failures << failure.merge({ order: item_charger.original_order.number, shipment: shipment.number, From 5ecca069b44610339ddeae5769392b620b87c997 Mon Sep 17 00:00:00 2001 From: Magnus von Koeller Date: Wed, 23 Sep 2015 18:05:22 -0400 Subject: [PATCH 2/3] Safer new_order handling --- core/lib/spree/core/unreturned_item_charger.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/lib/spree/core/unreturned_item_charger.rb b/core/lib/spree/core/unreturned_item_charger.rb index 976baaf71ad..7e31409e8ec 100644 --- a/core/lib/spree/core/unreturned_item_charger.rb +++ b/core/lib/spree/core/unreturned_item_charger.rb @@ -10,7 +10,7 @@ def initialize(message, new_order) class_attribute :failure_handler - attr_reader :original_order + attr_reader :original_order, :new_order def initialize(shipment, return_items) @shipment = shipment @@ -19,6 +19,8 @@ def initialize(shipment, return_items) end def charge_for_items + self.new_order = Spree::Order.create!(exchange_order_attributes) + new_order.associate_user!(@original_order.user) if @original_order.user add_exchange_variants_to_order @@ -53,12 +55,10 @@ def charge_for_items end - def new_order - @new_order ||= Spree::Order.create!(exchange_order_attributes) - end - private + attr_writer :new_order + def add_exchange_variants_to_order @return_items.group_by(&:exchange_variant).map do |variant, variant_return_items| variant_inventory_units = variant_return_items.map(&:exchange_inventory_unit) From 9bf8206cbd85244cb87892d68eb7657545bb0f27 Mon Sep 17 00:00:00 2001 From: Magnus von Koeller Date: Mon, 28 Sep 2015 13:23:28 -0400 Subject: [PATCH 3/3] Don't rescue Exception --- core/lib/tasks/exchanges.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/tasks/exchanges.rake b/core/lib/tasks/exchanges.rake index d06b966e3b6..6e6a62d9e0c 100644 --- a/core/lib/tasks/exchanges.rake +++ b/core/lib/tasks/exchanges.rake @@ -21,7 +21,7 @@ namespace :exchanges do item_charger.charge_for_items rescue Spree::UnreturnedItemCharger::ChargeFailure => e failure = { message: e.message } - rescue Exception => e + rescue => e failure = { message: "#{e.class}: #{e.message}" } end