Browse files

Improved handle of stock levels when backordering is disabled

  • Loading branch information...
1 parent be8a4bb commit 7c9d1782550df9d07a35ec63ab2125f6ec88fc38 @BDQ BDQ committed Sep 23, 2011
View
2 auth/features/checkout.feature
@@ -1,7 +1,7 @@
Feature: Checkout
In order to buy stuff
As a user
- I should be able make checkout
+ I should be able to checkout
@selenium @wip @stop
Scenario: Visitor make checkout as guest, without registration
View
4 core/app/assets/javascripts/admin/orders/edit_form.js
@@ -9,7 +9,9 @@ $.each($('td.qty input'), function(i, inpt){
type: "POST",
url: "/admin/orders/" + $('input#order_number').val() + "/line_items/" + $(id).val(),
data: ({_method: "put", "line_item[quantity]": $(this).val()}),
- success: function(html){ $('#order-form-wrapper').html(html)}
+ complete: function(resp){
+ $('#order-form-wrapper').html(resp.responseText);
+ }
});
}, 0,5);
View
4 core/app/controllers/admin/line_items_controller.rb
@@ -14,7 +14,9 @@ def create
format.html { render :partial => "admin/orders/form", :locals => {:order => @order.reload}, :layout => false }
end
else
- #TODO Handle failure gracefully, patches welcome.
+ respond_with(@line_item) do |format|
+ format.js { render :action => 'create', :locals => {:order => @order.reload}, :layout => false }
+ end
end
end
View
10 core/app/controllers/checkout_controller.rb
@@ -61,20 +61,12 @@ def object_params
def load_order
@order = current_order
redirect_to cart_path and return unless @order and @order.checkout_allowed?
- raise_insufficient_quantity and return unless inventory_available?
+ raise_insufficient_quantity and return if @order.insufficient_stock_lines.present?
redirect_to cart_path and return if @order.completed?
@order.state = params[:state] if params[:state]
state_callback(:before)
end
- def inventory_available?
- if Spree::Config[:allow_backorder_shipping] == false && Spree::Config[:track_inventory_levels] == true && Spree::Config[:create_inventory_units] == true
- @order.line_items.each do |line_item|
- return false unless line_item.variant.on_hand > 0
- end
- end
- end
-
def raise_insufficient_quantity
flash[:error] = t('spree_inventory_error_flash_for_insufficient_quantity')
redirect_to cart_path
View
1 core/app/helpers/spree/base_helper.rb
@@ -35,6 +35,7 @@ def yesterdays_short_date
# human readable list of variant options
def variant_options(v, allow_back_orders = Spree::Config[:allow_backorders], include_style = true)
+ ActiveSupport::Deprecation.warn("variant_options method is deprecated, and will be removed in 0.80.0", caller)
list = v.options_text
# We shouldn't show out of stock if the product is infact in stock
View
16 core/app/models/inventory_unit.rb
@@ -22,12 +22,6 @@ class InventoryUnit < ActiveRecord::Base
after_transition :to => 'returned', :do => :restock_variant
end
- # method deprecated in favour of adjust_units (which creates & destroys units as needed).
- def self.sell_units(order)
- warn "[DEPRECATION] `InventoryUnits#sell_units` is deprecated. Please use `InventoryUnits#assign_opening_inventory` instead. (called from #{caller[0]})"
- self.adjust_units(order)
- end
-
# Assigns inventory to a newly completed order.
# Should only be called once during the life-cycle of an order, on transition to completed.
#
@@ -88,7 +82,10 @@ def self.determine_backorder(order, variant, quantity)
end
def self.destroy_units(order, variant, quantity)
- variant_units = order.inventory_units.group_by(&:variant_id)[variant.id].sort_by(&:state)
+ variant_units = order.inventory_units.group_by(&:variant_id)
+ return unless variant_units.include? variant.id
+
+ variant_units = variant_units[variant.id].sort_by(&:state)
quantity.times do
inventory_unit = variant_units.shift
@@ -97,9 +94,8 @@ def self.destroy_units(order, variant, quantity)
end
def self.create_units(order, variant, sold, back_order)
- if back_order > 0 && !Spree::Config[:allow_backorders]
- raise "Cannot request back orders when backordering is disabled"
- end
+ return if back_order > 0 && !Spree::Config[:allow_backorders]
+
shipment = order.shipments.detect {|shipment| !shipment.shipped? }
View
19 core/app/models/line_item.rb
@@ -10,6 +10,8 @@ class LineItem < ActiveRecord::Base
validates :variant, :presence => true
validates :quantity, :numericality => { :only_integer => true, :message => I18n.t("validation.must_be_int") }
validates :price, :numericality => true
+ validate :stock_availability
+
# validate :meta_validation_of_quantities
attr_accessible :quantity
@@ -58,6 +60,14 @@ def adjust_quantity
self.quantity = 0 if self.quantity.nil? || self.quantity < 0
end
+ def sufficient_stock?
+ Spree::Config[:allow_backorders] ? true : (self.variant.on_hand >= self.quantity)
+ end
+
+ def insufficient_stock?
+ !sufficient_stock?
+ end
+
private
def update_inventory
return true unless self.order.completed?
@@ -89,7 +99,12 @@ def ensure_not_shipped
if order.try(:inventory_units).to_a.any?{|unit| unit.variant_id == variant_id && unit.shipped?}
errors.add :base, I18n.t("cannot_destory_line_item_as_inventory_units_have_shipped")
return false
- end
- end
+ end
+ end
+
+ def stock_availability
+ return if sufficient_stock?
+ errors.add(:quantiy, "can't be greater than avaiable stock.")
+ end
end
View
8 core/app/models/order.rb
@@ -45,8 +45,6 @@ def ip_address
make_permalink :field => :number
- attr_accessor :out_of_stock_items
-
class_attribute :update_hooks
self.update_hooks = Set.new
@@ -318,7 +316,7 @@ def process_payments!
# Called after transition to complete state when payments will have been processed
def finalize!
update_attribute(:completed_at, Time.now)
- self.out_of_stock_items = InventoryUnit.assign_opening_inventory(self)
+ InventoryUnit.assign_opening_inventory(self)
# lock any optional adjustments (coupon promotions, etc.)
adjustments.optional.each { |adjustment| adjustment.update_attribute("locked", true) }
OrderMailer.confirm_email(self).deliver
@@ -378,6 +376,10 @@ def products
line_items.map{|li| li.variant.product}
end
+ def insufficient_stock_lines
+ line_items.select &:insufficient_stock?
+ end
+
private
def create_user
self.email = user.email if self.user and not user.anonymous?
View
4 core/app/views/admin/orders/_line_item.html.erb
@@ -1,5 +1,7 @@
<tr id="<%= dom_id(f.object) %>" data-hook="admin_order_form_line_item_row">
- <td width="300"><%=f.object.variant.product.name%> <%= "(" + variant_options(f.object.variant) + ")" unless f.object.variant.option_values.empty? %></td>
+ <td width="300">
+ <%=f.object.variant.product.name%> <%= "(#{f.object.variant.options_text}))" unless f.object.variant.option_values.empty? %>
+ </td>
<td valign="top" class="price"><%= number_to_currency f.object.price %></td>
<td valign="top" class="qty"><%=f.text_field :quantity, :style => "width:30px;", :class => "qty" %></td>
<td valign="top" class="total"><%= number_to_currency (f.object.price * f.object.quantity)%></td>
View
2 core/app/views/admin/variants/index.html.erb
@@ -17,7 +17,7 @@
<!-- you can skip variant with no options: that's just the default variant that all products have -->
<% next if variant.option_values.empty? %>
<tr id="<%= dom_id(variant) %>" <%= 'style="color:red;"' if variant.deleted? %> data-hook="variants_row">
- <td><span class="handle"></span> <%= variant_options variant %></td>
+ <td><span class="handle"></span> <%= variant.options_text %></td>
<td><%= variant.price %></td>
<td><%= variant.sku %></td>
<% Variant.additional_fields.select{|f| f[:only].nil? || f[:only].include?(:variant) }.each do |field| %>
View
2 core/app/views/order_mailer/confirm_email.text.erb
@@ -6,7 +6,7 @@ Please review and retain the following order information for your records.
Order Summary
============================================================
<% for item in @order.line_items %>
-<%=item.variant.sku %> <%=item.variant.product.name%> <%= variant_options(item.variant) %> (<%=item.quantity%>) @ <%= number_to_currency item.price %> = <%= number_to_currency(item.price * item.quantity) %>
+<%=item.variant.sku %> <%=item.variant.product.name%> <%= item.variant.options_text -%> (<%=item.quantity%>) @ <%= number_to_currency item.price %> = <%= number_to_currency(item.price * item.quantity) %>
<% end %>
============================================================
Subtotal: <%= number_to_currency @order.item_total %>
View
9 core/app/views/orders/_line_item.html.erb
@@ -8,7 +8,12 @@
</td>
<td data-hook="cart_item_description">
<h4><%= link_to variant.product.name, product_path(variant.product) %></h4>
- <%= variant_options variant %>
+ <%= variant.options_text %>
+ <% if @order.insufficient_stock_lines.include? line_item %>
+ <span class="out-of-stock">
+ <%= variant.in_stock? ? t(:insufficient_stock, :on_hand => variant.on_hand) : t(:out_of_stock) %><br>
+ </span>
+ <% end %>
<%= truncate(variant.product.description, :length => 100, :omission => "...") %>
</td>
<td data-hook="cart_item_price">
@@ -21,6 +26,6 @@
<%= format_price(product_price(line_item.variant, :format_as_currency => false) * line_item.quantity) unless line_item.quantity.nil? %>
</td>
<td data-hook="cart_item_delete">
- <%= link_to(image_tag('icons/delete.png'), '#', :class => 'delete') %>
+ <%= link_to(image_tag('icons/delete.png'), '#', :class => 'delete', :id => "delete_#{dom_id(line_item)}" ) %>
</td>
</tr>
View
3 core/config/locales/en.yml
@@ -465,6 +465,7 @@ en:
included_in_other_shipment: Included in another Shipment
included_in_this_shipment: Included in this Shipment
instructions_to_reset_password: "Fill out the form below and instructions to reset your password will be emailed to you:"
+ insufficient_stock: "Insufficient stock available, only %{on_hand} remaining"
integration_settings_warning: "If you are changing the billing integration, you must save first before you can edit the integration settings"
intercept_email_address: Intercept Email Address
intercept_email_instructions: "Override email recipient and replace with this address."
@@ -930,7 +931,7 @@ en:
date: Date
time: Time
spree_gateway_error_flash_for_checkout: "There was a problem with your payment information. Please check your information and try again."
- spree_inventory_error_flash_for_insufficient_quantity: "An item in your cart is no longer available please remove %{line_item}"
+ spree_inventory_error_flash_for_insufficient_quantity: "An item in your cart has become unavailable."
ssl_will_be_used_in_development_and_test_modes: "SSL will be used in development and test mode if necessary."
ssl_will_be_used_in_production_mode: "SSL will be used in production mode"
ssl_will_not_be_used_in_development_and_test_modes: "SSL will not be used in development and test mode if necessary."
View
19 core/features/checkout.feature
@@ -0,0 +1,19 @@
+Feature: Checkout
+ In order to buy stuff
+ As a user
+ I should be warned about out of stock items
+
+ @selenium @stop
+ Scenario: Visitor make checkout as guest, without registration
+ Given backordering is disabled
+ When I add a product with name: "RoR Mug" to cart
+ Then I should see "Shopping Cart" within "h1"
+ Then product with name: "RoR Mug" goes out of stock
+ When I follow "Checkout"
+ Then I should see "An item in your cart has become unavailable."
+ Then I should see "Out of Stock" within "span.out-of-stock"
+
+ When I click first link from selector "a.delete"
+ When I add a product with name: "RoR Shirt" to cart
+ When I follow "Checkout"
+ Then I should see "Checkout" within "h1"
View
1 core/features/products.feature
@@ -6,6 +6,7 @@ Feature: Visiting products
| Brand |
| Categories |
Given the custom taxons and custom products exist
+ Given backordering is enabled
Scenario: show page
When I go to the home page
View
1 core/features/support/env.rb
@@ -2,4 +2,3 @@
# load shared env with features
require File.expand_path('../../../../features/support/env', __FILE__)
-
View
15 core/spec/controllers/checkout_controller_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
describe CheckoutController do
- let(:order) { mock_model(Order, :checkout_allowed? => true, :completed? => false, :update_attributes => true, :payment? => false).as_null_object }
+ let(:order) { mock_model(Order, :checkout_allowed? => true, :completed? => false, :update_attributes => true, :payment? => false, :insufficient_stock_lines => []).as_null_object }
before { controller.stub :current_order => order, :current_user => nil }
it "should understand checkout routes" do
@@ -143,15 +143,16 @@
end
context "When last inventory item has been purchased and no backorders" do
+ let(:product) { mock_model(Product, :name => "Amazing Object") }
let(:variant) { mock_model(Variant, :on_hand => 0) }
- let(:line_item) { mock_model(LineItem, :variant => variant, :quantity => 1) }
- let(:order) { mock_model(Order, :checkout_allowed? => true, :line_items => [line_item], :inventory_units => []) }
+ let(:line_item) { mock_model(LineItem, :variant => variant, :quantity => 1, :product => product) }
+ let(:order) { Factory.new(:order) }
- before {
+ before do
+ order.stub(:line_items => [line_item])
Spree::Config.set :track_inventory_levels => true
- Spree::Config.set :create_inventory_units => true
Spree::Config.set :allow_backorders => false
- }
+ end
context "back orders == false" do
before do
@@ -163,7 +164,7 @@
end
it "should set flash message for no inventory" do
- flash[:error].should == I18n.t('spree_inventory_error_flash_for_insufficient_quantity')
+ flash[:error].should == I18n.t('spree_inventory_error_flash_for_insufficient_quantity' , :names => "'#{product.name}'" )
end
end
View
4 core/spec/models/inventory_unit_spec.rb
@@ -205,10 +205,6 @@
context "when :allow_backorders is false" do
before { Spree::Config.set :allow_backorders => false }
- it "should raise an exception when back_order units are requested" do
- lambda {InventoryUnit.create_units(order, variant, 2, 3) }.should raise_error
- end
-
it "should create sold items" do
order.inventory_units.should_receive(:create).with(:variant => variant, :state => "sold", :shipment => shipment).exactly(2).times
InventoryUnit.create_units(order, variant, 2, 0)
View
45 core/spec/models/line_item_spec.rb
@@ -93,9 +93,8 @@
end
-
context "with inventory units" do
- let(:inventory_unit) { mock_model(LineItem, :variant_id => variant.id, :shipped? => false) }
+ let(:inventory_unit) { mock_model(InventoryUnit, :variant_id => variant.id, :shipped? => false) }
before do
order.stub(:inventory_units => [inventory_unit])
line_item.stub(:order => order, :variant_id => variant.id)
@@ -114,6 +113,48 @@
end
+ end
+
+ context "(in)sufficient_stock?" do
+
+ context "when backordering is disabled" do
+ before { Spree::Config.set :allow_backorders => false }
+
+ it "should report insufficient stock when variant is out of stock" do
+ line_item.stub_chain :variant, :on_hand => 0
+ line_item.insufficient_stock?.should be_true
+ line_item.sufficient_stock?.should be_false
+ end
+
+ it "should report insufficient stock when variant has less on_hand that line_item quantity" do
+ line_item.stub_chain :variant, :on_hand => 3
+ line_item.insufficient_stock?.should be_true
+ line_item.sufficient_stock?.should be_false
+ end
+
+ it "should report sufficient stock when variant has enough on_hand" do
+ line_item.stub_chain :variant, :on_hand => 300
+ line_item.insufficient_stock?.should be_false
+ line_item.sufficient_stock?.should be_true
+ end
+ end
+
+ context "when backordering is enabled" do
+ before { Spree::Config.set :allow_backorders => true }
+
+ it "should report sufficient stock regardless of on_hand value" do
+ [-99,0,99].each do |i|
+ line_item.stub_chain :variant, :on_hand => i
+ line_item.insufficient_stock?.should be_false
+ line_item.sufficient_stock?.should be_true
+ end
+ end
+
+ end
+
end
+
+
+
end
View
11 core/spec/models/order_spec.rb
@@ -573,4 +573,15 @@
end
+ context "insufficient_stock_lines" do
+ let(:line_item) { mock_model LineItem, :insufficient_stock? => true }
+
+ before { order.stub(:line_items => [line_item]) }
+
+ it "should return line_item that has insufficent stock on hand" do
+ order.insufficient_stock_lines.size.should == 1
+ order.insufficient_stock_lines.include?(line_item).should be_true
+ end
+
+ end
end
View
33 features/step_definitions/checkout_steps.rb
@@ -17,7 +17,7 @@
end
Given /^a product with (.*?)? exists$/ do |captured_fields|
- fields = {'name' => "RoR Mug", 'count_on_hand' => '10', 'price' => "14.99"}
+ fields = {'name' => "RoR Mug", 'price' => "14.99"}
captured_fields.split(/,\s+/).each do |field|
(name, value) = field.split(/:\s*/, 2)
fields[name] = value.delete('"')
@@ -26,13 +26,16 @@
price = fields.delete('price')
if Product.search.master_price_equals(price).count(:conditions => fields) == 0
- Factory(:product, fields.merge('price' => price, :sku => 'ABC',
+ product = Factory(:product, fields.merge('price' => price, :sku => 'ABC',
:available_on => (Time.now - 100.days)))
+
+ product.on_hand = 10
+ product.save
end
end
When /^(?:|I )add a product with (.*?)? to cart$/ do |captured_fields|
- fields = {'name' => "RoR Mug", 'count_on_hand' => '10', 'price' => "14.99"}
+ fields = {'name' => "RoR Mug", 'price' => "14.99"}
captured_fields.split(/,\s+/).each do |field|
(name, value) = field.split(/:\s*/, 2)
fields[name] = value.delete('"')
@@ -41,8 +44,11 @@
price = fields.delete('price')
if Product.search.master_price_equals(price).count(:conditions => fields) == 0
- Factory(:product, fields.merge('price' => price, :sku => 'ABC',
+ product = Factory(:product, fields.merge('price' => price, :sku => 'ABC',
:available_on => (Time.now - 100.days)))
+
+ product.on_hand = 10
+ product.save
end
When %{I go to the products page}
@@ -53,12 +59,31 @@
When %{I press "Add To Cart"}
end
+
When /^I choose "(.*?)" as shipping method$/ do |shipping_method|
shipping_method = "order_shipping_method_id_#{ShippingMethod.find_by_name(shipping_method).id}"
When %{I choose "#{shipping_method}"}
And %{press "Save and Continue"}
end
+Then /^product with (.*?)? goes out of stock$/ do |captured_fields|
+ fields = {'name' => "RoR Mug"}
+ captured_fields.split(/,\s+/).each do |field|
+ (name, value) = field.split(/:\s*/, 2)
+ fields[name] = value.delete('"')
+ end
+
+ product = Product.where(:name => fields['name']).first
+ product.on_hand = 0
+ product.save
+end
+
+Given /^backordering is ([^"]*)$/ do |state|
+ @configuration ||= AppConfiguration.find_or_create_by_name("Default configuration")
+ Spree::Config.set :allow_backorders => (state == "disabled" ? false : true)
+end
+
+
When /^I choose "(.*?)" as shipping method and "(.*?)" as payment method(?: and set coupon code to "(.*?)")?$/ do |shipping_method, payment_method, coupon_code|
When %{I choose "#{shipping_method}" as shipping method}
View
3 features/support/env.rb
@@ -3,8 +3,7 @@
require File.expand_path("../spec/dummy/config/environment", FEATURES_PATH)
# sometimes tests fail randomly because cache is not refreshed, fixed that
-Spree::Config.set(:foo => "bar")
-
+Spree::Config
require 'bundler'
Bundler.setup(:cucumber)

0 comments on commit 7c9d178

Please sign in to comment.