Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

named_url and url_for: subdomain => false doesn't work #2025

Closed
abhishiv opened this issue Jul 9, 2011 · 15 comments · Fixed by #3209
Closed

named_url and url_for: subdomain => false doesn't work #2025

abhishiv opened this issue Jul 9, 2011 · 15 comments · Fixed by #3209

Comments

@abhishiv
Copy link

abhishiv commented Jul 9, 2011

Trying to use it as

= link_to "Me", user_url(@current_user.id, :subdomain => false)

In rails 3.1rc4

Apparently it has never worked.

@exviva
Copy link
Contributor

exviva commented Jul 10, 2011

What kind of behaviour would you expect from :subdomain => false?

@abhishiv
Copy link
Author

Say you are on customer.mysite.com

Using blogs_url would produces something like customer.mysite.com/blogs. While I want just mysite.com/blogs

In rails 3, I had a url_for helper which got from this railscast - http://asciicasts.com/episodes/221-subdomains-in-rails-3

As shown in the cast, it used to work like this

<p><%= link_to "All Blogs", root_url(:subdomain => false) %></p>  

However with 3.1 it doesn't work.

@ksob
Copy link
Contributor

ksob commented Jul 12, 2011

It doesn't work in v3.1 because the railscast suggests to override rails's internal behaviour (url_for method) and unfortunately the behaviour has now changed in v3.1.

Anyway if you still need to follow this fragile path just change /app/helpers/url_helper.rb so it now looks like:

module UrlHelper  
  def with_subdomain(subdomain)  
    subdomain = (subdomain || "")  
    subdomain += "." unless subdomain.empty?  
    [subdomain, request.domain].join
  end  

  def url_for(options = nil)  
    if options.kind_of?(Hash) && options.has_key?(:subdomain)
      options[:host] = with_subdomain(options.delete(:subdomain))
      options[:port] = request.port_string.gsub(':','') unless request.port_string.empty?
    end  
    super  
  end
end

You may also need to adjust routes.rb due to new v3.1 requirements:

MyBlog310::Application.routes.draw do
  resources :blogs

  constraints(Subdomain) do
    match '/' => 'blogs#show'
  end
  root :to => "blogs#index"
end

Tested with gem 'rails', '3.1.0.rc4' on a fresh project.

@Bodacious
Copy link
Contributor

Railscast aside, I think it makes sense to expect that users_url(:subdomain => false) should return "http://mydomain.com/users"
even if called @ http://subdomain.mydomain.com/

@dmathieu
Copy link
Contributor

Your "subdomain" can mean many things. What if you have sub.domain.mydomain.com ?
Will :subdomain => false return domain.mydomain.com or mydomain.com ?

This is very specific behavior which should be implemented in a plugin.

@lukaskonarovsky
Copy link
Contributor

Have :subdomain => false to remove all subdomains is reasonable request.

@exviva
Copy link
Contributor

exviva commented Jul 13, 2011

We should probably only keep tld_length + 1 parts of the host if :subdomain => false.

@Bodacious
Copy link
Contributor

@exviva Agreed!

dcadenas added a commit to dcadenas/rails that referenced this issue Aug 5, 2011
@dcadenas
Copy link
Contributor

dcadenas commented Aug 5, 2011

This was almost done with 2fe43b6
The missing thing was removing the '.' when an empty subdomain is specified which is what dcadenas@d6fd59c does.

@ksob
Copy link
Contributor

ksob commented Aug 7, 2011

I think it's better to follow the true / false convention like is already specified for :protocol,
that is we can already use :protocol => false like shown below:

rails/actionpack/test/controller/url_for_test.rb : 125

def test_without_protocol
  add_host!
  assert_equal('//www.basecamphq.com/c/a/i',
    W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => '//')
  )
  assert_equal('//www.basecamphq.com/c/a/i',
    W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => false)
  )
end

And so it's reasonable to add the same for :subdomain.
Here is my commit to make :subdomain => false possible:
ksob@cb4263e

@smaboshe
Copy link

Hello.

I'm using Rails 3.1.0 and would find the :subdomain => false behaviour described by @daeltar very useful.

In my application, at the moment :subdomain => false does not change the URL. Additionally :subdomain => '' leaves a leading "." in the URL.

Is there a work around in the pipeline? Would you recomment using @ksob's commit for the time being?

Thanks!

@szimek
Copy link
Contributor

szimek commented Oct 3, 2011

+1 for @ksob commit.

For now it looks like you can get rid of subdomains with:

root_url(:host => request.domain)

@smaboshe
Copy link

smaboshe commented Oct 4, 2011

@szimek - Thanks! Your hint root_url(:host => request.domain) works for me.

ksob added a commit to ksob/rails that referenced this issue Oct 4, 2011
…owing for subdomain(s) removal from the host during link generation. Closes rails#2025
ksob added a commit to ksob/rails that referenced this issue Oct 4, 2011
…owing for subdomain(s) removal from the host during link generation. Closes rails#2025
ksob added a commit to ksob/rails that referenced this issue Oct 4, 2011
…owing for subdomain(s) removal from the host during link generation. Closes rails#2025
@tgjones
Copy link

tgjones commented Dec 1, 2011

I notice this fix has not made it into 3-1-stable - presumably that means it will be in 3.2?

@mhuggins
Copy link

mhuggins commented Dec 7, 2011

I'm having the same issue with 3.1 stable. I am unable to use the root_url(:host => request.domain) suggestion made above, as I'm attempting to redirect within my routes definition, e.g.:

Dating::Application.routes.draw do
  constraints(SubdomainConstraint) do
    resource :signup
    match '/' => redirect('/signup/index')
    match '*a' => redirect(:subdomain => false)  # doesn't remove subdomain, resulting in redirect loop
  end

  # normal routes
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.