present error if title is missing

lib/api/utilities/endpoints/modify.rb

@@ -43,12 +43,16 @@ module API
         end
 
         def present_error(call)
-          errors = call.errors
-          errors = merge_dependent_errors call if errors.empty?
+          api_errors = [::API::Errors::ErrorBase.create_and_merge_errors(postprocess_errors(call))]
 
-          api_errors = [::API::Errors::ErrorBase.create_and_merge_errors(errors)]
+          fail(::API::Errors::MultipleErrors
+                 .create_if_many(api_errors))
+        end
 
-          fail ::API::Errors::MultipleErrors.create_if_many(api_errors)
+        def postprocess_errors(call)
+          errors = call.errors
+          errors = merge_dependent_errors call if errors.empty?
+          errors
         end
 
         def merge_dependent_errors(call)

modules/bcf/app/controllers/bcf/api/v2_1/endpoints/create.rb

@@ -37,6 +37,10 @@ module Bcf::API::V2_1::Endpoints
         .new(call.result)
     end
 
+    def postprocess_errors(call)
+      Bcf::API::V2_1::Errors::ErrorMapper.map(super)
+    end
+
     private
 
     def deduce_process_service

modules/bcf/app/representers/bcf/api/v2_1/errors/error_mapper.rb

@@ -0,0 +1,60 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# 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 is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# 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 docs/COPYRIGHT.rdoc for more details.
+#++
+
+module Bcf::API::V2_1::Errors
+  class ErrorMapper
+    extend ActiveModel::Naming
+    extend ActiveModel::Translation
+
+    def read_attribute_for_validation(_attr)
+      nil
+    end
+
+    def self.lookup_ancestors
+      [::Bcf::Issue]
+    end
+
+    def self.map(original_errors)
+      mapped_errors = ActiveModel::Errors.new(new)
+
+      original_errors.send(:error_symbols).each do |key, errors|
+        errors.map(&:first).each do |error|
+          mapped_errors.add(error_key_mapper(key), error)
+        end
+      end
+
+      mapped_errors
+    end
+
+    def self.error_key_mapper(key)
+      { subject: :title }[key] || key
+    end
+  end
+end

modules/bcf/app/representers/bcf/api/v2_1/topics/single_representer.rb

@@ -51,6 +51,14 @@ module Bcf::API::V2_1
                 .name
              }
 
