Add missing kit rspecs, DRY up kit base items and report service

app/controllers/admin/base_items_controller.rb

@@ -1,5 +1,7 @@
 # [Super Admin] Manage the BaseItems -- this is the only place in the app where Base Items can be
 # added / modified. Base Items are both the template and common thread for regular Items
+#
+# See #4656, BaseItems are pending significant changes/possible deletion
 class Admin::BaseItemsController < AdminController
   def edit
     @base_item = BaseItem.find(params[:id])
@@ -40,7 +42,9 @@ def show
 
   def destroy
     @base_item = BaseItem.includes(:items).find(params[:id])
-    if @base_item.items.any? && @base_item.destroy
+    if (@base_item.id = KitCreateService.find_or_create_kit_base_item!.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/controllers/donations_controller.rb

@@ -169,7 +169,7 @@ def strip_unnecessary_params
 
   # If line_items have submitted with empty rows, clear those out first.
   def compact_line_items
-    return params unless params[:donation].key?(:line_item_attributes)
+    return params unless params[:donation].key?(:line_items_attributes)
 
     params[:donation][:line_items_attributes].delete_if { |_row, data| data["quantity"].blank? && data["item_id"].blank? }
     params

app/controllers/purchases_controller.rb

@@ -112,7 +112,7 @@ def filter_params
 
   # If line_items have submitted with empty rows, clear those out first.
   def compact_line_items
-    return params unless params[:purchase].key?(:line_item_attributes)
+    return params unless params[:purchase].key?(:line_items_attributes)
 
     params[:purchase][:line_items_attributes].delete_if { |_row, data| data["quantity"].blank? && data["item_id"].blank? }
     params

app/models/base_item.rb

@@ -29,4 +29,3 @@ def to_h
     { partner_key: partner_key, name: name }
   end
 end
-

app/models/concerns/itemizable.rb

@@ -1,5 +1,5 @@
 # Creates a veritable powerhouse.
-# This module provides Duck Typed behaviors for anything that shuttle Items
+# This module provides Duck Typed behaviors for anything that shuttles LINE ITEMS (not items)
 # throughout the system. e.g. things that `has_many :line_items` -- this provides
 # all the logic about how those kinds of things behave.
 module Itemizable

app/models/item.rb

@@ -51,8 +51,8 @@ class Item < ApplicationRecord
 
   scope :active, -> { where(active: true) }
 
-  # Add spec for these
-  scope :kits, -> { where.not(kit_id: nil) }
+  # :housing_a_kit are items which house a kit, NOT items is_in_kit
+  scope :housing_a_kit, -> { where.not(kit_id: nil) }
   scope :loose, -> { where(kit_id: nil) }
 
   scope :visible, -> { where(visible_to_partners: true) }

app/services/itemizable_update_service.rb

@@ -2,7 +2,7 @@ module ItemizableUpdateService
   # @param itemizable [Itemizable]
   # @param type [Symbol] :increase or :decrease - if the original line items added quantities (purchases or
   # donations), use :increase. If the original line_items reduced quantities (distributions) use :decrease.
-  # @param params [Hash] Parameters passed from the controller. Should include `line_item_attributes`.
+  # @param params [Hash] Parameters passed from the controller. Should include `line_items_attributes`.
   # @param event_class [Class<Event>] the event class to publish the itemizable to.
   def self.call(itemizable:, type: :increase, params: {}, event_class: nil)
     StorageLocation.transaction do

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.find_or_create_kit_base_item!
+    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 = KitCreateService.find_or_create_kit_base_item!
 
-      # Create the Item.
+      # 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
@@ -56,18 +64,6 @@ def kit_params_with_organization
                      })
   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')
@@ -89,15 +85,4 @@ def kit_validation_errors
 
     @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
-

app/services/reports/children_served_report_service.rb

@@ -31,47 +31,8 @@ def average_children_monthly
       total_children_served / 12.0
     end
 
-    def disposable_diapers_from_kits_total
-      organization_id = @organization.id
-      year = @year
-
-      sql_query = <<-SQL
-        SELECT SUM(line_items.quantity * kit_line_items.quantity)
-        FROM distributions 
-        INNER JOIN line_items ON line_items.itemizable_type = 'Distribution' AND line_items.itemizable_id = distributions.id 
-        INNER JOIN items ON items.id = line_items.item_id 
-        INNER JOIN kits ON kits.id = items.kit_id 
-        INNER JOIN line_items AS kit_line_items ON kits.id = kit_line_items.itemizable_id
-        INNER JOIN items AS kit_items ON kit_items.id = kit_line_items.item_id
-        INNER JOIN base_items ON base_items.partner_key = kit_items.partner_key 
-        WHERE distributions.organization_id = ?
-          AND EXTRACT(year FROM issued_at) = ?
-          AND LOWER(base_items.category) LIKE '%diaper%'
-          AND NOT (LOWER(base_items.category) LIKE '%cloth%' OR LOWER(base_items.name) LIKE '%cloth%')
-          AND NOT (LOWER(base_items.category) LIKE '%adult%')
-      SQL
-
-      sanitized_sql = ActiveRecord::Base.send(:sanitize_sql_array, [sql_query, organization_id, year])
-
-      result = ActiveRecord::Base.connection.execute(sanitized_sql)
-      result.first['sum'].to_i
-    end
-
     private
 
