From edc66cd78e35286ffefe880dbc725868901823f9 Mon Sep 17 00:00:00 2001 From: WJWB Date: Mon, 24 Mar 2025 23:41:29 +0000 Subject: [PATCH 1/4] Tests for date range input pages --- spec/requests/adjustments_requests_spec.rb | 10 ++++++++++ spec/requests/distributions_requests_spec.rb | 9 +++++++++ spec/requests/donations_requests_spec.rb | 9 +++++++++ spec/requests/events_requests_spec.rb | 10 ++++++++++ spec/requests/product_drives_requests_spec.rb | 9 +++++++++ spec/requests/purchases_requests_spec.rb | 9 +++++++++ spec/requests/requests_requests_spec.rb | 9 +++++++++ spec/requests/transfers_requests_spec.rb | 9 +++++++++ 8 files changed, 74 insertions(+) diff --git a/spec/requests/adjustments_requests_spec.rb b/spec/requests/adjustments_requests_spec.rb index a3dd85e91b..d5da94abf0 100644 --- a/spec/requests/adjustments_requests_spec.rb +++ b/spec/requests/adjustments_requests_spec.rb @@ -68,6 +68,16 @@ end end end + + it "falls back to default date range and renders without error when given an invalid date_range param" do + get adjustments_path(format: :html, params: { + filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + }) + + expect(response).to be_successful + expect(response.body).to include("The date range you supplied was invalid") + end + end context "csv" do diff --git a/spec/requests/distributions_requests_spec.rb b/spec/requests/distributions_requests_spec.rb index 8487697d16..da940f32fc 100644 --- a/spec/requests/distributions_requests_spec.rb +++ b/spec/requests/distributions_requests_spec.rb @@ -75,6 +75,15 @@ expect(response).to be_successful end + it "falls back to default date range and renders without error when given an invalid date_range param" do + get distributions_path(format: :html, params: { + filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + }) + + expect(response).to be_successful + expect(response.body).to include("The date range you supplied was invalid") + end + it "sums distribution totals accurately" do create(:distribution, :with_items, item_quantity: 5, organization: organization) create(:line_item, :distribution, itemizable_id: distribution.id, quantity: 7) diff --git a/spec/requests/donations_requests_spec.rb b/spec/requests/donations_requests_spec.rb index 39c8e98afc..96b76d212f 100644 --- a/spec/requests/donations_requests_spec.rb +++ b/spec/requests/donations_requests_spec.rb @@ -80,6 +80,15 @@ expect(subject.body).to_not include("#{full_comment}") end end + + it "falls back to default date range and renders without error when given an invalid date_range param" do + get donations_path(format: :html, params: { + filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + }) + + expect(response).to be_successful + expect(response.body).to include("The date range you supplied was invalid") + end end context "csv" do diff --git a/spec/requests/events_requests_spec.rb b/spec/requests/events_requests_spec.rb index 555d731a97..2d54efa2bf 100644 --- a/spec/requests/events_requests_spec.rb +++ b/spec/requests/events_requests_spec.rb @@ -211,6 +211,16 @@ expect(response.body).to include("88
") expect(response.body).not_to include("99
") end + + it "falls back to default date range and renders without error when given an invalid date_range param" do + get events_path(format: :html, params: { + filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + }) + + expect(response).to be_successful + expect(response.body).to include("The date range you supplied was invalid") + end + end context "with eventable_id" do diff --git a/spec/requests/product_drives_requests_spec.rb b/spec/requests/product_drives_requests_spec.rb index a31f9702ab..21537de806 100644 --- a/spec/requests/product_drives_requests_spec.rb +++ b/spec/requests/product_drives_requests_spec.rb @@ -51,6 +51,15 @@ let(:index_path) { product_drives_path(params) } end + it "falls back to default date range and renders without error when given an invalid date_range param" do + get product_drives_path(format: :html, params: { + filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + }) + + expect(response).to be_successful + expect(response.body).to include("The date range you supplied was invalid") + end + context "csv" do it 'is successful' do get product_drives_path(format: :csv) diff --git a/spec/requests/purchases_requests_spec.rb b/spec/requests/purchases_requests_spec.rb index 12b574f4c2..823541205d 100644 --- a/spec/requests/purchases_requests_spec.rb +++ b/spec/requests/purchases_requests_spec.rb @@ -38,6 +38,15 @@ expect(subject.body).to include("Purchase Comment") end + it "falls back to default date range and renders without error when given an invalid date_range param" do + get purchases_path(format: :html, params: { + filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + }) + + expect(response).to be_successful + expect(response.body).to include("The date range you supplied was invalid") + end + context "with multiple purchases" do let!(:storage_location) { create(:storage_location, organization: organization) } let(:vendor) { create(:vendor, organization: organization) } diff --git a/spec/requests/requests_requests_spec.rb b/spec/requests/requests_requests_spec.rb index 6ff8095bfc..323ebea001 100644 --- a/spec/requests/requests_requests_spec.rb +++ b/spec/requests/requests_requests_spec.rb @@ -19,6 +19,15 @@ let(:response_format) { 'html' } it { is_expected.to be_successful } + + it "falls back to default date range and renders without error when given an invalid date_range param" do + get requests_path(format: :html, params: { + filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + }) + + expect(response).to be_successful + expect(response.body).to include("The date range you supplied was invalid") + end end context "csv" do diff --git a/spec/requests/transfers_requests_spec.rb b/spec/requests/transfers_requests_spec.rb index c0f891bbb7..7c1429dd01 100644 --- a/spec/requests/transfers_requests_spec.rb +++ b/spec/requests/transfers_requests_spec.rb @@ -36,6 +36,15 @@ get transfers_path(filters: { date_range: "#{start_date} - #{end_date}" }) expect(assigns(:transfers)).to eq([new_transfer]) end + + it "falls back to default date range and renders without error when given an invalid date_range param" do + get transfers_path(format: :html, params: { + filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + }) + + expect(response).to be_successful + expect(response.body).to include("The date range you supplied was invalid") + end end context 'when date parameters are not supplied' do From 6698de23dd371a84e065208c772812758ed36395 Mon Sep 17 00:00:00 2001 From: WJWB Date: Mon, 24 Mar 2025 23:42:02 +0000 Subject: [PATCH 2/4] Fix in date_range_helper.rb and application_controller.rb for invalid filter date_range param --- app/controllers/application_controller.rb | 32 +++++++++++++---------- app/helpers/date_range_helper.rb | 20 ++++++++++---- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8c25cff1d1..9fe05722d1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,12 +24,12 @@ def current_organization return @current_organization if instance_variable_defined? :@current_organization @current_organization = if !current_role - nil - elsif current_role.resource.is_a?(Organization) - current_role.resource - else - Organization.find_by(short_name: params[:organization_name]) - end + nil + elsif current_role.resource.is_a?(Organization) + current_role.resource + else + Organization.find_by(short_name: params[:organization_name]) + end end helper_method :current_organization @@ -37,10 +37,10 @@ def current_partner return @current_partner if instance_variable_defined? :@current_partner @current_partner = if !current_role || current_role.name.to_sym != Role::PARTNER - nil - else - current_role.resource - end + nil + else + current_role.resource + end end helper_method :current_partner @@ -48,10 +48,10 @@ def current_role return @role if instance_variable_defined? :@role @role = if !current_user - nil - else - Role.find_by(id: session[:current_role]) || UsersRole.current_role_for(current_user) - end + nil + else + Role.find_by(id: session[:current_role]) || UsersRole.current_role_for(current_user) + end end def dashboard_path_from_current_role @@ -142,6 +142,10 @@ def setup_date_range_picker @selected_date_interval = helpers.selected_interval @selected_date_range = helpers.selected_interval.map { |d| d.to_fs(:long) }.join(" - ") @selected_date_range_label = helpers.date_range_label + + if helpers.date_range_params_invalid? + flash.now[:error] = "The date range you supplied was invalid, so we used a default range instead." + end end def configure_permitted_parameters diff --git a/app/helpers/date_range_helper.rb b/app/helpers/date_range_helper.rb index cc73af80d8..843cfc305f 100644 --- a/app/helpers/date_range_helper.rb +++ b/app/helpers/date_range_helper.rb @@ -33,12 +33,18 @@ def default_date "#{start_date.strftime("%B %d, %Y")} - #{end_date.strftime("%B %d, %Y")}" end + def parse_date_range(date_range_string) + parts = date_range_string.to_s.split(" - ") + return nil unless parts.size == 2 + + parts.map { |d| Date.strptime(d.strip, "%B %d, %Y") } + rescue ArgumentError => e + Rails.logger.warn("Invalid date range '#{date_range_string}': #{e.message}") + nil + end + def selected_interval - date_range_params.split(" - ").map do |d| - Date.strptime(d, "%B %d, %Y") - rescue - raise "Invalid date: #{d} in #{date_range_params}" - end + parse_date_range(date_range_params) || parse_date_range(default_date) end def selected_range @@ -56,4 +62,8 @@ def selected_range_described "during the period #{start_date.to_fs(:short)} to #{end_date.to_fs(:short)}" end end + + def date_range_params_invalid? + parse_date_range(date_range_params).nil? + end end From 2221097053e5f38a8ea2bf4b3fcd50766512a020 Mon Sep 17 00:00:00 2001 From: WJWB Date: Mon, 24 Mar 2025 23:49:36 +0000 Subject: [PATCH 3/4] Fix Indenting --- app/controllers/application_controller.rb | 28 +++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9fe05722d1..c26c5657b7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,12 +24,12 @@ def current_organization return @current_organization if instance_variable_defined? :@current_organization @current_organization = if !current_role - nil - elsif current_role.resource.is_a?(Organization) - current_role.resource - else - Organization.find_by(short_name: params[:organization_name]) - end + nil + elsif current_role.resource.is_a?(Organization) + current_role.resource + else + Organization.find_by(short_name: params[:organization_name]) + end end helper_method :current_organization @@ -37,10 +37,10 @@ def current_partner return @current_partner if instance_variable_defined? :@current_partner @current_partner = if !current_role || current_role.name.to_sym != Role::PARTNER - nil - else - current_role.resource - end + nil + else + current_role.resource + end end helper_method :current_partner @@ -48,10 +48,10 @@ def current_role return @role if instance_variable_defined? :@role @role = if !current_user - nil - else - Role.find_by(id: session[:current_role]) || UsersRole.current_role_for(current_user) - end + nil + else + Role.find_by(id: session[:current_role]) || UsersRole.current_role_for(current_user) + end end def dashboard_path_from_current_role From b7e246b53fcc6bd576ef92e9b6bee869eceaec67 Mon Sep 17 00:00:00 2001 From: WJWB Date: Tue, 25 Mar 2025 00:05:50 +0000 Subject: [PATCH 4/4] Fix Linting Errors --- spec/requests/adjustments_requests_spec.rb | 1 - spec/requests/events_requests_spec.rb | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/requests/adjustments_requests_spec.rb b/spec/requests/adjustments_requests_spec.rb index d5da94abf0..f238db82e1 100644 --- a/spec/requests/adjustments_requests_spec.rb +++ b/spec/requests/adjustments_requests_spec.rb @@ -77,7 +77,6 @@ expect(response).to be_successful expect(response.body).to include("The date range you supplied was invalid") end - end context "csv" do diff --git a/spec/requests/events_requests_spec.rb b/spec/requests/events_requests_spec.rb index 2d54efa2bf..228b061d30 100644 --- a/spec/requests/events_requests_spec.rb +++ b/spec/requests/events_requests_spec.rb @@ -214,13 +214,12 @@ it "falls back to default date range and renders without error when given an invalid date_range param" do get events_path(format: :html, params: { - filters: { date_range: "Foo 10, 2025 - Bar 20, 2025" } + filters: {date_range: "Foo 10, 2025 - Bar 20, 2025"} }) expect(response).to be_successful expect(response.body).to include("The date range you supplied was invalid") end - end context "with eventable_id" do