Fix updating the order state after returning items

The problem was a composite of many problems which, hopefully, are
now fixed.

The first one was that order.inventory_units was memoized with
completely different instances from ReturnItem#inventory_unit. So
the check to see if they were all returned was, at times, ineffective.

The second part is that the state_machines library can halt callback
chains without telling anything to anyone. That was happening in some
cases when ReturnItem#attempt_accept was failing. In those cases, just
to increase confusion, the acceptance state-machine was interfering
with the reception state-machine.

The third part is that the introduction of the accessor for
skip_customer_return_processing was fixing one code path (the one
still visible in the original PR) but was disrupting another one,
namely marking the order as returned after the return-item state-
machine was getting the "receive" event.

backend/spec/controllers/spree/admin/return_items_controller_spec.rb

@@ -34,7 +34,13 @@
       let(:new_reception_status) { 'received' }
       let(:reception_status_event) { 'receive' }
 
-      before { return_item.update reception_status: 'in_transit'}
+      before do
+        allow(Spree::Deprecation).to receive(:warn) do |message|
+          expect(message).to match('#process_inventory_unit! will not call')
+        end
+
+        return_item.update reception_status: 'in_transit'
+      end
 
       subject do
         put :update, params: { id: return_item.to_param, return_item: { reception_status_event: reception_status_event } }

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

@@ -17,6 +17,10 @@ def order_state_label
   end
 
   before do
+    allow(Spree::Deprecation).to receive(:warn) do |message|
+      expect(message).to match('#process_inventory_unit! will not call')
+    end
+
     visit spree.new_admin_order_customer_return_path(order)
   end
 
@@ -31,5 +35,43 @@ def order_state_label
         expect(order_state_label).to eq('Returned')
       end
     end
+
+    context 'when creating a return with state "In Transit" and then marking it as "Received"' do
+      it 'marks the order as returned', :js do
+        create_customer_return('in_transit')
+        expect(page).to have_content 'Customer Return has been successfully created'
+        expect(order_state_label).to eq('Complete')
+
+        within('[data-hook="rejected_return_items"] tbody tr:nth-child(1)') { click_button('Receive') }
+        expect(order_state_label).to eq('Complete')
+
+        within('[data-hook="rejected_return_items"] tbody tr:nth-child(2)') { click_button('Receive') }
+        expect(order_state_label).to eq('Returned')
+      end
+    end
+  end
+
+  context 'when the order has only one line item' do
+    let(:order) { create :shipped_order, line_items_count: 1 }
+
+    context 'when creating a return with state "Received"' do
+      it 'marks the order as returned', :js do
+        create_customer_return('receive')
+
+        expect(page).to have_content 'Customer Return has been successfully created'
+        expect(order_state_label).to eq('Returned')
+      end
+    end
+
+    context 'when creating a return with state "In Transit" and then marking it as "Received"' do
+      it 'marks the order as returned', :js do
+        create_customer_return('in_transit')
+        expect(page).to have_content 'Customer Return has been successfully created'
+        expect(order_state_label).to eq('Complete')
+
+        within('[data-hook="rejected_return_items"] tbody tr:nth-child(1)') { click_button('Receive') }
+        expect(order_state_label).to eq('Returned')
+      end
+    end
   end
 end

core/app/models/spree/order.rb

@@ -286,7 +286,12 @@ def allow_cancel?
     end
 
     def all_inventory_units_returned?
-      inventory_units.all?(&:returned?)
+      # Inventory units are transitioned to the return state by ReturnItem
+      # across a different association path goes through CustomerReturn and
+      # ReturnItem instead of using Order#inventory_units. That makes any
+      # the cached association on order stale and require a *reload* before
+      # checking if they're returned.
+      inventory_units.reload.all?(&:returned?)
     end
 
     def contents

core/app/models/spree/return_item.rb

@@ -78,9 +78,10 @@ class ReturnItem < Spree::Base
     before_save :set_exchange_amount
 
     state_machine :reception_status, initial: :awaiting do
-      after_transition to: COMPLETED_RECEPTION_STATUSES,  do: :attempt_accept
+      after_transition to: COMPLETED_RECEPTION_STATUSES,  do: :attempt_accept, if: :can_attempt_accept?
       after_transition to: COMPLETED_RECEPTION_STATUSES,  do: :check_unexchange
       after_transition to: :received, do: :process_inventory_unit!
+      after_transition to: :received, do: :process_customer_return!
 
       event(:cancel) { transition to: :cancelled, from: :awaiting }
 
@@ -246,6 +247,10 @@ def process_inventory_unit!
       end
     end
 
+    def process_customer_return!
+      customer_return&.process_return!
+    end
+
     def sibling_intended_for_exchange(status)
       # This happens when we ship an exchange to a customer, but the customer keeps the original and returns the exchange
       self.class.find_by(reception_status: status, exchange_inventory_unit: inventory_unit)