Display value differences to actual environment (#158)

* Add `diffy` to create nice html diffs of values #151

* Display value differences to actual env #151

If a key is displayed from a different environment
than the node is actually in _and_ a file with
the same path exists in the original environment
_and_ the value in that file differs from the one
displayed, show the differences.

Uses the `diffy` gem to highlight the differences.

* Fix rubocop issues.

Gemfile

@@ -34,6 +34,7 @@ gem 'net-ldap', require: "net/ldap"
 gem 'breadcrumbs_on_rails'
 gem 'cancancan'
 gem 'ruby-saml'
+gem 'diffy'
 
 # To use retry middleware with Faraday v2.0+
 gem 'faraday-retry'

Gemfile.lock

@@ -125,6 +125,7 @@ GEM
       railties (>= 6.0.0)
     date (3.3.3)
     deep_merge (1.2.2)
+    diffy (3.4.2)
     docile (1.4.0)
     erubi (1.12.0)
     execjs (2.8.1)
@@ -423,6 +424,7 @@ DEPENDENCIES
   cancancan
   capybara (>= 2.15)
   dartsass-rails
+  diffy
   factory_bot_rails
   faker
   faraday-retry

app/assets/stylesheets/application.scss

@@ -41,3 +41,18 @@ ol.breadcrumb li:first-child {
   width: 1em;
   height: 1em;
 }
+
+// Diffy Styles
+.diff{overflow:auto;}
+.diff ul{background:#fff;overflow:auto;font-size:13px;list-style:none;margin:0;padding:0;display:table;width:100%;}
+.diff del, .diff ins{display:block;text-decoration:none;}
+.diff li{padding:0; display:table-row;margin: 0;height:1em;}
+.diff li.ins{background:#dfd; color:#080}
+.diff li.del{background:#fee; color:#b00}
+.diff li:hover{background:#ffc}
+/* try 'whitespace:pre;' if you don't want lines to wrap */
+.diff del, .diff ins, .diff span{white-space:pre-wrap;font-family:courier;}
+.diff del strong{font-weight:normal;background:#fcc;}
+.diff ins strong{font-weight:normal;background:#9f9;}
+.diff li.diff-comment { display: none; }
+.diff li.diff-block-info { background: none repeat scroll 0 0 gray; }

app/models/data_file.rb

@@ -66,4 +66,33 @@ def replaced_from_git?
   def to_param
     path
   end
+
+  def id
+    [node&.name, path].join("-").parameterize
+  end
+
+  def has_differing_value_in_original_environment?(key)
+    return false unless environment && node.environment
+    return false if environment == node.environment
+
+    candidate_files = node.environment.hierarchies.map do |h|
+      self.class.new(hierarchy: h, path:, node:)
+    end
+    other_file = candidate_files.find(&:exist?)
+    return false unless other_file
+
+    other_key = Key.new(environment: node.environment, name: key.name)
+    return false if value_for(key:).value == other_file.value_for(key: other_key).value
+
+    true
+  end
+
+  def value_from_original_environment(key:)
+    candidate_files = (node&.environment&.hierarchies || []).map do |h|
+      self.class.new(hierarchy: h, path:, node:)
+    end
+    other_file = candidate_files.find(&:exist?)
+    other_key = Key.new(environment: node.environment, name: key.name)
+    other_file.value_for(key: other_key)
+  end
 end

app/views/keys/_diff.html.erb

@@ -0,0 +1,15 @@
+<% split_diff = Diffy::SplitDiff.new(file.value_from_original_environment(key: @key).value, value.value, format: :html) %>
+<table class="table">
+  <thead>
+    <tr>
+      <th><%= @node.environment.name %></th>
+      <th><%= @environment.name %></th>
+    </tr>
+  </thead>
+  <tbody>
+    <tr>
+      <td><%= raw(split_diff.left) %></td>
+      <td><%= raw(split_diff.right) %></td>
+    </tr>
+  </tbody>
+</table>

app/views/keys/_value.html.erb

@@ -0,0 +1,22 @@
+<% if file.has_differing_value_in_original_environment?(@key) %>
+  <ul class="nav nav-tabs mb-2" role="tablist">
+    <li class="nav-item" role="presentation">
+      <button id="<%= "tab-value-#{dom_id(file)}" %>" class="nav-link active" data-target="<%= "#value-#{dom_id(file)}" %>" data-toggle="tab" type="button" role="tab" aria-controls="<%= "value-#{dom_id(file)}" %>" aria-selected="true">Value</button>
+    </li>
+    <li class="nav-item" role="presentation">
+      <button id="<%= "tab-diff-#{dom_id(file)}" %>" class="nav-link" data-target="<%= "#diff-#{dom_id(file)}" %>" data-toggle="tab" type="button" role="tab" aria-controls="<%= "diff-#{dom_id(file)}" %>" aria-selected="false">Diff</button>
+    </li>
+  </ul>
+
+  <div class="tab-content">
+    <div id="<%= "value-#{dom_id(file)}" %>" class="tab-pane fade show active" role="tabpanel" aria-labelledby="<%= "tab-value-#{dom_id(file)}" %>">
+      <%= render "form", file: file, value: value %>
+    </div>
+
+    <div id="<%= "diff-#{dom_id(file)}" %>" class="tab-pane fade" role="tabpanel" aria-labelledby="<%= "tab-diff-#{dom_id(file)}" %>">
+      <%= render "diff", file: file, value: value %>
+    </div>
+  </div>
+<% else %>
+  <%= render "form", file: file, value: value %>
+<% end %>

app/views/keys/show.html.erb

@@ -55,7 +55,7 @@
 
             <div id="collapse-<%= index %>" class="collapse" aria-labelledby="path-<%= index %>">
               <div class="card-body">
-                <%= render "form", file: file, value: value %>
+                <%= render "value", file: file, value: value %>
               </div>
             </div>
           </div>

test/fixtures/files/puppet/environments/development/data/role/hdm_test.yaml

@@ -1,3 +1,5 @@
 ---
 foobar::firstrun::linux_classes:
   hostname: hostname-role
+hdm::integer: 1
+hdm::float: 2.3

test/fixtures/files/puppet/environments/globs/data/role/hdm_test.yaml

@@ -1,3 +1,5 @@
 ---
 foobar::firstrun::linux_classes:
   hostname: hostname-role
+hdm::integer: 1
+hdm::float: 4.5

test/integration/key_search_test.rb

@@ -6,11 +6,11 @@ class KeySearchTest < ActionDispatch::IntegrationTest
     sign_in_as(@user)
   end
 
-  test "user withouth groups can find key" do
+  test "user without groups can find key" do
     get environment_key_files_path(environment_id: "development", key_id: "hdm::integer")
 
     assert_response :success
-    assert_select "p.lead", /Found\s+key\s+hdm::integer\s+in\s+1\s+file./
+    assert_select "p.lead", /Found\s+key\s+hdm::integer\s+in\s+2\s+files./
   end
 
   test "user with group that forbids access cannot find key" do

test/models/data_file_test.rb

@@ -0,0 +1,58 @@
+require 'test_helper'
+
+class DataFileTest < ActiveSupport::TestCase
+  setup do
+    @original_environment = Environment.new(name: "development")
+    @node = Node.new(environment: @original_environment, hostname: "test.host")
+    @globs_environment = Environment.new(name: "globs")
+    @path = "role/hdm_test.yaml"
+  end
+
+  test "#has_differing_value_in_original_environment? returns false if environments are the same" do
+    hierarchy = Hierarchy.find(@original_environment, "Eyaml hierarchy")
+    data_file = DataFile.new(hierarchy:, node: @node, path: @path)
+    key = Key.new(environment: @original_environment, name: "hdm::integer")
+
+    assert_not data_file.has_differing_value_in_original_environment?(key)
+  end
+
+  test "#has_differing_value_in_original_environment? returns false if original environment has no file with the same path" do
+    hierarchy = Hierarchy.find(@globs_environment, "Common")
+    data_file = DataFile.new(hierarchy:, node: @node, path: "common/foobar.yaml")
+    key = Key.new(environment: @globs_environment, name: "hdm::integer")
+
+    assert_not data_file.has_differing_value_in_original_environment?(key)
+  end
+
+  test "#has_differing_value_in_original_environment? returns false if original environment's value is the same" do
+    hierarchy = Hierarchy.find(@globs_environment, "Eyaml hierarchy")
+    data_file = DataFile.new(hierarchy:, node: @node, path: @path)
+    key = Key.new(environment: @globs_environment, name: "hdm::integer")
+
+    assert_not data_file.has_differing_value_in_original_environment?(key)
+  end
+
+  test "#has_differing_value_in_original_environment? returns true if original environment's value is different" do
+    hierarchy = Hierarchy.find(@globs_environment, "Eyaml hierarchy")
+    data_file = DataFile.new(hierarchy:, node: @node, path: @path)
+    key = Key.new(environment: @globs_environment, name: "hdm::float")
+
+    assert data_file.has_differing_value_in_original_environment?(key)
+  end
+
+  test "#value_for returns matching value from given environment" do
+    hierarchy = Hierarchy.find(@globs_environment, "Eyaml hierarchy")
+    data_file = DataFile.new(hierarchy:, node: @node, path: @path)
+    key = Key.new(environment: @globs_environment, name: "hdm::float")
+
+    assert_equal "4.5", data_file.value_for(key:).value
+  end
+
+  test "#value_from_original_environment returns matching value from the node's original environment" do
+    hierarchy = Hierarchy.find(@globs_environment, "Eyaml hierarchy")
+    data_file = DataFile.new(hierarchy:, node: @node, path: @path)
+    key = Key.new(environment: @globs_environment, name: "hdm::float")
+
+    assert_equal "2.3", data_file.value_from_original_environment(key:).value
+  end
+end