From b7f969eed90f47e34500d6f0ed31fe45c4293938 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 9 Jul 2025 11:20:47 +1000 Subject: [PATCH] Move the inventory feature check to ScopeVariantToHub Per review, the check is done on the same enterprise as the one use to initialize ScopeVariantToHub. So it makes sense to move the actual feature check to ScopeVariantToHub#scope --- .../subscription_line_items_controller.rb | 4 +-- .../api/v0/shipments_controller.rb | 5 +--- app/models/order_cycle.rb | 7 +---- app/models/spree/line_item.rb | 15 ++-------- app/models/spree/shipment.rb | 5 +--- app/services/cart_service.rb | 5 +--- app/services/orders/factory_service.rb | 4 +-- app/services/variants_stock_levels.rb | 4 +-- .../services/order_management/stock/packer.rb | 6 +--- .../subscriptions/estimator.rb | 4 +-- lib/open_food_network/scope_variant_to_hub.rb | 4 +-- lib/spree/core/controller_helpers/order.rb | 6 +--- .../scope_variant_to_hub_spec.rb | 28 +++++++++---------- spec/models/spree/line_item_spec.rb | 2 +- 14 files changed, 30 insertions(+), 69 deletions(-) diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb index 172445836e..b417cbb944 100644 --- a/app/controllers/admin/subscription_line_items_controller.rb +++ b/app/controllers/admin/subscription_line_items_controller.rb @@ -54,9 +54,7 @@ module Admin fee_calculator = OpenFoodNetwork::EnterpriseFeeCalculator.new(@shop, @order_cycle) - OpenFoodNetwork::ScopeVariantToHub.new(@shop).scope( - @variant, inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, @shop) - ) + OpenFoodNetwork::ScopeVariantToHub.new(@shop).scope(@variant) @variant.price + fee_calculator.indexed_fees_for(@variant) end diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb index be14069b5e..7f49dce13d 100644 --- a/app/controllers/api/v0/shipments_controller.rb +++ b/app/controllers/api/v0/shipments_controller.rb @@ -113,10 +113,7 @@ module Api def scoped_variant(variant_id) variant = Spree::Variant.find(variant_id) - OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope( - variant, - inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, @order.distributor) - ) + OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope(variant) variant end diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 91967e6bdc..06fcb3fe74 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -290,12 +290,7 @@ class OrderCycle < ApplicationRecord items = Spree::LineItem.includes(:variant).joins(:order).merge(orders) scoper = OpenFoodNetwork::ScopeVariantToHub.new(distributor) - items.each do |li| - scoper.scope( - li.variant, - inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, distributor) - ) - end + items.each { |li| scoper.scope(li.variant) } items end diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index 18fcd3f5ac..fed86cd802 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -164,7 +164,7 @@ module Spree return true if skip_stock_check return true if quantity <= 0 - scope_variant + scoper.scope(variant) variant.can_supply?(quantity) end @@ -177,7 +177,7 @@ module Spree end def cap_quantity_at_stock! - scope_variant + scoper.scope(variant) return if variant.on_demand update!(quantity: variant.on_hand) if quantity > variant.on_hand @@ -263,7 +263,7 @@ module Spree def update_inventory return unless changed? - scope_variant + scoper.scope(variant) Spree::OrderInventory.new(order).verify(self, target_shipment) end @@ -294,14 +294,5 @@ module Spree self.final_weight_volume = variant&.unit_value&.* quantity end end - - def scope_variant - scoper.scope( - variant, - inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, order.distributor) - ) - - variant - end end end diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb index 8b664e2dc3..d4de70754c 100644 --- a/app/models/spree/shipment.rb +++ b/app/models/spree/shipment.rb @@ -195,10 +195,7 @@ module Spree states = {} units.group_by(&:state).each { |state, iu| states[state] = iu.count } - scoper.scope( - variant, - inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, order.distributor) - ) + scoper.scope(variant) OpenStruct.new(variant:, quantity: units.length, states:) end diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb index 0cbd3acfb5..07c38b8b72 100644 --- a/app/services/cart_service.rb +++ b/app/services/cart_service.rb @@ -59,10 +59,7 @@ class CartService end def attempt_cart_add(variant, quantity, max_quantity = nil) - scoper.scope( - variant, - inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, order.distributor) - ) + scoper.scope(variant) return unless valid_variant?(variant) diff --git a/app/services/orders/factory_service.rb b/app/services/orders/factory_service.rb index 8e3066f0cb..3fb8d77540 100644 --- a/app/services/orders/factory_service.rb +++ b/app/services/orders/factory_service.rb @@ -51,9 +51,7 @@ module Orders attrs[:line_items].each do |li| next unless variant = Spree::Variant.find_by(id: li[:variant_id]) - scoper.scope( - variant, inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, shop) - ) + scoper.scope(variant) li[:quantity] = stock_limited_quantity(variant.on_demand, variant.on_hand, li[:quantity]) li[:price] = variant.price diff --git a/app/services/variants_stock_levels.rb b/app/services/variants_stock_levels.rb index 8abdf3990f..454658b220 100644 --- a/app/services/variants_stock_levels.rb +++ b/app/services/variants_stock_levels.rb @@ -39,9 +39,7 @@ class VariantsStockLevels def scoped_variant(distributor, variant) return variant if distributor.blank? - scoper(distributor).scope( - variant, inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, distributor) - ) + scoper(distributor).scope(variant) variant end diff --git a/engines/order_management/app/services/order_management/stock/packer.rb b/engines/order_management/app/services/order_management/stock/packer.rb index 0979dcf532..dc8c462783 100644 --- a/engines/order_management/app/services/order_management/stock/packer.rb +++ b/engines/order_management/app/services/order_management/stock/packer.rb @@ -15,11 +15,7 @@ module OrderManagement variant = line_item.variant next unless variant.stock_item - OpenFoodNetwork::ScopeVariantToHub.new(order.distributor).scope( - variant, - inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, - order.distributor) - ) + OpenFoodNetwork::ScopeVariantToHub.new(order.distributor).scope(variant) on_hand, backordered = variant.fill_status(line_item.quantity) package.add variant, on_hand, :on_hand if on_hand.positive? diff --git a/engines/order_management/app/services/order_management/subscriptions/estimator.rb b/engines/order_management/app/services/order_management/subscriptions/estimator.rb index ac1fef8638..e9eb76f10a 100644 --- a/engines/order_management/app/services/order_management/subscriptions/estimator.rb +++ b/engines/order_management/app/services/order_management/subscriptions/estimator.rb @@ -34,9 +34,7 @@ module OrderManagement def price_estimate_for(variant, fallback) return fallback unless fee_calculator && variant - scoper.scope( - variant, inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, shop) - ) + scoper.scope(variant) fees = fee_calculator.indexed_fees_for(variant) (variant.price + fees).to_d diff --git a/lib/open_food_network/scope_variant_to_hub.rb b/lib/open_food_network/scope_variant_to_hub.rb index f9dee9a080..0b4e341162 100644 --- a/lib/open_food_network/scope_variant_to_hub.rb +++ b/lib/open_food_network/scope_variant_to_hub.rb @@ -7,8 +7,8 @@ module OpenFoodNetwork @variant_overrides = variant_overrides || VariantOverride.indexed(@hub) end - def scope(variant, inventory_enabled: true) - return unless inventory_enabled + def scope(variant) + return unless OpenFoodNetwork::FeatureToggle.enabled?(:inventory, @hub) variant.extend(OpenFoodNetwork::ScopeVariantToHub::ScopeVariantToHub) variant.instance_variable_set :@hub, @hub diff --git a/lib/spree/core/controller_helpers/order.rb b/lib/spree/core/controller_helpers/order.rb index 3076357af2..d2a1795e20 100644 --- a/lib/spree/core/controller_helpers/order.rb +++ b/lib/spree/core/controller_helpers/order.rb @@ -20,11 +20,7 @@ module Spree if order&.line_items.present? scoper = OpenFoodNetwork::ScopeVariantToHub.new(order.distributor) order.line_items.each do |li| - scoper.scope( - li.variant, - inventory_enabled: OpenFoodNetwork::FeatureToggle.enabled?(:inventory, - order.distributor) - ) + scoper.scope(li.variant) end end diff --git a/spec/lib/open_food_network/scope_variant_to_hub_spec.rb b/spec/lib/open_food_network/scope_variant_to_hub_spec.rb index a35d5efdd4..3aac9e3c4b 100644 --- a/spec/lib/open_food_network/scope_variant_to_hub_spec.rb +++ b/spec/lib/open_food_network/scope_variant_to_hub_spec.rb @@ -22,7 +22,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do } let(:scoper) { described_class.new(hub) } - describe "overriding price" do + describe "overriding price", feature: :inventory do it "returns the overridden price when one is present" do vo scoper.scope v @@ -35,7 +35,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do end end - describe "overriding price_in" do + describe "overriding price_in", feature: :inventory do it "returns the overridden price when one is present" do vo scoper.scope v @@ -48,7 +48,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do end end - describe "overriding stock levels" do + describe "overriding stock levels", feature: :inventory do it "returns the overridden stock level when one is present" do vo scoper.scope v @@ -60,7 +60,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do expect(v.on_hand).to eq(1) end - describe "overriding stock on an on_demand variant" do + describe "overriding stock on an on_demand variant", feature: :inventory do let(:v) { create(:variant, price: 11.11, on_demand: true) } it "clears on_demand when the stock is overridden" do @@ -81,7 +81,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do end end - describe "overriding on_demand" do + describe "overriding on_demand", feature: :inventory do context "when an override exists" do before { vo } @@ -124,7 +124,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do # in_stock? is indirectly overridden through can_supply? # can_supply? is indirectly overridden by on_demand and total_on_hand # these tests validate this chain is working correctly - describe "overriding in_stock?" do + describe "overriding in_stock?", feature: :inventory do before { v.on_demand = false } context "when an override exists" do @@ -173,7 +173,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do end end - describe "overriding #move" do + describe "overriding #move", feature: :inventory do context "when override is on_demand" do before do vo2 @@ -205,7 +205,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do end end - describe "overriding sku" do + describe "overriding sku", feature: :inventory do context "when an override exists" do before { vo } @@ -233,13 +233,13 @@ RSpec.describe OpenFoodNetwork::ScopeVariantToHub do end end end + end - context "with inventory is disabled" do - it "doesn't override the variant" do - vo - scoper.scope(v, inventory_enabled: false) - expect(v.price).to eq(11.11) - end + context "with inventory is disabled" do + it "doesn't override the variant" do + vo + scoper.scope(v) + expect(v.price).to eq(11.11) end end end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 885cbde7e8..eb4d345ce5 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -343,7 +343,7 @@ RSpec.describe Spree::LineItem do let!(:line_item) { order.reload.line_items.first } let!(:variant) { line_item.variant } - context "when a variant override applies" do + context "when a variant override applies", feature: :inventory do let!(:vo) { create(:variant_override, hub: shop, variant:, count_on_hand: 3 ) } it "draws stock from the variant override" do