Improve CodeClimate

modules/bcf/app/controllers/bcf/issues_controller.rb

@@ -1,3 +1,32 @@
+#-- 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
   class IssuesController < BaseController
     include PaginationHelper
@@ -33,10 +62,12 @@ module ::Bcf
 
         @listing = @importer.get_extractor_list! @bcf_file.path
         if @listing.blank?
-          raise(StandardError.new I18n.t('bcf.exceptions.file_invalid'))
+          raise(StandardError.new(I18n.t('bcf.exceptions.file_invalid')))
         end
 
-        @issues = ::Bcf::Issue.with_markup.includes(work_package: %i[status priority assigned_to]).where(uuid: @listing.map { |e| e[:uuid] })
+        @issues = ::Bcf::Issue.with_markup
+                    .includes(work_package: %i[status priority assigned_to])
+                    .where(uuid: @listing.map { |e| e[:uuid] })
 
         all_people = @listing.map { |entry| entry[:people] }.flatten.uniq
         all_mails = @listing.map { |entry| entry[:mail_addresses] }.flatten.uniq

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

@@ -73,11 +73,12 @@ module OpenProject::Bcf::BcfXml
     end
 
     def mail_addresses
-      people.filter do |person|
-        # person value is an email address
-        person =~ /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
-      end
-      .uniq
+      people
+        .filter do |person|
+          # person value is an email address
+          person =~ /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
+        end
+        .uniq
     end
 
     def people

modules/bcf/spec/api/v3/work_packages/work_package_representer_spec.rb

@@ -140,15 +140,5 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
         .including('id')
         .at_path('bcf/viewpoints/')
     end
-
-    it "contains topic labels" do
-      is_expected.to be_json_eql([
-                                   {
-                                     labels: ["Structural", "IT Development"]
-                                   }
-                                 ].to_json)
-                       .including('id')
-                       .at_path('bcf/topic/')
-    end
   end
 end

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

@@ -27,20 +27,35 @@ describe ::OpenProject::Bcf::BcfXml::Importer do
       'application/octet-stream')
   end
   let(:type) { FactoryBot.create :type, name: 'Issue [BCF]' }
-  let(:project) { FactoryBot.create(:project,
-                                    identifier: 'bim_project',
-                                    types: [type]) }
-  let(:member_role) { FactoryBot.create(:role, permissions: %i[view_linked_issues view_work_packages]) }
-  let(:manage_bcf_role) { FactoryBot.create(:role, permissions: %i[manage_bcf view_linked_issues view_work_packages edit_work_packages add_work_packages]) }
+  let(:project) do
+    FactoryBot.create(:project,
+                      identifier: 'bim_project',
+                      types: [type])
+  end
+  let(:member_role) do
+    FactoryBot.create(:role,
+                      permissions: %i[view_linked_issues view_work_packages])
+  end
+  let(:manage_bcf_role) do
+    FactoryBot.create(
+      :role,
+      permissions: %i[manage_bcf view_linked_issues view_work_packages edit_work_packages add_work_packages]
+    )
+  end
   let(:bcf_manager) { FactoryBot.create(:user) }
-  let(:workflow) { FactoryBot.create(:workflow_with_default_status, role: manage_bcf_role, type: type) }
+  let(:workflow) do
+    FactoryBot.create(:workflow_with_default_status,
+                      role: manage_bcf_role,
+                      type: type)
+  end
   let(:priority) { FactoryBot.create :default_priority }
   let(:bcf_manager_member) {
     FactoryBot.create(:member,
-    project: project,
-    user: bcf_manager,
-    roles: [manage_bcf_role, member_role])
+                      project: project,
+                      user: bcf_manager,
+                      roles: [manage_bcf_role, member_role])
   }
+
   subject { described_class.new project, current_user: bcf_manager }
 
   before do
@@ -64,7 +79,7 @@ describe ::OpenProject::Bcf::BcfXml::Importer do
 
   describe '#import!' do
     it 'imports successfully' do
-      expect(subject.import! file).to be_present
+      expect(subject.import!(file)).to be_present
     end
 
     it 'creates to work packages' do

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

@@ -22,9 +22,10 @@ require 'spec_helper'
 describe ::OpenProject::Bcf::BcfXml::MarkupExtractor do
   let(:filename) { 'MaximumInformation.bcf' }
   let(:file) do
-   Rack::Test::UploadedFile.new(
+    Rack::Test::UploadedFile.new(
       File.join(Rails.root, "modules/bcf/spec/fixtures/files/#{filename}"),
-      'application/octet-stream')
+      'application/octet-stream'
+    )
   end
   let(:entries) do
     Zip::File.open(file) do |zip|
