[PATCH conductor] bug 784108: Hide additional links for non-privileged users.

Matt Wagner matt.wagner at redhat.com
Wed Sep 5 20:00:37 UTC 2012


On Wed, Sep 05, 2012 at 12:04:33PM -0400, Scott Seago wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=784108
> 
> "Images Valid"/"Repair Images" links are disabled if the user doesn't have edit permission on the deployable
> On the deployables list, the "add catalog" form filters the dropdown by catalogs that the user has permission to add to, and the form is hidden entirely if the user doesn't have edit rights on the deployable
> 
> On the Environments index page, the add image links and edit pool links are hidden for users without permission to perform those actions
> ---
>  src/app/controllers/deployables_controller.rb |  2 +-
>  src/app/helpers/deployables_helper.rb         | 17 ++++++++++++-----
>  src/app/views/deployables/show.html.haml      | 16 +++++++++-------
>  src/app/views/pool_families/_list.html.haml   |  9 ++++++---
>  4 files changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/src/app/controllers/deployables_controller.rb b/src/app/controllers/deployables_controller.rb
> index e07a603..947f578 100644
> --- a/src/app/controllers/deployables_controller.rb
> +++ b/src/app/controllers/deployables_controller.rb
> @@ -68,7 +68,7 @@ class DeployablesController < ApplicationController
>      save_breadcrumb(polymorphic_path([@catalog, @deployable]), @deployable.name)
>      @providers = Provider.all
>      @catalogs_options = Catalog.list_for_user(current_session, current_user,
> -                                              Privilege::VIEW).select do |c|
> +                                              Privilege::MODIFY).select do |c|
>        !@deployable.catalogs.include?(c) and
>          @deployable.catalogs.first.pool_family == c.pool_family
>      end
> diff --git a/src/app/helpers/deployables_helper.rb b/src/app/helpers/deployables_helper.rb
> index d97eaef..51a8fee 100644
> --- a/src/app/helpers/deployables_helper.rb
> +++ b/src/app/helpers/deployables_helper.rb
> @@ -1,15 +1,22 @@
>  module DeployablesHelper
> -  def image_valid?(assembly)
> +  def image_valid?(assembly, modify_perm)
>      edit_xml_path = edit_polymorphic_path([@catalog, @deployable], :edit_xml => true)
>  
>      if @missing_images.empty? && assembly[:hwp_name].present?
> -      link_to t('.images_valid'), edit_xml_path, :class => 'images_valid', :id => 'edit_xml_button'
> +      link_hash = {:label => '.images_valid', :class => 'images_valid'}
>      elsif assembly[:hwp_name].nil?
> -      link_to t('.repair_images'), edit_xml_path, :class => 'repair_images', :id => 'edit_xml_button'
> +      link_hash = {:label => '.repair_images', :class => 'repair_images'}
>      elsif @missing_images.include?(assembly[:image_uuid])
> -      link_to t('.repair_images'), edit_xml_path, :class => 'repair_images', :id => 'edit_xml_button'
> +      link_hash = {:label => '.repair_images', :class => 'repair_images'}
>      else
> -      link_to t('.images_valid'), edit_xml_path, :class => 'images_valid', :id => 'edit_xml_button'
> +      link_hash = {:label => '.images_valid', :class => 'images_valid'}

This is all perfectly fine, but noticing that :label is always just
:class with a dot in front, I might have been tempted to try to just
define the string ('images_valid') rather than a hash of the two, and
append the dot when displaying the link. I'm not sure that doing that is
better, versus an alternative idea that popped into my head when reading
this. No sense in changing this code.

