REFACTOR DRY up kit base_item, seed_base_items, prevent kit base_item…

… deletion

* Move seed_base_items code into one static function
* Move kit base item creation code into one static function
* Add code to prevent calling destroy on kit base item and corresponding rspec
* Added comments - not sure about whether other base item request specs are useful or what the purpose of destroy is in the controller if it can't be called

app/controllers/admin/base_items_controller.rb

@@ -38,9 +38,12 @@ def show
     @items = @base_item.items
   end
 
+  # TODO there are no buttons on the view to call this method, consider removing?
   def destroy
     @base_item = BaseItem.includes(:items).find(params[:id])
-    if @base_item.items.any? && @base_item.destroy
+    if (@base_item.id = KitCreateService.FindOrCreateKitBaseItem.id)
+      redirect_to admin_base_items_path, alert: "You cannot delete the Kits base item. This is reserved for all Kits."
+    elsif @base_item.items.any? && @base_item.destroy
       redirect_to admin_base_items_path, notice: "Base Item deleted!"
     else
       redirect_to admin_base_items_path, alert: "Failed to delete Base Item. Are there still items attached?"

app/models/base_item.rb

@@ -28,5 +28,24 @@ class BaseItem < ApplicationRecord
   def to_h
     { partner_key: partner_key, name: name }
   end
-end
 
+  def self.seed_base_items
+    # Initial starting qty for our test organizations
+    base_items = File.read(Rails.root.join("db", "base_items.json"))
+    items_by_category = JSON.parse(base_items)
+
+    items_by_category.each do |category, entries|
+      entries.each do |entry|
+        BaseItem.find_or_create_by!(
+          name: entry["name"],
+          category: category,
+          partner_key: entry["key"],
+          updated_at: Time.zone.now,
+          created_at: Time.zone.now
+        )
+      end
+    end
+    # Create global 'Kit' base item
+    KitCreateService.FindOrCreateKitBaseItem
+  end
+end

app/services/kit_create_service.rb

@@ -1,8 +1,18 @@
 class KitCreateService
   include ServiceObjectErrorsMixin
 
+  KIT_BASE_ITEM_ATTRS = {
+    name: 'Kit',
+    category: 'kit',
+    partner_key: 'kit'
+  }
+
   attr_reader :kit
 
+  def self.FindOrCreateKitBaseItem
+    BaseItem.find_or_create_by!(KIT_BASE_ITEM_ATTRS)
+  end
+
   def initialize(organization_id:, kit_params:)
     @organization_id = organization_id
     @kit_params = kit_params
@@ -16,16 +26,15 @@ def call
       @kit = Kit.new(kit_params_with_organization)
       @kit.save!
 
-      # Create a BaseItem that houses each
-      # kit item created.
-      kit_base_item = fetch_or_create_kit_base_item
+      # Find or create the BaseItem for all items housing kits
+      item_housing_a_kit_base_item = BaseItem.find_or_create_by!(KIT_BASE_ITEM_ATTRS)
 
       # Create the Item.
       item_creation = ItemCreateService.new(
         organization_id: organization.id,
         item_params: {
           name: kit.name,
-          partner_key: kit_base_item.partner_key,
+          partner_key: item_housing_a_kit_base_item.partner_key,
           kit_id: kit.id
         }
       )
@@ -45,7 +54,6 @@ def call
   private
 
   attr_reader :organization_id, :kit_params
-
   def organization
     @organization ||= Organization.find_by(id: organization_id)
   end
@@ -55,19 +63,6 @@ def kit_params_with_organization
                        organization_id: organization.id
                      })
   end
-
-  def fetch_or_create_kit_base_item
-    BaseItem.find_or_create_by!({
-                                  name: 'Kit',
-                                  category: 'kit',
-                                  partner_key: 'kit'
-                                })
-  end
-
-  def partner_key_for_kits
-    'kit'
-  end
-
   def valid?
     if organization.blank?
       errors.add(:organization_id, 'does not match any Organization')
@@ -88,16 +83,6 @@ def kit_validation_errors
     kit.valid?
 
     @kit_validation_errors = kit.errors
-  end
 
-  def associated_item_params
-    {
-      kit: kit.name
-    }
-  end
-
-  def partner_key_for(name)
-    "kit_#{name.underscore.gsub(/\s+/, '_')}"
   end
 end
-

db/seeds.rb

@@ -19,26 +19,11 @@ def random_record_for_org(org, klass)
 # Script-Global Variables
 # ----------------------------------------------------------------------------
 
-# Initial starting qty for our test organizations
-base_items = File.read(Rails.root.join("db", "base_items.json"))
-items_by_category = JSON.parse(base_items)
-
 # ----------------------------------------------------------------------------
 # Base Items
 # ----------------------------------------------------------------------------
 
-items_by_category.each do |category, entries|
-  entries.each do |entry|
-    BaseItem.find_or_create_by!(name: entry["name"], category: category, partner_key: entry["key"])
-  end
-end
-
-# Create global 'Kit' base item
-BaseItem.find_or_create_by!(
-  name: 'Kit',
-  category: 'kit',
-  partner_key: 'kit'
-)
+BaseItem.seed_base_items
 
 # ----------------------------------------------------------------------------
 # NDBN Members

spec/factories/organizations.rb

@@ -61,7 +61,7 @@
 
     trait :with_items do
       after(:create) do |instance, evaluator|
-        seed_base_items if BaseItem.count.zero? # seeds 45 base items if none exist
+        BaseItem.seed_base_items if BaseItem.count.zero? # seeds 45 base items if none exist
         Organization.seed_items(instance) # creates 1 item for each base item
       end
     end

spec/rails_helper.rb

@@ -244,21 +244,3 @@ def await_select2(select2, container = nil, &block)
 
   find("#{container} select option[data-select2-id=\"#{current_id.to_i + 1}\"]", wait: 10)
 end
-
-def seed_base_items
-  base_items = File.read(Rails.root.join("db", "base_items.json"))
-  items_by_category = JSON.parse(base_items)
-  base_items_data = items_by_category.map do |category, entries|
-    entries.map do |entry|
-      {
-        name: entry["name"],
-        category: category,
-        partner_key: entry["key"],
-        updated_at: Time.zone.now,
-        created_at: Time.zone.now
-      }
-    end
-  end.flatten
-
-  BaseItem.create!(base_items_data)
-end

spec/requests/admin/base_items_requests_spec.rb

@@ -1,15 +1,25 @@
 RSpec.describe "Admin::BaseItems", type: :request do
   let(:organization) { create(:organization) }
   let(:user) { create(:user, organization: organization) }
+  let(:super_admin) { create(:super_admin, organization: organization) }
   let(:organization_admin) { create(:organization_admin, organization: organization) }
 
-  # TODO: should this be testing something?
   context "while signed in as a super admin" do
     before do
-      sign_in(@super_admin)
+      sign_in(super_admin)
+    end
+
+    it "doesn't let you delete the Kit base item" do
+      kit_base_item = KitCreateService.FindOrCreateKitBaseItem
+      delete admin_base_item_path(id: kit_base_item.id)
+      expect(flash[:alert]).to include("You cannot delete the Kits base item")
+      expect(response).to be_redirect
+      expect(BaseItem.exists?(kit_base_item.id)).to be true
     end
   end
 
+  # TODO aren't organization_admins not allowed to view base items?
+  # also, some of these tests are sending organization.id instead of BaseItem.id as args
   context "When logged in as an organization admin" do
     before do
       sign_in(organization_admin)