Upgrade to Rails 6.1 to prepare to replace secure_headers gem (LG-3832)#4544
Upgrade to Rails 6.1 to prepare to replace secure_headers gem (LG-3832)#4544mitchellhenke merged 5 commits intomasterfrom
Conversation
| require_relative 'boot' | ||
| require 'rails/all' | ||
|
|
||
| require 'active_record/railtie' |
There was a problem hiding this comment.
Was getting errors related to active storage not being configured when it was required by rails/all. We don't use it (or some of the other things like action cable), so I've only included the ones we use: https://guides.rubyonrails.org/initialization.html#railties-lib-rails-all-rb
There was a problem hiding this comment.
Big fan of explicit requires like this!
|
|
||
| # We can enable this once we know we're not rolling back from Rails 6 | ||
| # https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#purpose-in-signed-or-encrypted-cookie-is-now-embedded-within-cookies | ||
| config.action_dispatch.use_cookies_with_metadata = false |
There was a problem hiding this comment.
We've had a couple releases on Rails 6, so it's probably safe to assume we aren't rolling back to 5.2
| }, | ||
| "dependencies": { | ||
| "basscss-sass": "^3.0.0", | ||
| "@babel/core": "^7.12.10", |
There was a problem hiding this comment.
These deps were moved from dev to regular dependencies as I was having issues building on production (command "webpack" not found). Since we build our assets in production environments, these aren't strictly development dependencies.
It looks like webpacker has also moved these from devDependencies for similar reasons: rails/webpacker#137
cc @aduth
There was a problem hiding this comment.
These deps were moved from dev to regular dependencies as I was having issues building on production (command "webpack" not found). Since we build our assets in production environments, these aren't strictly development dependencies.
It looks like webpacker has also moved these from
devDependenciesfor similar reasons: rails/webpacker#137cc @aduth
Seems okay 👍 We had similar need in GSA-TTS/identity-site#468 (comment)
| get :index | ||
|
|
||
| expect(response.headers['Cache-Control']).to eq 'private, no-store' | ||
| expect(response.headers['Cache-Control']).to eq 'no-store' |
There was a problem hiding this comment.
Since private with no-store is superfluous, the private segment was removed: rails/rails#39461
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control says:
private
The response may be stored only by a browser's cache, even if the response is normally non-cacheable. If you mean to not store the response in any cache, use no-store instead. This directive is not effective in preventing caches from storing your response.
| PhoneConfigurationDecorator = Struct.new(:phone_configuration) do | ||
| def default_number_message | ||
| I18n.t('account.index.default') if | ||
| phone_configuration == phone_configuration.user.default_phone_configuration |
There was a problem hiding this comment.
Bullet was failing due to this being an N+1 query from loading all of the phone configurations and calling .user on each one. That led me down a rabbit hole of simplifying the view to do the logic directly:
<% if current_user.default_phone_configuration == phone_configuration %>
<%= I18n.t('account.index.default') %>
<% end %>When looking into other uses of the struct, I noticed it was being used when rendering PIV/CACs and authentication apps in a similar manner to check if it's the default. An authentication app or PIV configuration will never equal the user's default phone configuration though, so I removed those uses, and the class was no longer being used.
3 participants
Upgrades to Rails 6.1 to be able to use the new CSP and cookie functionality that will be used to replace secure_headers
config.i18n.raise_on_missing_translations = trueuncovered a few places in specs where we were referencing translations that no longer existed. Some were due to renamed keys, but others didn't seem to be applicable and I removed the specs.