Product totals - custom units (#4609)

* Implement custom units in request total popup

* Sort request counts by name instead of by count

* Remove unneeded test, this case doesn't happen

Verified that in production there are zero of these instances

* Pluralize units and update spec [#4408]

* Do not show dash for blank unit

* Sort by the lower-cased name for consistency

* Use default hash value

Co-authored-by: Daniel Orner <daniel.orner@flipp.com>

* Spec description improvement

Co-authored-by: Daniel Orner <daniel.orner@flipp.com>

* Spec description improvement

Co-authored-by: Daniel Orner <daniel.orner@flipp.com>

* Make item names explicit in test [#4408]

* Unify on request item name with unit refactor across PRs

[#4408]

---------

Co-authored-by: Daniel Orner <daniel.orner@flipp.com>

app/models/partners/item_request.rb

@@ -35,5 +35,15 @@ def request_unit_is_supported
         errors.add(:request_unit, "is not supported")
       end
     end
+
+    def name_with_unit
+      if item
+        if Flipper.enabled?(:enable_packs) && request_unit.present?
+          "#{name} - #{request_unit.pluralize(quantity.to_i)}"
+        else
+          name
+        end
+      end
+    end
   end
 end

app/services/requests_total_items_service.rb

@@ -6,39 +6,19 @@ def initialize(requests:)
   def calculate
     return unless requests
 
-    request_items_array = []
-
-    request_items.each do |items|
-      items.each do |json|
-        request_items_array << [item_name(json['item_id']), json['quantity']]
-      end
+    totals = Hash.new(0)
+    item_requests.each do |item_request|
+      totals[item_request.name_with_unit] += item_request.quantity.to_i
     end
 
-    request_items_array.inject({}) do |item, (quantity, total)|
-      item[quantity] ||= 0
-      item[quantity] += total.to_i
-      item
-    end
+    totals
   end
 
   private
 
   attr_accessor :requests
 
-  def request_items
-    @request_items ||= requests.pluck(:request_items)
-  end
-
-  def request_items_ids
-    request_items.flat_map { |jitem| jitem.map { |item| item["item_id"] } }
-  end
-
-  def items_names
-    @items_names ||= Item.where(id: request_items_ids).as_json(only: [:id, :name])
-  end
-
-  def item_name(id)
-    item_found = items_names.find { |item| item["id"] == id }
-    item_found&.fetch('name') || '*Unknown Item*'
+  def item_requests
+    @item_requests ||= requests.flat_map(&:item_requests)
   end
 end

app/views/requests/_calculate_product_totals.html.erb

@@ -16,7 +16,7 @@
           </tr>
           </thead>
           <tbody>
-            <% @calculate_product_totals.sort_by { |name, quantity| -quantity}.each do |name, quantity| %>
+            <% @calculate_product_totals.sort_by { |name, quantity| name.downcase }.each do |name, quantity| %>
               <tr>
                 <td><%= name %></td>
                 <td><%= quantity %></td>

spec/services/requests_total_items_service_spec.rb

@@ -4,42 +4,65 @@
     subject { described_class.new(requests: requests).calculate }
 
     context 'when the request items is not blank' do
-      let(:sample_items) { create_list(:item, 3, organization: organization) }
+      let(:sample_items) do
+        create_list(:item, 3, :with_unit, organization: organization, unit: "bundle") do |item, n|
+          item.name = "item_name_#{n}"
+          item.save!
+        end
+      end
       let(:item_names) { sample_items.pluck(:name) }
       let(:item_ids) { sample_items.pluck(:id) }
       let(:requests) do
         [
-          create(:request, request_items: item_ids.map { |k| { "item_id" => k, "quantity" => 20 } }),
-          create(:request, request_items: item_ids.map { |k| { "item_id" => k, "quantity" => 10 } })
+          create(:request, :with_item_requests, request_items: item_ids.map { |k| { "item_id" => k, "quantity" => 20 } }),
+          create(:request, :with_item_requests, request_items: item_ids.map { |k| { "item_id" => k, "quantity" => 10, "request_unit" => "bundle" } }),
+          create(:request, :with_item_requests, request_items: item_ids.map { |k| { "item_id" => k, "quantity" => 50, "request_unit" => "bundle" } })
         ]
       end
 
       it 'return items with correct quantities calculated' do
-        expect(subject.first.last).to eq(30)
+        expect(subject.first.last).to eq(80)
       end
 
       it 'return the names of items correctly' do
-        expect(subject.keys).to eq(item_names)
+        expect(subject.keys).to eq([
+          "item_name_0",
+          "item_name_1",
+          "item_name_2"
+        ])
       end
-    end
 
-    context 'when item name is nil' do
-      let(:item) do
-        i = build(:item, name: nil)
-        i.save(validate: false)
-        i
-      end
-      let(:requests) do
-        [create(:request, request_items: [{ "item_id" => item.id, "quantity" => 20 }])]
-      end
+      context 'when custom request units are specified and enabled' do
+        before do
+          Flipper.enable(:enable_packs)
+        end
+
+        it 'returns the names of items correctly' do
+          expect(subject.keys).to eq([
+            "item_name_0",
+            "item_name_1",
+            "item_name_2",
+            "item_name_0 - bundles",
+            "item_name_1 - bundles",
+            "item_name_2 - bundles"
+          ])
+        end
 
-      it 'return Unknown Item' do
-        expect(subject.first.first).to eq('*Unknown Item*')
+        it 'returns items with correct quantities calculated' do
+          expect(subject).to eq({
+            "item_name_0" => 20,
+            "item_name_0 - bundles" => 60,
+            "item_name_1" => 20,
+            "item_name_1 - bundles" => 60,
+            "item_name_2" => 20,
+            "item_name_2 - bundles" => 60
+          })
+        end
       end
     end
 
     context 'when provided with requests that have no request items' do
-      let(:requests) { [create(:request, request_items: {})] }
+      let(:requests) { [create(:request, :with_item_requests, request_items: {})] }
 
       it { is_expected.to be_blank }
     end