From 85da11f904a35b47353d9b8d1fbde9dffc84c19e Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 5 May 2026 16:11:07 -0600 Subject: [PATCH 01/12] Refactor requests controller Moving view instances to a View Object. --- app/controllers/requests_controller.rb | 20 +---- app/models/view/requests.rb | 84 +++++++++++++++++++ .../_calculate_product_totals.html.erb | 2 +- app/views/requests/_new.html.erb | 2 +- app/views/requests/_request_row.html.erb | 2 +- app/views/requests/index.html.erb | 18 ++-- 6 files changed, 98 insertions(+), 30 deletions(-) create mode 100644 app/models/view/requests.rb diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index ce5677da0c..5dea6b16a3 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -3,27 +3,11 @@ class RequestsController < ApplicationController def index setup_date_range_picker - @requests = current_organization - .ordered_requests - .undiscarded - .during(helpers.selected_range) - .class_filter(filter_params) - @unfulfilled_requests_count = current_organization.requests.where(status: [:pending, :started]).during(helpers.selected_range).class_filter(filter_params).count - @paginated_requests = @requests.includes(:partner).page(params[:page]) - @calculate_product_totals = RequestsTotalItemsService.new(requests: @requests).calculate - @items = current_organization.items.alphabetized.select(:id, :name) - @partners = current_organization.partners.alphabetized.select(:id, :name, :status) - @statuses = Request.statuses.transform_keys(&:humanize) - @partner_users = User.where(id: @paginated_requests.map(&:partner_user_id)).select(:id, :name, :email) - @request_types = Request.request_types.transform_keys(&:humanize) - @selected_request_type = filter_params[:by_request_type] - @selected_request_item = filter_params[:by_request_item_id] - @selected_partner = filter_params[:by_partner] - @selected_status = filter_params[:by_status] + @request_info = View::Requests.from_params(params: params, organization: current_organization, helpers: helpers) respond_to do |format| format.html - format.csv { send_data Exports::ExportRequestService.new(@requests).generate_csv, filename: "Requests-#{Time.zone.today}.csv" } + format.csv { send_data Exports::ExportRequestService.new(@request_info.requests).generate_csv, filename: "Requests-#{Time.zone.today}.csv" } end end diff --git a/app/models/view/requests.rb b/app/models/view/requests.rb new file mode 100644 index 0000000000..55ae5cf90c --- /dev/null +++ b/app/models/view/requests.rb @@ -0,0 +1,84 @@ +module View + Requests = Data.define( + :requests, + :filters, + :paginated_requests, + :organization, + :helpers + ) do + include DateRangeHelper + + class << self + def filter_params(params = {}) + if params.key?(:filters) + params.require(:filters).permit(:by_request_item_id, :by_partner, :by_status, :by_request_type) + else + {} + end + end + + def from_params(params:, organization:, helpers:) + filters = filter_params(params) + + requests = organization + .ordered_requests + .undiscarded + .during(helpers.selected_range) + .class_filter(filters) + + paginated_requests = requests.includes(:partner).page(params[:page]) + + new(requests:, filters:, paginated_requests:, organization:, helpers:) + end + end + + def unfulfilled_requests_count + organization + .requests + .where(status: [:pending, :started]) + .during(helpers.selected_range) + .class_filter(filters) + .count + end + + def calculate_product_totals + RequestsTotalItemsService.new(requests: requests).calculate + end + + def items + organization.items.alphabetized.select(:id, :name) + end + + def partners + organization.partners.alphabetized.select(:id, :name, :status) + end + + def statuses + Request.statuses.transform_keys(&:humanize) + end + + def partner_users + User.where(id: paginated_requests.map(&:partner_user_id)).select(:id, :name, :email) + end + + def request_types + Request.request_types.transform_keys(&:humanize) + end + + def selected_request_type + filter_params[:by_request_type] + end + + def selected_request_item + filter_params[:by_request_item_id] + end + + def selected_partner + filter_params[:by_partner] + end + + def selected_status + filter_params[:by_status] + end + end +end diff --git a/app/views/requests/_calculate_product_totals.html.erb b/app/views/requests/_calculate_product_totals.html.erb index 5410923fb9..9b53f483af 100644 --- a/app/views/requests/_calculate_product_totals.html.erb +++ b/app/views/requests/_calculate_product_totals.html.erb @@ -16,7 +16,7 @@ - <% @calculate_product_totals.sort_by { |name, quantity| name.downcase }.each do |name, quantity| %> + <% @request_info.calculate_product_totals.sort_by { |name, quantity| name.downcase }.each do |name, quantity| %> <%= name %> <%= quantity %> diff --git a/app/views/requests/_new.html.erb b/app/views/requests/_new.html.erb index 9463b0b9ff..c6e774f1f9 100644 --- a/app/views/requests/_new.html.erb +++ b/app/views/requests/_new.html.erb @@ -10,7 +10,7 @@ From 8089be6cb25bfff90506e634fc486dbaf6454e9b Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 5 May 2026 16:11:33 -0600 Subject: [PATCH 02/12] Remove unnecessary donations eager loading product_drive_participant This was raising a bullet error: GET /donations AVOID eager loading detected Donation => [:product_drive_participant] Remove from your query: .includes([:product_drive_participant]) --- app/models/view/donations.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/view/donations.rb b/app/models/view/donations.rb index 9518e98047..24e52d16f1 100644 --- a/app/models/view/donations.rb +++ b/app/models/view/donations.rb @@ -31,7 +31,6 @@ def from_params(params:, organization:, helpers:) .includes(:storage_location, :donation_site, :product_drive, - :product_drive_participant, :manufacturer, line_items: [:item]) .order(created_at: :desc) From b6908014b08854a31eb0616cf17fb3d0dd33e7b0 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 5 May 2026 16:42:24 -0600 Subject: [PATCH 03/12] Refactor show action Moving view instances to a View Object to keep the controller clean. Add coverage for business logic --- app/models/view/requests.rb | 8 ++--- spec/models/view/requests_spec.rb | 59 +++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 spec/models/view/requests_spec.rb diff --git a/app/models/view/requests.rb b/app/models/view/requests.rb index 55ae5cf90c..0a9697aa05 100644 --- a/app/models/view/requests.rb +++ b/app/models/view/requests.rb @@ -66,19 +66,19 @@ def request_types end def selected_request_type - filter_params[:by_request_type] + filters[:by_request_type] end def selected_request_item - filter_params[:by_request_item_id] + filters[:by_request_item_id] end def selected_partner - filter_params[:by_partner] + filters[:by_partner] end def selected_status - filter_params[:by_status] + filters[:by_status] end end end diff --git a/spec/models/view/requests_spec.rb b/spec/models/view/requests_spec.rb new file mode 100644 index 0000000000..dd556a96d0 --- /dev/null +++ b/spec/models/view/requests_spec.rb @@ -0,0 +1,59 @@ +RSpec.describe View::Requests do + describe "#unfulfilled_requests_count" do + it "returns the unfulfilled requests count for the given date range" do + organization = create(:organization) + create(:request, :pending, organization:) + create(:request, :started, organization:) + create(:request, :fulfilled, organization:) + + requests = View::Requests.from_params(params: {}, organization:, helpers:) + + expect(requests.unfulfilled_requests_count).to eq(2) + end + end + + describe "#calculate_product_totals" do + it "returns the product total items service" do + organization = create(:organization) + create(:request, :pending, organization:) + total_items_service_double = instance_double(RequestsTotalItemsService, calculate: {"Diaper" => 10}) + allow(RequestsTotalItemsService).to receive(:new).with(requests: organization.requests).and_return(total_items_service_double) + + requests = View::Requests.from_params(params: {}, organization:, helpers:) + + expect(requests.calculate_product_totals).to eq({"Diaper" => 10}) + end + end + + describe "selected filter params" do + it "returns the filter params given" do + organization = create(:organization) + create(:request, :pending, organization:) + params = ActionController::Parameters.new( + { + filters: { + by_request_type: "quantity", + by_request_item_id: "1", + by_partner: "A Local Partner", + by_status: "pending" + } + } + ).permit! + + requests = View::Requests.from_params(params:, organization:, helpers:) + + expect(requests.selected_request_type).to eq("quantity") + expect(requests.selected_request_item).to eq("1") + expect(requests.selected_partner).to eq("A Local Partner") + expect(requests.selected_status).to eq("pending") + end + end + + def helpers + Class.new do + def self.selected_range + (1.month.ago..2.days.from_now) + end + end + end +end From 72f9a3eac41e7ea4c725f3d5b92682a7b7420477 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Mon, 11 May 2026 15:20:25 -0600 Subject: [PATCH 04/12] rename requests_info to avoid clashing with show request_info add coverage for Request model view --- app/controllers/requests_controller.rb | 13 +-- app/models/view/request_info.rb | 34 ++++++++ .../_calculate_product_totals.html.erb | 2 +- app/views/requests/_new.html.erb | 2 +- app/views/requests/_request_row.html.erb | 2 +- app/views/requests/index.html.erb | 18 ++-- app/views/requests/show.html.erb | 48 +++++------ spec/models/view/request_info_spec.rb | 82 +++++++++++++++++++ 8 files changed, 155 insertions(+), 46 deletions(-) create mode 100644 app/models/view/request_info.rb create mode 100644 spec/models/view/request_info_spec.rb diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 5dea6b16a3..f0d625c7af 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -3,23 +3,16 @@ class RequestsController < ApplicationController def index setup_date_range_picker - @request_info = View::Requests.from_params(params: params, organization: current_organization, helpers: helpers) + @requests_info = View::Requests.from_params(params: params, organization: current_organization, helpers: helpers) respond_to do |format| format.html - format.csv { send_data Exports::ExportRequestService.new(@request_info.requests).generate_csv, filename: "Requests-#{Time.zone.today}.csv" } + format.csv { send_data Exports::ExportRequestService.new(@requests_info.requests).generate_csv, filename: "Requests-#{Time.zone.today}.csv" } end end def show - @request = current_organization.requests.find(params[:id]) - @item_requests = @request.item_requests.includes(:item) - - @inventory = View::Inventory.new(@request.organization_id) - @default_storage_location = @request.partner.default_storage_location_id || @request.organization.default_storage_location - @location = StorageLocation.find_by(id: @default_storage_location) - - @custom_units = Flipper.enabled?(:enable_packs) && @request.item_requests.any? { |item| item.request_unit } + @request_info = View::RequestInfo.from_params(params:, organization: current_organization) end # Clicking the "New Distribution" button will set the the request to started diff --git a/app/models/view/request_info.rb b/app/models/view/request_info.rb new file mode 100644 index 0000000000..8b4a4b3bd5 --- /dev/null +++ b/app/models/view/request_info.rb @@ -0,0 +1,34 @@ +module View + RequestInfo = Data.define( + :request + ) do + + class << self + def from_params(params:, organization:) + request = organization.requests.find(params[:id]) + + new(request:) + end + end + + def item_requests + request.item_requests.includes(:item) + end + + def inventory + View::Inventory.new(request.organization_id) + end + + def default_storage_location + request.partner.default_storage_location_id || request.organization.default_storage_location + end + + def location + StorageLocation.find_by(id: default_storage_location) + end + + def custom_units + Flipper.enabled?(:enable_packs) && request.item_requests.any? { |item| item.request_unit } + end + end +end diff --git a/app/views/requests/_calculate_product_totals.html.erb b/app/views/requests/_calculate_product_totals.html.erb index 9b53f483af..add383533f 100644 --- a/app/views/requests/_calculate_product_totals.html.erb +++ b/app/views/requests/_calculate_product_totals.html.erb @@ -16,7 +16,7 @@ - <% @request_info.calculate_product_totals.sort_by { |name, quantity| name.downcase }.each do |name, quantity| %> + <% @requests_info.calculate_product_totals.sort_by { |name, quantity| name.downcase }.each do |name, quantity| %> <%= name %> <%= quantity %> diff --git a/app/views/requests/_new.html.erb b/app/views/requests/_new.html.erb index c6e774f1f9..a4a1c075e9 100644 --- a/app/views/requests/_new.html.erb +++ b/app/views/requests/_new.html.erb @@ -10,7 +10,7 @@ diff --git a/app/views/requests/show.html.erb b/app/views/requests/show.html.erb index 19a44d5cd5..b3bd1f21d2 100644 --- a/app/views/requests/show.html.erb +++ b/app/views/requests/show.html.erb @@ -2,10 +2,10 @@
- <% content_for :title, "Requests - #{@request.id} - #{current_organization.name}" %> + <% content_for :title, "Requests - #{@request_info.request.id} - #{current_organization.name}" %>

