refactor: improve request item code

Renames formatted_requestable_items to requestable_items

Moves formatting and sorting to service object
Adds tests for service object

Removes method with side effect in favor of direct call to
service object to set @requestable_items

app/controllers/partners/children_controller.rb

@@ -36,7 +36,7 @@ def new
       family = current_partner.families.find_by!(id: params[:family_id])
       @child = family.children.new
 
-      set_formated_requestable_items
+      @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
     end
 
     def active
@@ -47,7 +47,7 @@ def active
 
     def edit
       @child = current_partner.children.find_by(id: params[:id])
-      set_formated_requestable_items
+      @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
     end
 
     def create
@@ -94,13 +94,6 @@ def sort_order
       sort_column + ' ' + sort_direction
     end
 
-    def set_formated_requestable_items
-      requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
-      @formatted_requestable_items = requestable_items.map do |rt|
-        [rt.name, rt.id]
-      end
-    end
-
     helper_method :sort_direction # used in SortableHelper
     def sort_direction
       %w[asc desc].include?(params[:direction]) ? params[:direction] : "asc"

app/controllers/partners/individuals_requests_controller.rb

@@ -2,7 +2,7 @@ module Partners
   class IndividualsRequestsController < BaseController
     def new
       @request = FamilyRequest.new({}, initial_items: 1)
-      set_formated_requestable_items
+      @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
     end
 
     def create
@@ -21,7 +21,7 @@ def create
         @request = FamilyRequest.new({}, initial_items: 1)
         @errors = create_service.errors
 
-        set_formated_requestable_items
+        @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
 
         Rails.logger.info("[Request Creation Failure] partner_user_id=#{current_user.id} reason=#{@errors.full_messages}")
 
@@ -35,12 +35,5 @@ def individuals_request_params
       params.require(:partners_family_request)
             .permit(:comments, items_attributes: %i[item_id person_count])
     end
-
-    def set_formated_requestable_items
-      requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
-      @formatted_requestable_items = requestable_items.map do |rt|
-        [rt.name, rt.id]
-      end.sort
-    end
   end
 end

app/controllers/partners/requests_controller.rb

@@ -11,7 +11,7 @@ def new
       @partner_request = ::Request.new
       @partner_request.item_requests.build
 
-      set_formated_requestable_items
+      @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
     end
 
     def show
@@ -34,7 +34,7 @@ def create
         @partner_request = create_service.partner_request
         @errors = create_service.errors
 
-        set_formated_requestable_items
+        @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
 
         Rails.logger.info("[Request Creation Failure] partner_user_id=#{current_user.id} reason=#{@errors.full_messages}")
 
@@ -44,13 +44,6 @@ def create
 
     private
 
-    def set_formated_requestable_items
-      requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
-      @formatted_requestable_items = requestable_items.map do |rt|
-        [rt.name, rt.id]
-      end.sort
-    end
-
     def partner_request_params
       params.require(:request).permit(:comments, item_requests_attributes: [:item_id, :quantity])
     end

app/services/partner_fetch_requestable_items_service.rb

@@ -4,9 +4,13 @@ def initialize(partner_id:)
   end
 
   def call
-    return organization.items.active.visible.sort if partner.partner_group.blank?
+    requestable_items = if partner.partner_group.blank?
+      organization.items.active.visible
+    else
+      partner.requestable_items.active.visible
+    end
 
-    partner.requestable_items.active.visible
+    requestable_items.map { |item| [item.name, item.id] }.sort
   end
 
   private

app/views/partners/children/_form.html.erb

@@ -17,7 +17,7 @@
               <%= form.text_field :last_name, class: "form-control" %>
 
               <%= form.label :item_needed, "Diaper/Item Used" %>
-              <%= form.select :item_needed_diaperid, @formatted_requestable_items,
+              <%= form.select :item_needed_diaperid, @requestable_items,
                               {include_blank: 'Select an item'},
                               {class: 'form-control'} %>
               <br>

app/views/partners/individuals_requests/_item_request.html.erb

@@ -2,7 +2,8 @@
   <tr>
     <td>
       <%= field.label :item_id, "Item Requested", {class: 'sr-only'} %>
-      <%= field.input :item_id, collection: @formatted_requestable_items, label: false, allow_blank: true, class: 'form-control' %>
+      <%= field.select :item_id, @requestable_items, {include_blank: 'Select an item'}, {class: 'form-control'} %>
+
     </td>
     <td>
       <%= field.label :person_count, "Number of Individuals", {class: 'sr-only'} %>

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

@@ -2,7 +2,7 @@
   <tr>
     <td>
       <%= field.label :item_id, "Item Requested", {class: 'sr-only'} %>
-      <%= field.select :item_id, @formatted_requestable_items, {include_blank: 'Select an item'}, {class: 'form-control'} %>
+      <%= field.select :item_id, @requestable_items, {include_blank: 'Select an item'}, {class: 'form-control'} %>
     </td>
     <td>
       <%= field.label :quantity, "Quantity", {class: 'sr-only'} %>

spec/services/partner_fetch_requestable_items_service_spec.rb

@@ -21,7 +21,8 @@
       end
 
       it 'should return all active and visible items' do
-        expect(subject).to eq(organization.items.active.visible)
+        expected_items = organization.items.active.visible.map { |item| [item.name, item.id] }.sort
+        expect(subject).to eq(expected_items)
       end
     end
 
@@ -33,7 +34,8 @@
       end
 
       it 'should return all active and visible items specified by the item associated with' do
-        expect(subject).to eq(partner.requestable_items.active.visible)
+        expected_items = partner.requestable_items.active.visible.map { |item| [item.name, item.id] }.sort
+        expect(subject).to eq(expected_items)
       end
     end
   end

spec/system/partners/managing_requests_system_spec.rb

@@ -5,22 +5,22 @@
 
     context 'GIVEN a partner user is permitted to make a request' do
       describe 'Select Input Tests' do
-        let(:requestable_items) { build_stubbed_list(:item, 3) }
+        let(:requestable_items) { [["Item 1", 1], ["Item 2", 2], ["Item 3", 3]] }
         context 'WHEN they reach the page' do
           before do
             allow_any_instance_of(PartnerFetchRequestableItemsService).to receive(:call).and_return(requestable_items)
             login_as(partner_user)
             visit new_partners_individuals_request_path
           end
           it 'should show the proper items in the select box' do
-            expected_items = requestable_items.map(&:name).unshift('Select an item')
+            expected_items = requestable_items.map(&:first).unshift('Select an item')
             expect(page.all('select[name="partners_family_request[items_attributes][0][item_id]"] option').map(&:text)).to eq(expected_items)
           end
 
           context 'WHEN they create a request inproperly' do
             before { click_button 'Submit Essentials Request' }
             it 'should show the proper items in the select box' do
-              expected_items = requestable_items.map(&:name).unshift('Select an item')
+              expected_items = requestable_items.map(&:first).unshift('Select an item')
               expect(page.all('select[name="partners_family_request[items_attributes][0][item_id]"] option').map(&:text)).to eq(expected_items)
             end
           end
@@ -115,7 +115,7 @@
 
     context 'GIVEN a partner user is permitted to make a request' do
       describe 'Select Input Tests' do
-        let(:requestable_items) { build_stubbed_list(:item, 3) }
+        let(:requestable_items) { [["Item 1", 1], ["Item 2", 2], ["Item 3", 3]] }
         context 'WHEN they reach the page' do
           before do
             allow_any_instance_of(PartnerFetchRequestableItemsService).to receive(:call).and_return(requestable_items)
@@ -124,15 +124,15 @@
           end
 
           it 'should show the proper items in the select box' do
-            expected_items = requestable_items.map(&:name).unshift('Select an item')
+            expected_items = requestable_items.map(&:first).unshift('Select an item')
             expect(page.all('select[name="request[item_requests_attributes][0][item_id]"] option').map(&:text)).to eq(expected_items)
           end
 
           context 'WHEN they create a request inproperly' do
             before { click_button 'Submit Essentials Request' }
 
             it 'should show the proper items in the select box' do
-              expected_items = requestable_items.map(&:name).unshift('Select an item')
+              expected_items = requestable_items.map(&:first).unshift('Select an item')
               expect(page.all('select[name="request[item_requests_attributes][0][item_id]"] option').map(&:text)).to eq(expected_items)
             end
           end