From b843b871f6b92acc6d817022d72752a153cd93a4 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 30 Apr 2021 12:53:12 +0100
Subject: [PATCH] Ensure order totals and payment/shipment states are correct
 when changing shipping method on a completed order.

This used to happen via an after_save callback in Shipment, which called `order.update!`. That has recently been removed. After changing a shipment's selected shipping rate (shipping method), we need to ensure the order's totals and states are updated. We don't need to update all of the order's adjustments though (see previous commit).

FYI Spree handled this issue like this: https://github.com/spree/spree/commit/24388485ed9d2a6f5d5b16e8f1c9528d393d8981, but there are lots of things about that commit that are clearly awful, like: params handling in a model, duplication of Order::Updater logic across the codebase, the Shipment class having responsiblity for knowing which things need to be updated on the order, etc. The result is ultimately the same as what we're doing here though.
---
 app/controllers/api/v0/shipments_controller.rb       | 4 +++-
 spec/controllers/api/v0/shipments_controller_spec.rb | 7 +------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb
index 0d92b5915a..33706ff971 100644
--- a/app/controllers/api/v0/shipments_controller.rb
+++ b/app/controllers/api/v0/shipments_controller.rb
@@ -34,7 +34,9 @@ module Api
           @shipment.fee_adjustment.open
         end
 
-        @shipment.update(shipment_params)
+        if @shipment.update(shipment_params)
+          @order.updater.update_totals_and_states
+        end
 
         if unlock == 'yes'
           @shipment.fee_adjustment.close
diff --git a/spec/controllers/api/v0/shipments_controller_spec.rb b/spec/controllers/api/v0/shipments_controller_spec.rb
index 86e269cb17..a5bde9b708 100644
--- a/spec/controllers/api/v0/shipments_controller_spec.rb
+++ b/spec/controllers/api/v0/shipments_controller_spec.rb
@@ -225,12 +225,7 @@ describe Api::V0::ShipmentsController, type: :controller do
 
           expect(order.shipment.shipping_method).to eq shipping_method2
           expect(order.shipment.cost).to eq 20
-          expect(order.total).to eq 60 # order totals have not been updated
-          expect(order.payment_state).to eq "paid" # payment state not updated
-
-          order.update! # Simulate "Update and recalculate fees" button
-
-          expect(order.total).to eq 70 # order total has now changed
+          expect(order.total).to eq 70 # item total is 50, shipping cost is 20
           expect(order.payment_state).to eq "balance_due" # total changed, payment is due
         end
       end