Merge pull request #3968 from mkllnk/3924-rescue-failing-shipment

Rescue checkout on shipping id conflict

app/models/spree/order_decorator.rb

@@ -166,7 +166,23 @@ Spree::Order.class_eval do
     shipments.each do |shipment|
       next if shipment.shipped?
       update_adjustment! shipment.adjustment if shipment.adjustment
-      shipment.save # updates included tax
+      save_or_rescue_shipment(shipment)
+    end
+  end
+
+  def save_or_rescue_shipment(shipment)
+    shipment.save # updates included tax
+  rescue ActiveRecord::RecordNotUnique => error
+    # This error was seen in production on `shipment.save` above.
+    # It caused lost payments and duplicate payments due to database rollbacks.
+    # While we don't understand the cause of this error yet, we rescue here
+    # because an outdated shipping fee is not as bad as a lost payment.
+    # And the shipping fee is already up-to-date when this error occurs.
+    # https://github.com/openfoodfoundation/openfoodnetwork/issues/3924
+    Bugsnag.notify(error) do |report|
+      report.add_tab(:order, attributes)
+      report.add_tab(:shipment, shipment.attributes)
+      report.add_tab(:shipment_in_db, Spree::Shipment.find_by_id(shipment.id).attributes)
     end
   end
 

spec/models/spree/order_spec.rb

@@ -756,6 +756,24 @@ describe Spree::Order do
 
         expect { order.next! }.to change { order.state }.from("delivery").to("payment")
       end
+
+      it "advances to complete state despite error" do
+        advance_to_delivery_state(order)
+        # advance to payment state
+        order.next!
+
+        create(:payment, order: order)
+        # https://github.com/openfoodfoundation/openfoodnetwork/issues/3924
+        observed_error = ActiveRecord::RecordNotUnique.new(
+          "PG::UniqueViolation",
+          StandardError.new
+        )
+        expect(order.shipment).to receive(:save).and_call_original
+        expect(order.shipment).to receive(:save).and_call_original
+        expect(order.shipment).to receive(:save).and_raise(observed_error)
+
+        expect { order.next! }.to change { order.state }.from("payment").to("complete")
+      end
     end
 
     context "when the order is a subscription" do