-
Notifications
You must be signed in to change notification settings - Fork 18
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-28925-33581 - Update to Rails 6.1 & Replace 'multiverse' with standard Rails multi-DB configuration #22181
Closed
jcroteau
wants to merge
59
commits into
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
from
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
Closed
jcroteau/APPEALS-28925-33581 - Update to Rails 6.1 & Replace 'multiverse' with standard Rails multi-DB configuration #22181
jcroteau
wants to merge
59
commits into
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
from
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
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
This was referenced Jul 15, 2024
jcroteau
commented
Jul 24, 2024
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
July 26, 2024 17:14
39ba072
to
be7a4c7
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
branch
from
July 26, 2024 17:14
37e7330
to
e45190a
Compare
This was referenced Jul 26, 2024
Merged
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
August 1, 2024 20:20
be7a4c7
to
130791a
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
branch
from
August 1, 2024 20:25
e45190a
to
ee12568
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
August 1, 2024 20:26
130791a
to
ccbe047
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
August 2, 2024 14:47
ccbe047
to
dad1ca8
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
branch
from
August 2, 2024 14:51
ee12568
to
2dd28e3
Compare
1 task
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
August 13, 2024 15:47
dad1ca8
to
c136a12
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
branch
from
August 13, 2024 15:48
2dd28e3
to
8a4d928
Compare
This was referenced Aug 13, 2024
jcroteau
force-pushed
the
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
branch
from
August 13, 2024 16:23
8a4d928
to
ba76458
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
August 13, 2024 16:23
c136a12
to
754d700
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
branch
from
August 16, 2024 15:38
ba76458
to
0039454
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
August 16, 2024 15:38
754d700
to
5b091e3
Compare
1 task
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
August 27, 2024 14:50
5b091e3
to
42e016d
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
branch
from
August 27, 2024 14:51
0039454
to
4755288
Compare
1 task
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
September 6, 2024 15:51
42e016d
to
41aa32a
Compare
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.
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.
Task `db:schema:load` already loads the checked in schema, so there should be no need to run `db:migrate` afterwards.
Addresses the following error: NoMethodError: undefined method `build_lookup_context' for ActionView::Base:Class
…Error` Addresses the following error: FrozenError: can't modify frozen Hash: {}
…valid` Apparently, there were some changes to the inner workings of `ActiveModel::Errors` in Rails 6.1, causing a model to be considered invalid in the case that `errors[:base] == [[]]`. This makes sense, as `[[]]` is not considered "empty". Unfortunately, this was causing `RoSchedulePeriod #validate_spreadsheet` to inadvertently mark the model as invalid upon creation. `HearingSchedule::ValidateRoSpreadsheet #validate` returns an empty array (`[]`) when valid, which gets pushes onto the `RoSchedulePeriod` `errors[:base]` array, resulting in a non-empty array (`[[]]`) and an erroneously invalid disposition. Furthermore, calling `<<` to an `ActiveModel::Errors` message array in order to add an error is a deprecated, so we can take this opportunity to use the new `#add` API to hit two birds with one stone. The change implemented here is not a pure refactoring, however the end-user experience is unchanged in terms of how errors are presented when attempting to upload a spreadsheet with multiple non-conformities. Down the road, we may want to consider moving `HearingSchedule::ValidateRoSpreadsheet` toward using `ActiveModel::Validations` in order to leverage the full `ActiveModel::Errors` API and construct the errors object in the prescribed manner. For more details see - https://api.rubyonrails.org/v6.1.7.7/classes/ActiveModel/Validations.html - https://api.rubyonrails.org/v6.1.7.7/classes/ActiveModel/Errors.html
Addresses failures such as the below: 0) AMA Cases Tableau data source expected report calculates age and AOD based on person.dob Failure/Error: expect(aod_case["aod_veteran.age"]).to eq("76") expected: "76" got: 0.76e2
Test helper method `#perform_enqueued_jobs` now wraps exceptions in an `Minitest::UnexpectedError`: https://github.com/rails/rails/blob/914caca2d31bd753f47f9168f2a375921d9e91cc/activejob/lib/active_job/test_helper.rb#L591 So, to assert that a specific exception is raised during execution of the `#perform_enqueued_jobs` block, we must rescue the `Minitest::UnexpectedError` and make the assertion on its error message instead.
In Rails 6.1, `ActiveJob #perform_now` was changed to behave as it did once before (at the behest of GitHub), returning the value fo the job instead of true/false. See related GH issue: rails/rails#38040
Addresses error: NoMethodError: undefined method `workflow' for #<CaseSearchResultsForVeteranFileNumber:0x00007f9a030966c8> 0) AppealsController GET appeals when current user is a System Admin when request header does not contain Veteran ID responds with an error Failure/Error: errors: errors.messages[:workflow], NoMethodError: undefined method `workflow' for #<CaseSearchResultsForVeteranFileNumber:0x00007f9a030966c8> # ./app/workflows/case_search_results_base.rb:31:in `search_call'
Addresses test failures in `spec/controllers/appeals_controller_spec.rb` similar to the below: AppealsController GET appeals when current user is a System Admin when request header does not contain Veteran ID responds with an error Failure/Error: expect(response_body["errors"][0]["title"]).to eq "Veteran file number missing" expected: "Veteran file number missing" got: nil Using `ActiveModel::Errors` to store error data in an arbitrary format may have been somewhat permissible in the past, but it is an abuse of the object's intended use and is also proving incompatible with the more formalized `ActiveModels::Errors` API in Rails 6.1. In order to preserve the existing response shape of the affected JSON endpoints, we need to move away from the `ActiveModel::Validations` implementation on `CaseSearchResultsBase` (and its descendent classes) to a more bespoke method of performing validations and aggregating errors, since Rails 6.1 `ActiveModel::Errors` is no longer appropriate for our needs here.
…rol error Addresses the test failure below: ApplicationController no cache headers when toggle set sets Cache-Control etc Failure/Error: expect(response.headers["Cache-Control"]).to eq "no-cache, no-store" expected: "no-cache, no-store" got: "no-store" (compared using ==) # ./spec/controllers/application_controller_spec.rb:59:in `block (4 levels) in <top (required)>' In Rails 6.1, the `no-store` directive is exclusive of any others that are set on the `Cache-Control` header, which makes sense given the specification https://datatracker.ietf.org/doc/html/rfc7234#section-3 This change was implemented in PR rails/rails#39461 Since it no longer makese sense to set both `no-store` and `no-cache` directives, we will only set `no-store` here, as that is the stronger of the two.
Addresses multiple test failures caused by the error below: QueueConfig.to_hash title when assigned to an org is formatted as expected Failure/Error: tasks.with_assignees.group("assignees.display_name").count(:all).each_pair.map do |option, count| label = self.class.format_option_label(option, count) self.class.filter_option_hash(option, label) end ActiveRecord::EagerLoadPolymorphicError: Cannot eagerly load the polymorphic association :appeal # ./app/models/queue_column.rb:110:in `assignee_options'
In Rails 6.1.7.7, the method `ActiveRecord::Relation #update_all` will now clear any records cached by the calling relation. This was altering the behavior of `Task #cancel_task_and_child_subtasks` and causing the following test failure: Task#cancel_task_and_child_subtasks cancels all tasks and child subtasks Failure/Error: expect(second_level_tasks[0].versions.count).to eq(initial_versions + 2) expected: 3 got: 2 (compared using ==) # ./spec/models/task_spec.rb:368:in `block (3 levels) in <top (required)>' To remedy, we will now cache the necessary Task records in an Array, which can be used for generating PaperTrail versions both before and after the `update_all`.
…ndering error Addresses the following test failure: Hearings::CalendarService.confirmation_calendar_invite returns appropriate iCalendar event Failure/Error: expect(ical_event.description).to eq(expected_description) expected: "You're scheduled for a virtual hearing with a Veterans Law Judge of the Board of Veterans' Appeals.\...to reschedule or cancel your virtual hearing, contact us by email at bvahearingteamhotline@va.gov\n" got: #<Icalendar::Values::Text("You're scheduled for a virtual hearing with a Veterans Law Judge of the Bo... reschedule or cancel your virtual hearing, contact us by email at bvahearingteamhotline@va.gov\n")>
Address the following error, found during demo deployment: rake aborted! Cannot load database configuration: YAML syntax error occurred while parsing /caseflow/config/database.yml. Please note that YAML must be consistently indented using spaces. Tabs are not allowed. Error: (<unknown>): could not find expected ':' while scanning a simple key at line 49 column 5
Removes `bourbon` and `neat` dependencies.
jcroteau
force-pushed
the
jcroteau/APPEALS-28925-33581-update-to-rails-6-1-and-sunset-multiverse
branch
from
September 12, 2024 14:31
933d6a2
to
ed450ad
Compare
jcroteau
force-pushed
the
jcroteau/APPEALS-29105-fix-deprecation-controller-level-force_ssl
branch
from
September 12, 2024 14:31
efecfa9
to
d6bf3f0
Compare
This was referenced Sep 12, 2024
Changes to be released in #22780 |
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.
This PR is part of a 4 PR stack:
Caseflow::Migration
for concurrent index migrations on ActiveRecord 6.0+ #21336force_ssl
is deprecated and will be removed from Rails 6.1 (dev) #21453Resolves:
6.1.Z
Note: It was necessary to combine the changesets for both of the above stories into a single PR, as there is a co-dependency between them.
APPEALS-33581 encompasses the replacement of the
multiverse
gem with Rails' native multi-DB configuration (asmultiverse
was not compatible with Rails 6.1). One of the consequences of this transition was a change in behavior of many often-used database Rake tasks. In order to preserve the existing behavior of our DB-relatedmake
commands and other workflows, we needed to replace the invocations of these database Rake tasks with their new, database-specific counterparts. The difficulty in this was that some of the new DB tasks needed would not be available until Rails 6.1, thus forcing us to combine the work with that of APPEALS-28925, the Rails 6.1 upgrade.Test Plans: