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

[DO NOT MERGE] APPEALS-28923 - Rails 6.0.Z - Raise on deprecation warnings in test env #19287

Conversation

jcroteau
Copy link
Contributor

@jcroteau jcroteau commented Aug 28, 2023

DO NOT MERGE.

This is just an exploratory branch to see what deprecation warnings will emerge from the test suite on Rails 6.0.

This will ensure cleaner diffs when generating config file changes during Rails upgrades.
CVE-2019-5420 was fixed in Rails 5.2.2.1, negating the need for this workaround.

For further details, see https://nvd.nist.gov/vuln/detail/CVE-2019-5420.
 This will ensure cleaner diffs when generating config file changes during Rails upgrades.
This will ensure cleaner diffs when generating config file changes during Rails upgrades.
We were already effectively assuming the default value (true) for this setting, so we can omit it.
…essage_encryption'

We can migrate this setting to the default (true) after the 5.2.8.1 upgrade is verified and stable in production.
…from_forgery'

Migrating to the default value (true) will require a non-trivial refactoring of several controllers
…olean_as_integer'

Migrating to the default value (true) will first require a migration of old values in any sqlite3 boolean columns to the new values.
Currently, 'demo_vacols' is the only sqlite3 database affected.
We can safely assume the default (true) for this setting.
We may notice an initial spike in requests since all cached assets will be invalided.
…s_ids'

We're not currently using 'form_with' helper anywhere, so we can safely assume the default.
…ticated_cookie_encryption'

We can migrate to the default value (true) after the upgrade is verified and stable in production.
'config.serve_static_files' was replaced by 'config.public_file_server.enabled' in Rails 5.0
We're not currently using ActionCable, so changes to this file are inconsequential.
Also override Zeitwerk autoloading default to preserve classic behavior.
We must eventually transition to Zeitwerk when upgrading to Rails 7.0.
`on:` is not a valid option for `before/after/around_save` callbacks and will be ignored on Rails versions before 6.0.

Rails 6.0 adds strict argument checking on callbacks, which will cause an exception to be raised here when loading
the application when we upgrade.

See related rails PR which introduces this change: rails/rails#30919

Simply removing the invalid `on:` option will not change the existing behavior and will allow us to avoid this
error upon upgrading to Rails 6.0.
…d-oracle_enhanced-adapter'

oracle-enhanced v6.00 changes the default sequence start value from 10,000 to 1:
  Release Notes: https://github.com/rsim/oracle-enhanced/blob/v6.0.6/History.md#600--2019-08-17
  Related PR: rsim/oracle-enhanced#1636

Preserve the original default value of 10,000
…ure/APPEALS-26093-APPEALS-29103-APPEALS-26725-update-to-rails-6-0
…lback-argument' into feature/APPEALS-26093-APPEALS-29103-APPEALS-26725-update-to-rails-6-0
…OT-MERGE/APPEALS-26093-APPEALS-29103-APPEALS-26725-provoke-deprecation-warnings-in-test
@codeclimate
Copy link

codeclimate bot commented Aug 28, 2023

Code Climate has analyzed commit 51e9c76 and detected 9 issues on this pull request.

Here's the issue category breakdown:

Category Count
Security 4
Style 5

View more on Code Climate.

@jcroteau jcroteau changed the title DO-NOT-MERGE/APPEALS-28923 Provoke Rails 6.0 deprecation warnings in test [DO NOT MERGE] APPEALS-28923 - Rails 6.0.Z - Raise on deprecation warnings in test env Aug 28, 2023
@jcroteau jcroteau closed this Dec 4, 2023
@jcroteau jcroteau deleted the DO-NOT-MERGE/APPEALS-26093-APPEALS-29103-APPEALS-26725-provoke-rails-6-0-deprecation-warnings-in-test branch December 4, 2023 21:34
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.

1 participant