Remove and resolve rubocop exemptions and violations

* Allow both snake_case and normal case as both are used and valid

* Omit the error class when rescuing StandardError by itself.

.rubocop.yml

@@ -51,6 +51,9 @@ Style/GuardClause:
 # problems as above.
 Style/Next:
   Enabled: false
+
+Naming/VariableNumber:
+  Enabled: false
 
 # This forces you to replace things like: `[1, 2, 3].length == 0` with `[1,2,3].empty?`. The problem is that
 # not all things that implement length also implement empty? so you will get errors that cannot be resolved,
@@ -81,6 +84,11 @@ Metrics/AbcSize:
 Naming/AccessorMethodName:
   Enabled: false
 
+Rails/LexicallyScopedActionFilter:
+  Exclude:
+    - "app/controllers/concerns/filterable.rb"	  	
+
+# Blocks are limited to 25 lines or less
 # removing block length rule for rspec DSL
 Metrics/BlockLength:
   Exclude:
@@ -91,6 +99,8 @@ Metrics/BlockLength:
     - 'app/controllers/v0/apidocs_controller.rb'
 
 # Don't worry about ambiguous blocks in RSpec
+# Official recommendation from rubocop team is to disable this rule for specs. 
+# See: https://github.com/rubocop-hq/rubocop/issues/4222
 Lint/AmbiguousBlockAssociation:
   Exclude:
     - "spec/**/*"
@@ -104,70 +114,27 @@ Naming/FileName:
 
 # We should try to use StandardError, but some errors like timeout inherit from Exception (beware)
 Style/RescueStandardError:
-  Exclude:
-    - "app/controllers/v0/id_card_attributes_controller.rb"
-    - "app/controllers/v0/sessions_controller.rb"
-    - "app/models/form_profile.rb"
-    - "app/models/mhv_account.rb"
-    - "app/services/mhv_accounts_service.rb"
-    - "app/uploaders/claim_documentation/uploader.rb"
-    - "app/workers/education_form/create_daily_spool_files.rb"
-    - "lib/common/client/concerns/monitoring.rb"
-    - "lib/common/client/configuration/soap.rb"
-    - "lib/common/exceptions/base_error.rb"
-    - "lib/github/github_service.rb"
-    - "lib/mvi/messages/find_profile_message_helpers.rb"
-    - "lib/mvi/responses/profile_parser.rb"
-    - "lib/saml/settings_service.rb"
-    - "lib/saml/ssoe_settings_service.rb"
-    - "rakelib/connectivity.rake"
-    - "rakelib/mvi.rake"
-    - "rakelib/redis.rake"
-    - "spec/request/authentication/standard_authentication.rb"
-
-# Variable naming style apparently doesn't like: something_2, something_3
-Naming/VariableNumber:
-  Exclude:
-    - "lib/hca/enrollment_system.rb"
-    - "spec/controllers/v0/post_911_gi_bill_statuses_controller_spec.rb"
-    - "spec/lib/preneeds/service_spec.rb"
+  EnforcedStyle: implicit
 
 # All kinds of issues with this cop right now, so disabling it.
 Style/FormatStringToken:
   Exclude:
     - "rakelib/mvi.rake"
 
-# This cop has an improvement for false positives in Mixins in future release, Exclude for now.
-# https://github.com/bbatsov/rubocop/issues/5448
-Rails/LexicallyScopedActionFilter:
-  Exclude:
-    - "app/controllers/concerns/filterable.rb"
-    - "app/controllers/v0/sessions_controller.rb"
 
 # Disabling this rule for now. We should consider changing.
 Lint/BooleanSymbol:
   Exclude:
-    - "app/swagger/schemas/gibct/institutions.rb"
-    - "spec/request/breakers_integration_spec.rb"
-    - "spec/request/statsd_middleware_spec.rb"
-    - "spec/support/authenticated_session_helper.rb"
+    - "app/swagger/schemas/gibct/institutions.rb" #Swagger blocks defines keys as :true, need to keep as is
 
