Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,9 @@ Rails/Blank:
Rails/CompactBlank:
Enabled: false

Rails/DangerousColumnNames:
Enabled: true

Rails/Delegate:
Enabled: false

Expand All @@ -868,6 +871,12 @@ Rails/DynamicFindBy:
Rails/EagerEvaluationLogMessage:
Enabled: true

Rails/EnumSyntax:
Enabled: true

Rails/EnvLocal:
Enabled: true

Rails/ExpandedDateRange:
Enabled: true

Expand Down Expand Up @@ -932,6 +941,9 @@ Rails/PluckInWhere:
Rails/Present:
Enabled: false

Rails/RedundantActiveRecordAllMethod:
Enabled: true

Rails/RedundantPresenceValidationOnBelongsTo:
Enabled: false

Expand Down Expand Up @@ -959,6 +971,9 @@ Rails/RootPathnameMethods:
Rails/RootPublicPath:
Enabled: true

Rails/SelectMap:
Enabled: true

Rails/ShortI18n:
Enabled: true

Expand Down Expand Up @@ -998,6 +1013,9 @@ Rails/TransactionExitStatement:
Rails/UnusedIgnoredColumns:
Enabled: true

Rails/UnusedRenderContent:
Enabled: true

Rails/WhereEquals:
Enabled: true

Expand All @@ -1013,6 +1031,9 @@ Rails/WhereNot:
Rails/WhereNotWithMultipleConditions:
Enabled: true

Rails/WhereRange:
Enabled: false
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could enable this. It's a bit more of a stylistic one, and the "Safety" notes gave me pause to trust the autocorrecter without closer inspection.

https://docs.rubocop.org/rubocop-rails/cops_rails.html#railswhererange

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it seems more stylistic and I don't find it significantly better? I could go either way on it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I have trouble recalling whether .. or ... is the inclusive version in a range (i.e. <= or >=), so I'd worry this could also negatively impact readability. Maybe that's just me though 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a slight preference for the explicit >, < as well, so it's not just you 🙂


RSpec/LeakyConstantDeclaration:
Enabled: true

Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ group :development, :test do
gem 'rspec-rails', '~> 6.0'
gem 'rubocop', '~> 1.62.0', require: false
gem 'rubocop-performance', '~> 1.20.2', require: false
gem 'rubocop-rails', '>= 2.5.2', require: false
gem 'rubocop-rails', '>= 2.26.2', require: false
gem 'rubocop-rspec', require: false
gem 'sqlite3', require: false
end
Expand Down
7 changes: 4 additions & 3 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -633,10 +633,11 @@ GEM
rubocop-performance (1.20.2)
rubocop (>= 1.48.1, < 2.0)
rubocop-ast (>= 1.30.0, < 2.0)
rubocop-rails (2.20.2)
rubocop-rails (2.26.2)
activesupport (>= 4.2.0)
rack (>= 1.1)
rubocop (>= 1.33.0, < 2.0)
rubocop (>= 1.52.0, < 2.0)
rubocop-ast (>= 1.31.1, < 2.0)
rubocop-rspec (2.24.1)
rubocop (~> 1.33)
rubocop-capybara (~> 2.17)
Expand Down Expand Up @@ -851,7 +852,7 @@ DEPENDENCIES
rspec_junit_formatter
rubocop (~> 1.62.0)
rubocop-performance (~> 1.20.2)
rubocop-rails (>= 2.5.2)
rubocop-rails (>= 2.26.2)
rubocop-rspec
ruby-progressbar
ruby-saml
Expand Down
2 changes: 1 addition & 1 deletion app/models/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Event < ApplicationRecord
belongs_to :user
belongs_to :device

