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

Tomas Sedovic tsedovic at redhat.com
Tue Dec 13 10:28:38 UTC 2011


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
-- 
No trees were killed to send this message, but a large number of 
electrons were terribly inconvenienced.



More information about the aeolus-devel mailing list