From 96b8c8ac2caa4a3ac4bf1120c3d130ac5d6f66ef Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 22 Mar 2019 09:47:35 +0100
Subject: [PATCH] Refresh products cache also on Variant#delete

Note that, as explained in
https://apidock.com/rails/v3.2.13/ActiveRecord/Relation/delete, `delete` does
not trigger callbacks and so it skips the products cache logic.

If we still want to avoid instantiating the AR object, we need to explicitly
call that logic for the cache to be up-to-date.
---
 .../admin/variants_controller_decorator.rb    |  3 ++-
 app/models/spree/variant_decorator.rb         |  5 ++++
 spec/models/spree/variant_spec.rb             | 23 ++++++++++++++++++-
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/app/controllers/spree/admin/variants_controller_decorator.rb b/app/controllers/spree/admin/variants_controller_decorator.rb
index f93c6fcafc..a87155b300 100644
--- a/app/controllers/spree/admin/variants_controller_decorator.rb
+++ b/app/controllers/spree/admin/variants_controller_decorator.rb
@@ -10,7 +10,8 @@ Spree::Admin::VariantsController.class_eval do
 
   def destroy
     @variant = Spree::Variant.find(params[:id])
-    @variant.delete # This line changed, as well as removal of following conditional
+    @variant.delete_and_refresh_cache
+
     flash[:success] = I18n.t('notice_messages.variant_deleted')
 
     respond_with(@variant) do |format|
diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index cefb7917da..69359c6851 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -118,6 +118,11 @@ Spree::Variant.class_eval do
     end
   end
 
+  # Deletes the record, skipping callbacks, but it also refreshes the cache
+  def delete_and_refresh_cache
+    destruction { delete }
+  end
+
   private
 
   def update_weight_from_unit_value
diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index fe4e132ae8..cc37f889ef 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -196,7 +196,6 @@ module Spree
         end
 
         it "refreshes the products cache for the entire product on destroy" do
-          # Does this ever happen?
           expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product)
           expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).never
           master.destroy
@@ -551,4 +550,26 @@ module Spree
       end
     end
   end
+
+  describe '#delete_and_refresh_cache' do
+    context 'when it is not the master variant' do
+      let(:variant) { create(:variant) }
+
+      it 'refreshes the products cache on delete' do
+        expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).with(variant)
+        variant.delete_and_refresh_cache
+      end
+    end
+
+    context "when it is the master variant" do
+      let(:product) { create(:simple_product) }
+      let(:master) { product.master }
+
+      it 'refreshes the products cache for the entire product on delete' do
+        expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product)
+        expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).never
+        master.delete_and_refresh_cache
+      end
+    end
+  end
 end