diff --git a/api/app/controllers/spree/api/line_items_controller.rb b/api/app/controllers/spree/api/line_items_controller.rb index e8ee1f0f01b..bd6e5c51528 100644 --- a/api/app/controllers/spree/api/line_items_controller.rb +++ b/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 ) diff --git a/api/spec/requests/spree/api/line_items_spec.rb b/api/spec/requests/spree/api/line_items_spec.rb index bb092fa1171..85eab021747 100644 --- a/api/spec/requests/spree/api/line_items_spec.rb +++ b/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 } } diff --git a/core/app/models/spree/order_contents.rb b/core/app/models/spree/order_contents.rb index 5151df63030..bdaed7f9475 100644 --- a/core/app/models/spree/order_contents.rb +++ b/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) diff --git a/core/spec/models/spree/order_contents_spec.rb b/core/spec/models/spree/order_contents_spec.rb index eb34512f870..ad6c193a32b 100644 --- a/core/spec/models/spree/order_contents_spec.rb +++ b/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