From 2b0590bbc723b11b197db7364f0e63af5b04e96c Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Thu, 9 May 2019 21:28:09 +0200 Subject: [PATCH] Improve CodeClimate --- .../app/controllers/bcf/issues_controller.rb | 35 ++++++++++- .../bcf/bcf_xml/markup_extractor.rb | 11 ++-- .../work_package_representer_spec.rb | 10 ---- modules/bcf/spec/bcf/bcf_xml/importer_spec.rb | 35 +++++++---- .../spec/bcf/bcf_xml/markup_extractor_spec.rb | 7 ++- .../controllers/issues_controller_spec.rb | 60 +++++++++++-------- 6 files changed, 104 insertions(+), 54 deletions(-) diff --git a/modules/bcf/app/controllers/bcf/issues_controller.rb b/modules/bcf/app/controllers/bcf/issues_controller.rb index 2d5e420b04..699163f1ab 100644 --- a/modules/bcf/app/controllers/bcf/issues_controller.rb +++ b/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 diff --git a/modules/bcf/lib/open_project/bcf/bcf_xml/markup_extractor.rb b/modules/bcf/lib/open_project/bcf/bcf_xml/markup_extractor.rb index 98cd2d3059..2acf72f239 100644 --- a/modules/bcf/lib/open_project/bcf/bcf_xml/markup_extractor.rb +++ b/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 diff --git a/modules/bcf/spec/api/v3/work_packages/work_package_representer_spec.rb b/modules/bcf/spec/api/v3/work_packages/work_package_representer_spec.rb index 424250fa57..74bf981622 100644 --- a/modules/bcf/spec/api/v3/work_packages/work_package_representer_spec.rb +++ b/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 diff --git a/modules/bcf/spec/bcf/bcf_xml/importer_spec.rb b/modules/bcf/spec/bcf/bcf_xml/importer_spec.rb index 57a70dbf36..e645923ed8 100644 --- a/modules/bcf/spec/bcf/bcf_xml/importer_spec.rb +++ b/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 diff --git a/modules/bcf/spec/bcf/bcf_xml/markup_extractor_spec.rb b/modules/bcf/spec/bcf/bcf_xml/markup_extractor_spec.rb index 0c475abe46..d4097286ab 100644 --- a/modules/bcf/spec/bcf/bcf_xml/markup_extractor_spec.rb +++ b/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 diff --git a/modules/bcf/spec/controllers/issues_controller_spec.rb b/modules/bcf/spec/controllers/issues_controller_spec.rb index 341ee792fd..b6f35e928c 100644 --- a/modules/bcf/spec/controllers/issues_controller_spec.rb +++ b/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)