From 1a5aeea60418c3cfefe99bd9237f29000a857ae6 Mon Sep 17 00:00:00 2001
From: Krunal <krunal.patel@stitchfix.com>
Date: Fri, 31 May 2024 13:44:28 -0400
Subject: [PATCH 01/11] Eagerly de-duplicating items when submitting request

---
 .../partners/request_create_service.rb        | 27 +++++++++-----
 .../partners/request_create_service_spec.rb   | 35 +++++++++++++++++++
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/app/services/partners/request_create_service.rb b/app/services/partners/request_create_service.rb
index e5c4058083..160835d158 100644
--- a/app/services/partners/request_create_service.rb
+++ b/app/services/partners/request_create_service.rb
@@ -54,16 +54,27 @@ def populate_item_request(partner_request)
         attrs['item_id'].blank? && attrs['quantity'].blank?
       end
 
-      item_requests = formatted_line_items.map do |ira|
-        Partners::ItemRequest.new(
-          item_id: ira['item_id'],
-          quantity: ira['quantity'],
-          children: ira['children'] || [], # will create ChildItemRequests if there are any
-          name: fetch_organization_item_name(ira['item_id']),
-          partner_key: fetch_organization_partner_key(ira['item_id'])
-        )
+      items = {}
+
+      formatted_line_items.each do |input_item|
+        pre_existing_entry = items[input_item['item_id']]
+        if pre_existing_entry
+          pre_existing_entry.quantity = (pre_existing_entry.quantity.to_i + input_item['quantity'].to_i).to_s
+          # TODO: Is the following the correct way to merge?
+          pre_existing_entry.children += input_item['children'] || []
+        else
+          items[input_item['item_id']] = Partners::ItemRequest.new(
+            item_id: input_item['item_id'],
+            quantity: input_item['quantity'],
+            children: input_item['children'] || [], # will create ChildItemRequests if there are any
+            name: fetch_organization_item_name(input_item['item_id']),
+            partner_key: fetch_organization_partner_key(input_item['item_id'])
+          )
+        end
       end
 
+      item_requests = items.values
+
       partner_request.item_requests << item_requests
 
       partner_request.request_items = partner_request.item_requests.map do |ir|
diff --git a/spec/services/partners/request_create_service_spec.rb b/spec/services/partners/request_create_service_spec.rb
index abb1a3b20f..c3ab8f4fa2 100644
--- a/spec/services/partners/request_create_service_spec.rb
+++ b/spec/services/partners/request_create_service_spec.rb
@@ -86,6 +86,41 @@
         expect(NotifyPartnerJob).to have_received(:perform_now).with(Request.last.id)
       end
 
+      it 'should have created item requests' do
+        subject
+        expect(Partners::ItemRequest.count).to eq(item_requests_attributes.count)
+      end
+
+      context 'when we have duplicate item as part of request' do
+        let(:duplicate_item) { FactoryBot.create(:item) }
+        let(:unique_item) { FactoryBot.create(:item) }
+        let(:item_requests_attributes) do
+          [
+            ActionController::Parameters.new(
+              item_id: duplicate_item.id,
+              quantity: 3
+            ),
+            ActionController::Parameters.new(
+              item_id: unique_item.id,
+              quantity: 7
+            ),
+            ActionController::Parameters.new(
+              item_id: duplicate_item.id,
+              quantity: 5
+            )
+          ]
+        end
+        it 'should add the quantity of the duplicate item' do
+          subject
+          aggregate_failures {
+            expect(Partners::ItemRequest.count).to eq(2)
+            expect(Partners::ItemRequest.find_by(item_id: duplicate_item.id).quantity).to eq("8")
+            expect(Partners::ItemRequest.find_by(item_id: unique_item.id).quantity).to eq("7")
+            expect(Partners::ItemRequest.first.item_id).to eq(duplicate_item.id)
+          }
+        end
+      end
+
       context 'but a unexpected error occured during the save' do
         let(:error_message) { 'boom' }
 

From d181cee104fd778e2a3a7744b44128b3a438becc Mon Sep 17 00:00:00 2001
From: Krunal <krunal.patel@stitchfix.com>
Date: Fri, 31 May 2024 15:02:28 -0400
Subject: [PATCH 02/11] Adding validation on Request model to have unique
 request for item

---
 app/models/request.rb       |  8 ++++++++
 spec/models/request_spec.rb | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/app/models/request.rb b/app/models/request.rb
index c6963aeeb3..cb7bfef44a 100644
--- a/app/models/request.rb
+++ b/app/models/request.rb
@@ -33,6 +33,7 @@ class Request < ApplicationRecord
   enum status: { pending: 0, started: 1, fulfilled: 2, discarded: 3 }, _prefix: true
 
   validates :distribution_id, uniqueness: true, allow_nil: true
+  validate :item_requests_uniqueness_by_item_id
   before_save :sanitize_items_data
 
   include Filterable
@@ -59,6 +60,13 @@ def user_email
 
   private
 
+  def item_requests_uniqueness_by_item_id
+    item_ids = request_items.map { |item| item["item_id"] }
+    if item_ids.uniq.length != item_ids.length
+      errors.add(:request_items, "should have unique item_ids")
+    end
+  end
+
   def sanitize_items_data
     return unless request_items && request_items_changed?
 
diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb
index 512cac62e4..e8732b2ed2 100644
--- a/spec/models/request_spec.rb
+++ b/spec/models/request_spec.rb
@@ -56,6 +56,39 @@
     end
   end
 
+  describe "validations" do
+    let(:item_one) { create(:item) }
+    let(:item_two) { create(:item) }
+    subject { build(:request, request_items: request_items) }
+
+    context "when request_items have unique item_ids" do
+      let(:request_items) do
+        [
+          { "item_id" => item_one.id, "quantity" => 5 },
+          { "item_id" => item_two.id, "quantity" => 3 }
+        ]
+      end
+
+      it "is valid" do
+        expect(subject).to be_valid
+      end
+    end
+
+    context "when request_items do not have unique item_ids" do
+      let(:request_items) do
+        [
+          { "item_id" => item_one.id, "quantity" => 5 },
+          { "item_id" => item_one.id, "quantity" => 3 }
+        ]
+      end
+
+      it "is not valid" do
+        expect(subject).to_not be_valid
+        expect(subject.errors[:request_items]).to include("should have unique item_ids")
+      end
+    end
+  end
+
   describe "versioning" do
     it { is_expected.to be_versioned }
   end

From 6e44bce45c5ec3d7be8c7faf53d444be370b0b3e Mon Sep 17 00:00:00 2001
From: Krunal <krunal.patel@stitchfix.com>
Date: Fri, 31 May 2024 15:58:29 -0400
Subject: [PATCH 03/11] Since we are no longer storing duplicate items per
 request, removing this logic
 https://github.com/rubyforgood/human-essentials/pull/3437/files

---
 app/mailers/requests_confirmation_mailer.rb   | 19 +++++--------------
 spec/factories/requests.rb                    | 11 -----------
 .../requests_confirmation_mailer_spec.rb      |  8 --------
 3 files changed, 5 insertions(+), 33 deletions(-)

diff --git a/app/mailers/requests_confirmation_mailer.rb b/app/mailers/requests_confirmation_mailer.rb
index 7d520b69af..cc0f0c52aa 100644
--- a/app/mailers/requests_confirmation_mailer.rb
+++ b/app/mailers/requests_confirmation_mailer.rb
@@ -11,20 +11,11 @@ def confirmation_email(request)
   private
 
   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
+    items_sorted = request.request_items.sort_by { |k| k['item_id'] }
+    item_ids = items_sorted&.map { |item| item['item_id'] }
+    items_names = Item.where(id: item_ids).order(:id).pluck(:name)
+    names_hash = items_names.map { |name| { 'name' => name } }
 
-  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'] }
-    # convert hash into an array of items with combined quantities
-    grouped.map do |id, items|
-      { 'item_id' => id, 'quantity' => items.map { |i| i['quantity'] }.sum }
-    end
+    items_sorted.zip(names_hash).map { |items, names| items.merge(names) }
   end
 end
