mbleigh / subdomain-fu

A new plugin approach to attempting to solve the usage of subdomains in linking and routing in Rails projects.
http://rdoc.info/projects/mbleigh/subdomain-fu
Other
586 stars 118 forks source link

Subdomain conditions not passed to generated named routes generated by resources. #7

Open EmFi opened 14 years ago

EmFi commented 14 years ago

I'm new to subdomain-fu. So I might be mistaking the desired behaviour for a defect.

The named route helpers generated by the following resource declaration do not add the subdomain option to the underlying url_for calls.

map.resources :foos, :conditions => {:subdomain => :bar}

foos_url produces http://current_subdomain.example.com/foos. I'm expecting http://bar.example.com/foos. regardless of what the current subdomain is.

I understand that this behaviour ensures that subdomains are not changed unless explicitly stated. It's not very DRY to require an explicitly supplied subdomain for routes that are only valid under specific subdomains.

mbleigh commented 14 years ago

Hmm, this is a scenario I hadn't considered before. There are a number of factors that would make it a tricky fix. The URL rewriting doesn't talk to the routing portion at all, and I'm not sure of a "smart" way to hook that up. Also, as the routing portion of the plugin will be deprecated when Rails 3 comes along, I'm not sure if I'll take the time to dig in or not. A fork+pull would be welcome, though!

EmFi commented 14 years ago

I think the solution is simpler than you make it out to be.

You are correct in your understanding that the url rewriting doesn't talk to the routing portions directly. However, the creation of a named route defines four helpers, one produces the hash that when given to url_for produces the correct route. Another bundles a call to url_for with that hash.

All that really needs to be done is add the subdomain argument to that url_for_hash the helpers produces for named routes created with the :subdomain condition.

I won't really have time to play around with this until the weekend. So hopefully I'll find some time before Monday to fork, tweak, test, and issue a pull request.

I have the feeling that we're looking at a couple line tweak to ActionController::Routing::RouteBuilder around the time divide_route_options is called to copy the :subdomain hash in the :conditions to the default options.

Essentially doing:

options = options.merge({:subdomain => options[:conditions][:subdomain]}) if options[:conditions][:subdomain]

in or before divide_route_options. Probably part of a new alias_method_chain.

I'll have to look a little closer at the subdomain-fu code to ensure the unless subdomain clauses are uneffected. But I don't expect it to take too long.

In hindsight, it seems I've done the hard part while writing this response.

mbleigh commented 14 years ago

EmFi: You're right, I just wasn't thinking of it in those terms. If I remember back from when I was implementing, map.resources actually ignores the :conditions hash since the only default condition is :method which is overridden. I think I may have tried to go down that path at some point but stopped short of a full implementation. Anyways, I'd be happy to have a patch!