Merge pull request #3639 from coopdevs/fix-variant-cache-refreshing-on-delete

Fix variant cache refreshing on delete

app/controllers/spree/admin/variants_controller_decorator.rb

@@ -10,12 +10,13 @@ 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|
       format.html { redirect_to admin_product_variants_url(params[:product_id]) }
-      format.js   { render_js_for_destroy }
+      format.js { render_js_for_destroy }
     end
   end
 

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

spec/controllers/spree/admin/variants_controller_spec.rb

@@ -35,6 +35,84 @@ module Spree
           assigns(:variants).should match_array [v1, v2]
         end
       end
+
+      describe '#destroy' do
+        let(:variant) { create(:variant) }
+
+        context 'when requesting with js' do
+          before do
+            allow(Spree::Variant).to receive(:find).with(variant.id.to_s) { variant }
+            allow(variant).to receive(:delete)
+          end
+
+          it 'deletes the variant' do
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js'
+            expect(variant).to have_received(:delete)
+          end
+
+          it 'shows a success flash message' do
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js'
+            expect(flash[:success]).to eq(I18n.t('notice_messages.variant_deleted'))
+          end
+
+          it 'renders spree/admin/shared/destroy' do
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js'
+            expect(response).to render_template('spree/admin/shared/_destroy')
+          end
+
+          it 'refreshes the cache' do
+            expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).with(variant)
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js'
+          end
+
+          it 'destroys all its exchanges' do
+            exchange = create(:exchange)
+            variant.exchanges << exchange
+
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js'
+            expect(variant.exchanges).to be_empty
+          end
+        end
+
+        context 'when requesting with html' do
+          before do
+            allow(Spree::Variant).to receive(:find).with(variant.id.to_s) { variant }
+            allow(variant).to receive(:delete)
+          end
+
+          it 'deletes the variant' do
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'html'
+            expect(variant).to have_received(:delete)
+          end
+
+          it 'shows a success flash message' do
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'html'
+            expect(flash[:success]).to eq(I18n.t('notice_messages.variant_deleted'))
+          end
+
+          it 'redirects to admin_product_variants_url' do
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'html'
+            expect(response).to redirect_to(
+              controller: 'spree/admin/variants',
+              action: :index,
+              product_id: variant.product.permalink
+            )
+          end
+
+          it 'refreshes the cache' do
+            expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).with(variant)
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js'
+          end
+
+          it 'destroys all its exchanges' do
+            exchange = create(:exchange)
+            variant.exchanges << exchange
+
+            spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js'
+            expect(variant.exchanges).to be_empty
+          end
+        end
+      end
     end
   end
 end

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