Merge pull request #1 from bonobos/bonobos_cherry_picks_2

Bonobos cherry-picks 2

api/app/controllers/spree/api/checkouts_controller.rb

@@ -5,7 +5,7 @@ class CheckoutsController < Spree::Api::BaseController
       around_filter :lock_order, only: [:next, :advance, :update, :complete]
       before_filter :update_order_state, only: [:next, :advance, :update, :complete]
 
-      rescue_from Spree::LineItem::InsufficientStock, with: :insufficient_stock_for_line_items
+      rescue_from Spree::Order::InsufficientStock, with: :insufficient_stock_error
 
       include Spree::Core::ControllerHelpers::Order
       # This before_filter comes from Spree::Core::ControllerHelpers::Order
@@ -125,8 +125,8 @@ def expected_total_ok?(expected_total)
           @order.total == BigDecimal(expected_total)
         end
 
-        def insufficient_stock_for_line_items(exception)
-          render json: { errors: ["Quantity is not available for items in your order"], type: 'insufficient_stock' }, status: 422
+        def insufficient_stock_error(exception)
+          render json: { errors: [I18n.t(:quantity_is_not_available, :scope => "spree.api.order")], type: 'insufficient_stock' }, status: 422
         end
     end
   end

api/app/controllers/spree/api/shipments_controller.rb

@@ -54,7 +54,9 @@ def ready
       end
 
       def ship
+        authorize! :ship, @shipment
         unless @shipment.shipped?
+          @shipment.suppress_mailer = (params[:send_mailer] == 'false')
           @shipment.ship!
         end
         respond_with(@shipment, default_template: :show)

api/app/controllers/spree/api/stock_items_controller.rb

@@ -73,6 +73,9 @@ def count_on_hand_adjustment
       end
 
       def adjust_stock_item_count_on_hand(count_on_hand_adjustment)
+        if @stock_item.count_on_hand + count_on_hand_adjustment < 0
+          raise StockLocation::InvalidMovementError.new(Spree.t(:stock_not_below_zero))
+        end
         @stock_movement = @stock_location.move(@stock_item.variant, count_on_hand_adjustment, current_api_user)
         @stock_item = @stock_movement.stock_item
       end

api/app/controllers/spree/api/stock_transfers_controller.rb

@@ -5,13 +5,13 @@ def receive
         authorize! :update, TransferItem
         @stock_transfer = Spree::StockTransfer.accessible_by(current_ability, :update).find_by!(number: params[:id])
         variant = Spree::Variant.accessible_by(current_ability, :show).find(params[:variant_id])
-        transfer_item = @stock_transfer.transfer_items.find_by(variant: variant)
-        if transfer_item.nil?
+        @transfer_item = @stock_transfer.transfer_items.find_by(variant: variant)
+        if @transfer_item.nil?
           render "spree/api/errors/variant_not_in_stock_transfer", status: 422
-        elsif transfer_item.update_attributes(received_quantity: transfer_item.received_quantity + 1)
-          respond_with(@stock_transfer, status: 200, default_template: :show)
+        elsif @transfer_item.update_attributes(received_quantity: @transfer_item.received_quantity + 1)
+          render 'spree/api/stock_transfers/receive', status: 200
         else
-          invalid_resource!(@stock_transfer)
+          invalid_resource!(@transfer_item)
         end
       end
     end

api/app/controllers/spree/api/variants_controller.rb

@@ -62,7 +62,9 @@ def scope
             variants = variants.with_deleted
           end
 
-          variants.accessible_by(current_ability, :read)
+          variants = variants.accessible_by(current_ability, :read)
+          variants = variants.in_stock if params[:in_stock_only] || cannot?(:view_out_of_stock, Spree::Variant)
+          variants
         end
 
         def variant_params

api/app/views/spree/api/stock_transfers/receive.v1.rabl

@@ -0,0 +1,5 @@
+object @stock_transfer
+attributes *stock_transfer_attributes
+node(:received_item) do
+  partial('spree/api/transfer_items/show', object: @transfer_item)
+end

api/config/locales/en.yml

@@ -19,6 +19,7 @@ en:
         could_not_transition: "The order could not be transitioned. Please fix the errors and try again."
         invalid_shipping_method: "Invalid shipping method specified."
         expected_total_mismatch: "Expected total does not match actual total."
+        quantity_is_not_available: "Quantity is not available for items in your order"
       payment:
         credit_over_limit: "This payment can only be credited up to %{limit}. Please specify an amount less than or equal to this number."
         update_forbidden: "This payment cannot be updated because it is %{state}."

api/spec/controllers/spree/api/checkouts_controller_spec.rb

@@ -351,7 +351,7 @@ module Spree
       context 'insufficient stock' do
         let(:order) { create(:order_with_line_items) }
         before do
