From 0024454a5d02432567c49bab56b920bc3db9cc8a Mon Sep 17 00:00:00 2001 From: Andrew Garvin Date: Thu, 4 Sep 2025 21:59:15 -0400 Subject: [PATCH 1/7] close icon no longer doubled --- app/views/layouts/partners/application.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/layouts/partners/application.html.erb b/app/views/layouts/partners/application.html.erb index b1e6f1129a..35c342be27 100644 --- a/app/views/layouts/partners/application.html.erb +++ b/app/views/layouts/partners/application.html.erb @@ -66,7 +66,7 @@
<% flash.each do |key, value| %> <% end %> From 7e0e23b545ad639fd5b735b8174355890e354751 Mon Sep 17 00:00:00 2001 From: Andrew Garvin Date: Tue, 16 Sep 2025 00:57:36 -0400 Subject: [PATCH 2/7] Error message specifies item and child that can't be requested --- .../partners/family_requests_controller.rb | 63 +++++++++++++++---- .../partners/family_requests_requests_spec.rb | 16 +++++ 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/app/controllers/partners/family_requests_controller.rb b/app/controllers/partners/family_requests_controller.rb index 0784dc0ee1..b840557122 100644 --- a/app/controllers/partners/family_requests_controller.rb +++ b/app/controllers/partners/family_requests_controller.rb @@ -15,7 +15,12 @@ def new end def create - family_requests_attributes = build_family_requests_attributes(params) + begin + family_requests_attributes = build_family_requests_attributes(params) + rescue ArgumentError => e + redirect_to new_partners_family_request_path, error: e + return + end create_service = Partners::FamilyRequestCreateService.new( partner_user_id: current_user.id, @@ -28,7 +33,7 @@ def create if create_service.errors.none? redirect_to partners_request_path(create_service.partner_request), notice: "Requested items successfully!" else - redirect_to new_partners_family_request_path, error: "Request failed! #{create_service.errors.map { |error| error.message.to_s }}}" + redirect_to new_partners_family_request_path, error: "Request failed! #{create_service.errors.map { |error| error.message.to_s }}" end end @@ -55,21 +60,55 @@ def validate private def build_family_requests_attributes(params) - children_ids = [] + child_ids = params.keys.grep(/^child-/).map { |key| key.split('-').last } - params.each do |key, _| - is_child, id = key.split('-') - if is_child == 'child' - children_ids << id - end - end + validate_visible_items!(child_ids) - children = current_partner.children.where(id: children_ids).joins(:requested_items).select('children.*', :item_id) + children_grouped_by_item_id = current_partner + .children + .where(id: child_ids) + .joins(:requested_items) + .select('children.*', 'items.id as item_id', + 'items.name as item_name', + 'items.visible_to_partners') + .group_by(&:item_id) - children_grouped_by_item_id = children.group_by(&:item_id) children_grouped_by_item_id.map do |item_id, item_requested_children| - { item_id: item_id, person_count: item_requested_children.size, children: item_requested_children } + { + item_id: item_id, + person_count: item_requested_children.size, + children: item_requested_children + } + end + end + + def validate_visible_items!(child_ids) + invisible_item_requests = current_partner + .children + .where(id: child_ids) + .joins(:requested_items) + .where(items: { visible_to_partners: false }) + .select( + 'children.first_name', + 'children.last_name', + 'items.id as item_id', + 'items.name as item_name', + 'items.visible_to_partners' + ) + .group_by(&:item_id) + + return if invisible_item_requests.empty? + + # raise an exception if there are requests + # to items that aren't visible to partners + item_errors = invisible_item_requests.map do |_, children| + children_names = children.map { |c| "#{c.first_name} #{c.last_name}" }.join(", ") + item_name = children.first.item_name + + "\"#{item_name}\" requested for #{children_names} is not currently available for request." end + + raise ArgumentError.new item_errors.join(", ") end end end diff --git a/spec/requests/partners/family_requests_requests_spec.rb b/spec/requests/partners/family_requests_requests_spec.rb index 5aed5137a6..8852ea3d03 100644 --- a/spec/requests/partners/family_requests_requests_spec.rb +++ b/spec/requests/partners/family_requests_requests_spec.rb @@ -48,6 +48,22 @@ expect(subject).to redirect_to(partners_requests_path) end + it "does not allow requesting non-visible items" do + partner.update!(status: :approved) + + # update child item to not be visible to partners + i = Item.first + i.update!(visible_to_partners: false) + + subject + + child_with_unavailable_item = children[0] + + expected = "\"#{i.name}\" requested for #{child_with_unavailable_item.first_name} #{child_with_unavailable_item.last_name} is not currently available for request." + + expect(response.request.flash[:error].message).to eql expected + end + it "submits the request" do partner.update!(status: :approved) From adb953aada4d1d29674ccabac2e32e07906e8738 Mon Sep 17 00:00:00 2001 From: Andrew Garvin Date: Thu, 9 Oct 2025 23:34:26 -0400 Subject: [PATCH 3/7] item visibility error moved to service file --- .../partners/family_requests_controller.rb | 38 +------------------ .../partners/family_request_create_service.rb | 29 ++++++++++++++ 2 files changed, 30 insertions(+), 37 deletions(-) diff --git a/app/controllers/partners/family_requests_controller.rb b/app/controllers/partners/family_requests_controller.rb index b840557122..72898b524e 100644 --- a/app/controllers/partners/family_requests_controller.rb +++ b/app/controllers/partners/family_requests_controller.rb @@ -15,12 +15,7 @@ def new end def create - begin - family_requests_attributes = build_family_requests_attributes(params) - rescue ArgumentError => e - redirect_to new_partners_family_request_path, error: e - return - end + family_requests_attributes = build_family_requests_attributes(params) create_service = Partners::FamilyRequestCreateService.new( partner_user_id: current_user.id, @@ -62,8 +57,6 @@ def validate def build_family_requests_attributes(params) child_ids = params.keys.grep(/^child-/).map { |key| key.split('-').last } - validate_visible_items!(child_ids) - children_grouped_by_item_id = current_partner .children .where(id: child_ids) @@ -81,34 +74,5 @@ def build_family_requests_attributes(params) } end end - - def validate_visible_items!(child_ids) - invisible_item_requests = current_partner - .children - .where(id: child_ids) - .joins(:requested_items) - .where(items: { visible_to_partners: false }) - .select( - 'children.first_name', - 'children.last_name', - 'items.id as item_id', - 'items.name as item_name', - 'items.visible_to_partners' - ) - .group_by(&:item_id) - - return if invisible_item_requests.empty? - - # raise an exception if there are requests - # to items that aren't visible to partners - item_errors = invisible_item_requests.map do |_, children| - children_names = children.map { |c| "#{c.first_name} #{c.last_name}" }.join(", ") - item_name = children.first.item_name - - "\"#{item_name}\" requested for #{children_names} is not currently available for request." - end - - raise ArgumentError.new item_errors.join(", ") - end end end diff --git a/app/services/partners/family_request_create_service.rb b/app/services/partners/family_request_create_service.rb index 8640ed249a..4a01119778 100644 --- a/app/services/partners/family_request_create_service.rb +++ b/app/services/partners/family_request_create_service.rb @@ -57,6 +57,8 @@ def valid? errors.add(:base, 'detected a unknown item_id') end + check_for_item_visibility + errors.none? end @@ -71,6 +73,33 @@ def item_requests_attributes end end + # If requested item(s) isn't visible to partners, + # an error specifying which item is not available is raised + def check_for_item_visibility + invisible_items = item_requests_attributes.select { |attr| !included_items_by_id[attr[:item_id]].visible_to_partners } + + unless invisible_items.empty? + + item_errors = invisible_items.map do |item| + item_name = included_items_by_id[item[:item_id]].name + + child_count = item[:children].length + + "#{item_name} requested for #{child_count} child#{"ren" if child_count > 1} is not currently available for request." + end + + joined_errors = item_errors.join(", ") + + # don't want to show a memflash error + if joined_errors.length >= Memflash.threshold + errors.add(:base, item_errors.first) + else + errors.add(:base, joined_errors) + end + + end + end + def convert_person_count_to_item_quantity(item_id:, person_count:) item = included_items_by_id[item_id.to_i] From b7c0550e7dad9b3d0721b5b94021beb6ae0a0088 Mon Sep 17 00:00:00 2001 From: Andrew Garvin Date: Thu, 9 Oct 2025 23:49:09 -0400 Subject: [PATCH 4/7] updated tests to reflect change in error message --- .../partners/family_requests_requests_spec.rb | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/spec/requests/partners/family_requests_requests_spec.rb b/spec/requests/partners/family_requests_requests_spec.rb index 8852ea3d03..f7161e4f83 100644 --- a/spec/requests/partners/family_requests_requests_spec.rb +++ b/spec/requests/partners/family_requests_requests_spec.rb @@ -57,11 +57,25 @@ subject - child_with_unavailable_item = children[0] + expected = "Request failed! [\"#{i.name} requested for 1 child is not currently available for request.\"]" - expected = "\"#{i.name}\" requested for #{child_with_unavailable_item.first_name} #{child_with_unavailable_item.last_name} is not currently available for request." + expect(response.request.flash[:error]).to eql expected + end + + it "does not allow requesting non-visible items for multiple children" do + partner.update!(status: :approved) + + # update child item to not be visible to partners + i = Item.first + i.update!(visible_to_partners: false) + + children[1].update(requested_item_ids: [i.id]) + + subject + + expected = "Request failed! [\"#{i.name} requested for 2 children is not currently available for request.\"]" - expect(response.request.flash[:error].message).to eql expected + expect(response.request.flash[:error]).to eql expected end it "submits the request" do From 13beeb4cc5b3b25df5beb4dd9312876b0607ecfe Mon Sep 17 00:00:00 2001 From: Andrew Garvin Date: Thu, 9 Oct 2025 23:51:18 -0400 Subject: [PATCH 5/7] addressing rubocop offense --- spec/requests/partners/family_requests_requests_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/partners/family_requests_requests_spec.rb b/spec/requests/partners/family_requests_requests_spec.rb index f7161e4f83..56b970ab59 100644 --- a/spec/requests/partners/family_requests_requests_spec.rb +++ b/spec/requests/partners/family_requests_requests_spec.rb @@ -61,7 +61,7 @@ expect(response.request.flash[:error]).to eql expected end - + it "does not allow requesting non-visible items for multiple children" do partner.update!(status: :approved) From 5cc93fbcaed3432ddb7f9547a4567551bad7cbe9 Mon Sep 17 00:00:00 2001 From: Andrew Garvin Date: Fri, 10 Oct 2025 00:44:53 -0400 Subject: [PATCH 6/7] fixes for failing tests --- app/services/partners/family_request_create_service.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/services/partners/family_request_create_service.rb b/app/services/partners/family_request_create_service.rb index 4a01119778..75d65709ce 100644 --- a/app/services/partners/family_request_create_service.rb +++ b/app/services/partners/family_request_create_service.rb @@ -76,7 +76,10 @@ def item_requests_attributes # If requested item(s) isn't visible to partners, # an error specifying which item is not available is raised def check_for_item_visibility - invisible_items = item_requests_attributes.select { |attr| !included_items_by_id[attr[:item_id]].visible_to_partners } + invisible_items = item_requests_attributes.select { |attr| + !included_items_by_id[attr[:item_id]].nil? && + !included_items_by_id[attr[:item_id]].visible_to_partners + } unless invisible_items.empty? From b4fab28711900655866b14fdc5f9228626669668 Mon Sep 17 00:00:00 2001 From: Andrew Garvin Date: Wed, 26 Nov 2025 01:29:11 -0500 Subject: [PATCH 7/7] Error message is more inline with expectations --- app/controllers/partners/family_requests_controller.rb | 2 +- app/services/partners/family_request_create_service.rb | 7 ++++--- spec/requests/partners/family_requests_requests_spec.rb | 9 +++++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/controllers/partners/family_requests_controller.rb b/app/controllers/partners/family_requests_controller.rb index e7e557d819..c13c88d27f 100644 --- a/app/controllers/partners/family_requests_controller.rb +++ b/app/controllers/partners/family_requests_controller.rb @@ -29,7 +29,7 @@ def create if create_service.errors.none? redirect_to partners_request_path(create_service.partner_request), notice: "Requested items successfully!" else - redirect_to new_partners_family_request_path, error: "Request failed! #{create_service.errors.map { |error| error.message.to_s }}" + redirect_to new_partners_family_request_path, error: create_service.errors.map { |error| error.message.to_s }.join(",").to_s end end diff --git a/app/services/partners/family_request_create_service.rb b/app/services/partners/family_request_create_service.rb index 75d65709ce..d11bb56975 100644 --- a/app/services/partners/family_request_create_service.rb +++ b/app/services/partners/family_request_create_service.rb @@ -86,16 +86,17 @@ def check_for_item_visibility item_errors = invisible_items.map do |item| item_name = included_items_by_id[item[:item_id]].name - child_count = item[:children].length + children_names = item[:children].map { |c| "#{c.first_name} #{c.last_name}" }.join(", ") - "#{item_name} requested for #{child_count} child#{"ren" if child_count > 1} is not currently available for request." + "\"#{item_name}\" requested for #{children_names} is not currently available for request." end joined_errors = item_errors.join(", ") # don't want to show a memflash error if joined_errors.length >= Memflash.threshold - errors.add(:base, item_errors.first) + truncated_errors = joined_errors[0...(Memflash.threshold - 10)] + errors.add(:base, "#{truncated_errors}...") else errors.add(:base, joined_errors) end diff --git a/spec/requests/partners/family_requests_requests_spec.rb b/spec/requests/partners/family_requests_requests_spec.rb index 56b970ab59..4a1a1114fb 100644 --- a/spec/requests/partners/family_requests_requests_spec.rb +++ b/spec/requests/partners/family_requests_requests_spec.rb @@ -57,7 +57,8 @@ subject - expected = "Request failed! [\"#{i.name} requested for 1 child is not currently available for request.\"]" + child_with_unavailable_item = children[0] + expected = "\"#{i.name}\" requested for #{child_with_unavailable_item.first_name} #{child_with_unavailable_item.last_name} is not currently available for request." expect(response.request.flash[:error]).to eql expected end @@ -73,7 +74,11 @@ subject - expected = "Request failed! [\"#{i.name} requested for 2 children is not currently available for request.\"]" + children_with_unavailable_item = children[0..1] + + child_formatting = children_with_unavailable_item.map { |c| "#{c.first_name} #{c.last_name}" }.join(", ") + + expected = "\"#{i.name}\" requested for #{child_formatting} is not currently available for request." expect(response.request.flash[:error]).to eql expected end