Update to Rails 6.1 #1556
Update to Rails 6.1 #1556
Conversation
This commit *begins* an update from Rails 6.0.X -> 6.1.X. This requires a large number of simultaneous library updates (some took a while to identify). This does *not* complete the update. The biggest problem is the many error reports of this form: > Error: > ProjectsControllerTest#test_should_fail_to_create_project_with_duplicate_repo: > ActiveRecord::StatementInvalid: PG::UndefinedColumn: > ERROR: column users.email does not exist It's true that there is no users.email column in the database, but that's because it's a virtual column that is *supposed* to be managed by the `attr_encrypted` gem. Since it's not being handled, it *appears* that this gem does not work with ActiveRecord 6.1. I went to check out its status, and they are looking for new maintainers: attr-encrypted/attr_encrypted#379 All options are not the desired ones here. In addition, there are at least two kinds of deprecation warnings which will need to be addressed (probably many times): * DEPRECATION WARNING: action_view.raise_on_missing_translations is deprecated and will be removed in Rails 6.2. Set i18n.raise_on_missing_translations instead. Note that this new setting also affects how missing translations are handled in controllers. (called from call at /home/dwheeler/best-practices-badge/config/initializers/canonical_trailing_slash.rb:30) * DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: static_pages/error_404.html.erb (called from error_404 at /home/dwheeler/best-practices-badge/app/controllers/static_pages_controller.rb:46) Signed-off-by: David A. Wheeler <[email protected]>
david-a-wheeler
changed the title
|
Do not merge yet. I just wanted to show public progress. There are some nontrivial problems that need to be addressed. |
|
@jdossett (and anyone else) - good news, it's a little awkward, but a lot of the Rails 6.0->6.1 conversion can be done by simultaneously upgrading a lot of other libraries. The deprecation warnings are a pain, but should be straightforward to fix. HOWEVER: the fact that the attr_encrypted gem doesn't seem to work any more is a big potential problem :-(. It's a single parameter, so I might be able to just hand-define a getter & setter for "email" (which is really what attr_encrypted does). But it's more work that we didn't have to hand-code before. Sigh. |
Update gem blind_index (0.3.4->2.2.0) with a tweak to how it's called, and tweak tests so that the tests at least run and pass. There are many deprecation warnings (that's next). Signed-off-by: David A. Wheeler <[email protected]>
Signed-off-by: David A. Wheeler <[email protected]>
VCR sometimes gets called in ways that end up invoking the chromedriver checks, even when we're not doing system tests. Move the configuration so that we *NEVER* capture version tests that have nothing to do with the software under test (SUT). Signed-off-by: David A. Wheeler <[email protected]>
Signed-off-by: David A. Wheeler <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1556 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 53 53
Lines 1926 1926
=========================================
Hits 1926 1926
Continue to review full report at Codecov.
|
|
@jdossett - HALLELUJAH! This looks like a working transition to Rails 6.1. Comments welcome. |
david-a-wheeler
changed the title
This updates gem web-console, an in-browser debugger. Insert <% console %> or console into the source code to use it. This gem has been transferred to be part of the "rails" project. Note that we only include this gem during development, *not* in production. This gem enables running arbitrary code, which would be a security problem if it was included in production. But we do not call it from production code, and we don't include the gem in production mode, preventing a security issue. Signed-off-by: David A. Wheeler <[email protected]>
|
Clarification: the gem attr_encrypted works just fine. The problem was that I needed to update the gem blind_index, but the error messages misled me to blame the wrong gem. All's okay now. |
|
Wow @david-a-wheeler, great work! |
|
@jdossett - thanks!! What a pain, but success feels sweet. Any comments before merging? |
david-a-wheeler
changed the title
|
I have found a problem. The updated gem This will need to be fixed before merging. One solution is to regenerate all the hashes (e.g., as part of a migration). However, there are over 4,000 such accounts, so doing that all at once during a migration will take a little time. That may be the easy way though. |
The sessions controller uses the standard method names, but it might not be obvious what they mean in the context of the sessions controller. This adds comments to clarify that. Signed-off-by: David A. Wheeler <[email protected]>
The update to the blind_index gem requires a number of changes no matter what. This commit switches the application to the newer blind_index naming conventions and algorithm (argon2id), requiring a migration to do it and changes to the test fixture for user data. This commit was more work than expected. Calculating the bidx values doesn't work correctly from the fixtures within the ERB (the calls *run* but don't produce the right values, presumably because it's not fully set up at that point). This was resolved by inserting the calculated values into the fixtures, which also makes the tests more independent of the system being tested. One of the older test values was subtly wrong (.com vs. .org email address), which created a mysterious hard-to-track failure (we prevent future versions of this problem via a new test that sanity-checks all test values). The migration had to be tweaked several times to work with real production data. We need to not change the user updated_at by setting touch to false (the user-visible data isn't changing), we need not validate (some entries may not be acceptable any more but we still need to migrate them as they are), and we need to skip cases where encrypted_email is nil (in those cases we have no data to decrypt). Signed-off-by: David A. Wheeler <[email protected]>
|
Regenerating all the hashes doesn't take long at all, it's about a minute. However, there were a crazy number of little details that needed to be worked out before I had a finished solution. I think I've got it. |
david-a-wheeler
changed the title
|
@jdossett or anyone else, please let me know if you have any concerns about merging this. I think it's all well. The tests all pass. I did a trial conversion of the real database & that seemed to work. |
|
I hear no complaints, and I see no reason to complain. I'm finally merging it in! |
This commit begins an update from Rails 6.0.X -> 6.1.X.
This requires a large number of simultaneous library updates
(some took a while to identify).
This does not complete the update.
The biggest problem is the many error reports of this form:
It's true that there is no users.email column in the database,
but that's because it's a virtual column that is supposed to be
managed by the
attr_encryptedgem.Since it's not being handled, it appears that this gem does not
work with ActiveRecord 6.1. I went to check out its status, and they
are looking for new maintainers:
attr-encrypted/attr_encrypted#379
All options are not the desired ones here.
In addition, there are at least two kinds of deprecation warnings
which will need to be addressed (probably many times):
DEPRECATION WARNING: action_view.raise_on_missing_translations is deprecated and will be removed in Rails 6.2. Set i18n.raise_on_missing_translations instead. Note that this new setting also affects how missing translations are handled in controllers. (called from call at /home/dwheeler/best-practices-badge/config/initializers/canonical_trailing_slash.rb:30)
DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: static_pages/error_404.html.erb (called from error_404 at /home/dwheeler/best-practices-badge/app/controllers/static_pages_controller.rb:46)
Signed-off-by: David A. Wheeler [email protected]