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

"No route matches" for a model with friendly_id and other extension #259

Closed
jeremywadsack opened this issue Apr 5, 2012 · 10 comments
Closed

Comments

@jeremywadsack
Copy link

Using friendly_id 4.0.0, we are seeing the "No route matches" for a set up similar to the following:

class Client < ActiveRecord::Base
  extend FriendlyId
  friendly_id :name, :use => :slugged
  ...
end

class ClientsController < ApplicationController
  include Custom::NumberHelper

  def index
    @clients.each { |client| client.extend ClientView }
  end
end

module Custom
  module NumberHelper
    def percentage_to_number(percentage)
      percentage =~ /\%/ ? percentage.to_f / 100 : percentage
    end
  end
end

This shown up in index.html template for the link_to @client but not for edit_client_path(@client). It shows up for edit_client_path(@client) in the show.html template.

We had no issue using these before we added friendly_id, although the module was included on the ActiveRecord objects. I can remove the problem by removing the module (retaining friendly_id).

As a workaround, I just changed links to use @client.id in all places where errors were raised and this seems to have resolved it in the short term.

Thoughts?

@norman
Copy link
Owner

norman commented Apr 5, 2012

Check your routes.rb to make sure you don't have a regular expression that stops the url with the text-based, rather than numeric, id from matching the route.

FriendlyId perhaps counter-intuitively doesn't modify Rails's routing code in any way, it only adds functionality to Active Record. So if you add FriendyId and your routes break, it's almost certainly a problem that needs to be resolved in your own routes.

@jeremywadsack
Copy link
Author

Hmm... I get that it doesn't modify routes, it just changes the behavior of ActiveRecord #find, and such. I understand that there may be more at play here, but our routes are all using standard rails 3 calls for this model:

  resources :clients do
    resources :blah1, :only => [:create, :destroy], :controller => "client_blah1"
    resources :blah2, :only => [:create, :destroy], :controller => "client_blah2"
    resources :blah3, :only => [:index], :controller => "clients/blah3"
    member do
      get :blah4
      get :blah5
    end
  end

I should also point out that this happens for the first client in the database but not the second one, neither have slugs and the URL's are using ID's. If I add a slug in the database to the first client then I can access it, even though I'm still using the URL with the ID.

In my test database I only have two client records and, other than id and timestamps, they are identical.

Slugs are on the :name field which in this case is nill (and allowed to be) for both records.

I can see if I can pull this out to a simple test app as an example.

@norman
Copy link
Owner

norman commented Apr 6, 2012

Ok, if you could reproduce it in a sample app I'd be more than happy to investigate.

@tomups
Copy link

tomups commented Apr 23, 2012

I'm facing the same issue.

In my case, it breaks when I try to update a record that didn't pass validation. When trying to show the validation errors to a view, I get "No route matches" because the slug got generated even though validation failed, so I have an object with id and slug, but with no slug in the DB.

If I edit a record that does indeed have a slug in the DB, or if I manually nil the slug before rendering the view, then it works fine.

@tomups
Copy link

tomups commented Apr 24, 2012

After further investigation, I think it has to do with how friendly_id overrides to_param. ActionDispatch uses the parametrization of the object to generate the URL through Journey.

