4977 add ability to delete unused item categories (#5197)

* 4978 rework donation site import

* 4978 rework donation site import

* 4978 rework donation site import added tests fix

* 4978 corrected donation sites csv file, contact name allowed to be blank, phone is not mandatory, fixed tests

* 4978 added contact name, email, phone to donation_sites csv tests

* Fix CSV import logic and error handling

* Fix CSV import logic and error handling

* Fix CSV import logic and error handling

* Fixed Donation Sites CSV

* minor updates

* Geocoder implemeted in tests, revert of changes in schema  and other minor requested changes

* 4977_add_ability_to_delete_unused_item_categories

* 4977_added_minor_fix_to_delete_item_category

* 4977_added_request_spec_and_removed_unnecessary_tables_from_schema

* Resolve merge conflict in organizations.rb

* changes in schema

* minor fixes

* rearranged specs order in item category spec

* replaced item categories request spec to request spec file

* request spec moved to spec/request

* moved redirect out of conditional

---------

Co-authored-by: CL Fisher <[email protected]>

Author: someykoku

app/controllers/item_categories_controller.rb

@@ -33,6 +33,17 @@ def update
     end
   end
 
+  def destroy
+    @item_category = current_organization.item_categories.find_by(id: params[:id])
+    if @item_category.items.exists?
+      flash[:alert] = "Cannot delete item category because it has associated items."
+    else
+      @item_category.destroy
+      flash[:notice] = "#{@item_category.name} has been deleted."
+    end
+    redirect_to items_path
+  end
+
   private
 
   def item_category_params

app/models/item_category.rb

@@ -16,5 +16,16 @@ class ItemCategory < ApplicationRecord
 
   belongs_to :organization
   has_many :items, -> { order(name: :asc) }, inverse_of: :item_category, dependent: :nullify
-  has_many :partner_groups, dependent: :nullify
+  has_and_belongs_to_many :partner_groups, dependent: :nullify
+
+  before_destroy :ensure_no_associated_partner_groups
+
+  private
+
+  def ensure_no_associated_partner_groups
+    if partner_groups.exists?
+      errors.add(:base, "Cannot delete item category with associated partner groups")
+      throw(:abort)
+    end
+  end
 end

app/views/items/_item_categories.html.erb

@@ -27,9 +27,10 @@
               <% end %>
             </ul>
           </td>
-          <td>
+          <td class="text-right">
             <%= view_button_to item_category_path(item_category) %>
             <%= edit_button_to edit_item_category_path(item_category.id) %>
+            <%= delete_button_to(item_category_path(item_category.id), { confirm: confirm_delete_msg(item_category.name) }) if (item_category.items.count <= 0) %>
           </td>
         </tr>
       <% end %>

db/schema.rb

@@ -324,7 +324,7 @@
     t.bigint "old_partner_id"
     t.boolean "archived", default: false
     t.index ["partner_id"], name: "index_families_on_partner_id"
-  end
+  end 
 
   create_table "flipper_features", force: :cascade do |t|
     t.string "key", null: false

spec/models/item_category_spec.rb

@@ -28,4 +28,24 @@
   describe "versioning" do
     it { is_expected.to be_versioned }
   end
+
+  describe "delete" do
+    let(:item_category) { create(:item_category) }
+    let(:partner_group) { create(:partner_group, item_categories: [item_category]) }
+
+    before do
+      partner_group
+    end
+
+    it "should not delete if associated with partner groups" do
+      expect(item_category.partner_groups).not_to be_empty
+      expect { item_category.destroy }.not_to change(ItemCategory, :count)
+      expect(item_category.errors.full_messages).to include("Cannot delete item category with associated partner groups")
+    end
+
+    it "should delete if not associated with partner groups" do
+      item_category.partner_groups.destroy_all
+      expect { item_category.destroy }.to change(ItemCategory, :count).by(-1)
+    end
+  end
 end

spec/requests/item_categories_requests_spec.rb

@@ -93,4 +93,29 @@
       end
     end
   end
+
+  describe "DELETE #destroy" do
+    let!(:item_category) { create(:item_category, organization: organization) }
+
+    context "when the item category has no associated items" do
+      it "destroys the requested item category" do
+        expect {
+          delete item_category_url(id: item_category.id)
+        }.to change(ItemCategory, :count).by(-1)
+        expect(response).to redirect_to(items_path)
+      end
+    end
+
+    context "when the item category has associated items" do
+      let!(:item) { create(:item, item_category: item_category) }
+
+      it "does not destroy the item category and shows an error" do
+        expect {
+          delete item_category_url(id: item_category.id)
+        }.not_to change(ItemCategory, :count)
+        expect(response).to redirect_to(items_path)
+        expect(flash[:alert]).to eq("Cannot delete item category because it has associated items.")
+      end
+    end
+  end
 end