Stop setting line items with quantity of zero

api/app/controllers/spree/api/line_items_controller.rb

@@ -13,7 +13,7 @@ def create
         variant = Spree::Variant.find(params[:line_item][:variant_id])
         @line_item = @order.contents.add(
           variant,
-          params[:line_item][:quantity] || 1,
+          params[:line_item][:quantity].presence || 1,
           options: line_item_params[:options].to_h
         )
 

api/spec/requests/spree/api/line_items_spec.rb

@@ -95,13 +95,27 @@ module Spree::Api
         expect(response.status).to eq(201)
       end
 
-      it "default quantity to 1 if none is given" do
+      it "sets default quantity to 1 if none is given" do
         post spree.api_order_line_items_path(order), params: { line_item: { variant_id: product.master.to_param } }
         expect(response.status).to eq(201)
         expect(json_response).to have_attributes(attributes)
         expect(json_response[:quantity]).to eq 1
       end
 
+      it "sets default quantity to 1 if empty values are given" do
+        post spree.api_order_line_items_path(order), params: { line_item: { variant_id: product.master.to_param, quantity: '' } }
+        expect(response.status).to eq(201)
+        expect(json_response).to have_attributes(attributes)
+        expect(json_response[:quantity]).to eq 1
+      end
+
+      it "allows to explicitly set quantity to 0" do
+        post spree.api_order_line_items_path(order), params: { line_item: { variant_id: product.master.to_param, quantity: 0 } }
+        expect(response.status).to eq(201)
+        expect(json_response).to have_attributes(attributes)
+        expect(json_response[:quantity]).to eq 0
+      end
+
       it "increases a line item's quantity if it exists already" do
         order.line_items.create(variant_id: product.master.id, quantity: 10)
         post spree.api_order_line_items_path(order), params: { line_item: { variant_id: product.master.to_param, quantity: 1 } }

core/app/models/spree/order_contents.rb

@@ -35,9 +35,10 @@ def remove_line_item(line_item, options = {})
     end
 
     def update_cart(params)
+      params = destroy_non_positive_line_items(params)
+
       if order.update(params)
         unless order.completed?
-          order.line_items = order.line_items.select { |li| li.quantity > 0 }
           # Update totals, then check if the order is eligible for any cart promotions.
           # If we do not update first, then the item total will be wrong and ItemTotal
           # promotion rules would not be triggered.
@@ -83,6 +84,20 @@ def reload_totals
       @order.recalculate
     end
 
+    def destroy_non_positive_line_items(params)
+      line_items_to_destroy_ids = []
+      # Could be a single LineItem or multiple via nesting
+      if id = params[:line_items_attributes][:id].present?
+        line_items_to_destroy_ids << id if params[:line_items_attributes][:quantity].to_i <= 0
+      else
+        params[:line_items_attributes].reject! do |_index, attributes|
+          line_items_to_destroy_ids << attributes[:id] if attributes[:quantity].to_i <= 0
+        end
+      end
+      order.line_items.where(id: line_items_to_destroy_ids).destroy_all
+      params
+    end
+
     def add_to_line_item(variant, quantity, options = {})
       line_item = grab_line_item_by_variant(variant, false, options)
 

core/spec/models/spree/order_contents_spec.rb

@@ -263,40 +263,66 @@
   context "update cart" do
     let!(:shirt) { subject.add variant, 1 }
 
-    let(:params) do
-      { line_items_attributes: {
-        "0" => { id: shirt.id, quantity: 3 }
-      } }
-    end
+    context "with a multi-level nested params hash" do
+      let(:params) do
+        { line_items_attributes: {
+          "0" => { id: shirt.id, quantity: 3 }
+        } }
+      end
 
-    it "changes item quantity" do
-      subject.update_cart params
-      expect(shirt.reload.quantity).to eq 3
-    end
+      it "changes item quantity" do
+        subject.update_cart params
+        expect(shirt.reload.quantity).to eq 3
+      end
 
-    it "updates order totals" do
-      expect {
+      it "updates order totals" do
+        expect {
+          subject.update_cart params
+        }.to change { subject.order.total }
+      end
+
+      context "submits item quantity 0" do
+        let(:params) do
+          { line_items_attributes: {
+            "0" => { id: shirt.id, quantity: 0 }
+          } }
+        end
+
+        it "removes item from order" do
+          expect {
+            subject.update_cart params
+          }.to change { subject.order.line_items.count }.from(1).to(0)
+        end
+      end
+
+      it "ensures updated shipments" do
+        expect(subject.order).to receive(:check_shipments_and_restart_checkout)
         subject.update_cart params
-      }.to change { subject.order.total }
+      end
     end
 
-    context "submits item quantity 0" do
+    context "with a single-level nested params hash" do
       let(:params) do
         { line_items_attributes: {
-          "0" => { id: shirt.id, quantity: 0 }
+          id: shirt.id, quantity: new_quantity
         } }
       end
+      let(:new_quantity) { 3 }
 
-      it "removes item from order" do
-        expect {
-          subject.update_cart params
-        }.to change { subject.order.line_items.count }
+      it "changes item quantity" do
+        subject.update_cart params
+        expect(shirt.reload.quantity).to eq 3
       end
-    end
 
-    it "ensures updated shipments" do
-      expect(subject.order).to receive(:check_shipments_and_restart_checkout)
-      subject.update_cart params
+      context "submits item quantity 0" do
+        let(:new_quantity) { 0 }
+
+        it "removes item from order" do
+          expect {
+            subject.update_cart params
+          }.to change { subject.order.line_items.count }.from(1).to(0)
+        end
+      end
     end
   end