diff --git a/spec/factories/requests.rb b/spec/factories/requests.rb
index 57dc94cf28..e8a28128d8 100644
--- a/spec/factories/requests.rb
+++ b/spec/factories/requests.rb
@@ -42,17 +42,6 @@ def random_request_items
     status { 'pending' }
   end
 
-  trait :with_duplicates do
-    request_items {
-      # get 3 unique item ids
-      keys = Item.active.pluck(:id).sample(3)
-      # add an extra of the first key, so we have one duplicated item
-      keys.push(keys[0])
-      # give each item a quantity of 50
-      keys.map { |k| { "item_id" => k, "quantity" => 50 } }
-    }
-  end
-
   trait :with_varied_quantities do
     request_items {
       # get 10 unique item ids
diff --git a/spec/mailers/requests_confirmation_mailer_spec.rb b/spec/mailers/requests_confirmation_mailer_spec.rb
index a0fc95396d..b25a9f107e 100644
--- a/spec/mailers/requests_confirmation_mailer_spec.rb
+++ b/spec/mailers/requests_confirmation_mailer_spec.rb
@@ -3,9 +3,7 @@
   let(:request) { create(:request, organization: organization) }
   let(:mail) { RequestsConfirmationMailer.confirmation_email(request) }
 
-  let(:request_w_duplicates) { create(:request, :with_duplicates, organization: organization) }
   let(:request_w_varied_quantities) { create(:request, :with_varied_quantities, organization: organization) }
-  let(:mail_w_duplicates) { RequestsConfirmationMailer.confirmation_email(request_w_duplicates) }
   let(:mail_w_varied_quantities) { RequestsConfirmationMailer.confirmation_email(request_w_varied_quantities) }
 
   describe "#confirmation_email" do
@@ -23,12 +21,6 @@
     end
   end
 
-  it 'handles duplicates' do
-    organization.update!(email: "me@org.com")
-    expect(mail_w_duplicates.body.encoded).to match('This is an email confirmation')
-    expect(mail_w_duplicates.body.encoded).to match(' - 100')
-  end
-
   it 'pairs the right quantities with the right item names' do
     organization.update!(email: "me@org.com")
     expect(mail_w_varied_quantities.body.encoded).to match('This is an email confirmation')

From 4b5e42f9cc316e6d6d1c0eaa67ad17991c18e9a5 Mon Sep 17 00:00:00 2001
From: Krunal <krunal.patel@stitchfix.com>
Date: Fri, 31 May 2024 16:24:15 -0400
Subject: [PATCH 04/11] Removing the need to have a test for duplicate items in
 the request

---
 .../exports/export_request_service_spec.rb      | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/spec/services/exports/export_request_service_spec.rb b/spec/services/exports/export_request_service_spec.rb
index b36732996c..c6a1eb38e3 100644
--- a/spec/services/exports/export_request_service_spec.rb
+++ b/spec/services/exports/export_request_service_spec.rb
@@ -25,26 +25,25 @@
            request_items: [{ item_id: 0, quantity: 200 }, { item_id: -1, quantity: 200 }])
   end
 
-  let!(:duplicate_items) do
+  let!(:unique_items) do
     [
       {item_id: item_3t.id, quantity: 2},
-      {item_id: item_2t.id, quantity: 3},
-      {item_id: item_3t.id, quantity: 5}
+      {item_id: item_2t.id, quantity: 3}
     ]
   end
 
-  let!(:duplicate_item_quantities) do
-    duplicate_items.each_with_object(Hash.new(0)) do |item, hsh|
+  let!(:item_quantities) do
+    unique_items.each_with_object(Hash.new(0)) do |item, hsh|
       hsh[item[:item_id]] += item[:quantity]
     end
   end
 
-  let!(:request_with_duplicate_items) do
+  let!(:request_with_items) do
     create(
       :request,
       :started,
       organization: org,
-      request_items: duplicate_items
+      request_items: unique_items
     )
   end
 
