Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/controllers/admin/base_items_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# [Super Admin] Manage the BaseItems -- this is the only place in the app where Base Items can be
# added / modified. Base Items are both the template and common thread for regular Items
#
# See #4656, BaseItems are pending significant changes/possible deletion
class Admin::BaseItemsController < AdminController
def edit
@base_item = BaseItem.find(params[:id])
Expand Down Expand Up @@ -40,7 +42,9 @@ def show

def destroy
@base_item = BaseItem.includes(:items).find(params[:id])
if @base_item.items.any? && @base_item.destroy
if @base_item.id == KitCreateService.find_or_create_kit_base_item!.id
redirect_to admin_base_items_path, alert: "You cannot delete the Kits base item. This is reserved for all Kits."
elsif @base_item.items.empty? && @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?"
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/donations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/purchases_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion app/models/base_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,3 @@ def to_h
{ partner_key: partner_key, name: name }
end
end

2 changes: 1 addition & 1 deletion app/models/concerns/itemizable.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions app/models/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down
2 changes: 1 addition & 1 deletion app/services/itemizable_update_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module ItemizableUpdateService
# @param itemizable [Itemizable]
# @param type [Symbol] :increase or :decrease - if the original line items added quantities (purchases or
# donations), use :increase. If the original line_items reduced quantities (distributions) use :decrease.
# @param params [Hash] Parameters passed from the controller. Should include `line_item_attributes`.
# @param params [Hash] Parameters passed from the controller. Should include `line_items_attributes`.
# @param event_class [Class<Event>] the event class to publish the itemizable to.
def self.call(itemizable:, type: :increase, params: {}, event_class: nil)
StorageLocation.transaction do
Expand Down
43 changes: 14 additions & 29 deletions app/services/kit_create_service.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
class KitCreateService
include ServiceObjectErrorsMixin

KIT_BASE_ITEM_ATTRS = {
name: 'Kit',
category: 'kit',
partner_key: 'kit'
}

attr_reader :kit

def self.find_or_create_kit_base_item!
BaseItem.find_or_create_by!(KIT_BASE_ITEM_ATTRS)
end

def initialize(organization_id:, kit_params:)
@organization_id = organization_id
@kit_params = kit_params
Expand All @@ -16,16 +26,15 @@ def call
@kit = Kit.new(kit_params_with_organization)
@kit.save!

# Create a BaseItem that houses each
# kit item created.
kit_base_item = fetch_or_create_kit_base_item
# Find or create the BaseItem for all items housing kits
item_housing_a_kit_base_item = KitCreateService.find_or_create_kit_base_item!

# Create the Item.
# Create the item
item_creation = ItemCreateService.new(
organization_id: organization.id,
item_params: {
name: kit.name,
partner_key: kit_base_item.partner_key,
partner_key: item_housing_a_kit_base_item.partner_key,
kit_id: kit.id
}
)
Expand All @@ -45,7 +54,6 @@ def call
private

attr_reader :organization_id, :kit_params

def organization
@organization ||= Organization.find_by(id: organization_id)
end
Expand All @@ -56,18 +64,6 @@ def kit_params_with_organization
})
end

def fetch_or_create_kit_base_item
BaseItem.find_or_create_by!({
name: 'Kit',
category: 'kit',
partner_key: 'kit'
})
end

def partner_key_for_kits
'kit'
end

def valid?
if organization.blank?
errors.add(:organization_id, 'does not match any Organization')
Expand All @@ -89,15 +85,4 @@ def kit_validation_errors

@kit_validation_errors = kit.errors
end

def associated_item_params
{
kit: kit.name
}
end

def partner_key_for(name)
"kit_#{name.underscore.gsub(/\s+/, '_')}"
end
end

39 changes: 0 additions & 39 deletions app/services/reports/children_served_report_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,47 +31,8 @@ def average_children_monthly
total_children_served / 12.0
end

def disposable_diapers_from_kits_total
organization_id = @organization.id
Copy link
Contributor Author

