raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-01-24 20:36:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3147 from luisramos0/2-0-vos

Browse Source 
[Spree Upgrade] Adapt variant overrides to spree 2
This commit is contained in:
Maikel
2018-12-05 10:50:20 +11:00
committed by GitHub
parent 30e7d32cea 02d0b7e88c
commit a2c8bea147
6 changed files with 98 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
.rubocop_todo.yml
View File

@@ -245,7 +245,6 @@ Layout/EmptyLines:
- 'spec/models/spree/product_spec.rb'
- 'spec/models/spree/shipping_method_spec.rb'
- 'spec/models/spree/variant_spec.rb'
- 'spec/models/variant_override_spec.rb'
- 'spec/serializers/admin/exchange_serializer_spec.rb'
- 'spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb'
- 'spec/serializers/admin/for_order_cycle/supplied_product_serializer_spec.rb'
@@ -389,7 +388,6 @@ Layout/ExtraSpacing:
- 'spec/models/spree/adjustment_spec.rb'
- 'spec/models/spree/gateway/stripe_connect_spec.rb'
- 'spec/models/spree/order_spec.rb'
- 'spec/models/variant_override_spec.rb'
- 'spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb'
- 'spec/serializers/admin/for_order_cycle/supplied_product_serializer_spec.rb'
- 'spec/spec_helper.rb'
@@ -557,7 +555,6 @@ Layout/SpaceAfterColon:
- 'spec/features/admin/variants_spec.rb'
- 'spec/features/consumer/account_spec.rb'
- 'spec/models/spree/ability_spec.rb'
- 'spec/models/variant_override_spec.rb'
- 'spec/spec_helper.rb'
# Offense count: 83
@@ -870,7 +867,6 @@ Layout/SpaceInsideHashLiteralBraces:
- 'spec/models/spree/taxon_spec.rb'
- 'spec/models/spree/variant_spec.rb'
- 'spec/models/tag_rule/discount_order_spec.rb'
- 'spec/models/variant_override_spec.rb'
- 'spec/performance/orders_controller_spec.rb'
- 'spec/requests/checkout/failed_checkout_spec.rb'
- 'spec/requests/checkout/stripe_connect_spec.rb'
@@ -965,7 +961,6 @@ Lint/DuplicateMethods:
Lint/IneffectiveAccessModifier:
Exclude:
- 'app/models/column_preference.rb'
- 'app/models/variant_override.rb'
- 'lib/open_food_network/feature_toggle.rb'
- 'lib/open_food_network/products_cache.rb'
- 'lib/open_food_network/property_merge.rb'
@@ -1130,7 +1125,6 @@ Lint/Void:
- 'spec/models/spree/payment_spec.rb'
- 'spec/models/spree/product_spec.rb'
- 'spec/models/spree/variant_spec.rb'
- 'spec/models/variant_override_spec.rb'
- 'spec/serializers/enterprise_serializer_spec.rb'
- 'spec/support/request/web_helper.rb'
@@ -1522,7 +1516,6 @@ Rails/TimeZone:
- 'spec/lib/open_food_network/products_cache_refreshment_spec.rb'
- 'spec/lib/open_food_network/products_cache_spec.rb'
- 'spec/models/enterprise_relationship_spec.rb'
- 'spec/models/variant_override_spec.rb'
# Offense count: 1
# Configuration parameters: Environments.
@@ -1545,7 +1538,6 @@ Rails/Validation:
- 'app/models/order_cycle.rb'
- 'app/models/product_distribution.rb'
- 'app/models/spree/product_decorator.rb'
- 'app/models/variant_override.rb'
# Offense count: 18
# Cop supports --auto-correct.
@@ -2283,7 +2275,6 @@ Style/RedundantSelf:
- 'app/models/spree/taxon_decorator.rb'
- 'app/models/spree/user_decorator.rb'
- 'app/models/spree/variant_decorator.rb'
- 'app/models/variant_override.rb'
- 'lib/open_food_network/locking.rb'
- 'lib/open_food_network/rack_request_blocker.rb'
- 'lib/open_food_network/reports/report.rb'

19
app/models/concerns/variant_stock.rb
View File

@@ -109,6 +109,20 @@ module VariantStock
end
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
def move(quantity, originator = nil)
raise_error_if_no_stock_item_available
# Creates a stock movement: it updates stock_item.count_on_hand and fills backorders
#
# This is the original Spree::StockLocation#move,
# except that we raise an error if the stock item is missing,
# because, unlike Spree, we should always have exactly one stock item per variant.
stock_item.stock_movements.create!(quantity: quantity, originator: originator)
end
private
# Persists the single stock item associated to this variant. As defined in
@@ -126,7 +140,10 @@ module VariantStock
raise message if stock_items.empty?
end
# Backwards compatible setting of stock levels in Spree 2.0.
# Overwrites stock_item.count_on_hand
#
# Calling stock_item.adjust_count_on_hand will bypass filling backorders and creating stock movements
# If that was required we could call self.move
def overwrite_stock_levels(new_level)
stock_item.adjust_count_on_hand(new_level - stock_item.count_on_hand)
end