-          expect_any_instance_of(Spree::Order).to receive(:next!).and_raise(Spree::LineItem::InsufficientStock)
+          expect_any_instance_of(Spree::Order).to receive(:next!).and_raise(Spree::Order::InsufficientStock)
         end
 
         subject { api_put :next, :id => order.to_param, :order_token => order.guest_token }

api/spec/controllers/spree/api/inventory_units_controller_spec.rb

@@ -11,23 +11,24 @@ module Spree
 
     context "as an admin" do
       sign_in_as_admin!
+      let(:variant) { create(:variant) }
 
       it "gets an inventory unit" do
         api_get :show, :id => @inventory_unit.id
         expect(json_response['state']).to eq @inventory_unit.state
       end
 
-      it "updates an inventory unit (only shipment is accessable by default)" do
+      it "updates an inventory unit" do
         api_put :update, :id => @inventory_unit.id,
-                         :inventory_unit => { :shipment => nil }
-        expect(json_response['shipment_id']).to be_nil
+                         :inventory_unit => { variant_id: variant.id }
+        json_response['variant_id'].should eq variant.id
       end
 
       context 'fires state event' do
         it 'if supplied with :fire param' do
           api_put :update, :id => @inventory_unit.id,
                            :fire => 'ship',
-                           :inventory_unit => { :shipment => nil }
+                           :inventory_unit => { variant_id: variant.id }
 
           expect(json_response['state']).to eq 'shipped'
         end

api/spec/controllers/spree/api/shipments_controller_spec.rb

@@ -119,26 +119,6 @@
       end
     end
 
-    context "can transition a shipment from ready to ship" do
-      before do
-        allow_any_instance_of(Spree::Order).to receive_messages(:paid? => true, :complete? => true)
-        # For the shipment notification email
-        Spree::Config[:mails_from] = "spree@example.com"
-
-        shipment.update!(shipment.order)
-        expect(shipment.state).to eq("ready")
-        allow_any_instance_of(Spree::ShippingRate).to receive_messages(:cost => 5)
-      end
-
-      it "can transition a shipment from ready to ship" do
-        shipment.reload
-        api_put :ship, id: shipment.to_param, shipment: { tracking: "123123", order_id: shipment.order.to_param }
-        expect(json_response).to have_attributes(attributes)
-        expect(json_response["state"]).to eq("shipped")
-      end
-
-    end
-
     describe '#mine' do
       subject do
         api_get :mine, format: 'json', params: params
@@ -187,4 +167,86 @@
     end
 
   end
+
+  describe "#ship" do
+    let(:shipment) { create(:order_ready_to_ship).shipments.first }
+
+    let(:send_mailer) { nil }
+    subject { api_put :ship, id: shipment.to_param, send_mailer: send_mailer }
+
+    context "the user is allowed to ship the shipment" do
+      sign_in_as_admin!
+      it "ships the shipment" do
+        Timecop.freeze do
+          subject
+          shipment.reload
+          expect(shipment.state).to eq 'shipped'
+          expect(shipment.shipped_at.to_i).to eq Time.now.to_i
+        end
+      end
+
+      context "send_mailer not present" do
+        it "sends the shipped shipments mailer" do
+          with_test_mail { subject }
+          expect(ActionMailer::Base.deliveries.size).to eq 1
+          expect(ActionMailer::Base.deliveries.last.subject).to match /Shipment Notification/
+        end
+      end
+
+      context "send_mailer set to false" do
+        let(:send_mailer) { 'false' }
+        it "does not send the shipped shipments mailer" do
+          with_test_mail { subject }
+          expect(ActionMailer::Base.deliveries.size).to eq 0
+        end
+      end
+
+      context "send_mailer set to true" do
+        let(:send_mailer) { 'true' }
+        it "sends the shipped shipments mailer" do
+          with_test_mail { subject }
+          expect(ActionMailer::Base.deliveries.size).to eq 1
+          expect(ActionMailer::Base.deliveries.last.subject).to match /Shipment Notification/
+        end
+      end
+    end
+
+    context "the user is not allowed to ship the shipment" do
+      sign_in_as_admin!
+
+      before do
+        ability = Spree::Ability.new(current_api_user)
+        ability.cannot :ship, Spree::Shipment
+        allow(controller).to receive(:current_ability) { ability }
+      end
+
+      it "does nothing" do
+        expect {
+          expect {
+            subject
+          }.not_to change(shipment, :state)
+        }.not_to change(shipment, :shipped_at)
+      end
+
+      it "responds with a 401" do
+        subject
+        expect(response.status).to eq 401
+      end
+    end
+
+    context "the user is not allowed to view the shipment" do
+      it "does nothing" do
+        expect {
+          expect {
+            subject
+          }.not_to change(shipment, :state)
+        }.not_to change(shipment, :shipped_at)
+      end
+
+      it "responds with a 404" do
+        subject
+        expect(response).to be_not_found
+      end
+    end
+  end
 end

