Skip to content

Commit

Permalink
Merge pull request openfoodfoundation#3232 from luisramos0/2-0-out-of…
Browse files Browse the repository at this point in the history 
…-stock

[Spree Upgrade] Scope variants in line items availability validator
  • Loading branch information
@luisramos0
luisramos0 authored Dec 21, 2018
2 parents 6935bfd + 6c70998 commit 1007add
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 78 deletions.
33 changes: 33 additions & 0 deletions app/models/concerns/variant_stock.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,39 @@ def on_demand=(new_value)
end
end

# Moving Spree::Stock::Quantifier.can_supply? to the variant enables us to override this behaviour for variant overrides
# We can have this responsibility here in the variant because there is only one stock item per variant
#
# Here we depend only on variant.total_on_hand and variant.on_demand.
# This way, variant_overrides only need to override variant.total_on_hand and variant.on_demand.
def can_supply?(quantity)
return true unless Spree::Config[:track_inventory_levels]

on_demand || total_on_hand >= quantity
end

# We override Spree::Variant.in_stock? to avoid depending on the non-overidable method Spree::Stock::Quantifier.can_supply?
def in_stock?(quantity = 1)
can_supply?(quantity)
end

# Moving Spree::StockLocation.fill_status to the variant enables us to override this behaviour for variant overrides
# We can have this responsibility here in the variant because there is only one stock item per variant
#
# Here we depend only on variant.total_on_hand and variant.on_demand.
# This way, variant_overrides only need to override variant.total_on_hand and variant.on_demand.
def fill_status(quantity)
if count_on_hand >= quantity
on_hand = quantity
backordered = 0
else
on_hand = [0, total_on_hand].max
backordered = on_demand ? (quantity - on_hand) : 0
end

[on_hand, backordered]
end

# We can have this responsibility here in the variant because there is only one stock item per variant
#
# This enables us to override this behaviour for variant overrides
Expand Down
27 changes: 12 additions & 15 deletions app/models/spree/line_item_decorator.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,19 @@ def unit_value
final_weight_volume / quantity
end

# MONKEYPATCH of Spree method
# Enables scoping of variant to hub/shop, so we check stock against relevant overrides if they exist
# Also skips stock check if requested quantity is zero
# Overrides Spree version to:
# - skip stock check if skip_stock_check flag is active
# - skip stock check if requested quantity is zero or negative
# - scope variants to hub and thus acivate variant overrides
def sufficient_stock?
return true if quantity == 0 # This line added
scoper.scope(variant) # This line added
return true if variant.on_demand
if new_record? || !order.completed?
variant.on_hand >= quantity
else
variant.on_hand >= (quantity - self.changed_attributes['quantity'].to_i)
end
return true if skip_stock_check
return true if quantity <= 0
scoper.scope(variant)
variant.can_supply?(quantity)
end

def scoper
@scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor)
end

private
Expand All @@ -133,10 +134,6 @@ def update_inventory_with_scoping
end
alias_method_chain :update_inventory, :scoping

def scoper
@scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor)
end

def calculate_final_weight_volume
if final_weight_volume.present? && quantity_was > 0
self.final_weight_volume = final_weight_volume * quantity / quantity_was
Expand Down
24 changes: 16 additions & 8 deletions app/models/spree/stock/availability_validator_decorator.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,26 @@ def quantity_in_shipment(line_item)

def line_item_shipment(line_item)
return line_item.target_shipment if line_item.target_shipment
return line_item.order.shipments.first if line_item.order.present? && line_item.order.shipments.any?
return line_item.order.shipments.first if line_item.order.andand.shipments.any?
end

# This is the spree v2.0.4 implementation of validate
# But using the calculated quantity instead of the line_item.quantity.
# Overrides Spree v2.0.4 validate method version to:
# - scope variants to hub and thus acivate variant overrides
# - use calculated quantity instead of the line_item.quantity
# - rely on Variant.can_supply? instead of Stock::Quantified.can_supply?
# so that it works correctly for variant overrides
def validate_quantity(line_item, quantity)
quantifier = Spree::Stock::Quantifier.new(line_item.variant_id)
return if quantifier.can_supply? quantity
line_item.scoper.scope(line_item.variant)

