Fixing up resource route naming in 1.2.x

(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&gt; 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

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.

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

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

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

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

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

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.

Patches for stable and edge with tests have been uploaded.

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