diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index b447dd6b50f..e1232572336 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -33,6 +33,11 @@ production database. See #2127 for an example when a migration caused deployment issues. In that case, all the migration did was add a new column and an index to the Users table, which might seem innocuous. +- [ ] When fetching a single record from the database, `#take` is used instead +of `#first` unless there is an `#order` call on the ActiveRecord relations. +This prevents ActiveRecord from sorting by primary key which can result in slow +queries. + ### Encryption - [ ] The changes are compatible with data that was encrypted with the old code. diff --git a/app/controllers/account_reset/request_controller.rb b/app/controllers/account_reset/request_controller.rb index 106533d5673..baa7430e3ee 100644 --- a/app/controllers/account_reset/request_controller.rb +++ b/app/controllers/account_reset/request_controller.rb @@ -12,7 +12,7 @@ def show def create analytics.track_event(Analytics::ACCOUNT_RESET, analytics_attributes) AccountReset::CreateRequest.new(current_user).call - flash[:email] = current_user.email_addresses.first.email + flash[:email] = current_user.email_addresses.take.email redirect_to account_reset_confirm_request_url end diff --git a/app/controllers/concerns/two_factor_authenticatable.rb b/app/controllers/concerns/two_factor_authenticatable.rb index 6d57fcd803d..2e3afc3056a 100644 --- a/app/controllers/concerns/two_factor_authenticatable.rb +++ b/app/controllers/concerns/two_factor_authenticatable.rb @@ -210,7 +210,7 @@ def account_reset_token def authenticator_view_data { two_factor_authentication_method: two_factor_authentication_method, - user_email: current_user.email_addresses.first.email, + user_email: current_user.email_addresses.take.email, }.merge(generic_data) end diff --git a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb index ff78f8941cc..8c8a6c93eb6 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -68,7 +68,7 @@ def render_show_after_invalid def piv_cac_view_data { two_factor_authentication_method: two_factor_authentication_method, - user_email: current_user.email_addresses.first.email, + user_email: current_user.email_addresses.take.email, }.merge(generic_data) end diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb index 556601edd8c..69b65e1b3c9 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -30,7 +30,7 @@ def create private def delivery_preference - MfaContext.new(current_user).phone_configurations.first&.delivery_preference || + MfaContext.new(current_user).phone_configurations.take&.delivery_preference || current_user.otp_delivery_preference end diff --git a/app/decorators/mfa_context.rb b/app/decorators/mfa_context.rb index 4d4d1ac4acb..ffc66e57fab 100644 --- a/app/decorators/mfa_context.rb +++ b/app/decorators/mfa_context.rb @@ -16,7 +16,7 @@ def phone_configurations end def phone_configuration(id = nil) - return phone_configurations.first if id.blank? + return phone_configurations.take if id.blank? phone_configurations.find { |cfg| cfg.id.to_s == id.to_s } end diff --git a/app/decorators/user_decorator.rb b/app/decorators/user_decorator.rb index 5ed74e93e6f..a9d10ab6414 100644 --- a/app/decorators/user_decorator.rb +++ b/app/decorators/user_decorator.rb @@ -12,7 +12,7 @@ def initialize(user) end def email - user.email_addresses.first&.email + user.email_addresses.take&.email end def lockout_time_remaining_in_words @@ -40,7 +40,7 @@ def confirmation_period end def masked_two_factor_phone_number - masked_number(MfaContext.new(user).phone_configurations.first&.phone) + masked_number(MfaContext.new(user).phone_configurations.take&.phone) end def active_identity_for(service_provider) diff --git a/app/forms/idv/phone_form.rb b/app/forms/idv/phone_form.rb index 4d5fa7abeb2..02aec37a917 100644 --- a/app/forms/idv/phone_form.rb +++ b/app/forms/idv/phone_form.rb @@ -51,7 +51,7 @@ def initial_phone_value(input_phone, other_phone) return PhoneFormatter.format(input_phone) if input_phone.present? - user_phone = MfaContext.new(user).phone_configurations.first&.phone + user_phone = MfaContext.new(user).phone_configurations.take&.phone return unless Phonelib.valid_for_country?(user_phone, 'US') PhoneFormatter.format(user_phone) end diff --git a/app/forms/openid_connect_token_form.rb b/app/forms/openid_connect_token_form.rb index 3c1fcc8b316..678c611bc53 100644 --- a/app/forms/openid_connect_token_form.rb +++ b/app/forms/openid_connect_token_form.rb @@ -62,7 +62,8 @@ def find_identity_with_code session_expiration = Figaro.env.session_timeout_in_minutes.to_i.minutes.ago @identity = Identity.where(session_uuid: code). - where('updated_at >= ?', session_expiration).first + where('updated_at >= ?', session_expiration). + order(updated_at: :desc).first end def pkce? diff --git a/app/forms/update_user_email_form.rb b/app/forms/update_user_email_form.rb index cd8334e80fc..5433039ebb2 100644 --- a/app/forms/update_user_email_form.rb +++ b/app/forms/update_user_email_form.rb @@ -10,7 +10,7 @@ def persisted? def initialize(user) @user = user - self.email = @user.email_addresses.first&.email + self.email = @user.email_addresses.take&.email end def submit(params) @@ -31,7 +31,7 @@ def valid_form? end def email_changed? - valid? && email != @user.email_addresses.first&.email + valid? && email != @user.email_addresses.take&.email end private diff --git a/app/forms/webauthn_verification_form.rb b/app/forms/webauthn_verification_form.rb index 693872b352c..a8560aeeef3 100644 --- a/app/forms/webauthn_verification_form.rb +++ b/app/forms/webauthn_verification_form.rb @@ -69,7 +69,7 @@ def allowed_credential def public_key WebauthnConfiguration. - where(user_id: user.id, credential_id: @credential_id).first.credential_public_key + where(user_id: user.id, credential_id: @credential_id).take.credential_public_key end def extra_analytics_attributes diff --git a/app/models/anonymous_user.rb b/app/models/anonymous_user.rb index 1d0ee1c0db4..7c1b911a322 100644 --- a/app/models/anonymous_user.rb +++ b/app/models/anonymous_user.rb @@ -1,11 +1,5 @@ # :reek:UtilityFunction class AnonymousUser - EMPTY_EMAIL_ADDRESS = OpenStruct.new( - email: nil, - confirmed?: false, - confirmed_at: nil, - ).freeze - def uuid 'anonymous-uuid' end @@ -37,7 +31,7 @@ def otp_secret_key; end def email; end def email_addresses - [EMPTY_EMAIL_ADDRESS] + EmailAddress.none end def confirmed_at diff --git a/app/models/concerns/email_address_callback.rb b/app/models/concerns/email_address_callback.rb index df43d12025f..03203dc6d62 100644 --- a/app/models/concerns/email_address_callback.rb +++ b/app/models/concerns/email_address_callback.rb @@ -20,7 +20,7 @@ def update_email_address private def update_email_address_record - email_addresses.first.update!( + email_addresses.take.update!( encrypted_email: encrypted_email, confirmation_token: confirmation_token, confirmed_at: confirmed_at, diff --git a/app/models/concerns/user_encrypted_attribute_overrides.rb b/app/models/concerns/user_encrypted_attribute_overrides.rb index 8e5fdf66b74..f9b360c9b28 100644 --- a/app/models/concerns/user_encrypted_attribute_overrides.rb +++ b/app/models/concerns/user_encrypted_attribute_overrides.rb @@ -35,6 +35,6 @@ def email=(email) set_encrypted_attribute(name: :email, value: email) self.email_fingerprint = email.present? ? encrypted_attributes[:email].fingerprint : '' return if email_addresses.empty? - email_addresses.first.email = email + email_addresses.take.email = email end end diff --git a/app/models/identity.rb b/app/models/identity.rb index 26d46c96636..886d81b7473 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -36,6 +36,6 @@ def piv_cac_available? end def email - user.email_addresses.first&.email + user.email_addresses.take&.email end end diff --git a/app/models/user.rb b/app/models/user.rb index 500ea788fd3..4b573c85dc1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -85,7 +85,7 @@ def confirmation_period_expired? end def last_identity - identities.where.not(session_uuid: nil).order(last_authenticated_at: :desc).limit(1).first || + identities.where.not(session_uuid: nil).order(last_authenticated_at: :desc).take || NullIdentity.new end diff --git a/app/services/access_token_verifier.rb b/app/services/access_token_verifier.rb index 449edef30ef..db0ba46f038 100644 --- a/app/services/access_token_verifier.rb +++ b/app/services/access_token_verifier.rb @@ -27,7 +27,7 @@ def validate_access_token end def load_identity(access_token) - identity = Identity.where(access_token: access_token).first + identity = Identity.where(access_token: access_token).take if identity && Pii::SessionStore.new(identity.rails_session_id).ttl.positive? @identity = identity diff --git a/app/services/account_reset/cancel.rb b/app/services/account_reset/cancel.rb index f1e0b908ef2..ff8e48d5d8b 100644 --- a/app/services/account_reset/cancel.rb +++ b/app/services/account_reset/cancel.rb @@ -44,7 +44,7 @@ def user end def phone - MfaContext.new(user).phone_configurations.first&.phone + MfaContext.new(user).phone_configurations.take&.phone end def extra_analytics_attributes diff --git a/app/services/account_reset/create_request.rb b/app/services/account_reset/create_request.rb index 9d6a1834173..1c1544f0989 100644 --- a/app/services/account_reset/create_request.rb +++ b/app/services/account_reset/create_request.rb @@ -33,7 +33,7 @@ def notify_user_by_email(request) end def notify_user_by_sms_if_applicable - phone = MfaContext.new(user).phone_configurations.first&.phone + phone = MfaContext.new(user).phone_configurations.take&.phone return unless phone SmsAccountResetNotifierJob.perform_now( phone: phone, diff --git a/app/services/account_reset/delete_account.rb b/app/services/account_reset/delete_account.rb index 967263fe857..22e1b4b611f 100644 --- a/app/services/account_reset/delete_account.rb +++ b/app/services/account_reset/delete_account.rb @@ -51,7 +51,7 @@ def extra_analytics_attributes { user_id: user.uuid, event: 'delete', - email: user.email_addresses.first&.email, + email: user.email_addresses.take&.email, account_age_in_days: account_age, mfa_method_counts: mfa_method_counts, } diff --git a/app/services/account_reset_health_checker.rb b/app/services/account_reset_health_checker.rb index 4a19670e392..9b937727702 100644 --- a/app/services/account_reset_health_checker.rb +++ b/app/services/account_reset_health_checker.rb @@ -17,10 +17,9 @@ def check # @api private def find_request_not_serviced_within_26_hours - records = AccountResetRequest.where( + AccountResetRequest.where( sql, tvalue: Time.zone.now - Figaro.env.account_reset_wait_period_days.to_i.days - 2.hours - ).order('requested_at ASC').limit(1) - records.first + ).order('requested_at ASC').first end def sql diff --git a/app/services/agency_identity_linker.rb b/app/services/agency_identity_linker.rb index 2416f529203..d7c900bf775 100644 --- a/app/services/agency_identity_linker.rb +++ b/app/services/agency_identity_linker.rb @@ -10,13 +10,13 @@ def link_identity end def self.sp_identity_from_uuid_and_sp(uuid, service_provider) - ai = AgencyIdentity.where(uuid: uuid).first + ai = AgencyIdentity.where(uuid: uuid).take criteria = if ai { user_id: ai.user_id, service_provider: service_provider } else { uuid: uuid, service_provider: service_provider } end - Identity.where(criteria).first + Identity.where(criteria).take end private @@ -33,11 +33,11 @@ def create_agency_identity_for_sp end def agency_identity - ai = AgencyIdentity.where(uuid: @sp_identity.uuid).first + ai = AgencyIdentity.where(uuid: @sp_identity.uuid).take return ai if ai - sp = ServiceProvider.where(issuer: @sp_identity.service_provider).first + sp = ServiceProvider.where(issuer: @sp_identity.service_provider).take return unless agency_id(sp) - AgencyIdentity.where(agency_id: agency_id, user_id: @sp_identity.user_id).first + AgencyIdentity.where(agency_id: agency_id, user_id: @sp_identity.user_id).take end def agency_id(service_provider = nil) diff --git a/app/services/idv/steps/doc_auth_base_step.rb b/app/services/idv/steps/doc_auth_base_step.rb index af29effa64a..5dd8f18e669 100644 --- a/app/services/idv/steps/doc_auth_base_step.rb +++ b/app/services/idv/steps/doc_auth_base_step.rb @@ -64,7 +64,7 @@ def pii_from_test_doc def parse_pii(data) Idv::Utils::PiiFromDoc.new(data).call( - current_user&.phone_configurations&.first&.phone, + current_user&.phone_configurations&.take&.phone, ) end diff --git a/app/services/idv/steps/upload_step.rb b/app/services/idv/steps/upload_step.rb index 13a997cb30d..9b6115fa60a 100644 --- a/app/services/idv/steps/upload_step.rb +++ b/app/services/idv/steps/upload_step.rb @@ -18,7 +18,7 @@ def mobile? end def identity - current_user&.identities&.order('created_at DESC')&.limit(1)&.map(&:decorate)&.first + current_user&.identities&.order('created_at DESC')&.first&.decorate end def link diff --git a/app/services/reset_user_password.rb b/app/services/reset_user_password.rb index ca9ed18ab7c..0952fee6b79 100644 --- a/app/services/reset_user_password.rb +++ b/app/services/reset_user_password.rb @@ -13,6 +13,6 @@ def call def reset_user_password_and_log_event user.update!(password: SecureRandom.hex(8)) - Kernel.puts "Password for user with email #{user.email_addresses.first.email} has been reset" + Kernel.puts "Password for user with email #{user.email_addresses.take.email} has been reset" end end