Skip to content
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

Rails 4.2 Support #607

Closed
ScotterC opened this issue Oct 2, 2014 · 5 comments
Closed

Rails 4.2 Support #607

ScotterC opened this issue Oct 2, 2014 · 5 comments

Comments

@ScotterC
Copy link
Contributor

ScotterC commented Oct 2, 2014

It appears that Rails 4.2.0.beta2 breaks core FriendlyId functionality.

class Category < ActiveRecord::Base
  extend FriendlyId
  friendly_id :title, use: [:slugged, :finders]
end
#  id             :integer          not null, primary key
#  title          :string(255)
#  slug           :string(255)      indexed

title = "foo bar"
Category.create(title: title)
Category.find title
# => ActiveRecord::RecordNotFound Exception: Couldn't find Category with 'id'=foo bar
@norman
Copy link
Owner

norman commented Oct 2, 2014

Ugh, this is why I've wanted to get rid of :finders for so long. Each major release breaks this. 😠

Does Category.friendly.find(title) still work?

@ScotterC
Copy link
Contributor Author

ScotterC commented Oct 2, 2014

Yup. friendly still works so we got that going for us.

@petergoldstein
Copy link
Collaborator

There are two failing specs under Rails 4.2.0.beta1.

  1. HistoryTestWithFriendlyFinders#test_should_be_findable_by_old_slugs - Just started looking at this one.
  2. ScopedTest#test_should_detect_scope_column_from_belongs_to_relation - I've got a fix for this one. Rails 4.2 switched to using String keys for the reflections hash. So the FriendlyId method just needs to check the symbol key first, and fall back to a String key.

I'm going to see if I can suss out the first issue today and submit a PR

@norman
Copy link
Owner

norman commented Dec 16, 2014

@petergoldstein did you ever adance on this? The only one still failing is test_should_be_findable_by_old_slugs; I'm looking at it right now, but if you already have a fix let me know and I'll move on to something else.

@norman
Copy link
Owner

norman commented Dec 17, 2014

I've been looking at this since late yesterday and have isolated the spec failure to some behavior I can't explain and suspect is a bug in Rails.

When the model's table has a type column, and the model uses the :finders module, then Model.find will use the friendly find method. When it does not, then it uses the default find method from the model and fails. However, exists? will work as expected.

You can reproduce this by editing schema.rb to add this line

add_column :restaurants, :type, :string

and then running

BUNDLE_GEMFILE=gemfiles/Gemfile.rails-4.2.rb bundle exec ruby -Itest test/history_test.rb -n HistoryTestWithFriendlyFinders#test_should_be_findable_by_old_slugs

which will pass. If you comment out the line in the schema, then it will fail.

@norman norman closed this as completed in 3b70e9e Dec 17, 2014
norman added a commit that referenced this issue Jan 16, 2015
Rails has done its usual song and dance and changed the way Model.find
works in 4.2, so it's now no longer delegated to the internal relation
class, but is its own method, so we solve this by including the finders
module in the model class as well as the relation class.

Resolves #607
tekin added a commit to alphagov/whitehall that referenced this issue Jan 26, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Jan 27, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Jan 28, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Jan 29, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Jan 29, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Jan 29, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Jan 29, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Jan 29, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Feb 11, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Feb 11, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Feb 11, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Feb 18, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Feb 19, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
tekin added a commit to alphagov/whitehall that referenced this issue Feb 19, 2015
The friendly_id magic find override module doesn't play nice with Rails STI,
which is a problem for Topics and TopicalEvents. For these models, use the new
recommended method of finding instances based on their slug, which is to go via
the `friendly` scope.

Eventually, all code should be updated to use this instead of the magic find
override module as it isn't very stable and keeps breaking with new releases of
Rails[1]. Doing this is before the upgrade to Rails 4 is complete is going to be
hard however, so deferring until later.

[1] norman/friendly_id#607
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants