Fix tests, remove duplicate error messages and reformat for readibility

Author: embarnard

app/models/partners/item_request.rb

@@ -22,16 +22,20 @@ class ItemRequest < Base
     has_many :children, through: :child_item_requests
 
     validates :quantity, presence: true
-    validates :quantity, numericality: { only_integer: true, greater_than_or_equal_to: 1 }
+    validates :quantity, numericality: {
+      only_integer: true,
+      greater_than_or_equal_to: 1,
+      message: "quantity must be a whole number greater than or equal to 1"
+    }
     validates :name, presence: true
     validate :request_unit_is_supported
 
     def request_unit_is_supported
-      return if request_unit.blank?
+      return if request_unit.blank? || request_unit == "-1" # nothing selected
 
       names = item.request_units.map(&:name)
       unless names.include?(request_unit)
-        errors.add(:request_unit, "is not supported")
+        errors.add(:base, "#{request_unit} is not a supported unit type")
       end
     end
 

app/models/request.rb

@@ -68,7 +68,7 @@ def request_type_label
   def item_requests_uniqueness_by_item_id
     item_ids = item_requests.map(&:item_id)
     if item_ids.uniq.length != item_ids.length
-      errors.add(:item_requests, "should have unique item_ids")
+      errors.add(:base, "Please ensure a single unit is selected for each item that supports it")
     end
   end
 

app/services/partners/request_create_service.rb

@@ -65,9 +65,7 @@ def populate_item_request(partner_request)
         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}")
-          else
+          unless pre_existing_entry.request_unit != input_item['request_unit']
             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
@@ -82,7 +80,7 @@ def populate_item_request(partner_request)
         end
 
         if input_item['request_unit'].to_s == '-1' # nothing selected
-          errors.add(:base, "Please select a unit for #{Item.find(input_item["item_id"]).name}")
+          errors.add(:base, "Please select a unit for #{Item.find_by_id(input_item["item_id"]).name}")
         end
 
         item_request = Partners::ItemRequest.new(
@@ -105,9 +103,10 @@ def populate_item_request(partner_request)
         }.compact
       end
 
-      # Validate if request quantity exceeds the request limit for the item and unit type
+      # Validate request quantity doesn't exceed the request limit for the item and unit type
       partner_request.request_items.each do |ir|
-        item = Item.find(ir["item_id"])
+        item = Item.find_by_id(ir["item_id"])
+        next if item.nil?
         unit_type = ir["request_unit"]
         quantity_requested = ir["quantity"].to_i
 
@@ -118,7 +117,12 @@ def populate_item_request(partner_request)
         end
 
         if limit.present? && (quantity_requested > limit)
-          errors.add(:base, "#{item.name}: You requested #{quantity_requested} #{unit_type.pluralize}, but are limited to #{limit} #{unit_type.pluralize}")
+          message = if unit_type.blank?
+                      "#{item.name}: You requested #{quantity_requested}, but are limited to #{limit}"
+                    else
+                      "#{item.name}: You requested #{quantity_requested} #{unit_type&.pluralize}, but are limited to #{limit} #{unit_type&.pluralize}"
+                    end
+          errors.add(:base, message)
         end
       end
 

spec/requests/items_requests_spec.rb

@@ -100,7 +100,7 @@
         get edit_item_path(item)
 
         parsed_body = Nokogiri::HTML(response.body)
-        checkboxes = parsed_body.css("input[type='checkbox'][name='item[request_unit_ids][]']")
+        checkboxes = parsed_body.css("input[type='checkbox'][name='item[unit_ids][]']")
         expect(checkboxes.length).to eq organization_units.length
         checkboxes.each do |checkbox|
           if checkbox['value'] == selected_unit.id.to_s

spec/requests/partners/requests_spec.rb

@@ -242,11 +242,7 @@
         expect { post partners_requests_path, params: request_attributes }.to_not change { Request.count }
 
         expect(response).to be_unprocessable
-        expect(response.body).to include("Oops! Something went wrong with your Request")
-        expect(response.body).to include("Ensure each line item has a item selected AND a quantity greater than 0.")
-        expect(response.body).to include("Still need help? Please contact your essentials bank, #{partner.organization.name}")
-        expect(response.body).to include("Our email on record for them is:")
-        expect(response.body).to include(partner.organization.email)
+        expect(response.body).to include("quantity must be a whole number greater than or equal to 1")
       end
     end
 
@@ -346,11 +342,7 @@
         expect { post partners_requests_path, params: request_attributes }.to_not change { Request.count }
 
         expect(response).to be_unprocessable
-        expect(response.body).to include("Oops! Something went wrong with your Request")
-        expect(response.body).to include("Ensure each line item has a item selected AND a quantity greater than 0.")
-        expect(response.body).to include("Still need help? Please contact your essentials bank, #{partner.organization.name}")
-        expect(response.body).to include("Our email on record for them is:")
-        expect(response.body).to include(partner.organization.email)
+        expect(response.body).to include("Completely empty request")
       end
     end
 

spec/services/partners/request_create_service_spec.rb

@@ -193,7 +193,8 @@
     let(:partner) { create(:partner) }
     let(:request_type) { "child" }
     let(:comments) { Faker::Lorem.paragraph }
-    let(:item) { FactoryBot.create(:item) }
+    let(:item) { FactoryBot.create(:item, unit_request_limit: request_limit) }
+    let(:request_limit) { nil }
     let(:item_requests_attributes) do
       [
         ActionController::Parameters.new(
@@ -209,5 +210,14 @@
       expect(subject.partner_request.item_requests.first.item.name).to eq(item.name)
       expect(subject.partner_request.item_requests.first.quantity).to eq("25")
     end
+
+    context "request quantity exceeds the request limit for the item and unit type" do
+      let(:request_limit) { 10 }
+
+      it "adds error" do
+        expect(subject.errors).not_to be_empty
+        expect(subject.errors[:base].first).to include "You requested 25, but are limited to 10"
+      end
+    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 single unit is selected for each item that supports it."
+        expect(page).to have_text "Please select a unit for item 1."
         expect(Request.count).to eq(0)
       end