From b07fa59a5ef81d09ca93888fd19425494acabf15 Mon Sep 17 00:00:00 2001 From: Riley Seaburg Date: Mon, 22 Dec 2025 15:26:51 +0000 Subject: [PATCH 1/2] Address PR #1925 feedback 1. Fix element_selector handling in template_renderer.rs - Added null check for element_selector before calling getElementById - Prevents potential JS errors with empty/null selectors 2. Remove unnecessary cargo clean from extconf.rb - Removing cached artifacts significantly increases build time - Clean CI environment doesn't need cargo clean before build 3. Extract mock request setup to helper method in form.rb - Created build_controller_with_mock_request private method - Reduces code duplication between touchpoints_js_string and render_widget_css - Improves maintainability --- app/models/form.rb | 70 ++++++++------------ ext/widget_renderer/extconf.rb | 2 - ext/widget_renderer/src/template_renderer.rs | 3 +- 3 files changed, 31 insertions(+), 44 deletions(-) diff --git a/app/models/form.rb b/app/models/form.rb index edffcb94e..16d0bb40f 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -358,51 +358,14 @@ def touchpoints_js_string end # Always use ERB template rendering for now to avoid Rust compilation issues - controller = ApplicationController.new - - # Set up a mock request with default URL options to avoid "undefined method 'host' for nil" errors - # This is necessary because the ERB templates use root_url which requires request context - # Try action_controller first, fall back to action_mailer if not set - default_options = Rails.application.config.action_controller.default_url_options || - Rails.application.config.action_mailer.default_url_options || - {} - host = default_options[:host] || 'localhost' - port = default_options[:port] || 3000 - protocol = default_options[:protocol] || (port == 443 ? 'https' : 'http') - - # Create a mock request - mock_request = ActionDispatch::Request.new( - 'rack.url_scheme' => protocol, - 'HTTP_HOST' => "#{host}#{":#{port}" if port != 80 && port != 443}", - 'SERVER_NAME' => host, - 'SERVER_PORT' => port.to_s, - ) - - controller.request = mock_request - controller.render_to_string(partial: 'components/widget/fba', formats: :js, locals: { form: self }) + controller_with_request = build_controller_with_mock_request + controller_with_request.render_to_string(partial: 'components/widget/fba', formats: :js, locals: { form: self }) end # Renders the widget CSS partial for use with the Rust widget renderer def render_widget_css - controller = ApplicationController.new - - # Set up a mock request with default URL options - default_options = Rails.application.config.action_controller.default_url_options || - Rails.application.config.action_mailer.default_url_options || - {} - host = default_options[:host] || 'localhost' - port = default_options[:port] || 3000 - protocol = default_options[:protocol] || (port == 443 ? 'https' : 'http') - - mock_request = ActionDispatch::Request.new( - 'rack.url_scheme' => protocol, - 'HTTP_HOST' => "#{host}#{":#{port}" if port != 80 && port != 443}", - 'SERVER_NAME' => host, - 'SERVER_PORT' => port.to_s, - ) - - controller.request = mock_request - controller.render_to_string(partial: 'components/widget/widget', formats: :css, locals: { form: self }) + controller_with_request = build_controller_with_mock_request + controller_with_request.render_to_string(partial: 'components/widget/widget', formats: :css, locals: { form: self }) end def reportable_submissions(start_date: nil, end_date: nil) @@ -1076,6 +1039,31 @@ def self.forms_whose_retention_period_has_passed private + # Builds an ApplicationController instance with a mock request for rendering partials + # This is necessary because ERB templates use URL helpers which require request context + def build_controller_with_mock_request + controller = ApplicationController.new + + # Set up a mock request with default URL options + # Try action_controller first, fall back to action_mailer if not set + default_options = Rails.application.config.action_controller.default_url_options || + Rails.application.config.action_mailer.default_url_options || + {} + host = default_options[:host] || 'localhost' + port = default_options[:port] || 3000 + protocol = default_options[:protocol] || (port == 443 ? 'https' : 'http') + + mock_request = ActionDispatch::Request.new( + 'rack.url_scheme' => protocol, + 'HTTP_HOST' => "#{host}#{":#{port}" if port != 80 && port != 443}", + 'SERVER_NAME' => host, + 'SERVER_PORT' => port.to_s, + ) + + controller.request = mock_request + controller + end + def set_uuid self.uuid ||= SecureRandom.uuid self.short_uuid ||= self.uuid[0..7] diff --git a/ext/widget_renderer/extconf.rb b/ext/widget_renderer/extconf.rb index ab434ccdc..6b8fde0a5 100644 --- a/ext/widget_renderer/extconf.rb +++ b/ext/widget_renderer/extconf.rb @@ -30,8 +30,6 @@ def ensure_rust puts "Current directory: #{Dir.pwd}" puts "Using cargo executable: #{cargo_bin}" -puts "Cleaning previous build artifacts..." -system("#{cargo_bin} clean 2>&1") puts "Running cargo build --release..." system("#{cargo_bin} build --release 2>&1") or abort 'Failed to build Rust extension' diff --git a/ext/widget_renderer/src/template_renderer.rs b/ext/widget_renderer/src/template_renderer.rs index 80f90db77..4e5004a3b 100644 --- a/ext/widget_renderer/src/template_renderer.rs +++ b/ext/widget_renderer/src/template_renderer.rs @@ -965,7 +965,8 @@ window.touchpointForm{uuid}.init(touchpointFormOptions{uuid}); }} }} // Ensure the custom button is also initialized if it exists (for 'custom-button-modal' delivery method) - const customButtonEl = document.getElementById('{element_selector}'); + const customButtonSelector = '{element_selector}'; + const customButtonEl = customButtonSelector ? document.getElementById(customButtonSelector) : null; if (customButtonEl && ('{delivery_method}' === 'custom-button-modal')) {{ if (fbaUswds.Modal) {{ fbaUswds.Modal.on(customButtonEl); From f6b55552cb6169ebe6018784a06e32447b57d9e5 Mon Sep 17 00:00:00 2001 From: Riley Seaburg Date: Mon, 22 Dec 2025 15:35:51 +0000 Subject: [PATCH 2/2] Fix empty string check for customButtonSelector Empty string '' is truthy in JavaScript when interpolated from Rust. Use length check to properly handle empty element_selector values. --- ext/widget_renderer/src/template_renderer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/widget_renderer/src/template_renderer.rs b/ext/widget_renderer/src/template_renderer.rs index 4e5004a3b..736df31aa 100644 --- a/ext/widget_renderer/src/template_renderer.rs +++ b/ext/widget_renderer/src/template_renderer.rs @@ -966,7 +966,7 @@ window.touchpointForm{uuid}.init(touchpointFormOptions{uuid}); }} // Ensure the custom button is also initialized if it exists (for 'custom-button-modal' delivery method) const customButtonSelector = '{element_selector}'; - const customButtonEl = customButtonSelector ? document.getElementById(customButtonSelector) : null; + const customButtonEl = (customButtonSelector && customButtonSelector.length > 0) ? document.getElementById(customButtonSelector) : null; if (customButtonEl && ('{delivery_method}' === 'custom-button-modal')) {{ if (fbaUswds.Modal) {{ fbaUswds.Modal.on(customButtonEl);