-
Notifications
You must be signed in to change notification settings - Fork 19
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
feature/APPEALS-34124-43428-29105-28925-33581 - Rails 6.1 upgrade #22390
feature/APPEALS-34124-43428-29105-28925-33581 - Rails 6.1 upgrade #22390
Commits on Aug 1, 2024
-
🔧 Assume defaults for `config.action_dispatch.use_cookies_with_metada…
…ta` and `config.action_mailer.delivery_job` The following config settings are not backwards compatible: - config.action_dispatch.use_cookies_with_metadata - config.action_mailer.delivery_job Now that Rails 6.0 is stable on production, we can assume their default values going forward.
Configuration menu - View commit details
-
Copy full SHA for d63f844 - Browse repository at this point
Copy the full SHA d63f844View commit details -
Configuration menu - View commit details
-
Copy full SHA for 4a4784d - Browse repository at this point
Copy the full SHA 4a4784dView commit details -
🔧 Assume default for `config.action_dispatch.use_authenticated_cookie…
…_encryption` Since we are making other cookie configuration changes in this PR for Rails 6.0, this is an opportune time to migrate this Rails 5.2 cookie setting to its default value as well.
Configuration menu - View commit details
-
Copy full SHA for a3cf2c1 - Browse repository at this point
Copy the full SHA a3cf2c1View commit details -
✨ Add new utility module for adding DB indexes concurrently
Introduces `Caseflow::Migrations::AddIndexConcurrently` as a replacement for `Caseflow::Migration` for migrations on ActiveRecord 6.0 and beyond, since `Caseflow::Migration` is forever coupled to ActiveRecord 5.1 due to its extensive use on legacy migrations and should be deprecated moving forward.
Configuration menu - View commit details
-
Copy full SHA for ac74fd0 - Browse repository at this point
Copy the full SHA ac74fd0View commit details -
Configuration menu - View commit details
-
Copy full SHA for 99d622d - Browse repository at this point
Copy the full SHA 99d622dView commit details -
Configuration menu - View commit details
-
Copy full SHA for 81f7599 - Browse repository at this point
Copy the full SHA 81f7599View commit details -
Configuration menu - View commit details
-
Copy full SHA for 74e8f40 - Browse repository at this point
Copy the full SHA 74e8f40View commit details -
✨ Introduce
SslRedirectExclusionPolicy
To be used in the environment configuration settings for excluding exempt request paths from SSL redirects when `config. force_ssl = true`
Configuration menu - View commit details
-
Copy full SHA for a6b497e - Browse repository at this point
Copy the full SHA a6b497eView commit details
Commits on Aug 2, 2024
-
♻️ Replace deprecated controller-level
force_ssl
Replace deprecated controller-level `force_ssl` with equivalent configuration settings in preparation for the Rails 6.1 upgrade.
Configuration menu - View commit details
-
Copy full SHA for dad1ca8 - Browse repository at this point
Copy the full SHA dad1ca8View commit details -
🔥 Remove deprecated config setting `config.active_record.sqlite3.repr…
…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).
Configuration menu - View commit details
-
Copy full SHA for 2e70f55 - Browse repository at this point
Copy the full SHA 2e70f55View commit details -
⬆️ Update
caseflow-commons
to resolve sub-dependency conflictsRemoves unneeded gems `bourbon` and `neat`, which had a sub-dependency conflict on `thor`.
Configuration menu - View commit details
-
Copy full SHA for 05328a0 - Browse repository at this point
Copy the full SHA 05328a0View commit details -
Configuration menu - View commit details
-
Copy full SHA for d7af53f - Browse repository at this point
Copy the full SHA d7af53fView commit details -
Configuration menu - View commit details
-
Copy full SHA for 11a7090 - Browse repository at this point
Copy the full SHA 11a7090View commit details -
Configuration menu - View commit details
-
Copy full SHA for 796906d - Browse repository at this point
Copy the full SHA 796906dView commit details -
Configuration menu - View commit details
-
Copy full SHA for 0a10c68 - Browse repository at this point
Copy the full SHA 0a10c68View commit details -
🔧 Assume default for
config.active_storage.track_variants
We're not currently using ActiveStorage in Caseflow, so it is safe to just assume the default here.
Configuration menu - View commit details
-
Copy full SHA for a150cbc - Browse repository at this point
Copy the full SHA a150cbcView commit details -
🔧 Override default for
config.active_job.retry_jitter
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.
Configuration menu - View commit details
-
Copy full SHA for 01c4c7f - Browse repository at this point
Copy the full SHA 01c4c7fView commit details -
🔧 Assume default for `config.active_job.skip_after_callbacks_if_termi…
…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
Configuration menu - View commit details
-
Copy full SHA for 4b26560 - Browse repository at this point
Copy the full SHA 4b26560View commit details -
🔧 Assume default for `config.action_dispatch.cookies_same_site_protec…
…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
Configuration menu - View commit details
-
Copy full SHA for 2d9ca4f - Browse repository at this point
Copy the full SHA 2d9ca4fView commit details -
Configuration menu - View commit details
-
Copy full SHA for afba287 - Browse repository at this point
Copy the full SHA afba287View commit details -
🔧 Assume default for `ActiveSupport.utc_to_local_returns_utc_offset_t…
…imes` We're not using `ActiveSupport::TimeZone.utc_to_local` anywhere, so the default is safe to assume here.
Configuration menu - View commit details
-
Copy full SHA for bd7c056 - Browse repository at this point
Copy the full SHA bd7c056View commit details -
🔧 Assume default for `config.action_dispatch.ssl_default_redirect_sta…
…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
Configuration menu - View commit details
-
Copy full SHA for 3fec0f8 - Browse repository at this point
Copy the full SHA 3fec0f8View commit details -
🔧 Assume default for
config.active_record.legacy_connection_handling
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
Configuration menu - View commit details
-
Copy full SHA for ff7817b - Browse repository at this point
Copy the full SHA ff7817bView commit details -
🔧 Assume default for `config.action_view.form_with_generates_remote_f…
…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.
Configuration menu - View commit details
-
Copy full SHA for 2172a01 - Browse repository at this point
Copy the full SHA 2172a01View commit details -
🔧 Assume default for
config.active_storage.queues.analysis
We do not use ActiveStorage, so the default is safe to assume here.
Configuration menu - View commit details
-
Copy full SHA for a4ccebc - Browse repository at this point
Copy the full SHA a4ccebcView commit details -
🔧 Assume default for
config.active_storage.queues.purge
We do not use ActiveStorage, so the default is safe to assume here.
Configuration menu - View commit details
-
Copy full SHA for 79744a9 - Browse repository at this point
Copy the full SHA 79744a9View commit details -
🔧 Assume default for
config.action_mailbox.queues.incineration
We don't use ActionMailbox, so the new default is safe to assume here.
Configuration menu - View commit details
-
Copy full SHA for 724977c - Browse repository at this point
Copy the full SHA 724977cView commit details -
🔧 Assume default for
config.action_mailbox.queues.routing
We do not use ActionMailbox, so the default is safe to assume here.
Configuration menu - View commit details
-
Copy full SHA for 3919279 - Browse repository at this point
Copy the full SHA 3919279View commit details -
🔧 Assume default for
config.action_mailer.deliver_later_queue_name
We're not using `ActionMailer::MessageDelivery #deliver_later` anywhere, so the default is safe to assume.
Configuration menu - View commit details
-
Copy full SHA for c633a81 - Browse repository at this point
Copy the full SHA c633a81View commit details -
🔧 Assume default for
config.action_view.preload_links_header
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
Configuration menu - View commit details
-
Copy full SHA for d4232e9 - Browse repository at this point
Copy the full SHA d4232e9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 0c8b0e7 - Browse repository at this point
Copy the full SHA 0c8b0e7View commit details -
Configuration menu - View commit details
-
Copy full SHA for 39eb586 - Browse repository at this point
Copy the full SHA 39eb586View commit details -
Configuration menu - View commit details
-
Copy full SHA for e0b4876 - Browse repository at this point
Copy the full SHA e0b4876View commit details -
Configuration menu - View commit details
-
Copy full SHA for c663a54 - Browse repository at this point
Copy the full SHA c663a54View commit details -
Configuration menu - View commit details
-
Copy full SHA for db45ccb - Browse repository at this point
Copy the full SHA db45ccbView commit details -
🩹 Add forgotten disallowed deprecation warning
This deprecation warning was addressed by the following PR, but we forgot to add it to the list of disallowed deprecation warnings: #21614
Configuration menu - View commit details
-
Copy full SHA for bf66f8c - Browse repository at this point
Copy the full SHA bf66f8cView commit details -
Task `rake routes` has been replaced with `rails routes`
Configuration menu - View commit details
-
Copy full SHA for 60fcbd1 - Browse repository at this point
Copy the full SHA 60fcbd1View commit details -
✅ Update test to account for change to `ActionDispatch::Response#cont…
…ent_type` `ActionDispatch::Response#content_type` now returns the full Content-Type header
Configuration menu - View commit details
-
Copy full SHA for 1ec0b3e - Browse repository at this point
Copy the full SHA 1ec0b3eView commit details -
Configuration menu - View commit details
-
Copy full SHA for 1a4edc5 - Browse repository at this point
Copy the full SHA 1a4edc5View commit details -
Configuration menu - View commit details
-
Copy full SHA for 296e5dd - Browse repository at this point
Copy the full SHA 296e5ddView commit details -
Configuration menu - View commit details
-
Copy full SHA for 99383fe - Browse repository at this point
Copy the full SHA 99383feView commit details -
♻️ Arrange 'database.yml' configs by environment
Group DB configs by environment in anticipation of reformatting for Rails 6+ multi-DB configuration.
Configuration menu - View commit details
-
Copy full SHA for 66910c9 - Browse repository at this point
Copy the full SHA 66910c9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 4a4ce21 - Browse repository at this point
Copy the full SHA 4a4ce21View commit details -
Configuration menu - View commit details
-
Copy full SHA for c84ec99 - Browse repository at this point
Copy the full SHA c84ec99View commit details -
Configuration menu - View commit details
-
Copy full SHA for 1767c7c - Browse repository at this point
Copy the full SHA 1767c7cView commit details -
♻️ Use new database-specific rake tasks
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.
Configuration menu - View commit details
-
Copy full SHA for 5387433 - Browse repository at this point
Copy the full SHA 5387433View commit details -
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.
Configuration menu - View commit details
-
Copy full SHA for 30a5895 - Browse repository at this point
Copy the full SHA 30a5895View commit details -
🗃️ Prohibit execution of vacols DB and non-DB-specific rake tasks
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.
Configuration menu - View commit details
-
Copy full SHA for 4d348c5 - Browse repository at this point
Copy the full SHA 4d348c5View commit details -
♻️ Create new environment for GH workflow 'Make-docs-to-webpage'
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.
Configuration menu - View commit details
-
Copy full SHA for 75a92ae - Browse repository at this point
Copy the full SHA 75a92aeView commit details -
💚 Remove redundant DB migrations from CI workflow
Task `db:schema:load` already loads the checked in schema, so there should be no need to run `db:migrate` afterwards.
Configuration menu - View commit details
-
Copy full SHA for 4910804 - Browse repository at this point
Copy the full SHA 4910804View commit details -
🐛 Fix
spec/mailers/hearing_mailer_spec.rb
-NoMethodError
Addresses the following error: NoMethodError: undefined method `build_lookup_context' for ActionView::Base:Class
Configuration menu - View commit details
-
Copy full SHA for a463506 - Browse repository at this point
Copy the full SHA a463506View commit details -
🐛 Fix
spec/workflows/post_decision_motion_updater_spec.rb
- `Frozen……Error` Addresses the following error: FrozenError: can't modify frozen Hash: {}
Configuration menu - View commit details
-
Copy full SHA for 8ce0fdc - Browse repository at this point
Copy the full SHA 8ce0fdcView commit details -
Configuration menu - View commit details
-
Copy full SHA for bc89485 - Browse repository at this point
Copy the full SHA bc89485View commit details -
🐛 Fix
spec/models/schedule_period_spec.rb
- `ActiveRecord::RecordIn……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
Configuration menu - View commit details
-
Copy full SHA for 70c0a74 - Browse repository at this point
Copy the full SHA 70c0a74View commit details -
Configuration menu - View commit details
-
Copy full SHA for ed43ef4 - Browse repository at this point
Copy the full SHA ed43ef4View commit details -
Configuration menu - View commit details
-
Copy full SHA for d61b146 - Browse repository at this point
Copy the full SHA d61b146View commit details -
✅ Fix
spec/sql/ama_cases_sql_spec.rb
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
Configuration menu - View commit details
-
Copy full SHA for 9cbd0a0 - Browse repository at this point
Copy the full SHA 9cbd0a0View commit details -
✅ Fix multiple specs -
Minitest::UnexpectedError
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.
Configuration menu - View commit details
-
Copy full SHA for 6c800a0 - Browse repository at this point
Copy the full SHA 6c800a0View commit details -
Configuration menu - View commit details
-
Copy full SHA for 9c1830a - Browse repository at this point
Copy the full SHA 9c1830aView commit details -
✅ Fix
spec/controllers/api/v1/jobs_controller_spec.rb
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
Configuration menu - View commit details
-
Copy full SHA for b899a2f - Browse repository at this point
Copy the full SHA b899a2fView commit details -
🐛 Fix
spec/controllers/appeals_controller_spec.rb
-NoMethodError
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'
Configuration menu - View commit details
-
Copy full SHA for 6fdaf6b - Browse repository at this point
Copy the full SHA 6fdaf6bView commit details -
🐛 Fix
CaseSearchResultsBase
validationsAddresses 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.
Configuration menu - View commit details
-
Copy full SHA for aee0b03 - Browse repository at this point
Copy the full SHA aee0b03View commit details -
✅ Fix
spec/controllers/application_controller_spec.rb
-- Cache-Cont……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.
Configuration menu - View commit details
-
Copy full SHA for 6c1ccbd - Browse repository at this point
Copy the full SHA 6c1ccbdView commit details -
🐛 Fix multiple specs -
ActiveRecord::EagerLoadPolymorphicError
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'
Configuration menu - View commit details
-
Copy full SHA for 8661a26 - Browse repository at this point
Copy the full SHA 8661a26View commit details -
🐛 Fix
spec/models/task_spec.rb
-update_all
clears query cacheIn 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`.
Configuration menu - View commit details
-
Copy full SHA for 1c12507 - Browse repository at this point
Copy the full SHA 1c12507View commit details -
🐛 Fix
spec/services/hearings/calendar_service_spec.rb
- template re……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")>
Configuration menu - View commit details
-
Copy full SHA for 237c3f3 - Browse repository at this point
Copy the full SHA 237c3f3View commit details -
🐛 Fix YAML syntax error caused by whitespace in ENV var
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
Configuration menu - View commit details
-
Copy full SHA for 74525fd - Browse repository at this point
Copy the full SHA 74525fdView commit details -
⬆️ Update
caseflow-commons
dependency to latest refRemoves `bourbon` and `neat` dependencies.
Configuration menu - View commit details
-
Copy full SHA for 2dd28e3 - Browse repository at this point
Copy the full SHA 2dd28e3View commit details