5
app/models/spree/stock_location_decorator.rb Normal file
View File

@@ -0,0 +1,5 @@
Spree::StockLocation.class_eval do
def move(variant, quantity, originator = nil)
variant.move(quantity, originator)
end
end

48
app/models/variant_override.rb
View File

@@ -6,7 +6,8 @@ class VariantOverride < ActiveRecord::Base
belongs_to :hub, class_name: 'Enterprise'
belongs_to :variant, class_name: 'Spree::Variant'
validates_presence_of :hub_id, :variant_id
validates :hub_id, presence: true
validates :variant_id, presence: true
# Default stock can be nil, indicating stock should not be reset or zero, meaning reset to zero. Need to ensure this can be set by the user.
validates :default_stock, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true
@@ -33,45 +34,20 @@ class VariantOverride < ActiveRecord::Base
]
end
def self.price_for(hub, variant)
self.for(hub, variant).andand.price
end
def self.count_on_hand_for(hub, variant)
self.for(hub, variant).andand.count_on_hand
end
def self.stock_overridden?(hub, variant)
count_on_hand_for(hub, variant).present?
end
def self.decrement_stock!(hub, variant, quantity)
vo = self.for(hub, variant)
if vo.nil?
Bugsnag.notify RuntimeError.new "Attempting to decrement stock level for a variant without a VariantOverride."
else
vo.decrement_stock! quantity
end
end
def stock_overridden?
count_on_hand.present?
end
def decrement_stock!(quantity)
if stock_overridden?
decrement! :count_on_hand, quantity
else
Bugsnag.notify RuntimeError.new "Attempting to decrement stock level on a VariantOverride without a count_on_hand specified."
def move_stock!(quantity)
unless stock_overridden?
Bugsnag.notify RuntimeError.new "Attempting to move stock of a VariantOverride without a count_on_hand specified."
return
end
end
def increment_stock!(quantity)
if stock_overridden?
if quantity > 0
increment! :count_on_hand, quantity
else
Bugsnag.notify RuntimeError.new "Attempting to decrement stock level on a VariantOverride without a count_on_hand specified."
elsif quantity < 0
decrement! :count_on_hand, -quantity
end
end
@@ -83,7 +59,7 @@ class VariantOverride < ActiveRecord::Base
if resettable
if default_stock?
self.attributes = { count_on_hand: default_stock }
self.save
save
else
Bugsnag.notify RuntimeError.new "Attempting to reset stock level for a variant with no default stock level."
end
@@ -93,10 +69,6 @@ class VariantOverride < ActiveRecord::Base
private
def self.for(hub, variant)
VariantOverride.where(variant_id: variant, hub_id: hub).first
end
def refresh_products_cache_from_save
OpenFoodNetwork::ProductsCache.variant_override_changed self
end

21
lib/open_food_network/scope_variant_to_hub.rb
View File

@@ -28,7 +28,8 @@ module OpenFoodNetwork
on_demand || (count_on_hand > 0)
end
def count_on_hand
# 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
end
@@ -46,17 +47,13 @@ module OpenFoodNetwork
end
end
def decrement!(attribute, by = 1)
if attribute == :count_on_hand && @variant_override.andand.stock_overridden?
@variant_override.decrement_stock! by
else
super
end
end
def increment!(attribute, by = 1)
if attribute == :count_on_hand && @variant_override.andand.stock_overridden?
@variant_override.increment_stock! by
# If it is an variant override with a count_on_hand value:
# - updates variant_override.count_on_hand
# - does not create stock_movement
# - does not update stock_item.count_on_hand
def move(quantity, originator = nil)
if @variant_override.andand.stock_overridden?
@variant_override.move_stock! quantity
else
super
end

129
spec/models/variant_override_spec.rb
View File

@@ -9,7 +9,7 @@ describe VariantOverride do
let(:hub2) { create(:distributor_enterprise) }
let!(:vo1) { create(:variant_override, hub: hub1, variant: variant, import_date: Time.zone.now.yesterday) }
let!(:vo2) { create(:variant_override, hub: hub2, variant: variant, import_date: Time.zone.now) }
let!(:vo3) { create(:variant_override, hub: hub1, variant: variant, permission_revoked_at: Time.now) }
let!(:vo3) { create(:variant_override, hub: hub1, variant: variant, permission_revoked_at: Time.zone.now) }
it "ignores variant_overrides with revoked_permissions by default" do
expect(VariantOverride.all).to_not include vo3
@@ -17,7 +17,7 @@ describe VariantOverride do
end
it "finds variant overrides for a set of hubs" do
VariantOverride.for_hubs([hub1, hub2]).should match_array [vo1, vo2]
expect(VariantOverride.for_hubs([hub1, hub2])).to match_array [vo1, vo2]
end
it "fetches import dates for hubs in descending order" do
@@ -29,13 +29,12 @@ describe VariantOverride do
describe "fetching variant overrides indexed by variant" do
it "gets indexed variant overrides for one hub" do
VariantOverride.indexed(hub1).should == {variant => vo1}
VariantOverride.indexed(hub2).should == {variant => vo2}
expect(VariantOverride.indexed(hub1)).to eq( variant => vo1 )
expect(VariantOverride.indexed(hub2)).to eq( variant => vo2 )
end
end
end
describe "callbacks" do
let!(:vo) { create(:variant_override, hub: hub, variant: variant) }
@@ -51,87 +50,71 @@ describe VariantOverride do
end
end
describe "with price" do
let(:variant_override) { create(:variant_override, variant: variant, hub: hub, price: 12.34) }
describe "looking up prices" do
it "returns the numeric price when present" do
VariantOverride.create!(variant: variant, hub: hub, price: 12.34)
VariantOverride.price_for(hub, variant).should == 12.34
end
it "returns nil otherwise" do
VariantOverride.price_for(hub, variant).should be_nil
it "returns the numeric price" do
expect(variant_override.price).to eq(12.34)
end
end
describe "looking up count on hand" do
it "returns the numeric stock level when present" do
VariantOverride.create!(variant: variant, hub: hub, count_on_hand: 12)
VariantOverride.count_on_hand_for(hub, variant).should == 12
end
describe "with nil count on hand" do
let(:variant_override) { create(:variant_override, variant: variant, hub: hub, count_on_hand: nil) }
it "returns nil otherwise" do
VariantOverride.count_on_hand_for(hub, variant).should be_nil
end
end
describe "checking if stock levels have been overriden" do
it "returns true when stock level has been overridden" do
create(:variant_override, variant: variant, hub: hub, count_on_hand: 12)
VariantOverride.stock_overridden?(hub, variant).should be true
end
it "returns false when the override has no stock level" do
create(:variant_override, variant: variant, hub: hub, count_on_hand: nil)
VariantOverride.stock_overridden?(hub, variant).should be false
end
it "returns false when there is no override for the hub/variant" do
VariantOverride.stock_overridden?(hub, variant).should be false
end
end
describe "decrementing stock" do
it "decrements stock" do
vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12)
VariantOverride.decrement_stock! hub, variant, 2
vo.reload.count_on_hand.should == 10
end
it "silently logs an error if the variant override does not exist" do
Bugsnag.should_receive(:notify)
VariantOverride.decrement_stock! hub, variant, 2
end
end
describe "incrementing stock" do
let!(:vo) { create(:variant_override, variant: variant, hub: hub, count_on_hand: 8) }
context "when the vo overrides stock" do
it "increments stock" do
vo.increment_stock! 2
vo.reload.count_on_hand.should == 10
describe "stock_overridden?" do
it "returns false" do
expect(variant_override.stock_overridden?).to be false
end
end
context "when the vo doesn't override stock" do
before { vo.update_attributes(count_on_hand: nil) }
describe "move_stock!" do
it "silently logs an error" do
Bugsnag.should_receive(:notify)
vo.increment_stock! 2
expect(Bugsnag).to receive(:notify)
variant_override.move_stock!(5)
end
end
end
describe "with count on hand" do
let(:variant_override) { create(:variant_override, variant: variant, hub: hub, count_on_hand: 12) }
it "returns the numeric count on hand" do
expect(variant_override.count_on_hand).to eq(12)
end
describe "stock_overridden?" do
it "returns true" do
expect(variant_override.stock_overridden?).to be true
end
end
describe "move_stock!" do
it "does nothing for quantity zero" do
variant_override.move_stock!(0)
expect(variant_override.reload.count_on_hand).to eq(12)
end
it "increments count_on_hand when quantity is negative" do
variant_override.move_stock!(2)
expect(variant_override.reload.count_on_hand).to eq(14)
end
it "decrements count_on_hand when quantity is negative" do
variant_override.move_stock!(-2)
expect(variant_override.reload.count_on_hand).to eq(10)
end
end
end
describe "checking default stock value is present" do
it "returns true when a default stock level has been set" do
it "returns true when a default stock level has been set" do
vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: 20)
vo.default_stock?.should be true
expect(vo.default_stock?).to be true
end
it "returns false when the override has no default stock level" do
vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock:nil)
vo.default_stock?.should be false
vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: nil)
expect(vo.default_stock?).to be false
end
end
@@ -139,18 +122,18 @@ describe VariantOverride do
it "resets the on hand level to the value in the default_stock field" do
vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: 20, resettable: true)
vo.reset_stock!
vo.reload.count_on_hand.should == 20
expect(vo.reload.count_on_hand).to eq(20)
end
it "silently logs an error if the variant override doesn't have a default stock level" do
vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock:nil, resettable: true)
Bugsnag.should_receive(:notify)
vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: nil, resettable: true)
expect(Bugsnag).to receive(:notify)
vo.reset_stock!
vo.reload.count_on_hand.should == 12
expect(vo.reload.count_on_hand).to eq(12)
end
it "doesn't reset the level if the behaviour is disabled" do
vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: 10, resettable: false)
vo.reset_stock!
vo.reload.count_on_hand.should == 12
expect(vo.reload.count_on_hand).to eq(12)
end
end