Merge pull request #4007 from SuperGoodSoft/supergoodsoft/create-cust…

…omer-return-with-existing-items

Allow customer returns to reference existing `ReturnItem`s on create through API

api/app/controllers/spree/api/customer_returns_controller.rb

@@ -4,13 +4,14 @@ module Spree
   module Api
     class CustomerReturnsController < Spree::Api::BaseController
       before_action :load_order
+      before_action :build_customer_return, 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)
+
         if @customer_return.save
           respond_with(@customer_return, status: 201, default_template: :show)
         else
@@ -62,6 +63,34 @@ def load_order
       def customer_return_params
         params.require(:customer_return).permit(permitted_customer_return_attributes)
       end
+
+      def build_customer_return
+        customer_return_attributes = customer_return_params
+        return_items_params = customer_return_attributes.
+          delete(:return_items_attributes)
+
+        if return_items_params.is_a? ActionController::Parameters
+          return_items_params = return_items_params.values
+          Spree::Deprecation.warn(
+            "Passing `return_items_attributes` as a hash of hashes is \
+deprecated and will be removed in future versions of Solidus."
+          )
+        end
+
+        @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)
+
+          return_item
+        end
+      end
     end
   end
 end

api/spec/requests/spree/api/customer_returns_controller_spec.rb

@@ -112,23 +112,111 @@ module Spree
         expect(json_response["stock_location_id"]).to eq final_stock_location.id
       end
 
-      it "can create a new customer return" do
-        stock_location = FactoryBot.create(:stock_location)
-        unit = FactoryBot.create(:inventory_unit, state: "shipped")
-        cr_params = { stock_location_id: stock_location.id,
-                       return_items_attributes: [{
-                         inventory_unit_id: unit.id,
-                         reception_status_event: "receive",
-                       }] }
-
-        post spree.api_order_customer_returns_path(order), params: { order_id: order.number, customer_return: cr_params }
-
-        expect(response.status).to eq(201)
-        expect(json_response).to have_attributes(attributes)
+      context "when creating new return items" do
+        it "can create a new customer return" do
+          stock_location = FactoryBot.create(:stock_location)
+          unit = FactoryBot.create(:inventory_unit, state: "shipped")
+          cr_params = { stock_location_id: stock_location.id,
+                         return_items_attributes: [{
+                           inventory_unit_id: unit.id,
+                           reception_status_event: "receive",
+                         }] }
+
+          post spree.api_order_customer_returns_path(order), params: { order_id: order.number, customer_return: cr_params }
+
+          expect(response.status).to eq(201)
+          expect(json_response).to have_attributes(attributes)
 
-        customer_return = Spree::CustomerReturn.last
+          customer_return = Spree::CustomerReturn.last
+
+          expect(customer_return.return_items.first.reception_status).to eql "received"
+        end
+      end
 
-        expect(customer_return.return_items.first.reception_status).to eql "received"
+      context "when referencing existing return items" do
+        subject do
+          post(
+            spree.api_order_customer_returns_path(order),
+            params: {
+              order_id: order.number,
+              customer_return: customer_return_params
+            }
+          )
+        end
+
+        let(:stock_location) { create(:stock_location) }
+        let(:inventory_unit) { create(:inventory_unit, state: "shipped") }
+        let(:order) { inventory_unit.order }
+        let(:return_item) do
+          create(:return_item, inventory_unit: inventory_unit)
+        end
+
+        let(:customer_return_params) do
+          {
+            stock_location_id: stock_location.id,
+            return_items_attributes: [return_item.attributes]
+          }
+        end
+
+        it "can create a new customer return" do
+          expect { subject }.to change { Spree::CustomerReturn.count }.
+            from(0).to(1)
+
+          expect(response).to have_http_status(:success)
+          expect(json_response).to have_attributes(attributes)
+        end
+
+        it "does not change the reception status of the return item" do
+          expect { subject }.
+            to_not change { return_item.reload.reception_status }.
+            from("awaiting")
+        end
+
+        context "and return items attributes passed in as a hash of hashes" do
+          let(:customer_return_params) do
+            {
+              stock_location_id: stock_location.id,
+              return_items_attributes: {
+                "0" => return_item.attributes
+              }
+            }
+          end
+
+          it "can create a new customer return" do
+            expect(Spree::Deprecation).to receive(:warn)
+            expect { subject }.to change { Spree::CustomerReturn.count }.
+              from(0).to(1)
+
+            expect(response).to have_http_status(:success)
+            expect(json_response).to have_attributes(attributes)
+          end
+
+          it "logs a deprecation warning" do
+            expect(Spree::Deprecation).
+              to receive(:warn).
+              with(
+                /Passing `return_items_attributes` as a hash of hashes is deprecated/
+              )
+            subject
+          end
+        end
+
+        context "with reception_status_event provided for return item" do
+          let(:customer_return_params) do
+            {
+              stock_location_id: stock_location.id,
+              return_items_attributes: [
+                return_item.attributes.merge(reception_status_event: "receive")
+              ]
+            }
+          end
+
+          it "updates the reception status of the return item" do
+            expect { subject }.
+              to change { return_item.reload.reception_status }.
+              from("awaiting").to("received")
+          end
+        end
       end
     end
   end