Merge pull request #4099 from elasticspoon/4069-individual-requests-i…

…ssue

fix: select fields on request pages (#4069 partial)

app/controllers/partners/children_controller.rb

@@ -36,10 +36,7 @@ def new
       family = current_partner.families.find_by!(id: params[:family_id])
       @child = family.children.new
 
-      requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
-      @formatted_requestable_items = requestable_items.map do |rt|
-        [rt.name, rt.id]
-      end
+      @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
     end
 
     def active
@@ -50,10 +47,7 @@ def active
 
     def edit
       @child = current_partner.children.find_by(id: params[:id])
-      requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
-      @formatted_requestable_items = requestable_items.map do |rt|
-        [rt.name, rt.id]
-      end
+      @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
     end
 
     def create

app/controllers/partners/individuals_requests_controller.rb

@@ -2,11 +2,7 @@ module Partners
   class IndividualsRequestsController < BaseController
     def new
       @request = FamilyRequest.new({}, initial_items: 1)
-
-      requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
-      @formatted_requestable_items = requestable_items.map do |rt|
-        [rt.name, rt.id]
-      end.sort
+      @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
     end
 
     def create
@@ -24,9 +20,8 @@ def create
       else
         @request = FamilyRequest.new({}, initial_items: 1)
         @errors = create_service.errors
-        @requestable_items = Organization.find(current_partner.organization_id).valid_items.map do |item|
-          [item[:name], item[:id]]
-        end.sort
+
+        @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}")
 

app/controllers/partners/requests_controller.rb

@@ -11,10 +11,7 @@ def new
       @partner_request = ::Request.new
       @partner_request.item_requests.build
 
-      requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
-      @formatted_requestable_items = requestable_items.map do |rt|
-        [rt.name, rt.id]
-      end.sort
+      @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
     end
 
     def show
@@ -36,9 +33,8 @@ def create
 
         @partner_request = create_service.partner_request
         @errors = create_service.errors
-        @formatted_requestable_items = Organization.find(current_partner.organization_id).valid_items.map do |item|
-          [item[:name], item[:id]]
-        end.sort
+
+        @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}")
 

app/javascript/controllers/request_item_controller.js

