[PATCH conductor] Fixing return_path in permissions#set_permission_object

Jan Provazník jprovazn at redhat.com
Fri Dec 16 13:03:53 UTC 2011


On 12/14/2011 04:25 PM, ifarkas at redhat.com wrote:
> From: Imre Farkas<ifarkas at redhat.com>
>
> https://bugzilla.redhat.com/show_bug.cgi?id=766473
>
> The original parameterization of polymorphic_path doesn't work correctly with nested resources.
> To handle this @polymorphic_path_extras is used by to_polymorphic_path_param method of the model.
> ---
>   src/app/controllers/application_controller.rb    |    6 ++++--
>   src/app/controllers/deployables_controller.rb    |    2 +-
>   src/app/controllers/permissions_controller.rb    |   10 ++++++----
>   src/app/models/deployable.rb                     |    5 +++++
>   src/app/views/permissions/_permissions.html.haml |    4 ++++
>   5 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/src/app/controllers/application_controller.rb b/src/app/controllers/application_controller.rb
> index 053a87b..368411e 100644
> --- a/src/app/controllers/application_controller.rb
> +++ b/src/app/controllers/application_controller.rb
> @@ -280,15 +280,17 @@ class ApplicationController<  ActionController::Base
>       %w[asc desc].include?(params[:order_dir]) ? params[:order_dir] : "asc"
>     end
>
> -  def add_permissions_inline(perm_obj, path_prefix = "")
> +  def add_permissions_inline(perm_obj, path_prefix = '', polymorphic_path_extras = {})
>       @permission_object = perm_obj
>       @path_prefix = path_prefix
> +    @polymorphic_path_extras = polymorphic_path_extras
>       @roles = Role.find_all_by_scope(@permission_object.class.name)
>       set_permissions_header
>     end
>
> -  def add_permissions_tab(perm_obj, path_prefix = "")
> +  def add_permissions_tab(perm_obj, path_prefix = '', polymorphic_path_extras = {})
>       @path_prefix = path_prefix
> +    @polymorphic_path_extras = polymorphic_path_extras
>       if "permissions" == params[:details_tab]
>         require_privilege(Privilege::PERM_VIEW, perm_obj)
>         @permission_object = perm_obj
> diff --git a/src/app/controllers/deployables_controller.rb b/src/app/controllers/deployables_controller.rb
> index 7271695..ba7795d 100644
> --- a/src/app/controllers/deployables_controller.rb
> +++ b/src/app/controllers/deployables_controller.rb
> @@ -55,7 +55,7 @@ class DeployablesController<  ApplicationController
>       save_breadcrumb(catalog_deployable_path(@catalog, @deployable), @deployable.name)
>       @providers = Provider.all
>       @catalogs_options = Catalog.list_for_user(current_user, Privilege::VIEW).select {|c| !@deployable.catalogs.include?(c)}
> -    add_permissions_inline(@deployable)
> +    add_permissions_inline(@deployable, '', {:catalog_id =>  @catalog.id})
>       @images_details = @deployable.get_image_details
>       images = @deployable.fetch_images
>       uuids = @deployable.fetch_image_uuids
> diff --git a/src/app/controllers/permissions_controller.rb b/src/app/controllers/permissions_controller.rb
> index a5c5bb7..86787de 100644
> --- a/src/app/controllers/permissions_controller.rb
> +++ b/src/app/controllers/permissions_controller.rb
> @@ -185,7 +185,7 @@ class PermissionsController<  ApplicationController
>     def set_permission_object (required_role=Privilege::PERM_SET)
>       obj_type = params[:permission_object_type]
>       id = params[:permission_object_id]
> -    @path_prefix = params[:path_prefix]
> +    @polymorphic_path_extras = params[:polymorphic_path_extras]
>       @use_tabs = params[:use_tabs]
>       unless obj_type or id
>         @permission_object = BasePermissionObject.general_permission_scope
> @@ -196,10 +196,12 @@ class PermissionsController<  ApplicationController
>         @return_path = permissions_path
>         set_admin_users_tabs 'permissions'
>       else
> -      @return_path = send("#{@path_prefix}polymorphic_path",

@path_prefix should be kept, it's used at least in 
app/controllers/providers_controller.rb

> -                                     @permission_object,
> +      @return_path = send("polymorphic_path",
> +                          @permission_object.respond_to?(:to_polymorphic_path_param) ?
> +                              @permission_object.to_polymorphic_path_param(@polymorphic_path_extras) :
> +                              @permission_object,
>                             @use_tabs == "yes" ? {:details_tab =>  :permissions,
> -                            :only_tab =>  true} : {})
> +                                                :only_tab =>  true} : {})
>       end
>       require_privilege(required_role, @permission_object)
>     end
> diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
> index ebd230c..33f844c 100644
> --- a/src/app/models/deployable.rb
> +++ b/src/app/models/deployable.rb
> @@ -193,6 +193,11 @@ class Deployable<  ActiveRecord::Base
>       end
>     end
>
> +  def to_polymorphic_path_param(polymorphic_path_extras)
> +    catalog = polymorphic_path_extras.has_key?(:catalog_id) ? catalogs.find(polymorphic_path_extras[:catalog_id]) : catalogs.first
> +    [catalog, self]
> +  end
> +

I'm not sure it's good idea to return first catalog if 'catalog_id' is 
not passed in params. Why not return only [self] instead of [catalog, 
self] in this case?

>     private
>
>     def self.apply_search_filter(search)
> diff --git a/src/app/views/permissions/_permissions.html.haml b/src/app/views/permissions/_permissions.html.haml
> index 8846dd8..3ac19de 100644
> --- a/src/app/views/permissions/_permissions.html.haml
> +++ b/src/app/views/permissions/_permissions.html.haml
> @@ -5,6 +5,10 @@
>       %li= restful_submit_tag t('edit'), "multi_update", multi_update_permissions_path, 'POST', :id =>  'perm_edit_button', :class =>  'hidden'
>       = hidden_field_tag :permission_object_type, @permission_object.class.name
>       = hidden_field_tag :permission_object_id, @permission_object.id
> +
> +    - @polymorphic_path_extras.each do |key, value|
> +      = hidden_field_tag("polymorphic_path_extras[#{key}]", value)
> +
>       = hidden_field_tag :path_prefix, @path_prefix
>       = hidden_field_tag :use_tabs, @use_tabs ? @use_tabs : (@tabs ? 'yes' : 'no')
>


Hi Imre,
looks good, but there is a couple of notes inline.

Jan



More information about the aeolus-devel mailing list