[PATCH conductor] Fixing recognize_path calls when the path contains prefix

Imre Farkas ifarkas at redhat.com
Tue Dec 13 13:02:58 UTC 2011


On 12/13/2011 11:28 AM, Tomas Sedovic wrote:
> On 12/13/2011 10:18 AM, ifarkas at redhat.com wrote:
>> From: Imre Farkas<ifarkas at redhat.com>
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=758653
>> ---
>> src/app/controllers/application_controller.rb | 11 +++++++++++
>> src/app/controllers/catalogs_controller.rb | 4 +---
>> src/app/controllers/deployables_controller.rb | 4 +---
>> src/app/controllers/deployments_controller.rb | 4 +---
>> .../controllers/hardware_profiles_controller.rb | 4 +---
>> src/app/controllers/instances_controller.rb | 4 +---
>> src/app/controllers/pools_controller.rb | 5 +----
>> .../controllers/provider_accounts_controller.rb | 4 +---
>> src/app/controllers/realms_controller.rb | 4 +---
>> src/app/controllers/users_controller.rb | 4 +---
>> 10 files changed, 20 insertions(+), 28 deletions(-)
>>
>> diff --git a/src/app/controllers/application_controller.rb
>> b/src/app/controllers/application_controller.rb
>> index 16fa71d..df2a0fc 100644
>> --- a/src/app/controllers/application_controller.rb
>> +++ b/src/app/controllers/application_controller.rb
>> @@ -349,4 +349,15 @@ class ApplicationController< ActionController::Base
>>
>> (ordered_languages& I18n.available_locales).first
>> end
>> +
>> + def redirect_to_original(extended_params = {})
>> + original_path = params[:current_path]
>> + # In cases when current_path contains prefix like '/conductor'
>> recognize_path fails.
>> + # It needs to be stripped out in order to get the path correctly.
>> + path = original_path.split(ENV['RAILS_RELATIVE_URL_ROOT']).last
>
> I'm a bit worried about this ^^^. RAILS_RELATIVE_URL_ROOT can be
> anything the admins who deploy Conductor set it out to be. And if that
> prefix happens to be repeated in the path, this won't work.
>
> How about something like this instead:
>
> root = ENV['RAILS_RELATIVE_URL_ROOT']
> path = path.slice(root.length..path.length) if path.start_with? root
>
> It's a tad longer but more robust.
>
>> + path = Rails.application.routes.recognize_path(path)
>> + original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> + redirect_to path.merge(original_params).merge(extended_params)
>> + end
>> +
>> end
>> diff --git a/src/app/controllers/catalogs_controller.rb
>> b/src/app/controllers/catalogs_controller.rb
>> index c530a6a..1f92d3b 100644
>> --- a/src/app/controllers/catalogs_controller.rb
>> +++ b/src/app/controllers/catalogs_controller.rb
>> @@ -99,9 +99,7 @@ class CatalogsController< ApplicationController
>> end
>>
>> def filter
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("catalogs_preset_filter" =>
>> params[:catalogs_preset_filter], "catalogs_search" =>
>> params[:catalogs_search])
>> + redirect_to_original({"catalogs_preset_filter" =>
>> params[:catalogs_preset_filter], "catalogs_search" =>
>> params[:catalogs_search]})
>> end
>>
>> private
>> diff --git a/src/app/controllers/deployables_controller.rb
>> b/src/app/controllers/deployables_controller.rb
>> index 19306d2..f909947 100644
>> --- a/src/app/controllers/deployables_controller.rb
>> +++ b/src/app/controllers/deployables_controller.rb
>> @@ -187,9 +187,7 @@ class DeployablesController< ApplicationController
>> end
>>
>> def filter
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("catalog_entries_preset_filter"
>> => params[:catalog_entries_preset_filter], "catalog_entries_search" =>
>> params[:catalog_entries_search])
>> + redirect_to_original({"catalog_entries_preset_filter" =>
>> params[:catalog_entries_preset_filter], "catalog_entries_search" =>
>> params[:catalog_entries_search]})
>> end
>>
>> def build
>> diff --git a/src/app/controllers/deployments_controller.rb
>> b/src/app/controllers/deployments_controller.rb
>> index ebaaf7a..3ca38c3 100644
>> --- a/src/app/controllers/deployments_controller.rb
>> +++ b/src/app/controllers/deployments_controller.rb
>> @@ -317,9 +317,7 @@ class DeploymentsController< ApplicationController
>> end
>>
>> def filter
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("deployments_preset_filter"
>> => params[:deployments_preset_filter], "deployments_search" =>
>> params[:deployments_search])
>> + redirect_to_original({"deployments_preset_filter" =>
>> params[:deployments_preset_filter], "deployments_search" =>
>> params[:deployments_search]})
>> end
>>
>> private
>> diff --git a/src/app/controllers/hardware_profiles_controller.rb
>> b/src/app/controllers/hardware_profiles_controller.rb
>> index e490adb..8773d6a 100644
>> --- a/src/app/controllers/hardware_profiles_controller.rb
>> +++ b/src/app/controllers/hardware_profiles_controller.rb
>> @@ -150,9 +150,7 @@ class HardwareProfilesController<
>> ApplicationController
>> end
>>
>> def filter
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("hardware_profiles_preset_filter"
>> => params[:hardware_profiles_preset_filter],
>> "hardware_profiles_search" => params[:hardware_profiles_search])
>> + redirect_to_original({"hardware_profiles_preset_filter" =>
>> params[:hardware_profiles_preset_filter], "hardware_profiles_search"
>> => params[:hardware_profiles_search]})
>> end
>>
>> private
>> diff --git a/src/app/controllers/instances_controller.rb
>> b/src/app/controllers/instances_controller.rb
>> index 9ae96a2..5979a1c 100644
>> --- a/src/app/controllers/instances_controller.rb
>> +++ b/src/app/controllers/instances_controller.rb
>> @@ -179,9 +179,7 @@ class InstancesController< ApplicationController
>> end
>>
>> def filter
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("instances_preset_filter"
>> => params[:instances_preset_filter], "instances_search" =>
>> params[:instances_search])
>> + redirect_to_original({"instances_preset_filter" =>
>> params[:instances_preset_filter], "instances_search" =>
>> params[:instances_search]})
>> end
>>
>> private
>> diff --git a/src/app/controllers/pools_controller.rb
>> b/src/app/controllers/pools_controller.rb
>> index 3918d14..7e4503a 100644
>> --- a/src/app/controllers/pools_controller.rb
>> +++ b/src/app/controllers/pools_controller.rb
>> @@ -234,10 +234,7 @@ class PoolsController< ApplicationController
>> end
>>
>> def filter
>> - #redirects to path of the table view medged with filter and search
>> params
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("pools_preset_filter" =>
>> params[:pools_preset_filter], "pools_search" => params[:pools_search])
>> + redirect_to_original({"pools_preset_filter" =>
>> params[:pools_preset_filter], "pools_search" => params[:pools_search]})
>> end
>>
>> protected
>> diff --git a/src/app/controllers/provider_accounts_controller.rb
>> b/src/app/controllers/provider_accounts_controller.rb
>> index 43bcd86..d66a6d5 100644
>> --- a/src/app/controllers/provider_accounts_controller.rb
>> +++ b/src/app/controllers/provider_accounts_controller.rb
>> @@ -184,9 +184,7 @@ class ProviderAccountsController<
>> ApplicationController
>> end
>>
>> def filter
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("provider_accounts_preset_filter"
>> => params[:provider_accounts_preset_filter],
>> "provider_accounts_search" => params[:provider_accounts_search])
>> + redirect_to_original({"provider_accounts_preset_filter" =>
>> params[:provider_accounts_preset_filter], "provider_accounts_search"
>> => params[:provider_accounts_search]})
>> end
>>
>> protected
>> diff --git a/src/app/controllers/realms_controller.rb
>> b/src/app/controllers/realms_controller.rb
>> index 4b0ef96..c0e01d4 100644
>> --- a/src/app/controllers/realms_controller.rb
>> +++ b/src/app/controllers/realms_controller.rb
>> @@ -129,9 +129,7 @@ class RealmsController< ApplicationController
>> end
>>
>> def filter
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("realms_preset_filter" =>
>> params[:realms_preset_filter], "realms_search" => params[:realms_search])
>> + redirect_to_original({"realms_preset_filter" =>
>> params[:realms_preset_filter], "realms_search" =>
>> params[:realms_search]})
>> end
>>
>> protected
>> diff --git a/src/app/controllers/users_controller.rb
>> b/src/app/controllers/users_controller.rb
>> index d9cf8c9..0af28e7 100644
>> --- a/src/app/controllers/users_controller.rb
>> +++ b/src/app/controllers/users_controller.rb
>> @@ -135,9 +135,7 @@ class UsersController< ApplicationController
>> end
>>
>> def filter
>> - original_path =
>> Rails.application.routes.recognize_path(params[:current_path])
>> - original_params =
>> Rack::Utils.parse_nested_query(URI.parse(params[:current_path]).query)
>> - redirect_to
>> original_path.merge(original_params).merge("users_preset_filter" =>
>> params[:users_preset_filter], "users_search" => params[:users_search])
>> + redirect_to_original({"users_preset_filter" =>
>> params[:users_preset_filter], "users_search" => params[:users_search]})
>> end
>>
>> protected
>
> Nice one, ACK. Before pushing, see a small nit inline please.
>
> Thomas

Thanks for your suggestion!
Fixed & pushed.



More information about the aeolus-devel mailing list