Fixing up resource route naming in 1.2.x

5 views
Skip to first unread message

Andrew White

unread,
Aug 25, 2007, 1:58:03 AM8/25/07
to rubyonra...@googlegroups.com
(First time post here, so apologies in advance for any breaches of
etiquette)

I'm writing a plugin for myself to change how resource routes are
named to work round the issues with the semi-colon. I can't switch to
edge as I'm using a e-commerce engine that I've written and the
engines plugin doesn't support edge at the moment.

Leaving the issue of whether I should be using the engines plugin at
the door, I'd like to be consistent with what might happen in 1.2.4.
I've read <http://dev.rubyonrails.org/ticket/8558> and looked at the
attached patch to stable and I was wondering what the likely outcome
was as there has been no activity on the ticket for 7 weeks.

AFAICS the questions that need answering are:

1. Should the semi-colon be changed in 1.2.x?
2. Should nested resources routes be named as in edge with the old
names being deprecated?
3. Should new resource route functionality such as namespaces,
associations, requirements and conditions be included?
4. Should ActionController::RecordIdentifier be included?
5. Should ActionController::PolymorphicRoutes be included?
6. Should MethodNotAllowed and NotImplemented be included?

I'd be more than willing to write up an acceptable patch to stable if
required.


Andrew White

Michael Koziarski

unread,
Aug 28, 2007, 1:02:14 AM8/28/07
to rubyonra...@googlegroups.com
> 1. Should the semi-colon be changed in 1.2.x?

No, though we should probably just make that a class variable so you
can override if needs be. (for both trunk and stable)

> 2. Should nested resources routes be named as in edge with the old
> names being deprecated?

Yes

> 3. Should new resource route functionality such as namespaces,
> associations, requirements and conditions be included?

No

> 4. Should ActionController::RecordIdentifier be included?

No

> 5. Should ActionController::PolymorphicRoutes be included?

No

> 6. Should MethodNotAllowed and NotImplemented be included?

No

> I'd be more than willing to write up an acceptable patch to stable if
> required.

In my view, all that's needed is to warn about the old generated named
routes, and support changing from ; to / for :member and friends.
Everything else should probably be written up in a wiki page for
inclusion in the release notes.

--
Cheers

Koz

Andrew White

unread,
Aug 28, 2007, 1:34:48 AM8/28/07
to rubyonra...@googlegroups.com

On 28 Aug 2007, at 06:02, Michael Koziarski wrote:

> In my view, all that's needed is to warn about the old generated named
> routes, and support changing from ; to / for :member and friends.
> Everything else should probably be written up in a wiki page for
> inclusion in the release notes.

Okay I'll come up with a patch for stable and edge that uses a class
variable for the resource action and deprecates the old route names
for stable. Should I put them both under one ticket or separate tickets?

One other question - should the new formatted named routes place the
format parameter at the end of the url like they are in edge? The old
named routes could maintain the existing placement for backwards
compatibility.


Andrew White

Pixeltrix T: 024 7625 6244
12 Turbine Hall M: 07973 190 907
Electric Wharf F: 024 7625 6266
Coventry CV1 4JB www.pixeltrix.co.uk

Andrew White

unread,
Aug 28, 2007, 2:17:30 AM8/28/07
to rubyonra...@googlegroups.com

On 28 Aug 2007, at 06:02, Michael Koziarski wrote:

>> 2. Should nested resources routes be named as in edge with the old
>> names being deprecated?
>
> Yes

One other thing - should the deprecated old names remain in edge/2.0
or should they be removed?

Andrew White

unread,
Aug 28, 2007, 5:57:56 AM8/28/07
to rubyonra...@googlegroups.com
On 28 Aug 2007, at 06:02, Michael Koziarski wrote:

> In my view, all that's needed is to warn about the old generated named
> routes, and support changing from ; to / for :member and friends.
> Everything else should probably be written up in a wiki page for
> inclusion in the release notes.


Just looking at the names of the routes generated in edge at the
moment I notice that the namespace is passed using the name_prefix
option. This means that the follow route definition:

map.namespace :admin do |admin|
admin.resources :images, :member => {:thumbnail => :get}
end

gives the following names:

thumbnail_admin_image = /admin/images/:id/thumbnail/
formatted_thumbnail_admin_image = /admin/images/:id/
thumbnail.:format/

Is this correct? Shouldn't the namespace go before the action and
after the formatted keyword? e.g.

admin_thumbnail_image = /admin/images/:id/thumbnail/
formatted_admin_thumbnail_image = /admin/images/:id/
thumbnail.:format/

Obviously if stable is not having namespaces then this isn't an issue
there unless people emulate namespaces by using name_prefix and
path_prefix and then upgrade to 2.0 and rewrite their routes using
namespaces.


Andrew White

Andrew White

unread,
Aug 28, 2007, 11:52:07 AM8/28/07
to rubyonra...@googlegroups.com
On 28 Aug 2007, at 06:02, Michael Koziarski wrote:

> In my view, all that's needed is to warn about the old generated named
> routes, and support changing from ; to / for :member and friends.
> Everything else should probably be written up in a wiki page for
> inclusion in the release notes.

I have a patch for stable which does the following:

1. Adds a class variable to ActionController::Base called
resource_action_separator defaulting to ';'

2. If a named route has changed it generates methods with the old names
which redirect to the new name with a deprecation warning

3. Moves the format parameter to the end of the path for all urls.
This has the added benefit of fixing caching for these actions

It passes the current tests but I still need to add some to check for
the deprecated routes names being added. Are there any existing tests
for deprecated functionality that I can look at to get an idea of how
to do them?

When I've done that should I create a new ticket or upload the patch
to the existing ticket 8558?


Andrew White

Tobias Luetke

unread,
Aug 28, 2007, 5:56:49 PM8/28/07
to Ruby on Rails: Core
Andrew,

Sounds great, please do attach the patch to http://dev.rubyonrails.org/ticket/8558
and i'll get it in if you think its ready.
Thanks a lot for looking into this.

Andrew White

unread,
Aug 28, 2007, 10:37:00 PM8/28/07
to rubyonra...@googlegroups.com

On 28 Aug 2007, at 22:56, Tobias Luetke wrote:

> Sounds great, please do attach the patch to http://
> dev.rubyonrails.org/ticket/8558
> and i'll get it in if you think its ready.
> Thanks a lot for looking into this.

Patches for stable and edge with tests have been uploaded.

Reply all
Reply to author
Forward
0 new messages