@jimmyli97 jimmyli97 Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(continuing conversation from #4585)

these lines were removed because they're unused in this service and duplicated in AcquisitionReportService (probably the author copied AcquisitionReportService as a template when creating this service and forgot to delete them)

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
Expand Down
26 changes: 6 additions & 20 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,12 @@ def random_record_for_org(org, klass)
# Script-Global Variables
# ----------------------------------------------------------------------------

# Initial starting qty for our test organizations
base_items = File.read(Rails.root.join("db", "base_items.json"))
items_by_category = JSON.parse(base_items)

# ----------------------------------------------------------------------------
# Base Items
# ----------------------------------------------------------------------------

items_by_category.each do |category, entries|
entries.each do |entry|
BaseItem.find_or_create_by!(name: entry["name"], category: category, partner_key: entry["key"])
end
end

# Create global 'Kit' base item
BaseItem.find_or_create_by!(
name: 'Kit',
category: 'kit',
partner_key: 'kit'
)
require 'seeds'
Seeds.seed_base_items

# ----------------------------------------------------------------------------
# NDBN Members
Expand Down Expand Up @@ -553,7 +539,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'])
Expand Down Expand Up @@ -870,12 +856,12 @@ def seed_quantity(item_name, organization, storage_location, quantity)
# Users invitation status
# ----------------------------------------------------------------------------
# Mark users `invitation_status` as `accepted`
#
#
# Addresses and resolves issue #4689, which can be found in:
# https://github.com/rubyforgood/human-essentials/issues/4689
User.where(invitation_token: nil).each do |user|
User.where(invitation_token: nil).each do |user|
user.update!(
invitation_sent_at: Time.current,
invitation_accepted_at: Time.current
)
end
end
21 changes: 21 additions & 0 deletions lib/seeds.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module Seeds
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.find_or_create_kit_base_item!
end
end
5 changes: 0 additions & 5 deletions spec/factories/line_items.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 2 additions & 1 deletion spec/factories/organizations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
# account_request_id :integer
# ndbn_member_id :bigint
#
require 'seeds'

FactoryBot.define do
factory :organization do
Expand Down Expand Up @@ -61,7 +62,7 @@

trait :with_items do
after(:create) do |instance, evaluator|
seed_base_items if BaseItem.count.zero? # seeds 45 base items if none exist
Seeds.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
Expand Down
42 changes: 39 additions & 3 deletions spec/models/item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
expect(subject.class).to respond_to :class_filter
end

it "->by_size returns all items with the same size, per their BaseItem parent" do
specify "->by_size returns all items with the same size, per their BaseItem parent" do
size4 = create(:base_item, size: "4", name: "Size 4 Diaper")
size_z = create(:base_item, size: "Z", name: "Size Z Diaper")

Expand All @@ -57,7 +57,30 @@
expect(Item.by_size("4").length).to eq(2)
end

it "->alphabetized retrieves items in alphabetical order" do
specify "->housing_a_kit returns all items which belongs_to (house) a kit" do
name = "test kit"
kit_params = attributes_for(:kit, name: name)
kit_params[:line_items_attributes] = [{item_id: create(:item).id, quantity: 1}] # shouldn't be counted
KitCreateService.new(organization_id: organization.id, kit_params: kit_params).call

create(:item) # shouldn't be counted
expect(Item.housing_a_kit.count).to eq(1)
expect(Item.housing_a_kit.first.name = name)
end

specify "->loose returns all items which do not belongs_to a kit" do
name = "A"
item = create(:item, name: name, organization: organization)

kit_params = attributes_for(:kit)
kit_params[:line_items_attributes] = [{item_id: item.id, quantity: 1}]
KitCreateService.new(organization_id: organization.id, kit_params: kit_params).call # shouldn't be counted

expect(Item.loose.count).to eq(1)
expect(Item.loose.first.name = name)
end

specify "->alphabetized retrieves items in alphabetical order" do
item_c = create(:item, name: "C")
item_b = create(:item, name: "B")
item_a = create(:item, name: "A")
Expand All @@ -67,7 +90,7 @@
expect(Item.alphabetized.map(&:name)).to eq(alphabetized_list)
end

it "->active shows items that are still active" do
specify "->active shows items that are still active" do
inactive_item = create(:line_item, :purchase).item
item = create(:item)
inactive_item.deactivate!
Expand Down Expand Up @@ -362,6 +385,19 @@
end
end

describe '#is_in_kit?' do
it "is true for items that are in a kit and false otherwise" do
item_not_in_kit = create(:item, organization: organization)
item_in_kit = create(:item, organization: organization)

kit_params = attributes_for(:kit)
kit_params[:line_items_attributes] = [{item_id: item_in_kit.id, quantity: 1}]
KitCreateService.new(organization_id: organization.id, kit_params: kit_params).call
expect(item_in_kit.is_in_kit?).to be true
expect(item_not_in_kit.is_in_kit?).to be false
end
end

describe "other?" do
it "is true for items that are partner_key 'other'" do
item = create(:item, base_item: create(:base_item, name: "Base"))
Expand Down
18 changes: 0 additions & 18 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,21 +242,3 @@ def await_select2(select2, container = nil, &block)

find("#{container} select option[data-select2-id=\"#{current_id.to_i + 1}\"]", wait: 10)
end

def seed_base_items
base_items = File.read(Rails.root.join("db", "base_items.json"))
items_by_category = JSON.parse(base_items)
base_items_data = items_by_category.map do |category, entries|
entries.map do |entry|
{
name: entry["name"],
category: category,
partner_key: entry["key"],
updated_at: Time.zone.now,
created_at: Time.zone.now
}
end
end.flatten

BaseItem.create!(base_items_data)
end
Loading