List Info

Thread: Polymorphic URL helpers documentation and fixes




Polymorphic URL helpers documentation and fixes
user name
2008-01-21 14:28:05
Yesterday I answered a question regarding polymorphic URL helpers on Core ML and noticed that the module has no documentation. I've documented it and rewritten unit tests using Mocha. I also optimized some of the code slightly (nothing major, though). The patch also includes two fixes by Geoff Buesing, who has done awesome work in this area in the past.

http://dev.rubyonrails.org/ticket/10883

Check the documentation, please yell if anything is wrong:
http://dev.rubyonrails.org/attachment/ticket/10883/PolymorphicRoutes.html?format=raw

What's still broken? (I decided not to fix everything in one go.)
  1. Hash argument: polymorphic_url(:id => article). Test is commented out (like before).
  2. Should this work: polymorphic_url(article, :format => :pdf) ? Currently it doesn't. I've included a failing tests, it's commented out.
Open ActionPack tickets with "polymorphic" in title:
http://dev.rubyonrails.org/query?status=new&status=assigned&status=reopened&;component=ActionPack&summary=%7Epolymorphic&order=priority

This one needs a discussion of its own: http://dev.rubyonrails.org/ticket/9621

Thanks.
- Mislav

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-coregooglegroups.com
To unsubscribe from this group, send email to rubyonrails-core-unsubscribegooglegroups.com
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Re: Polymorphic URL helpers documentation and fixes
user name
2008-01-21 20:13:41
On Jan 22, 2008 9:28 AM, Mislav Marohniæ
<mislav.marohnicgmail.com> wrote:
> Yesterday I answered a question regarding polymorphic
URL helpers on Core ML
> and noticed that the module has no documentation. I've
documented it and
> rewritten unit tests using Mocha. I also optimized some
of the code slightly
> (nothing major, though). The patch also includes two
fixes by Geoff Buesing,
> who has done awesome work in this area in the past.
>
> http://dev.ru
byonrails.org/ticket/10883
>
> Check the documentation, please yell if anything is
wrong:
>
> http://dev.rubyonrails.org/a
ttachment/ticket/10883/PolymorphicRoutes.html?format=raw
>
> What's still broken? (I decided not to fix everything
in one go.)
> Hash argument: polymorphic_url(:id => article).
Test is commented out (like
> before).
> Should this work: polymorphic_url(article,
:format => :pdf) ? Currently it
> doesn't. I've included a failing tests, it's commented
out.Open ActionPack

I don't think it should work with these 'advanced' hash
arguments.  do
they do the right thing when passing through to the
optimised named
route generators?  Do we want to support them going
forwards?

> This one needs a discussion of its own:
> http://dev.rub
yonrails.org/ticket/9621

I think that's just an abuse of inheritance and I'm not sure
we could
do anything performant and reasonable to support that case,
and the
case where AssetsController is meant to handle all the
different
subclasses?


-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.com
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Re: Polymorphic URL helpers documentation and fixes
user name
2008-01-21 20:22:39
2008/1/22 Michael Koziarski < michaelkoziarski.com">michaelkoziarski.com>:
> What's still broken? (I decided not to fix everything in one go.)
> Hash argument: polymorphic_url(:id => article). Test is commented out (like
> before).
&gt; Should this work: polymorphic_url(article, :format => :pdf) ? Currently it
> doesn't. I've included a failing tests, it's commented out.Open ActionPack

I don't think it should work with these 'advanced' hash arguments.  do
they do the right thing when passing through to the optimised named
route generators?  Do we want to support them going forwards?

In my experience, formatted_foo_path(foo, :pdf) and formatted_foo_path(foo, :format => :pdf) are identical. Still, my patch only adds support for formatted_polymorphic_path([foo, :pdf]), and I've put the hash argument up for discussion.

I think that's just an abuse of inheritance and I'm not sure we could
do anything performant and reasonable to support that case, and the
case where AssetsController is meant to handle all the different
subclasses?

I agree with such thinking; I'd only like these issues regarding polymorphic helpers resolved. Make the API concrete, reject features we don't want etc. Thanks Koz

P.S. somehow you avoided to comment on my patch

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-coregooglegroups.com
To unsubscribe from this group, send email to rubyonrails-core-unsubscribegooglegroups.com
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Re: Polymorphic URL helpers documentation and fixes
user name
2008-01-21 22:35:47


On 1/21/08, Michael Koziarski < michaelkoziarski.com">michaelkoziarski.com> wrote:
On Jan 22, 2008 9:28 AM, Mislav Marohniæ < mislav.marohnicgmail.com">mislav.marohnicgmail.com> wrote:
>;
> Should this work: polymorphic_url(article, :format => :pdf) ? Currently it
> doesn't. I've included a failing tests, it's commented out.Open ActionPack

I don't think it should work with these 'advanced' hash arguments.  ;do
they do the right thing when passing through to the optimised named
route generators? &nbsp;Do we want to support them going forwards?


Koz,

I've often wondered why (apart from specific implementation reasons) we need a formatted_xxx helper to cater for formats at all.

Bearing in mind that the generated helper code (including optimization stuff) could just look at the size of the hash and whether it's only member is :format, is there a reason the generated helpers can't simply distinguish between these signatures?

album_path(an_album)
album_path(an_album, :format => ml)
albums_path()
albums_path(:format => ml)

Somehow that seems easier on the eyes than:

album_path(an_album)
formatted_album_path(an_album, ml)
albums_path()
formatted_albums_path(ml)

 
I think it's because formatted_ seems to just add noise to the method name.  The format isn't (to me) the most important thing, it's more of an afterthought.  Keeping it's specification exclusively in the params mirrors that imho.

Regards,
Trevor
--
--
Trevor Squires
http://somethinglearned.com

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-coregooglegroups.com
To unsubscribe from this group, send email to rubyonrails-core-unsubscribegooglegroups.com
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

[1-4]

about | contact  Other archives ( Real Estate discussion Medical topics )