-
Notifications
You must be signed in to change notification settings - Fork 7
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
Bump rails 6.0 to to 6.1 #3753
Merged
Merged
Bump rails 6.0 to to 6.1 #3753
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
f2f1b07
to
b3c6c09
Compare
d952097
to
d953ec9
Compare
f5fb2da
to
dab4a18
Compare
761fb0f
to
ed262ed
Compare
naseberry
reviewed
Jun 21, 2021
naseberry
reviewed
Jun 21, 2021
naseberry
reviewed
Jun 21, 2021
naseberry
reviewed
Jun 21, 2021
e7b6e87
to
cc3b2d8
Compare
jrmhaig
reviewed
Jun 22, 2021
add_foreign_key "claims", "case_stages", name: "fk_claims_case_stage_id" | ||
add_foreign_key "injection_attempts", "claims" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added in this #3940 and I do not know why.
1) TypeError: module attributes should be defined directly on class, not singleton * cattr_accessor must be outside of a `class << self` block 2) NameError: uninitialized constant Rack::NoAnimations * rack/no_animations and rack/no_popups need to be explicitly included in config/environments/test.rb
When running in CircleCI it fails as it is not able to create tmp/pids/server.pid
Model errors have changed in Rails 6.1 and new errors need to be added with: ```ruby errors.add(:attribute, 'Message') ``` instead of: ```ruby errors[:attribute] << 'Message' ``` However, `Allocation#rollback_all_allocations!` prepends a message to an existing list of errors and this cannot be done with `.add` so some behaviour has to be changed.
After fixing `ErrorPresenter.generate_messages` to use Active Model errors correctly for Rails 6.1 it triggered a Metrics/AbcSize Rubocop offense. I have made a small refactor that clears this but more could be done. When iterating over `@errors` there is now just one loop variable representing each error instead of the field name and message, as there was before. This is now passed to `populate_messages` as a single argument instead of three, including an instance of `ErrorMessageTranslator`. Further work could be done to move the logic inside `populate_messages` into `ErrorMessageTranslator` and update `ErrorDetails` so that its initializer can take an instance of `ErrorMessageTranslator` as an argument. This is (probably) outside of the scope of this PR (upgrading to Rails 6.1).
This scenario should not happen and now it raises a `ActiveSupport::MessageVerifier::InvalidSignature` error. Could be related to this new feature in 6.1 https://blog.saeloun.com/2020/05/20/rails-6-1-adds-support-for-signed-ids-to-active-record.html rails/rails#39313 And various issues that were supposedly fixed rails/rails#41233
The same path is returned and more idiomatic Idea from https://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#globs-in-config-autoload-paths
The previous method of mocking an error on the fee object had ceased to work. It is unclear what rails 6.1 change caused the issue. The fix involves actually validating the fee which therefore is more of an integration test.
The first two of these appears to simply be the an ordering of columns issue that may reflect changes in activerecord 6.1 The last deletion of a FK is odd?! ``` - add_foreign_key "injection_attempts", "claims" ```
Handle ``` DEPRECATION WARNING: Enumerating ActiveModel::Errors as a hash has been deprecated. In Rails 6.1, `errors` is an array of Error objects, therefore it should be accessed by a block with a single block parameter like this: person.errors.each do |error| attribute = error.attribute message = error.message end You are passing a block expecting two parameters, so the old hash behavior is simulated. As this is deprecated, this will result in an ArgumentError in Rails 6.2. (called from copy_errors_to_base_record at /home/circleci/repo/app/validators/base_sub_model_validator.rb:54) ``` ...and ``` DEPRECATION WARNING: Calling `<<` to an ActiveModel::Errors message array in order to add an error is deprecated. Please call `ActiveModel::Errors#add` instead. (called from block in copy_errors_to_base_record at /home/circleci/repo/app/validators/base_sub_model_validator.rb:57) ```
Fixes ``` DEPRECATION WARNING: Calling `clear` to an ActiveModel::Errors message array in order to delete all errors is deprecated. Please call `ActiveModel::Errors#delete` instead. (called from clear_pre_existing_error at app/validators/base_validator.rb:85) ```
prevent output which IMO is intended to warn devs that there is a problem with the test rather than whether it is applicable or not.
Fixes ``` DEPRECATION WARNING: ActiveModel::Errors#keys is deprecated and will be removed in Rails 6.2. To achieve the same use: errors.attribute_names (called from block (4 levels) in <top (required)> at spec/validators/fee/base_fee_validator_spec.rb:237) ```
Fixes: ``` DEPRECATION WARNING: Calling `<<` to an ActiveModel::Errors message array in order to add an error is deprecated. Please call `ActiveModel::Errors#add` instead. (called from offence_class_xor_offence_band at /Users/jsugarman/rails_projects/ds_work/cccd/Claim-for-Crown-Court-Defence/app/models/offence.rb:58) (called from roles_valid at /Users/jsugarman/rails_projects/ds_work/cccd/Claim-for-Crown-Court-Defence/app/models/concerns/roles.rb:52) (called from expenses_valid at /Users/jsugarman/rails_projects/ds_work/cccd/Claim-for-Crown-Court-Defence/app/models/determination.rb:73) (called from expenses_valid at /Users/jsugarman/rails_projects/ds_work/cccd/Claim-for-Crown-Court-Defence/app/models/determination.rb:77) (called from expenses_valid at /Users/jsugarman/rails_projects/ds_work/cccd/Claim-for-Crown-Court-Defence/app/models/determination.rb:81) (called from block (3 levels) in <top (required)> at /Users/jsugarman/rails_projects/ds_work/cccd/Claim-for-Crown-Court-Defence/spec/models/claim/transfer_detail_spec.rb:56) ```
Follows our official release of 3.2.0 with 6.1 support.
Additional submodel errors were appearing on the model instances and then the presenter was falling back to displaying them in the summary. These errors are duplicates of other errors already being handled by the custom error handler and, in addition, do not result in a working links to the relevant (or any) field. I have widdened the deletion to delete any error with an attribute indicating it is a nested error. This should be safe to do as such errors are copied to the parent/base record in a specifci custom format for handling by the custom error presenter. I have sanity tested its impact on a final claim for fixed fees
Use delegation and aliases instead of defined methods that simply call the behaviour of the delegated object. Since we now alias `errors_for?` as `keys?` to match the behaviour of `ActiveModel::Errors.keys?` we can use this in the adp_text_field following duck-typing principles.
cc3b2d8
to
bfbde94
Compare
jrmhaig
approved these changes
Jun 22, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot see any blockers to this. I will test it on dev-lgfs.
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.
What
Bump rails 6.0 to to 6.1
Why
Maintenance and support
TODO: