Set WP's comment created_at to BCF Comment's ModifiedDate

modules/bcf/lib/open_project/bcf/bcf_xml/issue_reader.rb

@@ -151,7 +151,7 @@ module OpenProject::Bcf::BcfXml
     end
 
     def start_date
-      extractor.creation_date unless is_update
+      extractor.creation_date.to_date unless is_update
     end
 
     def update_work_package
@@ -331,10 +331,15 @@ module OpenProject::Bcf::BcfXml
     end
 
     def update_comment(comment_data)
+      if comment_data[:modified_date]
         bcf_comment = issue.comments.find_by(comment_data.slice(:uuid))
-      bcf_comment.journal.update_attribute(:notes, comment_data[:comment])
+        if bcf_comment.journal.created_at < comment_data[:modified_date]
+          bcf_comment.journal.update_attributes(notes: comment_data[:comment],
+                                                created_at: comment_data[:modified_date])
           bcf_comment.journal.save
         end
+      end
+    end
 
     ##
     # Keep a hash map of current status ids for faster lookup

modules/bcf/lib/open_project/bcf/bcf_xml/markup_extractor.rb

@@ -14,54 +14,51 @@ module OpenProject::Bcf::BcfXml
     end
 
     def uuid
-      extract_non_empty :@Guid, attribute: true
+      extract :@Guid, attribute: true
     end
 
     def title
-      extract_non_empty :Title
+      extract :Title
     end
 
     def priority
-      extract_non_empty :Priority
+      extract :Priority
     end
 
     def status
-      extract_non_empty :@TopicStatus, attribute: true
+      extract :@TopicStatus, attribute: true
     end
 
     def type
-      extract_non_empty :@TopicType, attribute: true
+      extract :@TopicType, attribute: true
     end
 
     def description
-      extract_non_empty :Description
+      extract :Description
     end
 
     def author
-      extract_non_empty :CreationAuthor
+      extract :CreationAuthor
     end
 
     def assignee
-      extract_non_empty :AssignedTo
+      extract :AssignedTo
     end
 
     def modified_author
-      extract_non_empty :ModifiedAuthor
+      extract :ModifiedAuthor
     end
 
     def creation_date
-      date = extract_non_empty :CreationDate
-      Date.iso8601(date) unless date.nil?
+      extract_date_time '/Markup/Topic/CreationDate'
     end
 
     def modified_date
-      date = extract_non_empty :ModifiedDate
-      Date.iso8601(date) unless date.nil?
+      extract_date_time '/Markup/Topic/ModifiedDate'
     end
 
     def due_date
-      date = extract_non_empty :DueDate
-      Date.iso8601(date) unless date.nil?
+      extract_date_time '/Markup/Topic/DueDate'
     rescue ArgumentError
       nil
     end
@@ -70,8 +67,8 @@ module OpenProject::Bcf::BcfXml
       doc.xpath('/Markup/Viewpoints').map do |node|
         {
           uuid: node['Guid'],
-          viewpoint: node.xpath('Viewpoint/text()').to_s,
-          snapshot: node.xpath('Snapshot/text()').to_s
+          viewpoint: extract_from_node('Viewpoint', node),
+          snapshot: extract_from_node('Snapshot', node)
         }.with_indifferent_access
       end
     end
@@ -80,9 +77,11 @@ module OpenProject::Bcf::BcfXml
       doc.xpath('/Markup/Comment').map do |node|
         {
           uuid: node['Guid'],
-          date: node.xpath('Date/text()').to_s,
-          author: node.xpath('Author/text()').to_s,
-          comment: node.xpath('Comment/text()').to_s
+          date: extract_date_time("Date", node),
+          author: extract_from_node('Author', node),
+          comment: extract_from_node('Comment', node),
+          modified_date: extract_date_time("ModifiedDate", node),
+          modified_author: extract_from_node("ModifiedAuthor", node)
         }.with_indifferent_access
       end
     end
@@ -102,10 +101,21 @@ module OpenProject::Bcf::BcfXml
 
     private
 