-# Should consider enforcing this rule. Disabling for now.
+# Use `Time` when dealing with current dates and times; it has support for timezones (system/local and utc), whereas `DateTime` only supports offsets from UTC.
+# However, if you need to deal with dates and times in a historical context you'll want to use DateTime to avoid making the same mistakes as UNESCO.
+# See: https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/DateTime
 Style/DateTime:
   Exclude:
-    - "lib/common/models/attribute_types/date_time_string.rb"
-    - "lib/evss/auth_headers.rb"
-    - "lib/evss/pciu_address/service.rb"
     - "rakelib/evss.rake"
-    - "rakelib/mvi.rake"
+    - "lib/evss/auth_headers.rb" #DateTime is needed for date formatting here
 
-# IMPORTANT: this needs to be changed or handled differently. NoMethodError is not acceptable.
-# Skipping it for now as I dont have time to address.
-Lint/SafeNavigationChain:
-  Exclude:
-    - "app/workers/education_form/delete_old_applications.rb"
-    - "lib/saml/health_status.rb"
-    - "lib/saml/ssoe_health_status.rb"
 
 # Skipping for now, should revisit:
 Rails/HasManyOrHasOneDependent:
@@ -176,15 +143,3 @@ Rails/HasManyOrHasOneDependent:
     - "app/models/saved_claim/education_benefits.rb"
     - "app/models/saved_claim.rb"
     - "app/models/terms_and_conditions.rb"
-
-# Skipping for now, should revisit:
-Rails/SkipsModelValidations:
-  Exclude:
-    - "app/workers/education_form/create_daily_spool_files.rb"
-    - "app/models/saved_claim.rb"
-    - 'spec/**/*.rb'
-
-# Need to revisit why this is necessary:
-Lint/RescueException:
-  Exclude:
-    - 'app/workers/github/create_issue_job.rb'

app/controllers/concerns/accountable.rb

@@ -10,7 +10,7 @@ module Accountable
   #
   def create_user_account
     Account.cache_or_create_by! @current_user
-  rescue StandardError => error
+  rescue => error
     log error
   end
 

app/controllers/openid_application_controller.rb

@@ -47,7 +47,7 @@ def establish_session
     @current_user = OpenidUser.build_from_identity(identity: user_identity, ttl: ttl)
     @session = build_session(token, token_identifiers.uuid, ttl)
     @session.save && user_identity.save && @current_user.save
-  rescue StandardError => e
+  rescue => e
     Rails.logger.warn(e)
   end
 

app/controllers/v0/id_card_attributes_controller.rb

@@ -33,7 +33,7 @@ def authorize
         current_user.veteran_status.title38_status
       rescue EMISRedis::VeteranStatus::RecordNotFound
         nil
-      rescue StandardError => e
+      rescue => e
         log_exception_to_sentry(e)
         raise ::VIC::IDCardAttributeError, ::VIC::IDCardAttributeError::VIC010
       end

app/controllers/v0/ppiu_controller.rb

@@ -37,7 +37,7 @@ def current_user_email
       vet360_email =
         begin
           current_user.vet360_contact_info&.email&.email_address
-        rescue StandardError
+        rescue
           nil
         end
       vet360_email || current_user.email

app/controllers/v0/sessions_controller.rb

@@ -42,7 +42,7 @@ def saml_logout_callback
         log_error(saml_response)
         Rails.logger.info("SLO callback response invalid for originating_request_id '#{originating_request_id}'")
       end
-    rescue StandardError => e
+    rescue => e
       log_exception_to_sentry(e, {}, {}, :error)
     ensure
       redirect_to url_service.logout_redirect_url
@@ -58,7 +58,7 @@ def saml_callback
         redirect_to url_service.login_redirect_url(auth: 'fail', code: auth_error_code(saml_response.error_code))
         stats(:failure, saml_response, saml_response.error_instrumentation_code)
       end
-    rescue StandardError => e
+    rescue => e
       log_exception_to_sentry(e, {}, {}, :error)
       redirect_to url_service.login_redirect_url(auth: 'fail', code: '007') unless performed?
       stats(:failed_unknown)