enum event_type: {
enum :event_type, {
account_created: 1,
phone_confirmed: 2,
password_changed: 3,
Expand Down
2 changes: 1 addition & 1 deletion app/models/in_person_enrollment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class InPersonEnrollment < ApplicationRecord
STATUS_EXPIRED = 'expired'
STATUS_CANCELLED = 'cancelled'

enum status: {
enum :status, {
STATUS_ESTABLISHING.to_sym => 0,
STATUS_PENDING.to_sym => 1,
STATUS_PASSED.to_sym => 2,
Expand Down
2 changes: 1 addition & 1 deletion app/models/phone_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class PhoneConfiguration < ApplicationRecord

encrypted_attribute(name: :phone)

enum delivery_preference: { sms: 0, voice: 1 }
enum :delivery_preference, { sms: 0, voice: 1 }

def formatted_phone
PhoneFormatter.format(phone)
Expand Down
6 changes: 3 additions & 3 deletions app/models/profile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,20 @@ class Profile < ApplicationRecord
class_name: 'InPersonEnrollment', foreign_key: :profile_id, inverse_of: :profile,
dependent: :destroy

enum deactivation_reason: {
enum :deactivation_reason, {
password_reset: 1,
encryption_error: 2,
gpo_verification_pending_NO_LONGER_USED: 3, # deprecated
verification_cancelled: 4,
in_person_verification_pending_NO_LONGER_USED: 5, # deprecated
}

enum fraud_pending_reason: {
enum :fraud_pending_reason, {
threatmetrix_review: 1,
threatmetrix_reject: 2,
}

enum idv_level: {
enum :idv_level, {
legacy_unsupervised: 1,
legacy_in_person: 2,
unsupervised_with_selfie: 3,
Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class User < ApplicationRecord
MAX_RECENT_EVENTS = 5
MAX_RECENT_DEVICES = 5

enum otp_delivery_preference: { sms: 0, voice: 1 }
enum :otp_delivery_preference, { sms: 0, voice: 1 }

# rubocop:disable Rails/HasManyOrHasOneDependent
# identities need to be orphaned to prevent UUID reuse
Expand Down
2 changes: 1 addition & 1 deletion app/services/access_token_verifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def validate_access_token
end

def load_identity(access_token)
identity = ServiceProviderIdentity.where(access_token: access_token).take
identity = ServiceProviderIdentity.find_by(access_token: access_token)

if identity && OutOfBandSessionAccessor.new(identity.rails_session_id).ttl.positive?
@identity = identity
Expand Down
16 changes: 8 additions & 8 deletions app/services/agency_identity_linker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,25 @@ def link_identity
def self.for(user:, service_provider:)
agency = service_provider.agency

ai = AgencyIdentity.where(user: user, agency: agency).take
ai = AgencyIdentity.find_by(user: user, agency: agency)
return ai if ai.present?

spi = ServiceProviderIdentity.where(
spi = ServiceProviderIdentity.find_by(
user: user, service_provider: service_provider.issuer,
).take
)

return nil unless spi.present?
new(spi).link_identity
end

def self.sp_identity_from_uuid_and_sp(uuid, service_provider)
ai = AgencyIdentity.where(uuid: uuid).take
ai = AgencyIdentity.find_by(uuid: uuid)
criteria = if ai
{ user_id: ai.user_id, service_provider: service_provider }
else
{ uuid: uuid, service_provider: service_provider }
end
ServiceProviderIdentity.where(criteria).take
ServiceProviderIdentity.find_by(criteria)
end

private
Expand All @@ -53,11 +53,11 @@ def create_agency_identity_for_sp
end

def agency_identity
ai = AgencyIdentity.where(uuid: @sp_identity.uuid).take
ai = AgencyIdentity.find_by(uuid: @sp_identity.uuid)
return ai if ai
sp = ServiceProvider.where(issuer: @sp_identity.service_provider).take
sp = ServiceProvider.find_by(issuer: @sp_identity.service_provider)
return unless agency_id(sp)
AgencyIdentity.where(agency_id: agency_id, user_id: @sp_identity.user_id).take
AgencyIdentity.find_by(agency_id: agency_id, user_id: @sp_identity.user_id)
end

def agency_id(service_provider = nil)
Expand Down
2 changes: 1 addition & 1 deletion app/services/create_new_device_alert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def perform(now)
User.where(
sql_query_for_users_with_new_device,
tvalue: now - IdentityConfig.store.new_device_alert_delay_in_minutes.minutes,
).each do |user|
).find_each do |user|
emails_sent += 1 if expire_sign_in_notification_timeframe_and_send_alert(user)
end

Expand Down
2 changes: 1 addition & 1 deletion app/services/gpo_reminder_sender.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def send_emails(for_letters_sent_before)
IdentityConfig.store.usps_confirmation_max_days.days.ago..for_letters_sent_before
profiles_due_for_reminder(for_letters_sent_before).each do |profile|
next if profile.user.active_profile
profile.gpo_confirmation_codes.all.each do |gpo_code|
profile.gpo_confirmation_codes.find_each do |gpo_code|
next if gpo_code.reminder_sent_at
next unless reminder_eligible_range.cover?(gpo_code.created_at)

Expand Down
10 changes: 5 additions & 5 deletions app/services/proofing/socure/reason_codes/importer.rb
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYSA @jmhooper re: #11350, picked these changes up on a rebase.

Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def synchronize
end

create_or_update_downloaded_reason_codes
deactive_missing_reason_codes
deactivate_missing_reason_codes

FormResponse.new(
success: true,
Expand Down Expand Up @@ -64,14 +64,14 @@ def create_or_update_downloaded_reason_codes
end
end

def deactive_missing_reason_codes
def deactivate_missing_reason_codes
active_codes = downloaded_reason_codes.flat_map do |_group, reason_codes|
reason_codes.keys
end
deactivated_at = Time.zone.now
SocureReasonCode.active.where.not(code: active_codes).each do |deactivateable_reason_code|
deactivateable_reason_code.update!(deactivated_at:)
deactivated_reason_code_records.push(deactivateable_reason_code)
SocureReasonCode.active.where.not(code: active_codes).find_each do |reason_code|
reason_code.update!(deactivated_at:)
deactivated_reason_code_records.push(reason_code)
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/services/usps_in_person_proofing/enrollment_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def cancel_stale_establishing_enrollments_for_user(user)
user.
in_person_enrollments.
where(status: :establishing).
each(&:cancelled!)
find_each(&:cancelled!)
end

def usps_proofer
Expand Down
2 changes: 1 addition & 1 deletion lib/identity_cors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class IdentityCors
].freeze

def self.allowed_origins_static_sites
return STATIC_SITE_ALLOWED_ORIGINS unless Rails.env.development? || Rails.env.test?
return STATIC_SITE_ALLOWED_ORIGINS unless Rails.env.local?
allowed_origins = STATIC_SITE_ALLOWED_ORIGINS.dup
allowed_origins << %r{https?://localhost(:\d+)?\z}
allowed_origins << %r{https?://127\.0\.0\.1(:\d+)?\z}
Expand Down
2 changes: 0 additions & 2 deletions lib/reporting/authentication_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,12 +211,10 @@ def format_as_percent(numerator:, denominator:)
end
end

# rubocop:disable Rails/Output
if __FILE__ == $PROGRAM_NAME
options = Reporting::CommandLineOptions.new.parse!(ARGV)

Reporting::AuthenticationReport.new(**options).to_csvs.each do |csv|
puts csv
end
end
# rubocop:enable Rails/Output
2 changes: 0 additions & 2 deletions lib/reporting/drop_off_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -450,12 +450,10 @@ def cloudwatch_client
end
end

# rubocop:disable Rails/Output
if __FILE__ == $PROGRAM_NAME
options = Reporting::CommandLineOptions.new.parse!(ARGV, require_issuer: false)

Reporting::DropOffReport.new(**options).to_csvs.each do |csv|
puts csv
end
end
# rubocop:enable Rails/Output
2 changes: 0 additions & 2 deletions lib/reporting/mfa_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,10 @@ def multi_factor_auth_table
end
end

# rubocop:disable Rails/Output
if __FILE__ == $PROGRAM_NAME
options = Reporting::CommandLineOptions.new.parse!(ARGV)

Reporting::MfaReport.new(**options).to_csvs.each do |csv|
puts csv
end
end
# rubocop:enable Rails/Output
2 changes: 0 additions & 2 deletions lib/reporting/protocols_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -375,12 +375,10 @@ def to_percent(numerator, denominator)
end
end

# rubocop:disable Rails/Output
if __FILE__ == $PROGRAM_NAME
options = Reporting::CommandLineOptions.new.parse!(ARGV, require_issuer: false)

Reporting::ProtocolsReport.new(**options).to_csvs.each do |csv|
puts csv
end
end
# rubocop:enable Rails/Output
2 changes: 1 addition & 1 deletion spec/jobs/in_person/send_proofing_notification_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@

context 'enrollment does not exist' do
it 'returns without doing anything' do
bad_id = (InPersonEnrollment.all.pluck(:id).max || 0) + 1
bad_id = (InPersonEnrollment.pluck(:id).max || 0) + 1
job.perform(bad_id)
expect(analytics).not_to have_logged_event('SendProofingNotificationJob: job started')
expect(analytics).to have_logged_event('SendProofingNotificationJob: job skipped')
Expand Down
2 changes: 1 addition & 1 deletion spec/services/agency_identity_linker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@

it 'persists the service provider identity as an agency identity' do
expect(subject.uuid).to eq uuid
ai = AgencyIdentity.where(user: user, agency: agency).take
ai = AgencyIdentity.find_by(user: user, agency: agency)
expect(subject).to eq ai
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/services/funnel/registration/add_mfa_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
user = create(:user)
user.id
end
let(:funnel) { RegistrationLog.all.first }
let(:funnel) { RegistrationLog.first }

it 'shows user is not fully registered with no mfa' do
expect(funnel&.registered_at).to_not be_present
Expand Down
2 changes: 2 additions & 0 deletions spec/support/ab_tests_helper.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
module AbTestsHelper
def reload_ab_tests
# rubocop:disable Rails/FindEach
AbTests.all.each do |(name, _)|
AbTests.send(:remove_const, name)
end
# rubocop:enable Rails/FindEach
load('config/initializers/ab_tests.rb')
end
end
4 changes: 4 additions & 0 deletions spec/support/matchers/accessibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@

match do |page|
['aria-describedby', 'aria-labelledby'].each do |idref_attribute|
# rubocop:disable Rails/FindEach
page.all(:css, "[#{idref_attribute}]").each do |element|
element[idref_attribute].split(' ').each do |referenced_id|
page.find_by_id(referenced_id, visible: :all)
end
rescue Capybara::ElementNotFound
invalid_idref_messages << "[#{idref_attribute}=\"#{element[idref_attribute]}\"]"
end
# rubocop:enable Rails/FindEach
end

invalid_idref_messages.blank?
Expand All @@ -28,9 +30,11 @@
elements = []

match do |page|
# rubocop:disable Rails/FindEach
page.all(:css, 'input[required]').each do |input|
elements << input if input['aria-invalid'].blank?
end
# rubocop:enable Rails/FindEach
elements.empty?
end

Expand Down