api/spec/controllers/spree/api/stock_items_controller_spec.rb

@@ -116,12 +116,13 @@ module Spree
           StockItem.delete_all
           variant
         end
+        let(:count_on_hand) { '20' }
         let(:params) do
           {
             stock_location_id: stock_location.to_param,
             stock_item: {
               variant_id: variant.id,
-              count_on_hand: '20'
+              count_on_hand: count_on_hand
             }
           }
         end
@@ -160,6 +161,17 @@ module Spree
             expect(assigns(:stock_item).count_on_hand).to eq 0
           end
         end
+
+        context "attempting to set negative inventory" do
+          let(:count_on_hand) { '-1' }
+
+          it "does not allow negative inventory for the stock item" do
+            subject
+            expect(response.status).to eq 422
+            expect(response.body).to match Spree.t(:stock_not_below_zero)
+            expect(assigns(:stock_item).count_on_hand).to eq 0
+          end
+        end
       end
 
       context 'updating a stock item' do
@@ -170,11 +182,12 @@ module Spree
         subject { api_put :update, params }
 
         context 'adjusting count_on_hand' do
+          let(:count_on_hand) { 40 }
           let(:params) do
             {
               id: stock_item.to_param,
               stock_item: {
-                count_on_hand: 40,
+                count_on_hand: count_on_hand,
                 backorderable: true
               }
             }
@@ -213,14 +226,26 @@ module Spree
               expect(assigns(:stock_item).count_on_hand).to eq 10
             end
           end
+
+          context "attempting to set negative inventory" do
+            let(:count_on_hand) { '-11' }
+
+            it "does not allow negative inventory for the stock item" do
+              subject
+              expect(response.status).to eq 422
+              expect(response.body).to match Spree.t(:stock_not_below_zero)
+              expect(assigns(:stock_item).count_on_hand).to eq 10
+            end
+          end
         end
 
         context 'setting count_on_hand' do
+          let(:count_on_hand) { 40 }
           let(:params) do
             {
               id: stock_item.to_param,
               stock_item: {
-                count_on_hand: 40,
+                count_on_hand: count_on_hand,
                 force: true,
               }
             }
@@ -258,6 +283,17 @@ module Spree
               expect(assigns(:stock_item).count_on_hand).to eq 10
             end
           end
+
+          context "attempting to set negative inventory" do
+            let(:count_on_hand) { '-1' }
+
+            it "does not allow negative inventory for the stock item" do
+              subject
+              expect(response.status).to eq 422
+              expect(response.body).to match Spree.t(:stock_not_below_zero)
+              expect(assigns(:stock_item).count_on_hand).to eq 10
+            end
+          end
         end
       end
 

api/spec/controllers/spree/api/stock_transfers_controller_spec.rb

@@ -39,6 +39,29 @@ module Spree
           it "increments the received quantity for the transfer_item" do
             expect { subject }.to change { transfer_item.reload.received_quantity }.by(1)
           end
+
+          it "returns the received transfer item in the response" do
+            subject
+            expect(JSON.parse(response.body)["received_item"]["id"]).to eq transfer_item.id
+          end
+        end
+
+        context "transfer item has been fully received" do
+          let(:variant_id) { transfer_item.variant.to_param }
+
+          before do
+            transfer_item.update_attributes!(expected_quantity: 1, received_quantity: 1)
+          end
+
+          it "returns a 422" do
+            subject
+            expect(response.status).to eq 422
+          end
+
+          it "returns a specific error message" do
+            subject
+            expect(JSON.parse(response.body)["errors"]["received_quantity"]).to eq ["must be less than or equal to 1"]
+          end
         end
 
         context "variant is not in the transfer order" do

api/spec/controllers/spree/api/variants_controller_spec.rb

@@ -89,6 +89,32 @@ module Spree
         end
       end
 
+      context "stock filtering" do
+        subject { api_get :index, in_stock_only: true }
+
+        context "variant is out of stock" do
+          before do
+            variant.stock_items.update_all(count_on_hand: 0)
+          end
+
+          it "is not returned in the results" do
+            subject
+            expect(json_response["variants"].count).to eq 0
+          end
+        end
+
+        context "variant is in stock" do
+          before do
+            variant.stock_items.update_all(count_on_hand: 10)
+          end
+
+          it "is returned in the results" do
+            subject
+            expect(json_response["variants"].count).to eq 1
+          end
+        end
+      end
+
       context "pagination" do
         it "can select the next page of variants" do
           second_variant = create(:variant)