Ensure unique units are used when creating a request [#4579]

app/services/partners/request_create_service.rb

@@ -65,6 +65,10 @@ def populate_item_request(partner_request)
       formatted_line_items.each do |input_item|
         pre_existing_entry = items[input_item['item_id']]
         if pre_existing_entry
+          if pre_existing_entry.request_unit != input_item['request_unit']
+            errors.add(:base, "Please use the same unit for every #{Item.find(input_item["item_id"]).name}")
+            next
+          end
           pre_existing_entry.quantity = (pre_existing_entry.quantity.to_i + input_item['quantity'].to_i).to_s
           # NOTE: When this code was written (and maybe it's still the
           # case as you read it!), the FamilyRequestsController does a

app/views/partners/requests/_error.html.erb

@@ -9,7 +9,7 @@
     <p class='text-lg font-bold'>
       Ensure each line item has a item selected AND a quantity greater than 0.
       <% if Flipper.enabled?(:enable_packs) && current_partner.organization.request_units.any? %>
-        Please ensure a unit is selected for each item that supports it.
+        Please ensure a single unit is selected for each item that supports it.
       <% end %>
     </p>
     <p class='text-md'>

spec/requests/partners/requests_spec.rb

@@ -124,6 +124,7 @@
   describe "POST #create" do
     subject { post partners_requests_path, params: request_attributes }
     let(:item1) { create(:item, name: "First item", organization: organization) }
+    let(:item2) { create(:item, name: "Second item", organization: organization) }
 
     let(:request_attributes) do
       {
@@ -142,8 +143,15 @@
 
     before do
       sign_in(partner_user)
+
+      # Set up a variety of units that these two items are allowed to have
       FactoryBot.create(:unit, organization: organization, name: 'pack')
+      FactoryBot.create(:unit, organization: organization, name: 'box')
+      FactoryBot.create(:unit, organization: organization, name: 'notallowed')
       FactoryBot.create(:item_unit, item: item1, name: 'pack')
+      FactoryBot.create(:item_unit, item: item1, name: 'box')
+      FactoryBot.create(:item_unit, item: item2, name: 'pack')
+      FactoryBot.create(:item_unit, item: item2, name: 'box')
     end
 
     context 'when given valid parameters' do
@@ -185,6 +193,66 @@
       end
     end
 
+    context "when there are mixed units" do
+      context "on different items" do
+        let(:request_attributes) do
+          {
+            request: {
+              comments: Faker::Lorem.paragraph,
+              item_requests_attributes: {
+                "0" => {
+                  item_id: item1.id,
+                  request_unit: 'pack',
+                  quantity: 12
+                },
+                "1" => {
+                  item_id: item2.id,
+                  request_unit: 'box',
+                  quantity: 17
+                }
+              }
+            }
+          }
+        end
+
+        it "creates without error" do
+          Flipper.enable(:enable_packs)
+          expect { subject }.to change { Request.count }.by(1)
+          expect(response).to redirect_to(partners_request_path(Request.last.id))
+          expect(response.request.flash[:success]).to eql "Request was successfully created."
+        end
+      end
+
+      context "on the same item" do
+        let(:request_attributes) do
+          {
+            request: {
+              comments: Faker::Lorem.paragraph,
+              item_requests_attributes: {
+                "0" => {
+                  item_id: item1.id,
+                  request_unit: 'pack',
+                  quantity: 12
+                },
+                "1" => {
+                  item_id: item1.id,
+                  request_unit: 'box',
+                  quantity: 17
+                }
+              }
+            }
+          }
+        end
+
+        it "results in an error" do
+          Flipper.enable(:enable_packs)
+          expect { post partners_requests_path, params: request_attributes }.to_not change { Request.count }
+          expect(response).to be_unprocessable
+          expect(response.body).to include("Please ensure a single unit is selected for each item")
+        end
+      end
+    end
+
     context "when a request empty" do
       let(:request_attributes) do
         {

spec/system/partners/children_system_spec.rb

@@ -30,7 +30,8 @@
       expect(page).to have_text("Child Last Name")
       expect(page).to have_text("01234")
       expect(page).to have_text("Some Comment")
-      expect(page).to have_text("Item 1, Item 2")
+      expect(page).to have_text("Item 1")
+      expect(page).to have_text("Item 2")
     end
   end
 end

spec/system/partners/requests_system_spec.rb

@@ -43,7 +43,7 @@
         fill_in "request_item_requests_attributes_0_quantity", with: 50
         click_on "Submit Essentials Request"
 
-        expect(page).to have_text "Please ensure a unit is selected for each item that supports it."
+        expect(page).to have_text "Please ensure a single unit is selected for each item that supports it."
         expect(Request.count).to eq(0)
       end