Refactor variant override model and its spec: simplify move_stock and remove unused methods

app/models/variant_override.rb

@@ -33,53 +33,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 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
+
     if quantity > 0
-      increment_stock! quantity
-    elsif quantity < 0
-      decrement_stock! -quantity
-    end
-  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."
-    end
-  end
-
-  def increment_stock!(quantity)
-    if stock_overridden?
       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
 
@@ -101,10 +68,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

spec/models/variant_override_spec.rb

@@ -51,74 +51,58 @@ 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
+      variant_override.price.should == 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
+        variant_override.stock_overridden?.should 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
+        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
+      variant_override.count_on_hand.should == 12
+    end
+
+    describe "stock_overridden?" do
+      it "returns true" do
+        variant_override.stock_overridden?.should be true
+      end
+    end
+
+    describe "move_stock!" do
+      it "does nothing for quantity zero" do
+        variant_override.move_stock! 0
+        variant_override.reload.count_on_hand.should == 12
+      end
+
+      it "increments count_on_hand when quantity is negative" do
+        variant_override.move_stock! 2
+        variant_override.reload.count_on_hand.should == 14
+      end
+
+      it "decrements count_on_hand when quantity is negative" do
+        variant_override.move_stock! -2
+        variant_override.reload.count_on_hand.should == 10
       end
     end
   end