This project is archived and is in readonly mode.

#2986 open
Luke Melia

polymorphic_url should handle STI better

Reported by Luke Melia | August 3rd, 2009 @ 05:38 AM | in 3.x

If I have a class Project and a subclass SpecialProject, but only map.resources :projects defined in routes, it seems natural that polymorphic_url(@special_projects) would return http://example.com/projects/#{@special_project.id}

Currently, however, it fails looking for the method special_projects_path.

The attached patch (against master) implements a partial solution to this problem. When the named route determined by the normal heuristics is missing, the code walks up the inheritance chain checking if there a suitable route exists. The patch includes thorough test coverage.

Limitations of the patch in it's current state:

1) It handles the inheritance walking for only the last segment of a nested route. Handling more than that opens up more complexity, and while I think it's the right way to go, I thought I would start this conversation modestly.
2) It does not cache the results of the inheritance walking, which could be a opportunity for better performance here.
3) I have not used it in production. It started as a proof of concept and was freshened up in response to some conversation between wycats and brynary.

Comments and changes to this ticket

  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) August 3rd, 2009 @ 07:07 AM

    • State changed from “new” to “open”
    • Milestone set to 2.x

    I like this a lot.

    I'd like to see (1) and (2) handled. Caching will make this fast, and therefore viable. Handling only the last segment would probably produce confusion when other parts don't work. I'd also be interested to see if you could make this work against Josh's rack-mount, which will probably form the basis for the Rails 3 router.

    I'm attaching this patch to the 2.x milestone since we'll need a different patch against 3.0 once Josh's router is in place.

  • Luke Melia

    Luke Melia August 3rd, 2009 @ 07:31 AM

    I'll take a look at implementing on top of rack-mount. Handling (1) on 2.x will require major surgery the routing engine that seems dubious given a different approach for 3.x.

  • Jeremy Kemper

    Jeremy Kemper May 4th, 2010 @ 06:48 PM

    • Milestone changed from 2.x to 3.x
  • Rohit Arondekar

    Rohit Arondekar October 9th, 2010 @ 03:30 AM

    • State changed from “open” to “stale”
    • Importance changed from “” to “”

    Marking ticket as stale. If this is still an issue please leave a comment with suggested changes, creating a patch with tests, rebasing an existing patch or just confirming the issue on a latest release or master/branches.

  • Sven Fuchs

    Sven Fuchs October 13th, 2010 @ 07:54 PM

    I'm interested in this, too. I don't think it's stale, at least the issue/limitation does not seem resolved.

    Unfortunately the patch is missing on S3. Luke, any updates on this? Could you post the patch again? I realize this is over 1 year old ... :/

  • Luke Melia

    Luke Melia October 13th, 2010 @ 07:58 PM

    I never got anywhere on the Rails 3 compatible approach due to lack of time. We're upgrading our app to Rails 3 soonish, so hopefully I can dig in and take a crack at this then.

  • Rohit Arondekar

    Rohit Arondekar October 14th, 2010 @ 10:05 AM

    • State changed from “stale” to “incomplete”
    • Assigned user cleared.
    • Importance changed from “” to “Low”
  • Ryan Bigg

    Ryan Bigg October 19th, 2010 @ 08:31 AM

    • Tag cleared.

    Automatic cleanup of spam.

  • Ryan Bigg

    Ryan Bigg November 8th, 2010 @ 01:54 AM

    Automatic cleanup of spam.

  • Sven Fuchs

    Sven Fuchs November 21st, 2010 @ 04:52 PM

    • State changed from “incomplete” to “open”

    So, here's a commit that implements this behavior: https://github.com/svenfuchs/rails/commit/2d298f2a140cdc8388bf09ce5...

    Given an array of records it will first build an array of permutations of these record's classes and superclasses (excluding ActiveRecord::Base). Then it will build a named route url generation helper from the first permutation, check if the view responds to it and if so, return the method name. If not, it will try the next permutation and so on.

    The first matching permutation will be cached with a cache key that is cheaper to calculate than the whole named route helper method name. I'd expect that this implementation of build_named_route_call therefor is also slightly faster than the current one which doesn't use any caching.

    Say we have:

    class Site < ActiveRecord::Base; end
    class Section < ActiveRecord::Base; end
    class Blog < Section; end
    class Content < ActiveRecord::Base; end
    class Post < Content; end
    

    Then passing [site, blog, post] will try the following permutations:

    site_blog_post_url
    site_blog_content_url
    site_section_post_url
    site_section_content_url
    

    We're already using something like this, so this code has been extracted from a working code base.

    Caching needs to be improved though. Currently we're just populating a mattr_accessor Hash on the NamedRouteCall class. This cache never gets invalidated but probably should be invalidated in development mode. I'm not sure what the best way is to accomplish this. Maybe named route helper methods should be cached on the actual route set which will be thrown away and rebuilt?

    Also, the patch above applies to current 3-0-stable. I'm unclear about which branch to base this on.

  • Joe Hannon

    Joe Hannon December 22nd, 2010 @ 08:12 AM

    Rails' inability to automatically route my link_to and form_for in STI subclasses to the superclass is a constant source of frustration to me. +1 for fixing this bug.

  • bingbing

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Pages