Skip to content

Commit

Permalink
VariantOverride with on_demand now overriding stock
Browse files Browse the repository at this point in the history
Otherwise we would try to take stock from the producer stock level
without respecting their on-demand settings. So from now on:
If stock level or on_demand are set on the override then it's not using
producer stock levels.
  • Loading branch information
mkllnk committed Aug 2, 2024
1 parent b6c4079 commit 2201d2e
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 21 deletions.
2 changes: 1 addition & 1 deletion app/models/concerns/stock_settings_override_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# | on_demand | count_on_hand | stock_overridden? | use_producer_stock_settings? | valid? |
# |-----------|---------------|-------------------|------------------------------|--------|
# | 1 | nil | false | false | true |
# | 1 | nil | true | false | true |
# | 0 | x | true | false | true |
# | nil | nil | false | true | true |
# | 1 | x | true | false | true |
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/variant_stock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def can_supply?(quantity)
# 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)
on_hand = if total_on_hand >= quantity || on_demand
on_hand = if total_on_hand.to_i >= quantity || on_demand
quantity
else
[0, total_on_hand].max
Expand Down
5 changes: 2 additions & 3 deletions app/models/variant_override.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ def self.indexed(hub)
end

def stock_overridden?
# If count_on_hand is present, it means on_demand is false
# See StockSettingsOverrideValidation for details
count_on_hand.present?
# Testing for not nil because for a boolean `false.present?` is false.
!on_demand.nil? || !count_on_hand.nil?
end

def use_producer_stock_settings?
Expand Down
5 changes: 2 additions & 3 deletions spec/lib/open_food_network/scope_variant_to_hub_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,10 @@ module OpenFoodNetwork
scoper.scope v2
end

it "doesn't reduce variant's stock" do
pending "updating override stock"

it "reduces override stock, not variant's stock" do
v2.move(-2)
expect(Spree::Variant.find(v2.id).on_hand).to eq 5
expect(v2.on_hand).to eq(-2)
end
end

Expand Down
6 changes: 2 additions & 4 deletions spec/models/spree/variant_stock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,14 @@
end

it "doesn't fail on negative stock when on demand" do
pending "https://github.com/openfoodfoundation/openfoodnetwork/issues/12586"

override.update!(on_demand: true, count_on_hand: nil)

expect {
hub_variant.move(-8)
override.reload
}
.to change { override.count_on_hand }.from(7).to(-1)
.and change { hub_variant.on_hand }.from(7).to(-1)
.to change { override.count_on_hand }.from(nil).to(-8)
.and change { hub_variant.on_hand }.from(nil).to(-8)
.and change { variant.on_hand }.by(0)
end
end
Expand Down
13 changes: 8 additions & 5 deletions spec/models/variant_override_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@

describe "with nil count on hand" do
let(:variant_override) do
build_stubbed(
build(
:variant_override,
variant: build_stubbed(:variant),
hub: build_stubbed(:distributor_enterprise),
Expand All @@ -175,15 +175,18 @@
end

describe "stock_overridden?" do
it "returns false" do
expect(variant_override.stock_overridden?).to be false
it "returns true" do
expect(variant_override.stock_overridden?).to be true
end
end

describe "move_stock!" do
it "silently logs an error" do
expect(Bugsnag).to receive(:notify)
variant_override.move_stock!(5)
expect {
variant_override.move_stock!(5)
}.to change {
variant_override.count_on_hand
}.from(nil).to(5)
end
end
end
Expand Down
6 changes: 2 additions & 4 deletions spec/system/consumer/shopping/variant_overrides_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,13 @@
expect(product1_variant1_override.reload.count_on_hand).to be_nil
end

it "does not subtract stock from variants where the override has on_demand: true" do
pending "update override stock"

it "subtracts stock from override but not variants where the override has on_demand: true" do
click_add_to_cart product4_variant1, 2
click_checkout
expect do
complete_checkout
end.to change { product4_variant1.reload.on_hand }.by(0)
expect(product4_variant1_override.reload.count_on_hand).to be_nil
expect(product4_variant1_override.reload.count_on_hand).to eq(-2)
end

it "does not show out of stock flags on order confirmation page" do
Expand Down

0 comments on commit 2201d2e

Please sign in to comment.