Skip to content
15 changes: 7 additions & 8 deletions app/controllers/requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ def index

def show
@request = Request.find(params[:id])
@request_items = load_items
@item_requests = @request.item_requests.includes(:item)

@inventory = View::Inventory.new(@request.organization_id)
@default_storage_location = @request.partner.default_storage_location_id || @request.organization.default_storage_location
@location = StorageLocation.find_by(id: @default_storage_location)

@custom_units = Flipper.enabled?(:enable_packs) && @request.item_requests.any? { |item| item.request_unit }
Comment on lines +34 to +38
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These variables were previously being built directly in the view.

end

# Clicking the "New Distribution" button will set the the request to started
Expand Down Expand Up @@ -81,13 +87,6 @@ def print_unfulfilled

private

def load_items
return unless @request.request_items

inventory = View::Inventory.new(@request.organization_id)
@request.request_items.map { |json| RequestItem.from_json(json, @request, inventory) }
end

helper_method \
def filter_params
return {} unless params.key?(:filters)
Expand Down
23 changes: 1 addition & 22 deletions app/mailers/requests_confirmation_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class RequestsConfirmationMailer < ApplicationMailer
def confirmation_email(request)
@organization = request.organization
@partner = request.partner
@request_items = fetch_items(request)
@item_requests = request.item_requests.includes(:item)
requester = request.requester
@requester_user_name = requester.is_a?(User) ? requester.name : nil # Requester can be the partner, if no user is specified
# If the organization has opted in to receiving an email when a request is made, CC them
Expand All @@ -15,25 +15,4 @@ def confirmation_email(request)
cc.uniq!
mail(to: requester.email, cc: cc, subject: "#{@organization.name} - Requests Confirmation")
end

private

# TODO: remove the need to de-duplicate items in the request
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Going forward (for a while now), duplication isn't allowed when the request is created in the first place, so we don't need to merge things for the email anymore.

def fetch_items(request)
combined = combined_items(request)
item_ids = combined&.map { |item| item['item_id'] }
db_items = Item.where(id: item_ids).select(:id, :name)
combined.each { |i| i['name'] = db_items.find { |db_item| i["item_id"] == db_item.id }.name }
combined.sort_by { |i| i['name'] }
end

def combined_items(request)
return [] if request.request_items.size == 0
# convert items into a hash of (id => list of items with that ID)
grouped = request.request_items.group_by { |i| [i['item_id'], i['request_unit']] }
# convert hash into an array of items with combined quantities
grouped.map do |id_unit, items|
{ 'item_id' => id_unit.first, 'quantity' => items.map { |i| i['quantity'] }.sum, "unit" => id_unit.last }
end
end
end
17 changes: 15 additions & 2 deletions app/models/partners/item_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,24 @@ def request_unit_is_supported
end
end

# Usually the item_name, but fall back to our local copy
def item_name
item&.name || name
end

def quantity_with_units
if Flipper.enabled?(:enable_packs) && request_unit.present?
"#{quantity} #{request_unit.pluralize(quantity.to_i)}"
else
quantity
end
end

def name_with_unit(quantity_override = nil)
if Flipper.enabled?(:enable_packs) && request_unit.present?
"#{item&.name || name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}"
"#{item_name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you forgot to use the new method here? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is one of the display helpers:

  • ItemRequest#item_name gives the item's name and falls back to the local copy
  • ItemRequest#quantity_with_units shows the qty and units but not the name
  • ItemRequest#name_with_unit shows the name and the unit but not qty

