From 7a351cd839b96515532a021d6953bbd0de579734 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Wed, 7 Aug 2024 18:56:53 +0000 Subject: [PATCH 01/21] Update bundler version --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 47ab0c331b..1077292fdd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -800,4 +800,4 @@ DEPENDENCIES webmock (~> 3.23) BUNDLED WITH - 2.5.16 + 2.5.17 From ceec0023a7143c5f341868acd34a209f4c35d669 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Thu, 1 Aug 2024 13:25:02 +0000 Subject: [PATCH 02/21] REFACTOR clean up unused line item trait introduced in #4163 --- spec/factories/line_items.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/spec/factories/line_items.rb b/spec/factories/line_items.rb index 913be2b479..d6bdddc5ad 100644 --- a/spec/factories/line_items.rb +++ b/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 From 4e1bc8b15c32b44ac8d28034a0108f51e462927b Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Thu, 1 Aug 2024 19:02:57 +0000 Subject: [PATCH 03/21] 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 --- spec/events/inventory_aggregate_spec.rb | 23 ++++++++++++------- spec/factories/kits.rb | 14 +++++------ spec/models/item_spec.rb | 20 ++++++++++------ spec/models/kit_spec.rb | 19 ++++++++++----- spec/requests/kit_requests_spec.rb | 14 +++++++++-- .../acquisition_report_service_spec.rb | 20 +++++++++++----- .../children_served_report_service_spec.rb | 4 ++-- 7 files changed, 76 insertions(+), 38 deletions(-) diff --git a/spec/events/inventory_aggregate_spec.rb b/spec/events/inventory_aggregate_spec.rb index 4c65ad97c9..a0fa7c4773 100644 --- a/spec/events/inventory_aggregate_spec.rb +++ b/spec/events/inventory_aggregate_spec.rb @@ -381,11 +381,14 @@ end it "should process a kit allocation event" do - kit = FactoryBot.create(:kit, :with_item, organization: organization) + params = FactoryBot.attributes_for(:kit) + params[:line_items_attributes] = [ + {item_id: item1.id, quantity: 10}, + {item_id: item2.id, quantity: 3} + ] + + kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit - 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 +419,14 @@ end it "should process a kit deallocation event" do - kit = FactoryBot.create(:kit, :with_item, organization: organization) + params = FactoryBot.attributes_for(:kit) + params[:line_items_attributes] = [ + {item_id: item1.id, quantity: 20}, + {item_id: item2.id, quantity: 5} + ] + + kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit + TestInventory.create_inventory(organization, { storage_location1.id => { @@ -432,9 +442,6 @@ }) inventory = InventoryAggregate.inventory_for(organization.id) # reload - kit.line_items = [] - kit.line_items << build(:line_item, quantity: 20, item: item1, itemizable: kit) - kit.line_items << build(:line_item, quantity: 5, item: item2, itemizable: kit) KitDeallocateEvent.publish(kit, storage_location1, 2) # 30 + (20*2) = 70, 10 + (5*2) = 20 diff --git a/spec/factories/kits.rb b/spec/factories/kits.rb index ef81343014..3a90279870 100644 --- a/spec/factories/kits.rb +++ b/spec/factories/kits.rb @@ -13,19 +13,19 @@ # 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 KitCreateService with: + # kit_params = attributes_for(:kit) + # kit_params[:line_items_attributes] = [{ item_id: _id_, quantity: _quantity_, ...}] + # KitCreateService.new(organization_id: organization.id, kit_params: kit_params).call.kit end end diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb index c2ea38cf07..36c001ab65 100644 --- a/spec/models/item_spec.rb +++ b/spec/models/item_spec.rb @@ -236,9 +236,12 @@ end context "in a kit" do - let(:kit) { create(:kit, organization: organization) } before do - create(:line_item, itemizable: kit, item: item) + params = FactoryBot.attributes_for(:kit) + params[:line_items_attributes] = [ + {item_id: item.id, quantity: 1} + ] + KitCreateService.new(organization_id: organization.id, kit_params: params).call end it "should return false" do @@ -271,10 +274,12 @@ end context "in a kit" do - let(:kit) { create(:kit, organization: organization) } - before do - create(:line_item, itemizable: kit, item: item) + params = FactoryBot.attributes_for(:kit) + params[:line_items_attributes] = [ + {item_id: item.id, quantity: 1} + ] + KitCreateService.new(organization_id: organization.id, kit_params: params).call end it "should return false" do @@ -424,8 +429,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 diff --git a/spec/models/kit_spec.rb b/spec/models/kit_spec.rb index 989fb83ce2..acfb4d3d36 100644 --- a/spec/models/kit_spec.rb +++ b/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 @@ -131,7 +134,11 @@ end specify 'deactivate and reactivate' do - kit = create(:kit, :with_item) + params = FactoryBot.attributes_for(:kit) + params[:line_items_attributes] = [ + {item_id: create(:item).id, quantity: 1} + ] + kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit expect(kit.active).to eq(true) expect(kit.item.active).to eq(true) kit.deactivate diff --git a/spec/requests/kit_requests_spec.rb b/spec/requests/kit_requests_spec.rb index 35284743bb..243713a409 100644 --- a/spec/requests/kit_requests_spec.rb +++ b/spec/requests/kit_requests_spec.rb @@ -3,7 +3,13 @@ let(:user) { create(:user, organization: organization) } let(:organization_admin) { create(:organization_admin, organization: organization) } - let!(:kit) { create(:kit, :with_item, organization: organization) } + let!(:kit) { + params = FactoryBot.attributes_for(:kit) + params[:line_items_attributes] = [ + {item_id: create(:item).id, quantity: 1} + ] + KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit + } describe "while signed in" do before do @@ -13,7 +19,11 @@ describe "GET #index" do before do # this shouldn't be shown - create(:kit, :with_item, active: false, name: "DOOBIE KIT", organization: organization) + params = FactoryBot.attributes_for(:kit, active: false, name: "DOOBIE KIT") + params[:line_items_attributes] = [ + {item_id: create(:item).id, quantity: 1} + ] + KitCreateService.new(organization_id: organization.id, kit_params: params).call end it "should include deactivate" do diff --git a/spec/services/reports/acquisition_report_service_spec.rb b/spec/services/reports/acquisition_report_service_spec.rb index 8ea62471d3..e2f6390bdd 100644 --- a/spec/services/reports/acquisition_report_service_spec.rb +++ b/spec/services/reports/acquisition_report_service_spec.rb @@ -15,17 +15,24 @@ 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) + params = FactoryBot.attributes_for(:kit) + params[:line_items_attributes] = [ + {item_id: disposable_kit_item.id, quantity: 5} + ] + disposable_kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit - 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]) + params = FactoryBot.attributes_for(:kit) + params[:line_items_attributes] = [ + {item_id: another_disposable_kit_item.id, quantity: 5} + ] + another_disposable_kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit 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 +49,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 +163,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", diff --git a/spec/services/reports/children_served_report_service_spec.rb b/spec/services/reports/children_served_report_service_spec.rb index e660760693..e54268cb65 100644 --- a/spec/services/reports/children_served_report_service_spec.rb +++ b/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") From 7cabfa6eb875d1040efd77d1c6059aa5889b9f53 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Thu, 1 Aug 2024 17:44:39 +0000 Subject: [PATCH 04/21] FIX typo in params in donation and purchase controllers * Another typo in docs --- app/controllers/donations_controller.rb | 2 +- app/controllers/purchases_controller.rb | 2 +- app/services/itemizable_update_service.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index a925ab6c41..1c44db3299 100644 --- a/app/controllers/donations_controller.rb +++ b/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 diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb index 32e288c6b6..6053712c7d 100644 --- a/app/controllers/purchases_controller.rb +++ b/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 diff --git a/app/services/itemizable_update_service.rb b/app/services/itemizable_update_service.rb index 45dc7133da..972b432efe 100644 --- a/app/services/itemizable_update_service.rb +++ b/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] the event class to publish the itemizable to. def self.call(itemizable:, type: :increase, params: {}, event_class: nil) StorageLocation.transaction do From 1109170fffcd97178d44ecd16347c40f80cc5463 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Fri, 26 Jul 2024 03:40:09 +0000 Subject: [PATCH 05/21] DOCS add comment clarifying itemizable works with line items --- app/models/concerns/itemizable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb index 9248238f8b..c77f6d2a21 100644 --- a/app/models/concerns/itemizable.rb +++ b/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 From b9b875548f03825650b37c0dda0f7d008cc17056 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Thu, 1 Aug 2024 21:11:52 +0000 Subject: [PATCH 06/21] REFACTOR rename Item:kits scope to :housing_a_kit, add rspecs * Clearer name * Add rspecs to test :housing_a_kit and :loose scopes --- app/models/item.rb | 4 ++-- spec/models/item_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/app/models/item.rb b/app/models/item.rb index 66ca08f5c7..8ecf42f1b8 100644 --- a/app/models/item.rb +++ b/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) } diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb index 36c001ab65..4a67d22a85 100644 --- a/spec/models/item_spec.rb +++ b/spec/models/item_spec.rb @@ -57,6 +57,29 @@ expect(Item.by_size("4").length).to eq(2) end + it "->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 + + it "->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 + it "->alphabetized retrieves items in alphabetical order" do item_c = create(:item, name: "C") item_b = create(:item, name: "B") From 8c18e4286748cef70b06b8c8e0e013e724d92b24 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Fri, 2 Aug 2024 00:43:51 +0000 Subject: [PATCH 07/21] 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 --- .../admin/base_items_controller.rb | 5 ++- app/models/base_item.rb | 21 +++++++++- app/services/kit_create_service.rb | 41 ++++++------------- db/seeds.rb | 17 +------- spec/factories/organizations.rb | 2 +- spec/rails_helper.rb | 18 -------- .../admin/base_items_requests_spec.rb | 14 ++++++- 7 files changed, 51 insertions(+), 67 deletions(-) diff --git a/app/controllers/admin/base_items_controller.rb b/app/controllers/admin/base_items_controller.rb index 90d37f0a0e..c62e9ce6cf 100644 --- a/app/controllers/admin/base_items_controller.rb +++ b/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?" diff --git a/app/models/base_item.rb b/app/models/base_item.rb index d910a46f25..c3effddb30 100644 --- a/app/models/base_item.rb +++ b/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 diff --git a/app/services/kit_create_service.rb b/app/services/kit_create_service.rb index 80e976d2ee..42853e38be 100644 --- a/app/services/kit_create_service.rb +++ b/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 - diff --git a/db/seeds.rb b/db/seeds.rb index 614143cbd0..0e38fa44f2 100644 --- a/db/seeds.rb +++ b/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 diff --git a/spec/factories/organizations.rb b/spec/factories/organizations.rb index 30ef97daa7..945075bb5d 100644 --- a/spec/factories/organizations.rb +++ b/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 diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index ec498f97c3..ccd77cad60 100644 --- a/spec/rails_helper.rb +++ b/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 diff --git a/spec/requests/admin/base_items_requests_spec.rb b/spec/requests/admin/base_items_requests_spec.rb index ee04d33c1e..a539f52216 100644 --- a/spec/requests/admin/base_items_requests_spec.rb +++ b/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) From e1e2c316a9e0a8fee7a29bbd58dbec2b07759cc2 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Fri, 2 Aug 2024 01:34:52 +0000 Subject: [PATCH 08/21] Add rspec for item.is_in_kit? --- spec/models/item_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb index 4a67d22a85..1b0004b6bf 100644 --- a/spec/models/item_spec.rb +++ b/spec/models/item_spec.rb @@ -390,6 +390,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")) From 7f166cc5991e669fecc996cf77092075d1e46d0f Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Thu, 8 Aug 2024 22:38:55 +0000 Subject: [PATCH 09/21] Fix lint --- app/services/kit_create_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/kit_create_service.rb b/app/services/kit_create_service.rb index 42853e38be..f042af0b5d 100644 --- a/app/services/kit_create_service.rb +++ b/app/services/kit_create_service.rb @@ -63,6 +63,7 @@ def kit_params_with_organization organization_id: organization.id }) end + def valid? if organization.blank? errors.add(:organization_id, 'does not match any Organization') @@ -83,6 +84,5 @@ def kit_validation_errors kit.valid? @kit_validation_errors = kit.errors - end end From 2c5ed05114496c1056201618127388bb4f02953e Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Fri, 9 Aug 2024 01:33:16 +0000 Subject: [PATCH 10/21] rename findorcreatebaseitem to add ! --- app/controllers/admin/base_items_controller.rb | 2 +- app/models/base_item.rb | 2 +- app/services/kit_create_service.rb | 6 +++--- spec/requests/admin/base_items_requests_spec.rb | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/base_items_controller.rb b/app/controllers/admin/base_items_controller.rb index c62e9ce6cf..2bd658613e 100644 --- a/app/controllers/admin/base_items_controller.rb +++ b/app/controllers/admin/base_items_controller.rb @@ -41,7 +41,7 @@ def show # 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.id = KitCreateService.FindOrCreateKitBaseItem.id) + 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!" diff --git a/app/models/base_item.rb b/app/models/base_item.rb index c3effddb30..eba5785e35 100644 --- a/app/models/base_item.rb +++ b/app/models/base_item.rb @@ -46,6 +46,6 @@ def self.seed_base_items end end # Create global 'Kit' base item - KitCreateService.FindOrCreateKitBaseItem + KitCreateService.FindOrCreateKitBaseItem! end end diff --git a/app/services/kit_create_service.rb b/app/services/kit_create_service.rb index f042af0b5d..3bf13e9f80 100644 --- a/app/services/kit_create_service.rb +++ b/app/services/kit_create_service.rb @@ -9,7 +9,7 @@ class KitCreateService attr_reader :kit - def self.FindOrCreateKitBaseItem + def self.FindOrCreateKitBaseItem! BaseItem.find_or_create_by!(KIT_BASE_ITEM_ATTRS) end @@ -27,9 +27,9 @@ def call @kit.save! # 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) + item_housing_a_kit_base_item = KitCreateService.FindOrCreateKitBaseItem! - # Create the Item. + # Create the item item_creation = ItemCreateService.new( organization_id: organization.id, item_params: { diff --git a/spec/requests/admin/base_items_requests_spec.rb b/spec/requests/admin/base_items_requests_spec.rb index a539f52216..aee105de93 100644 --- a/spec/requests/admin/base_items_requests_spec.rb +++ b/spec/requests/admin/base_items_requests_spec.rb @@ -10,7 +10,7 @@ end it "doesn't let you delete the Kit base item" do - kit_base_item = KitCreateService.FindOrCreateKitBaseItem + 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 From 141d959acb448ba55d4f8ce7ec72b40e8ed74543 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Thu, 15 Aug 2024 00:18:11 +0000 Subject: [PATCH 11/21] 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) --- .../reports/children_served_report_service.rb | 39 ------------ .../children_served_report_service_spec.rb | 61 +------------------ 2 files changed, 1 insertion(+), 99 deletions(-) diff --git a/app/services/reports/children_served_report_service.rb b/app/services/reports/children_served_report_service.rb index 1242ba29a6..70886d5e00 100644 --- a/app/services/reports/children_served_report_service.rb +++ b/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 diff --git a/spec/services/reports/children_served_report_service_spec.rb b/spec/services/reports/children_served_report_service_spec.rb index e54268cb65..91551f299b 100644 --- a/spec/services/reports/children_served_report_service_spec.rb +++ b/spec/services/reports/children_served_report_service_spec.rb @@ -42,6 +42,7 @@ create_list(:line_item, 5, :distribution, quantity: 200, item: disposable_item, itemizable: dist) create_list(:line_item, 5, :distribution, quantity: 300, item: non_disposable_item, itemizable: dist) end + # within_time total distributed i infant_distribution = create(:distribution, organization: organization, issued_at: within_time) toddler_distribution = create(:distribution, organization: organization, issued_at: within_time) @@ -105,64 +106,4 @@ }) end end - describe "#disposable_diapers_from_kits_total" do - it "calculates the number of disposable diapers that have been distributed within kits this year" do - organization = create(:organization) - - # create disposable/ nondisposable base items - 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") - create(:base_item, name: "Infant Cloth Diaper", partner_key: "infant cloth diapers", category: "cloth diaper") - create(:base_item, name: "Adult Brief LXL Test", partner_key: "adult lxl test", category: "Diapers - Adult Briefs") - - # create disposable/ nondisposable items - toddler_disposable_kit_item = create(:item, name: "Toddler Disposable Diaper", partner_key: "toddler diapers", organization: organization) - infant_disposable_kit_item = create(:item, name: "Infant Disposable Diapers", partner_key: "infant diapers", organization: organization) - infant_cloth_kit_item = create(:item, name: "Infant Cloth Diapers", partner_key: "infant cloth diapers", organization: organization) - adult_brief_kit_item = create(:item, name: "Adult Brief L/XL", partner_key: "adult lxl test", organization: organization) - - # create line items that contain the d/nd items - toddler_disposable_line_item = create(:line_item, item: toddler_disposable_kit_item, quantity: 5) - infant_disposable_line_item = create(:line_item, item: infant_disposable_kit_item, quantity: 5) - infant_cloth_line_item = create(:line_item, item: infant_cloth_kit_item, quantity: 5) - adult_brief_line_item = create(:line_item, item: adult_brief_kit_item, quantity: 5) - - # create kits that contain the d/nd line items - toddler_disposable_kit = create(:kit, organization: organization, line_items: [toddler_disposable_line_item]) - infant_disposable_kit = create(:kit, organization: organization, line_items: [infant_disposable_line_item]) - infant_cloth_kit = create(:kit, organization: organization, line_items: [infant_cloth_line_item]) - adult_brief_kit = create(:kit, organization: organization, line_items: [adult_brief_line_item]) - - # create items which have the kits - create(:base_item, name: "Unrelated Base", partner_key: "unrelated base", category: "unrelated base") - infant_disposable_dist_item = create(:item, name: "Dist Item 1", organization: organization, partner_key: "unrelated base", kit: toddler_disposable_kit) - toddler_disposable_dist_item = create(:item, name: "Dist Item 2", organization: organization, partner_key: "unrelated base", kit: infant_disposable_kit) - infant_cloth_dist_item = create(:item, name: "Dist Item 3", organization: organization, partner_key: "unrelated base", kit: infant_cloth_kit) - adult_brief_dist_item = create(:item, name: "Dist Item 4", organization: organization, partner_key: "unrelated base", kit: adult_brief_kit) - - within_time = Time.zone.parse("2020-05-31 14:00:00") - - # create empty distributions - infant_distribution = create(:distribution, organization: organization, issued_at: within_time) - toddler_distribution = create(:distribution, organization: organization, issued_at: within_time) - adult_distribution = create(:distribution, organization: organization, issued_at: within_time) - - # add line items to distributions which contain the d/nd kits - create(:line_item, quantity: 10, item: toddler_disposable_dist_item, itemizable: toddler_distribution) - create(:line_item, quantity: 10, item: infant_disposable_dist_item, itemizable: infant_distribution) - create(:line_item, quantity: 10, item: infant_cloth_dist_item, itemizable: infant_distribution) - create(:line_item, quantity: 10, item: adult_brief_dist_item, itemizable: adult_distribution) - - service = described_class.new(organization: organization, year: within_time.year) - - # Find distributions, that has a - # Line item, that has an - # Item, which has a - # Kit, which has a - # Line item, which has an - # Item, which is a disposable diaper. - # And then add all those quantities up - expect(service.disposable_diapers_from_kits_total).to eq(100) - end - end end From 080721e09e9ff452aa69b05f6187b1b8f131ef92 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Thu, 15 Aug 2024 03:45:30 +0000 Subject: [PATCH 12/21] Fix bin/setup so it's working with seed_base_item move --- db/seeds.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/seeds.rb b/db/seeds.rb index 0e38fa44f2..ce7a52bab7 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -500,7 +500,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']) From a111f743c2bc723ba09ccd27b1e99d1cb167b799 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Tue, 24 Sep 2024 01:33:48 +0000 Subject: [PATCH 13/21] Revert "REFACTOR clean up unused line item trait introduced in #4163" This reverts commit ceec0023a7143c5f341868acd34a209f4c35d669. --- spec/factories/line_items.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/factories/line_items.rb b/spec/factories/line_items.rb index d6bdddc5ad..913be2b479 100644 --- a/spec/factories/line_items.rb +++ b/spec/factories/line_items.rb @@ -51,5 +51,10 @@ itemizable_type { "Transfer" } itemizable_id { create(:transfer).id } end + + trait :kit do + itemizable_type { "Kit" } + itemizable_id { create(:kit).id } + end end end From 81c99af2bf9bab456dfa2daf37545368a464f488 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Tue, 24 Sep 2024 01:34:12 +0000 Subject: [PATCH 14/21] Revert "FIX typo in params in donation and purchase controllers" This reverts commit 7cabfa6eb875d1040efd77d1c6059aa5889b9f53. --- app/controllers/donations_controller.rb | 2 +- app/controllers/purchases_controller.rb | 2 +- app/services/itemizable_update_service.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index 1c44db3299..a925ab6c41 100644 --- a/app/controllers/donations_controller.rb +++ b/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_items_attributes) + return params unless params[:donation].key?(:line_item_attributes) params[:donation][:line_items_attributes].delete_if { |_row, data| data["quantity"].blank? && data["item_id"].blank? } params diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb index 6053712c7d..32e288c6b6 100644 --- a/app/controllers/purchases_controller.rb +++ b/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_items_attributes) + return params unless params[:purchase].key?(:line_item_attributes) params[:purchase][:line_items_attributes].delete_if { |_row, data| data["quantity"].blank? && data["item_id"].blank? } params diff --git a/app/services/itemizable_update_service.rb b/app/services/itemizable_update_service.rb index 972b432efe..45dc7133da 100644 --- a/app/services/itemizable_update_service.rb +++ b/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_items_attributes`. + # @param params [Hash] Parameters passed from the controller. Should include `line_item_attributes`. # @param event_class [Class] the event class to publish the itemizable to. def self.call(itemizable:, type: :increase, params: {}, event_class: nil) StorageLocation.transaction do From 7b4da7db31ebb637d5dcc6c829bbc3221bebc499 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Tue, 24 Sep 2024 01:34:29 +0000 Subject: [PATCH 15/21] Revert "DOCS add comment clarifying itemizable works with line items" This reverts commit 1109170fffcd97178d44ecd16347c40f80cc5463. --- app/models/concerns/itemizable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb index c77f6d2a21..9248238f8b 100644 --- a/app/models/concerns/itemizable.rb +++ b/app/models/concerns/itemizable.rb @@ -1,5 +1,5 @@ # Creates a veritable powerhouse. -# This module provides Duck Typed behaviors for anything that shuttles LINE ITEMS (not items) +# This module provides Duck Typed behaviors for anything that shuttle 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 From 04917ba1f25bca20ba07ebc3db54acd8ac579b61 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Tue, 24 Sep 2024 01:34:42 +0000 Subject: [PATCH 16/21] Revert "REFACTOR rename Item:kits scope to :housing_a_kit, add rspecs" This reverts commit b9b875548f03825650b37c0dda0f7d008cc17056. --- app/models/item.rb | 4 ++-- spec/models/item_spec.rb | 23 ----------------------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/app/models/item.rb b/app/models/item.rb index 8ecf42f1b8..66ca08f5c7 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -51,8 +51,8 @@ class Item < ApplicationRecord scope :active, -> { where(active: true) } - # :housing_a_kit are items which house a kit, NOT items is_in_kit - scope :housing_a_kit, -> { where.not(kit_id: nil) } + # Add spec for these + scope :kits, -> { where.not(kit_id: nil) } scope :loose, -> { where(kit_id: nil) } scope :visible, -> { where(visible_to_partners: true) } diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb index 1b0004b6bf..f5d92f4f09 100644 --- a/spec/models/item_spec.rb +++ b/spec/models/item_spec.rb @@ -57,29 +57,6 @@ expect(Item.by_size("4").length).to eq(2) end - it "->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 - - it "->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 - it "->alphabetized retrieves items in alphabetical order" do item_c = create(:item, name: "C") item_b = create(:item, name: "B") From e4de92c5dd713c33ecbd5794288f1f277a3f00d1 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Tue, 24 Sep 2024 01:45:36 +0000 Subject: [PATCH 17/21] Revert "REFACTOR DRY up kit base_item, seed_base_items, prevent kit base_item deletion" This reverts commit 8c18e4286748cef70b06b8c8e0e013e724d92b24. --- .../admin/base_items_controller.rb | 5 +-- app/models/base_item.rb | 20 ---------- app/services/kit_create_service.rb | 40 +++++++++++++------ db/seeds.rb | 17 +++++++- spec/factories/organizations.rb | 2 +- spec/rails_helper.rb | 18 +++++++++ .../admin/base_items_requests_spec.rb | 14 +------ 7 files changed, 65 insertions(+), 51 deletions(-) diff --git a/app/controllers/admin/base_items_controller.rb b/app/controllers/admin/base_items_controller.rb index 2bd658613e..90d37f0a0e 100644 --- a/app/controllers/admin/base_items_controller.rb +++ b/app/controllers/admin/base_items_controller.rb @@ -38,12 +38,9 @@ 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.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 + if @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?" diff --git a/app/models/base_item.rb b/app/models/base_item.rb index eba5785e35..2623528887 100644 --- a/app/models/base_item.rb +++ b/app/models/base_item.rb @@ -28,24 +28,4 @@ class BaseItem < ApplicationRecord def to_h { partner_key: partner_key, name: name } 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 diff --git a/app/services/kit_create_service.rb b/app/services/kit_create_service.rb index 3bf13e9f80..b797af25fa 100644 --- a/app/services/kit_create_service.rb +++ b/app/services/kit_create_service.rb @@ -1,18 +1,8 @@ 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 @@ -26,15 +16,16 @@ def call @kit = Kit.new(kit_params_with_organization) @kit.save! - # Find or create the BaseItem for all items housing kits - item_housing_a_kit_base_item = KitCreateService.FindOrCreateKitBaseItem! + # Create a BaseItem that houses each + # kit item created. + kit_base_item = fetch_or_create_kit_base_item # Create the item item_creation = ItemCreateService.new( organization_id: organization.id, item_params: { name: kit.name, - partner_key: item_housing_a_kit_base_item.partner_key, + partner_key: kit_base_item.partner_key, kit_id: kit.id } ) @@ -54,6 +45,7 @@ def call private attr_reader :organization_id, :kit_params + def organization @organization ||= Organization.find_by(id: organization_id) end @@ -64,6 +56,18 @@ 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') @@ -85,4 +89,14 @@ 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 diff --git a/db/seeds.rb b/db/seeds.rb index ce7a52bab7..b5923c79f8 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -19,11 +19,26 @@ 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 # ---------------------------------------------------------------------------- -BaseItem.seed_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' +) # ---------------------------------------------------------------------------- # NDBN Members diff --git a/spec/factories/organizations.rb b/spec/factories/organizations.rb index 945075bb5d..30ef97daa7 100644 --- a/spec/factories/organizations.rb +++ b/spec/factories/organizations.rb @@ -61,7 +61,7 @@ trait :with_items do after(:create) do |instance, evaluator| - BaseItem.seed_base_items if BaseItem.count.zero? # seeds 45 base items if none exist + 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 diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index ccd77cad60..ec498f97c3 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -244,3 +244,21 @@ 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 diff --git a/spec/requests/admin/base_items_requests_spec.rb b/spec/requests/admin/base_items_requests_spec.rb index aee105de93..ee04d33c1e 100644 --- a/spec/requests/admin/base_items_requests_spec.rb +++ b/spec/requests/admin/base_items_requests_spec.rb @@ -1,25 +1,15 @@ 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) - 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 + sign_in(@super_admin) 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) From 96b3ceb6382dd137955ac84747b1366361e01887 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Tue, 24 Sep 2024 01:45:55 +0000 Subject: [PATCH 18/21] Revert "Add rspec for item.is_in_kit?" This reverts commit e1e2c316a9e0a8fee7a29bbd58dbec2b07759cc2. --- spec/models/item_spec.rb | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb index f5d92f4f09..36c001ab65 100644 --- a/spec/models/item_spec.rb +++ b/spec/models/item_spec.rb @@ -367,19 +367,6 @@ 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")) From af1dfd5d5592dcb3db83db3e69263310ff594ccf Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Tue, 24 Sep 2024 01:50:55 +0000 Subject: [PATCH 19/21] Revert "REFACTOR remove unused code childrenservedreportservice" This reverts commit 141d959acb448ba55d4f8ce7ec72b40e8ed74543. --- .../reports/children_served_report_service.rb | 39 ++++++++++++ .../children_served_report_service_spec.rb | 61 ++++++++++++++++++- 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/app/services/reports/children_served_report_service.rb b/app/services/reports/children_served_report_service.rb index 70886d5e00..1242ba29a6 100644 --- a/app/services/reports/children_served_report_service.rb +++ b/app/services/reports/children_served_report_service.rb @@ -31,8 +31,47 @@ 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 diff --git a/spec/services/reports/children_served_report_service_spec.rb b/spec/services/reports/children_served_report_service_spec.rb index 91551f299b..e54268cb65 100644 --- a/spec/services/reports/children_served_report_service_spec.rb +++ b/spec/services/reports/children_served_report_service_spec.rb @@ -42,7 +42,6 @@ create_list(:line_item, 5, :distribution, quantity: 200, item: disposable_item, itemizable: dist) create_list(:line_item, 5, :distribution, quantity: 300, item: non_disposable_item, itemizable: dist) end - # within_time total distributed i infant_distribution = create(:distribution, organization: organization, issued_at: within_time) toddler_distribution = create(:distribution, organization: organization, issued_at: within_time) @@ -106,4 +105,64 @@ }) end end + describe "#disposable_diapers_from_kits_total" do + it "calculates the number of disposable diapers that have been distributed within kits this year" do + organization = create(:organization) + + # create disposable/ nondisposable base items + 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") + create(:base_item, name: "Infant Cloth Diaper", partner_key: "infant cloth diapers", category: "cloth diaper") + create(:base_item, name: "Adult Brief LXL Test", partner_key: "adult lxl test", category: "Diapers - Adult Briefs") + + # create disposable/ nondisposable items + toddler_disposable_kit_item = create(:item, name: "Toddler Disposable Diaper", partner_key: "toddler diapers", organization: organization) + infant_disposable_kit_item = create(:item, name: "Infant Disposable Diapers", partner_key: "infant diapers", organization: organization) + infant_cloth_kit_item = create(:item, name: "Infant Cloth Diapers", partner_key: "infant cloth diapers", organization: organization) + adult_brief_kit_item = create(:item, name: "Adult Brief L/XL", partner_key: "adult lxl test", organization: organization) + + # create line items that contain the d/nd items + toddler_disposable_line_item = create(:line_item, item: toddler_disposable_kit_item, quantity: 5) + infant_disposable_line_item = create(:line_item, item: infant_disposable_kit_item, quantity: 5) + infant_cloth_line_item = create(:line_item, item: infant_cloth_kit_item, quantity: 5) + adult_brief_line_item = create(:line_item, item: adult_brief_kit_item, quantity: 5) + + # create kits that contain the d/nd line items + toddler_disposable_kit = create(:kit, organization: organization, line_items: [toddler_disposable_line_item]) + infant_disposable_kit = create(:kit, organization: organization, line_items: [infant_disposable_line_item]) + infant_cloth_kit = create(:kit, organization: organization, line_items: [infant_cloth_line_item]) + adult_brief_kit = create(:kit, organization: organization, line_items: [adult_brief_line_item]) + + # create items which have the kits + create(:base_item, name: "Unrelated Base", partner_key: "unrelated base", category: "unrelated base") + infant_disposable_dist_item = create(:item, name: "Dist Item 1", organization: organization, partner_key: "unrelated base", kit: toddler_disposable_kit) + toddler_disposable_dist_item = create(:item, name: "Dist Item 2", organization: organization, partner_key: "unrelated base", kit: infant_disposable_kit) + infant_cloth_dist_item = create(:item, name: "Dist Item 3", organization: organization, partner_key: "unrelated base", kit: infant_cloth_kit) + adult_brief_dist_item = create(:item, name: "Dist Item 4", organization: organization, partner_key: "unrelated base", kit: adult_brief_kit) + + within_time = Time.zone.parse("2020-05-31 14:00:00") + + # create empty distributions + infant_distribution = create(:distribution, organization: organization, issued_at: within_time) + toddler_distribution = create(:distribution, organization: organization, issued_at: within_time) + adult_distribution = create(:distribution, organization: organization, issued_at: within_time) + + # add line items to distributions which contain the d/nd kits + create(:line_item, quantity: 10, item: toddler_disposable_dist_item, itemizable: toddler_distribution) + create(:line_item, quantity: 10, item: infant_disposable_dist_item, itemizable: infant_distribution) + create(:line_item, quantity: 10, item: infant_cloth_dist_item, itemizable: infant_distribution) + create(:line_item, quantity: 10, item: adult_brief_dist_item, itemizable: adult_distribution) + + service = described_class.new(organization: organization, year: within_time.year) + + # Find distributions, that has a + # Line item, that has an + # Item, which has a + # Kit, which has a + # Line item, which has an + # Item, which is a disposable diaper. + # And then add all those quantities up + expect(service.disposable_diapers_from_kits_total).to eq(100) + end + end end From 6b505185f442fcd079465c630f69d53b57739205 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Tue, 24 Sep 2024 01:53:09 +0000 Subject: [PATCH 20/21] Revert "Fix bin/setup so it's working with seed_base_item move" This reverts commit 080721e09e9ff452aa69b05f6187b1b8f131ef92. --- db/seeds.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/seeds.rb b/db/seeds.rb index b5923c79f8..614143cbd0 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -515,7 +515,7 @@ def seed_quantity(item_name, organization, storage_location, quantity) AdjustmentCreateService.new(adjustment).call end -JSON.parse(File.read(Rails.root.join("db", "base_items.json"))).each do |_category, entries| +items_by_category.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']) From 730371a14689e5a8a7c1bab825f4df08fc3e279e Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Wed, 25 Sep 2024 23:49:36 +0000 Subject: [PATCH 21/21] REFACTOR move kit creation in specs into helper method --- spec/events/inventory_aggregate_spec.rb | 14 ++++---------- spec/factories/kits.rb | 6 ++---- spec/models/item_spec.rb | 12 ++++-------- spec/models/kit_spec.rb | 6 +----- spec/requests/kit_requests_spec.rb | 12 ++---------- .../reports/acquisition_report_service_spec.rb | 12 ++++-------- spec/support/kit_helper.rb | 10 ++++++++++ 7 files changed, 27 insertions(+), 45 deletions(-) create mode 100644 spec/support/kit_helper.rb diff --git a/spec/events/inventory_aggregate_spec.rb b/spec/events/inventory_aggregate_spec.rb index a0fa7c4773..27dadc349b 100644 --- a/spec/events/inventory_aggregate_spec.rb +++ b/spec/events/inventory_aggregate_spec.rb @@ -381,13 +381,10 @@ end it "should process a kit allocation event" do - params = FactoryBot.attributes_for(:kit) - params[:line_items_attributes] = [ + kit = create_kit(organization: organization, line_items_attributes: [ {item_id: item1.id, quantity: 10}, {item_id: item2.id, quantity: 3} - ] - - kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit + ]) KitAllocateEvent.publish(kit, storage_location1.id, 2) @@ -419,13 +416,10 @@ end it "should process a kit deallocation event" do - params = FactoryBot.attributes_for(:kit) - params[:line_items_attributes] = [ + kit = create_kit(organization: organization, line_items_attributes: [ {item_id: item1.id, quantity: 20}, {item_id: item2.id, quantity: 5} - ] - - kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit + ]) TestInventory.create_inventory(organization, { diff --git a/spec/factories/kits.rb b/spec/factories/kits.rb index 3a90279870..28a4a3585d 100644 --- a/spec/factories/kits.rb +++ b/spec/factories/kits.rb @@ -23,9 +23,7 @@ end # See #3652, changes to this factory are in progress - # For now, to create corresponding item and line item and persist to database call KitCreateService with: - # kit_params = attributes_for(:kit) - # kit_params[:line_items_attributes] = [{ item_id: _id_, quantity: _quantity_, ...}] - # KitCreateService.new(organization_id: organization.id, kit_params: kit_params).call.kit + # For now, to create corresponding item and line item and persist to database call create_kit + # from spec/support/kit_helper.rb end end diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb index 36c001ab65..109ff4f191 100644 --- a/spec/models/item_spec.rb +++ b/spec/models/item_spec.rb @@ -237,11 +237,9 @@ context "in a kit" do before do - params = FactoryBot.attributes_for(:kit) - params[:line_items_attributes] = [ + create_kit(organization: organization, line_items_attributes: [ {item_id: item.id, quantity: 1} - ] - KitCreateService.new(organization_id: organization.id, kit_params: params).call + ]) end it "should return false" do @@ -275,11 +273,9 @@ context "in a kit" do before do - params = FactoryBot.attributes_for(:kit) - params[:line_items_attributes] = [ + create_kit(organization: organization, line_items_attributes: [ {item_id: item.id, quantity: 1} - ] - KitCreateService.new(organization_id: organization.id, kit_params: params).call + ]) end it "should return false" do diff --git a/spec/models/kit_spec.rb b/spec/models/kit_spec.rb index acfb4d3d36..45b9440b7e 100644 --- a/spec/models/kit_spec.rb +++ b/spec/models/kit_spec.rb @@ -134,11 +134,7 @@ end specify 'deactivate and reactivate' do - params = FactoryBot.attributes_for(:kit) - params[:line_items_attributes] = [ - {item_id: create(:item).id, quantity: 1} - ] - kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit + kit = create_kit(organization: organization) expect(kit.active).to eq(true) expect(kit.item.active).to eq(true) kit.deactivate diff --git a/spec/requests/kit_requests_spec.rb b/spec/requests/kit_requests_spec.rb index 243713a409..1ac17d5935 100644 --- a/spec/requests/kit_requests_spec.rb +++ b/spec/requests/kit_requests_spec.rb @@ -4,11 +4,7 @@ let(:organization_admin) { create(:organization_admin, organization: organization) } let!(:kit) { - params = FactoryBot.attributes_for(:kit) - params[:line_items_attributes] = [ - {item_id: create(:item).id, quantity: 1} - ] - KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit + create_kit(organization: organization) } describe "while signed in" do @@ -19,11 +15,7 @@ describe "GET #index" do before do # this shouldn't be shown - params = FactoryBot.attributes_for(:kit, active: false, name: "DOOBIE KIT") - params[:line_items_attributes] = [ - {item_id: create(:item).id, quantity: 1} - ] - KitCreateService.new(organization_id: organization.id, kit_params: params).call + create_kit(organization: organization, active: false, name: "DOOBIE KIT") end it "should include deactivate" do diff --git a/spec/services/reports/acquisition_report_service_spec.rb b/spec/services/reports/acquisition_report_service_spec.rb index e2f6390bdd..5f3ba09cf3 100644 --- a/spec/services/reports/acquisition_report_service_spec.rb +++ b/spec/services/reports/acquisition_report_service_spec.rb @@ -15,17 +15,13 @@ 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") - params = FactoryBot.attributes_for(:kit) - params[:line_items_attributes] = [ + disposable_kit = create_kit(organization: organization, line_items_attributes: [ {item_id: disposable_kit_item.id, quantity: 5} - ] - disposable_kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit + ]) - params = FactoryBot.attributes_for(:kit) - params[:line_items_attributes] = [ + another_disposable_kit = create_kit(organization: organization, line_items_attributes: [ {item_id: another_disposable_kit_item.id, quantity: 5} - ] - another_disposable_kit = KitCreateService.new(organization_id: organization.id, kit_params: params).call.kit + ]) 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) diff --git a/spec/support/kit_helper.rb b/spec/support/kit_helper.rb new file mode 100644 index 0000000000..958ab7b230 --- /dev/null +++ b/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