Merge pull request #7673 from opf/bim/fix--add-status-type-priority-when-missing

BCF: Bug fix. Add status, type, and priority when missing

modules/bcf/lib/api/bcf_xml/v1/bcf_xml_api.rb

@@ -65,10 +65,6 @@ module API
               get do
                 project = find_project
 
-                authorize_any(%i[view_linked_issues view_work_packages], projects: [project]) do
-                  raise ::API::Errors::NotFound.new
-                end
-
                 authorize(:view_linked_issues, context: project) do
                   raise API::Errors::NotFound.new
                 end
@@ -96,7 +92,7 @@ module API
                                                                       current_user: User.current)
                   importer.import!(import_options)
                 rescue StandardError => e
-                  raise API::Errors::InternalError.new e.message
+                  raise API::Errors::InternalError.new(e.message)
                 ensure
                   file.delete
                 end

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

@@ -45,7 +45,7 @@ module OpenProject::Bcf::BcfXml
     def import!(options = {})
       options = DEFAULT_IMPORT_OPTIONS.merge(options)
       Zip::File.open(@file) do |zip|
-        apply_import_replacements(options)
+        create_or_add_missing_members(options)
 
         # Extract all topics of the zip and save them
         synchronize_topics(zip, options)
@@ -62,7 +62,7 @@ module OpenProject::Bcf::BcfXml
 
     private
 
-    def apply_import_replacements(options)
+    def create_or_add_missing_members(options)
       treat_invalid_people(options)
       treat_unknown_mails(options)
       treat_non_members(options)
@@ -157,6 +157,7 @@ module OpenProject::Bcf::BcfXml
                                   current_user: current_user,
                                   import_options: import_options,
                                   aggregations: aggregations).extract!
+
           if issue.errors.blank?
             issue.save
           end

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

@@ -63,7 +63,7 @@ module OpenProject::Bcf::BcfXml
     ##
     # Handle unknown types during import
     def treat_unknown_types
-      if aggregations.unknown_types.any?
+      if aggregations.unknown_types.present?
         if import_options[:unknown_types_action] == 'use_default'
           replace_type_with(::Type.default.first&.name)
         elsif import_options[:unknown_types_action] == 'chose' && import_options[:unknown_types_chose_ids].any?
@@ -83,7 +83,7 @@ module OpenProject::Bcf::BcfXml
     ##
     # Handle unknown statuses during import
     def treat_unknown_statuses
-      if aggregations.unknown_statuses.any?
+      if aggregations.unknown_statuses.present?
         if import_options[:unknown_statuses_action] == 'use_default'
           replace_status_with(::Status.default&.name)
         elsif import_options[:unknown_statuses_action] == 'chose' && import_options[:unknown_statuses_chose_ids].any?
@@ -103,7 +103,7 @@ module OpenProject::Bcf::BcfXml
     ##
     # Handle unknown priorities during import
     def treat_unknown_priorities
-      if aggregations.unknown_priorities.any?
+      if aggregations.unknown_priorities.present?
         if import_options[:unknown_priorities_action] == 'use_default'
           # NOP The 'use_default' case gets already covered by OP.
         elsif import_options[:unknown_priorities_action] == 'chose' && import_options[:unknown_priorities_chose_ids].any?

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

@@ -30,7 +30,7 @@ require 'spec_helper'
 
 describe ::OpenProject::Bcf::BcfXml::IssueReader do
   let(:absolute_file_path) { "63E78882-7C6A-4BF7-8982-FC478AFB9C97/markup.bcf" }
-  let(:type) { FactoryBot.create :type, name: 'Issue', is_standard: true }
+  let(:type) { FactoryBot.create :type, name: 'Issue', is_standard: true, is_default: true }
   let(:project) do
     FactoryBot.create(:project,
                       identifier: 'bim_project',
@@ -87,14 +87,16 @@ describe ::OpenProject::Bcf::BcfXml::IssueReader do
       .new(absolute_file_path, entry_stream)
   end
   let(:entry_stream) { StringIO.new(markup) }
+  let(:import_options) { OpenProject::Bcf::BcfXml::Importer::DEFAULT_IMPORT_OPTIONS }
+  let(:aggregations) { OpenProject::Bcf::BcfXml::Aggregations.new([], project) }
 
   subject do
     described_class.new(project,
                         nil,
                         entry,
                         current_user: bcf_manager,
-                        import_options: {},
-                        aggregations: OpenProject::Bcf::BcfXml::Aggregations.new([], project))
+                        import_options: import_options,
+                        aggregations: aggregations)
   end
 
   before do
@@ -107,9 +109,52 @@ describe ::OpenProject::Bcf::BcfXml::IssueReader do
   context 'on initial import' do
     let(:bcf_issue) { subject.extract! }
 
-    it 'WP start date gets initialized with BCF CreationDate' do
-      expect(bcf_issue.uuid).to eql("63E78882-7C6A-4BF7-8982-FC478AFB9C97")
-      expect(bcf_issue.work_package.start_date).to eql(subject.extractor.creation_date.to_date)
+    context 'with happy path, everything has a match' do
+      it 'WP start date gets initialized with BCF CreationDate' do
+        expect(bcf_issue.work_package.start_date).to eql(subject.extractor.creation_date.to_date)
+      end
+
+      it 'BCF::Issue get initialized with the GUID form the XML file' do
+        expect(bcf_issue.uuid).to eql("63E78882-7C6A-4BF7-8982-FC478AFB9C97")
+      end
+    end
+
+    context 'Topic does not provide any title, status, type or priority' do
+      let(:markup) do
+        <<-MARKUP
+        <Markup xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+          <Topic Guid="63E78882-7C6A-4BF7-8982-FC478AFB9C97">
+            <Title></Title>
+            <CreationDate>2015-06-21T12:00:00Z</CreationDate>
+            <ModifiedDate>2015-04-21T12:00:00Z</ModifiedDate>
+            <CreationAuthor>mike@example.com</CreationAuthor>
+          </Topic>
+        </Markup>
+        MARKUP
+      end
+
+      context 'with no import options provided' do
+        let(:aggregations) do
+          Struct
+            .new(:unknown_statuses, :unknown_types, :unknown_priorities)
+            .new([nil], [nil], [nil])
+        end
+
+        let(:bcf_issue) { subject.extract! }
+
+        it 'sets a status' do
+          expect(bcf_issue.work_package.status).to eql(Status.default)
+        end
+
+        it 'sets a type' do
+          expect(bcf_issue.work_package.type).to eql(type)
+        end
+
+        it 'ignores missing priority' do
+          # as it gets set automatically when creating new work packages.
+          expect(bcf_issue.work_package.priority).to eql(priority)
+        end
+      end
     end
   end