Request - from <%= @request.partner.name %> + from <%= @request_info.request.partner.name %>

@@ -29,7 +29,7 @@
-

This request was sent on <%= @request.created_at.to_fs(:distribution_date) %>

+

This request was sent on <%= @request_info.request.created_at.to_fs(:distribution_date) %>

@@ -44,11 +44,11 @@ - - - - - + + + + +
<%= @request.partner.name %><%= @request.partner_user&.formatted_email %><%= @request.request_type&.humanize %><%= render partial: "status", locals: {status: @request.status} %><%= @request.comments || 'None' %><%= @request_info.request.partner.name %><%= @request_info.request.partner_user&.formatted_email %><%= @request_info.request.request_type&.humanize %><%= render partial: "status", locals: {status: @request_info.request.status} %><%= @request_info.request.comments || 'None' %>
@@ -67,36 +67,36 @@ Item Quantity - <% if @custom_units %> + <% if @request_info.custom_units %> Units (if applicable) <% end %> - <% if @default_storage_location %> + <% if @request_info.default_storage_location %> Default storage location inventory <% end %> Total Inventory - <% @item_requests.each do |item_request| %> + <% @request_info.item_requests.each do |item_request| %> <%= item_request.item_name %> <%= item_request.quantity %> - <% if @custom_units %> + <% if @request_info.custom_units %> <%= item_request.request_unit&.pluralize(item_request.quantity.to_i) %> <% end %> - <% if @default_storage_location %> - <% on_hand_for_location = @inventory.quantity_for(storage_location: @location&.id, item_id: item_request.item_id) %> + <% if @request_info.default_storage_location %> + <% on_hand_for_location = @request_info.inventory.quantity_for(storage_location: @request_info.location&.id, item_id: item_request.item_id) %> <%= on_hand_for_location&.positive? ? on_hand_for_location : 'N/A' %> <% end %> - <% on_hand = @inventory.quantity_for(item_id: item_request.item_id) %> + <% on_hand = @request_info.inventory.quantity_for(item_id: item_request.item_id) %> <%= on_hand || 0 %> <% end %> Total (Quota) - <%= @request.total_items %> - <%= quota_display(@request.partner) %> + <%= @request_info.request.total_items %> + <%= quota_display(@request_info.request.partner) %> @@ -104,11 +104,11 @@
diff --git a/spec/models/view/request_info_spec.rb b/spec/models/view/request_info_spec.rb new file mode 100644 index 0000000000..be4c9fce01 --- /dev/null +++ b/spec/models/view/request_info_spec.rb @@ -0,0 +1,82 @@ +RSpec.describe View::RequestInfo do + describe "#default_storage_location" do + context "when the request partner has a default_storage_location_id" do + it "returns the request partner's default_storage_location_id" do + storage_location = build(:storage_location) + organization = build(:organization) + request = create( + :request, + partner: build(:partner, default_storage_location_id: storage_location.id), + organization: + ) + + request = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request.default_storage_location).to eq(storage_location.id) + end + end + + context "when the request organization has a default_storage_location_id" do + it "returns the request organization's default_storage_location_id" do + organization = build(:organization) + storage_location = build(:storage_location, organization:) + organization.update!(default_storage_location: storage_location.id) + request = create( + :request, + organization: + ) + + request = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request.default_storage_location).to eq(storage_location.id) + end + end + end + + describe "#custom_units" do + context "when a request has request units for an item request" do + context "when enable_packs is disabled" do + it "returns false" do + organization = build(:organization) + item = build(:item, name: "First item") + create(:item_unit, item: item, name: "flat") + request = create( + :request, + :with_item_requests, + organization:, + request_items: [ + {item_id: item.id, quantity: '559', request_unit: 'flat'} + ] + ) + + request = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request.custom_units).to be_falsey + end + end + + context "when enable_packs is enabled" do + it "returns true" do + Flipper.enable(:enable_packs) + + organization = build(:organization) + item = build(:item, name: "First item") + create(:item_unit, item: item, name: "flat") + request = create( + :request, + :with_item_requests, + organization:, + request_items: [ + {item_id: item.id, quantity: '559', request_unit: 'flat'}, + ] + ) + request = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request.custom_units).to be_truthy + + Flipper.disable(:enable_packs) + end + end + end + end +end From 9a694c4c45f2770cd618df498690b198165009c5 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Mon, 11 May 2026 15:58:01 -0600 Subject: [PATCH 05/12] Create factories only when necessary --- spec/models/view/request_info_spec.rb | 2 +- spec/models/view/requests_spec.rb | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/models/view/request_info_spec.rb b/spec/models/view/request_info_spec.rb index be4c9fce01..ea207260cf 100644 --- a/spec/models/view/request_info_spec.rb +++ b/spec/models/view/request_info_spec.rb @@ -34,7 +34,7 @@ end describe "#custom_units" do - context "when a request has request units for an item request" do + context "when there are request units for an item request" do context "when enable_packs is disabled" do it "returns false" do organization = build(:organization) diff --git a/spec/models/view/requests_spec.rb b/spec/models/view/requests_spec.rb index dd556a96d0..40794a3d17 100644 --- a/spec/models/view/requests_spec.rb +++ b/spec/models/view/requests_spec.rb @@ -1,7 +1,7 @@ RSpec.describe View::Requests do describe "#unfulfilled_requests_count" do - it "returns the unfulfilled requests count for the given date range" do - organization = create(:organization) + it "returns the unfulfilled requests count" do + organization = build(:organization) create(:request, :pending, organization:) create(:request, :started, organization:) create(:request, :fulfilled, organization:) @@ -13,9 +13,9 @@ end describe "#calculate_product_totals" do - it "returns the product total items service" do - organization = create(:organization) - create(:request, :pending, organization:) + it "returns the calculated product total items" do + organization = build(:organization) + build(:request, :pending, organization:) total_items_service_double = instance_double(RequestsTotalItemsService, calculate: {"Diaper" => 10}) allow(RequestsTotalItemsService).to receive(:new).with(requests: organization.requests).and_return(total_items_service_double) @@ -26,9 +26,9 @@ end describe "selected filter params" do - it "returns the filter params given" do - organization = create(:organization) - create(:request, :pending, organization:) + it "returns the given filter params" do + organization = build(:organization) + build(:request, :pending, organization:) params = ActionController::Parameters.new( { filters: { @@ -50,7 +50,7 @@ end def helpers - Class.new do + Module.new do def self.selected_range (1.month.ago..2.days.from_now) end From 1f08698c3f2a443fc4fe7d14d6dd2015949181de Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Mon, 11 May 2026 17:37:34 -0600 Subject: [PATCH 06/12] Rename filter variables with the view object instance from the controller --- app/views/requests/index.html.erb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/requests/index.html.erb b/app/views/requests/index.html.erb index 42d9d323f5..6cde5bfb30 100644 --- a/app/views/requests/index.html.erb +++ b/app/views/requests/index.html.erb @@ -39,19 +39,19 @@
<% if @requests_info.items.present? %>
- <%= filter_select(label: "Filter by item", scope: :by_request_item_id, collection: @items, selected: @selected_request_item) %> + <%= filter_select(label: "Filter by item", scope: :by_request_item_id, collection: @requests_info.items, selected: @requests_info.selected_request_item) %>
<% end %> <% if @requests_info.partners.present? %>
- <%= filter_select(scope: :by_partner, collection: @partners, selected: @selected_partner) %> + <%= filter_select(scope: :by_partner, collection: @requests_info.partners, selected: @requests_info.selected_partner) %>
<% end %>
- <%= filter_select(scope: :by_request_type, collection: @requests_info.request_types, key: :last, value: :first, selected: @selected_request_type) %> + <%= filter_select(scope: :by_request_type, collection: @requests_info.request_types, key: :last, value: :first, selected: @requests_info.selected_request_type) %>
- <%= filter_select(scope: :by_status, collection: @requests_info.statuses, key: :last, value: :first, selected: @selected_status) %> + <%= filter_select(scope: :by_status, collection: @requests_info.statuses, key: :last, value: :first, selected: @requests_info.selected_status) %>
<%= label_tag "Date Range" %> From 7664d93d2e8873678956d27c91fc797f891ed497 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Mon, 11 May 2026 17:42:25 -0600 Subject: [PATCH 07/12] linter --- app/controllers/requests_controller.rb | 2 +- app/models/view/request_info.rb | 1 - spec/models/view/request_info_spec.rb | 6 +++--- spec/models/view/requests_spec.rb | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index f0d625c7af..bf89ccd067 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -12,7 +12,7 @@ def index end def show - @request_info = View::RequestInfo.from_params(params:, organization: current_organization) + @request_info = View::RequestInfo.from_params(params:, organization: current_organization) end # Clicking the "New Distribution" button will set the the request to started diff --git a/app/models/view/request_info.rb b/app/models/view/request_info.rb index 8b4a4b3bd5..9e325e232a 100644 --- a/app/models/view/request_info.rb +++ b/app/models/view/request_info.rb @@ -2,7 +2,6 @@ module View RequestInfo = Data.define( :request ) do - class << self def from_params(params:, organization:) request = organization.requests.find(params[:id]) diff --git a/spec/models/view/request_info_spec.rb b/spec/models/view/request_info_spec.rb index ea207260cf..fd8a64706b 100644 --- a/spec/models/view/request_info_spec.rb +++ b/spec/models/view/request_info_spec.rb @@ -35,7 +35,7 @@ describe "#custom_units" do context "when there are request units for an item request" do - context "when enable_packs is disabled" do + context "when enable_packs is disabled" do it "returns false" do organization = build(:organization) item = build(:item, name: "First item") @@ -45,7 +45,7 @@ :with_item_requests, organization:, request_items: [ - {item_id: item.id, quantity: '559', request_unit: 'flat'} + {item_id: item.id, quantity: "559", request_unit: "flat"} ] ) @@ -67,7 +67,7 @@ :with_item_requests, organization:, request_items: [ - {item_id: item.id, quantity: '559', request_unit: 'flat'}, + {item_id: item.id, quantity: "559", request_unit: "flat"} ] ) request = View::RequestInfo.from_params(params: {id: request.id}, organization:) diff --git a/spec/models/view/requests_spec.rb b/spec/models/view/requests_spec.rb index 40794a3d17..855af78a0b 100644 --- a/spec/models/view/requests_spec.rb +++ b/spec/models/view/requests_spec.rb @@ -16,7 +16,7 @@ it "returns the calculated product total items" do organization = build(:organization) build(:request, :pending, organization:) - total_items_service_double = instance_double(RequestsTotalItemsService, calculate: {"Diaper" => 10}) + total_items_service_double = instance_double(RequestsTotalItemsService, calculate: {"Diaper" => 10}) allow(RequestsTotalItemsService).to receive(:new).with(requests: organization.requests).and_return(total_items_service_double) requests = View::Requests.from_params(params: {}, organization:, helpers:) From 57a830d60d820a787583b6a3830bdd9e23c3b7ba Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Thu, 21 May 2026 15:29:23 -0600 Subject: [PATCH 08/12] Add full coverage to View::RequestInfo --- spec/models/view/request_info_spec.rb | 86 ++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 8 deletions(-) diff --git a/spec/models/view/request_info_spec.rb b/spec/models/view/request_info_spec.rb index fd8a64706b..cae527a7d1 100644 --- a/spec/models/view/request_info_spec.rb +++ b/spec/models/view/request_info_spec.rb @@ -1,4 +1,45 @@ RSpec.describe View::RequestInfo do + describe "#item_requests" do + context "when the request has item requests" do + it "returns the request's item requests" do + organization = build(:organization) + item = build(:item) + item_request = build(:item_request, item: item, quantity: 5) + request = create( + :request, + organization:, + item_requests: [item_request] + ) + + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request_info.item_requests).to eq([item_request]) + end + end + + context "when the request has no item requests" do + it "returns an empty array" do + organization = build(:organization) + request = create(:request, organization:) + + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request_info.item_requests).to eq([]) + end + end + end + + describe "#inventory" do + it "returns a View::Inventory instance" do + organization = build(:organization) + request = create(:request, organization:) + + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request_info.inventory).to be_a_kind_of(View::Inventory) + end + end + describe "#default_storage_location" do context "when the request partner has a default_storage_location_id" do it "returns the request partner's default_storage_location_id" do @@ -10,9 +51,9 @@ organization: ) - request = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) - expect(request.default_storage_location).to eq(storage_location.id) + expect(request_info.default_storage_location).to eq(storage_location.id) end end @@ -26,9 +67,38 @@ organization: ) - request = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request_info.default_storage_location).to eq(storage_location.id) + end + end + end + + describe "#location" do + context "when no storage location is found" do + it "returns nil" do + organization = build(:organization) + request = create(:request, organization:) + + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + + expect(request_info.location).to be_nil + end + end + + context "when a storage location is found" do + it "returns the found location" do + organization = build(:organization) + storage_location = create(:storage_location, organization:) + organization.update!(default_storage_location: storage_location.id) + request = create( + :request, + organization: + ) + + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) - expect(request.default_storage_location).to eq(storage_location.id) + expect(request_info.location).to be_a_kind_of(StorageLocation) end end end @@ -49,9 +119,9 @@ ] ) - request = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) - expect(request.custom_units).to be_falsey + expect(request_info.custom_units).to be_falsey end end @@ -70,9 +140,9 @@ {item_id: item.id, quantity: "559", request_unit: "flat"} ] ) - request = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) - expect(request.custom_units).to be_truthy + expect(request_info.custom_units).to be_truthy Flipper.disable(:enable_packs) end From 2cfa66098844b08e288fe83ac20aea60bb8c7a6b Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Thu, 21 May 2026 16:18:08 -0600 Subject: [PATCH 09/12] Add full coverage to View::Requests --- spec/models/view/requests_spec.rb | 72 ++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/spec/models/view/requests_spec.rb b/spec/models/view/requests_spec.rb index 855af78a0b..f7bc745a41 100644 --- a/spec/models/view/requests_spec.rb +++ b/spec/models/view/requests_spec.rb @@ -15,7 +15,7 @@ describe "#calculate_product_totals" do it "returns the calculated product total items" do organization = build(:organization) - build(:request, :pending, organization:) + build(:request, organization:) total_items_service_double = instance_double(RequestsTotalItemsService, calculate: {"Diaper" => 10}) allow(RequestsTotalItemsService).to receive(:new).with(requests: organization.requests).and_return(total_items_service_double) @@ -25,10 +25,78 @@ end end + describe "#items" do + it "returns the organization's items id and name, alphabetized" do + organization = build(:organization) + build(:request, organization:) + base_item = build(:base_item) + item_two = create(:item, base_item:, organization:, name: "B item") + item_one = create(:item, base_item:, organization:, name: "A item") + + requests = View::Requests.from_params(params: {}, organization:, helpers:) + + expect(requests.items.map(&:id)).to eq([item_one.id, item_two.id]) + expect(requests.items.map(&:name)).to eq(["A item", "B item"]) + end + end + + describe "#partners" do + it "returns the organization's partners id, name and status, alphabetized" do + organization = build(:organization) + build(:request, organization:) + partner_two = create(:partner, organization:, name: "B partner", status: "approved") + partner_one = create(:partner, organization:, name: "A partner", status: "invited") + + requests = View::Requests.from_params(params: {}, organization:, helpers:) + + expect(requests.partners.map(&:id)).to eq([partner_one.id, partner_two.id]) + expect(requests.partners.map(&:name)).to eq(["A partner", "B partner"]) + expect(requests.partners.map(&:status)).to eq(["invited", "approved"]) + end + end + + describe "#statuses" do + it "returns the request statuses" do + organization = build(:organization) + build(:request, organization:) + humanized_statuses = {"Discarded" => 3, "Fulfilled" => 2, "Pending" => 0, "Started" => 1} + + requests = View::Requests.from_params(params: {}, organization:, helpers:) + + expect(requests.statuses).to eq(humanized_statuses) + end + end + + describe "#partner_users" do + it "returns the organization's partner users id, name and email" do + organization = build(:organization) + partner_user = create(:partner_user, name: "Jane Smith", email: "janesmith@example.com") + create(:request, organization:, partner_user:) + + requests = View::Requests.from_params(params: {}, organization:, helpers:) + + expect(requests.partner_users.map(&:id)).to eq([partner_user.id]) + expect(requests.partner_users.map(&:name)).to eq(["Jane Smith"]) + expect(requests.partner_users.map(&:email)).to eq(["janesmith@example.com"]) + end + end + + describe "#request_types" do + it "returns the request types" do + organization = build(:organization) + build(:request, organization:) + humanized_types = {"Child" => "child", "Individual" => "individual", "Quantity" => "quantity"} + + requests = View::Requests.from_params(params: {}, organization:, helpers:) + + expect(requests.request_types).to eq(humanized_types) + end + end + describe "selected filter params" do it "returns the given filter params" do organization = build(:organization) - build(:request, :pending, organization:) + build(:request, organization:) params = ActionController::Parameters.new( { filters: { From 257f971f64071f27ba03acc029d026649d694074 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Fri, 22 May 2026 14:05:45 -0600 Subject: [PATCH 10/12] Memoize RequestInfo to avoid unnecessary DB calls To prevent performance degradation, `View::RequestInfo` memoizes expensive operations, especially DB calls. To make that happen, the object needs to be a regular class and not a Data object, which is immutable. --- app/controllers/requests_controller.rb | 2 +- app/models/view/request_info.rb | 26 +++++++++++++------------- spec/models/view/request_info_spec.rb | 18 +++++++++--------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index bf89ccd067..813f1b2283 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -12,7 +12,7 @@ def index end def show - @request_info = View::RequestInfo.from_params(params:, organization: current_organization) + @request_info = View::RequestInfo.new(params:, organization: current_organization) end # Clicking the "New Distribution" button will set the the request to started diff --git a/app/models/view/request_info.rb b/app/models/view/request_info.rb index 9e325e232a..682a34eefd 100644 --- a/app/models/view/request_info.rb +++ b/app/models/view/request_info.rb @@ -1,29 +1,29 @@ module View - RequestInfo = Data.define( - :request - ) do - class << self - def from_params(params:, organization:) - request = organization.requests.find(params[:id]) - - new(request:) - end + class RequestInfo + attr_reader :request + + def initialize(params:, organization:) + @request = organization.requests.find(params[:id]) end def item_requests - request.item_requests.includes(:item) + @item_requests ||= @request.item_requests.includes(:item) end def inventory - View::Inventory.new(request.organization_id) + @inventory ||= View::Inventory.new(@request.organization_id) end def default_storage_location - request.partner.default_storage_location_id || request.organization.default_storage_location + return @default_storage_location if defined?(@default_storage_location) + + @efault_storage_location ||= @request.partner.default_storage_location_id || @request.organization.default_storage_location end def location - StorageLocation.find_by(id: default_storage_location) + return @location if defined?(@location) + + @location ||= StorageLocation.find_by(id: default_storage_location) end def custom_units diff --git a/spec/models/view/request_info_spec.rb b/spec/models/view/request_info_spec.rb index cae527a7d1..16ba43a575 100644 --- a/spec/models/view/request_info_spec.rb +++ b/spec/models/view/request_info_spec.rb @@ -11,7 +11,7 @@ item_requests: [item_request] ) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.item_requests).to eq([item_request]) end @@ -22,7 +22,7 @@ organization = build(:organization) request = create(:request, organization:) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.item_requests).to eq([]) end @@ -34,7 +34,7 @@ organization = build(:organization) request = create(:request, organization:) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.inventory).to be_a_kind_of(View::Inventory) end @@ -51,7 +51,7 @@ organization: ) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.default_storage_location).to eq(storage_location.id) end @@ -67,7 +67,7 @@ organization: ) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.default_storage_location).to eq(storage_location.id) end @@ -80,7 +80,7 @@ organization = build(:organization) request = create(:request, organization:) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.location).to be_nil end @@ -96,7 +96,7 @@ organization: ) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.location).to be_a_kind_of(StorageLocation) end @@ -119,7 +119,7 @@ ] ) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.custom_units).to be_falsey end @@ -140,7 +140,7 @@ {item_id: item.id, quantity: "559", request_unit: "flat"} ] ) - request_info = View::RequestInfo.from_params(params: {id: request.id}, organization:) + request_info = View::RequestInfo.new(params: {id: request.id}, organization:) expect(request_info.custom_units).to be_truthy From e7257c01700d917810c0257e3e18e5667e971fb7 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Fri, 22 May 2026 14:16:01 -0600 Subject: [PATCH 11/12] Memoize View::Requests to avoid unnecessary DB calls To prevent performance degradation, `View::Requests` memoizes expensive operations, especially DB calls. To make that happen, the object needs to be a regular class and not a Data object, which is immutable. --- app/controllers/requests_controller.rb | 2 +- app/models/view/requests.rb | 57 ++++++++++++-------------- spec/models/view/requests_spec.rb | 16 ++++---- 3 files changed, 35 insertions(+), 40 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 813f1b2283..0df37c5993 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -3,7 +3,7 @@ class RequestsController < ApplicationController def index setup_date_range_picker - @requests_info = View::Requests.from_params(params: params, organization: current_organization, helpers: helpers) + @requests_info = View::Requests.new(params: params, organization: current_organization, helpers: helpers) respond_to do |format| format.html diff --git a/app/models/view/requests.rb b/app/models/view/requests.rb index 0a9697aa05..a82153a304 100644 --- a/app/models/view/requests.rb +++ b/app/models/view/requests.rb @@ -1,39 +1,34 @@ module View - Requests = Data.define( - :requests, - :filters, - :paginated_requests, - :organization, - :helpers - ) do + class Requests include DateRangeHelper - class << self - def filter_params(params = {}) - if params.key?(:filters) - params.require(:filters).permit(:by_request_item_id, :by_partner, :by_status, :by_request_type) - else - {} - end - end + attr_reader :filters, :helpers, :organization, :paginated_requests, :params, :requests - def from_params(params:, organization:, helpers:) - filters = filter_params(params) + def initialize(params:, organization:, helpers:) + @params = params + @organization = organization + @filters = filter_params(params) + @helpers = helpers - requests = organization - .ordered_requests - .undiscarded - .during(helpers.selected_range) - .class_filter(filters) + @requests = organization + .ordered_requests + .undiscarded + .during(helpers.selected_range) + .class_filter(filters) - paginated_requests = requests.includes(:partner).page(params[:page]) + @paginated_requests = requests.includes(:partner).page(params[:page]) + end - new(requests:, filters:, paginated_requests:, organization:, helpers:) + def filter_params(params = {}) + if params.key?(:filters) + params.require(:filters).permit(:by_request_item_id, :by_partner, :by_status, :by_request_type) + else + {} end end def unfulfilled_requests_count - organization + @unfulfilled_requests_count ||= organization .requests .where(status: [:pending, :started]) .during(helpers.selected_range) @@ -42,27 +37,27 @@ def unfulfilled_requests_count end def calculate_product_totals - RequestsTotalItemsService.new(requests: requests).calculate + @calculate_product_totals ||= RequestsTotalItemsService.new(requests: requests).calculate end def items - organization.items.alphabetized.select(:id, :name) + @items ||= organization.items.alphabetized.select(:id, :name) end def partners - organization.partners.alphabetized.select(:id, :name, :status) + @partners ||= organization.partners.alphabetized.select(:id, :name, :status) end def statuses - Request.statuses.transform_keys(&:humanize) + @statuses ||= Request.statuses.transform_keys(&:humanize) end def partner_users - User.where(id: paginated_requests.map(&:partner_user_id)).select(:id, :name, :email) + @partner_users ||= User.where(id: paginated_requests.map(&:partner_user_id)).select(:id, :name, :email) end def request_types - Request.request_types.transform_keys(&:humanize) + @request_types ||= Request.request_types.transform_keys(&:humanize) end def selected_request_type diff --git a/spec/models/view/requests_spec.rb b/spec/models/view/requests_spec.rb index f7bc745a41..3798b1a5e9 100644 --- a/spec/models/view/requests_spec.rb +++ b/spec/models/view/requests_spec.rb @@ -6,7 +6,7 @@ create(:request, :started, organization:) create(:request, :fulfilled, organization:) - requests = View::Requests.from_params(params: {}, organization:, helpers:) + requests = View::Requests.new(params: {}, organization:, helpers:) expect(requests.unfulfilled_requests_count).to eq(2) end @@ -19,7 +19,7 @@ total_items_service_double = instance_double(RequestsTotalItemsService, calculate: {"Diaper" => 10}) allow(RequestsTotalItemsService).to receive(:new).with(requests: organization.requests).and_return(total_items_service_double) - requests = View::Requests.from_params(params: {}, organization:, helpers:) + requests = View::Requests.new(params: {}, organization:, helpers:) expect(requests.calculate_product_totals).to eq({"Diaper" => 10}) end @@ -33,7 +33,7 @@ item_two = create(:item, base_item:, organization:, name: "B item") item_one = create(:item, base_item:, organization:, name: "A item") - requests = View::Requests.from_params(params: {}, organization:, helpers:) + requests = View::Requests.new(params: {}, organization:, helpers:) expect(requests.items.map(&:id)).to eq([item_one.id, item_two.id]) expect(requests.items.map(&:name)).to eq(["A item", "B item"]) @@ -47,7 +47,7 @@ partner_two = create(:partner, organization:, name: "B partner", status: "approved") partner_one = create(:partner, organization:, name: "A partner", status: "invited") - requests = View::Requests.from_params(params: {}, organization:, helpers:) + requests = View::Requests.new(params: {}, organization:, helpers:) expect(requests.partners.map(&:id)).to eq([partner_one.id, partner_two.id]) expect(requests.partners.map(&:name)).to eq(["A partner", "B partner"]) @@ -61,7 +61,7 @@ build(:request, organization:) humanized_statuses = {"Discarded" => 3, "Fulfilled" => 2, "Pending" => 0, "Started" => 1} - requests = View::Requests.from_params(params: {}, organization:, helpers:) + requests = View::Requests.new(params: {}, organization:, helpers:) expect(requests.statuses).to eq(humanized_statuses) end @@ -73,7 +73,7 @@ partner_user = create(:partner_user, name: "Jane Smith", email: "janesmith@example.com") create(:request, organization:, partner_user:) - requests = View::Requests.from_params(params: {}, organization:, helpers:) + requests = View::Requests.new(params: {}, organization:, helpers:) expect(requests.partner_users.map(&:id)).to eq([partner_user.id]) expect(requests.partner_users.map(&:name)).to eq(["Jane Smith"]) @@ -87,7 +87,7 @@ build(:request, organization:) humanized_types = {"Child" => "child", "Individual" => "individual", "Quantity" => "quantity"} - requests = View::Requests.from_params(params: {}, organization:, helpers:) + requests = View::Requests.new(params: {}, organization:, helpers:) expect(requests.request_types).to eq(humanized_types) end @@ -108,7 +108,7 @@ } ).permit! - requests = View::Requests.from_params(params:, organization:, helpers:) + requests = View::Requests.new(params:, organization:, helpers:) expect(requests.selected_request_type).to eq("quantity") expect(requests.selected_request_item).to eq("1") From bb5133299847707903f1b238a96b283bc27ab9fe Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Fri, 22 May 2026 14:24:31 -0600 Subject: [PATCH 12/12] Trigger Build