diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index fec601be76..22a44e26de 100644 --- a/app/models/spree/order_decorator.rb +++ b/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 diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index b39bad4ccc..c6d96c71be 100644 --- a/spec/models/spree/order_spec.rb +++ b/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