Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement field differ and sync for DiffedContent #1294

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/models/concerns/has_fields.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 13 additions & 0 deletions app/models/field_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:
#
Expand Down
77 changes: 72 additions & 5 deletions app/services/diffed_content.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class DiffedContent
EXCLUDED_FIELDS = %w(id plugin plugin_id AddonTags)

attr_reader :source, :target

def initialize(source, target)
Expand All @@ -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')

Expand Down
77 changes: 75 additions & 2 deletions spec/services/diffed_content_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
})
Expand All @@ -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