@@ -78,8 +77,8 @@
       item_column_idx = expected_headers.each_with_index.to_h[Exports::ExportRequestService::DELETED_ITEMS_COLUMN_HEADER]
       expect(subject.fourth[item_column_idx]).to eq(400)
 
-      expect(subject.fifth[item_2t_column_idx]).to eq(duplicate_item_quantities[item_2t.id])
-      expect(subject.fifth[item_3t_column_idx]).to eq(duplicate_item_quantities[item_3t.id])
+      expect(subject.fifth[item_2t_column_idx]).to eq(item_quantities[item_2t.id])
+      expect(subject.fifth[item_3t_column_idx]).to eq(item_quantities[item_3t.id])
     end
   end
 end

From e206bd6f6d30783955c79fcf5de9a5797693065a Mon Sep 17 00:00:00 2001
From: Scott Steele <scottlsteele@gmail.com>
Date: Sat, 1 Jun 2024 11:10:47 -0400
Subject: [PATCH 05/11] squashme! tweak child merge logic and post disclaimer

---
 app/services/partners/request_create_service.rb | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/app/services/partners/request_create_service.rb b/app/services/partners/request_create_service.rb
index 160835d158..dc185c9d78 100644
--- a/app/services/partners/request_create_service.rb
+++ b/app/services/partners/request_create_service.rb
@@ -60,8 +60,14 @@ def populate_item_request(partner_request)
         pre_existing_entry = items[input_item['item_id']]
         if pre_existing_entry
           pre_existing_entry.quantity = (pre_existing_entry.quantity.to_i + input_item['quantity'].to_i).to_s
