[katello-devel] Navigation.rb refactor ideas needed :)

[Justin Sherrill]

On 08/23/2011 07:58 AM, Ivan Nec(as wrote:
> Hello
>
> On 08/19/2011 09:25 PM, Partha Aji wrote:
>> Issue:
>> At the end of our roles-ui rewrite, we realized that we need to add 
>> rules to navigation.rb to acl things off. So based on rules in the 
>> controllers I came up with the following rules based navigation rb.
>> http://git.fedorahosted.org/git/?p=katello.git;a=blob;f=src/config/navigation.rb;h=5d48039d1ce90cacf1f5b0e936715ff5f3adefff;hb=bea1d8c6d253cb79430586c5f92c4bc8ead6e9b9 
>>
>>
>> I notice a couple of problems here and need suggestions on way to 
>> refactor this.
>> 1) Rules are on every line causing confusion and ugliness. For example
>>
>> providers_sub.item :edit, _("Basics"), (@provider.nil? || 
>> @provider.new_record?) ? "" : edit_provider_path(@provider.id), 
>> :class =>  'navigation_element', :if =>  Proc.new { 
>> !@provider.nil?&&  @provider.readable?&&  !@provider.new_record? }
> How about using rules we've defined in the controller, so instead of 
> the Proc.new there would be something like 
> ProvidersController.rules_for(:index).
<http://t.co/dlDa9oD>
Would this work properly since in many cases the rules rely on specific 
instance variables?

>
>>
>> 2) The top level block always needs to know if the bottom level ones 
>> are going to all be hidden so that it can hide itself.
>> For example look at Content management
>>
>>
>>      top_level.item :content, _("Content Management"),<snip>  do 
>> |content_sub|
>>        content_sub.item :providers,<snip>   do |providers_sub|
>>          providers_sub.item :edit, _("Basics"),<snip>   :if =>  
>> Proc.new { !@provider.nil?&&  @provider.readable?&&  
>> !@provider.new_record? }
>> <snip>
>>        end if Provider.any_readable?(current_organization)
>> <snip>
>>        content_sub.item :promotions,<snip>  do |package_sub|
>> <snip>
>>        end if current_organization.readable_for_promotions?
>>        content_sub.item(:changeset,<snip>  if 
>> current_organization.any_changesets_readable?
>>      end if current_organization()&&  
>> (Provider.any_readable?(current_organization)|| 
>> current_organization.readable_for_promotions?&&   
>> current_organization.any_changesets_readable?)  #end content
>>
>> notice that the last condition has to include the rules for the sub 
>> items again to not show up.
>>
>> I was not sure what a good way to refactor this would be so as to 
>> make this file a little more readable. I started with an idea of 
>> creating a navigation_rules, that would embed the rules and some how 
>> we have line refer to that rules file and figure out.
>> Any suggestions. May be there is some ruby magic that will make this 
>> file totally readable :).. Or may be its perfect as it is now.
> Maybe using custom renderer for navigation, that would check, whether 
> an item has some visible children.
>
> There should be some info on renderers in the wiki for simple-navigation.
> github.com/andi/simple-navigation/wiki/Registering-Renderers
>
>
> I think it's worth of refactoring since there are those duplications.
>
> -- Ivan
>>
>> Partha
>>
>>
>> _______________________________________________
>> katello-devel mailing list
>> katello-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/katello-devel
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/katello-devel/attachments/20110824/782ff9e4/attachment.htm>