diff --git a/app/assets/javascripts/darkswarm/controllers/shop_variant_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/shop_variant_controller.js.coffee index 825bc9f39f..e71a7a0a29 100644 --- a/app/assets/javascripts/darkswarm/controllers/shop_variant_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/shop_variant_controller.js.coffee @@ -1,9 +1,5 @@ Darkswarm.controller "ShopVariantCtrl", ($scope, $modal, Cart) -> - $scope.$watchGroup [ - 'variant.line_item.quantity', - 'variant.line_item.max_quantity' - ], (new_value, old_value) -> - return if old_value[0] == null && new_value[0] == null + $scope.updateCart = (line_item) -> Cart.adjust($scope.variant.line_item) $scope.variant.line_item.quantity ||= 0 @@ -44,10 +40,12 @@ Darkswarm.controller "ShopVariantCtrl", ($scope, $modal, Cart) -> $scope.add = (quantity) -> item = $scope.variant.line_item item.quantity = $scope.sanitizedQuantity() + quantity + $scope.updateCart(item) $scope.addMax = (quantity) -> item = $scope.variant.line_item item.max_quantity = $scope.sanitizedMaxQuantity() + quantity + $scope.updateCart(item) $scope.canAdd = (quantity) -> wantedQuantity = $scope.sanitizedQuantity() + quantity diff --git a/app/controllers/admin/bulk_line_items_controller.rb b/app/controllers/admin/bulk_line_items_controller.rb index b9e95c6e49..91973448e9 100644 --- a/app/controllers/admin/bulk_line_items_controller.rb +++ b/app/controllers/admin/bulk_line_items_controller.rb @@ -32,10 +32,7 @@ module Admin # See https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/locking/pessimistic.rb#L69 # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE order.with_lock do - if @line_item.update(line_item_params) - order.update_line_item_fees! @line_item - order.update_order_fees! - order.update_order! + if order.contents.update_item(@line_item, line_item_params) render body: nil, status: :no_content # No Content, does not trigger ng resource auto-update else render json: { errors: @line_item.errors }, status: :precondition_failed @@ -49,7 +46,7 @@ module Admin load_line_item authorize! :update, order - @line_item.destroy + order.contents.remove(@line_item.variant) render body: nil, status: :no_content # No Content, does not trigger ng resource auto-update end @@ -63,16 +60,9 @@ module Admin Spree::LineItem end - # Returns the appropriate serializer for this controller - # - # @return [Api::Admin::LineItemSerializer] - def serializer(_ams_prefix) - Api::Admin::LineItemSerializer - end - def serialized_line_items ActiveModel::ArraySerializer.new( - @line_items, each_serializer: serializer(nil) + @line_items, each_serializer: Api::Admin::LineItemSerializer ) end diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb index c4ed320e17..cd0f31dcd3 100644 --- a/app/controllers/api/v0/shipments_controller.rb +++ b/app/controllers/api/v0/shipments_controller.rb @@ -81,7 +81,6 @@ module Api quantity = params[:quantity].to_i @order.contents.remove(variant, quantity, @shipment) - @order.recreate_all_fees! @shipment.reload if @shipment.persisted? render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb index f3a0de59ea..8ea8d31ab0 100644 --- a/app/controllers/cart_controller.rb +++ b/app/controllers/cart_controller.rb @@ -3,33 +3,28 @@ class CartController < BaseController def populate order = current_order(true) - cart_service = CartService.new(order) - cart_service.populate(params.slice(:variants, :quantity), true) - if cart_service.valid? + if cart_service.populate(params.slice(:variants, :quantity)) order.cap_quantity_at_stock! order.recreate_all_fees! - variant_ids = variant_ids_in(cart_service.variants_h) - - render json: { error: false, - stock_levels: VariantsStockLevels.new.call(order, variant_ids) }, - status: :ok + render json: { error: false, stock_levels: stock_levels(order) }, status: :ok else render json: { error: cart_service.errors.full_messages.join(",") }, status: :precondition_failed end - - populate_variant_attributes - end - - def variant_ids_in(variants_h) - variants_h.map { |v| v[:variant_id].to_i } end private + def stock_levels(order) + variants_in_cart = order.line_items.pluck(:variant_id) + variants_in_request = raw_params[:variants]&.map(&:first) || [] + + VariantsStockLevels.new.call(order, (variants_in_cart + variants_in_request).uniq) + end + def check_authorization session[:access_token] ||= params[:token] order = Spree::Order.find_by(number: params[:id]) || current_order @@ -40,26 +35,4 @@ class CartController < BaseController authorize! :create, Spree::Order end end - - def populate_variant_attributes - order = current_order.reload - - populate_variant_attributes_from_variant(order) if params.key? :variant_attributes - populate_variant_attributes_from_product(order) if params.key? :quantity - end - - def populate_variant_attributes_from_variant(order) - params[:variant_attributes].each do |variant_id, attributes| - permitted = attributes.permit(:quantity, :max_quantity).to_h.with_indifferent_access - order.set_variant_attributes(Spree::Variant.find(variant_id), permitted) - end - end - - def populate_variant_attributes_from_product(order) - params[:products].each do |_product_id, variant_id| - max_quantity = params[:max_quantity].to_i - order.set_variant_attributes(Spree::Variant.find(variant_id), - max_quantity: max_quantity) - end - end end diff --git a/app/controllers/line_items_controller.rb b/app/controllers/line_items_controller.rb index 4459249ba9..b61d476d2e 100644 --- a/app/controllers/line_items_controller.rb +++ b/app/controllers/line_items_controller.rb @@ -39,12 +39,8 @@ class LineItemsController < BaseController def destroy_with_lock(item) order = item.order order.with_lock do - item.destroy - order.update_shipping_fees! + order.contents.remove(item.variant) order.update_payment_fees! - order.update_order_fees! - order.update_order! - order.create_tax_charge! end end end diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index b519f3043f..015ad64936 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -38,17 +38,6 @@ module Spree redirect_to main_app.cart_path end - def check_authorization - session[:access_token] ||= params[:token] - order = Spree::Order.find_by(number: params[:id]) || current_order - - if order - authorize! :edit, order, session[:access_token] - else - authorize! :create, Spree::Order - end - end - # Patching to redirect to shop if order is empty def edit @order = current_order(true) @@ -77,14 +66,10 @@ module Spree # This action is called either from the cart page when the order is not yet complete, or from # the edit order page (frontoffice) if the hub allows users to update completed orders. - # This line item updating logic should probably be handled through CartService#populate. - if @order.update(order_params) - discard_empty_line_items - + if @order.contents.update_cart(order_params) @order.recreate_all_fees! # Enterprise fees on line items and on the order itself if @order.complete? - @order.update_shipping_fees! @order.update_payment_fees! @order.create_tax_charge! end @@ -109,23 +94,6 @@ module Spree end end - def set_current_order - @order = current_order(true) - end - - def filter_order_params - if params[:order] && params[:order][:line_items_attributes] - params[:order][:line_items_attributes] = - remove_missing_line_items(params[:order][:line_items_attributes]) - end - end - - def remove_missing_line_items(attrs) - attrs.select do |_i, line_item| - Spree::LineItem.find_by(id: line_item[:id]) - end - end - def cancel @order = Spree::Order.find_by!(number: params[:id]) authorize! :cancel, @order @@ -140,6 +108,21 @@ module Spree private + def set_current_order + @order = current_order(true) + end + + def check_authorization + session[:access_token] ||= params[:token] + order = Spree::Order.find_by(number: params[:id]) || current_order + + if order + authorize! :edit, order, session[:access_token] + else + authorize! :create, Spree::Order + end + end + # Stripe can redirect here after a payment is processed in the backoffice. # We verify if it was successful here and persist the changes. def handle_stripe_response @@ -153,6 +136,19 @@ module Spree @order.reload end + def filter_order_params + if params[:order] && params[:order][:line_items_attributes] + params[:order][:line_items_attributes] = + remove_missing_line_items(params[:order][:line_items_attributes]) + end + end + + def remove_missing_line_items(attrs) + attrs.select do |_i, line_item| + Spree::LineItem.find_by(id: line_item[:id]) + end + end + def discard_empty_line_items @order.line_items = @order.line_items.select { |li| li.quantity > 0 } end diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index 25636291ed..38e92f0ccd 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -230,7 +230,6 @@ module Spree # update the order totals, etc. order.create_tax_charge! - order.update_order! end def update_inventory_before_destroy diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 0a3042c418..c9bbec9989 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -35,7 +35,7 @@ module Spree alias_attribute :shipping_address, :ship_address has_many :state_changes, as: :stateful - has_many :line_items, -> { order('created_at ASC') }, dependent: :destroy + has_many :line_items, -> { order('created_at ASC') }, class_name: "Spree::LineItem", dependent: :destroy has_many :payments, dependent: :destroy has_many :return_authorizations, dependent: :destroy, inverse_of: :order has_many :adjustments, -> { order "#{Spree::Adjustment.table_name}.created_at ASC" }, @@ -243,63 +243,11 @@ module Spree return_authorizations.any?(&:authorized?) end - # This is currently used when adding a variant to an order in the BackOffice. - # Spree::OrderContents#add is equivalent but slightly different from add_variant below. + # OrderContents should always be used when modifying an order's line items def contents @contents ||= Spree::OrderContents.new(self) end - # This is currently used when adding a variant to an order in the FrontOffice. - # This add_variant is equivalent but slightly different from Spree::OrderContents#add above. - # Spree::OrderContents#add is the more modern version in Spree history - # but this add_variant has been customized for OFN FrontOffice. - def add_variant(variant, quantity = 1, max_quantity = nil, currency = nil) - line_items.reload - current_item = find_line_item_by_variant(variant) - - # Notify bugsnag if we get line items with a quantity of zero - if quantity == 0 - Bugsnag.notify(RuntimeError.new("Zero Quantity Line Item"), - current_item: current_item.as_json, - line_items: line_items.map(&:id), - variant: variant.as_json) - end - - if current_item - current_item.quantity = quantity - current_item.max_quantity = max_quantity - - current_item.currency = currency unless currency.nil? - current_item.save - else - current_item = Spree::LineItem.new(quantity: quantity, max_quantity: max_quantity) - current_item.variant = variant - if currency - current_item.currency = currency unless currency.nil? - current_item.price = variant.price_in(currency).amount - else - current_item.price = variant.price - end - line_items << current_item - end - - reload - current_item - end - - def set_variant_attributes(variant, attributes) - line_item = find_line_item_by_variant(variant) - - return unless line_item - - if attributes.key?(:max_quantity) && attributes[:max_quantity].to_i < line_item.quantity - attributes[:max_quantity] = line_item.quantity - end - - line_item.assign_attributes(attributes) - line_item.save! - end - # Associates the specified user with the order. def associate_user!(user) self.user = user @@ -517,6 +465,16 @@ module Spree shipments end + # Clear shipments and move order back to address state unless compete. This is relevant where + # an order is part-way through checkout and the user changes items in the cart; in that case + # we need to reset the checkout flow to ensure the order is processed correctly. + def ensure_updated_shipments + if !self.completed? && shipments.any? + shipments.destroy_all + restart_checkout_flow + end + end + def refresh_shipment_rates shipments.map(&:refresh_rates) end @@ -580,12 +538,6 @@ module Spree save! end - def remove_variant(variant) - line_items.reload - current_item = find_line_item_by_variant(variant) - current_item.andand.destroy - end - def cap_quantity_at_stock! line_items.includes(variant: :stock_items).find_each(&:cap_quantity_at_stock!) end @@ -749,7 +701,7 @@ module Spree return if adjustment.finalized? adjustment.update_adjustment!(force: true) - update_order! + updater.update_totals_and_states end # object_params sets the payment amount to the order total, but it does this diff --git a/app/models/spree/order/checkout.rb b/app/models/spree/order/checkout.rb index 4d34f091ca..9d50612390 100644 --- a/app/models/spree/order/checkout.rb +++ b/app/models/spree/order/checkout.rb @@ -130,6 +130,13 @@ module Spree steps << "complete" unless steps.include?("complete") steps end + + def restart_checkout_flow + update_columns( + state: checkout_steps.first, + updated_at: Time.zone.now, + ) + end end end end diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb index fb745ef891..7914256e7a 100644 --- a/app/models/spree/order_contents.rb +++ b/app/models/spree/order_contents.rb @@ -11,25 +11,79 @@ module Spree # Get current line item for variant if exists # Add variant qty to line_item def add(variant, quantity = 1, shipment = nil) - line_item = order.find_line_item_by_variant(variant) - add_to_line_item(line_item, variant, quantity, shipment) + line_item = add_to_line_item(variant, quantity, shipment) + update_shipment(shipment) + update_order + line_item end # Get current line item for variant # Remove variant qty from line_item - def remove(variant, quantity = 1, shipment = nil) - line_item = order.find_line_item_by_variant(variant) + def remove(variant, quantity = nil, shipment = nil) + line_item = remove_from_line_item(variant, quantity, shipment) + update_shipment(shipment) + order.update_order_fees! if order.completed? + update_order + line_item + end - unless line_item - raise ActiveRecord::RecordNotFound, "Line item not found for variant #{variant.sku}" + def update_or_create(variant, attributes) + line_item = find_line_item_by_variant(variant) + + if line_item + line_item.update(attributes) + else + line_item = Spree::LineItem.new(attributes) + line_item.variant = variant + line_item.price = variant.price + order.line_items << line_item end - remove_from_line_item(line_item, variant, quantity, shipment) + order.reload + line_item + end + + def update_cart(params) + if order.update_attributes(params) + discard_empty_line_items + update_shipment + update_order + true + else + false + end + end + + def update_item(line_item, params) + if line_item.update_attributes(params) + discard_empty_line_items + order.update_line_item_fees! line_item + order.update_order_fees! if order.completed? + update_shipment + update_order + true + else + false + end end private - def add_to_line_item(line_item, variant, quantity, shipment = nil) + def discard_empty_line_items + order.line_items = order.line_items.select {|li| li.quantity.positive? } + end + + def update_shipment(target_shipment = nil) + if order.completed? || target_shipment.present? + order.update_shipping_fees! + else + order.ensure_updated_shipments + end + end + + def add_to_line_item(variant, quantity, shipment = nil) + line_item = find_line_item_by_variant(variant) + if line_item line_item.target_shipment = shipment line_item.quantity += quantity.to_i @@ -40,23 +94,37 @@ module Spree end line_item.save - order.reload line_item end - def remove_from_line_item(line_item, _variant, quantity, shipment = nil) - line_item.quantity += -quantity + def remove_from_line_item(variant, quantity, shipment = nil) + line_item = find_line_item_by_variant(variant, true) + + quantity.present? ? line_item.quantity += -quantity : line_item.quantity = 0 line_item.target_shipment = shipment if line_item.quantity == 0 - Spree::OrderInventory.new(order).verify(line_item, shipment) line_item.destroy else line_item.save! end - order.reload line_item end + + def find_line_item_by_variant(variant, raise_error = false) + line_item = order.find_line_item_by_variant(variant) + + if !line_item.present? && raise_error + raise ActiveRecord::RecordNotFound, "Line item not found for variant #{variant.sku}" + end + + line_item + end + + def update_order + order.update_order! + order.reload + end end end diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb index dd4cb87fe6..447363cc7f 100644 --- a/app/models/spree/shipment.rb +++ b/app/models/spree/shipment.rb @@ -160,10 +160,6 @@ module Spree Spree::Money.new(item_cost, currency: currency) end - def editable_by?(_user) - !shipped? - end - def update_amounts return unless fee_adjustment&.amount != cost @@ -187,14 +183,6 @@ module Spree @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor) end - def line_items - if order.complete? - order.line_items.select { |li| inventory_units.pluck(:variant_id).include?(li.variant_id) } - else - order.line_items - end - end - def finalize! InventoryUnit.finalize_units!(inventory_units) manifest.each { |item| manifest_unstock(item) } @@ -295,6 +283,15 @@ module Spree private + def line_items + if order.complete? + inventory_unit_ids = inventory_units.pluck(:variant_id) + order.line_items.select { |li| inventory_unit_ids.include?(li.variant_id) } + else + order.line_items + end + end + def manifest_unstock(item) stock_location.unstock item.variant, item.quantity, self end diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb index c4f3b5a5c0..5587438996 100644 --- a/app/services/cart_service.rb +++ b/app/services/cart_service.rb @@ -3,23 +3,22 @@ require 'open_food_network/scope_variant_to_hub' # Previously Spree::OrderPopulator. Modified to work with max_quantity and variant overrides. class CartService - attr_accessor :order, :currency - attr_reader :variants_h + attr_accessor :order attr_reader :errors def initialize(order) @order = order - @currency = order.currency @errors = ActiveModel::Errors.new(self) end - def populate(from_hash, overwrite = false) + def populate(from_hash) @distributor, @order_cycle = distributor_and_order_cycle + variants_data = read_variants_hash(from_hash) + @order.with_lock do - variants_data = read_variants from_hash attempt_cart_add_variants variants_data - overwrite_variants variants_data if overwrite + overwrite_variants variants_data end valid? end @@ -34,10 +33,10 @@ class CartService loaded_variants = indexed_variants(variants_data) variants_data.each do |variant_data| - loaded_variant = loaded_variants[variant_data[:variant_id].to_i] + loaded_variant = loaded_variants[variant_data[:variant_id]] - if loaded_variant.deleted? - remove_deleted_variant(loaded_variant) + if loaded_variant.deleted? || !variant_data[:quantity].positive? + cart_remove(loaded_variant) next end @@ -57,15 +56,7 @@ class CartService end end - def remove_deleted_variant(variant) - line_item_for_variant(variant).andand.destroy - end - def attempt_cart_add(variant, quantity, max_quantity = nil) - quantity = quantity.to_i - max_quantity = max_quantity.to_i if max_quantity - return unless quantity > 0 - scoper.scope(variant) return unless valid_variant?(variant) @@ -73,27 +64,37 @@ class CartService end def cart_add(variant, quantity, max_quantity) - quantity_to_add, max_quantity_to_add = quantities_to_add(variant, quantity, max_quantity) - if quantity_to_add > 0 - @order.add_variant(variant, quantity_to_add, max_quantity_to_add, currency) + attributes = final_quantities(variant, quantity, max_quantity) + + if attributes[:quantity].positive? + @order.contents.update_or_create(variant, attributes) else - @order.remove_variant variant + cart_remove(variant) end end - def quantities_to_add(variant, quantity, max_quantity) + def cart_remove(variant) + begin + order.contents.remove(variant) + rescue ActiveRecord::RecordNotFound + # Nothing to remove; no line items for this variant were found. + end + end + + def final_quantities(variant, quantity, max_quantity) # If not enough stock is available, add as much as we can to the cart on_hand = variant.on_hand on_hand = [quantity, max_quantity].compact.max if variant.on_demand - quantity_to_add = [quantity, on_hand].min - max_quantity_to_add = max_quantity # max_quantity is not capped + final_quantity = [quantity, on_hand].min + final_max_quantity = max_quantity # max_quantity is not capped - [quantity_to_add, max_quantity_to_add] + { quantity: final_quantity, max_quantity: final_max_quantity } end def overwrite_variants(variants) - variants_removed(variants).each do |id| - cart_remove(id) + variants_removed(variants).each do |variant_id| + variant = Spree::Variant.with_deleted.find(variant_id) + cart_remove(variant) end end @@ -101,27 +102,25 @@ class CartService @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(@distributor) end - def read_variants(data) - @variants_h = read_variants_hash(data) - end - def read_variants_hash(data) variants_array = [] (data[:variants] || []).each do |variant_id, quantity| if quantity.is_a?(ActionController::Parameters) - variants_array.push({ variant_id: variant_id, quantity: quantity[:quantity], max_quantity: quantity[:max_quantity] }) + variants_array.push({ + variant_id: variant_id.to_i, + quantity: quantity[:quantity].to_i, + max_quantity: quantity[:max_quantity].to_i + }) else - variants_array.push({ variant_id: variant_id, quantity: quantity }) + variants_array.push({ + variant_id: variant_id.to_i, + quantity: quantity.to_i + }) end end variants_array end - def cart_remove(variant_id) - variant = Spree::Variant.find(variant_id) - @order.remove_variant(variant) - end - def distributor_and_order_cycle [@order.distributor, @order.order_cycle] end @@ -131,7 +130,7 @@ class CartService li = line_item_for_variant loaded_variant li_added = li.nil? && (variant_data[:quantity].to_i > 0 || variant_data[:max_quantity].to_i > 0) - li_quantity_changed = li.present? && li.quantity.to_i != variant_data[:quantity].to_i + li_quantity_changed = li.present? && li.quantity != variant_data[:quantity].to_i li_max_quantity_changed = li.present? && li.max_quantity.to_i != variant_data[:max_quantity].to_i li_added || li_quantity_changed || li_max_quantity_changed diff --git a/app/services/order_fees_handler.rb b/app/services/order_fees_handler.rb index 14e7300a0b..6aa71dd9b9 100644 --- a/app/services/order_fees_handler.rb +++ b/app/services/order_fees_handler.rb @@ -19,9 +19,6 @@ class OrderFeesHandler create_line_item_fees! create_order_fees! - - order.updater.update_totals - order.updater.persist_totals end order.update_order! diff --git a/app/services/order_syncer.rb b/app/services/order_syncer.rb index 98e4fc9a53..343e73c665 100644 --- a/app/services/order_syncer.rb +++ b/app/services/order_syncer.rb @@ -15,6 +15,7 @@ class OrderSyncer distributor_id: shop_id) update_associations_for(order) line_item_syncer.sync!(order) + order.update_order! order.save end end diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb index 966722bdc7..ea6cf9da30 100644 --- a/spec/controllers/admin/bulk_line_items_controller_spec.rb +++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb @@ -209,10 +209,10 @@ describe Admin::BulkLineItemsController, type: :controller do allow(Spree::LineItem) .to receive(:find).with(line_item1.id.to_s).and_return(line_item1) - expect(line_item1.order).to receive(:reload).with(lock: true) + expect(line_item1.order).to receive(:with_lock).and_call_original expect(line_item1.order).to receive(:update_line_item_fees!) expect(line_item1.order).to receive(:update_order_fees!) - expect(line_item1.order).to receive(:update_order!).twice + expect(line_item1.order).to receive(:update_order!).once spree_put :update, params end @@ -349,6 +349,8 @@ describe Admin::BulkLineItemsController, type: :controller do it "correctly updates order totals and states" do expect(order.total).to eq 35.0 + expect(order.shipment_adjustments.shipping.sum(:amount)).to eq 6.0 + expect(order.shipment_adjustments.tax.sum(:amount)).to eq 0.29 expect(order.item_total).to eq 20.0 expect(order.adjustment_total).to eq 15.0 expect(order.included_tax_total).to eq 1.22 @@ -360,10 +362,11 @@ describe Admin::BulkLineItemsController, type: :controller do spree_put :update, params order.reload - expect(order.total).to eq 61.0 + expect(order.total).to eq 67.0 + expect(order.shipment_adjustments.sum(:amount)).to eq 12.57 expect(order.item_total).to eq 40.0 - expect(order.adjustment_total).to eq 21.0 - expect(order.included_tax_total).to eq 2.65 # Pending: this should be 3.10! + expect(order.adjustment_total).to eq 27.0 + expect(order.included_tax_total).to eq 2.93 # Pending: taxes on enterprise fees unchanged expect(order.payment_state).to eq "balance_due" end end @@ -373,21 +376,22 @@ describe Admin::BulkLineItemsController, type: :controller do it "correctly updates order totals and states" do expect(order.total).to eq 35.0 + expect(order.shipment_adjustments.shipping.sum(:amount)).to eq 6.0 + expect(order.shipment_adjustments.tax.sum(:amount)).to eq 0.29 expect(order.item_total).to eq 20.0 expect(order.adjustment_total).to eq 15.0 expect(order.included_tax_total).to eq 1.22 expect(order.payment_state).to eq "paid" - expect(order).to receive(:update_order!).at_least(:once).and_call_original - expect(order).to receive(:create_tax_charge!).at_least(:once).and_call_original - spree_delete :destroy, params order.reload - expect(order.total).to eq 22.0 + expect(order.total).to eq 19.0 + expect(order.shipment_adjustments.shipping.sum(:amount)).to eq 3.0 + expect(order.shipment_adjustments.tax.sum(:amount)).to eq 0.14 expect(order.item_total).to eq 10.0 - expect(order.adjustment_total).to eq 12.0 - expect(order.included_tax_total).to eq 0.99 + expect(order.adjustment_total).to eq 9.0 + expect(order.included_tax_total).to eq 0.84 expect(order.payment_state).to eq "credit_owed" end end diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index a63c023651..b8271ff27c 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -70,6 +70,7 @@ module Admin let!(:line_item) { create(:line_item, order: order, price: 10.0) } it 'includes the customer balance in the response' do + order.update_order! get :index, params: params expect(json_response.first["balance"]).to eq("$-10.00") end @@ -77,13 +78,16 @@ module Admin context 'when the customer has canceled orders' do let(:order) { create(:order, customer: customer) } - let!(:line_item) { create(:line_item, order: order, price: 10.0) } - let!(:payment) { create(:payment, order: order, amount: order.total) } + let!(:variant) { create(:variant, price: 10.0) } before do allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true) - order.process_payments! + order.contents.add(variant) + order.payments << create(:payment, order: order, amount: order.total) + order.reload + + order.process_payments! order.update_attribute(:state, 'canceled') end @@ -104,8 +108,7 @@ module Admin end context 'when the customer has an order with a void payment' do - let(:order) { create(:order, customer: customer, state: 'complete') } - let!(:line_item) { create(:line_item, order: order, price: 10.0) } + let(:order) { create(:order_with_totals, customer: customer, state: 'complete') } let!(:payment) { create(:payment, order: order, amount: order.total) } before do diff --git a/spec/controllers/api/v0/shipments_controller_spec.rb b/spec/controllers/api/v0/shipments_controller_spec.rb index dd1be3c0fc..35aebc1b88 100644 --- a/spec/controllers/api/v0/shipments_controller_spec.rb +++ b/spec/controllers/api/v0/shipments_controller_spec.rb @@ -174,6 +174,42 @@ describe Api::V0::ShipmentsController, type: :controller do }.to_not change { existing_variant.reload.on_hand } end end + + context "with shipping fees" do + let!(:distributor) { create(:distributor_enterprise) } + let(:fee_amount) { 10 } + let!(:shipping_method_with_fee) { + create(:shipping_method_with, :shipping_fee, distributors: [distributor], + shipping_fee: fee_amount) + } + let!(:order_cycle) { create(:order_cycle, distributors: [distributor]) } + let!(:order) { + create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor) + } + let(:shipping_fee) { order.reload.shipment.adjustments.first } + + before do + order.shipments.first.shipping_methods = [shipping_method_with_fee] + order.select_shipping_method(shipping_method_with_fee.id) + order.update_order! + end + + context "adding item to a shipment" do + it "updates the shipping fee" do + expect { + api_put :add, params.merge(variant_id: new_variant.to_param) + }.to change { order.reload.shipment.adjustments.first.amount }.by(20) + end + end + + context "removing item from a shipment" do + it "updates the shipping fee" do + expect { + api_put :remove, params.merge(variant_id: existing_variant.to_param) + }.to change { order.reload.shipment.adjustments.first.amount }.by(-20) + end + end + end end describe "#update" do diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb index 585c96a37e..86c78eacd5 100644 --- a/spec/controllers/cart_controller_spec.rb +++ b/spec/controllers/cart_controller_spec.rb @@ -16,7 +16,6 @@ describe CartController, type: :controller do it "returns HTTP success when successful" do allow(cart_service).to receive(:populate) { true } allow(cart_service).to receive(:valid?) { true } - allow(cart_service).to receive(:variants_h) { {} } post :populate, xhr: true, params: { use_route: :spree }, as: :json expect(response.status).to eq(200) end @@ -30,32 +29,17 @@ describe CartController, type: :controller do expect(response.status).to eq(412) end - it "tells cart_service to overwrite" do - allow(cart_service).to receive(:variants_h) { {} } - allow(cart_service).to receive(:valid?) { true } - expect(cart_service).to receive(:populate).with({}, true) - post :populate, xhr: true, params: { use_route: :spree }, as: :json - end - it "returns stock levels as JSON on success" do allow(controller).to receive(:variant_ids_in) { [123] } allow_any_instance_of(VariantsStockLevels).to receive(:call).and_return("my_stock_levels") allow(cart_service).to receive(:populate) { true } allow(cart_service).to receive(:valid?) { true } - allow(cart_service).to receive(:variants_h) { {} } post :populate, xhr: true, params: { use_route: :spree }, as: :json data = JSON.parse(response.body) expect(data['stock_levels']).to eq('my_stock_levels') end - - it "extracts variant ids from the cart service" do - variants_h = [{ variant_id: "900", quantity: 2, max_quantity: nil }, - { variant_id: "940", quantity: 3, max_quantity: 3 }] - - expect(controller.variant_ids_in(variants_h)).to eq([900, 940]) - end end context "handling variant overrides correctly" do @@ -110,7 +94,6 @@ describe CartController, type: :controller do order = subject.current_order(true) allow(order).to receive(:distributor) { distributor } allow(order).to receive(:order_cycle) { order_cycle } - expect(order).to receive(:set_variant_attributes).with(variant, max_quantity: "3") allow(controller).to receive(:current_order).and_return(order) expect do diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index 78f0459110..7934617294 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -250,7 +250,7 @@ describe Spree::OrdersController, type: :controller do before do order.set_distribution! d, oc - order.add_variant variant, 5 + order.contents.add(variant, 5) end describe "the page" do @@ -296,7 +296,7 @@ describe Spree::OrdersController, type: :controller do describe "when I pass params that includes a line item no longer in our cart" do it "should silently ignore the missing line item" do order = subject.current_order(true) - li = order.add_variant(create(:simple_product, on_hand: 110).variants.first) + li = order.contents.add(create(:simple_product, on_hand: 110).variants.first) get :update, params: { order: { line_items_attributes: { "0" => { quantity: "0", id: "9999" }, "1" => { quantity: "99", id: li.id } @@ -308,21 +308,21 @@ describe Spree::OrdersController, type: :controller do it "filters line items that are missing from params" do order = subject.current_order(true) - li = order.add_variant(create(:simple_product).master) + li = order.contents.add(create(:simple_product).variants.first) attrs = { "0" => { quantity: "0", id: "9999" }, "1" => { quantity: "99", id: li.id } } - expect(controller.remove_missing_line_items(attrs)).to eq( + expect(controller.__send__(:remove_missing_line_items, attrs)).to eq( "1" => { quantity: "99", id: li.id } ) end it "keeps the adjustments' previous state" do order = subject.current_order(true) - line_item = order.add_variant(create(:simple_product, on_hand: 110).variants.first) + line_item = order.contents.add(create(:simple_product, on_hand: 110).variants.first) adjustment = create(:adjustment, adjustable: order) get :update, params: { order: { line_items_attributes: { diff --git a/spec/factories/order_factory.rb b/spec/factories/order_factory.rb index 926020546d..1fc3156897 100644 --- a/spec/factories/order_factory.rb +++ b/spec/factories/order_factory.rb @@ -15,6 +15,7 @@ FactoryBot.define do after(:create) do |order| create(:line_item, order: order) order.line_items.reload # to ensure order.line_items is accessible after + order.updater.update_totals_and_states end end diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 758d7e6d68..7073da51be 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -414,6 +414,7 @@ feature ' let!(:adj_manual2) { create(:adjustment, order: order1, adjustable: order1, originator: nil, label: "Manual adjustment", amount: 40, included_tax: 3) } before do + order1.update_order! order1.update_attribute :email, 'customer@email.com' order1.shipment.update_columns(included_tax_total: 10.06) Timecop.travel(Time.zone.local(2015, 4, 25, 14, 0, 0)) { order1.finalize! } diff --git a/spec/features/consumer/shopping/cart_spec.rb b/spec/features/consumer/shopping/cart_spec.rb index 9af20fa926..b34c45ac7f 100644 --- a/spec/features/consumer/shopping/cart_spec.rb +++ b/spec/features/consumer/shopping/cart_spec.rb @@ -164,7 +164,7 @@ feature "full-page cart", js: true do let(:variant2) { product_with_fee.variants.first } before do - add_product_to_cart order, product_with_tax + order.contents.add(product_with_tax.variants.first) end describe "when on_hand is zero but variant is on demand" do @@ -179,7 +179,7 @@ feature "full-page cart", js: true do describe "with insufficient stock available" do it "prevents user from entering invalid values" do - add_product_to_cart order, product_with_fee + order.contents.add(product_with_fee.variants.first) variant.update!(on_hand: 2, on_demand: false) variant2.update!(on_hand: 3, on_demand: false) diff --git a/spec/javascripts/unit/darkswarm/controllers/checkout/shop_variant_controller_spec.js.coffee b/spec/javascripts/unit/darkswarm/controllers/checkout/shop_variant_controller_spec.js.coffee index 4576e376c7..c560a287c4 100644 --- a/spec/javascripts/unit/darkswarm/controllers/checkout/shop_variant_controller_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/controllers/checkout/shop_variant_controller_spec.js.coffee @@ -1,6 +1,7 @@ describe "ShopVariantCtrl", -> ctrl = null scope = null + CartMock = null beforeEach -> module 'Darkswarm' @@ -16,7 +17,10 @@ describe "ShopVariantCtrl", -> max_quantity: undefined } } - ctrl = $controller 'ShopVariantCtrl', {$scope: scope, $modal: $modal, Cart: null} + CartMock = + adjust: -> + true + ctrl = $controller 'ShopVariantCtrl', {$scope: scope, $modal: $modal, Cart: CartMock} it "initializes the quantity for shop display", -> expect(scope.variant.line_item.quantity).toEqual 0 diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index a98685bab4..c740d4b018 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -11,7 +11,6 @@ module Spree it 'should update inventory, totals, and tax' do # Regression check for Spree #1481 expect(line_item.order).to receive(:create_tax_charge!) - expect(line_item.order).to receive(:update_order!) line_item.quantity = 2 line_item.save end diff --git a/spec/models/spree/order_contents_spec.rb b/spec/models/spree/order_contents_spec.rb index 7a333521ee..88f6ff48fd 100644 --- a/spec/models/spree/order_contents_spec.rb +++ b/spec/models/spree/order_contents_spec.rb @@ -3,12 +3,11 @@ require 'spec_helper' describe Spree::OrderContents do - let(:order) { Spree::Order.create } + let!(:order) { create(:order) } + let!(:variant) { create(:variant) } subject { described_class.new(order) } context "#add" do - let(:variant) { create(:variant) } - context 'given quantity is not explicitly provided' do it 'should add one line item' do line_item = subject.add(variant) @@ -42,8 +41,6 @@ describe Spree::OrderContents do end context "#remove" do - let(:variant) { create(:variant) } - context "given an invalid variant" do it "raises an exception" do expect { @@ -53,11 +50,12 @@ describe Spree::OrderContents do end context 'given quantity is not explicitly provided' do - it 'should remove one line item' do - line_item = subject.add(variant, 3) - subject.remove(variant) + it 'should remove line item' do + subject.add(variant, 3) - expect(line_item.reload.quantity).to eq 2 + expect{ + subject.remove(variant) + }.to change(Spree::LineItem, :count).by -1 end end @@ -89,4 +87,105 @@ describe Spree::OrderContents do expect(order.total.to_f).to eq 19.99 end end + + context "#update_cart" do + let!(:line_item) { subject.add variant, 1 } + + let(:params) do + { line_items_attributes: { + "0" => { id: line_item.id, quantity: 3 } + } } + end + + it "changes item quantity" do + subject.update_cart params + expect(line_item.reload.quantity).to eq 3 + end + + 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: line_item.id, quantity: 0 } + } } + end + + it "removes item from order" do + expect { + subject.update_cart params + }.to change { order.line_items.count } + end + end + + it "ensures updated shipments" do + expect(subject.order).to receive(:ensure_updated_shipments) + subject.update_cart params + end + end + + describe "#update_item" do + let!(:line_item) { subject.add variant, 1 } + + context "updating enterprise fees" do + it "updates the line item's enterprise fees" do + expect(order).to receive(:update_line_item_fees!).with(line_item) + + subject.update_item(line_item, { quantity: 3 }) + end + + it "updates the order's enterprise fees if completed" do + allow(order).to receive(:completed?) { true } + expect(order).to receive(:update_order_fees!) + + subject.update_item(line_item, { quantity: 3 }) + end + + it "does not update the order's enterprise fees if not complete" do + expect(order).to_not receive(:update_order_fees!) + + subject.update_item(line_item, { quantity: 3 }) + end + end + + it "ensures updated shipments" do + expect(order).to receive(:ensure_updated_shipments) + + subject.update_item(line_item, { quantity: 3 }) + end + + it "updates the order" do + expect(order).to receive(:update_order!) + + subject.update_item(line_item, { quantity: 3 }) + end + end + + describe "#update_or_create" do + describe "creating" do + it "creates a new line item with given attributes" do + subject.update_or_create(variant, { quantity: 2, max_quantity: 3 }) + + line_item = order.line_items.reload.first + expect(line_item.quantity).to eq 2 + expect(line_item.max_quantity).to eq 3 + expect(line_item.price).to eq variant.price + end + end + + describe "updating" do + let!(:line_item) { subject.add variant, 2 } + + it "updates existing line item with given attributes" do + subject.update_or_create(variant, { quantity: 3, max_quantity: 4 }) + + expect(line_item.reload.quantity).to eq 3 + expect(line_item.max_quantity).to eq 4 + end + end + end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index e97ac29062..67c6306680 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -455,26 +455,6 @@ describe Spree::Order do end end - describe "setting variant attributes" do - it "sets attributes on line items for variants" do - d = create(:distributor_enterprise) - p = create(:product) - - subject.distributor = d - subject.save! - - subject.add_variant(p.master, 1, 3) - - li = Spree::LineItem.last - expect(li.max_quantity).to eq(3) - end - - it "does nothing when the line item is not found" do - p = build_stubbed(:simple_product) - subject.set_variant_attributes(p.master, { 'max_quantity' => '3' }.with_indifferent_access) - end - end - describe "applying enterprise fees" do subject { create(:order) } let(:fee_handler) { ::OrderFeesHandler.new(subject) } @@ -727,59 +707,6 @@ describe Spree::Order do end end - describe "removing an item from the order" do - let(:order) { create(:order) } - let(:v1) { create(:variant) } - let(:v2) { create(:variant) } - let(:v3) { create(:variant) } - - before do - order.add_variant v1 - order.add_variant v2 - - order.recreate_all_fees! - end - - it "removes the variant's line item" do - order.remove_variant v1 - expect(order.line_items.reload.map(&:variant)).to eq([v2]) - end - - it "does nothing when there is no matching line item" do - expect do - order.remove_variant v3 - end.to change(order.line_items.reload, :count).by(0) - end - - context "when the item has an associated adjustment" do - let(:distributor) { create(:distributor_enterprise) } - - let(:order_cycle) do - create(:order_cycle).tap do - create(:exchange, variants: [v1], incoming: true) - create(:exchange, variants: [v1], incoming: false, receiver: distributor) - end - end - - let(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) } - - it "removes the variant's line item" do - order.remove_variant v1 - expect(order.line_items.reload.map(&:variant)).to eq([v2]) - end - - it "removes the variant's adjustment" do - line_item = order.line_items.where(variant_id: v1.id).first - adjustment_scope = Spree::Adjustment.where(adjustable_type: "Spree::LineItem", - adjustable_id: line_item.id) - expect(adjustment_scope.count).to eq(1) - adjustment = adjustment_scope.first - order.remove_variant v1 - expect { adjustment.reload }.to raise_error - end - end - end - describe "emptying the order" do it "removes shipments" do subject.shipments << create(:shipment) @@ -1199,8 +1126,11 @@ describe Spree::Order do context "when no order has been finalised in this order cycle" do let(:product) { create(:product) } + before do + order.contents.update_or_create(product.variants.first, { quantity: 1, max_quantity: 3 }) + end + it "returns no items even though the cart contains items" do - order.add_variant(product.master, 1, 3) expect(order.finalised_line_items).to eq [] end end @@ -1210,9 +1140,12 @@ describe Spree::Order do let!(:prev_order2) { create(:completed_order_with_totals, distributor: distributor, order_cycle: order_cycle, user: order.user) } let(:product) { create(:product) } - it "returns previous items" do - prev_order.add_variant(product.master, 1, 3) + before do + prev_order.contents.update_or_create(product.variants.first, { quantity: 1, max_quantity: 3 }) prev_order2.reload # to get the right response from line_items + end + + it "returns previous items" do expect(order.finalised_line_items.length).to eq 11 expect(order.finalised_line_items).to match_array(prev_order.line_items + prev_order2.line_items) end @@ -1380,4 +1313,36 @@ describe Spree::Order do end end end + + describe "#ensure_updated_shipments" do + before { Spree::Shipment.create!(order: order) } + + context "when the order is not completed" do + it "destroys current shipments" do + order.ensure_updated_shipments + expect(order.shipments).to be_empty + end + + it "puts order back in address state" do + order.ensure_updated_shipments + expect(order.state).to eq "address" + end + end + + context "when the order is completed" do + before do + allow(order).to receive(:completed?) { true } + end + + it "does not change the shipments" do + expect { + order.ensure_updated_shipments + }.not_to change { order.shipments } + + expect { + order.ensure_updated_shipments + }.not_to change { order.state } + end + end + end end diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb index bddd520118..812b74b513 100644 --- a/spec/models/spree/payment_method_spec.rb +++ b/spec/models/spree/payment_method_spec.rb @@ -79,7 +79,7 @@ module Spree expect(flat_percent_payment_method.compute_amount(order)).to eq 0 product = create(:product) - order.add_variant(product.master) + order.contents.add(product.variants.first) expect(flat_percent_payment_method.compute_amount(order)).to eq 2.0 end diff --git a/spec/models/spree/stock/availability_validator_spec.rb b/spec/models/spree/stock/availability_validator_spec.rb index 44125cedab..385ffd46e2 100644 --- a/spec/models/spree/stock/availability_validator_spec.rb +++ b/spec/models/spree/stock/availability_validator_spec.rb @@ -52,6 +52,33 @@ module Spree expect(line_item).not_to be_valid end end + + context "when the line item's variant has an override" do + let(:hub) { order.distributor } + let(:variant) { line_item.variant } + let(:vo_stock) { 999 } + let!(:variant_override) { + create(:variant_override, variant: variant, hub: hub, count_on_hand: vo_stock) + } + + context "when the override has stock" do + it "is valid" do + line_item.quantity = 999 + validator.validate(line_item) + expect(line_item).to be_valid + end + end + + context "when the override is out of stock" do + let(:vo_stock) { 1 } + + it "is not valid" do + line_item.quantity = 999 + validator.validate(line_item) + expect(line_item).to_not be_valid + end + end + end end end end diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb index 51e8f2b882..b92e03355a 100644 --- a/spec/services/cart_service_spec.rb +++ b/spec/services/cart_service_spec.rb @@ -27,9 +27,9 @@ describe CartService do describe "#populate" do it "adds a variant" do cart_service.populate( - ActionController::Parameters.new({ variants: { variant.id.to_s => { quantity: '1', - max_quantity: '2' } } }), - true + ActionController::Parameters.new( + { variants: { variant.id.to_s => { quantity: '1', max_quantity: '2' } } } + ) ) li = order.find_line_item_by_variant(variant) expect(li).to be @@ -38,28 +38,35 @@ describe CartService do expect(li.final_weight_volume).to eq(1.0) end - it "updates a variant's quantity, max quantity and final_weight_volume" do - order.add_variant variant, 1, 2 + context "updating an existing variant" do + before do + order.contents.update_or_create(variant, { quantity: 1, max_quantity: 2 }) + end - cart_service.populate( - ActionController::Parameters.new({ variants: { variant.id.to_s => { quantity: '2', - max_quantity: '3' } } }), - true - ) - li = order.find_line_item_by_variant(variant) - expect(li).to be - expect(li.quantity).to eq(2) - expect(li.max_quantity).to eq(3) - expect(li.final_weight_volume).to eq(2.0) - end + it "updates a variant's quantity, max quantity and final_weight_volume" do + cart_service.populate( + ActionController::Parameters.new( + { variants: { variant.id.to_s => { quantity: '2', max_quantity: '3' } } } + ) + ) - it "removes a variant" do - order.add_variant variant, 1, 2 + li = order.find_line_item_by_variant(variant) + expect(li).to be + expect(li.quantity).to eq(2) + expect(li.max_quantity).to eq(3) + expect(li.final_weight_volume).to eq(2.0) + end - cart_service.populate(ActionController::Parameters.new({ variants: {} }), true) - order.line_items.reload - li = order.find_line_item_by_variant(variant) - expect(li).not_to be + it "removes a variant" do + cart_service.populate( + ActionController::Parameters.new( + { variants: { variant.id.to_s => { quantity: '0' } } } + ) + ) + order.line_items.reload + li = order.find_line_item_by_variant(variant) + expect(li).not_to be + end end context "when a variant has been soft-deleted" do @@ -69,7 +76,11 @@ describe CartService do it "does not add the deleted variant to the cart" do variant.delete - cart_service.populate(ActionController::Parameters.new({ variants: { variant.id.to_s => { quantity: '2' } } }), true) + cart_service.populate( + ActionController::Parameters.new( + { variants: { variant.id.to_s => { quantity: '2' } } } + ) + ) expect(relevant_line_item).to be_nil expect(cart_service.errors.count).to be 0 @@ -84,7 +95,11 @@ describe CartService do it "removes the line_item from the cart" do variant.delete - cart_service.populate(ActionController::Parameters.new({ variants: { variant.id.to_s => { quantity: '3' } } }), true) + cart_service.populate( + ActionController::Parameters.new( + { variants: { variant.id.to_s => { quantity: '3' } } } + ) + ) expect(Spree::LineItem.where(id: relevant_line_item).first).to be_nil expect(cart_service.errors.count).to be 0 @@ -98,28 +113,28 @@ describe CartService do let!(:variant) { create(:variant) } it "returns true when item is not in cart and a quantity is specified" do - variant_data = { variant_id: variant.id, quantity: '2' } + variant_data = { variant_id: variant.id, quantity: 2 } expect(cart_service).to receive(:line_item_for_variant).with(variant).and_return(nil) expect(cart_service.send(:varies_from_cart, variant_data, variant )).to be true end it "returns true when item is not in cart and a max_quantity is specified" do - variant_data = { variant_id: variant.id, quantity: '0', max_quantity: '2' } + variant_data = { variant_id: variant.id, quantity: 0, max_quantity: 2 } expect(cart_service).to receive(:line_item_for_variant).with(variant).and_return(nil) expect(cart_service.send(:varies_from_cart, variant_data, variant)).to be true end it "returns false when item is not in cart and no quantity or max_quantity are specified" do - variant_data = { variant_id: variant.id, quantity: '0' } + variant_data = { variant_id: variant.id, quantity: 0 } expect(cart_service).to receive(:line_item_for_variant).with(variant).and_return(nil) expect(cart_service.send(:varies_from_cart, variant_data, variant)).to be false end it "returns true when quantity varies" do - variant_data = { variant_id: variant.id, quantity: '2' } + variant_data = { variant_id: variant.id, quantity: 2 } line_item = double(:line_item, quantity: 1, max_quantity: nil) allow(cart_service).to receive(:line_item_for_variant) { line_item } @@ -127,7 +142,7 @@ describe CartService do end it "returns true when max_quantity varies" do - variant_data = { variant_id: variant.id, quantity: '1', max_quantity: '3' } + variant_data = { variant_id: variant.id, quantity: 1, max_quantity: 3 } line_item = double(:line_item, quantity: 1, max_quantity: nil) allow(cart_service).to receive(:line_item_for_variant) { line_item } @@ -135,7 +150,7 @@ describe CartService do end it "returns false when max_quantity varies only in nil vs 0" do - variant_data = { variant_id: variant.id, quantity: '1' } + variant_data = { variant_id: variant.id, quantity: 1 } line_item = double(:line_item, quantity: 1, max_quantity: nil) allow(cart_service).to receive(:line_item_for_variant) { line_item } @@ -143,7 +158,7 @@ describe CartService do end it "returns false when both are specified and neither varies" do - variant_data = { variant_id: variant.id, quantity: '1', max_quantity: '2' } + variant_data = { variant_id: variant.id, quantity: 1, max_quantity: 2 } line_item = double(:line_item, quantity: 1, max_quantity: 2) allow(cart_service).to receive(:line_item_for_variant) { line_item } @@ -151,30 +166,6 @@ describe CartService do end end - describe "variants_removed" do - it "returns the variant ids when one is in the cart but not in those given" do - allow(cart_service).to receive(:variant_ids_in_cart) { [123] } - expect(cart_service.send(:variants_removed, [])).to eq([123]) - end - - it "returns nothing when all items in the cart are provided" do - allow(cart_service).to receive(:variant_ids_in_cart) { [123] } - expect(cart_service.send(:variants_removed, [{ variant_id: '123' }])).to eq([]) - end - - it "returns nothing when items are added to cart" do - allow(cart_service).to receive(:variant_ids_in_cart) { [123] } - expect( - cart_service.send(:variants_removed, [{ variant_id: '123' }, { variant_id: '456' }]) - ).to eq([]) - end - - it "does not return duplicates" do - allow(cart_service).to receive(:variant_ids_in_cart) { [123, 123] } - expect(cart_service.send(:variants_removed, [])).to eq([123]) - end - end - describe "attempt_cart_add" do let!(:variant) { create(:variant, on_hand: 250) } let(:quantity) { 123 } @@ -189,38 +180,40 @@ describe CartService do expect(cart_service).to receive(:check_variant_available_under_distribution).with(variant). and_return(true) expect(variant).to receive(:on_demand).and_return(false) - expect(order).to receive(:add_variant).with(variant, quantity, nil, currency) + expect(order).to receive_message_chain(:contents, :update_or_create). + with(variant, { quantity: quantity, max_quantity: nil }) - cart_service.send(:attempt_cart_add, variant, quantity.to_s) + cart_service.send(:attempt_cart_add, variant, quantity) end - it "filters quantities through #quantities_to_add" do - expect(cart_service).to receive(:quantities_to_add).with(variant, 123, 123). - and_return([5, 5]) + it "filters quantities through #final_quantities" do + expect(cart_service).to receive(:final_quantities).with(variant, 123, 123). + and_return({ quantity: 5, max_quantity: 5 }) allow(cart_service).to receive(:check_order_cycle_provided) { true } allow(cart_service).to receive(:check_variant_available_under_distribution) { true } - expect(order).to receive(:add_variant).with(variant, 5, 5, currency) + expect(order).to receive_message_chain(:contents, :update_or_create). + with(variant, { quantity: 5, max_quantity: 5 }) - cart_service.send(:attempt_cart_add, variant, quantity.to_s, quantity.to_s) + cart_service.send(:attempt_cart_add, variant, quantity, quantity) end it "removes variants which have become out of stock" do - expect(cart_service).to receive(:quantities_to_add).with(variant, 123, 123). - and_return([0, 0]) + expect(cart_service).to receive(:final_quantities).with(variant, 123, 123). + and_return({ quantity: 0, max_quantity: 0 }) allow(cart_service).to receive(:check_order_cycle_provided) { true } allow(cart_service).to receive(:check_variant_available_under_distribution) { true } - expect(order).to receive(:remove_variant).with(variant) - expect(order).to receive(:add_variant).never + expect(cart_service).to receive(:cart_add).with(variant, 123, 123).and_call_original + expect(order).to receive_message_chain(:contents, :remove).with(variant) - cart_service.send(:attempt_cart_add, variant, quantity.to_s, quantity.to_s) + cart_service.send(:attempt_cart_add, variant, quantity, quantity) end end - describe "quantities_to_add" do + describe "#final_quantities" do let(:v) { double(:variant, on_hand: 10) } context "when backorders are not allowed" do @@ -228,23 +221,27 @@ describe CartService do expect(v).to receive(:on_demand).and_return(false) end - context "when max_quantity is not provided" do + context "getting quantity and max_quantity" do it "returns full amount when available" do - expect(cart_service.send(:quantities_to_add, v, 5, nil)).to eq([5, nil]) + expect(cart_service.send(:final_quantities, v, 5, nil)). + to eq({ quantity: 5, max_quantity: nil }) end it "returns a limited amount when not entirely available" do - expect(cart_service.send(:quantities_to_add, v, 15, nil)).to eq([10, nil]) + expect(cart_service.send(:final_quantities, v, 15, nil)). + to eq({ quantity: 10, max_quantity: nil }) end end context "when max_quantity is provided" do it "returns full amount when available" do - expect(cart_service.send(:quantities_to_add, v, 5, 6)).to eq([5, 6]) + expect(cart_service.send(:final_quantities, v, 5, 6)). + to eq({ quantity: 5, max_quantity: 6 }) end it "also returns the full amount when not entirely available" do - expect(cart_service.send(:quantities_to_add, v, 15, 16)).to eq([10, 16]) + expect(cart_service.send(:final_quantities, v, 15, 16)). + to eq({ quantity: 10, max_quantity: 16 }) end end end @@ -255,11 +252,13 @@ describe CartService do end it "does not limit quantity" do - expect(cart_service.send(:quantities_to_add, v, 15, nil)).to eq([15, nil]) + expect(cart_service.send(:final_quantities, v, 15, nil)). + to eq({ quantity: 15, max_quantity: nil }) end it "does not limit max_quantity" do - expect(cart_service.send(:quantities_to_add, v, 15, 16)).to eq([15, 16]) + expect(cart_service.send(:final_quantities, v, 15, 16)). + to eq({ quantity: 15, max_quantity: 16 }) end end end diff --git a/spec/services/order_syncer_spec.rb b/spec/services/order_syncer_spec.rb index 93c91448e4..f2158aaff0 100644 --- a/spec/services/order_syncer_spec.rb +++ b/spec/services/order_syncer_spec.rb @@ -443,7 +443,10 @@ describe OrderSyncer do before { variant.update_attribute(:on_hand, 3) } context "when the changed line_item quantity matches the new quantity on the subscription line item" do - before { changed_line_item.update(quantity: 3) } + before do + changed_line_item.update(quantity: 3) + order.update_order! + end it "does not change the quantity, and doesn't add the order to order_update_issues" do expect(order.reload.total.to_f).to eq 99.95 @@ -456,7 +459,10 @@ describe OrderSyncer do end context "when the changed line_item quantity doesn't match the new quantity on the subscription line item" do - before { changed_line_item.update(quantity: 2) } + before do + changed_line_item.update(quantity: 2) + order.update_order! + end it "does not change the quantity, and adds the order to order_update_issues" do expect(order.reload.total.to_f).to eq 79.96