Merge pull request #1294 from dradis/feature/issue-entry-association-…

…fields

Implement field differ and sync for DiffedContent

app/models/concerns/has_fields.rb

@@ -59,6 +59,16 @@ def dradis_has_fields_for(container_field)
           updated_fields.to_a.map { |h| "#[#{h[0]}]#\n#{h[1]}" }.join("\n\n")
         )
       end
+
+      # Completely removes the field (field header and value) from the content
+      define_method :delete_field do |field|
+        updated_fields = fields
+        updated_fields.except!(field)
+        self.send(
+          :"#{container_field}=",
+          FieldParser.fields_hash_to_source(updated_fields)
+        )
+      end
     end
   end
 

app/models/field_parser.rb

@@ -16,6 +16,19 @@ def self.fields_to_source(serialized_form)
     end.compact.join("\n\n")
   end
 
+  # Convert a hash of field name/value pairs to Dradis-style item content.
+  def self.fields_hash_to_source(fields)
+    fields.map do |field, value|
+      value = value.to_s
+
+      str = ''
+      str << "#[#{field}]#\n" unless field.empty?
+      str << "#{value}" unless value.empty?
+
+      str
+    end.compact.join("\n\n")
+  end
+
   # Parse the contents of the field and split it to return a Hash of field
   # name/value pairs. Field / values are defined using this syntax:
   #

app/services/diffed_content.rb

@@ -1,4 +1,6 @@
 class DiffedContent
+  EXCLUDED_FIELDS = %w(id plugin plugin_id AddonTags)
+
   attr_reader :source, :target
 
   def initialize(source, target)
@@ -9,22 +11,87 @@ def initialize(source, target)
     normalize_content(@target)
   end
 
-  def diff
-    Differ.format = :html
-    differ_result = Differ.diff_by_word(source.content, target.content)
+  def content_diff
+    diff_by_word(source.content, target.content)
+  end
 
-    output = highlighted_string(differ_result)
+  # Lists the fields that have changed between the source and the target and
+  # saves the diffed content for each field.
+  #
+  # Returns a hash with the following structure:
+  # { field: { source: <diffed_source_field>, target: <diffed_target_field> } }
+  def unsynced_fields
+    @unsynced_fields ||=
+      begin
+        fields = @source.fields.except(*EXCLUDED_FIELDS).keys |
+          @target.fields.except(*EXCLUDED_FIELDS).keys
 
-    { source: output[1], target: output[0] }
+        fields.filter_map do |field|
+          source_value = source.fields[field] || ''
+          target_value = target.fields[field] || ''
+
+          if source_value != target_value
+            [field, diff_by_word(source_value, target_value)]
+          end
+        end.to_h
+      end
   end
 
   def changed?
     source.updated_at != target.updated_at &&
       source.content != target.content
   end
 
+  def content_for_update(field_params = nil)
+    if field_params
+      {
+        source: content_with_updated_field_from_target(field: field_params, source: @target.reload, target: @source.reload),
+        target: content_with_updated_field_from_target(field: field_params, source: @source.reload, target: @target.reload)
+      }
+    else
+      { source: @target.content, target: @source.content }
+    end
+  end
+
   private
 
+  # Given a target record, update its field depending on the following cases:
+  # 1) If the source record has the existing field:
+  #   1.1) If the target record also has the existing field, update the value
+  #     with the value from the source record
+  #   1.2) If the target record does not have the field, insert the field
+  #     at the same index where the field is present in the source record
+  # 2) If the source record is missing the field, delete the field in the
+  #   target record
+  def content_with_updated_field_from_target(field:, source:, target:)
+    source_fields = source.fields.keys
+    source_index = source_fields.excluding(*EXCLUDED_FIELDS).index(field)
+
+    # Case 1)
+    if source_fields.include?(field)
+      # Case 1.1)
+      if target.fields.keys.include?(field)
+        target.set_field(field, source.fields[field])
+      # Case 1.2)
+      else
+        updated_fields = target.fields.to_a.insert(source_index, [field, source.fields[field]])
+        FieldParser.fields_hash_to_source(updated_fields.compact)
+      end
+    # Case 2)
+    else
+      target.delete_field(field)
+    end
+  end
+
+  def diff_by_word(source_content, target_content)
+    Differ.format = :html
+    differ_result = Differ.diff_by_word(source_content, target_content)
+
+    output = highlighted_string(differ_result)
+
+    { source: output[1], target: output[0] }
+  end
+
   def normalize_content(record)
     fields = record.fields.except('id', 'plugin', 'plugin_id')
 

spec/services/diffed_content_spec.rb

@@ -6,9 +6,9 @@
 
   subject { described_class.new(issue1, issue2) }
 
-  describe '#diff' do
+  describe '#content_diff' do
     it 'returns the diff' do
-      expect(subject.diff).to eq({
+      expect(subject.content_diff).to eq({
         source: "#[Title]#\n<mark>Issue1</mark>\n",
         target: "#[Title]#\n<mark>Issue2</mark>\n"
       })
@@ -30,4 +30,77 @@
       end
     end
   end
+
+  describe '#unsynced_fields' do
+    it 'returns the fields that have changed between the source and the target' do
+      expect(subject.unsynced_fields).to eq({
+        'Title' => {
+          source: '<mark>Issue1</mark>',
+          target: '<mark>Issue2</mark>'
+        }
+      })
+    end
+  end
+
+  describe '#content_for_update' do
+    context 'field_params is present' do
+      context 'the field is present in both the source and target' do
+        it 'returns the updated content' do
+          expect(subject.content_for_update('Title')).to eq({
+            :source => "#[Title]#\n#{issue2.reload.title}",
+            :target => "#[Title]#\n#{issue1.reload.title}"
+          })
+        end
+      end
+
+      context 'the field is not present in the source' do
+        before do
+          issue2.update(content: "#[Title]#\nIssue2\n\n#[Description]#\nTest Description\n")
+        end
+
+        it 'returns the updated content' do
+          expect(subject.content_for_update('Description')).to eq({
+            :source => "#[Title]#\n#{issue1.reload.title}\n\n#[Description]#\nTest Description",
+            :target => "#[Title]#\n#{issue2.reload.title}"
+          })
+        end
+      end
+
+      context 'the field is blank in the issue' do
+        before do
+          issue1.update(content: "#[Title]#\nIssue1\n\n#[Description]#\n\n")
+          issue2.update(content: "#[Title]#\nIssue2\n\n#[Description]#\nTest Description\n")
+        end
+
+        it 'returns the updated content' do
+          expect(subject.content_for_update('Description')).to eq({
+            :source => "#[Title]#\n#{issue1.reload.title}\n\n#[Description]#\nTest Description",
+            :target => "#[Title]#\n#{issue2.reload.title}\n\n#[Description]#\n"
+          })
+        end
+      end
+
+      context 'the field is found on an index not present in the issue' do
+        before do
+          issue2.update(content: "#[Title]#\nIssue2\n\n#[Description]#\nTest Description\n\n#[Mitigation]#\nTest Mitigation\n")
+        end
+
+        it 'returns the updated content' do
+          expect(subject.content_for_update('Mitigation')).to eq({
+            :source => "#[Title]#\n#{issue1.reload.title}\n\n#[Mitigation]#\nTest Mitigation",
+            :target => "#[Title]#\n#{issue2.reload.title}\n\n#[Description]#\nTest Description"
+          })
+        end
+      end
+    end
+
+    context 'field_params is not present' do
+      it 'returns the issue and entry content' do
+        expect(subject.content_for_update(nil)).to eq({
+          :source => "#[Title]#\n#{issue2.reload.title}\n",
+          :target => "#[Title]#\n#{issue1.reload.title}\n"
+        })
+      end
+    end
+  end
 end