fixes #15250 - ignore current controller from link_to/hash_for#3564
fixes #15250 - ignore current controller from link_to/hash_for#3564domcleal wants to merge 1 commit intotheforeman:developfrom
Conversation
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
There was a problem hiding this comment.
This is a test for the pre-existing opts.dup line, explained at https://github.com/theforeman/foreman/pull/3294/files#r55810116.
|
Fixes the reported issue with foreman_salt for me 👍 |
I think the commit message is quite reasonable, so have submitted a PR to prprocessor to change the behaviour around URLs: theforeman/prprocessor#43 |
When link_to generates a link on a page served from a nested controller (e.g. foreman_example/examples) with a hash of controller/action from our hash_for_*_path extension, it calls url_for which added the current controller prefix to the controller name, and this wouldn't exist. When linking to hash_for_hosts_path (`:controller => 'hosts', :action => 'index'`), this would attempt to link to `:controller => 'foreman_example/hosts'` instead. By returning :use_route from hash_for as in Rails 4.1, actionpack assumes the controller name is absolute and will not add the current controller prefix. Rails source where relative controller is assumed by url_for if no named route was given (the :use_route option): https://github.com/rails/rails/blob/v4.2.6/actionpack/lib/action_dispatch/routing/route_set.rb#L695-L698 Rails 4.1 source where :use_route was always added: https://github.com/rails/rails/blob/v4.1.14.2/actionpack/lib/action_dispatch/routing/route_set.rb#L289-L294
domcleal
force-pushed
the
15250-hash-for-plugins
branch
from
3098a9f to
4dd2eb6
Compare
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
theforeman-bot
added
Needs testing
Waiting on contributor
and removed
Waiting on contributor
labels
|
|
||
| def url | ||
| add_relative_path(@url || @context.routes.url_for(url_hash.merge(:only_path=>true))) | ||
| add_relative_path(@url || @context.routes.url_for(url_hash.merge(:only_path=>true).except(:use_route))) |
There was a problem hiding this comment.
This fixes the menu system (and its integration tests), as the url_for call here won't use :use_route so it was getting appended as a query parameter.
4 participants
When link_to generates a link on a page served from a nested controller
(e.g. foreman_example/examples) with a hash of controller/action from
our hash_for_*_path extension, it calls url_for which added the current
controller prefix to the controller name, and this wouldn't exist.
When linking to hash_for_hosts_path (
:controller => 'hosts', :action => 'index'), this would attempt to link to:controller => 'foreman_example/hosts'instead. By returning :use_route from hash_foras in Rails 4.1, actionpack assumes the controller name is absolute and
will not add the current controller prefix.
Rails source where relative controller is assumed by url_for if no named
route was given (the :use_route option):
https://github.com/rails/rails/blob/v4.2.6/actionpack/lib/action_dispatch/routing/route_set.rb#L695-L698
Rails 4.1 source where :use_route was always added:
https://github.com/rails/rails/blob/v4.1.14.2/actionpack/lib/action_dispatch/routing/route_set.rb#L289-L294