Do not process order returns automatically in #process_inventory_unit…

…! callback

Processing order returns in `ReturnItem#process_inventory_unit!` does not work
reliably when there are multiple return items processed at the same time (see
issue solidusio#3198).

Also, decoupling sensible interactions with other models in ActiveRecord
callbacks simplifies the understandability of code and its flow.

Order processing in `#process_inventory_unit!` is now only deprecated, but will
be completely removed in the following release of Solidus.

backend/app/controllers/spree/admin/customer_returns_controller.rb

@@ -10,6 +10,7 @@ class CustomerReturnsController < ResourceController
       before_action :load_form_data, only: [:new, :edit]
       before_action :build_return_items_from_params, only: [:create]
       create.fails  :load_form_data
+      create.after :order_process_return
 
       def edit
         @pending_return_items = @customer_return.return_items.select(&:pending?)
@@ -23,6 +24,10 @@ def edit
 
       private
 
+      def order_process_return
+        @customer_return.process_return!
+      end
+
       def location_after_save
         url_for([:edit, :admin, @order, @customer_return])
       end
@@ -60,11 +65,13 @@ def permitted_resource_params
 
       def build_return_items_from_params
         return_items_params = permitted_resource_params.delete(:return_items_attributes).values
-
         @customer_return.return_items = return_items_params.map do |item_params|
           next unless item_params.delete('returned') == '1'
           return_item = item_params[:id] ? Spree::ReturnItem.find(item_params[:id]) : Spree::ReturnItem.new
           return_item.assign_attributes(item_params)
+
+          return_item.skip_customer_return_processing = true
+
           if item_params[:reception_status_event].blank?
             return redirect_to(new_object_url, flash: { error: 'Reception status choice required' })
           end

backend/spec/features/admin/orders/customer_returns_spec.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Customer returns', type: :feature do
+  stub_authorization!
+
+  context 'when the order has more than one line item' do
+    let(:order) { create :shipped_order, line_items_count: 2 }
+
+    context 'when creating a return with state "Received"' do
+      it 'marks the order as returned', :js do
+        visit spree.new_admin_order_customer_return_path(order)
+
+        find('#select-all').click
+        page.execute_script "$('select.add-item').val('receive')"
+        select 'NY Warehouse', from: 'Stock Location'
+        click_button 'Create'
+
+        expect(page).to have_content 'Customer Return has been successfully created'
+
+        within 'dd.order-state' do
+          expect(page).to have_content 'Returned'
+        end
+      end
+    end
+  end
+end

core/app/models/spree/return_item.rb

@@ -130,6 +130,8 @@ def reception_completed?
       after_transition any => any, do: :persist_acceptance_status_errors
     end
 
+    attr_accessor :skip_customer_return_processing
+
     # @param inventory_unit [Spree::InventoryUnit] the inventory for which we
     #   want a return item
     # @return [Spree::ReturnItem] a valid return item for the given inventory
@@ -233,10 +235,12 @@ def currency
 
     def process_inventory_unit!
       inventory_unit.return!
-
       if customer_return
         customer_return.stock_location.restock(inventory_unit.variant, 1, customer_return) if should_restock?
-        customer_return.process_return!
+        unless skip_customer_return_processing
+          Deprecation.warn 'From Solidus v2.9 onwards, #process_inventory_unit! will not call customer_return#process_return!'
+          customer_return.process_return!
+        end
       end
     end
 

core/spec/models/spree/return_item_spec.rb

@@ -43,6 +43,15 @@
       subject
     end
 
+    context 'when the `skip_customer_return_processing` flag is not set to true' do
+      before { return_item.skip_customer_return_processing = false }
+
+      it 'shows a deprecation warning' do
+        expect(Spree::Deprecation).to receive(:warn)
+        subject
+      end
+    end
+
     context 'when there is a received return item with the same inventory unit' do
       let!(:return_item_with_dupe_inventory_unit) { create(:return_item, inventory_unit: inventory_unit, reception_status: 'received') }