Merge pull request #10135 from abdellani/fix_when_user_deletes_a_shipping_method_existing_orders_are_updated_with_a_random_shipping_method

Fix When user deletes a shipping method existing orders are updated with a random shipping method

app/models/spree/shipment.rb

@@ -116,7 +116,22 @@ module Spree
 
       # The call to Stock::Estimator below will replace the current shipping_method
       original_shipping_method_id = shipping_method.try(:id)
-      self.shipping_rates = OrderManagement::Stock::Estimator.new(order).shipping_rates(to_package)
+
+      estimator = OrderManagement::Stock::Estimator.new(order)
+      distributor_shipping_rates = estimator.shipping_rates(to_package)
+
+      if original_shipping_method_id.present? &&
+         distributor_shipping_rates.map(&:shipping_method_id)
+             .exclude?(original_shipping_method_id)
+        cost = estimator.calculate_cost(shipping_method, to_package)
+        unless cost.nil?
+          original_shipping_rate = shipping_method.shipping_rates.new(cost: cost)
+          self.shipping_rates = distributor_shipping_rates + [original_shipping_rate]
+          self.selected_shipping_rate_id = original_shipping_rate.id
+        end
+      else
+        self.shipping_rates = distributor_shipping_rates
+      end
 
       keep_original_shipping_method_selection(original_shipping_method_id)
 

engines/order_management/app/services/order_management/stock/estimator.rb

@@ -29,6 +29,10 @@ module OrderManagement
         shipping_rates
       end
 
+      def calculate_cost(shipping_method, package)
+        shipping_method.calculator.compute(package)
+      end
+
       private
 
       # Sets the first available shipping method to "selected".
@@ -56,10 +60,6 @@ module OrderManagement
         }
         shipping_methods
       end
-
-      def calculate_cost(shipping_method, package)
-        shipping_method.calculator.compute(package)
-      end
     end
   end
 end

spec/controllers/api/v0/shipments_controller_spec.rb

@@ -246,6 +246,7 @@ describe Api::V0::ShipmentsController, type: :controller do
 
       before do
         order.shipments.first.shipping_methods = [shipping_method1, shipping_method2]
+        order.shipments.each(&:refresh_rates)
         order.select_shipping_method(shipping_method1.id)
         order.update_order!
         order.update_columns(

spec/system/admin/order_spec.rb

@@ -420,7 +420,7 @@ describe '
 
     describe "viewing the edit page" do
       let!(:shipping_method_for_distributor1) do
-        create(:shipping_method, name: "Normal", distributors: [distributor1])
+        create(:shipping_method_with, :flat_rate, name: "Normal", amount: 12, distributors: [distributor1])
       end
       let!(:order) do
         create(:order_with_taxes, distributor: distributor1, ship_address: create(:address),
@@ -548,7 +548,7 @@ describe '
 
       context "with different shipping methods" do
         let!(:different_shipping_method_for_distributor1) do
-          create(:shipping_method, name: "Different", distributors: [distributor1])
+          create(:shipping_method_with, :flat_rate, name: "Different", amount: 15, distributors: [distributor1])
         end
         let!(:shipping_method_for_distributor2) do
           create(:shipping_method, name: "Other", distributors: [distributor2])
@@ -570,6 +570,81 @@ describe '
           find('.save-method').click
 
           expect(page).to have_content "Shipping: #{different_shipping_method_for_distributor1.name}"
+
+          within "#order-total" do 
+            expect(page).to have_content "$175.00"
+          end
+        end
+
+        context "when the distributor unsupport a shipping method that's selected in an existing order " do
+          before do
+            distributor1.shipping_methods = [shipping_method_for_distributor1, different_shipping_method_for_distributor1]
+            order.shipments.each(&:refresh_rates)
+            order.shipment.adjustments.first.open
+            order.select_shipping_method(different_shipping_method_for_distributor1)
+            order.shipment.adjustments.first.close
+            distributor1.shipping_methods = [shipping_method_for_distributor1]
+          end
+          context "shipment is shipped" do
+            before do
+              order.shipments.first.update_attribute(:state, 'shipped')
+            end
+
+            it "should not change the shipping method" do
+              visit spree.edit_admin_order_path(order)
+              expect(page).to have_content "Shipping: #{different_shipping_method_for_distributor1.name} $15.00"
+
+              within "#order-total" do 
+                expect(page).to have_content "$160.00"
+              end
+            end
+
+            context "when shipping rate is updated" do
+              before do
+                different_shipping_method_for_distributor1.shipping_rates.first.update!(cost: 16)              
+              end
+
+              it "should not update the shipping cost" do
+                visit spree.edit_admin_order_path(order)
+                expect(page).to have_content "Shipping: #{different_shipping_method_for_distributor1.name} $15.00"
+
+                within "#order-total" do 
+                  expect(page).to have_content "$160.00"
+                end  
+              end
+            end
+          end
+          context "shipment is pending" do
+            before do
+              order.shipments.first.ensure_correct_adjustment
+              expect(order.shipments.first.state).to eq('pending')
+            end
+
+            it "should not replace the selected shipment method" do
+              visit spree.edit_admin_order_path(order)
+              expect(page).to have_content "Shipping: #{different_shipping_method_for_distributor1.name} $15.00"
+
+              within "#order-total" do 
+                expect(page).to have_content "$160.00"
+              end
+            end
+
+            context "when shipping rate is updated" do
+              before do
+                different_shipping_method_for_distributor1.shipping_rates.first.update!(cost: 16)             
+              end
+
+              it "should not update the shipping cost" do
+                # Since the order is completed, the price is not supposed to be updated 
+                visit spree.edit_admin_order_path(order)
+                expect(page).to have_content "Shipping: #{different_shipping_method_for_distributor1.name} $15.00"
+
+                within "#order-total" do 
+                  expect(page).to have_content "$160.00"
+                end  
+              end
+            end
+          end
         end
       end