-    def extract_non_empty(path, prefix: '/Markup/Topic/'.freeze, attribute: false)
+    def extract_date_time(path, node = nil)
+      node ||= doc
+      date_time = extract_from_node(path, node)
+      Time.iso8601(date_time) unless date_time.nil?
+    end
+
+    def extract(path, prefix: '/Markup/Topic/'.freeze, attribute: false)
+      path = [prefix, path.to_s].join('')
+      extract_from_node(path, doc, attribute: attribute)
+    end
+
+    def extract_from_node(path, node, attribute: false)
       suffix = attribute ? '' : '/text()'.freeze
-      path = [prefix, path.to_s, suffix].join('')
-      doc.xpath(path).to_s.presence
+      path = [path.to_s, suffix].join('')
+      node.xpath(path).to_s.presence
     end
   end
 end

modules/bcf/spec/bcf/bcf_xml/importer_spec.rb

@@ -27,7 +27,7 @@ describe ::OpenProject::Bcf::BcfXml::Importer do
       'application/octet-stream'
     )
   end
-  let(:type) { FactoryBot.create :type, name: 'Issue [BCF]' }
+  let(:type) { FactoryBot.create :type, name: 'Issue' }
   let(:project) do
     FactoryBot.create(:project,
                       identifier: 'bim_project',

modules/bcf/spec/bcf/bcf_xml/issue_reader_spec.rb

@@ -114,17 +114,17 @@ describe ::OpenProject::Bcf::BcfXml::IssueReader do
 
   context 'on updating import' do
     context '#update_comment' do
-      let(:bcf_issue) { FactoryBot.create :bcf_issue_with_comment}
+      let!(:bcf_issue) { FactoryBot.create :bcf_issue_with_comment}
 
       it '#update_comment' do
         allow(subject).to receive(:issue).and_return(bcf_issue)
 
-        modified_time = Time.iso8601('2019-07-11T12:00:00Z')
+        modified_time = Time.now + 1.minute
         comment_data = { uuid: bcf_issue.comments.first.uuid, comment: 'Updated comment', modified_date: modified_time }
         subject.send(:update_comment, comment_data)
 
         expect(bcf_issue.comments.first.journal.notes).to eql('Updated comment')
-        expect(bcf_issue.comments.first.journal.created_at).to eql(modified_time)
+        expect(bcf_issue.comments.first.journal.created_at.utc.to_s).to eql(modified_time.utc.to_s)
       end
     end
   end

modules/bcf/spec/bcf/bcf_xml/markup_extractor_spec.rb

@@ -87,11 +87,11 @@ describe ::OpenProject::Bcf::BcfXml::MarkupExtractor do
   end
 
   it '#creation_date' do
-    expect(subject.creation_date).to eql Date.iso8601('2015-06-21T12:00:00Z')
+    expect(subject.creation_date).to eql(Time.iso8601('2015-06-21T12:00:00Z'))
   end
 
   it '#modified_date' do
-    expect(subject.modified_date).to eql Date.iso8601('2015-06-21T14:22:47Z')
+    expect(subject.modified_date).to eql(Time.iso8601('2015-06-21T14:22:47Z'))
   end
 
   it '#viewpoints' do
@@ -103,11 +103,12 @@ describe ::OpenProject::Bcf::BcfXml::MarkupExtractor do
 
   it '#comments' do
     expect(subject.comments.size).to eql 4
-    expect(subject.comments.first[:uuid]).to eql '780FAE52-C432-42BE-ADEA-FF3E7A8CD8E1'
-    expect(subject.comments.first[:date]).to eql '2015-08-31T12:40:17Z'
-    expect(subject.comments.first[:author]).to eql 'mike@example.com'
-    expect(subject.comments.first[:comment]).to eql 'This is an unmodified topic at the uppermost hierarchical level.
-All times in the XML are marked as UTC times.'
+    expect(subject.comments.first[:uuid]).to eql('780FAE52-C432-42BE-ADEA-FF3E7A8CD8E1')
+    expect(subject.comments.first[:date]).to eql(Time.iso8601('2015-08-31T12:40:17Z'))
+    expect(subject.comments.first[:modified_date]).to eql(Time.iso8601('2015-08-31T16:07:11Z'))
+    expect(subject.comments.first[:author]).to eql('mike@example.com')
+    expect(subject.comments.first[:modified_author]).to eql('mike@example.com')
+    expect(subject.comments.first[:comment]).to eql("This comment contained some spllng errs.\nHopefully, the modifier did catch them all.")
   end
 
   it '#people' do