else
item&.name || name
item_name
end
end
end
Expand Down
31 changes: 14 additions & 17 deletions app/pdfs/distribution_pdf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -180,38 +180,35 @@ def request_data
"Value/item",
"In-Kind Value Received",
"Packages"]]
inventory = View::Inventory.new(@distribution.organization_id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interestingly, this appears to already have not been used. This time rubocop noticed and had me remove it.

request_items = @distribution.request.request_items.map do |request_item|
RequestItem.from_json(request_item, @distribution.request, inventory)
end
item_requests = @distribution.request.item_requests.includes(:item)
line_items = @distribution.line_items.sorted

requested_not_received = request_items.select do |request_item|
line_items.none? { |i| i.item_id == request_item.item.id }
requested_not_received = item_requests.select do |item_request|
line_items.none? { |i| i.item_id == item_request.item_id }
end

data += line_items.map do |c|
request_item = request_items.find { |i| i.item&.id == c.item_id }
item_request = item_requests.find { |i| i.item_id == c.item_id }
[c.item.name,
request_display_qty(request_item),
request_display_qty(item_request),
c.quantity,
dollar_value(c.item.value_in_cents),
dollar_value(c.value_per_line_item),
c.package_count]
end

data += requested_not_received.sort_by(&:name).map do |request_item|
[request_item.item.name,
request_display_qty(request_item),
data += requested_not_received.sort_by(&:name).map do |item_request|
[item_request.item.name,
request_display_qty(item_request),
"",
dollar_value(request_item.item.value_in_cents),
dollar_value(item_request.item.value_in_cents),
nil,
nil]
end

data + [["", "", "", "", ""],
["Total Items Received",
request_items.map(&:quantity).sum,
item_requests.sum { |ir| ir.quantity.to_i },
@distribution.line_items.total,
"",
dollar_value(@distribution.value_per_itemizable),
Expand Down Expand Up @@ -290,11 +287,11 @@ def signature_lines_for(label)
draw_text "(Signature and Date)", at: [right_start, cursor]
end

def request_display_qty(request_item)
if Flipper.enabled?(:enable_packs) && request_item&.unit
"#{request_item.quantity} #{request_item.unit.pluralize(request_item.quantity)}"
def request_display_qty(item_request)
if Flipper.enabled?(:enable_packs) && item_request&.request_unit
"#{item_request.quantity} #{item_request.request_unit.pluralize(item_request.quantity.to_i)}"
else
request_item&.quantity || ""
item_request&.quantity || ""
end
end
end
26 changes: 12 additions & 14 deletions app/views/requests/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -67,31 +67,29 @@
<tr>
<th>Item</th>
<th>Quantity</th>
<% custom_units = Flipper.enabled?(:enable_packs) &&
@request.item_requests.any? { |item| item.request_unit } %>
<% if custom_units %>
<% if @custom_units %>
<th>Units (if applicable)</th>
<% end %>
<% default_storage_location = @request.partner.default_storage_location_id ||
@request.organization.default_storage_location %>
<% if default_storage_location %>
<% if @default_storage_location %>
<th>Default storage location inventory</th>
<% end %>
<th>Total Inventory</th>
</tr>
</thead>
<tbody>
<% @request_items.each do |item| %>
<% @item_requests.each do |item_request| %>
<tr>
<td><%= item.name %></td>
<td><%= item.quantity %></td>
<% if custom_units %>
<td><%= item.unit&.pluralize(item.quantity) %></td>
<td><%= item_request.item_name %></td>
<td><%= item_request.quantity %></td>
<% if @custom_units %>
<td><%= item_request.request_unit&.pluralize(item_request.quantity.to_i) %></td>
<% end %>
<% if default_storage_location %>
<td><%= item.on_hand_for_location %></td>
<% if @default_storage_location %>
<% on_hand_for_location = @inventory.quantity_for(storage_location: @location&.id, item_id: item_request.item_id) %>
<td><%= on_hand_for_location&.positive? ? on_hand_for_location : 'N/A' %></td>
<% end %>
<td><%= item.on_hand %></td>
<% on_hand = @inventory.quantity_for(item_id: item_request.item_id) %>
<td><%= on_hand || 0 %></td>
</tr>
<% end %>
<tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,8 @@

<p>This email confirms that <%= @organization.name %> has received a request<%= " submitted by #{@requester_user_name}" if @requester_user_name.present? %> for:</p>
<ul>
<% @request_items.each do |item| %>
<li><%= item['name'] %> -
<% if Flipper.enabled?(:enable_packs) && item['unit'] %>
<%= pluralize(item['quantity'] || item['person_count'], item['unit']) %>
<% else %>
<%= item['quantity'] || item['person_count'] %>
<% end %>
</li>

<% @item_requests.each do |item_request| %>
<li><%= item_request.item_name %> - <%= item_request.quantity_with_units %></li>
<% end %>
</ul>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
Hello <%= @partner.name %>,

This email confirms that <%= @organization.name %> has received a request<%= " submitted by #{@requester_user_name}" if @requester_user_name.present? %> for:
<% @request_items.each do |item| %>
<%= item['name'] %> - <%= item['quantity'] || item['person_count'] %>
<% @item_requests.each do |item_request| %>
<%= item_request.item_name %> - <%= item_request.quantity_with_units %>
<% end %>

You will receive a notification when a distribution has been created of the pick-up time and date.
Expand Down
6 changes: 3 additions & 3 deletions spec/mailers/requests_confirmation_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
let(:request) { create(:request, organization:, partner_user:) }
let(:mail) { RequestsConfirmationMailer.confirmation_email(request) }

let(:request_w_varied_quantities) { create(:request, :with_varied_quantities, organization: organization) }
let(:request_w_varied_quantities) { create(:request, :with_varied_quantities, :with_item_requests, organization: organization) }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A lot of these factories now require :with_item_requests role; in a further PR I'll make that the default and then remove all of the instances of the param.

let(:mail_w_varied_quantities) { RequestsConfirmationMailer.confirmation_email(request_w_varied_quantities) }

describe "#confirmation_email" do
Expand Down Expand Up @@ -69,7 +69,7 @@
{item_id: item1.id, quantity: 1, request_unit: "Pack"},
{item_id: item2.id, quantity: 7, request_unit: "Pack"}
]
request = create(:request, :pending, request_items:)
request = create(:request, :pending, :with_item_requests, request_items:)
email = RequestsConfirmationMailer.confirmation_email(request)
expect(email.body.encoded).to match("1 Pack")
expect(email.body.encoded).to match("7 Packs")
Expand All @@ -79,7 +79,7 @@
Flipper.enable(:enable_packs)
item = create(:item, organization:)
create(:item_unit, item: item, name: "Pack")
request = create(:request, :pending, request_items: [{item_id: item.id, quantity: 7}])
request = create(:request, :pending, :with_item_requests, request_items: [{item_id: item.id, quantity: 7}])
email = RequestsConfirmationMailer.confirmation_email(request)

expect(email.body.encoded).not_to match("7 Packs")
Expand Down
34 changes: 17 additions & 17 deletions spec/pdfs/distribution_pdf_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
expect(results).to eq([
["Items Received", "Requested", "Received", "Value/item", "In-Kind Value Received", "Packages"],
["Item 1", "", 50, "$1.00", "$50.00", "1"],
["Item 2", 30, 100, "$2.00", "$200.00", nil],
["Item 3", 50, "", "$3.00", nil, nil],
["Item 2", "30", 100, "$2.00", "$200.00", nil],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are surprisingly stored as strings; in the old one they ended up number-ified somewhere, but they are turned into strings in the PDF anyway so strings are fine.

["Item 3", "50", "", "$3.00", nil, nil],
["Item 4", "120 packs", "", "$4.00", nil, nil],
["", "", "", "", ""],
["Total Items Received", 200, 150, "", "$250.00", ""]
Expand All @@ -55,9 +55,9 @@
expect(data).to eq([
["Items Received", "Requested", "Received"],
["Item 1", "", 50],
["Item 2", 30, 100],
["Item 3", 50, ""],
["Item 4", 120, ""],
["Item 2", "30", 100],
["Item 3", "50", ""],
["Item 4", "120", ""],
["", "", ""],
["Total Items Received", 200, 150]
])
Expand All @@ -70,9 +70,9 @@
expect(data).to eq([
["Items Received", "Requested", "Received", "Packages"],
["Item 1", "", 50, "1"],
["Item 2", 30, 100, nil],
["Item 3", 50, "", nil],
["Item 4", 120, "", nil],
["Item 2", "30", 100, nil],
["Item 3", "50", "", nil],
["Item 4", "120", "", nil],
["", "", ""],
["Total Items Received", 200, 150, ""]
])
Expand All @@ -88,9 +88,9 @@
expect(data).to eq([
["Items Received", "Requested", "Received"],
["Item 1", "", 50],
["Item 2", 30, 100],
["Item 3", 50, ""],
["Item 4", 120, ""],
["Item 2", "30", 100],
["Item 3", "50", ""],
["Item 4", "120", ""],
["", "", ""],
["Total Items Received", 200, 150]
])
Expand All @@ -103,9 +103,9 @@
expect(data).to eq([
["Items Received", "Requested", "Received", "Packages"],
["Item 1", "", 50, "1"],
["Item 2", 30, 100, nil],
["Item 3", 50, "", nil],
["Item 4", 120, "", nil],
["Item 2", "30", 100, nil],
["Item 3", "50", "", nil],
["Item 4", "120", "", nil],
["", "", ""],
["Total Items Received", 200, 150, ""]
])
Expand All @@ -121,9 +121,9 @@
expect(data).to eq([
["Items Received", "Requested", "Received", "Value/item", "In-Kind Value Received"],
["Item 1", "", 50, "$1.00", "$50.00"],
["Item 2", 30, 100, "$2.00", "$200.00"],
["Item 3", 50, "", "$3.00", nil],
["Item 4", 120, "", "$4.00", nil],
["Item 2", "30", 100, "$2.00", "$200.00"],
["Item 3", "50", "", "$3.00", nil],
["Item 4", "120", "", "$4.00", nil],
["", "", "", "", ""],
["Total Items Received", 200, 150, "", "$250.00"]
])
Expand Down
2 changes: 1 addition & 1 deletion spec/system/request_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@
{ item_id: item2.id, quantity: 100}
]
}
let!(:request) { create(:request, request_items: request_items, organization: organization) }
let!(:request) { create(:request, :with_item_requests, request_items: request_items, organization: organization) }

it "should show the request with a request sender if a partner user is set" do
visit subject
Expand Down