Merge branch 'ticket/next/5866' into next

* ticket/next/5866:
  (#5866) Get a consolidated report for baseline diffs in a group
  (#5866) Split dividing diff into pass and fail into a method
  maint: Fixing bad view logic related to enable_read_only_mode
  maint: Remove unused, user related views

app/controllers/node_groups_controller.rb

@@ -4,4 +4,31 @@ class NodeGroupsController < InheritedResources::Base
   before_filter :raise_if_enable_read_only_mode, :only => [:new, :edit, :create, :update, :destroy]
 
   include SearchableIndex
+
+  def diff
+    @node_group = NodeGroup.find(params[:id])
+
+    @nodes_without_latest_inspect_reports = []
+    @nodes_without_baselines = []
+    @nodes_without_differences = []
+    @nodes_with_differences = []
+    @node_group.all_nodes.each do |node|
+      @nodes_without_latest_inspect_reports << node and next unless node.last_inspect_report
+      @nodes_without_baselines << node and next unless node.baseline_report
+
+      report_diff = node.baseline_report.diff(node.last_inspect_report)
+      resource_statuses = Report.divide_diff_into_pass_and_fail(report_diff)
+
+      if resource_statuses[:failure].empty?
+        @nodes_without_differences << node
+      else
+        @nodes_with_differences << {
+          :baseline_report     => node.baseline_report,
+          :last_inspect_report => node.last_inspect_report,
+          :report_diff         => report_diff,
+          :resource_statuses   => resource_statuses,
+        }
+      end
+    end
+  end
 end

app/controllers/reports_controller.rb

@@ -41,14 +41,7 @@ def diff
     @my_report = Report.find(params[:id])
     @baseline_report = Report.find(params[:baseline_id])
     @diff = @baseline_report.diff(@my_report)
-    @resource_statuses = {:failure => [], :pass => []}
-    @diff.each do |resource, differences|
-      if ! differences.empty?
-        @resource_statuses[:failure] << resource
-      else
-        @resource_statuses[:pass] << resource
-      end
-    end
+    @resource_statuses = Report.divide_diff_into_pass_and_fail(@diff)
   end
 
   def make_baseline

app/models/report.rb

@@ -73,6 +73,18 @@ def diff(comparison_report)
     diff_stuff
   end
 
+  def self.divide_diff_into_pass_and_fail(diff)
+    divided_diff = {:failure => [], :pass => []}
+    diff.each do |resource, differences|
+      if ! differences.empty?
+        divided_diff[:failure] << resource
+      else
+        divided_diff[:pass] << resource
+      end
+    end
+    divided_diff
+  end
+
   def self.attribute_hash_from(report_hash)
     attribute_hash = report_hash.dup
     attribute_hash["logs_attributes"] = attribute_hash.delete("logs")

app/views/layouts/application.html.haml

@@ -20,7 +20,6 @@
     #wrap
       #header
         = render 'shared/global_nav'
-        = render 'shared/secondary_nav'
       #content
         - [:success, :warning, :failure].each do |type|
           - if message = flash[type]

app/views/node_classes/show.html.haml

@@ -5,7 +5,7 @@
       Class:
       = @node_class.name
     %ul.actions
-      - if SETTINGS.enable_read_only_mode
+      - unless SETTINGS.enable_read_only_mode
         %li= link_to 'Edit', edit_node_class_path(@node_class), :class => "edit button"
         %li= link_to 'Destroy', @node_class, :confirm => 'Are you sure?', :method => :delete, :class => "delete button"
 

app/views/node_groups/diff.html.haml

@@ -0,0 +1,29 @@
+#sidebar= render 'shared/node_manager_sidebar'
+#main
+  .header
+    %h2
+      Diff Report for
+      = @node_group.name
+  - unless @nodes_without_latest_inspect_reports.empty?
+    .item
+      %h2 No latest inspect report
+      = @nodes_without_latest_inspect_reports.map {|node| link_to node.name, node_path(node)}.join(", ")
+  - unless @nodes_without_baselines.empty?
+    .item
+      %h2 No baseline
+      = @nodes_without_baselines.map {|node| link_to node.name, node_path(node)}.join(", ")
+  - unless @nodes_without_differences.empty?
+    .item
+      %h2 No differences from baseline
+      = @nodes_without_differences.map {|node| link_to node.name, node_path(node)}.join(", ")
+  - unless @nodes_with_differences.empty?
+    .item
+      %h2 Nodes with difference from baseline
+      %a{ :href => '#', :class => ['expand-all', 'button'] } expand all
+      %br
+      - @nodes_with_differences.each do |diff_information|
+        = render 'reports/diff',                                        |
+          :baseline_report   => diff_information[:baseline_report],     |
+          :my_report         => diff_information[:last_inspect_report], |
+          :resource_statuses => diff_information[:resource_statuses],   |
+          :diff              => diff_information[:report_diff]

app/views/node_groups/show.html.haml

@@ -5,7 +5,7 @@
       Group:
       = @node_group.name
     %ul.actions
-      - if SETTINGS.enable_read_only_mode
+      - unless SETTINGS.enable_read_only_mode
         %li= link_to 'Edit', edit_node_group_path(@node_group), :class => "edit button", :rel => 'inspect'
         %li= link_to 'Destroy', @node_group, :confirm => 'Are you sure?', :method => :delete, :class => "delete button"
 
@@ -38,8 +38,12 @@
     - if @node_group.all_nodes.present?
       .section
         = render 'statuses/run_failure', :nodes => @node_group.all_nodes
+  .item
     .section
-      %h3 Nodes for this group
+      .header
+        %h2 Nodes for this group
+        %ul.actions
+          %li= link_to "Diffs against own baselines", diff_node_group_path(@node_group), :class => 'button'
       - if @node_group.all_nodes.present?
         = render 'nodes/nodes', :nodes => @node_group.all_nodes, :container => @node_group
       - else

app/views/reports/_diff.html.haml

@@ -0,0 +1,37 @@
+- show_expand_all_link ||= false
+.header
+  %h4
+    Comparing
+    = render 'reports/report_title', :report => my_report
+    %br
+    against baseline
+    = render 'reports/report_title', :report => baseline_report
+.item
+  %h4
+    = pluralize(resource_statuses[:failure].length, 'Mismatch')
+    - if resource_statuses[:failure].present? && show_expand_all_link
+      %a{ :href => '#', :class => 'expand-all' } (expand all)
+  - if resource_statuses[:failure].present?
+    %dl#baseline-diff-report
+      - resource_statuses[:failure].each do |resource|
+        %dt{:class => cycle( 'odd', 'even' )}
+          = link_to h(resource), {}, {:class => 'expandable-link collapsed-link'}
+        %dd.expandable.collapsed
+          %table
+            %tr
+              %th Property
+              %th Actual
+              %th Baseline
+            - diff[ resource ].each do |property, expected_actual|
+              %tr
+                - baseline, actual = expected_actual
+                %td= h property
+                %td= h actual
+                %td= h baseline
+  %h4= pluralize(resource_statuses[:pass].length, 'Match')
+  - if resource_statuses[:pass].present?
+    %dl#baseline-diff-report
+      - resource_statuses[:pass].each do |resource|
+        %dt{:class => cycle( 'odd', 'even' )}
+          %span{:class => 'non-expandable-bullet'}
+            = h(resource)

app/views/reports/diff.html.haml

@@ -1,38 +1,8 @@
 #sidebar= render 'shared/node_manager_sidebar'
 #main
-  .header
-    %h2
-      Comparing
-      = render 'report_title', :report => @my_report
-      %br
-      against baseline
-      = render 'report_title', :report => @baseline_report
-  .item
-    %h4
-      = pluralize(@resource_statuses[:failure].length, 'Mismatch')
-      - if @resource_statuses[:failure].present?
-        %a{ :href => '#', :class => 'expand-all' } (expand all)
-    - if @resource_statuses[:failure].present?
-      %dl#baseline-diff-report
-        - @resource_statuses[:failure].each do |resource|
-          %dt{:class => cycle( 'odd', 'even' )}
-            = link_to h(resource), {}, {:class => 'expandable-link collapsed-link'}
-          %dd.expandable.collapsed
-            %table
-              %tr
-                %th Property
-                %th Actual
-                %th Baseline
-              - @diff[ resource ].each do |property, expected_actual|
-                %tr
-                  - baseline, actual = expected_actual
-                  %td= h property
-                  %td= h actual
-                  %td= h baseline
-    %h4= pluralize(@resource_statuses[:pass].length, 'Match')
-    - if @resource_statuses[:pass].present?
-      %dl#baseline-diff-report
-        - @resource_statuses[:pass].each do |resource|
-          %dt{:class => cycle( 'odd', 'even' )}
-            %span{:class => 'non-expandable-bullet'}
-              = h(resource)
+  = render 'diff',                               |
+    :baseline_report      => @baseline_report,   |
+    :my_report            => @my_report,         |
+    :resource_statuses    => @resource_statuses, |
+    :diff                 => @diff,              |
+    :show_expand_all_link => true

config/routes.rb

@@ -3,9 +3,11 @@
     classes.resources :nodes, :requirements => {:id => /.*/}
   end
 
-  map.resources :node_groups, :collection => {:search => :get} do |groups|
-    groups.resources :nodes, :requirements => {:id => /.*/}
-  end
+  map.resources :node_groups,
+    :member     => { :diff  => :get },
+    :collection => {:search => :get } do |groups|
+      groups.resources :nodes, :requirements => {:id => /.*/}
+    end
 
   map.resources :nodes,
     :member => {

spec/controllers/node_groups_controller_spec.rb

@@ -3,10 +3,121 @@
 require 'shared_behaviors/sorted_index'
 
 describe NodeGroupsController do
+  integrate_views
   def model; NodeGroup end
 
   it_should_behave_like "without JSON pagination"
   it_should_behave_like "with search by q and tag"
   it_should_behave_like "sorted index"
 
+  describe "when diffing latest inspect report against baseline and" do
+    before :each do
+      @node_group = NodeGroup.generate!
+      @node = Node.generate! :name => "node_it_all"
+      @node_group.nodes << @node
+
+      @child_node_group = NodeGroup.generate!
+      @child_node = Node.generate! :name => "nema_node"
+      @child_node_group.nodes << @node
+
+      @node_group.node_group_children << @child_node_group
+    end
+
+    describe "the node has no inspect reports" do
+      it "should not produce a node diff and should keep track that there is no inspect report" do
+        get :diff, :id => @node_group.id
+        response.should be_success
+        assigns[:nodes_without_latest_inspect_reports].should == [@node]
+        assigns[:nodes_without_baselines].should be_empty
+        assigns[:nodes_with_differences].should be_empty
+        assigns[:nodes_without_differences].should be_empty
+      end
+    end
+
+    describe "the node has inspect reports but no baseline" do
+      it "should not produce a node diff and should keep track that there is no baseline" do
+        Report.generate!(:host => @node.name, :kind => 'inspect', :time => 2.hours.ago)
+
+        get :diff, :id => @node_group.id
+        response.should be_success
+        assigns[:nodes_without_latest_inspect_reports].should be_empty
+        assigns[:nodes_without_baselines].should == [@node]
+        assigns[:nodes_with_differences].should be_empty
+        assigns[:nodes_without_differences].should be_empty
+      end
+    end
+
+    describe "there is a baseline and latest inspect report" do
+      before :each do
+        @baseline = Report.generate!(:host => @node.name, :kind => 'inspect', :time => 2.hours.ago)
+        @baseline_status = ResourceStatus.generate!(
+          :report        => @baseline,
+          :resource_type => 'File',
+          :title         => '/tmp/test'
+        )
+
+        @latest = Report.generate!(:host => @node.name, :kind => 'inspect', :time => 1.hour.ago)
+        @latest_status = ResourceStatus.generate!(
+          :report        => @latest,
+          :resource_type => 'File',
+          :title         => '/tmp/test'
+        )
+
+        @node.reports = [@baseline, @latest]
+        @baseline.baseline!
+        @node.last_inspect_report = @latest
+      end
+
+      it "should not produce a node diff if the node doesn't have any differences" do
+        @baseline_event = ResourceEvent.generate!(
+          :resource_status => @baseline_status,
+          :property        => 'content',
+          :previous_value  => '{md5}0b8b61ed7bce7ffb93cedc19845468cc'
+        )
+
+        @latest_event = ResourceEvent.generate!(
+          :resource_status => @latest_status,
+          :property        => 'content',
+          :previous_value  => '{md5}0b8b61ed7bce7ffb93cedc19845468cc'
+        )
+
+        get :diff, :id => @node_group.id
+
+        response.should be_success
+
+        assigns[:nodes_without_latest_inspect_reports].should be_empty
+        assigns[:nodes_without_baselines].should be_empty
+        assigns[:nodes_with_differences].should be_empty
+        assigns[:nodes_without_differences].should == [@node]
+      end
+
+      it "should keep track of appropriate information when reports have differences" do
+        @baseline_event_with_difference = ResourceEvent.generate!(
+          :resource_status => @baseline_status,
+          :property        => 'content',
+          :previous_value  => '{md5}abcd'
+        )
+
+        @latest_event_with_difference = ResourceEvent.generate!(
+          :resource_status => @latest_status,
+          :property        => 'content',
+          :previous_value  => '{md5}efgh'
+        )
+
+        get :diff, :id => @node_group.id
+
+        response.should be_success
+
+        assigns[:nodes_without_latest_inspect_reports].should be_empty
+        assigns[:nodes_without_baselines].should be_empty
+        assigns[:nodes_without_differences].should be_empty
+        assigns[:nodes_with_differences].should == [ {
+          :resource_statuses   => {:pass=>[], :failure=>["File[/tmp/test]"]},
+          :last_inspect_report => @latest,
+          :baseline_report     => @baseline,
+          :report_diff         => {"File[/tmp/test]"=>{:content=>["{md5}abcd", "{md5}efgh"]}}
+        }]
+      end
+    end
+  end
 end