@@ -32,7 +33,7 @@ describe ::OpenProject::Bcf::BcfXml::MarkupExtractor do
     end
   end
 
-  subject { described_class.new entries.second }
+  subject { described_class.new(entries.first) }
 
   it '#initialize' do
     expect(subject).to be_a described_class

modules/bcf/spec/controllers/issues_controller_spec.rb

@@ -29,27 +29,33 @@
 require 'spec_helper'
 
 describe ::Bcf::IssuesController, type: :controller do
-  let(:manage_bcf_role) { FactoryBot.create(:role, permissions: %i[manage_bcf view_linked_issues view_work_packages]) }
-  let(:collaborator_role) {FactoryBot.create(:role, permissions: %i[view_linked_issues view_work_packages])}
+  let(:manage_bcf_role) do
+    FactoryBot.create(:role,
+                      permissions: %i[manage_bcf view_linked_issues view_work_packages])
+  end
+  let(:collaborator_role) do
+    FactoryBot.create(:role,
+                      permissions: %i[view_linked_issues view_work_packages])
+  end
   let(:bcf_manager) { FactoryBot.create(:user, firstname: "BCF Manager") }
   let(:collaborator) { FactoryBot.create(:user) }
 
   let(:non_member) { FactoryBot.create(:user) }
-  let(:project) { FactoryBot.create(:project,
-                                    identifier: 'bim_project'
-  ) }
-  let(:member) {
+  let(:project) do
+    FactoryBot.create(:project, identifier: 'bim_project')
+  end
+  let(:member) do
     FactoryBot.create(:member,
                       project: project,
                       user: collaborator,
                       roles: [collaborator_role])
-  }
-  let(:bcf_manager_member) {
+  end
+  let(:bcf_manager_member) do
     FactoryBot.create(:member,
                       project: project,
                       user: bcf_manager,
                       roles: [manage_bcf_role])
-  }
+  end
   before do
     bcf_manager_member
     member
@@ -58,9 +64,7 @@ describe ::Bcf::IssuesController, type: :controller do
 
   shared_examples_for 'check permissions' do
     context 'without sufficient permissions' do
-      before do
-        action
-      end
+      before { action }
 
       context 'not member of project' do
         let(:bcf_manager_member) {}
@@ -70,12 +74,12 @@ describe ::Bcf::IssuesController, type: :controller do
       end
 
       context 'no manage_bcf permission' do
-        let(:bcf_manager_member) {
+        let(:bcf_manager_member) do
           FactoryBot.create(:member,
                             project: project,
                             user: bcf_manager,
                             roles: [collaborator_role])
-        }
+        end
         it 'will return "not authorized"' do
           expect(response).to_not be_successful
         end
@@ -84,18 +88,24 @@ describe ::Bcf::IssuesController, type: :controller do
   end
 
   describe '#prepare_import' do
-    let(:params) { { project_id: project.identifier.to_s,
-                     bcf_file: file} }
-
+    let(:params) do
+      {
+        project_id: project.identifier.to_s,
+        bcf_file: file
+      }
+    end
     let(:action) do
       post :prepare_import, params: params
     end
 
     context 'with valid BCF file' do
       let(:filename) { 'MaximumInformation.bcf' }
-      let(:file) { Rack::Test::UploadedFile.new(
-        File.join(Rails.root, "modules/bcf/spec/fixtures/files/#{filename}"),
-        'application/octet-stream') }
+      let(:file) do
+        Rack::Test::UploadedFile.new(
+          File.join(Rails.root, "modules/bcf/spec/fixtures/files/#{filename}"),
+          'application/octet-stream'
+        )
+      end
 
       it 'should be successful' do
         expect { action }.to change { Attachment.count }.by(1)
@@ -120,12 +130,14 @@ describe ::Bcf::IssuesController, type: :controller do
       post :perform_import, params: { project_id: project.identifier.to_s }
     end
 
-
     context 'with valid BCF file' do
       let(:filename) { 'MaximumInformation.bcf' }
-      let(:file) { Rack::Test::UploadedFile.new(
-        File.join(Rails.root, "modules/bcf/spec/fixtures/files/#{filename}"),
-        'application/octet-stream') }
+      let(:file) do
+        Rack::Test::UploadedFile.new(
+          File.join(Rails.root, "modules/bcf/spec/fixtures/files/#{filename}"),
+          'application/octet-stream'
+        )
+      end
 
       before do
         allow_any_instance_of(Attachment).to receive(:local_file).and_return(file)