diff --git a/app/controllers/spree/admin/variants_controller_decorator.rb b/app/controllers/spree/admin/variants_controller_decorator.rb index 999ca0b22c..a87155b300 100644 --- a/app/controllers/spree/admin/variants_controller_decorator.rb +++ b/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 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/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb index 36fd0463a2..8846d80e22 100644 --- a/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/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 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