From b7e1f07c3b555ad7539441e108c5774a368bed69 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 17 Oct 2025 16:12:42 -0400 Subject: [PATCH 1/6] #3652: Switch line_items from kits to items --- app/controllers/kits_controller.rb | 4 ++-- app/events/kit_allocate_event.rb | 2 +- app/events/kit_deallocate_event.rb | 2 +- app/models/concerns/itemizable.rb | 2 +- app/models/item.rb | 10 ++++++---- app/models/kit.rb | 13 +------------ app/services/kit_create_service.rb | 6 ++++++ app/views/kits/_table.html.erb | 2 +- app/views/kits/allocations.html.erb | 2 +- ...4543_move_line_items_from_kits_to_items.rb | 8 ++++++++ spec/events/inventory_aggregate_spec.rb | 6 +++--- spec/factories/kits.rb | 5 +++-- spec/models/kit_spec.rb | 19 +++++++------------ spec/requests/kit_requests_spec.rb | 2 +- spec/services/kit_create_service_spec.rb | 2 ++ .../period_supply_report_service_spec.rb | 12 ++++++------ 16 files changed, 50 insertions(+), 47 deletions(-) create mode 100644 db/migrate/20251017194543_move_line_items_from_kits_to_items.rb diff --git a/app/controllers/kits_controller.rb b/app/controllers/kits_controller.rb index 5986696bae..b01d3bf722 100644 --- a/app/controllers/kits_controller.rb +++ b/app/controllers/kits_controller.rb @@ -16,7 +16,7 @@ def new load_form_collections @kit = current_organization.kits.new - @kit.line_items.build + @kit.item.line_items.build end def create @@ -33,7 +33,7 @@ def create @kit = Kit.new(kit_params) load_form_collections - @kit.line_items.build if @kit.line_items.empty? + @kit.item.line_items.build if @kit.item.line_items.empty? render :new end diff --git a/app/events/kit_allocate_event.rb b/app/events/kit_allocate_event.rb index e2e397e5e4..6c71e3195d 100644 --- a/app/events/kit_allocate_event.rb +++ b/app/events/kit_allocate_event.rb @@ -1,6 +1,6 @@ class KitAllocateEvent < Event def self.event_line_items(kit, storage_location, quantity) - items = kit.line_items.map do |item| + items = kit.item.line_items.map do |item| EventTypes::EventLineItem.new( quantity: item.quantity * quantity, item_id: item.item_id, diff --git a/app/events/kit_deallocate_event.rb b/app/events/kit_deallocate_event.rb index dd0724f0b6..5ddc7366a4 100644 --- a/app/events/kit_deallocate_event.rb +++ b/app/events/kit_deallocate_event.rb @@ -1,6 +1,6 @@ class KitDeallocateEvent < Event def self.event_line_items(kit, storage_location, quantity) - items = kit.line_items.map do |item| + items = kit.item.line_items.map do |item| EventTypes::EventLineItem.new( quantity: item.quantity * quantity, item_id: item.item_id, diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb index 004f0d3128..6bc82f458d 100644 --- a/app/models/concerns/itemizable.rb +++ b/app/models/concerns/itemizable.rb @@ -107,7 +107,7 @@ def total_value end def value_per_itemizable - line_items.sum(&:value_per_line_item) + item.line_items.sum(&:value_per_line_item) end def total_quantity diff --git a/app/models/item.rb b/app/models/item.rb index 15e7b1382c..d81c1d4cff 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -27,6 +27,7 @@ class Item < ApplicationRecord include Filterable include Exportable include Valuable + include Itemizable after_initialize :set_default_distribution_quantity, if: :new_record? after_update :update_associated_kit_name, if: -> { kit.present? } @@ -45,12 +46,13 @@ class Item < ApplicationRecord validates :on_hand_minimum_quantity, numericality: { greater_than_or_equal_to: 0 } validates :package_size, numericality: { greater_than_or_equal_to: 0 }, allow_blank: true validates :reporting_category, presence: true, unless: proc { |i| i.kit } + validate -> { line_items_quantity_is_at_least(1) } - has_many :line_items, dependent: :destroy + has_many :used_line_items, dependent: :destroy has_many :inventory_items, dependent: :destroy has_many :barcode_items, as: :barcodeable, dependent: :destroy - has_many :donations, through: :line_items, source: :itemizable, source_type: "::Donation" - has_many :distributions, through: :line_items, source: :itemizable, source_type: "::Distribution" + has_many :donations, through: :used_line_items, source: :itemizable, source_type: "::Donation" + has_many :distributions, through: :used_line_items, source: :itemizable, source_type: "::Distribution" has_many :request_units, class_name: "ItemUnit", dependent: :destroy scope :active, -> { where(active: true) } @@ -113,7 +115,7 @@ def is_in_kit?(kits = nil) end def can_delete?(inventory = nil, kits = nil) - can_deactivate_or_delete?(inventory, kits) && line_items.none? && !barcode_count&.positive? && !in_request? && kit.blank? + can_deactivate_or_delete?(inventory, kits) && used_line_items.none? && !barcode_count&.positive? && !in_request? && kit.blank? end # @return [Boolean] diff --git a/app/models/kit.rb b/app/models/kit.rb index 363407266f..87633f514d 100644 --- a/app/models/kit.rb +++ b/app/models/kit.rb @@ -28,9 +28,6 @@ class Kit < ApplicationRecord validates :name, presence: true validates :name, uniqueness: { scope: :organization } - validate :at_least_one_item - validate -> { line_items_quantity_is_at_least(1) } - # @param inventory [View::Inventory] # @return [Boolean] def can_deactivate?(inventory = nil) @@ -47,19 +44,11 @@ def deactivate # or deallocated, we are changing inventory for inactive items (which we don't allow). # @return [Boolean] def can_reactivate? - line_items.joins(:item).where(items: { active: false }).none? + item.line_items.joins(:item).where(items: { active: false }).none? end def reactivate update!(active: true) item.update!(active: true) end - - private - - def at_least_one_item - unless line_items.any? - errors.add(:base, 'At least one item is required') - end - end end diff --git a/app/services/kit_create_service.rb b/app/services/kit_create_service.rb index 7884d5d61c..8681536b72 100644 --- a/app/services/kit_create_service.rb +++ b/app/services/kit_create_service.rb @@ -23,8 +23,13 @@ def call organization.transaction do # Create the Kit record + line_items = kit_params.delete(:line_items_attributes) @kit = Kit.new(kit_params_with_organization) @kit.save! + if line_items.none? + @kit.errors.add(:base, 'At least one item is required') + raise ActiveRecord::RecordInvalid.new(@kit) + end # Find or create the BaseItem for all items housing kits item_housing_a_kit_base_item = KitCreateService.find_or_create_kit_base_item! @@ -33,6 +38,7 @@ def call item_creation = ItemCreateService.new( organization_id: organization.id, item_params: { + line_items_attributes: line_items, name: kit.name, partner_key: item_housing_a_kit_base_item.partner_key, kit_id: kit.id diff --git a/app/views/kits/_table.html.erb b/app/views/kits/_table.html.erb index 5f633ea8f1..119ef0986f 100644 --- a/app/views/kits/_table.html.erb +++ b/app/views/kits/_table.html.erb @@ -13,7 +13,7 @@ <%= kit.name %> diff --git a/app/views/kits/allocations.html.erb b/app/views/kits/allocations.html.erb index c0a07d527d..7b1f943331 100644 --- a/app/views/kits/allocations.html.erb +++ b/app/views/kits/allocations.html.erb @@ -100,7 +100,7 @@ <%= @kit.name %> - <% @kit.line_items.each do |li| %> + <% @kit.item.line_items.each do |li| %> <%= li.item.name %> diff --git a/db/migrate/20251017194543_move_line_items_from_kits_to_items.rb b/db/migrate/20251017194543_move_line_items_from_kits_to_items.rb new file mode 100644 index 0000000000..76d33af3df --- /dev/null +++ b/db/migrate/20251017194543_move_line_items_from_kits_to_items.rb @@ -0,0 +1,8 @@ +class MoveLineItemsFromKitsToItems < ActiveRecord::Migration[8.0] + def change + Item.where.not(kit_id: nil).each do |item| + LineItem.where(itemizable_type: 'Kit', itemizable_id: item.kit_id). + update_all(itemizable_type: 'Item', itemizable_id: item.id, updated_at: Time.current) + end + end +end diff --git a/spec/events/inventory_aggregate_spec.rb b/spec/events/inventory_aggregate_spec.rb index 392b24b389..22e58a9f88 100644 --- a/spec/events/inventory_aggregate_spec.rb +++ b/spec/events/inventory_aggregate_spec.rb @@ -436,9 +436,9 @@ }) 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) + kit.item.line_items = [] + kit.item.line_items << build(:line_item, quantity: 20, item: item1, itemizable: kit) + kit.item.line_items << build(:line_item, quantity: 5, item: item2, itemizable: kit) KitDeallocateEvent.publish(kit, storage_location1.id, 2) # 30 + (20*2) = 70, 10 + (5*2) = 20 diff --git a/spec/factories/kits.rb b/spec/factories/kits.rb index a80683d019..2530e2e735 100644 --- a/spec/factories/kits.rb +++ b/spec/factories/kits.rb @@ -17,8 +17,9 @@ organization after(:build) do |instance, _| - if instance.line_items.blank? - instance.line_items << build(:line_item, item: create(:item, organization: instance.organization), itemizable: nil) + instance.item ||= build(:item, organization: instance.organization) + if instance.item.line_items.blank? + instance.item.line_items << build(:line_item, item: create(:item, organization: instance.organization), itemizable: nil) end end diff --git a/spec/models/kit_spec.rb b/spec/models/kit_spec.rb index e4c308b858..699e4fb546 100644 --- a/spec/models/kit_spec.rb +++ b/spec/models/kit_spec.rb @@ -31,24 +31,19 @@ ).not_to be_valid end - it "requires at least one item" do - kit.line_items = [] - expect(kit).not_to be_valid - end - it "ensures the associated line_items are invalid with a nil quantity" do - kit.line_items << build(:line_item, quantity: nil) - expect(kit).not_to be_valid + kit.item.line_items << build(:line_item, quantity: nil) + expect(kit.item).not_to be_valid end it "ensures the associated line_items are invalid with a zero quantity" do - kit.line_items << build(:line_item, quantity: 0) - expect(kit).not_to be_valid + kit.item.line_items << build(:line_item, quantity: 0) + expect(kit.item).not_to be_valid end it "ensures the associated line_items are valid with a one quantity" do - kit.line_items << build(:line_item, quantity: 1) - expect(kit).to be_valid + kit.item.line_items << build(:line_item, quantity: 1) + expect(kit.item).to be_valid end end @@ -90,7 +85,7 @@ context "Value >" do describe ".value_per_itemizable" do it "calculates values from associated items" do - kit.line_items = [ + kit.item.line_items = [ create(:line_item, quantity: 1, item: create(:item, value_in_cents: 100)), create(:line_item, quantity: 1, item: create(:item, value_in_cents: 90)) ] diff --git a/spec/requests/kit_requests_spec.rb b/spec/requests/kit_requests_spec.rb index 17450308dc..b2ab7da695 100644 --- a/spec/requests/kit_requests_spec.rb +++ b/spec/requests/kit_requests_spec.rb @@ -83,7 +83,7 @@ it "cannot reactivate if it has an inactive item" do kit.deactivate expect(kit).not_to be_active - kit.line_items.first.item.update!(active: false) + kit.item.line_items.first.item.update!(active: false) put reactivate_kit_url(kit) expect(kit.reload).not_to be_active diff --git a/spec/services/kit_create_service_spec.rb b/spec/services/kit_create_service_spec.rb index 7a81c93683..7eed75e08a 100644 --- a/spec/services/kit_create_service_spec.rb +++ b/spec/services/kit_create_service_spec.rb @@ -32,6 +32,8 @@ context 'when the parameters are valid' do it 'should create a new Kit' do expect { subject }.to change { Kit.all.count }.by(1) + kit = Kit.last + expect(kit.item.line_items.count).to eq(3) end it 'should create a new Item' do diff --git a/spec/services/reports/period_supply_report_service_spec.rb b/spec/services/reports/period_supply_report_service_spec.rb index 4dd8d55b6f..b5ac34f4b1 100644 --- a/spec/services/reports/period_supply_report_service_spec.rb +++ b/spec/services/reports/period_supply_report_service_spec.rb @@ -44,13 +44,13 @@ another_period_supplies_kit_item = create(:item, name: "Adult Tampons", reporting_category: :tampons, organization:) purchased_period_supplies_kit_item = create(:item, name: "Liners", reporting_category: :period_liners, organization:) - period_supplies_kit.line_items.first.update!(item_id: period_supplies_kit_item.id, quantity: 5) - another_period_supply_kit.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 5) - donated_period_supply_kit.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 5) - purchased_period_supply_kit.line_items.first.update!(item_id: purchased_period_supplies_kit_item.id, quantity: 5) + period_supplies_kit.item.line_items.first.update!(item_id: period_supplies_kit_item.id, quantity: 5) + another_period_supply_kit.item.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 5) + donated_period_supply_kit.item.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 5) + purchased_period_supply_kit.item.line_items.first.update!(item_id: purchased_period_supplies_kit_item.id, quantity: 5) - pad_and_tampon_kit.line_items.first.update!(item_id: period_supplies_kit_item.id, quantity: 10) - pad_and_tampon_kit.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 10) + pad_and_tampon_kit.item.line_items.first.update!(item_id: period_supplies_kit_item.id, quantity: 10) + pad_and_tampon_kit.item.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 10) period_supplies_kit_distribution = create(:distribution, organization: organization, issued_at: within_time) another_period_supplies_kit_distribution = create(:distribution, organization: organization, issued_at: within_time) From 84bd594d2f5e5dd41be7a141c93658435200aec2 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 17 Oct 2025 16:22:02 -0400 Subject: [PATCH 2/6] fix spec --- app/services/reports/period_supply_report_service.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/reports/period_supply_report_service.rb b/app/services/reports/period_supply_report_service.rb index 1e542bf06b..4383558531 100644 --- a/app/services/reports/period_supply_report_service.rb +++ b/app/services/reports/period_supply_report_service.rb @@ -114,13 +114,13 @@ def kit_items_calculation(itemizable_type, string_itemizable_type) FROM #{itemizable_type} INNER JOIN line_items ON line_items.itemizable_type = #{string_itemizable_type} AND line_items.itemizable_id = #{itemizable_type}.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 line_items AS kit_line_items ON items.id = kit_line_items.itemizable_id INNER JOIN items AS kit_items ON kit_items.id = kit_line_items.item_id WHERE #{itemizable_type}.organization_id = ? AND EXTRACT(year FROM issued_at) = ? + AND items.kit_id IS NOT NULL AND kit_items.reporting_category IN ('pads', 'tampons', 'period_liners', 'period_underwear', 'period_other') - AND kit_line_items.itemizable_type = 'Kit'; + AND kit_line_items.itemizable_type = 'Item'; SQL sanitized_sql = ActiveRecord::Base.send(:sanitize_sql_array, [sql_query, organization_id, year]) From c43aea2038152af26bf11626ba05305fdf420862 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 17 Oct 2025 16:36:42 -0400 Subject: [PATCH 3/6] fixes --- app/models/concerns/itemizable.rb | 2 +- app/models/item.rb | 4 ++-- spec/models/kit_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb index 6bc82f458d..004f0d3128 100644 --- a/app/models/concerns/itemizable.rb +++ b/app/models/concerns/itemizable.rb @@ -107,7 +107,7 @@ def total_value end def value_per_itemizable - item.line_items.sum(&:value_per_line_item) + line_items.sum(&:value_per_line_item) end def total_quantity diff --git a/app/models/item.rb b/app/models/item.rb index d81c1d4cff..7982075a84 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -48,7 +48,7 @@ class Item < ApplicationRecord validates :reporting_category, presence: true, unless: proc { |i| i.kit } validate -> { line_items_quantity_is_at_least(1) } - has_many :used_line_items, dependent: :destroy + has_many :used_line_items, dependent: :destroy, class_name: "LineItem" has_many :inventory_items, dependent: :destroy has_many :barcode_items, as: :barcodeable, dependent: :destroy has_many :donations, through: :used_line_items, source: :itemizable, source_type: "::Donation" @@ -109,7 +109,7 @@ def is_in_kit?(kits = nil) else organization.kits .active - .joins(:line_items) + .joins(item: :line_items) .where(line_items: { item_id: id}).any? end end diff --git a/spec/models/kit_spec.rb b/spec/models/kit_spec.rb index 699e4fb546..1f805f5c90 100644 --- a/spec/models/kit_spec.rb +++ b/spec/models/kit_spec.rb @@ -89,7 +89,7 @@ create(:line_item, quantity: 1, item: create(:item, value_in_cents: 100)), create(:line_item, quantity: 1, item: create(:item, value_in_cents: 90)) ] - expect(kit.value_per_itemizable).to eq(190) + expect(kit.item.value_per_itemizable).to eq(190) end end From 07a7008a8994ad38b4005a8d588c0a5602513408 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 17 Oct 2025 16:39:52 -0400 Subject: [PATCH 4/6] moar fix --- app/services/reports/diaper_report_service.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/reports/diaper_report_service.rb b/app/services/reports/diaper_report_service.rb index 19cc76cec1..19a6f83434 100644 --- a/app/services/reports/diaper_report_service.rb +++ b/app/services/reports/diaper_report_service.rb @@ -61,13 +61,13 @@ def distributed_disposable_diapers_from_kits 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 line_items AS kit_line_items ON items.id = kit_line_items.itemizable_id INNER JOIN items AS kit_items ON kit_items.id = kit_line_items.item_id WHERE distributions.organization_id = ? AND EXTRACT(year FROM issued_at) = ? + AND items.kit_id IS NOT NULL AND kit_items.reporting_category = 'disposable_diapers' - AND kit_line_items.itemizable_type = 'Kit'; + AND kit_line_items.itemizable_type = 'Item'; SQL sanitized_sql = ActiveRecord::Base.send(:sanitize_sql_array, [sql_query, organization_id, year]) From 7a9339558c5912b619628e3e3a274d77a9fdad82 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Mon, 20 Oct 2025 10:42:06 -0400 Subject: [PATCH 5/6] fix --- app/controllers/kits_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/kits_controller.rb b/app/controllers/kits_controller.rb index b01d3bf722..e929c38011 100644 --- a/app/controllers/kits_controller.rb +++ b/app/controllers/kits_controller.rb @@ -16,6 +16,7 @@ def new load_form_collections @kit = current_organization.kits.new + @kit.item = current_organization.items.new @kit.item.line_items.build end From 92646704d3e360b6cdd625a7d571600df17d56ad Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 24 Oct 2025 16:23:50 -0400 Subject: [PATCH 6/6] spec fixes --- app/controllers/kits_controller.rb | 9 ++++++--- app/services/kit_create_service.rb | 2 +- app/views/kits/_form.html.erb | 24 +++++++++++++----------- spec/system/kit_system_spec.rb | 10 +++++----- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/app/controllers/kits_controller.rb b/app/controllers/kits_controller.rb index e929c38011..a8c2e29eca 100644 --- a/app/controllers/kits_controller.rb +++ b/app/controllers/kits_controller.rb @@ -34,6 +34,7 @@ def create @kit = Kit.new(kit_params) load_form_collections + @kit.item ||= current_organization.items.new @kit.item.line_items.build if @kit.item.line_items.empty? render :new @@ -88,12 +89,14 @@ def load_form_collections end def kit_params - params.require(:kit).permit( + kit_params = params.require(:kit).permit( :name, :visible_to_partners, - :value_in_dollars, - line_items_attributes: [:item_id, :quantity, :_destroy] + :value_in_dollars ) + item_params = params.require(:item) + .permit(line_items_attributes: [:item_id, :quantity, :_destroy]) + kit_params.to_h.merge(item_params.to_h) end def kit_adjustment_params diff --git a/app/services/kit_create_service.rb b/app/services/kit_create_service.rb index 8681536b72..8c5cda19c4 100644 --- a/app/services/kit_create_service.rb +++ b/app/services/kit_create_service.rb @@ -26,7 +26,7 @@ def call line_items = kit_params.delete(:line_items_attributes) @kit = Kit.new(kit_params_with_organization) @kit.save! - if line_items.none? + if line_items.blank? @kit.errors.add(:base, 'At least one item is required') raise ActiveRecord::RecordInvalid.new(@kit) end diff --git a/app/views/kits/_form.html.erb b/app/views/kits/_form.html.erb index dc250d5873..e5fe5ccac3 100644 --- a/app/views/kits/_form.html.erb +++ b/app/views/kits/_form.html.erb @@ -19,17 +19,19 @@ <%= f.input_field :value_in_dollars, class: "form-control", min: 0 %> <% end %> -
- Items in this Kit -
- <%= render 'line_items/line_item_fields', form: f %> -
- -
+ <%= fields_for @kit.item do |ff| %> +
+ Items in this Kit +
+ <%= render 'line_items/line_item_fields', form: ff %> +
+ +
+ <% end %>