Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

polymorphic_url gives incorrect url for :has_one nested resource

#1231

With a nested :has_one resource


  map.resources :articles, :has_one => :response

a call to polymorphic_url


  polymorphic_url([@article, @response])

should result in a call to:


  article_response_url(@article)
  # url is then: articles/:article_id/response

Currently the following is called, resulting in the incorrect id being used in the URL:


  article_response_url(@article, @response)
  # url is then incorrect: articles/:response_id/response

Below is one possible test and solution.

In rails/actionpack/test/controller/polymorphic_routes_test.rb


def test_path_with_has_one_nested
  @response.save
  Article.expects(:respond_to?).
      with(:reflect_on_association).
      returns(true)

  has_one = mock('association',
      :macro =>:has_one)
  Article.expects(:reflect_on_association).
      with(:response).returns(has_one)

  expects(:article_response_url).with(@article)
  polymorphic_url([@article, @response])
end

In rails/actionpack/lib/action_controller/polymorphic_routes.rb


def handle_has_one_nested(args)
  if args.size == 2
    model_class = args.first.class
    if model_class.respond_to?(:reflect_on_association)
      second_model_class = args.last.class
      association_name = second_model_class.name.tableize.singularize.to_sym
      association = model_class.reflect_on_association(association_name)
      if association && association.macro == :has_one
        args.pop
      end
    end
  end
  args
end

# call above method in existing polymorphic_url:

def polymorphic_url(record_or_hash_or_array, options = {})
  if record_or_hash_or_array.kind_of?(Array)
    record_or_hash_or_array = record_or_hash_or_array.dup
  end

  record    = extract_record(record_or_hash_or_array)
  format    = extract_format(record_or_hash_or_array, options)
  namespace = extract_namespace(record_or_hash_or_array)

  args = case record_or_hash_or_array
    when Hash;  [ record_or_hash_or_array ]
    when Array; record_or_hash_or_array.dup
    else        [ record_or_hash_or_array ]
  end

  inflection =
    case
    when options[:action].to_s == "new"
      args.pop
      :singular
    when record.respond_to?(:new_record?) && record.new_record?
      args.pop
      :plural
    else
      :singular
    end

  args = handle_has_one_nested(args) # ** <- Add here **

  args.delete_if {|arg| arg.is_a?(Symbol) || arg.is_a?(String)}
  args << format if format

  named_route = build_named_route_call(record_or_hash_or_array, namespace, inflection, options)

  url_options = options.except(:action, :routing_type, :format, :association)
  unless url_options.empty?
    args.last.kind_of?(Hash) ? args.last.merge!(url_options) : args << url_options
  end

  __send__(named_route, *args)
end

Reported by Rob McKinnon · October 17th, 2008 @ 11:35 AM

State: wontfix
Milestone: 2.x
Assigned to: Michael Koziarski Michael Koziarski
Importance: none

Activity

  1. Pratik
    Pratik
    • Assigned user set to Michael Koziarski

    January 18th, 2009 @ 06:15 AM

  2. CancelProfileIsBroken
    CancelProfileIsBroken

    Still broken under 2.3 final, though now getting URLs of the form

    
    http://localhost:3000/orders/1/line.%23%3Cline:0x24cbe34%3E
    

    March 16th, 2009 @ 03:31 PM

  3. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from actionpack, polymorphic, routes, url_helper to actionpack, bugmash, polymorphic, routes, url_helper

    August 5th, 2009 @ 03:39 PM

  4. José Valim
    José Valim

    I'm personally do not agree with checking for reflection and associations inside polymorphic_url. In my discussions with Yehuda, I get that this problem should be solved by looking into the router declaration, instead of inspecting the objects.

    Today, you can solve the problem by giving a symbol, instead of the object:

      polymorphic_url([@article, :response])
    

    August 8th, 2009 @ 02:01 PM

  5. dira
    dira

    -1 for the request. Using a symbol instead of an object reflect the definition of the route.

    August 8th, 2009 @ 05:04 PM

  6. Dan Pickett
    Dan Pickett

    -1 sounds like a lot of overhead for something that can be handled by a symbol currently

    August 8th, 2009 @ 08:25 PM

  7. José Valim
    José Valim
    • Tag changed from actionpack, bugmash, polymorphic, routes, url_helper to actionpack, polymorphic, routes, url_helper
    • State changed from new to wontfix

    Marking this as won't fix, but this should be supported after router refactoring for Rails 3.0. Feel free to discuss more in the mailing list, Yehuda created a post exactly to address polymorphic_url issues: http://groups.google.com/group/rubyonrails-core/browse_thread/threa....

    August 8th, 2009 @ 08:46 PM