Upgrade to rails 5 #3414
Merged
Upgrade to rails 5 #3414
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
houndci-bot
reviewed
Mar 31, 2019
microweb10
force-pushed
the
upgrade_to_rails_5
branch
5 times, most recently
from
584a746
to
5939e66
Compare
Custom overwrite of the devise implementation following https://github.com/plataformatec/devise#activejob-integration
Uniq is deprecated
DEPRECATION WARNING: uniq is deprecated and will be removed from Rails 5.1 (use distinct instead) (called from block in <class:User> at /home/travis/build/consul/consul/app/models/user.rb:67)
https://stackoverflow.com/questions/37341967/rails-5-undefined-method-fo r-for-devise-on-line-devise-parameter-sanitizer
Devise api changed autorize path method
Since Globalize gem update to v5.2.0 we cannot override translations anymore in the same way that before the update. Milestone::Translation class removed in this commit were no longer loaded correctly when translation class is retrieved by translation_class method provided by Globalize. Here is the diff between both gem versions: globalize/globalize@v5.0.0...v5.2.0#diff-a1370b109e0dd567545b072bc6447b8fR51 This problem is not happening on test environment but is throwing an exception in other environments as it has not loaded the delegation definition inside our custom translation class. To fix this we added a new class method inside globalizable model concern to allow to define method delegation on translations classes from parent globalizable classes when needed without having to override Translation classes. Another way to properly load our custom Milestone::Translation class is to place it inside parent model class, like the example below: class Milestone ... class Translation delegate :status_id, to: :globalized_model end end Or maybe monkey patching translation_class method from globalize gem to make it find our custom translation class. I don't like this option.
Since Globalize gem update to v5.2.0 we cannot override translations anymore in the same way that before the update. Milestone::Translation class removed in this commit were no longer loaded correctly when translation class is retrieved by translation_class method provided by Globalize. Here is the diff between both gem versions: globalize/globalize@v5.0.0...v5.2.0diff-a1370b109e0dd567545b072bc6447b8fR51 This problem is not happening on test environment but is throwing an exception in other environments as it has not loaded the delegation definition inside our custom translation class. To fix this we added a new class method inside globalizable model concern to allow to define method delegation on translations classes from parent globalizable classes when needed without having to override Translation classes.
Since Globalize gem update to v5.2.0 we cannot override translations anymore in the same way that before the update. Milestone::Translation class removed in this commit were no longer loaded correctly when translation class is retrieved by translation_class method provided by Globalize. Here is the diff between both gem versions: globalize/globalize@v5.0.0...v5.2.0diff-a1370b109e0dd567545b072bc6447b8fR51 This problem is not happening on test environment but is throwing an exception in other environments as it has not loaded the delegation definition inside our custom translation class. To fix this we added a new class method inside globalizable model concern to allow to define method delegation on translations classes from parent globalizable classes when needed without having to override Translation classes. Another way to properly load our custom Translation class is to place it inside parent model class.
Since Globalize gem update to v5.2.0 we cannot override translations anymore in the same way that before the update. Milestone::Translation class removed in this commit were no longer loaded correctly when translation class is retrieved by translation_class method provided by Globalize. Here is the diff between both gem versions: globalize/globalize@v5.0.0...v5.2.0diff-a1370b109e0dd567545b072bc6447b8fR51 This problem is not happening on test environment but is throwing an exception in other environments as it has not loaded the delegation definition inside our custom translation class. To fix this we added a new class method inside globalizable model concern to allow to define method delegation on translations classes from parent globalizable classes when needed without having to override Translation classes. Another way to properly load our custom Milestone::Translation class is to place it inside parent model class.
Since Globalize gem update to v5.2.0 we cannot override translations anymore in the same way that before the update. Milestone::Translation class removed in this commit were no longer loaded correctly when translation class is retrieved by translation_class method provided by Globalize. Here is the diff between both gem versions: globalize/globalize@v5.0.0...v5.2.0diff-a1370b109e0dd567545b072bc6447b8fR51 This problem is not happening on test environment but is throwing an exception in other environments as it has not loaded the delegation definition inside our custom translation class. To fix this we added a new class method inside globalizable model concern to allow to define method delegation on translations classes from parent globalizable classes when needed without having to override Translation classes. Another way to properly load our custom Milestone::Translation class is to place it inside parent model class.
Since Globalize gem update to v5.2.0 we cannot override translations anymore in the same way that before the update. Milestone::Translation class removed in this commit were no longer loaded correctly when translation class is retrieved by translation_class method provided by Globalize. Here is the diff between both gem versions: globalize/globalize@v5.0.0...v5.2.0diff-a1370b109e0dd567545b072bc6447b8fR51 This problem is not happening on test environment but is throwing an exception in other environments as it has not loaded the delegation definition inside our custom translation class. To fix this we added a new class method inside globalizable model concern to allow to define method delegation on translations classes from parent globalizable classes when needed without having to override Translation classes. Since module Sanitizable takes care of traslations, it's enough to include the module in order to correctly sanitize the description.
If tests run very fast all votes are created within the last 24 hours, so hot_score has the same value if the creation date for the votes is Time.current or 1.day.ago. Creating the votes 48 hours ago we make sure hot_score has the correct value and the tests pass correctly.
Metrics/LineLength: Line is too long. RSpec/InstanceVariable: Use let instead of an instance variable. Layout/TrailingBlankLines: Final newline missing. Style/StringLiterals: Prefer double-quoted strings.
There was a security vulnerability with previous version heartcombo/devise#4981
This patch adds 2 missing colons (:) for 2 migrations that are failing when the `rake db:migrate` task is ran. Whilst already-committed-to-source-control- and-ran-on-production-environments migrations shouldn't be modified whatsoever, this change does not modify in any way the current database schema so, technically, it shouldn't affect forks that might have ran these migrations already and those CONSUL installations that haven't done so should be able to without problems
DEPRECATION WARNING: Directly inheriting from ActiveRecord::Migration is deprecated. Please specify the Rails release the migration was written for: class MigrationClass < ActiveRecord::Migration[4.2] (called from require at bin/rails:4)
DEPRECATION WARNING: [Devise] `DeviseHelper.devise_error_messages!` is deprecated and it will be removed in the next major version. To customize the errors styles please run `rails g devise:views` and modify the `devise/shared/error_messages` partial. We will render the resource errors instead fo calling the deprecated method.
For some reason the paperclip method `attachment.exists?' was returning nil even when `attachment.url(style)' was correctly returning the url/path of the attachment. Therefore returning `nil' was causing to raise an error in the method `image_tag'. With this change we make sure we return the image url it it's available, or an empty string if it's not, but never a null value.
Because autoloading is disabled in production with Rails 5, using autoload_paths will not load needed classes from specified paths. The solution to this, is to ask Rails to eager load classes. https://sipsandbits.com/2017/02/14/upgrading-a-ruby-on-rails-application/
New version of globalize uses RequestStore gem to store I18n.locale and Globalize.fallbacks in a per request basis to avoid collissions between different requests. This gem update broke Globalize.fallback results because it tries to fetch fallbacks from RequestStore, where there is no locale fallbacks definition.
spec/lib/tasks/dev_seed_spec.rb:8
This test was failing and we could see messages like:
db/dev_seeds/polls.rb:147:
warning: toplevel constant Answer referenced by Poll::Answer
Resulting in the error:
rake db:dev_seed seeds the database without errors
Failure/Error: expect { run_rake_task }.not_to raise_error
expected no Exception, got #<ActiveModel::UnknownAttributeError:
unknown attribute 'question_id' for Answer
Apparently the lookup was not correclty being performed, due to
conflicting names.
"Naming conflicts of this kind are rare in practice, but if one
occurs, require_dependency provides a solution by ensuring that
the constant needed to trigger the heuristic is defined in the
conflicting place."
https://guides.rubyonrails.org/v5.0/autoloading_and_reloading_constants.html
Rails 5 changed the initialization order, and now our initializers were running before the `append_assets_path` initializer for each engine, which prepended application assets to the custom assets we prepended in the initializer. Moving the code to the `config.after_initialize` code didn't work either, since the paths added there were ignored by the application. Adding another initializer to the Rails Engine is a hack, but solves the problem.
voodoorai2000
approved these changes
Apr 23, 2019
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.
References
Objectives
Visual Changes
None
Notes