Merge pull request #13648 from deivid-rodriguez/improve-enterprise-removal

Improve enterprise removal

app/controllers/admin/enterprises_controller.rb

@@ -162,6 +162,18 @@ module Admin
       end
     end
 
+    def destroy
+      if @object.destroy
+        flash.now[:success] = flash_message_for(@object, :successfully_removed)
+      else
+        flash.now[:error] = @object.errors.full_messages.to_sentence
+      end
+
+      respond_to do |format|
+        format.turbo_stream { render :destroy, status: :ok }
+      end
+    end
+
     protected
 
     def delete_custom_tab

app/helpers/enterprises_helper.rb

@@ -63,8 +63,11 @@ module EnterprisesHelper
     url = object_url(enterprise)
     name = t(:delete)
     options = {}
-    options[:class] = "delete-resource"
-    options[:data] = { action: 'remove', confirm: enterprise_confirm_delete_message(enterprise) }
+    options[:data] = {
+      turbo: true,
+      'turbo-method': 'delete',
+      'turbo-confirm': enterprise_confirm_delete_message(enterprise)
+    }
     link_to_with_icon 'icon-trash', name, url, options
   end
 

app/models/enterprise.rb

@@ -50,11 +50,11 @@ class Enterprise < ApplicationRecord
   has_many :distributed_orders, class_name: 'Spree::Order',
                                 foreign_key: 'distributor_id',
                                 inverse_of: :distributor,
-                                dependent: :restrict_with_exception
+                                dependent: :restrict_with_error
 
   belongs_to :address, class_name: 'Spree::Address'
   belongs_to :business_address, optional: true, class_name: 'Spree::Address', dependent: :destroy
-  has_many :enterprise_fees, dependent: :restrict_with_exception
+  has_many :enterprise_fees, dependent: :restrict_with_error
   has_many :enterprise_roles, dependent: :destroy
   has_many :users, through: :enterprise_roles
   belongs_to :owner, class_name: 'Spree::User',
@@ -62,18 +62,18 @@ class Enterprise < ApplicationRecord
   has_many :distributor_payment_methods,
            inverse_of: :distributor,
            foreign_key: :distributor_id,
-           dependent: :restrict_with_exception
+           dependent: :restrict_with_error
   has_many :distributor_shipping_methods,
            inverse_of: :distributor,
            foreign_key: :distributor_id,
-           dependent: :restrict_with_exception
+           dependent: :restrict_with_error
   has_many :payment_methods, through: :distributor_payment_methods
   has_many :shipping_methods, through: :distributor_shipping_methods
   has_many :customers, dependent: :destroy
   has_many :inventory_items, dependent: :destroy
   has_many :tag_rules, dependent: :destroy
   has_one :stripe_account, dependent: :destroy
-  has_many :vouchers, dependent: :restrict_with_exception
+  has_many :vouchers, dependent: :restrict_with_error
   has_many :connected_apps, dependent: :destroy
   has_many :dfc_permissions, dependent: :destroy
   has_one :custom_tab, dependent: :destroy

app/views/admin/enterprises/_admin_index.html.haml

@@ -22,7 +22,7 @@
     %tbody
       = f.fields_for :collection do |enterprise_form|
         - enterprise = enterprise_form.object
-        %tr{class: "enterprise-#{enterprise.id}"}
+        %tr{class: "enterprise-#{enterprise.id}", id: "resource-#{enterprise.id}"}
           %td= link_to enterprise.name, main_app.edit_admin_enterprise_path(enterprise)
           %td
             = enterprise_form.check_box :is_primary_producer

app/views/admin/enterprises/destroy.turbo_stream.haml

@@ -0,0 +1,4 @@
+- unless flash[:error]
+  = turbo_stream.remove "resource-#{@object.id}"
+= turbo_stream.append "flashes" do
+  = render(partial: 'admin/shared/flashes', locals: { flashes: flash })

spec/models/enterprise_spec.rb

@@ -58,37 +58,40 @@ RSpec.describe Enterprise do
       expect(EnterpriseRelationship.where(id: [er1, er2])).to be_empty
     end
 
-    it "raises a DeleteRestrictionError on destroy if distributed_orders exist" do
+    it "does not destroy distributed_orders upon destroy" do
       enterprise = create(:distributor_enterprise)
       create_list(:order, 2, distributor: enterprise)
 
       expect do
         enterprise.destroy
