From 8ba34ba37f164505ab97e385bdde8573c58bed26 Mon Sep 17 00:00:00 2001
From: Chris Todorov <chris@super.gd>
Date: Thu, 25 Mar 2021 16:25:06 -0700
Subject: [PATCH] Allow referencing of existing return items in create

This change pulls the `before_action` from the backend customer returns
controller for parsing `return_items_attributes` in order to handle
creating a new customer return which references existing return items
from a return authorization. This change works around a limitation in
Rails when trying to update the association on existing nested resource
while creating the related record.

Co-authored-by: Mike Conlin <mike@super.gd>
---
 .../spree/api/customer_returns_controller.rb  | 32 ++++++++++++++++++-
 .../api/customer_returns_controller_spec.rb   | 24 +++++++++++++-
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/api/app/controllers/spree/api/customer_returns_controller.rb b/api/app/controllers/spree/api/customer_returns_controller.rb
index 6bc4b769896..c6064e1fefd 100644
--- a/api/app/controllers/spree/api/customer_returns_controller.rb
+++ b/api/app/controllers/spree/api/customer_returns_controller.rb
@@ -4,13 +4,16 @@ module Spree
   module Api
     class CustomerReturnsController < Spree::Api::BaseController
       before_action :load_order
+      before_action :build_return_items_from_params, only: [:create]
       around_action :lock_order, only: [:create, :update, :destroy, :cancel]
 
       rescue_from Spree::Order::InsufficientStock, with: :insufficient_stock_error
 
       def create
         authorize! :create, CustomerReturn
-        @customer_return = CustomerReturn.create(customer_return_params)
+
+        invalid_resource!(@customer_return) && return if @customer_return.errors.any?
+
         if @customer_return.save
           respond_with(@customer_return, status: 201, default_template: :show)
         else
@@ -62,6 +65,33 @@ def load_order
       def customer_return_params
         params.require(:customer_return).permit(permitted_customer_return_attributes)
       end
+
+      def build_return_items_from_params
+        customer_return_attributes = customer_return_params
+        return_items_params = customer_return_attributes.
+          delete(:return_items_attributes)
+
+        @customer_return = CustomerReturn.new(customer_return_attributes)
+
+        @customer_return.return_items = return_items_params.map do |item_params|
+          return_item = if item_params[:id]
+                          Spree::ReturnItem.find(item_params[:id])
+                        else
+                          Spree::ReturnItem.new
+                        end
+
+          return_item.assign_attributes(item_params)
+
+          if item_params[:reception_status_event].blank?
+            @customer_return.errors.add(
+              :return_items,
+              "reception_status_event required for return item with inventory unit id #{return_item.inventory_unit_id}"
+            )
+          end
+
+          return_item
+        end.compact
+      end
     end
   end
 end
diff --git a/api/spec/requests/spree/api/customer_returns_controller_spec.rb b/api/spec/requests/spree/api/customer_returns_controller_spec.rb
index db645d9cda0..ec1d43773e0 100644
--- a/api/spec/requests/spree/api/customer_returns_controller_spec.rb
+++ b/api/spec/requests/spree/api/customer_returns_controller_spec.rb
@@ -160,7 +160,6 @@ module Spree
         end
 
         it "can create a new customer return" do
-          pending("fix for referrencing existing return items")
           subject
 
           expect(response).to have_http_status(:success)
@@ -170,6 +169,29 @@ module Spree
 
           expect(customer_return.return_items.first.reception_status).to eql "received"
         end
+
+        context "and reception_status_event not provided for return item" do
+          let(:customer_return_params) do
+            {
+              stock_location_id: stock_location.id,
+              return_items_attributes: [return_item.attributes]
+            }
+          end
+
+          it "does not create a new customer return" do
+            expect { subject }.to_not change { Spree::CustomerReturn.count }
+
+            expect(response).to have_http_status(:unprocessable_entity)
+            expect(json_response).to include(
+              errors: {
+                return_items: [
+                  "reception_status_event required for return item with \
+inventory unit id #{return_item.inventory_unit_id}"
+                ]
+              }
+            )
+          end
+        end
       end
     end
   end