app/controllers/v1/sessions_controller.rb

@@ -42,7 +42,7 @@ def saml_logout_callback
         log_error(saml_response)
         Rails.logger.info("SLO callback response invalid for originating_request_id '#{originating_request_id}'")
       end
-    rescue StandardError => e
+    rescue => e
       log_exception_to_sentry(e, {}, {}, :error)
     ensure
       redirect_to url_service.logout_redirect_url
@@ -58,7 +58,7 @@ def saml_callback
         redirect_to url_service.login_redirect_url(auth: 'fail', code: auth_error_code(saml_response.error_code))
         stats(:failure, saml_response, saml_response.error_instrumentation_code)
       end
-    rescue StandardError => e
+    rescue => e
       log_exception_to_sentry(e, {}, {}, :error)
       redirect_to url_service.login_redirect_url(auth: 'fail', code: '007') unless performed?
       stats(:failed_unknown)
@@ -172,13 +172,13 @@ def log_persisted_session_and_warnings
 
     def originating_request_id
       JSON.parse(params[:RelayState] || '{}')['originating_request_id']
-    rescue StandardError
+    rescue
       'UNKNOWN'
     end
 
     def request_type
       JSON.parse(params[:RelayState] || '{}')['type']
-    rescue StandardError
+    rescue
       'UNKNOWN'
     end
 

app/models/form_profiles/va0994.rb

@@ -48,7 +48,7 @@ def initialize_payment_information(user)
     else
       {}
     end
-  rescue StandardError => e
+  rescue => e
     Rails.logger.error "Failed to retrieve PPIU data: #{e.message}"
     {}
   end

app/models/form_profiles/va526ez.rb

@@ -159,7 +159,7 @@ def get_common_address(user)
     else
       {}
     end
-  rescue StandardError
+  rescue
     {}
   end
 
@@ -214,7 +214,7 @@ def initialize_payment_information(user)
     else
       {}
     end
-  rescue StandardError => e
+  rescue => e
     Rails.logger.error "Failed to retrieve PPIU data: #{e.message}"
     {}
   end

app/models/id_card_attributes.rb

@@ -42,7 +42,7 @@ def traits
 
   def title38_status_code
     @user.veteran_status.title38_status || 'UNKNOWN'
-  rescue StandardError
+  rescue
     'UNKNOWN'
   end
 

app/models/saved_claim.rb

@@ -41,7 +41,7 @@ def self.add_form_and_validation(form_id)
   def process_attachments!
     refs = attachment_keys.map { |key| Array(open_struct_form.send(key)) }.flatten
     files = PersistentAttachment.where(guid: refs.map(&:confirmationCode))
-    files.update_all(saved_claim_id: id)
+    files.find_each { |f| f.update(saved_claim_id: id) }
 
     CentralMail::SubmitSavedClaimJob.perform_async(id)
   end

app/models/user.rb

@@ -188,7 +188,7 @@ def va_patient?
   def can_access_id_card?
     loa3? && edipi.present? &&
       ID_CARD_ALLOWED_STATUSES.include?(veteran_status.title38_status)
-  rescue StandardError # Default to false for any veteran_status error
+  rescue # Default to false for any veteran_status error
     false
   end
 
@@ -251,7 +251,7 @@ def vet360_contact_info
 
   def can_access_vet360?
     loa3? && icn.present? && vet360_id.present?
-  rescue StandardError # Default to false for any error
+  rescue # Default to false for any error
     false
   end
 

app/services/mhv_account_type_service.rb

@@ -72,7 +72,7 @@ def fetch_eligible_data_classes
       bb_client.authenticate
       bb_client.get_eligible_data_classes.members.map(&:name)
     end
-  rescue StandardError => e
+  rescue => e
     log_account_type_heuristic_once(MHV_DOWN_MESSAGE, error_message: e.message)
     nil
   end

app/services/users/profile.rb

@@ -53,7 +53,7 @@ def fetch_and_serialize_profile
 
     def account
       { account_uuid: user.account_uuid }