-      end.to raise_error(ActiveRecord::DeleteRestrictionError,
-                         /Cannot delete record because of dependent distributed_orders/)
-        .and change { Spree::Order.count }.by(0)
+        expect(enterprise.errors.full_messages).to eq(
+          ["Cannot delete record because dependent distributed orders exist"]
+        )
+      end.to change { Spree::Order.count }.by(0)
     end
 
-    it "raises an DeleteRestrictionError on destroy if distributor_payment_methods exist" do
+    it "does not destroy distributor_payment_methods upon destroy" do
       enterprise = create(:distributor_enterprise)
       create_list(:distributor_payment_method, 2, distributor: enterprise)
 
       expect do
         enterprise.destroy
-      end.to raise_error(ActiveRecord::DeleteRestrictionError,
-                         /Cannot delete record because of dependent distributor_payment_methods/)
-        .and change { DistributorPaymentMethod.count }.by(0)
+        expect(enterprise.errors.full_messages).to eq(
+          ["Cannot delete record because dependent distributor payment methods exist"]
+        )
+      end.to change { Spree::Order.count }.by(0)
     end
 
-    it "raises an DeleteRestrictionError on destroy if distributor_shipping_methods exist" do
+    it "does not destroy distributor_shipping_methods upon destroy" do
       enterprise = create(:distributor_enterprise)
       create_list(:distributor_shipping_method, 2, distributor: enterprise)
 
       expect do
         enterprise.destroy
-      end.to raise_error(ActiveRecord::DeleteRestrictionError,
-                         /Cannot delete record because of dependent distributor_shipping_methods/)
-        .and change { DistributorShippingMethod.count }.by(0)
+        expect(enterprise.errors.full_messages).to eq(
+          ["Cannot delete record because dependent distributor shipping methods exist"]
+        )
+      end.to change { Spree::Order.count }.by(0)
     end
 
     it "does not destroy enterprise_fees upon destroy" do
@@ -97,9 +100,10 @@ RSpec.describe Enterprise do
 
       expect do
         enterprise.destroy
-      end.to raise_error(ActiveRecord::DeleteRestrictionError,
-                         /Cannot delete record because of dependent enterprise_fees/)
-        .and change { EnterpriseFee.count }.by(0)
+        expect(enterprise.errors.full_messages).to eq(
+          ["Cannot delete record because dependent enterprise fees exist"]
+        )
+      end.to change { Spree::Order.count }.by(0)
     end
 
     it "does not destroy vouchers upon destroy" do
@@ -110,9 +114,10 @@ RSpec.describe Enterprise do
 
       expect do
         enterprise.destroy
-      end.to raise_error(ActiveRecord::DeleteRestrictionError,
-                         /Cannot delete record because of dependent vouchers/)
-        .and change { Voucher.count }.by(0)
+        expect(enterprise.errors.full_messages).to eq(
+          ["Cannot delete record because dependent vouchers exist"]
+        )
+      end.to change { Spree::Order.count }.by(0)
     end
 
     describe "relationships to other enterprises" do

spec/system/admin/enterprises_spec.rb

@@ -68,6 +68,48 @@ RSpec.describe '
     expect(page).to have_checked_field "enterprise_visible_only_through_links"
   end
 
+  it "deleting an existing enterprise successfully" do
+    enterprise = create(:enterprise)
+
+    user = create(:user)
+
+    admin = login_as_admin
+
+    visit '/admin/enterprises'
+
+    expect do
+      accept_alert do
+        within "tr.enterprise-#{enterprise.id}" do
+          first("a", text: 'Delete').click
+        end
+      end
+
+      expect(page).to have_content("Successfully Removed")
+    end.to change{ Enterprise.count }.by(-1)
+  end
+
+  it "deleting an existing enterprise unsuccessfully" do
+    enterprise = create(:enterprise)
+    create(:order, distributor: enterprise)
+
+    user = create(:user)
+
+    admin = login_as_admin
+
+    visit '/admin/enterprises'
+
+    expect do
+      accept_alert do
+        within "tr.enterprise-#{enterprise.id}" do
+          first("a", text: 'Delete').click
+        end
+      end
+
+      expect(page).to have_content("Cannot delete record because dependent distributed order")
+      expect(page).to have_content(enterprise.name)
+    end.to change{ Enterprise.count }.by(0)
+  end
+
   it "editing an existing enterprise" do
     @enterprise = create(:enterprise)
     e2 = create(:enterprise)