feature/APPEALS-28925-33581 - Update to Rails 6.1 & Replace 'multiverse' with standard Rails multi-DB configuration #21736
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…esent_boolean_as_integer` This will have no implications for Caseflow, since we are only using the `sqlite3` adapter nominally for the `demo_vacols` database, which is not actually being used in our demo environments (demo environments are deployed as `development` envs).
Removes unneeded gems `bourbon` and `neat`, which were causing a sub-dependency conflict on `thor`.
The default value is safe to assume here. There is a possible edge-case for recursive associations having the same name as the model, but we do not have any such associations in Caseflow. For more background, see https://lilyreile.medium.com/rails-6-1-new-framework-defaults-what-they-do-and-how-to-safely-uncomment-them-c546b70f0c5e#9072
We're not currently using ActiveStorage in Caseflow, so it is safe to just assume the default here.
The default jitter is probably safe, however, I'm not 100% sure that we don't have any jobs that need to be requeued with exact wait times. So we let's override this for now to stay on the safe side.
…nated` We're not currently using `throw :abort` within any `before_enqueue`/`before_perform` callbacks on existing Caseflow jobs, so the default should be fine here. For more background, see https://lilyreile.medium.com/rails-6-1-new-framework-defaults-what-they-do-and-how-to-safely-uncomment-them-c546b70f0c5e#4c60
…tion` This setting controls the `SameSite` optional attribute for the `Set-Cookie` header. `SameSite=Lax` means that the cookie is not sent on cross-site requests, such as on requests to load images or frames, but is sent when a user is navigating to the origin site from an external site (for example, when following a link). This is the default behavior if the SameSite attribute is not specified. `Lax` is currently the default assumed by both Chrome and Edge browsers when this attribute is left unspecified, so assuming this value should be sensible. It allows us to have our cake (blocking CSRF attacks) and eat it too (providing a logged-in experience when users navigate to Caseflow across origins). For more background, see - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#samesitesamesite-value - https://lilyreile.medium.com/rails-6-1-new-framework-defaults-what-they-do-and-how-to-safely-uncomment-them-c546b70f0c5e#1f15
…imes` We're not using `ActiveSupport::TimeZone.utc_to_local` anywhere, so the default is safe to assume here.
…tus` The default is safe to assume. For more background, see https://lilyreile.medium.com/rails-6-1-new-framework-defaults-what-they-do-and-how-to-safely-uncomment-them-c546b70f0c5e#4c3e
The default should be safe to assume here, as we do not do any role or shard switching on database connections. For more background, see https://lilyreile.medium.com/rails-6-1-new-framework-defaults-what-they-do-and-how-to-safely-uncomment-them-c546b70f0c5e#8007
…orms` We don't use the `form_with` helper anywhere, so this behavior change is inconsequential for us, and we can safely assume the new default.
We do not use ActiveStorage, so the default is safe to assume here.
We do not use ActiveStorage, so the default is safe to assume here.
We don't use ActionMailbox, so the new default is safe to assume here.
We do not use ActionMailbox, so the default is safe to assume here.
We're not using `ActionMailer::MessageDelivery #deliver_later` anywhere, so the default is safe to assume.
This flag can be safely uncommented. Browsers that support Link headers will get a performance boost. Browsers that don’t will ignore them. We override in `development` environments to avoid an edge case leading to an HTTP response header overflow. For more background, see https://lilyreile.medium.com/rails-6-1-new-framework-defaults-what-they-do-and-how-to-safely-uncomment-them-c546b70f0c5e#3679
This deprecation warning was addressed by the following PR, but we forgot to add it to the list of disallowed deprecation warnings: #21614
Task `rake routes` has been replaced with `rails routes`
…ent_type` `ActionDispatch::Response#content_type` now returns the full Content-Type header
Group DB configs by environment in anticipation of reformatting for Rails 6+ multi-DB configuration.
After migrating to the Rails 6+ native multi-database configuration, the behavior of some DB management tasks, such as `rake db:migrate` changed such that they now act on ALL databases and not just the primary database. So we must replace the invocations of these tasks with their new, database-specific counterparts.
Now that we have fiully transitioned to Rails-native multi-database support, we are no longer reliant on the 'multiverse' gem and can remove it.
After transitioning to Rails-native multi-DB support, the behavior of some DB tasks changed such that they will now act on ALL databases and not just the primary database (ex. `rake db:migrate` will now migrate ALL databases). To avoid accidents, we re-define these tasks here to no-op and output a helpful message to redirect developers toward using their new database-specific counterparts instead.
Explicitly clear any generated database tasks for the vacols DB. Once on Rails 7+, we can instead add the `database_tasks: false` option to the `database.yml` to permit connection to the vacols database without generating database tasks for it: https://github.com/rails/rails/blob/984c3ef2775781d47efa9f541ce570daa2434a80/guides/source/active_record_multiple_databases.md?plain=1#L203-L219
Instead of performing a bunch of hard-to-maintain `sed` gymnastics to modify the existing 'test' environment, let's create a new 'make_docs' environment (based off of 'test') and configure it appropriately for use by the 'Make-docs-to-webpage' GH workflow.
jcroteau
changed the base branch from
master
to
DO-NOT-MERGE/prereqs-for-update-to-rails-6-1
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
jcroteau
changed the base branch from
DO-NOT-MERGE/prereqs-for-update-to-rails-6-1
to
master
jcroteau
changed the base branch from
master
to
DO-NOT-MERGE/prereqs-for-update-to-rails-6-1
|
Code Climate has analyzed commit 38efb92 and detected 6 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
Superseded by #21737 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎗️ Reminder:
Resolves:
6.1.ZNote: It was necessary to combine the changesets for both of the above stories into a single PR, as they are co-dependent in terms of passing the test suite.
TBD