@@ -14,7 +14,6 @@ export default class extends Controller {
   }
 
   removeItem(event) {
-    console.log(event.target);
     const wrapper = event.target.closest("tr");
     const removeSoft = event.target.dataset.removeSoft === "false";
 

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/family_requests/new.html.erb

@@ -40,6 +40,8 @@
                 </div>
                 <div class="col-3">
                   <%= f.label :search_active, "Show Active Only?" %>
+                  <%# TODO: This checkbox needs styling fixes  %>
+
                   <%= f.check_box(
                           :search_active,
                           class: 'filterrific-periodically-observed form-control'

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

@@ -2,38 +2,76 @@
 
 describe PartnerFetchRequestableItemsService do
   describe '#call' do
-    subject { described_class.new(partner_id: partner_id).call }
-    let(:partner_id) { partner.id }
-    let(:partner) { create(:partner) }
-    let(:organization) { partner.organization }
+    subject { described_class.new(partner_id: partner.id).call }
+    let!(:organization) { create(:organization, skip_items: true, items: org_items) }
+    let(:partner) { create(:partner, organization: organization) }
+    let(:org_items) { [] }
+    let(:items_list) {
+      [
+        build(:item, active: true, visible_to_partners: true, name: 'Item 1'),
+        build(:item, active: false, visible_to_partners: true, name: 'Item 2'),
+        build(:item, active: true, visible_to_partners: false, name: 'Item 3'),
+        build(:item, active: false, visible_to_partners: false, name: 'Item 4')
+      ]
+    }
 
-    context 'when the partner id does not match any Partner' do
-      let(:partner_id) { 0 }
-
-      it 'raise an error indiciating the partner does not exist' do
-        expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
-      end
+    it 'raises an error indiciating the partner does not exist with invalid id' do
+      invalid_partner_id = 0
+      expect do
+        PartnerFetchRequestableItemsService.new(partner_id: invalid_partner_id).call
+      end.to raise_error(ActiveRecord::RecordNotFound)
     end
 
     context 'when the partner is not in any partner group' do
-      before do
-        expect(partner.partner_group).to be_nil
+      it { expect(partner.partner_group).to be_nil }
+
+      context 'when the organization has no items' do
+        let(:org_items) { [] }
+
+        it 'should return no items' do
+          expect(subject).to be_empty
+        end
       end
 
-      it 'should return all active and visible items' do
-        expect(subject).to eq(organization.items.active.visible)
+      context 'when the organization has items' do
+        let(:org_items) { items_list }
+
+        it { expect(organization.items).to eq(items_list) }
+
+        it 'should return only active and visible items' do
+          got_items = subject.map { |i| i[0] }
+          expect(got_items).to eq(["Item 1"])
+        end
       end
     end
 
-    context 'when the partner is in a partner group' do
-      before do
-        pg = create(:partner_group)
-        pg.item_categories << create(:item_category, organization: organization)
-        partner.update(partner_group: pg)
+    context 'org with any amount of items' do
+      let(:organization) { create(:organization) }
+
+      context 'when the partner is in a partner group and has items' do
+        before do
+          pg = create(:partner_group)
+          item_category = create(:item_category, organization: organization)
+          item_category.items << items_list
+          pg.item_categories << item_category
+          partner.update(partner_group: pg)
+        end
+
+        it 'should return only active and visible items from partner' do
+          got_items = subject.map { |i| i[0] }
+          expect(got_items).to eq(["Item 1"])
+        end
       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)
+      context 'when the partner is in a partner group and has no items' do
+        before do
+          pg = create(:partner_group)
+          partner.update(partner_group: pg)
+        end
+
+        it 'should return only active and visible items from partner' do
+          expect(subject).to be_empty
+        end
       end
     end
   end

spec/system/partner_system_spec.rb

@@ -431,14 +431,13 @@
         it 'it should say they can request every item' do
           assert page.has_content? 'All Items Requestable'
           assert page.has_content? 'Settings'
-          expect(PartnerFetchRequestableItemsService.new(partner_id: @partner.id).call).to eq(@organization.items.active.visible)
         end
       end
 
       context 'when a partner is assigned to partner group' do
         before do
           assert page.has_content? 'All Items Requestable'
-          expect(PartnerFetchRequestableItemsService.new(partner_id: @partner.id).call).to eq(@organization.items.active.visible)
+          expect(@partner.partner_group).to be_nil
         end
 
         context 'that has requestable item categories' do
@@ -457,7 +456,7 @@
 
           it 'should properly indicate the requestable items and adjust the partners requestable items' do
             assert page.has_content? item_category.name
-            expect(PartnerFetchRequestableItemsService.new(partner_id: @partner.id).call.sort).to eq(items_in_category.sort)
+            expect { @partner.reload }.to change(@partner, :requestable_items).from([]).to(items_in_category)
           end
         end
 
@@ -471,7 +470,7 @@
 
           it 'should properly indicate the requestable items and adjust the partners requestable items' do
             assert page.has_content? 'No Items Requestable'
-            expect(PartnerFetchRequestableItemsService.new(partner_id: @partner.id).call).to eq([])
+            expect { @partner.reload }.to change(@partner, :requestable_items).from([]).to([])
           end
         end
       end

spec/system/partners/managing_requests_system_spec.rb

@@ -1,9 +1,35 @@
 RSpec.describe "Managing requests", type: :system, js: true do
-  describe 'creating a individuals/family request' do
+  describe 'creating a # individuals request' do
     let(:partner_user) { partner.primary_user }
     let!(:partner) { FactoryBot.create(:partner) }
 
     context 'GIVEN a partner user is permitted to make a request' do
+      describe 'Select Input Tests' do
+        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(&: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'
+              click_link 'Add Another Item'
+            }
+            it 'should show the proper items in the select box' do
+              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
+        end
+      end
+
       before do
         login_as(partner_user)
         visit new_partners_individuals_request_path
@@ -51,14 +77,6 @@
 
           # delete an item
           find_all('td').last.click
-
-          # BUG: Consider how to make this work. Currently
-          # partners/family_request_create_service validates that no blank values
-          # got passed in. This results in an error. partners/request_create_service filters
-          # all blank items passed in. What is correct behavior?
-          #
-          # Trigger another row but keep it empty. It should still be valid!
-          # click_link 'Add Another Item'
         end
 
         context 'THEN a request records will be created and the partner will be notified via flash message on the dashboard' do
@@ -86,11 +104,39 @@
     end
   end
 
-  describe 'creating a request' do
+  describe 'creating a new quantity request' do
     let(:partner_user) { partner.primary_user }
     let!(:partner) { FactoryBot.create(:partner) }
 
     context 'GIVEN a partner user is permitted to make a request' do
+      describe 'Select Input Tests' do
+        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_request_path
+          end
+
+          it 'should show the proper items in the select box' do
+            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'
+              click_link 'Add Another Item'
+            }
+
+            it 'should show the proper items in the select box' do
+              expected_items = requestable_items.map(&:first).unshift('Select an item')
+              expect(page.all('select option').map(&:text)).to eq(expected_items)
+            end
+          end
+        end
+      end
+
       before do
         login_as(partner_user)
         visit new_partners_request_path