-    rescue StandardError => e
+    rescue => e
       scaffold.errors << Users::ExceptionHandler.new(e, 'Account').serialize_error
       nil
     end
@@ -91,7 +91,7 @@ def vet360_contact_information
         temporary_phone: person.temporary_phone,
         fax_number: person.fax_number
       }
-    rescue StandardError => e
+    rescue => e
       scaffold.errors << Users::ExceptionHandler.new(e, 'Vet360').serialize_error
       nil
     end
@@ -119,7 +119,7 @@ def veteran_status
         is_veteran: user.veteran?,
         served_in_military: user.served_in_military?
       }
-    rescue StandardError => e
+    rescue => e
       scaffold.errors << Users::ExceptionHandler.new(e, 'EMIS').serialize_error
       nil
     end

app/workers/central_mail/submit_form4142_job.rb

@@ -36,7 +36,7 @@ def perform(submission_id)
         response = CentralMail::Service.new.upload(processor.request_body)
         handle_service_exception(response) if response.present? && response.status.between?(201, 600)
       end
-    rescue StandardError => error
+    rescue => error
       # Cannot move job straight to dead queue dynamically within an executing job
       # raising error for all the exceptions as sidekiq will then move into dead queue
       # after all retries are exhausted

app/workers/central_mail/submit_saved_claim_job.rb

@@ -31,7 +31,7 @@ def perform(saved_claim_id)
       else
         raise CentralMailResponseError
       end
-    rescue StandardError
+    rescue
       update_submission('failed')
       raise
     end

app/workers/education_form/create_daily_spool_files.rb

@@ -82,7 +82,7 @@ def write_files(writer, structured_data:)
 
         # track and update the records as processed once the file has been successfully written
         track_submissions(region_id)
-        records.each { |r| r.record.update_attribute(:processed_at, Time.zone.now) }
+        records.each { |r| r.record.update_attributes(processed_at: Time.zone.now) }
       end
     ensure
       writer.close

app/workers/education_form/delete_old_applications.rb

@@ -13,7 +13,7 @@ def perform
                             .where("processed_at < '#{2.months.ago}'")
                             .find_each do |record|
         edu_claim_ids << record.id
-        saved_claim_ids << record&.saved_claim.id
+        saved_claim_ids << record&.saved_claim&.id
       end
 
       edu_claim_ids.compact!

app/workers/evss/dependents_application_job.rb

@@ -34,7 +34,7 @@ def perform(app_id, form, user_uuid)
         state: 'success',
         response: res.to_json
       )
-    rescue StandardError
+    rescue
       dependents_application.update_attributes!(
         state: 'failed'
       )

app/workers/evss/disability_compensation_form/job_status.rb

@@ -35,7 +35,7 @@ def job_exhausted(msg)
             'Form526 Exhausted', submission_id: submission_id, job_id: jid, error_message: error_message
           )
           Metrics.new(STATSD_KEY_PREFIX).increment_exhausted
-        rescue StandardError => error
+        rescue => error
           Rails.logger.error('error tracking job exhausted', error: error, class: klass)
         end
       end
@@ -62,7 +62,7 @@ def job_try
         upsert_job_status(Form526JobStatus::STATUS[:try])
         log_info('try')
         metrics.increment_try
-      rescue StandardError => error
+      rescue => error
         Rails.logger.error('error tracking job try', error: error, class: klass)
       end
 
@@ -72,7 +72,7 @@ def job_success
         upsert_job_status(Form526JobStatus::STATUS[:success])
         log_info('success')
         metrics.increment_success
-      rescue StandardError => error
+      rescue => error
         Rails.logger.error('error tracking job success', error: error, class: klass)
       end
 

app/workers/evss/disability_compensation_form/submit_form0781.rb

@@ -47,7 +47,7 @@ def perform(submission_id)
                          submission.submitted_claim_id, FORM_ID_0781A, parsed_forms['form0781a'])
           end
         end
-      rescue StandardError => error
+      rescue => error
         # Cannot move job straight to dead queue dynamically within an executing job
         # raising error for all the exceptions as sidekiq will then move into dead queue
         # after all retries are exhausted