diff --git a/CHANGELOG b/CHANGELOG index 0447cf480..68f3c24ce 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ [v#.#.#] ([month] [YYYY]) - Kit Import: Use file name sequencing when a template file with the same name exists - - Node merging: Update associated evidence, notes and child nodes updated_at column on node merge + - Node merging: + - Update associated evidence, notes and child nodes updated_at column on node merge + - Update attachment references in evidence to point to new node - Upgraded gems: - rexml - Bugs fixes: diff --git a/app/controllers/concerns/attachments_copier.rb b/app/controllers/concerns/attachments_copier.rb index 3ad9f5db7..e639e1996 100644 --- a/app/controllers/concerns/attachments_copier.rb +++ b/app/controllers/concerns/attachments_copier.rb @@ -7,14 +7,16 @@ def copy_attachments(record) if attachment new_attachment = attachment.copy_to(record.node) - new_filename = new_attachment.url_encoded_filename - new_path = full_screenshot_path.gsub( - /nodes\/[0-9]+\/attachments\/.+/, - "nodes/#{new_attachment.node_id}/attachments/#{new_filename}" - ) - - record.content = record.content.gsub(full_screenshot_path, new_path) + record.content = updated_record_content(record.content, full_screenshot_path, new_attachment) end end end + + def updated_record_content(record_content, full_screenshot_path, new_attachment) + new_path = full_screenshot_path.gsub( + /nodes\/[0-9]+\/attachments\/.+/, + "nodes/#{new_attachment.node_id}/attachments/#{new_attachment.url_encoded_filename}" + ) + record_content.gsub(full_screenshot_path, new_path) + end end diff --git a/app/services/nodes/merger.rb b/app/services/nodes/merger.rb index 0c6c536bc..1338ab13b 100644 --- a/app/services/nodes/merger.rb +++ b/app/services/nodes/merger.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Nodes::Merger + include AttachmentsCopier + def self.call(target_node, source_node) new(target_node, source_node).call end @@ -90,15 +92,33 @@ def update_properties end def copy_attachments - self.copied_attachments = [] + self.copied_attachments = {} source_node.attachments.each do |attachment| - copied_attachments << attachment.copy_to(target_node) + new_attachment = attachment.copy_to(target_node) + copied_attachments[attachment.filename] = new_attachment + end + update_attachment_references + end + + def update_attachment_references + target_node.evidence_ids.each do |evidence_id| + evidence = Evidence.find(evidence_id) + evidence.content.scan(Attachment::SCREENSHOT_REGEX).each do |screenshot_path| + full_screenshot_path, _, _, _, _, node_id, original_filename, _ = screenshot_path + # skip if the attachment already references the new node + next if node_id == target_node.id + + new_attachment = copied_attachments[original_filename] + new_content = updated_record_content(evidence.content, full_screenshot_path, new_attachment) + + evidence.update_attribute('content', new_content) + end end end def undo_attachments_copy return unless copied_attachments&.any? - copied_attachments.each(&:delete) + copied_attachments.values.each(&:delete) end end diff --git a/spec/services/nodes/merger_spec.rb b/spec/services/nodes/merger_spec.rb index 6e4318c9f..efec15caf 100644 --- a/spec/services/nodes/merger_spec.rb +++ b/spec/services/nodes/merger_spec.rb @@ -80,6 +80,22 @@ FileUtils.rm_rf(Dir.glob(Attachment.pwd + '*')) end + it 'updates evidence screenshot links to point to new node' do + attachment = create(:attachment, node: source_node) + evidence = create(:evidence, + content: "#[Description]#\n\n!/pro/projects/#{source_node.project_id}/nodes/#{source_node.id}/attachments/#{attachment.url_encoded_filename}!", + node: source_node + ) + merge_nodes + + expect(evidence.reload.content) + .to include("/nodes/#{target_node.id}/attachments/#{attachment.url_encoded_filename}") + expect(evidence.reload.content) + .not_to include("/nodes/#{source_node.id}/attachments/#{attachment.url_encoded_filename}") + + FileUtils.rm_rf(Dir.glob(Attachment.pwd + '*')) + end + describe 'property merges' do it 'merges basic properties together' do source_node = create(:node, :with_properties) @@ -223,6 +239,20 @@ FileUtils.rm_rf(Dir.glob(Attachment.pwd + '*')) end + + it 'does not update evidence screenshot links to point to new node' do + attachment = create(:attachment, node: source_node) + evidence = create(:evidence, + content: "#[Description]#\n\n!/pro/projects/#{source_node.project_id}/nodes/#{source_node.id}/attachments/#{attachment.url_encoded_filename}!", + node: source_node + ) + merge_nodes + + expect(evidence.content).to include("/nodes/#{source_node.id}/attachments/#{attachment.url_encoded_filename}") + expect(evidence.content).not_to include("/nodes/#{target_node.id}/attachments/#{attachment.url_encoded_filename}") + + FileUtils.rm_rf(Dir.glob(Attachment.pwd + '*')) + end end end end