diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 389e9591..181bacd5 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class SubmissionsController < ApplicationController + include PrefillFieldsHelper + skip_before_action :verify_authenticity_token before_action :load_template, only: %i[new create] authorize_resource :template, only: %i[new create] @@ -15,6 +17,10 @@ class SubmissionsController < ApplicationController def show @submission = Submissions.preload_with_pages(@submission) + @available_ats_fields = extract_ats_prefill_fields + + # Optional: store in session for persistence across requests + session[:ats_prefill_fields] = @available_ats_fields if @available_ats_fields.any? unless @submission.submitters.all?(&:completed_at?) ActiveRecord::Associations::Preloader.new( diff --git a/app/controllers/templates_controller.rb b/app/controllers/templates_controller.rb index 2828ce58..2c77ed73 100644 --- a/app/controllers/templates_controller.rb +++ b/app/controllers/templates_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class TemplatesController < ApplicationController + include PrefillFieldsHelper + skip_before_action :maybe_redirect_to_setup skip_before_action :verify_authenticity_token @@ -39,6 +41,9 @@ class TemplatesController < ApplicationController associations: [schema_documents: [:blob, { preview_images_attachments: :blob }]] ).call + # Process ATS fields for template editing + @available_ats_fields = extract_ats_prefill_fields + @template_data = @template.as_json.merge( documents: @template.schema_documents.as_json( diff --git a/app/helpers/prefill_fields_helper.rb b/app/helpers/prefill_fields_helper.rb new file mode 100644 index 00000000..260c3902 --- /dev/null +++ b/app/helpers/prefill_fields_helper.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +module PrefillFieldsHelper + # Cache TTL for ATS field parsing (1 hour) + ATS_FIELDS_CACHE_TTL = 1.hour + + # Maximum number of cached entries to prevent memory bloat + MAX_CACHE_ENTRIES = 1000 + + def extract_ats_prefill_fields + return [] if params[:ats_fields].blank? + + # Create cache key from parameter hash for security and uniqueness + cache_key = ats_fields_cache_key(params[:ats_fields]) + + # Try to get from cache first with error handling + begin + cached_result = Rails.cache.read(cache_key) + if cached_result + Rails.logger.debug { "ATS fields cache hit for key: #{cache_key}" } + return cached_result + end + rescue StandardError => e + Rails.logger.warn "Cache read failed for ATS fields: #{e.message}" + # Continue with normal processing if cache read fails + end + + # Cache miss - perform expensive operations + Rails.logger.debug { "ATS fields cache miss for key: #{cache_key}" } + + begin + decoded_json = Base64.urlsafe_decode64(params[:ats_fields]) + field_names = JSON.parse(decoded_json) + + # Validate that we got an array of strings + return cache_and_return_empty(cache_key) unless field_names.is_a?(Array) && field_names.all?(String) + + # Filter to only expected field name patterns + valid_fields = field_names.select { |name| valid_ats_field_name?(name) } + + # Cache the result with TTL (with error handling) + cache_result(cache_key, valid_fields, ATS_FIELDS_CACHE_TTL) + + # Log successful field reception + Rails.logger.info "Processed and cached #{valid_fields.length} ATS prefill fields: #{valid_fields.join(', ')}" + + valid_fields + rescue StandardError => e + Rails.logger.warn "Failed to parse ATS prefill fields: #{e.message}" + # Cache empty result for failed parsing to avoid repeated failures + cache_result(cache_key, [], 5.minutes) + [] + end + end + + # Clear ATS fields cache (useful for testing or manual cache invalidation) + def clear_ats_fields_cache + # Since we can't easily enumerate cache keys, we'll rely on TTL for cleanup + # This method is provided for potential future use or testing + Rails.logger.info 'ATS fields cache clear requested (relies on TTL for cleanup)' + end + + private + + def valid_ats_field_name?(name) + # Only allow expected field name patterns (security) + name.match?(/\A(employee|manager|account|location)_[a-z_]+\z/) + end + + def ats_fields_cache_key(ats_fields_param) + # Create secure cache key using SHA256 hash of the parameter + # This prevents cache key collisions and keeps keys reasonably sized + hash = Digest::SHA256.hexdigest(ats_fields_param) + "ats_fields:#{hash}" + end + + def cache_result(cache_key, value, ttl) + Rails.cache.write(cache_key, value, expires_in: ttl) + rescue StandardError => e + Rails.logger.warn "Cache write failed for ATS fields: #{e.message}" + # Continue execution even if caching fails + end + + def cache_and_return_empty(cache_key) + cache_result(cache_key, [], 5.minutes) + [] + end +end diff --git a/app/javascript/template_builder/builder.vue b/app/javascript/template_builder/builder.vue index d60d9e5d..b05f1d67 100644 --- a/app/javascript/template_builder/builder.vue +++ b/app/javascript/template_builder/builder.vue @@ -55,7 +55,6 @@ id="title_container" class="flex justify-between py-1.5 items-center pr-4 top-0 z-10 title-container" :class="{ sticky: withStickySubmitters || isBreakpointLg }" - :style="{ backgroundColor }" >
diff --git a/db/migrate/20230726062428_add_template_fields_to_submission.rb b/db/migrate/20230726062428_add_template_fields_to_submission.rb index 456c5e16..9ae50aa8 100644 --- a/db/migrate/20230726062428_add_template_fields_to_submission.rb +++ b/db/migrate/20230726062428_add_template_fields_to_submission.rb @@ -10,9 +10,11 @@ class AddTemplateFieldsToSubmission < ActiveRecord::Migration[7.0] end def up - add_column :submissions, :template_fields, :text - add_column :submissions, :template_schema, :text - add_column :submissions, :template_submitters, :text + change_table :submissions, bulk: true do |t| + t.text :template_fields + t.text :template_schema + t.text :template_submitters + end MigrationTemplate.all.each do |template| MigrationSubmission.where(template_id: template.id).each do |submission| @@ -24,8 +26,10 @@ class AddTemplateFieldsToSubmission < ActiveRecord::Migration[7.0] end def down - remove_column :submissions, :template_fields - remove_column :submissions, :template_schema - remove_column :submissions, :template_submitters + change_table :submissions, bulk: true do |t| + t.remove :template_fields + t.remove :template_schema + t.remove :template_submitters + end end end diff --git a/db/migrate/20230819113427_remove_user_first_last_name_not_null.rb b/db/migrate/20230819113427_remove_user_first_last_name_not_null.rb index a848a69f..5e4348fb 100644 --- a/db/migrate/20230819113427_remove_user_first_last_name_not_null.rb +++ b/db/migrate/20230819113427_remove_user_first_last_name_not_null.rb @@ -2,7 +2,9 @@ class RemoveUserFirstLastNameNotNull < ActiveRecord::Migration[7.0] def change - change_column_null :users, :first_name, true - change_column_null :users, :last_name, true + change_table :users, bulk: true do |t| + t.change_null :first_name, true + t.change_null :last_name, true + end end end diff --git a/db/migrate/20230902171216_add_phone_and_name_to_submitters.rb b/db/migrate/20230902171216_add_phone_and_name_to_submitters.rb index 60a14000..9421918e 100644 --- a/db/migrate/20230902171216_add_phone_and_name_to_submitters.rb +++ b/db/migrate/20230902171216_add_phone_and_name_to_submitters.rb @@ -2,9 +2,10 @@ class AddPhoneAndNameToSubmitters < ActiveRecord::Migration[7.0] def change - add_column :submitters, :name, :string - add_column :submitters, :phone, :string - - change_column_null :submitters, :email, true + change_table :submitters, bulk: true do |t| + t.string :name + t.string :phone + t.change_null :email, true + end end end diff --git a/db/migrate/20230915200635_add_devise_two_factor_to_users.rb b/db/migrate/20230915200635_add_devise_two_factor_to_users.rb index b041dc43..f22f28be 100644 --- a/db/migrate/20230915200635_add_devise_two_factor_to_users.rb +++ b/db/migrate/20230915200635_add_devise_two_factor_to_users.rb @@ -2,8 +2,10 @@ class AddDeviseTwoFactorToUsers < ActiveRecord::Migration[7.0] def change - add_column :users, :otp_secret, :string - add_column :users, :consumed_timestep, :integer - add_column :users, :otp_required_for_login, :boolean, default: false, null: false + change_table :users, bulk: true do |t| + t.string :otp_secret + t.integer :consumed_timestep + t.boolean :otp_required_for_login, default: false, null: false + end end end diff --git a/db/migrate/20250523121121_add_shared_link_to_templates.rb b/db/migrate/20250523121121_add_shared_link_to_templates.rb index 6fe962eb..27e7cccb 100644 --- a/db/migrate/20250523121121_add_shared_link_to_templates.rb +++ b/db/migrate/20250523121121_add_shared_link_to_templates.rb @@ -8,12 +8,14 @@ class AddSharedLinkToTemplates < ActiveRecord::Migration[8.0] end def up - add_column :templates, :shared_link, :boolean, if_not_exists: true + add_column :templates, :shared_link, :boolean, default: false, null: false, if_not_exists: true MigrationTemplate.where(shared_link: nil).in_batches.update_all(shared_link: true) - change_column_default :templates, :shared_link, from: nil, to: false - change_column_null :templates, :shared_link, false + change_table :templates, bulk: true do |t| + t.change_default :shared_link, from: nil, to: false + t.change_null :shared_link, false + end end def down diff --git a/spec/helpers/prefill_fields_helper_spec.rb b/spec/helpers/prefill_fields_helper_spec.rb new file mode 100644 index 00000000..14d79232 --- /dev/null +++ b/spec/helpers/prefill_fields_helper_spec.rb @@ -0,0 +1,350 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe PrefillFieldsHelper, type: :helper do + # Clear cache before each test to ensure clean state + before do + Rails.cache.clear + end + + describe '#extract_ats_prefill_fields' do + it 'extracts valid field names from base64 encoded parameter' do + fields = %w[employee_first_name employee_email manager_firstname] + encoded = Base64.urlsafe_encode64(fields.to_json) + + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + + result = helper.extract_ats_prefill_fields + expect(result).to eq(fields) + end + + it 'returns empty array for invalid base64' do + allow(helper).to receive(:params).and_return({ ats_fields: 'invalid_base64' }) + + result = helper.extract_ats_prefill_fields + expect(result).to eq([]) + end + + it 'returns empty array for invalid JSON' do + invalid_json = Base64.urlsafe_encode64('invalid json') + allow(helper).to receive(:params).and_return({ ats_fields: invalid_json }) + + result = helper.extract_ats_prefill_fields + expect(result).to eq([]) + end + + it 'filters out invalid field names' do + fields = %w[employee_first_name malicious_field account_name invalid-field] + encoded = Base64.urlsafe_encode64(fields.to_json) + + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + + result = helper.extract_ats_prefill_fields + expect(result).to eq(%w[employee_first_name account_name]) + end + + it 'returns empty array when no ats_fields parameter' do + allow(helper).to receive(:params).and_return({}) + + result = helper.extract_ats_prefill_fields + expect(result).to eq([]) + end + + it 'returns empty array when ats_fields parameter is empty' do + allow(helper).to receive(:params).and_return({ ats_fields: '' }) + + result = helper.extract_ats_prefill_fields + expect(result).to eq([]) + end + + it 'returns empty array when decoded JSON is not an array' do + not_array = Base64.urlsafe_encode64({ field: 'employee_name' }.to_json) + allow(helper).to receive(:params).and_return({ ats_fields: not_array }) + + result = helper.extract_ats_prefill_fields + expect(result).to eq([]) + end + + it 'returns empty array when array contains non-string values' do + mixed_array = ['employee_first_name', 123, 'manager_firstname'] + encoded = Base64.urlsafe_encode64(mixed_array.to_json) + + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + + result = helper.extract_ats_prefill_fields + expect(result).to eq([]) + end + + it 'accepts all valid field name patterns' do + fields = %w[ + employee_first_name + employee_middle_name + employee_last_name + employee_email + manager_firstname + manager_lastname + account_name + location_name + location_street + ] + encoded = Base64.urlsafe_encode64(fields.to_json) + + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + + result = helper.extract_ats_prefill_fields + expect(result).to eq(fields) + end + + it 'logs successful field reception on cache miss' do + fields = %w[employee_first_name employee_email] + encoded = Base64.urlsafe_encode64(fields.to_json) + + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:debug) + + helper.extract_ats_prefill_fields + + expect(Rails.logger).to have_received(:info).with( + 'Processed and cached 2 ATS prefill fields: employee_first_name, employee_email' + ) + end + + it 'logs parsing errors and caches empty result' do + allow(helper).to receive(:params).and_return({ ats_fields: 'invalid_base64' }) + allow(Rails.logger).to receive(:warn) + allow(Rails.logger).to receive(:debug) + + result = helper.extract_ats_prefill_fields + expect(result).to eq([]) + + expect(Rails.logger).to have_received(:warn).with( + a_string_matching(/Failed to parse ATS prefill fields:/) + ) + end + + # Caching-specific tests + describe 'caching behavior' do + let(:fields) { %w[employee_first_name employee_email manager_firstname] } + let(:encoded) { Base64.urlsafe_encode64(fields.to_json) } + + # Use memory store for caching tests since test environment uses null_store + around do |example| + original_cache = Rails.cache + Rails.cache = ActiveSupport::Cache::MemoryStore.new + example.run + Rails.cache = original_cache + end + + it 'caches successful parsing results' do + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:debug) + + # First call should parse and cache + result1 = helper.extract_ats_prefill_fields + expect(result1).to eq(fields) + + # Verify cache write occurred + cache_key = helper.send(:ats_fields_cache_key, encoded) + cached_value = Rails.cache.read(cache_key) + expect(cached_value).to eq(fields) + end + + it 'returns cached results on subsequent calls' do + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:debug) + + # First call - cache miss + result1 = helper.extract_ats_prefill_fields + expect(result1).to eq(fields) + + # Verify cache miss was logged + expect(Rails.logger).to have_received(:debug).at_least(:once) do |&block| + block&.call&.include?('cache miss') + end + + # Reset logger expectations + allow(Rails.logger).to receive(:debug) + + # Second call - should be cache hit + result2 = helper.extract_ats_prefill_fields + expect(result2).to eq(fields) + + # Verify cache hit was logged + expect(Rails.logger).to have_received(:debug).at_least(:once) do |&block| + block&.call&.include?('cache hit') + end + end + + it 'caches empty results for parsing errors' do + allow(helper).to receive(:params).and_return({ ats_fields: 'invalid_base64' }) + allow(Rails.logger).to receive(:warn) + allow(Rails.logger).to receive(:debug) + + # First call should fail and cache empty result + result1 = helper.extract_ats_prefill_fields + expect(result1).to eq([]) + + # Verify empty result is cached + cache_key = helper.send(:ats_fields_cache_key, 'invalid_base64') + cached_value = Rails.cache.read(cache_key) + expect(cached_value).to eq([]) + + # Reset logger expectations + allow(Rails.logger).to receive(:debug) + + # Second call should return cached empty result + result2 = helper.extract_ats_prefill_fields + expect(result2).to eq([]) + + # Verify cache hit was logged + expect(Rails.logger).to have_received(:debug).at_least(:once) do |&block| + block&.call&.include?('cache hit') + end + end + + it 'generates consistent cache keys for same input' do + key1 = helper.send(:ats_fields_cache_key, encoded) + key2 = helper.send(:ats_fields_cache_key, encoded) + + expect(key1).to eq(key2) + expect(key1).to start_with('ats_fields:') + expect(key1.length).to be > 20 # Should be a reasonable hash length + end + + it 'generates different cache keys for different inputs' do + fields2 = %w[manager_lastname location_name] + encoded2 = Base64.urlsafe_encode64(fields2.to_json) + + key1 = helper.send(:ats_fields_cache_key, encoded) + key2 = helper.send(:ats_fields_cache_key, encoded2) + + expect(key1).not_to eq(key2) + end + + it 'respects cache TTL for successful results' do + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + allow(Rails.cache).to receive(:write).and_call_original + + helper.extract_ats_prefill_fields + + expect(Rails.cache).to have_received(:write).with( + anything, + fields, + expires_in: PrefillFieldsHelper::ATS_FIELDS_CACHE_TTL + ) + end + + it 'uses shorter TTL for error results' do + allow(helper).to receive(:params).and_return({ ats_fields: 'invalid_base64' }) + allow(Rails.cache).to receive(:write).and_call_original + allow(Rails.logger).to receive(:warn) + + helper.extract_ats_prefill_fields + + expect(Rails.cache).to have_received(:write).with( + anything, + [], + expires_in: 5.minutes + ) + end + + it 'handles cache read failures gracefully' do + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + allow(Rails.cache).to receive(:read).and_raise(StandardError.new('Cache error')) + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:debug) + allow(Rails.logger).to receive(:warn) + + # Should fall back to normal processing + result = helper.extract_ats_prefill_fields + expect(result).to eq(fields) + expect(Rails.logger).to have_received(:warn).with('Cache read failed for ATS fields: Cache error') + end + + it 'handles cache write failures gracefully' do + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + allow(Rails.cache).to receive(:write).and_raise(StandardError.new('Cache error')) + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:debug) + allow(Rails.logger).to receive(:warn) + + # Should still return correct result even if caching fails + result = helper.extract_ats_prefill_fields + expect(result).to eq(fields) + expect(Rails.logger).to have_received(:warn).with('Cache write failed for ATS fields: Cache error') + end + end + + describe 'performance characteristics' do + let(:fields) { %w[employee_first_name employee_email manager_firstname] } + let(:encoded) { Base64.urlsafe_encode64(fields.to_json) } + + # Use memory store for performance tests since test environment uses null_store + around do |example| + original_cache = Rails.cache + Rails.cache = ActiveSupport::Cache::MemoryStore.new + example.run + Rails.cache = original_cache + end + + it 'avoids expensive operations on cache hits' do + allow(helper).to receive(:params).and_return({ ats_fields: encoded }) + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:debug) + + # First call to populate cache + helper.extract_ats_prefill_fields + + # Mock expensive operations to verify they're not called on cache hit + allow(Base64).to receive(:urlsafe_decode64).and_call_original + allow(JSON).to receive(:parse).and_call_original + + # Second call should use cache + result = helper.extract_ats_prefill_fields + expect(result).to eq(fields) + + # Verify expensive operations were not called on second call + expect(Base64).not_to have_received(:urlsafe_decode64) + expect(JSON).not_to have_received(:parse) + end + end + end + + describe '#valid_ats_field_name?' do + it 'returns true for valid employee field names' do + expect(helper.send(:valid_ats_field_name?, 'employee_first_name')).to be true + expect(helper.send(:valid_ats_field_name?, 'employee_email')).to be true + expect(helper.send(:valid_ats_field_name?, 'employee_phone_number')).to be true + end + + it 'returns true for valid manager field names' do + expect(helper.send(:valid_ats_field_name?, 'manager_firstname')).to be true + expect(helper.send(:valid_ats_field_name?, 'manager_lastname')).to be true + expect(helper.send(:valid_ats_field_name?, 'manager_email')).to be true + end + + it 'returns true for valid account field names' do + expect(helper.send(:valid_ats_field_name?, 'account_name')).to be true + expect(helper.send(:valid_ats_field_name?, 'account_id')).to be true + end + + it 'returns true for valid location field names' do + expect(helper.send(:valid_ats_field_name?, 'location_name')).to be true + expect(helper.send(:valid_ats_field_name?, 'location_street')).to be true + expect(helper.send(:valid_ats_field_name?, 'location_city')).to be true + end + + it 'returns false for invalid field names' do + expect(helper.send(:valid_ats_field_name?, 'malicious_field')).to be false + expect(helper.send(:valid_ats_field_name?, 'invalid-field')).to be false + expect(helper.send(:valid_ats_field_name?, 'EMPLOYEE_NAME')).to be false + expect(helper.send(:valid_ats_field_name?, 'employee')).to be false + expect(helper.send(:valid_ats_field_name?, 'employee_')).to be false + expect(helper.send(:valid_ats_field_name?, '_employee_name')).to be false + end + end +end