If the slug is only in memory and not in the db, (which happens when trying to save an object that didn't pass validation), object.to_param returns nil.

Example from the console:

> garage = Garage.find(1)
  Garage Load (0.6ms)  SELECT "garages".* FROM "garages" WHERE "garages"."id" = $1 LIMIT 1  [["id", 1]]
 => #<Garage id: 1, name: "Workshop for testing", mandatory_value: nil, slug: nil> 
> garage.slug
 => nil 
> garage.to_param
 => "1" 
> garage.save  # validation fails
   (0.3ms)  BEGIN
  Garage Load (0.9ms)  SELECT "garages".* FROM "garages" WHERE ("slug" = 'workshop-for-testing' OR "slug" LIKE 'workshop-for-testing--%') AND (id <> 1) ORDER BY LENGTH("slug") DESC, "slug" DESC LIMIT 1      
   (0.2ms)  ROLLBACK
 => false
> garage.slug  # the slug is set but not saved to DB
 => "workshop-for-testing"
> garage.to_param # no param, so url_for fails 
 => nil
> app.garage_path(garage)
ActionController::RoutingError: No route matches {:action=>"show", :controller=>"garages", :id=>#<Garage id: 1, name: "Workshop for testing", mandatory_value: nil, slug: "workshop-for-testing">}
    from /gems/actionpack-3.2.1/lib/action_dispatch/routing/route_set.rb:522:in `raise_routing_error'
    from /gems/actionpack-3.2.1/lib/action_dispatch/routing/route_set.rb:518:in `rescue in generate'
    from /gems/actionpack-3.2.1/lib/action_dispatch/routing/route_set.rb:510:in `generate'
    from /gems/actionpack-3.2.1/lib/action_dispatch/routing/route_set.rb:551:in `generate'
    from /gems/actionpack-3.2.1/lib/action_dispatch/routing/route_set.rb:576:in `url_for'
    from /gems/actionpack-3.2.1/lib/action_dispatch/routing/url_for.rb:148:in `url_for'
    from /gems/actionpack-3.2.1/lib/action_dispatch/routing/route_set.rb:202:in `garage_path'
    from (irb):22
    from /gems/railties-3.2.1/lib/rails/commands/console.rb:47:in `start'
    from /gems/railties-3.2.1/lib/rails/commands/console.rb:8:in `start'
    from /gems/railties-3.2.1/lib/rails/commands.rb:41:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6:in `<main>'

> garage.mandatory_field = "foo"
 => "foo"    
> garage.save
   (0.4ms)  BEGIN
   (0.7ms)  UPDATE "garages" SET "slug" = 'workshop-for-testing', "mandatory_field" = 'foo', "updated_at" = '2012-04-24 02:13:01.371807' WHERE "garages"."id" = 1
   (6.3ms)  COMMIT
 => true 
> garage.slug
 => "workshop-for-testing" 
> garage.to_param 
 => "workshop-for-testing"  
> app.garage_path(garage) # now it's ok but...
 => "/garages/workshop-for-testing"
> garage.slug = "some-other-slug" # ...if I change the slug...
 => "some-other-slug"
> garage.to_param # ...to_param returns the old slug!
 => "workshop-for-testing"
> app.garage_path(garage) # this clearly affects url_for output
 => "/garages/workshop-for-testing"

I isolated this behaviour to base.rb, due to the first condition of the if in to_param:

def to_param
  if diff = changes[friendly_id_config.query_field]
    diff.first
  else
    friendly_id.present? ? friendly_id : id.to_s
  end
end

If this is changed to simply

def to_param
  friendly_id.present? ? friendly_id : id.to_s
end

Everything works as (at least I) expected.

> garage = Garage.find(1)
  Garage Load (0.6ms)  SELECT "garages".* FROM "garages" WHERE "garages"."id" = $1 LIMIT 1  [["id", 1]]
 => #<Garage id: 1, name: "Workshop for testing", mandatory_value: nil, slug: nil> 
> garage.slug
 => nil 
> garage.to_param
 => "1" 
> garage.save
   (0.2ms)  BEGIN
  Garage Load (0.9ms)  SELECT "garages".* FROM "garages" WHERE ("slug" = 'workshop-for-testing' OR "slug" LIKE 'workshop-for-testing--%') AND (id <> 1) ORDER BY LENGTH("slug") DESC, "slug" DESC LIMIT 1      
   (0.2ms)  ROLLBACK
 => false 
> garage.slug
 => "workshop-for-testing" 
> garage.to_param
 => "workshop-for-testing"
> app.garage_path(garage)
 => "/garages/workshop-for-testing"
> garage.slug = "changing-the-slug"
 => "changing-the-slug" 
> garage.to_param
 => "changing-the-slug"
> app.garage_path(garage)
 => "/garages/changing-the-slug" 

What is the purpose of the first condition? Is it necessary for other functionalities of friendly_id?

@norman
Copy link
Owner

norman commented Apr 28, 2012

@RayMcCoy Thanks for looking into this. Yes, the first condition is needed - it's supposed to handle showing the new slug before it's saved, precisely so that routing works. In this case it's not accounting for it being changed, but nil. I think in this case the first condition should probably also check if the new value is nil, and if it is, return the old one.

tomups pushed a commit to tomups/friendly_id that referenced this issue May 5, 2012
@tomups
Copy link

tomups commented May 5, 2012

Sorry about the multiple references, github went a bit funky! Anyway that last commit seems to address this as you said.

@andoq
Copy link

andoq commented May 16, 2012

first of all, shoudn't the conditions be reversed? First check friendly_id.present?, and only if false then look through the diffs?

Second, shouldn't diff.second always be the default? Diff's come back as [old_value, new_value], and it seems like you'd want to use the new_value, and diff.first is the old value.

However, overall, I still can't figure out why the first condition is there... the set_slug method is done before_validations, and once it's set you can just use friendly_id.

@andoq
Copy link

andoq commented May 16, 2012

actually, found the test and commit for this line of code...

#148

it's actually to fix to reference the old value for when validations fail, but this will actually break trying to utilize the object during the callbacks on initial user creation.

norman added a commit that referenced this issue May 21, 2012
@norman norman closed this as completed in 71f73dd May 21, 2012
jsmestad pushed a commit to welltok/friendly_id that referenced this issue May 21, 2012
The previously pulled commit closes norman#259 and closes norman#279.
@fschwahn
Copy link
Contributor

fschwahn commented Nov 7, 2014

I'm having a similar issue with friendly_id 5.0.4 - the steps to reproduce are:

  • There must be a model with friendly_id :slug_column, use: [:slugged]
  • Create a model instance with .save(validate: false) - this bypasses the slug generation; there will be no slug, the slug-column is nil
  • Edit the model in a form and fail the validation (don't edit the slug column)
  • Submit the form again - now there is a routing error because the slug is set in the form, but not yet in the database.

Is there any way to get around this?

All the best,
Fabian

parndt pushed a commit that referenced this issue May 20, 2018
This ensures that we use the new value of a slug when calling
`to_param` within an ActiveRecord callback. It also seems to pass all
the other regression tests I've added, thus keeping the fixes working
for #259 and #148, and as a bonus, prevents getting a bunch of
deprecation warnings in Rails 5.1.
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

5 participants