Merge pull request #7458 from opf/bim/set-comment-created-at-to-bcf-modified-date

[BCF-XML] Set WP comment's created_at to BCF's Comment's ModifiedDate

.travis.yml

@@ -93,7 +93,7 @@ jobs:
       name: 'spec_legacy (1/1) - bim'
       script:
         - bash script/ci/setup.sh spec_legacy bim
-        - bash script/ci/runner.sh spec_legacy 1 1
+        - bash script/ci/runner.sh spec_legacy 1 1 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'units (1/4) - standard'
@@ -104,7 +104,7 @@ jobs:
       name: 'units (1/4) - bim'
       script:
         - bash script/ci/setup.sh units bim
-        - bash script/ci/runner.sh units 4 1
+        - bash script/ci/runner.sh units 4 1 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'units (2/4) - standard'
@@ -115,7 +115,7 @@ jobs:
       name: 'units (2/4) - bim'
       script:
         - bash script/ci/setup.sh units bim
-        - bash script/ci/runner.sh units 4 2
+        - bash script/ci/runner.sh units 4 2 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'units (3/4) - standard'
@@ -126,7 +126,7 @@ jobs:
       name: 'units (3/4) - bim'
       script:
         - bash script/ci/setup.sh units bim
-        - bash script/ci/runner.sh units 4 3
+        - bash script/ci/runner.sh units 4 3 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'units (4/4) - standard'
@@ -137,7 +137,7 @@ jobs:
       name: 'units (4/4) - bim'
       script:
       - bash script/ci/setup.sh units bim
-      - bash script/ci/runner.sh units 4 4
+      - bash script/ci/runner.sh units 4 4 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'features (1/4) - standard'
@@ -148,7 +148,7 @@ jobs:
       name: 'features (1/4) - bim'
       script:
       - bash script/ci/setup.sh features bim
-      - bash script/ci/runner.sh features 4 1
+      - bash script/ci/runner.sh features 4 1 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'features (2/4) - standard'
@@ -159,7 +159,7 @@ jobs:
       name: 'features (2/4) - bim'
       script:
       - bash script/ci/setup.sh features bim
-      - bash script/ci/runner.sh features 4 2
+      - bash script/ci/runner.sh features 4 2 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'features (3/4) - standard'
@@ -170,7 +170,7 @@ jobs:
       name: 'features (3/4) - bim'
       script:
       - bash script/ci/setup.sh features bim
-      - bash script/ci/runner.sh features 4 3
+      - bash script/ci/runner.sh features 4 3 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'features (4/4) - standard'
@@ -181,7 +181,7 @@ jobs:
       name: 'features (4/4) - bim'
       script:
       - bash script/ci/setup.sh features bim
-      - bash script/ci/runner.sh features 4 4
+      - bash script/ci/runner.sh features 4 4 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'plugins:units (1/1) - standard'
@@ -193,7 +193,7 @@ jobs:
       name: 'plugins:units (1/1) - bim'
       script:
       - bash script/ci/setup.sh plugins:units bim
-      - bash script/ci/runner.sh plugins:units 1 1
+      - bash script/ci/runner.sh plugins:units 1 1 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'plugins:features (1/1) - standard'
@@ -205,7 +205,7 @@ jobs:
       name: 'plugins:features (1/1) - bim'
       script:
       - bash script/ci/setup.sh plugins:features bim
-      - bash script/ci/runner.sh plugins:features 1 1
+      - bash script/ci/runner.sh plugins:features 1 1 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
     - stage: test
       name: 'plugins:cucumber (1/1) - standard'
@@ -217,7 +217,7 @@ jobs:
       name: 'plugins:cucumber (1/1) - bim'
       script:
       - bash script/ci/setup.sh plugins:cucumber bim
-      - bash script/ci/runner.sh plugins:cucumber 1 1
+      - bash script/ci/runner.sh plugins:cucumber 1 1 bim
       if: head_branch =~ /^(bim\/|dev|release\/)/
 
 addons:

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

@@ -6,6 +6,18 @@ module OpenProject::Bcf::BcfXml
   class Importer
     attr_reader :file, :project, :current_user
 
+    DEFAULT_IMPORT_OPTIONS = {
+      unknown_types_action: "use_default",
+      unknown_statuses_action: "use_default",
+      invalid_people_action: "anonymize",
+      unknown_mails_action: 'invite',
+      non_members_action: 'add',
+      unknown_types_chose_ids: [],
+      unknown_statuses_chose_ids: [],
+      unknown_mails_invite_role_ids: [],
+      non_members_add_role_ids: []
+    }.freeze
+
     def initialize(file, project, current_user:)
       @file = file
       @project = project
@@ -29,10 +41,9 @@ module OpenProject::Bcf::BcfXml
     end
 
     def import!(options = {})
+      options = options.merge(DEFAULT_IMPORT_OPTIONS)
       Zip::File.open(@file) do |zip|
-        treat_invalid_people(options)
-        treat_unknown_mails(options)
-        treat_non_members(options)
+        apply_import_replacements(options)
 
         # Extract all topics of the zip and save them
         synchronize_topics(zip, options)
@@ -49,6 +60,12 @@ module OpenProject::Bcf::BcfXml
 
     private
 
+    def apply_import_replacements(options)
+      treat_invalid_people(options)
+      treat_unknown_mails(options)
+      treat_non_members(options)
+    end
+
     def treat_invalid_people(options)
       if aggregations.invalid_people.any?
         unless options[:invalid_people_action] == 'anonymize'

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,8 +331,17 @@ module OpenProject::Bcf::BcfXml
     end
 
     def update_comment(comment_data)
-      bcf_comment = issue.comments.find_by(comment_data.slice(:uuid))
-      bcf_comment.journal.update_attribute(:notes, comment_data[:comment])
+      if comment_data[:modified_date]
+        bcf_comment = issue.comments.find_by(comment_data.slice(:uuid))
+        if bcf_comment.journal.created_at < comment_data[:modified_date]
+          update_journal_attributes(bcf_comment, comment_data)
+        end
+      end
+    end
+
+    def update_journal_attributes(bcf_comment, comment_data)
+      bcf_comment.journal.update_attributes(notes: comment_data[:comment],
+                                            created_at: comment_data[:modified_date])
       bcf_comment.journal.save
     end
 

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/api/v3/work_packages/work_package_representer_spec.rb

@@ -53,7 +53,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
         <ModifiedDate>2015-06-21T14:22:47Z</ModifiedDate>
         <ModifiedAuthor>mike@example.com</ModifiedAuthor>
         <AssignedTo>andy@example.com</AssignedTo>
-        <Description>This is a topic with all informations present.</Description>
+        <Description>This is a topic with all information present.</Description>
         <BimSnippet SnippetType="JSON">
           <Reference>JsonElement.json</Reference>
           <ReferenceSchema>http://json-schema.org</ReferenceSchema>
@@ -110,7 +110,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
     MARKUP
   end
   let(:bcf_issue) do
-    FactoryBot.create(:bcf_issue, markup: markup)
+    FactoryBot.create(:bcf_issue_with_comment, markup: markup)
   end
   let(:work_package) do
     FactoryBot.create(:work_package,

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

@@ -69,7 +69,7 @@ describe ::OpenProject::Bcf::BcfXml::IssueReader do
         <ModifiedDate>2015-06-21T14:22:47Z</ModifiedDate>
         <ModifiedAuthor>mike@example.com</ModifiedAuthor>
         <AssignedTo>andy@example.com</AssignedTo>
-        <Description>This is a topic with all informations present.</Description>
+        <Description>This is a topic with all information present.</Description>
         <RelatedTopic Guid="5019D939-62A4-45D9-B205-FAB602C98FE8" />
       </Topic>
       <Comment Guid="780FAE52-C432-42BE-ADEA-FF3E7A8CD8E1">
@@ -108,7 +108,24 @@ describe ::OpenProject::Bcf::BcfXml::IssueReader do
     let(:bcf_issue) { subject.extract! }
 
     it 'WP start date gets initialized with BCF CreationDate' do
-      expect(bcf_issue.work_package.start_date).to eql(subject.extractor.creation_date)
+      expect(bcf_issue.work_package.start_date).to eql(subject.extractor.creation_date.to_date)
+    end
+  end
+
+  context 'on updating import' do
+    context '#update_comment' do
+      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.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.utc.to_s).to eql(modified_time.utc.to_s)
+      end
     end
   end
 end

modules/bcf/spec/bcf/bcf_xml/issue_writer_spec.rb

@@ -52,7 +52,7 @@ describe ::OpenProject::Bcf::BcfXml::IssueWriter do
         <ModifiedDate>2015-06-21T14:22:47Z</ModifiedDate>
         <ModifiedAuthor>mike@example.com</ModifiedAuthor>
         <AssignedTo>andy@example.com</AssignedTo>
-        <Description>This is a topic with all informations present.</Description>
+        <Description>This is a topic with all information present.</Description>
         <BimSnippet SnippetType="JSON">
           <Reference>JsonElement.json</Reference>
           <ReferenceSchema>http://json-schema.org</ReferenceSchema>

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

@@ -67,7 +67,7 @@ describe ::OpenProject::Bcf::BcfXml::MarkupExtractor do
   end
 
   it '#description' do
-    expect(subject.description).to be_eql 'This is a topic with all informations present.'
+    expect(subject.description).to be_eql 'This is a topic with all information present.'
   end
 
   it '#author' do
@@ -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,14 @@ 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

modules/bcf/spec/factories/bcf_comment_factory.rb

@@ -0,0 +1,45 @@
+#-- copyright
+# OpenProject Backlogs Plugin
+#
+# Copyright (C)2013-2014 the OpenProject Foundation (OPF)
+# Copyright (C)2011 Stephan Eckardt, Tim Felgentreff, Marnen Laibow-Koser, Sandro Munda
+# Copyright (C)2010-2011 friflaj
+# Copyright (C)2010 Maxime Guilbot, Andrew Vit, Joakim Kolsjö, ibussieres, Daniel Passos, Jason Vasquez, jpic, Emiliano Heyns
+# Copyright (C)2009-2010 Mark Maglana
+# Copyright (C)2009 Joe Heck, Nate Lowrie
+#
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of the GNU General Public License version 3.
+#
+# OpenProject Backlogs is a derivative work based on ChiliProject Backlogs.
+# The copyright follows:
+# Copyright (C) 2010-2011 - Emiliano Heyns, Mark Maglana, friflaj
+# Copyright (C) 2011 - Jens Ulferts, Gregor Schmidt - Finn GmbH - Berlin, Germany
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+FactoryBot.define do
+  factory :bcf_comment, class: ::Bcf::Comment do
+    after(:create) do |bcf_comment|
+      bcf_comment.journal = create(:work_package_journal)
+      bcf_comment.journal.update_attribute(:notes, 'Some BCF comment.')
+      bcf_comment.journal.save
+      bcf_comment.save
+    end
+  end
+end

modules/bcf/spec/factories/bcf_issue_factory.rb

@@ -35,8 +35,11 @@
 
 FactoryBot.define do
   factory :bcf_issue, class: ::Bcf::Issue do
-    after(:create) do |issue|
-      create(:bcf_viewpoint, issue: issue)
+    factory :bcf_issue_with_comment do
+      after(:create) do |issue|
+        create(:bcf_viewpoint, issue: issue)
+        create(:bcf_comment, issue: issue)
+      end
     end
   end
 end

modules/bcf/spec/models/bcf/issue_spec.rb

@@ -53,7 +53,7 @@ describe ::Bcf::Issue, type: :model do
         <ModifiedDate>2015-06-21T14:22:47Z</ModifiedDate>
         <ModifiedAuthor>mike@example.com</ModifiedAuthor>
         <AssignedTo>andy@example.com</AssignedTo>
-        <Description>This is a topic with all informations present.</Description>
+        <Description>This is a topic with all information present.</Description>
         <BimSnippet SnippetType="JSON">
           <Reference>JsonElement.json</Reference>
           <ReferenceSchema>http://json-schema.org</ReferenceSchema>
@@ -115,7 +115,7 @@ describe ::Bcf::Issue, type: :model do
   shared_examples_for 'provides attributes' do
     it "provides attributes" do
       expect(subject.title).to be_eql 'Maximum Content'
-      expect(subject.description).to be_eql 'This is a topic with all informations present.'
+      expect(subject.description).to be_eql 'This is a topic with all information present.'
       expect(subject.priority_text).to be_eql 'High'
       expect(subject.status_text).to be_eql 'Open'
       expect(subject.type_text).to be_eql 'Structural'

script/ci/runner.sh

@@ -32,11 +32,18 @@
 # $1 = TEST_SUITE
 # $2 = GROUP_SIZE
 # $3 = GROUP
+# $4 = OPENPROJECT_EDITION
 
 #!/bin/sh
 
 set -e
 
+if [ "$4" = "bim" ]; then
+  export OPENPROJECT_EDITION="$4";
+else
+  unset OPENPROJECT_EDITION
+fi
+
 # Use the current HEAD as input to the seed
 export CI_SEED=$(git rev-parse HEAD | tr -d 'a-z' | cut -b 1-5 | tr -d '0')
 # Do not assume to have the angular cli running to serve assets. They are provided