-    def total_disposable_diapers_distributed
-      loose_disposable_distribution_total + disposable_diapers_from_kits_total
-    end
-
-    def loose_disposable_distribution_total
-      organization
-      .distributions
-      .for_year(year)
-      .joins(line_items: :item)
-      .merge(Item.disposable)
-      .sum("line_items.quantity")
-    end
-
     def total_children_served_with_loose_disposables
       organization
       .distributions

db/seeds.rb

@@ -19,26 +19,12 @@ 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'
-)
+require 'seed_base_items'
+seed_base_items
 
 # ----------------------------------------------------------------------------
 # NDBN Members
@@ -521,7 +507,7 @@ def seed_quantity(item_name, organization, storage_location, quantity)
   AdjustmentCreateService.new(adjustment).call
 end
 
-items_by_category.each do |_category, entries|
+JSON.parse(File.read(Rails.root.join("db", "base_items.json"))).each do |_category, entries|
   entries.each do |entry|
     seed_quantity(entry['name'], pdx_org, inv_arbor, entry['qty']['arbor'])
     seed_quantity(entry['name'], pdx_org, inv_pdxdb, entry['qty']['pdxdb'])

lib/seed_base_items.rb

@@ -0,0 +1,19 @@
+def 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.find_or_create_kit_base_item!
+end

spec/factories/line_items.rb

@@ -51,10 +51,5 @@
       itemizable_type { "Transfer" }
       itemizable_id { create(:transfer).id }
     end
-
-    trait :kit do
-      itemizable_type { "Kit" }
-      itemizable_id { create(:kit).id }
-    end
   end
 end

spec/factories/organizations.rb

@@ -34,6 +34,7 @@
 #  account_request_id             :integer
 #  ndbn_member_id                 :bigint
 #
+require 'seed_base_items'
 
 FactoryBot.define do
   factory :organization do

spec/models/item_spec.rb

@@ -47,7 +47,7 @@
       expect(subject.class).to respond_to :class_filter
     end
 
-    it "->by_size returns all items with the same size, per their BaseItem parent" do
+    specify "->by_size returns all items with the same size, per their BaseItem parent" do
       size4 = create(:base_item, size: "4", name: "Size 4 Diaper")
       size_z = create(:base_item, size: "Z", name: "Size Z Diaper")
 
@@ -57,7 +57,30 @@
       expect(Item.by_size("4").length).to eq(2)
     end
 
-    it "->alphabetized retrieves items in alphabetical order" do
+    specify "->housing_a_kit returns all items which belongs_to (house) a kit" do
+      name = "test kit"
+      kit_params = attributes_for(:kit, name: name)
+      kit_params[:line_items_attributes] = [{item_id: create(:item).id, quantity: 1}] # shouldn't be counted
+      KitCreateService.new(organization_id: organization.id, kit_params: kit_params).call
+
+      create(:item) # shouldn't be counted
+      expect(Item.housing_a_kit.count).to eq(1)
+      expect(Item.housing_a_kit.first.name = name)
+    end
+
+    specify "->loose returns all items which do not belongs_to a kit" do
+      name = "A"
+      item = create(:item, name: name, organization: organization)
+
+      kit_params = attributes_for(:kit)
+      kit_params[:line_items_attributes] = [{item_id: item.id, quantity: 1}]
+      KitCreateService.new(organization_id: organization.id, kit_params: kit_params).call # shouldn't be counted
+
+      expect(Item.loose.count).to eq(1)
+      expect(Item.loose.first.name = name)
+    end
+
+    specify "->alphabetized retrieves items in alphabetical order" do
       item_c = create(:item, name: "C")
       item_b = create(:item, name: "B")
       item_a = create(:item, name: "A")
@@ -67,7 +90,7 @@
       expect(Item.alphabetized.map(&:name)).to eq(alphabetized_list)
     end
 
-    it "->active shows items that are still active" do
+    specify "->active shows items that are still active" do
       inactive_item = create(:line_item, :purchase).item
       item = create(:item)
       inactive_item.deactivate!
@@ -362,6 +385,19 @@
       end
     end
 
+    describe '#is_in_kit?' do
+      it "is true for items that are in a kit and false otherwise" do
+        item_not_in_kit = create(:item, organization: organization)
+        item_in_kit = create(:item, organization: organization)
+
+        kit_params = attributes_for(:kit)
+        kit_params[:line_items_attributes] = [{item_id: item_in_kit.id, quantity: 1}]
+        KitCreateService.new(organization_id: organization.id, kit_params: kit_params).call
+        expect(item_in_kit.is_in_kit?).to be true
+        expect(item_not_in_kit.is_in_kit?).to be false
+      end
+    end
+
     describe "other?" do
       it "is true for items that are partner_key 'other'" do
         item = create(:item, base_item: create(:base_item, name: "Base"))

spec/rails_helper.rb

@@ -242,21 +242,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.find_or_create_kit_base_item!
+      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)