+    property :priority,
+             as: :priority,
+             getter: ->(*) {
+               work_package
+                 .priority
+                 .name
+             }
+
     property :reference_links,
              getter: ->(decorator:, **) {
                [decorator.api_v3_paths.work_package(work_package.id)]

modules/bcf/app/services/bcf/issues/create_service.rb

@@ -37,11 +37,15 @@ module Bcf::Issues
                 .new(user: user)
                 .call(params)
 
-      issue_params = {
-        work_package: wp_call.result
-      }.merge(params.slice(:stage, :labels, :index))
+      if wp_call.success?
+        issue_params = {
+          work_package: wp_call.result
+        }.merge(params.slice(:stage, :labels, :index))
 
-      super(issue_params)
+        super(issue_params)
+      else
+        wp_call
+      end
     end
   end
 end

modules/bcf/app/services/bcf/issues/transform_attributes_service.rb

@@ -40,7 +40,11 @@ module Bcf::Issues
     ##
     # BCF issues might have empty titles. OP needs one.
     def title(attributes)
-      attributes[:title] || '(Imported BCF issue contained no title)'
+      if attributes[:title]
+        attributes[:title]
+      elsif attributes[:import_options]
+        '(Imported BCF issue contained no title)'
+      end
     end
 
     def author(project, attributes)
@@ -67,15 +71,11 @@ module Bcf::Issues
 
       return unless import_options
 
-      return ::Type.default&.first if import_options[:unknown_types_action] == 'default'
-
-      if import_options[:unknown_types_action] == 'chose' &&
-         import_options[:unknown_types_chose_ids].any?
+      if import_options[:unknown_types_action] == 'default'
+        ::Type.default&.first
+      elsif import_options[:unknown_types_action] == 'chose' &&
+            import_options[:unknown_types_chose_ids].any?
         ::Type.find_by(id: import_options[:unknown_types_chose_ids].first)
-      else
-        ServiceResult.new success: false,
-                          errors: issue.errors,
-                          result: issue
       end
     end
 
@@ -113,7 +113,8 @@ module Bcf::Issues
 
       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?
+      elsif import_options[:unknown_priorities_action] == 'chose' &&
+            import_options[:unknown_priorities_chose_ids].any?
         ::IssuePriority.find_by(id: import_options[:unknown_priorities_chose_ids].first)
       end
     end

modules/bcf/spec/representers/bcf/api/v2_1/topics/single_representer_rendering_spec.rb

@@ -41,11 +41,13 @@ describe Bcf::API::V2_1::Topics::SingleRepresenter, 'rendering' do
   let(:journals) { [first_journal, last_journal] }
   let(:type) { FactoryBot.build_stubbed(:type) }
   let(:status) { FactoryBot.build_stubbed(:status) }
+  let(:priority) { FactoryBot.build_stubbed(:priority) }
   let(:work_package) do
     FactoryBot.build_stubbed(:stubbed_work_package,
                              assigned_to: assignee,
                              due_date: Date.today,
                              status: status,
+                             priority: priority,
                              type: type).tap do |wp|
       allow(wp)
         .to receive(:journals)
@@ -80,6 +82,13 @@ describe Bcf::API::V2_1::Topics::SingleRepresenter, 'rendering' do
       end
     end
 
+    context 'priority' do
+      it_behaves_like 'attribute' do
+        let(:value) { priority.name }
+        let(:path) { 'priority' }
+      end
+    end
+
     context 'reference_links' do
       it_behaves_like 'attribute' do
         let(:value) { [api_v3_paths.work_package(work_package.id)] }

modules/bcf/spec/requests/api/bcf/v2_1/shared_responses.rb

@@ -86,7 +86,7 @@ shared_examples_for 'bcf api not allowed response' do
 end
 
 shared_examples_for 'bcf api unprocessable response' do
-  it 'responds 403 NOT ALLOWED' do
+  it 'responds 422' do
     expect(subject.status)
       .to eql 422
   end

modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb

@@ -184,6 +184,12 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
     let!(:default_status) do
       FactoryBot.create(:default_status)
     end
+    let!(:default_type) do
+      FactoryBot.create(:type, is_default: true)
+    end
+    let!(:standard_type) do
+      FactoryBot.create(:type_standard)
+    end
     let!(:priority) do
       FactoryBot.create(:priority)
     end
@@ -198,6 +204,7 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
       {
         topic_type: type.name,
         topic_status: status.name,
+        priority: priority.name,
         title: 'BCF topic 101',
         labels: labels,
         stage: stage,
@@ -223,6 +230,7 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
           guid: issue&.uuid,
           topic_type: type.name,
           topic_status: status.name,
+          priority: priority.name,
           title: 'BCF topic 101',
           labels: labels,
           index: index,
@@ -237,9 +245,58 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
           modified_author: edit_member_user.mail,
           modified_date: work_package.updated_at.iso8601,
           description: description
+        }
+      end
+    end
+
+    context 'with minimal parameters' do
+      let(:params) do
+        {
+          title: 'BCF topic 101'
+        }
+      end
+
+      it_behaves_like 'bcf api successful response' do
+        let(:expected_status) { 201 }
+        let(:expected_body) do
+          issue = Bcf::Issue.last
+          work_package = WorkPackage.last
 
+          {
+            guid: issue&.uuid,
+            topic_type: standard_type.name,
+            topic_status: default_status.name,
+            priority: default_priority.name,
+            title: 'BCF topic 101',
+            labels: [],
+            index: nil,
+            reference_links: [
+              api_v3_paths.work_package(work_package.id)
+            ],
+            assigned_to: nil,
+            due_date: nil,
+            stage: nil,
+            creation_author: edit_member_user.mail,
+            creation_date: work_package.created_at.iso8601,
+            modified_author: edit_member_user.mail,
+            modified_date: work_package.updated_at.iso8601,
+            description: nil
+          }
+        end
+      end
+    end
+
+    context 'without a title' do
+      let(:params) do
+        {
         }
       end
+
+      it_behaves_like 'bcf api unprocessable response' do
+        let(:message) do
+          "Title can't be blank"
+        end
+      end
     end
   end
 end

modules/bcf/spec/services/bcf/issues/create_service_spec.rb

@@ -0,0 +1,173 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# 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 is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# 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 docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe Bcf::Issues::CreateService, type: :model do
+  let(:user) { FactoryBot.build_stubbed(:user) }
+  let(:contract_class) do
+    double('contract_class', '<=': true)
+  end
+  let(:issue_valid) { true }
+  let(:instance) do
+    described_class.new(user: user,
+                        contract_class: contract_class)
+  end
+  let(:call_attributes) { { subject: 'Some name' } }
+  let(:set_attributes_success) do
+    true
+  end
+  let(:set_attributes_errors) do
+    double('set_attributes_errors')
+  end
+  let(:set_attributes_result) do
+    ServiceResult.new result: created_issue,
+                      success: set_attributes_success,
+                      errors: set_attributes_errors
+  end
+  let!(:created_work_package) do
+    FactoryBot.build_stubbed(:work_package)
+  end
+  let(:wp_create_errors) do
+    double('wp_create_errors')
+  end
+  let(:wp_create_result) do
+    ServiceResult.new result: created_work_package,
+                      success: true,
+                      errors: wp_create_errors
+  end
+  let!(:wp_create_service) do
+    wp_service = double('wp create service')
+
+    allow(WorkPackages::CreateService)
+      .to receive(:new)
+      .with(user: user)
+      .and_return(wp_service)
+
+    allow(wp_service)
+      .to receive(:call)
+      .and_return(wp_create_result)
+
+    wp_service
+  end
+  let!(:created_issue) do
+    issue = FactoryBot.build_stubbed(:bcf_issue)
+
+    allow(Bcf::Issue)
+      .to receive(:new)
+      .and_return(issue)
+
+    allow(issue)
+      .to receive(:save)
+      .and_return(issue_valid)
+
+    issue
+  end
+  let!(:set_attributes_service) do
+    service = double('set_attributes_service_instance')
+
+    allow(Bcf::Issues::SetAttributesService)
+      .to receive(:new)
+      .with(user: user,
+            model: created_issue,
+            contract_class: contract_class)
+      .and_return(service)
+
+    allow(service)
+      .to receive(:call)
+      .and_return(set_attributes_result)
+  end
+
+  describe '#call' do
+    subject { instance.call(call_attributes) }
+
+    it 'is successful' do
+      expect(subject.success?).to be_truthy
+    end
+
+    it 'returns the result of the SetAttributesService' do
+      expect(subject)
+        .to eql set_attributes_result
+    end
+
+    it 'persists the issue' do
+      expect(created_issue)
+        .to receive(:save)
+        .and_return(issue_valid)
+
+      subject
+    end
+
+    it 'creates a issue' do
+      expect(subject.result)
+        .to eql created_issue
+    end
+
+    context 'if the SetAttributeService is unsuccessful' do
+      let(:set_attributes_success) { false }
+
+      it 'is unsuccessful' do
+        expect(subject.success?).to be_falsey
+      end
+
+      it 'returns the result of the SetAttributesService' do
+        expect(subject)
+          .to eql set_attributes_result
+      end
+
+      it 'does not persist the changes' do
+        expect(created_issue)
+          .to_not receive(:save)
+
+        subject
+      end
+
+      it "exposes the contract's errors" do
+        subject
+
+        expect(subject.errors).to eql set_attributes_errors
+      end
+    end
+
+    context 'when the issue is invalid' do
+      let(:issue_valid) { false }
+
+      it 'is unsuccessful' do
+        expect(subject.success?).to be_falsey
+      end
+
+      it "exposes the issue's errors" do
+        subject
+
+        expect(subject.errors).to eql created_issue.errors
+      end
+    end
+  end
+end