add_out_of_stock_error(line_item) unless line_item.variant.can_supply? quantity
end

def add_out_of_stock_error(line_item)
variant = line_item.variant
display_name = %Q{#{variant.name}}
display_name += %Q{ (#{variant.options_text})} unless variant.options_text.blank?
line_item.errors[:quantity] << Spree.t(:out_of_stock, :scope => :order_populator, :item => display_name.inspect)
display_name = %{#{variant.name}}
display_name += %{(#{variant.options_text})} if variant.options_text.present?
line_item.errors[:quantity] << Spree.t(:out_of_stock,
scope: :order_populator,
item: display_name.inspect)
end
end
4 changes: 4 additions & 0 deletions app/models/spree/stock_location_decorator.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@
def move(variant, quantity, originator = nil)
variant.move(quantity, originator)
end

def fill_status(variant, quantity)
variant.fill_status(quantity)
end
end
8 changes: 0 additions & 8 deletions lib/open_food_network/scope_variant_to_hub.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@ def price_in(currency)
Spree::Price.new(amount: price, currency: currency)
end

# Old Spree has the same logic as here and doesn't need this override.
# But we need this to use VariantOverrides with Spree 2.0.
def in_stock?
return true unless Spree::Config[:track_inventory_levels]

on_demand || (count_on_hand > 0)
end

# Uses variant_override.count_on_hand instead of Stock::Quantifier.stock_items.count_on_hand
def total_on_hand
@variant_override.andand.count_on_hand || super
Expand Down
94 changes: 47 additions & 47 deletions spec/features/consumer/shopping/variant_overrides_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,90 +13,90 @@
let(:outgoing_exchange) { oc.exchanges.outgoing.first }
let(:sm) { hub.shipping_methods.first }
let(:pm) { hub.payment_methods.first }
let(:p1) { create(:simple_product, supplier: producer) }
let(:p2) { create(:simple_product, supplier: producer) }
let(:p3) { create(:simple_product, supplier: producer, on_demand: true) }
let(:v1) { create(:variant, product: p1, price: 11.11, unit_value: 1) }
let(:v2) { create(:variant, product: p1, price: 22.22, unit_value: 2) }
let(:v3) { create(:variant, product: p2, price: 33.33, unit_value: 3) }
let(:v4) { create(:variant, product: p1, price: 44.44, unit_value: 4) }
let(:v5) { create(:variant, product: p3, price: 55.55, unit_value: 5, on_demand: true) }
let(:v6) { create(:variant, product: p3, price: 66.66, unit_value: 6, on_demand: true) }
let!(:vo1) { create(:variant_override, hub: hub, variant: v1, price: 55.55, count_on_hand: nil, default_stock: nil, resettable: false) }
let!(:vo2) { create(:variant_override, hub: hub, variant: v2, count_on_hand: 0, default_stock: nil, resettable: false) }
let!(:vo3) { create(:variant_override, hub: hub, variant: v3, count_on_hand: 0, default_stock: nil, resettable: false) }
let!(:vo4) { create(:variant_override, hub: hub, variant: v4, count_on_hand: 3, default_stock: nil, resettable: false) }
let!(:vo5) { create(:variant_override, hub: hub, variant: v5, count_on_hand: 0, default_stock: nil, resettable: false) }
let!(:vo6) { create(:variant_override, hub: hub, variant: v6, count_on_hand: 6, default_stock: nil, resettable: false) }
let(:ef) { create(:enterprise_fee, enterprise: hub, fee_type: 'packing', calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 10)) }
let(:product1) { create(:simple_product, supplier: producer) }
let(:product2) { create(:simple_product, supplier: producer) }
let(:product3) { create(:simple_product, supplier: producer, on_demand: true) }
let(:product1_variant1) { create(:variant, product: product1, price: 11.11, unit_value: 1) }
let(:product1_variant2) { create(:variant, product: product1, price: 22.22, unit_value: 2) }
let(:product2_variant1) { create(:variant, product: product2, price: 33.33, unit_value: 3) }
let(:product1_variant3) { create(:variant, product: product1, price: 44.44, unit_value: 4) }
let(:product3_variant1) { create(:variant, product: product3, price: 55.55, unit_value: 5, on_demand: true) }
let(:product3_variant2) { create(:variant, product: product3, price: 66.66, unit_value: 6, on_demand: true) }
let!(:product1_variant1_override) { create(:variant_override, hub: hub, variant: product1_variant1, price: 55.55, count_on_hand: nil, default_stock: nil, resettable: false) }
let!(:product1_variant2_override) { create(:variant_override, hub: hub, variant: product1_variant2, count_on_hand: 0, default_stock: nil, resettable: false) }
let!(:product2_variant1_override) { create(:variant_override, hub: hub, variant: product2_variant1, count_on_hand: 0, default_stock: nil, resettable: false) }
let!(:product1_variant3_override) { create(:variant_override, hub: hub, variant: product1_variant3, count_on_hand: 3, default_stock: nil, resettable: false) }
let!(:product3_variant1_override) { create(:variant_override, hub: hub, variant: product3_variant1, count_on_hand: 0, default_stock: nil, resettable: false) }
let!(:product3_variant2_override) { create(:variant_override, hub: hub, variant: product3_variant2, count_on_hand: 6, default_stock: nil, resettable: false) }
let(:enterprise_fee) { create(:enterprise_fee, enterprise: hub, fee_type: 'packing', calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 10)) }

before do
outgoing_exchange.variants = [v1, v2, v3, v4, v5, v6]
outgoing_exchange.enterprise_fees << ef
outgoing_exchange.variants = [product1_variant1, product1_variant2, product2_variant1, product1_variant3, product3_variant1, product3_variant2]
outgoing_exchange.enterprise_fees << enterprise_fee
sm.calculator.preferred_amount = 0
visit shops_path
click_link hub.name
end

describe "viewing products" do
it "shows the overridden price" do
page.should_not have_price with_currency(12.22) # $11.11 + 10% fee
page.should have_price with_currency(61.11)
page.should_not have_price with_currency(12.22) # product1_variant1.price ($11.11) + 10% fee
page.should have_price with_currency(61.11) # product1_variant1_override.price ($55.55) + 10% fee
end

it "looks up stock from the override" do
# Product should appear but one of the variants is out of stock
page.should_not have_content v2.options_text
page.should_not have_content product1_variant2.options_text

# Entire product should not appear - no stock
page.should_not have_content p2.name
page.should_not have_content v3.options_text
page.should_not have_content product2.name
page.should_not have_content product2_variant1.options_text

# On-demand product with VO of no stock should NOT appear
page.should_not have_content v5.options_text
page.should_not have_content product3_variant1.options_text
end

it "calculates fees correctly" do
page.find("#variant-#{v1.id} .graph-button").click
page.find("#variant-#{product1_variant1.id} .graph-button").click
page.find(".price_breakdown a").click
page.should have_selector 'li.cost div', text: with_currency(55.55)
page.should have_selector 'li.packing-fee div', text: with_currency(5.56)
page.should have_selector 'li.total div', text: "= #{with_currency(61.11)}"
end

it "shows the correct prices when products are in the cart" do
fill_in "variants[#{v1.id}]", with: "2"
fill_in "variants[#{product1_variant1.id}]", with: "2"
show_cart
wait_until_enabled 'li.cart a.button'
visit shop_path
page.should_not have_price with_currency(12.22)
page.should have_price with_currency(61.11)
end

# The two specs below reveal an unrelated issue with fee calculation. See:
# https://github.com/openfoodfoundation/openfoodnetwork/issues/312

it "shows the overridden price with fees in the quick cart" do
fill_in "variants[#{v1.id}]", with: "2"
fill_in "variants[#{product1_variant1.id}]", with: "2"
show_cart
page.should have_selector "#cart-variant-#{v1.id} .quantity", text: '2'
page.should have_selector "#cart-variant-#{v1.id} .price", text: with_currency(61.11)
page.should have_selector "#cart-variant-#{v1.id} .total-price", text: with_currency(122.22)
page.should have_selector "#cart-variant-#{product1_variant1.id} .quantity", text: '2'
page.should have_selector "#cart-variant-#{product1_variant1.id} .price", text: with_currency(61.11)
page.should have_selector "#cart-variant-#{product1_variant1.id} .total-price", text: with_currency(122.22)
end

it "shows the correct prices in the shopping cart" do
fill_in "variants[#{v1.id}]", with: "2"
fill_in "variants[#{product1_variant1.id}]", with: "2"
add_to_cart

page.should have_selector "tr.line-item.variant-#{v1.id} .cart-item-price", text: with_currency(61.11)
page.should have_selector "tr.line-item.variant-#{product1_variant1.id} .cart-item-price", text: with_currency(61.11)
page.should have_field "order[line_items_attributes][0][quantity]", with: '2'
page.should have_selector "tr.line-item.variant-#{v1.id} .cart-item-total", text: with_currency(122.22)
page.should have_selector "tr.line-item.variant-#{product1_variant1.id} .cart-item-total", text: with_currency(122.22)

page.should have_selector "#edit-cart .item-total", text: with_currency(122.22)
page.should have_selector "#edit-cart .grand-total", text: with_currency(122.22)
end

it "shows the correct prices in the checkout" do
fill_in "variants[#{v1.id}]", with: "2"
fill_in "variants[#{product1_variant1.id}]", with: "2"
click_checkout

page.should have_selector 'form.edit_order .cart-total', text: with_currency(122.22)
Expand All @@ -108,7 +108,7 @@

describe "creating orders" do
it "creates the order with the correct prices" do
fill_in "variants[#{v1.id}]", with: "2"
fill_in "variants[#{product1_variant1.id}]", with: "2"
click_checkout

complete_checkout
Expand All @@ -119,39 +119,39 @@
end

it "subtracts stock from the override" do
fill_in "variants[#{v4.id}]", with: "2"
fill_in "variants[#{product1_variant3.id}]", with: "2"
click_checkout

expect do
expect do
complete_checkout
end.to change { v4.reload.count_on_hand }.by(0)
end.to change { vo4.reload.count_on_hand }.by(-2)
end.to change { product1_variant3.reload.count_on_hand }.by(0)
end.to change { product1_variant3_override.reload.count_on_hand }.by(-2)
end

it "subtracts stock from stock-overridden on_demand variants" do
fill_in "variants[#{v6.id}]", with: "2"
fill_in "variants[#{product3_variant2.id}]", with: "2"
click_checkout

expect do
expect do
complete_checkout
end.to change { v6.reload.count_on_hand }.by(0)
end.to change { vo6.reload.count_on_hand }.by(-2)
end.to change { product3_variant2.reload.count_on_hand }.by(0)
end.to change { product3_variant2_override.reload.count_on_hand }.by(-2)
end

it "does not subtract stock from overrides that do not override count_on_hand" do
fill_in "variants[#{v1.id}]", with: "2"
fill_in "variants[#{product1_variant1.id}]", with: "2"
click_checkout
expect do
complete_checkout
end.to change { v1.reload.count_on_hand }.by(-2)
vo1.reload.count_on_hand.should be_nil
end.to change { product1_variant1.reload.count_on_hand }.by(-2)
product1_variant1_override.reload.count_on_hand.should be_nil
end

it "does not show out of stock flags on order confirmation page" do
v4.update_attribute :count_on_hand, 0
fill_in "variants[#{v4.id}]", with: "2"
product1_variant3.update_attribute :count_on_hand, 0
fill_in "variants[#{product1_variant3.id}]", with: "2"
click_checkout

complete_checkout
Expand Down

0 comments on commit 1007add

Please sign in to comment.