Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jcroteau/APPEALS-29633 Fix Deprecation Warning: ActiveRecord::Result#to_hash has been renamed to to_a. to_hash [pre Rails 6.1] #19553

Conversation

jcroteau
Copy link
Contributor

@jcroteau jcroteau commented Sep 21, 2023

🔴 Warning: To be merged into master only with or after the Rails 6.0 upgrade (#19261).

Resolves https://jira.devops.va.gov/browse/APPEALS-29633

Description

Deprecation Warning

DEPRECATION WARNING: `ActiveRecord::Result#to_hash` has been renamed to `to_a`. `to_hash` is deprecated and will be removed in Rails 6.1.

Deprecation Horizon: Rails 6.1

Affected locations

Found in CI / Test

The following locations were identified via DisallowedDeprecationError's in CI / Test:

  • called from block (2 levels) in index at app/controllers/organizations/task_summary_controller.rb:34
  • called from counts_by_priority_and_readiness at app/models/vacols/case_docket.rb:182
  • called from counts_by_priority_and_readiness at app/models/vacols/case_docket.rb:201
  • called from age_of_n_oldest_genpop_priority_appeals at app/models/vacols/case_docket.rb:283
  • called from age_of_oldest_priority_appeal at app/models/vacols/case_docket.rb:325
  • called from block in distribute_appeals at app/models/vacols/case_docket.rb:457
  • called from descriptions at app/models/vacols/case_issue.rb:106
  • called at app/models/concerns/belongs_to_polymorphic_appeal_concern_spec.rb:56

Found by Code Search

  • Codebase was manually searched for other potentially affected locations.
    • Regular expressions used:
      • /exec_query[\S\s]*to_hash/
      • /\.to_hash/
        • matches were carefully vetted for calls to ActiveRecord::Result #to_hash
  • called at app/models/vacols/case_assignment.rb:162
  • called from counts_by_priority_and_readiness at app/models/vacols/case_docket.rb:192
  • called from age_of_n_oldest_genpop_priority_appeals at app/models/vacols/case_docket.rb:298
  • called from age_of_oldest_priority_appeal at app/models/vacols/case_docket.rb:313
  • called from age_of_oldest_priority_appeal at app/models/vacols/case_docket.rb:336
  • called from age_of_oldest_priority_appeal at app/models/vacols/case_docket.rb:369
  • called from block in distribute_appeals at app/models/vacols/case_docket.rb:453
  • called at app/models/vacols/correspondent.rb:52
  • called at lib/tasks/doc.rake:44

Solution

Replace calls to ActiveRecord::Result #to_hash with #to_a.

Note: This is only a signature change. The result of ActiveRecord::Result #to_a is the same as for #to_hash.

Testing Plan

🔴 Warning: To be merged into master only with or after the Rails 6.0 upgrade (#19261).

@jcroteau jcroteau changed the base branch from master to DO-NOT-MERGE/rails-6-0-upgrade-plus-bug-and-deprecation-fixes September 21, 2023 22:18
@codeclimate

This comment was marked as off-topic.

@jcroteau jcroteau marked this pull request as ready for review September 22, 2023 00:15
@jcroteau jcroteau force-pushed the jcroteau/APPEALS-29633-fix-deprecation-warning-active_record-result-to_hash branch from 89057cd to cb54d0c Compare April 9, 2024 17:50
@jcroteau jcroteau changed the base branch from DO-NOT-MERGE/rails-6-0-upgrade-plus-bug-and-deprecation-fixes to master April 9, 2024 18:08
@jcroteau jcroteau requested a review from sbashamoni April 9, 2024 18:14
…o_a'

Method 'ActiveRecord::Result #to_hash' is deprecated and will be removed in Rails 6.1.
It is replaced by '#to_a', which returns the same result.
@jcroteau jcroteau force-pushed the jcroteau/APPEALS-29633-fix-deprecation-warning-active_record-result-to_hash branch from cb54d0c to 8fe5a98 Compare April 9, 2024 18:46
@jcroteau
Copy link
Contributor Author

There are two tests that consistently fail in CI, however they pass locally and have no apparent relation to the changes made in this PR (which are purely API changes, not behavioral changes):

Failures:

  1) BgsPowerOfAttorney.find_or_create_by_claimant_participant_id when concurrent calls cause a race condition does not raise an error on unique constraint violation
     Failure/Error: find_by(claimant_participant_id: claimant_participant_id)
     
     ActiveRecord::StatementInvalid:
       PG::InFailedSqlTransaction: ERROR:  current transaction is aborted, commands ignored until end of transaction block
     # ./app/models/bgs_power_of_attorney.rb:60:in `rescue in find_or_create_by_claimant_participant_id'
     # ./app/models/bgs_power_of_attorney.rb:51:in `find_or_create_by_claimant_participant_id'
     # ./spec/models/bgs_power_of_attorney_spec.rb:122:in `block (6 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # PG::UniqueViolation:
     #   ERROR:  duplicate key value violates unique constraint "bgs_poa_pid_fn_unique_idx"
     #   DETAIL:  Key (claimant_participant_id, file_number)=(1129318238, fn) already exists.
     #   ./app/models/bgs_power_of_attorney.rb:52:in `find_or_create_by_claimant_participant_id'

  2) BgsPowerOfAttorney.find_or_create_by_file_number when concurrent calls cause a race condition does not raise an error on unique constraint violation
     Failure/Error: find_by(file_number: file_number)
     
     ActiveRecord::StatementInvalid:
       PG::InFailedSqlTransaction: ERROR:  current transaction is aborted, commands ignored until end of transaction block
     # ./app/models/bgs_power_of_attorney.rb:48:in `rescue in find_or_create_by_file_number'
     # ./app/models/bgs_power_of_attorney.rb:37:in `find_or_create_by_file_number'
     # ./spec/models/bgs_power_of_attorney_spec.rb:184:in `block (6 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # PG::UniqueViolation:
     #   ERROR:  duplicate key value violates unique constraint "bgs_poa_pid_fn_unique_idx"
     #   DETAIL:  Key (claimant_participant_id, file_number)=(1738055, fn) already exists.
     #   ./app/models/bgs_power_of_attorney.rb:38:in `find_or_create_by_file_number'

@ThorntonMatthew
Copy link
Contributor

Closing due to inactivity as part of our post-PI cleanup effort. Feel free to reopen if this PR is still desired.

Copy link

Code Scanning Policy Findings

Your repository contains unresolved code scanning alerts. Policy requires that all code scanning alerts of critical severity be resolved within 30 days.

In the future, if your repository contains unresolved code scanning alerts older than 30 days, you will not be able to merge this pull request.

Learn more about how to triage and remediate these alerts in the GitHub Code Scanning documentation.

If this pull request remediates these alerts, after your pull requests CodeQL scan has completed, follow this link to re-run the policy check and select Re-run all jobs at the top of the page: https://github.com/department-of-veterans-affairs/caseflow/actions/runs/8835006858

You may also re-run this required check by simply commenting on this pull request with the following command:
/actions-bot rerun-required-workflows

Alert NumberURLAgePolicy Violation
24Link399 DaysYes
20Link402 DaysYes
19Link402 DaysYes
18Link402 DaysYes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants