From 5434fdb446b36a34015aaef9473a3a07dc723c9b Mon Sep 17 00:00:00 2001 From: Aaron Manaloto Date: Fri, 13 Sep 2024 14:53:39 +0800 Subject: [PATCH] Implement field differ and sync for DiffedContent --- app/models/concerns/has_fields.rb | 10 ++++ app/models/field_parser.rb | 13 +++++ app/services/diffed_content.rb | 77 ++++++++++++++++++++++++++-- spec/services/diffed_content_spec.rb | 77 +++++++++++++++++++++++++++- 4 files changed, 170 insertions(+), 7 deletions(-) diff --git a/app/models/concerns/has_fields.rb b/app/models/concerns/has_fields.rb index 27875cd6d..8059f1efd 100644 --- a/app/models/concerns/has_fields.rb +++ b/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 diff --git a/app/models/field_parser.rb b/app/models/field_parser.rb index cb743aa15..ad45f9c5b 100644 --- a/app/models/field_parser.rb +++ b/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: # diff --git a/app/services/diffed_content.rb b/app/services/diffed_content.rb index 71cfc3396..eebd4368e 100644 --- a/app/services/diffed_content.rb +++ b/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,13 +11,30 @@ 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: , target: } } + 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? @@ -23,8 +42,56 @@ def changed? 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') diff --git a/spec/services/diffed_content_spec.rb b/spec/services/diffed_content_spec.rb index e7877fa0e..21efad449 100644 --- a/spec/services/diffed_content_spec.rb +++ b/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]#\nIssue1\n", target: "#[Title]#\nIssue2\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: 'Issue1', + target: 'Issue2' + } + }) + 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