From 6d39cc39c6d87bd9be52f0151ec7b75a524dd011 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 28 Jan 2016 15:17:17 +1100
Subject: [PATCH] When a variant is destroyed, update product cache

---
 app/models/spree/variant_decorator.rb         | 29 ++++++++++++-------
 lib/open_food_network/products_cache.rb       | 11 +++++++
 .../open_food_network/products_cache_spec.rb  | 19 ++++++++++++
 3 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index 36a4d0e45a..143508185a 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -10,7 +10,7 @@ Spree::Variant.class_eval do
   include OpenFoodNetwork::VariantAndLineItemNaming
 
 
-  has_many :exchange_variants, dependent: :destroy
+  has_many :exchange_variants
   has_many :exchanges, through: :exchange_variants
   has_many :variant_overrides
 
@@ -26,16 +26,7 @@ Spree::Variant.class_eval do
   before_validation :update_weight_from_unit_value, if: -> v { v.product.present? }
   after_save :update_units
   after_save :refresh_products_cache
-  around_destroy :refresh_products_cache_from_destroy
-
-  def refresh_products_cache
-    OpenFoodNetwork::ProductsCache.variant_changed self
-  end
-
-  def refresh_products_cache_from_destroy
-    OpenFoodNetwork::ProductsCache.variant_destroyed(self) { yield }
-  end
-
+  around_destroy :destruction
 
 
   scope :with_order_cycles_inner, joins(exchanges: :order_cycle)
@@ -99,4 +90,20 @@ Spree::Variant.class_eval do
   def update_weight_from_unit_value
     self.weight = weight_from_unit_value if self.product.variant_unit == 'weight' && unit_value.present?
   end
+
+  def refresh_products_cache
+    OpenFoodNetwork::ProductsCache.variant_changed self
+  end
+
+  def destruction
+    OpenFoodNetwork::ProductsCache.variant_destroyed(self) do
+      # Remove this association here instead of using dependent: :destroy because
+      # dependent-destroy acts before this around_filter is called, so ProductsCache
+      # has no way of knowing which exchanges the variant was a member of.
+      exchange_variants.destroy_all
+
+      # Destroy the variant
+      yield
+    end
+  end
 end
diff --git a/lib/open_food_network/products_cache.rb b/lib/open_food_network/products_cache.rb
index e2d2753cc5..b5e83d7590 100644
--- a/lib/open_food_network/products_cache.rb
+++ b/lib/open_food_network/products_cache.rb
@@ -10,6 +10,17 @@ module OpenFoodNetwork
     end
 
 
+    def self.variant_destroyed(variant, &block)
+      exchanges = exchanges_featuring_variant(variant).to_a
+
+      block.call
+
+      exchanges.each do |exchange|
+        refresh_cache exchange.receiver, exchange.order_cycle
+      end
+    end
+
+
     private
 
     def self.exchanges_featuring_variant(variant)
diff --git a/spec/lib/open_food_network/products_cache_spec.rb b/spec/lib/open_food_network/products_cache_spec.rb
index c00fb16da0..afc54ae87b 100644
--- a/spec/lib/open_food_network/products_cache_spec.rb
+++ b/spec/lib/open_food_network/products_cache_spec.rb
@@ -51,6 +51,25 @@ module OpenFoodNetwork
       end
     end
 
+    describe "when a variant is destroyed" do
+      let(:variant) { create(:variant) }
+      let(:distributor) { create(:distributor_enterprise) }
+      let!(:oc) { create(:open_order_cycle, distributors: [distributor], variants: [variant]) }
+
+      it "refreshes the cache based on exchanges the variant was in before destruction" do
+        expect(ProductsCache).to receive(:refresh_cache).with(distributor, oc)
+        variant.destroy
+      end
+
+      it "performs the cache refresh after the variant has been destroyed" do
+        expect(ProductsCache).to receive(:refresh_cache).with(distributor, oc) do
+          expect(Spree::Variant.where(id: variant.id)).to be_empty
+        end
+
+        variant.destroy
+      end
+    end
+
     describe "refreshing the cache" do
       let(:distributor) { double(:distributor, id: 123) }
       let(:order_cycle) { double(:order_cycle, id: 456) }