> +    end
> +    if modify_perm
> +      link_to(t(link_hash[:label]), edit_xml_path,
> +              :class => link_hash[:class], :id => 'edit_xml_button')
> +    else
> +      content_tag(:a, t(link_hash[:label]), :class => link_hash[:class],
> +                  :id => 'edit_xml_button')
>      end
>    end
>  
> diff --git a/src/app/views/deployables/show.html.haml b/src/app/views/deployables/show.html.haml
> index a7ee021..ade679d 100644
> --- a/src/app/views/deployables/show.html.haml
> +++ b/src/app/views/deployables/show.html.haml
> @@ -1,8 +1,9 @@
>  = render :partial => 'layouts/admin_nav'
> +- modify_perm = check_privilege(Privilege::MODIFY, @deployable)
>  %header.page-header
>    .obj_actions
>      .button-group
> -      - if check_privilege(Privilege::MODIFY, @deployable)
> +      - if modify_perm
>          = link_to t('.edit'), edit_polymorphic_path([@catalog, @deployable]), :class => 'button', :id => 'edit_button'
>          = link_to t('.edit_xml'), edit_polymorphic_path([@catalog, @deployable], :edit_xml => true), :class => 'button', :id => 'edit_xml_button'
>          = button_to t('.delete'), polymorphic_path([@catalog, @deployable]), :method => 'delete', :confirm => "#{t'catalog_entries.show.confirm_delete'}", :class => 'button danger', :id => 'delete'
> @@ -33,7 +34,7 @@
>        %tbody
>          - @images_details.each do |assembly|
>            %tr
> -            %td.status= image_valid?(assembly)
> +            %td.status= image_valid?(assembly, modify_perm)
>              %td
>                %strong=assembly[:name]
>              %td.align-center=assembly[:images_count]
> @@ -77,11 +78,12 @@
>  %section.content-section
>    %header
>      .section-controls
> -      = form_for(:catalog_entry, :url => catalog_entries_path) do |f|
> -        = f.label :catalog_id, t('.choose_catalog')
> -        = f.select :catalog_id, options_for_select(@catalogs_options.map {|c| [c.name, c.id]})
> -        = f.hidden_field(:deployable_id, :value => @deployable.id)
> -        = f.submit t('.add_catalog'), :id => :add_catalog, :class => 'button pill', :disabled => @catalogs_options.empty?
> +      -if modify_perm
> +        = form_for(:catalog_entry, :url => catalog_entries_path) do |f|
> +          = f.label :catalog_id, t('.choose_catalog')
> +          = f.select :catalog_id, options_for_select(@catalogs_options.map {|c| [c.name, c.id]})
> +          = f.hidden_field(:deployable_id, :value => @deployable.id)
> +          = f.submit t('.add_catalog'), :id => :add_catalog, :class => 'button pill', :disabled => @catalogs_options.empty?
>      %h2.catalogs= t('.catalogs')
>    .content
>      .centered
> diff --git a/src/app/views/pool_families/_list.html.haml b/src/app/views/pool_families/_list.html.haml
> index 0a6515d..962b8cc 100644
> --- a/src/app/views/pool_families/_list.html.haml
> +++ b/src/app/views/pool_families/_list.html.haml
> @@ -14,8 +14,9 @@
>              %h2
>                = link_to pool_family.name, pool_family
>              .section-controls
> -              = link_to t("pools.new_pool"), new_pool_path(:pool_family_id => pool_family.id), :class => 'pool_family_button'
> -              - if pool_family.pools.any?
> +              - if check_privilege(Privilege::CREATE, Pool, pool_family)
> +                = link_to t("pools.new_pool"), new_pool_path(:pool_family_id => pool_family.id), :class => 'pool_family_button'
> +              - if pool_family.pools.any? && check_privilege(Privilege::USE, pool_family)
>                  = link_to t("images.import.import_image"), new_image_path(:environment => pool_family.id, :tab => 'import'), :class => 'new_image_button'
>                  = link_to t("images.new.new_image"), new_image_path( :environment => pool_family.id), :class => 'new_image_button'
>        - unless pool_family.pools.blank?
> @@ -47,7 +48,9 @@
>              %td= pool_stats[:available_quota].nil? ? raw('&infin;') : pool_stats[:available_quota]
>              %td= pool.enabled? ? t("pool_families.index.answer_yes") : t("pool_families.index.answer_no")
>              %td= link_to pool.catalogs.first.name, catalog_path(pool.catalogs.first) if pool.catalogs.any?
> -            %td= link_to t(:edit), edit_pool_path(pool), :class => 'pool_family_button'
> +            %td
> +              - if check_privilege(Privilege::MODIFY, pool)
> +                = link_to t(:edit), edit_pool_path(pool), :class => 'pool_family_button'
>              %td= link_to t('pool_families.index.provider_selection'), pool_provider_selection_path(pool), :class => 'pool_family_button'
>          %tr
>            %td= t("pool_families.index.total_statistics")
> -- 
> 1.7.11.4

ACK. I will push this as well.

-- Matt



More information about the aeolus-devel mailing list