From 9e145bd73694b81e9ccdb88a1aca4a71f2ab19b3 Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <martin@stembolt.com>
Date: Sun, 14 May 2017 00:32:37 +0200
Subject: [PATCH] Allow refreshing rates for unshipped complete orders

When an order is complete, but not shipped yet, admins need to be able
to refresh rates for a shipment.

This did not work for two reasons:
- `#refresh_rates` was not called on a new shipment
- If it had been called, it would not have done anything.

I added an expectation for a new shipment to be generated with a selected
shipping rate to test this behaviour.

This is a less scary change than it seems since the order updater does not
refresh rates automagically anymore.
---
 CHANGELOG.md                                         | 1 +
 backend/spec/features/admin/orders/shipments_spec.rb | 7 +++++++
 core/app/models/spree/shipment.rb                    | 3 ++-
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4fab8fd8b98..297c8b74740 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## Solidus 2.3.0 (master, unreleased)
 
+- Allow refreshing shipping rates for unshipped shipments on completed orders [\#1906](https://github.com/solidusio/solidus/pull/1906) ([mamhoff](https://github.com/mamhoff))
 - Renamed `PaymentMethod#method_type` into `partial_name` [\#1978](https://github.com/solidusio/solidus/pull/1978) ([tvdeyen](https://github.com/tvdeyen))
 - Order#outstanding_balance now uses reimbursements instead of refunds to calculate the amount that should be paid on an order. [#2002](https://github.com/solidusio/solidus/pull/2002) (many contributors :heart:)
 
diff --git a/backend/spec/features/admin/orders/shipments_spec.rb b/backend/spec/features/admin/orders/shipments_spec.rb
index db38be271bd..d98935b48d7 100644
--- a/backend/spec/features/admin/orders/shipments_spec.rb
+++ b/backend/spec/features/admin/orders/shipments_spec.rb
@@ -73,11 +73,18 @@ def ship_shipment
       expect(page).to have_css("#shipment_#{shipment1.id} tr.stock-item", count: 4)
       shipment2 = (order.reload.shipments.to_a - [shipment1]).first
       expect(page).to have_css("#shipment_#{shipment2.id} tr.stock-item", count: 1)
+      within "#shipment_#{shipment2.id}" do
+        expect(page).to have_content("UPS Ground")
+      end
 
       within_row(2) { click_icon 'arrows-h' }
       complete_split_to("LA(#{shipment2.number})")
       expect(page).to have_css("#shipment_#{shipment2.id} tr.stock-item", count: 2)
       expect(page).to have_css("#shipment_#{shipment1.id} tr.stock-item", count: 3)
+
+      within "#shipment_#{shipment2.id}" do
+        expect(page).to have_content("UPS Ground")
+      end
     end
 
     context "with a ready-to-ship order" do
diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb
index 50a737f6987..06b251782c4 100644
--- a/core/app/models/spree/shipment.rb
+++ b/core/app/models/spree/shipment.rb
@@ -172,7 +172,7 @@ def ready_or_pending?
     end
 
     def refresh_rates
-      return shipping_rates if shipped? || order.completed?
+      return shipping_rates if shipped?
       return [] unless can_get_rates?
 
       # StockEstimator.new assigment below will replace the current shipping_method
@@ -333,6 +333,7 @@ def transfer_to_location(variant, quantity, stock_location)
         order.contents.add(variant, quantity, { shipment: new_shipment })
 
         refresh_rates
+        new_shipment.refresh_rates
         save!
         new_shipment.save!
       end