-          # TODO: Is the following the correct way to merge?
-          pre_existing_entry.children += input_item['children'] || []
+          # NOTE: When this code was written (and maybe it's still the
+          # case as you read it!), the FamilyRequestsController does a
+          # ton of calculation to translate children to item quantities.
+          # If that logic is incorrect, there's not much we can do here
+          # to fix things. Could make sense to move more of that logic
+          # into one of the service objects that instantiate the Request
+          # object (either this one or the FamilyRequestCreateService).
+          pre_existing_entry.children = (pre_existing_entry.children + input_item['children'] || []).uniq
         else
           items[input_item['item_id']] = Partners::ItemRequest.new(
             item_id: input_item['item_id'],

From 789d9af140feb2d6df886f9c95954a856b6f0710 Mon Sep 17 00:00:00 2001
From: Krunal <krunal.patel@stitchfix.com>
Date: Sat, 1 Jun 2024 11:14:34 -0400
Subject: [PATCH 06/11] Testing the merging logic for duplicate child request
 which shouldn't occur in production

---
 .../partners/family_request_create_service_spec.rb  | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/spec/services/partners/family_request_create_service_spec.rb b/spec/services/partners/family_request_create_service_spec.rb
index ac22297dab..a7b5ca2275 100644
--- a/spec/services/partners/family_request_create_service_spec.rb
+++ b/spec/services/partners/family_request_create_service_spec.rb
@@ -55,6 +55,7 @@
         let(:second_item_id) { items_to_request.second.id.to_i }
         let(:child1) { create(:partners_child) }
         let(:child2) { create(:partners_child) }
+        let(:child3) { create(:partners_child) }
         let(:family_requests_attributes) do
           [
             {
@@ -66,6 +67,11 @@
               item_id: second_item_id,
               person_count: 2,
               children: [child1, child2]
+            },
+            {
+              item_id: second_item_id,
+              person_count: 2,
+              children: [child1, child3]
             }
           ]
         end
@@ -85,7 +91,12 @@
           expect(first_item_request.children).to contain_exactly(child1)
 
           second_item_request = partner_request.item_requests.find_by(item_id: second_item_id)
-          expect(second_item_request.children).to contain_exactly(child1, child2)
+          # testing the de-duping logic of request create service
+          expect(second_item_request.children).to contain_exactly(child1, child2, child3)
+          expect(second_item_request.children.count).to eq(3)
+
+          expect(first_item_request.quantity.to_i).to eq(first_item_request.item.default_quantity)
+          expect(second_item_request.quantity.to_i).to eq(second_item_request.item.default_quantity * 4)
         end
       end
 

From 67e45eb3f6231b21a00fd9e93a3ae305e3d6537d Mon Sep 17 00:00:00 2001
From: Scott Steele <scottlsteele@gmail.com>
Date: Sat, 1 Jun 2024 11:42:40 -0400
Subject: [PATCH 07/11] squashme! fix operator precedence of child merge

---
 app/services/partners/request_create_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/partners/request_create_service.rb b/app/services/partners/request_create_service.rb
index dc185c9d78..8df880ebbf 100644
--- a/app/services/partners/request_create_service.rb
+++ b/app/services/partners/request_create_service.rb
@@ -67,7 +67,7 @@ def populate_item_request(partner_request)
           # to fix things. Could make sense to move more of that logic
           # into one of the service objects that instantiate the Request
           # object (either this one or the FamilyRequestCreateService).
-          pre_existing_entry.children = (pre_existing_entry.children + input_item['children'] || []).uniq
+          pre_existing_entry.children = (pre_existing_entry.children + (input_item['children'] || [])).uniq
         else
           items[input_item['item_id']] = Partners::ItemRequest.new(
             item_id: input_item['item_id'],

From 0b8818493b3c8389fa12babee70baf83e58cbb52 Mon Sep 17 00:00:00 2001
From: Krunal <krunal.patel@stitchfix.com>
Date: Sat, 1 Jun 2024 11:42:59 -0400
Subject: [PATCH 08/11] Adding a documentation on the fetch_items method

---
 app/mailers/requests_confirmation_mailer.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/mailers/requests_confirmation_mailer.rb b/app/mailers/requests_confirmation_mailer.rb
index cc0f0c52aa..bc8754bd84 100644
--- a/app/mailers/requests_confirmation_mailer.rb
+++ b/app/mailers/requests_confirmation_mailer.rb
@@ -10,6 +10,11 @@ def confirmation_email(request)
 
   private
 
+  # fetch_items
+  #
+  # Fetches sorted items associated with the current Request object.
+  #
+  # @return Array of request_items with item names
   def fetch_items(request)
     items_sorted = request.request_items.sort_by { |k| k['item_id'] }
     item_ids = items_sorted&.map { |item| item['item_id'] }

From 1503829ec774b714c45f573a9107ebfdff503e5e Mon Sep 17 00:00:00 2001
From: Krunal <krunal.patel@stitchfix.com>
Date: Sat, 1 Jun 2024 15:15:23 -0400
Subject: [PATCH 09/11] Adding validation on item_reqests instead of
 request_items

---
 app/models/request.rb       |  4 ++--
 spec/models/request_spec.rb | 20 ++++++++++----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/models/request.rb b/app/models/request.rb
index cb7bfef44a..f4c2886bcc 100644
--- a/app/models/request.rb
+++ b/app/models/request.rb
@@ -61,9 +61,9 @@ def user_email
   private
 
   def item_requests_uniqueness_by_item_id
-    item_ids = request_items.map { |item| item["item_id"] }
+    item_ids = item_requests.map(&:item_id)
     if item_ids.uniq.length != item_ids.length
-      errors.add(:request_items, "should have unique item_ids")
+      errors.add(:item_requests, "should have unique item_ids")
     end
   end
 
diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb
index e8732b2ed2..531c829313 100644
--- a/spec/models/request_spec.rb
+++ b/spec/models/request_spec.rb
@@ -59,13 +59,13 @@
   describe "validations" do
     let(:item_one) { create(:item) }
     let(:item_two) { create(:item) }
-    subject { build(:request, request_items: request_items) }
+    subject { build(:request, item_requests: item_requests) }
 
-    context "when request_items have unique item_ids" do
-      let(:request_items) do
+    context "when item_requests have unique item_ids" do
+      let(:item_requests) do
         [
-          { "item_id" => item_one.id, "quantity" => 5 },
-          { "item_id" => item_two.id, "quantity" => 3 }
+          create(:item_request, item: item_one, quantity: 5),
+          create(:item_request, item: item_two, quantity: 3)
         ]
       end
 
@@ -74,17 +74,17 @@
       end
     end
 
-    context "when request_items do not have unique item_ids" do
-      let(:request_items) do
+    context "when item_requests do not have unique item_ids" do
+      let(:item_requests) do
         [
-          { "item_id" => item_one.id, "quantity" => 5 },
-          { "item_id" => item_one.id, "quantity" => 3 }
+          create(:item_request, item: item_one, quantity: 5),
+          create(:item_request, item: item_one, quantity: 3)
         ]
       end
 
       it "is not valid" do
         expect(subject).to_not be_valid
-        expect(subject.errors[:request_items]).to include("should have unique item_ids")
+        expect(subject.errors[:item_requests]).to include("should have unique item_ids")
       end
     end
   end

From a72d9c017849b38aecc4d4c5bbdea73b893951f3 Mon Sep 17 00:00:00 2001
From: Krunal <krunal.patel@stitchfix.com>
Date: Sat, 1 Jun 2024 16:42:57 -0400
Subject: [PATCH 10/11] Removing the refactor on request mailer to de-dupe

---
 app/mailers/requests_confirmation_mailer.rb | 25 ++++++++++++---------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/app/mailers/requests_confirmation_mailer.rb b/app/mailers/requests_confirmation_mailer.rb
index bc8754bd84..54c7981cf8 100644
--- a/app/mailers/requests_confirmation_mailer.rb
+++ b/app/mailers/requests_confirmation_mailer.rb
@@ -10,17 +10,22 @@ def confirmation_email(request)
 
   private
 
-  # fetch_items
-  #
-  # Fetches sorted items associated with the current Request object.
-  #
-  # @return Array of request_items with item names
+  # TODO: remove the need to de-duplicate items in the request
   def fetch_items(request)
-    items_sorted = request.request_items.sort_by { |k| k['item_id'] }
-    item_ids = items_sorted&.map { |item| item['item_id'] }
-    items_names = Item.where(id: item_ids).order(:id).pluck(:name)
-    names_hash = items_names.map { |name| { 'name' => name } }
+    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
 
-    items_sorted.zip(names_hash).map { |items, names| items.merge(names) }
+  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'] }
+    # convert hash into an array of items with combined quantities
+    grouped.map do |id, items|
+      { 'item_id' => id, 'quantity' => items.map { |i| i['quantity'] }.sum }
+    end
   end
 end

From 44cb8f279f522f8b0f28cffd649f54fab534e672 Mon Sep 17 00:00:00 2001
From: Scott Steele <scottlsteele@gmail.com>
Date: Sat, 1 Jun 2024 18:05:54 -0400
Subject: [PATCH 11/11] squashme! don't create dup'd items when seeding
 requests

---
 db/seeds.rb | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/db/seeds.rb b/db/seeds.rb
index f9686311a3..0a138b6301 100644
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -353,7 +353,6 @@ def random_record_for_org(org, klass)
       updated_at: date
     )
 
-    item_requests = []
     pads = p.organization.items.find_by(name: 'Pads')
     new_item_request = Partners::ItemRequest.new(
       item_id: pads.id,
@@ -367,9 +366,10 @@ def random_record_for_org(org, klass)
     )
     partner_request.item_requests << new_item_request
 
-    Array.new(Faker::Number.within(range: 4..14)) do
-      item = p.organization.items.sample
-      new_item_request = Partners::ItemRequest.new(
+    items = p.organization.items.sample(Faker::Number.within(range: 4..14)) - [pads]
+
+    partner_request.item_requests += items.map do |item|
+      Partners::ItemRequest.new(
         item_id: item.id,
         quantity: Faker::Number.within(range: 10..30),
         children: [],
@@ -378,7 +378,6 @@ def random_record_for_org(org, klass)
         created_at: date,
         updated_at: date
       )
-      partner_request.item_requests << new_item_request
     end
 
     partner_request.request_items = partner_request.item_requests.map do |ir|