Couple of comments inline.

-- 
Jacob Helwig

On Fri, 15 Oct 2010 17:16:11 -0700, Jacob Helwig wrote:
> 
> From: Nick Lewis <[email protected]>
> 
> Added methods to list nodes/groups/classes, along with the groups
> via which they were inherited.
> 
> Signed-off-by: Nick Lewis <[email protected]>
> ---
>  app/models/node.rb                    |    2 +-
>  app/models/node_class.rb              |   15 +++++++++
>  app/models/node_group.rb              |   34 +++++++++++++++++++-
>  app/views/node_classes/show.html.haml |    2 +-
>  app/views/node_groups/show.html.haml  |   38 +++++++++++-----------
>  lib/node_group_graph.rb               |   54 ++++++++++++++++++++++++++++++--
>  spec/models/node_spec.rb              |   17 ++++++++--
>  7 files changed, 131 insertions(+), 31 deletions(-)
> 
*snip*

> diff --git a/app/views/node_classes/show.html.haml 
> b/app/views/node_classes/show.html.haml
> index b6a035c..c792214 100644
> --- a/app/views/node_classes/show.html.haml
> +++ b/app/views/node_classes/show.html.haml
> @@ -15,6 +15,6 @@
>      .section
>        %h3 Nodes in this class
>        - if @node_class.nodes.present?
> -        = render 'nodes/nodes', :nodes => @node_class.nodes
> +        = render 'nodes/nodes', :nodes => 
> @node_class.node_list.map(&:first), :container => @node_class

What about { ... }.keys instead of { ... }.map(&:first) ?

>        - else
>          = describe_no_matches_for :nodes, :class
> diff --git a/app/views/node_groups/show.html.haml 
> b/app/views/node_groups/show.html.haml
> index 9b7f084..3a9743d 100644
> --- a/app/views/node_groups/show.html.haml
> +++ b/app/views/node_groups/show.html.haml
> @@ -9,27 +9,27 @@
>        %li= link_to 'Destroy', @node_group, :confirm => 'Are you sure?', 
> :method => :delete, :class => "delete button"
>  
>    .item
> -    .section
> -      %h3 Parameters
> -      - unless @node_group.parameters.blank?
> -        = inspector_table @node_group.parameters.to_hash
> -      - else
> -        = describe_no_matches_as 'No parameters'
> -
> -    .section.half
> -      %h3 Inherited groups
> -      - unless @node_group.node_groups.blank?
> -        = inspector_table @node_group.node_groups, :name, false, :link => 
> true
> -      - else
> -        = describe_no_matches_as 'No groups'
> +    = render 'shared/parameters', :resource => @node_group
> +    = render 'shared/groups', :resource => @node_group
> +    = render 'shared/classes', :resource => @node_group
>  
>      .section.half
> -      %h3 Classes
> -      - unless @node_group.node_classes.blank?
> -        = inspector_table @node_group.node_classes, :name, false, :link => 
> true
> +      %h3 Derived groups
> +      - unless @node_group.node_group_child_list.empty?
> +        %table.inspector
> +          %thead
> +            %tr
> +              %th Group
> +              %th Source groups
> +          %tbody
> +            - @node_group.node_group_child_list.each do |group,parents|
> +              %tr
> +                %td
> +                  %strong= link_to(group.name,group)
> +                %td
> +                  = parents.map{|p| link_to(p.name,p)}.join(", ") unless 
> parents.include?(@node_group)
>        - else
> -        = describe_no_matches_as 'No classes'
> -
> +        = describe_no_matches_as 'No child groups'

"No derived groups" to match the h3? Alternatively, switch the h3 to
match this?)

>    .item
>      - if @node_group.nodes.present?
>        .section
> @@ -37,6 +37,6 @@
>      .section
>        %h3 Nodes for this group
>        - if @node_group.nodes.present?
> -        = render 'nodes/nodes', :nodes => @node_group.nodes
> +        = render 'nodes/nodes', :nodes => 
> @node_group.node_list.map(&:first), :container => @node_group

Same comment about .keys as above.

*snip*

> -- 
> 1.7.3.1
> 

Attachment: signature.asc
Description: Digital signature

Reply via email to