Merge pull request #5196 from rubyforgood/item-request

Request Item → Item Request

Author: awwaiid

app/controllers/requests_controller.rb

@@ -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 }
   end
 
   # Clicking the "New Distribution" button will set the the request to started
@@ -86,13 +92,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)

app/mailers/requests_confirmation_mailer.rb

@@ -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
@@ -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
-  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

app/models/partners/item_request.rb

@@ -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)}"
       else
-        item&.name || name
+        item_name
       end
     end
   end

app/pdfs/distribution_pdf.rb

@@ -180,38 +180,35 @@ def request_data
       "Value/item",
       "In-Kind Value Received",
       "Packages"]]
-    inventory = View::Inventory.new(@distribution.organization_id)
-    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),
@@ -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

app/views/requests/show.html.erb

@@ -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>

app/views/requests_confirmation_mailer/confirmation_email.html.erb

@@ -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>
 

app/views/requests_confirmation_mailer/confirmation_email.text.erb

@@ -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.

spec/mailers/requests_confirmation_mailer_spec.rb

@@ -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) }
   let(:mail_w_varied_quantities) { RequestsConfirmationMailer.confirmation_email(request_w_varied_quantities) }
 
   describe "#confirmation_email" do
@@ -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")
@@ -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")

spec/pdfs/distribution_pdf_spec.rb

@@ -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],
+        ["Item 3", "50", "", "$3.00", nil, nil],
         ["Item 4", "120 packs", "", "$4.00", nil, nil],
         ["", "", "", "", ""],
         ["Total Items Received", 200, 150, "", "$250.00", ""]
@@ -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]
           ])
@@ -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, ""]
           ])
@@ -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]
         ])
@@ -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, ""]
         ])
@@ -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"]
           ])

spec/system/request_system_spec.rb

@@ -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