Support rails 6.0.0 #1172
Support rails 6.0.0 #1172
Conversation
We are trying to test our application on rails 6.0.0.alpha but we can't install this because of the version lock Probably this will be helpful that we can try and report any issues with edge rails
In Rails 6.0 update_attributes/update_attributes! is considered deprecated. Method update/update! is the samish replacement.
Edouard-chin
force-pushed
the
ec-support-rails-6
branch
from
e015dfb
to
dfcde06
Compare
- The conditional wasn't correct, any version with a `0` minor and `< 2` patch (which is true for 6.0.0) would return that the version was `< 5.0.2`.
Edouard-chin
force-pushed
the
ec-support-rails-6
branch
from
dfcde06
to
6e3013e
Compare
- Bundler 1.16.1 has bug where dependencies can't be resolved properly when a gem is a release candidate or an alpha version. The underlying bundler issue can be found here rubygems/bundler#6449 You can see a failing build here https://travis-ci.org/paper-trail-gem/paper_trail/jobs/463055122 This commit updates bundler to latest version before installing deps
spec/dummy_app/app/models/person.rb
Outdated
| end | ||
|
|
||
| serialize :time_zone, TimeZoneSerializer.new | ||
| end |
There was a problem hiding this comment.
Does rails 6 change how serialize works? If so, can you point me to some reading on the subject?
There was a problem hiding this comment.
Dug into this and the TimeZone Attribute is technically not a requirement to make tests pass on AR 6.0.
Without it, some tests were failing but the reason for the failures weren't directly related do that. I explained the reason in detail in ac12912
That being said, even though adding a new TimeZone Attribute isn't a requirement, it makes the code cleaner. It's not a new 6.0 thing, we could have used it since Rails 5.0 when the Attribute API got released https://api.rubyonrails.org/classes/ActiveRecord/Attributes/ClassMethods.html
Let me know if you prefer to stick with our old serializer.
There was a problem hiding this comment.
Dug into this and the TimeZone Attribute is technically not a requirement to make tests pass on AR 6.0.
Thanks for investigating. For now, I have removed this. Let's discuss it in a separate PR.
Edouard-chin
force-pushed
the
ec-support-rails-6
branch
from
13f387c
to
df8d29b
Compare
- In Rails 6.0, rails/rails@3b95478 made a change to eagerly define attribute methods of a Model when `eager_load` is enabled. This breaks our test suite because of the way we run migration. The TL;DR is that doing `People.attribute_names` will return an empty array instead of `[:id, time_zone, ...]`. You can find a failing build here https://travis-ci.org/paper-trail-gem/paper_trail/jobs/463369634 Basically what happens is: 1) The dummy app boot, attribute methods of each model are defined but since migration didn't run yet, the tables aren't even created resulting in a empty attribute set. 2) Migration runs, but it's already too late. In this commit I disabled eager_loading in test, AFAIT there isn't much benefit in eager_loading the dummy app anyway. Also renaming the `user.rb` file to `postgres_user.rb` in order for rails autoloading to work correctly.
Edouard-chin
force-pushed
the
ec-support-rails-6
branch
from
007bfa8
to
ac12912
Compare
|
CI is green, each commit should explain the changes. I will squash everything once you give your ✅ . Thanks! |
|
Thanks Anton and Edouard for your work on this. @iggant @Edouard-chin. I think I'll release this as PT 10.1.0 |
|
Thanks a lot for your quick review and help on this, much appreciated ❤️ |
* Change update_attributes to update In Rails 6.0 update_attributes/update_attributes! is considered deprecated. Method update/update! is the replacement. * CI: Don't use Bundler 1.16.1 - Bundler 1.16.1 has bug where dependencies can't be resolved properly when a gem is a release candidate or an alpha version. The underlying bundler issue can be found here rubygems/bundler#6449 * Disable eager_load in test env: - In Rails 6.0, rails/rails@3b95478 made a change to eagerly define attribute methods of a Model when `eager_load` is enabled. This breaks our test suite because of the way we run migration. The TL;DR is that doing `People.attribute_names` will return an empty array instead of `[:id, time_zone, ...]`. You can find a failing build here https://travis-ci.org/paper-trail-gem/paper_trail/jobs/463369634 Basically what happens is: 1) The dummy app boot, attribute methods of each model are defined but since migration didn't run yet, the tables aren't even created resulting in a empty attribute set. 2) Migration runs, but it's already too late. In this commit I disabled eager_loading in test, AFAIT there isn't much benefit in eager_loading the dummy app anyway. Also renaming the `user.rb` file to `postgres_user.rb` in order for rails autoloading to work correctly.
Supersede #1159
Check the following boxes:
master(if not - rebase it).code introduces user-observable changes.
and description in grammatically correct, complete sentences.