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

Fallback to RAILS_RELATIVE_URL_ROOT in url_for #18775

Closed
wants to merge 1 commit into from

Conversation

yasyf
Copy link
Contributor

@yasyf yasyf commented Feb 1, 2015

Fixed an issue where the RAILS_RELATIVE_URL_ROOT environment
variable is not prepended to the path when url_for is called.
If SCRIPT_NAME (used by Rack) is set, it takes precedence.

Fixes #5122.

@yasyf
Copy link
Contributor Author

yasyf commented Feb 3, 2015

Just a note that I consulted with @pixeltrix before deciding on the order of precedence with these different prefix options.

ActionController::Base.config.relative_url_root = '/subdir'
add_host!
assert_equal('https://www.basecamphq.com/subdir/c/a/i',
W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New tests should use the Ruby 1.9 hash syntax instead of hash rockets.

@yasyf
Copy link
Contributor Author

yasyf commented Feb 18, 2015

@eileencodes fixed.

@@ -691,7 +691,7 @@ def optimize_routes_generation?
end

def find_script_name(options)
options.delete(:script_name) || ''
options.delete(:script_name) || ActionController::Base.config.relative_url_root || ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we should be coupling Action Dispatch with Action Controller here. Do we have any precedent? cc @pixeltrix

Can we inject this config at load time using the railtie?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change this to use railties.

@yasyf
Copy link
Contributor Author

yasyf commented Feb 24, 2015

@rafaelfranca I added a new config for Action Dispatch to prevent coupling it with Action Controller.

Fixed an issue where the `RAILS_RELATIVE_URL_ROOT` environment
variable is not prepended to the path when `url_for` is called.
If `SCRIPT_NAME` (used by Rack) is set, it takes precedence.
rafaelfranca added a commit that referenced this pull request Mar 3, 2015
Fallback to RAILS_RELATIVE_URL_ROOT in `url_for`
@rafaelfranca
Copy link
Member

Merged at 44ff031

rafaelfranca added a commit that referenced this pull request Mar 3, 2015
Fallback to RAILS_RELATIVE_URL_ROOT in `url_for`
@rafaelfranca
Copy link
Member

Backported at 0703453

jiannis pushed a commit to moneyadviceservice/publify that referenced this pull request May 10, 2016
This reverts commit 2d9514e which has been fixed since Rails 4.2.3

See:
 - [Rails 4.2.3 Changelog](https://github.com/rails/rails/blob/v4.2.4/actionpack/CHANGELOG.md#rails-423-june-25-2015)
 - [Fallback to RAILS_RELATIVE_URL_ROOT in `url_for` #18775](rails/rails#18775)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENV['RAILS_RELATIVE_URL_ROOT'] and url_for
3 participants