diff --git a/app/assets/stylesheets/shared/_icons.scss b/app/assets/stylesheets/shared/_icons.scss new file mode 100644 index 000000000..d63fbacbf --- /dev/null +++ b/app/assets/stylesheets/shared/_icons.scss @@ -0,0 +1,11 @@ +.fa-check { + color: $green; +} + +.fa-info-circle { + color: $blue; +} + +.fa-times { + color: $red; +} diff --git a/app/assets/stylesheets/shared/_validations.scss b/app/assets/stylesheets/shared/_validations.scss index 5dcb09dcf..dec75f637 100644 --- a/app/assets/stylesheets/shared/_validations.scss +++ b/app/assets/stylesheets/shared/_validations.scss @@ -1,29 +1,18 @@ .validation { - .fa-check { - color: $green; - } - - .fa-info-circle { - color: $blue; - } - - .fa-times { - color: $red; - } - &.validation-feed { .validation-list { + list-style-type: none; margin: 1rem 0 0; padding: 0; li { i { - margin-right: .15rem; + margin-right: 0.15rem; } .text-error { font-size: 90%; - margin: 0 0 .5rem 1.3rem; + margin: 0 0 0.5rem 1.6rem; } &:last-of-type .text-error { diff --git a/app/assets/stylesheets/tylium.scss b/app/assets/stylesheets/tylium.scss index cba0d31e5..64cdfcd2c 100644 --- a/app/assets/stylesheets/tylium.scss +++ b/app/assets/stylesheets/tylium.scss @@ -23,6 +23,7 @@ @import 'shared/datatables'; @import 'shared/editor_toolbar'; @import 'shared/empty_state.scss'; +@import 'shared/icons'; @import 'shared/inline_editable'; @import 'shared/mixins'; @import 'shared/noscript'; diff --git a/app/assets/stylesheets/tylium/modules/_buttons.scss b/app/assets/stylesheets/tylium/modules/_buttons.scss index bd7ee3bf4..fcad08a51 100644 --- a/app/assets/stylesheets/tylium/modules/_buttons.scss +++ b/app/assets/stylesheets/tylium/modules/_buttons.scss @@ -49,6 +49,23 @@ } } +.btn-link { + background-color: transparent; + border: none; + color: $linkColor; + padding: 0; + text-decoration: none; + + &:active, + &:focus, + &:hover, + &:not(:disabled):not(.disabled):active { + background-color: transparent; + box-shadow: none !important; + color: $linkColorHover; + } +} + .btn-placeholder { align-items: center; background-color: $primaryColor; diff --git a/app/assets/stylesheets/tylium/modules/_dots-menu.scss b/app/assets/stylesheets/tylium/modules/_dots-menu.scss index 00ef9293c..416083b4b 100644 --- a/app/assets/stylesheets/tylium/modules/_dots-menu.scss +++ b/app/assets/stylesheets/tylium/modules/_dots-menu.scss @@ -10,7 +10,7 @@ .dots-dropdown-header { font-weight: 400 !important; - + &:hover { color: $linkColor !important; background-color: initial !important; @@ -21,6 +21,10 @@ margin-top: 0.3rem; padding-top: 0.3rem; } + + &:not(:has(+ .dropdown-item)) { + display: none; + } } .dropdown-menu { diff --git a/app/assets/stylesheets/tylium/modules/_modals.scss b/app/assets/stylesheets/tylium/modules/_modals.scss index b58a016b6..aa4b5069d 100644 --- a/app/assets/stylesheets/tylium/modules/_modals.scss +++ b/app/assets/stylesheets/tylium/modules/_modals.scss @@ -1,16 +1,16 @@ .modal { - .modal-dialog { - max-width: 560px; + &:not(.modal-lg, .modal-xl) { + max-width: 560px; + } .modal-content { font-size: 0.9rem; .modal-body { - .add_multiple_nodes_form { display: none; - + .add_multiple_nodes_error { display: none; color: #b94a48; @@ -49,7 +49,6 @@ } &#modal_delete_node { - ul { list-style: disc; padding-left: 30px; @@ -70,7 +69,7 @@ text-decoration: underline !important; } } - + .invalid-selection { cursor: not-allowed; } @@ -109,13 +108,12 @@ } &#try-pro { - .modal-dialog { max-width: 80%; - + .modal-body { height: 80vh; - + iframe { border: none; top: 0; @@ -125,7 +123,7 @@ width: 100%; } } - + .modal-header h5 span { color: $primaryColor; } diff --git a/app/models/concerns/has_fields.rb b/app/models/concerns/has_fields.rb index 27875cd6d..61ff4dafb 100644 --- a/app/models/concerns/has_fields.rb +++ b/app/models/concerns/has_fields.rb @@ -11,13 +11,22 @@ def local_fields # Extract a Title file if one exists. def title fields.fetch( - "Title", - "(No #[Title]# field)" + 'Title', + '(No #[Title]# field)' ) end def title? - fields["Title"].present? + fields['Title'].present? + end + + private + + def update_container(container_field, updated_fields) + self.send( + :"#{container_field}=", + FieldParser.fields_hash_to_source(updated_fields) + ) end module ClassMethods @@ -52,14 +61,17 @@ def dradis_has_fields_for(container_field) define_method :set_field do |field, value| # Don't use 'fields' as a local variable name or it conflicts with the # #fields getter method - updated_fields = fields + updated_fields = fields updated_fields[field] = value - self.send( - :"#{container_field}=", - updated_fields.to_a.map { |h| "#[#{h[0]}]#\n#{h[1]}" }.join("\n\n") - ) + self.update_container(container_field, updated_fields) + 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.update_container(container_field, updated_fields) end end end - end diff --git a/app/models/field_parser.rb b/app/models/field_parser.rb index cb743aa15..3b5de25fd 100644 --- a/app/models/field_parser.rb +++ b/app/models/field_parser.rb @@ -1,19 +1,15 @@ class FieldParser FIELDS_REGEX = /#\[(.+?)\]#[\r|\n](.*?)(?=#\[|\z)/m - FIELDLESS_REGEX = /^([\s\S]*?)(?=\n{,2}#\[.+?\]#|\z)/ + HEADERLESS_REGEX = /^([\s\S]*?)(?=\n{,2}#\[.+?\]#|\z)/ # Convert serialized form data to Dradis-style item content. def self.fields_to_source(serialized_form) - serialized_form.each_slice(2).map do |field_name, field_value| - field = field_name[:value] - value = field_value[:value] - - str = '' - str << "#[#{field}]#\n" unless field.empty? - str << "#{value}" unless value.empty? + serialized_form.each_slice(2).map(&to_source).compact.join("\n\n") + end - str - end.compact.join("\n\n") + # Convert a hash of field name/value pairs to Dradis-style item content. + def self.fields_hash_to_source(fields) + fields.map(&to_source).compact.join("\n\n") end # Parse the contents of the field and split it to return a Hash of field @@ -38,7 +34,7 @@ def self.source_to_fields(string) # #[Description]# # Lorem ipsum... # - # If the string contains a fieldless string, it will be prepended to + # If the string contains a headerless field, it will be prepended to # the result. E.g. # # Line 1 @@ -54,18 +50,33 @@ def self.source_to_fields_array(string) field.map(&:strip) end - fieldless_string = parse_fieldless_string(string) + headerless_fields = parse_fields_without_headers(string) - if fieldless_string.present? - array.prepend(['', fieldless_string]) + if headerless_fields.present? + array.prepend(['', headerless_fields]) end array end - # Field-less strings are strings that do not have a field header (#[Field]#). + # Headerless fields are strings that do not have a field header (#[Field]#). # This parses all characters before a field header or end of string. - def self.parse_fieldless_string(source) - source[FIELDLESS_REGEX, 1] + def self.parse_fields_without_headers(source) + source[HEADERLESS_REGEX, 1] + end + + private + + def self.to_source + return Proc.new do |field, value| + field = field.is_a?(String) ? field.to_s : field[:value] + value = value.is_a?(String) ? value.to_s : value[:value] + + str = '' + str << "#[#{field}]#\n" unless field.empty? + str << value unless value.empty? + + str + end end end diff --git a/app/services/diffed_content.rb b/app/services/diffed_content.rb new file mode 100644 index 000000000..9a46c6894 --- /dev/null +++ b/app/services/diffed_content.rb @@ -0,0 +1,120 @@ +class DiffedContent + EXCLUDED_FIELDS = %w(id plugin plugin_id AddonTags) + + attr_reader :source, :target + + def initialize(source, target) + @source = source + @target = target + + normalize_content(@source) + normalize_content(@target) + end + + def content_diff + diff_by_word(source.content, target.content) + end + + # 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 + + 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(*EXCLUDED_FIELDS) + + record.content = + fields.map do |field, value| + "#[#{field}]#\n#{value.gsub("\r", '')}\n" + end.join("\n") + end + + def highlighted_string(differ_result) + [:delete, :insert].map do |highlight_type| + result_str = differ_result.dup.to_s + + case highlight_type + when :delete + result_str.gsub!(/(.*?)<\/del>/m, '\1') + result_str.gsub!(/(.*?)<\/ins>/m, '') + when :insert + result_str.gsub!(/(.*?)<\/ins>/m, '\1') + result_str.gsub!(/(.*?)<\/del>/m, '') + end + + result_str.html_safe + end + end +end diff --git a/spec/services/diffed_content_spec.rb b/spec/services/diffed_content_spec.rb new file mode 100644 index 000000000..d2024377c --- /dev/null +++ b/spec/services/diffed_content_spec.rb @@ -0,0 +1,106 @@ +require 'rails_helper' + +describe DiffedContent do + let(:issue1) { create(:issue, text: "#[Title]#\nIssue1\n") } + let(:issue2) { create(:issue, text: "#[Title]#\nIssue2\n") } + + subject { described_class.new(issue1, issue2) } + + describe '#content_diff' do + it 'returns the diff' do + expect(subject.content_diff).to eq({ + source: "#[Title]#\nIssue1\n", + target: "#[Title]#\nIssue2\n" + }) + end + end + + describe '#changed?' do + context 'source and target does not match' do + it 'returns true' do + issue1.update text: 'test', updated_at: Time.now + 1.day + expect(subject.changed?).to eq true + end + end + + context 'source and target matches' do + it 'returns false' do + issue1.update text: issue2.content + expect(subject.changed?).to eq false + 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