#4217 kit factory refactor (#4585)

* Update bundler version

* REFACTOR clean up unused line item trait introduced in #4163

* REFACTOR replace kit :with_item with KitCreateService, hard code rspec

* Replace :with_item trait with calls to KitCreateService to make it easy
  to change line_item itemizable behavior later
* Remove unnecessary :with_item traits
* Hard code all rspecs matching against kit values, finishes #4217 for kits

* FIX typo in params in donation and purchase controllers

* Another typo in docs

* DOCS add comment clarifying itemizable works with line items

* REFACTOR rename Item:kits scope to :housing_a_kit, add rspecs

* Clearer name
* Add rspecs to test :housing_a_kit and :loose scopes

* 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

* Add rspec for item.is_in_kit?

* Fix lint

* rename findorcreatebaseitem to add !

* REFACTOR remove unused code childrenservedreportservice

* All SQL code is duplicated in AcquisitionReportService
* RSpec is close enough to what is in AcquisitionReportService Spec that it can be removed without merging in
  (only difference is Diapers - Adult Briefs category is not created, but that shouldn't matter
  because the SQL looks for %diaper% so this category isn't testing anything different)

* Fix bin/setup so it's working with seed_base_item move

* Revert "REFACTOR clean up unused line item trait introduced in #4163"

This reverts commit ceec002.

* Revert "FIX typo in params in donation and purchase controllers"

This reverts commit 7cabfa6.

* Revert "DOCS add comment clarifying itemizable works with line items"

This reverts commit 1109170.

* Revert "REFACTOR rename Item:kits scope to :housing_a_kit, add rspecs"

This reverts commit b9b8755.

* Revert "REFACTOR DRY up kit base_item, seed_base_items, prevent kit base_item deletion"

This reverts commit 8c18e42.

* Revert "Add rspec for item.is_in_kit?"

This reverts commit e1e2c31.

* Revert "REFACTOR remove unused code childrenservedreportservice"

This reverts commit 141d959.

* Revert "Fix bin/setup so it's working with seed_base_item move"

This reverts commit 080721e.

* REFACTOR move kit creation in specs into helper method

* fix failing rspecs

Author: jimmyli97

spec/events/inventory_aggregate_spec.rb

@@ -381,11 +381,11 @@
     end
 
     it "should process a kit allocation event" do
-      kit = FactoryBot.create(:kit, :with_item, organization: organization)
+      kit = create_kit(organization: organization, line_items_attributes: [
+        {item_id: item1.id, quantity: 10},
+        {item_id: item2.id, quantity: 3}
+      ])
 
-      kit.line_items = []
-      kit.line_items << build(:line_item, quantity: 10, item: item1, itemizable: kit)
-      kit.line_items << build(:line_item, quantity: 3, item: item2, itemizable: kit)
       KitAllocateEvent.publish(kit, storage_location1.id, 2)
 
       # 30 - (10*2) = 10, 10 - (3*2) = 4
@@ -416,7 +416,11 @@
     end
 
     it "should process a kit deallocation event" do
-      kit = FactoryBot.create(:kit, :with_item, organization: organization)
+      kit = create_kit(organization: organization, line_items_attributes: [
+        {item_id: item1.id, quantity: 20},
+        {item_id: item2.id, quantity: 5}
+      ])
+
       TestInventory.create_inventory(organization,
         {
           storage_location1.id => {

spec/factories/kits.rb

@@ -13,19 +13,17 @@
 #
 FactoryBot.define do
   factory :kit do
-    sequence(:name) { |n| "Test Kit #{n}" }
+    sequence(:name) { |n| "Default Kit Name #{n} - Don't Match" }
     organization
 
     after(:build) do |instance, _|
       if instance.line_items.blank?
-        instance.line_items << create(:line_item, item: create(:item, organization: instance.organization), itemizable: instance)
+        instance.line_items << build(:line_item, item: create(:item, organization: instance.organization), itemizable: nil)
       end
     end
 
-    trait :with_item do
-      after(:create) do |instance, _|
-        create(:item, kit: instance, organization: instance.organization)
-      end
-    end
+    # See #3652, changes to this factory are in progress
+    # For now, to create corresponding item and line item and persist to database call create_kit
+    # from spec/support/kit_helper.rb
   end
 end

spec/models/item_spec.rb

@@ -249,9 +249,10 @@
       end
 
       context "in a kit" do
-        let(:kit) { create(:kit, organization: organization) }
         before do
-          create(:line_item, itemizable: kit, item: item)
+          create_kit(organization: organization, line_items_attributes: [
+            {item_id: item.id, quantity: 1}
+          ])
         end
 
         it "should return false" do
@@ -284,10 +285,10 @@
       end
 
       context "in a kit" do
-        let(:kit) { create(:kit, organization: organization) }
-
         before do
-          create(:line_item, itemizable: kit, item: item)
+          create_kit(organization: organization, line_items_attributes: [
+            {item_id: item.id, quantity: 1}
+          ])
         end
 
         it "should return false" do
@@ -460,8 +461,9 @@
       let(:kit) { create(:kit, name: "my kit") }
 
       it "updates kit name" do
-        item.update(name: "my new name")
-        expect(item.name).to eq kit.name
+        name = "my new name"
+        item.update(name: name)
+        expect(kit.name).to eq name
       end
     end
 

spec/models/kit_spec.rb

@@ -58,10 +58,13 @@
     end
 
     it "->alphabetized retrieves items in alphabetical order" do
-      kit_c = create(:kit, name: "KitC")
-      kit_b = create(:kit, name: "KitB")
-      kit_a = create(:kit, name: "KitA")
-      alphabetized_list = [kit_a.name, kit_b.name, kit_c.name]
+      a_name = "KitA"
+      b_name = "KitB"
+      c_name = "KitC"
+      create(:kit, name: c_name)
+      create(:kit, name: b_name)
+      create(:kit, name: a_name)
+      alphabetized_list = [a_name, b_name, c_name]
 
       expect(Kit.alphabetized.count).to eq(3)
       expect(Kit.alphabetized.map(&:name)).to eq(alphabetized_list)
@@ -107,7 +110,7 @@
   end
 
   describe '#can_deactivate?' do
-    let(:kit) { create(:kit, :with_item, organization: organization) }
+    let(:kit) { create_kit(organization: organization) }
 
     context 'with inventory' do
       it 'should return false' do
@@ -132,7 +135,7 @@
   end
 
   specify 'deactivate and reactivate' do
-    kit = create(:kit, :with_item)
+    kit = create_kit(organization: organization)
     expect(kit.active).to eq(true)
     expect(kit.item.active).to eq(true)
     kit.deactivate

spec/requests/kit_requests_spec.rb

@@ -3,7 +3,9 @@
   let(:user) { create(:user, organization: organization) }
   let(:organization_admin) { create(:organization_admin, organization: organization) }
 
-  let!(:kit) { create(:kit, :with_item, organization: organization) }
+  let!(:kit) {
+    create_kit(organization: organization)
+  }
 
   describe "while signed in" do
     before do
@@ -13,7 +15,7 @@
     describe "GET #index" do
       before do
         # this shouldn't be shown
-        create(:kit, :with_item, active: false, name: "DOOBIE KIT", organization: organization)
+        create_kit(organization: organization, active: false, name: "DOOBIE KIT")
       end
 
       it "should include deactivate" do

spec/services/reports/children_served_report_service_spec.rb

@@ -27,7 +27,7 @@
       non_disposable_item = organization.items.where.not(id: organization.items.disposable).first
 
       # Kits
-      kit = create(:kit, :with_item, organization: organization)
+      kit = create(:kit, organization: organization)
 
       create(:base_item, name: "Toddler Disposable Diaper", partner_key: "toddler diapers", category: "disposable diaper")
       create(:base_item, name: "Infant Disposable Diaper", partner_key: "infant diapers", category: "infant disposable diaper")
@@ -71,7 +71,7 @@
       non_disposable_item = organization.items.where.not(id: organization.items.disposable).first
 
       # Kits
-      kit = create(:kit, :with_item, organization: organization)
+      kit = create(:kit, organization: organization)
 
       create(:base_item, name: "Toddler Disposable Diaper", partner_key: "toddler diapers", category: "disposable diaper")
       create(:base_item, name: "Infant Disposable Diaper", partner_key: "infant diapers", category: "infant disposable diaper")

spec/services/reports/diaper_report_service_spec.rb

@@ -15,17 +15,20 @@
       disposable_kit_item = create(:item, name: "Adult Disposable Diapers", partner_key: "adult diapers")
       another_disposable_kit_item = create(:item, name: "Infant Disposable Diapers", partner_key: "infant diapers")
 
-      disposable_line_item = create(:line_item, item: disposable_kit_item, quantity: 5)
-      another_disposable_line_item = create(:line_item, item: another_disposable_kit_item, quantity: 5)
+      disposable_kit = create_kit(organization: organization, line_items_attributes: [
+        {item_id: disposable_kit_item.id, quantity: 5}
+      ])
 
-      disposable_kit = create(:kit, :with_item, organization: organization, line_items: [disposable_line_item])
-      another_disposable_kit = create(:kit, :with_item, organization: organization, line_items: [another_disposable_line_item])
+      another_disposable_kit = create_kit(organization: organization, line_items_attributes: [
+        {item_id: another_disposable_kit_item.id, quantity: 5}
+      ])
 
       disposable_kit_item_distribution = create(:distribution, organization: organization, issued_at: within_time)
       another_disposable_kit_item_distribution = create(:distribution, organization: organization, issued_at: within_time)
 
       create(:line_item, :distribution, quantity: 10, item: disposable_kit.item, itemizable: disposable_kit_item_distribution)
       create(:line_item, :distribution, quantity: 10, item: another_disposable_kit.item, itemizable: another_disposable_kit_item_distribution)
+      # Total disposable items distributed so far: 5*10 + 5*10 = 100
 
       # create disposable and non disposable items
       create(:base_item, name: "3T Diaper", partner_key: "toddler diapers", category: "disposable diaper")
@@ -42,6 +45,7 @@
         create_list(:line_item, 5, :distribution, quantity: 20, item: disposable_item, itemizable: dist)
         create_list(:line_item, 5, :distribution, quantity: 30, item: non_disposable_item, itemizable: dist)
       end
+      # Total disposable items distributed: (100) + 2*5*20 = 300
 
       # Donations outside drives
       non_drive_donations = create_list(:donation, 2,
@@ -155,9 +159,9 @@
 
     it "should return the proper results on #report" do
       expect(subject.report).to eq({
-        entries: {"Disposable diapers distributed" => "320",
+        entries: {"Disposable diapers distributed" => "300",
                   "Cloth diapers distributed" => "300",
-                  "Average monthly disposable diapers distributed" => "27",
+                  "Average monthly disposable diapers distributed" => "25",
                   "Total product drives" => 2,
                   "Disposable diapers collected from drives" => "600",
                   "Cloth diapers collected from drives" => "900",

spec/services/reports/period_supply_report_service_spec.rb

@@ -34,11 +34,11 @@
         # Spec will ensure that only the required data is included.
 
         # Kits
-        period_supplies_kit = create(:kit, :with_item, organization: organization)
-        another_period_supply_kit = create(:kit, :with_item, organization: organization)
-        donated_period_supply_kit = create(:kit, :with_item, organization: organization)
-        purchased_period_supply_kit = create(:kit, :with_item, organization: organization)
-        pad_and_tampon_kit = create(:kit, :with_item, organization: organization)
+        period_supplies_kit = create_kit(organization: organization)
+        another_period_supply_kit = create_kit(organization: organization)
+        donated_period_supply_kit = create_kit(organization: organization)
+        purchased_period_supply_kit = create_kit(organization: organization)
+        pad_and_tampon_kit = create_kit(organization: organization)
 
         create(:base_item, name: "Adult Pads", partner_key: "adult pads", category: "Menstrual Supplies")
         create(:base_item, name: "Adult Tampons", partner_key: "adult tampons", category: "Menstrual Supplies")

spec/support/kit_helper.rb

@@ -0,0 +1,10 @@
+def create_kit(name: nil, active: true, organization: create(:organization), line_items_attributes: nil)
+  params = FactoryBot.attributes_for(:kit, active: active)
+  params[:name] = name if name
+
+  params[:line_items_attributes] = (line_items_attributes || [
+      {item_id: create(:item, organization: organization).id, quantity: 1}
+    ])
+
+  KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit
+end