Revert "refactor product and variant model for restrict destroy if line items present. Change flash to show error of product if not destroyed in admin/products_controller"

This reverts commit f2e3447.

Author: damianlegawiec

backend/app/controllers/spree/admin/products_controller.rb

@@ -47,11 +47,10 @@ def destroy
 
         begin
           # TODO: why is @product.destroy raising ActiveRecord::RecordNotDestroyed instead of failing with false result
-          # Issue found for above comment: https://github.com/rails/rails/issues/19761
           if @product.destroy
             flash[:success] = Spree.t('notice_messages.product_deleted')
           else
-            flash[:error] = @object.errors.full_messages.join(', ')
+            flash[:error] = Spree.t('notice_messages.product_not_deleted')
           end
         rescue ActiveRecord::RecordNotDestroyed => e
           flash[:error] = Spree.t('notice_messages.product_not_deleted')

backend/spec/controllers/spree/admin/products_controller_spec.rb

@@ -62,13 +62,10 @@ def send_request
     end
 
     context 'will not successfully destroy product' do
-      let!(:error_message) { 'Test error' }
-
       before do
         allow(Spree::Product).to receive(:friendly).and_return(products)
         allow(products).to receive(:find).with(product.id.to_s).and_return(product)
         allow(product).to receive(:destroy).and_return(false)
-        allow(product).to receive_message_chain(:errors, :full_messages).and_return([error_message])
       end
 
       describe 'expects to receive' do
@@ -87,7 +84,7 @@ def send_request
       describe 'response' do
         before { send_request }
         it { expect(response).to have_http_status(:ok) }
-        it { expect(flash[:error]).to eq(error_message) }
+        it { expect(flash[:error]).to eq(Spree.t('notice_messages.product_not_deleted')) }
       end
     end
   end

core/app/models/spree/product.rb

@@ -88,6 +88,7 @@ class Product < Spree::Base
     after_save :run_touch_callbacks, if: :anything_changed?
     after_save :reset_nested_changes
     after_touch :touch_taxons
+    before_destroy :ensure_no_line_items
 
     before_validation :normalize_slug, on: :update
     before_validation :validate_master
@@ -356,6 +357,13 @@ def touch_taxons
       Spree::Taxonomy.where(id: taxonomy_ids).update_all(updated_at: Time.current)
     end
 
+    def ensure_no_line_items
+      if line_items.any?
+        errors.add(:base, Spree.t(:cannot_destroy_if_attached_to_line_items))
+        return false
+      end
+    end
+
     def remove_taxon(taxon)
       removed_classifications = classifications.where(taxon: taxon)
       removed_classifications.each &:remove_from_list

core/app/models/spree/variant.rb

@@ -14,11 +14,10 @@ class Variant < Spree::Base
 
     with_options inverse_of: :variant do
       has_many :inventory_units
+      has_many :line_items
       has_many :stock_items, dependent: :destroy
     end
 
-    has_many :line_items, dependent: :restrict_with_error
-
     has_many :orders, through: :line_items
     with_options through: :stock_items do
       has_many :stock_locations
@@ -49,6 +48,7 @@ class Variant < Spree::Base
 
     after_create :create_stock_items
     after_create :set_master_out_of_stock, unless: :is_master?
+    before_destroy :ensure_no_line_items
 
     after_touch :clear_in_stock_cache
 
@@ -258,6 +258,13 @@ def discontinued?
 
     private
 
+    def ensure_no_line_items
+      if line_items.any?
+        errors.add(:base, Spree.t(:cannot_destroy_if_attached_to_line_items))
+        return false
+      end
+    end
+
     def quantifier
       Spree::Stock::Quantifier.new(self)
     end

core/config/locales/en.yml

@@ -313,6 +313,10 @@ en:
           attributes:
             expires_at:
               invalid_date_range: must be later than start date
+        spree/product:
+          attributes:
+            base:
+              cannot_destroy_if_attached_to_line_items: Cannot delete products once they are attached to line items.
         spree/refund:
           attributes:
             amount:
@@ -346,6 +350,10 @@ en:
           attributes:
             base:
               cannot_destroy_if_attached_to_line_items: Cannot delete variants once they are attached to line items.
+        spree/shipping_method:
+          attributes:
+            base:
+              cannot_destroy_if_attached_to_line_items: Cannot delete variants once they are attached to line items.
 
   devise:
     confirmations: