Merge branch '31362_decrease_cyclomatic_complexity_threshold_step3' into 'master'
Decrease Cyclomatic Complexity threshold to 14 See merge request !13972
This commit is contained in:
commit
45510326d0
6 changed files with 60 additions and 28 deletions
|
@ -643,7 +643,7 @@ Metrics/ClassLength:
|
||||||
# of test cases needed to validate a method.
|
# of test cases needed to validate a method.
|
||||||
Metrics/CyclomaticComplexity:
|
Metrics/CyclomaticComplexity:
|
||||||
Enabled: true
|
Enabled: true
|
||||||
Max: 15
|
Max: 14
|
||||||
|
|
||||||
# Limit lines to 80 characters.
|
# Limit lines to 80 characters.
|
||||||
Metrics/LineLength:
|
Metrics/LineLength:
|
||||||
|
|
|
@ -15,9 +15,13 @@ module ProjectsHelper
|
||||||
end
|
end
|
||||||
|
|
||||||
def link_to_member_avatar(author, opts = {})
|
def link_to_member_avatar(author, opts = {})
|
||||||
default_opts = { avatar: true, name: true, size: 16, author_class: 'author', title: ":name" }
|
default_opts = { size: 16 }
|
||||||
opts = default_opts.merge(opts)
|
opts = default_opts.merge(opts)
|
||||||
image_tag(avatar_icon(author, opts[:size]), width: opts[:size], class: "avatar avatar-inline #{"s#{opts[:size]}" if opts[:size]}", alt: '') if opts[:avatar]
|
|
||||||
|
classes = %W[avatar avatar-inline s#{opts[:size]}]
|
||||||
|
classes << opts[:avatar_class] if opts[:avatar_class]
|
||||||
|
|
||||||
|
image_tag(avatar_icon(author, opts[:size]), width: opts[:size], class: classes, alt: '')
|
||||||
end
|
end
|
||||||
|
|
||||||
def link_to_member(project, author, opts = {}, &block)
|
def link_to_member(project, author, opts = {}, &block)
|
||||||
|
@ -29,7 +33,7 @@ module ProjectsHelper
|
||||||
author_html = ""
|
author_html = ""
|
||||||
|
|
||||||
# Build avatar image tag
|
# Build avatar image tag
|
||||||
author_html << image_tag(avatar_icon(author, opts[:size]), width: opts[:size], class: "avatar avatar-inline #{"s#{opts[:size]}" if opts[:size]} #{opts[:avatar_class] if opts[:avatar_class]}", alt: '') if opts[:avatar]
|
author_html << link_to_member_avatar(author, opts) if opts[:avatar]
|
||||||
|
|
||||||
# Build name span tag
|
# Build name span tag
|
||||||
if opts[:by_username]
|
if opts[:by_username]
|
||||||
|
|
|
@ -16,9 +16,9 @@ module Ci
|
||||||
protected: project.protected_for?(ref)
|
protected: project.protected_for?(ref)
|
||||||
)
|
)
|
||||||
|
|
||||||
result = validate(current_user,
|
result = validate_project_and_git_items ||
|
||||||
ignore_skip_ci: ignore_skip_ci,
|
validate_pipeline(ignore_skip_ci: ignore_skip_ci,
|
||||||
save_on_errors: save_on_errors)
|
save_on_errors: save_on_errors)
|
||||||
|
|
||||||
return result if result
|
return result if result
|
||||||
|
|
||||||
|
@ -47,13 +47,13 @@ module Ci
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def validate(triggering_user, ignore_skip_ci:, save_on_errors:)
|
def validate_project_and_git_items
|
||||||
unless project.builds_enabled?
|
unless project.builds_enabled?
|
||||||
return error('Pipeline is disabled')
|
return error('Pipeline is disabled')
|
||||||
end
|
end
|
||||||
|
|
||||||
unless allowed_to_trigger_pipeline?(triggering_user)
|
unless allowed_to_trigger_pipeline?
|
||||||
if can?(triggering_user, :create_pipeline, project)
|
if can?(current_user, :create_pipeline, project)
|
||||||
return error("Insufficient permissions for protected ref '#{ref}'")
|
return error("Insufficient permissions for protected ref '#{ref}'")
|
||||||
else
|
else
|
||||||
return error('Insufficient permissions to create a new pipeline')
|
return error('Insufficient permissions to create a new pipeline')
|
||||||
|
@ -67,7 +67,9 @@ module Ci
|
||||||
unless commit
|
unless commit
|
||||||
return error('Commit not found')
|
return error('Commit not found')
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def validate_pipeline(ignore_skip_ci:, save_on_errors:)
|
||||||
unless pipeline.config_processor
|
unless pipeline.config_processor
|
||||||
unless pipeline.ci_yaml_file
|
unless pipeline.ci_yaml_file
|
||||||
return error("Missing #{pipeline.ci_yaml_file_path} file")
|
return error("Missing #{pipeline.ci_yaml_file_path} file")
|
||||||
|
@ -85,25 +87,25 @@ module Ci
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def allowed_to_trigger_pipeline?(triggering_user)
|
def allowed_to_trigger_pipeline?
|
||||||
if triggering_user
|
if current_user
|
||||||
allowed_to_create?(triggering_user)
|
allowed_to_create?
|
||||||
else # legacy triggers don't have a corresponding user
|
else # legacy triggers don't have a corresponding user
|
||||||
!project.protected_for?(ref)
|
!project.protected_for?(ref)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def allowed_to_create?(triggering_user)
|
def allowed_to_create?
|
||||||
access = Gitlab::UserAccess.new(triggering_user, project: project)
|
return unless can?(current_user, :create_pipeline, project)
|
||||||
|
|
||||||
can?(triggering_user, :create_pipeline, project) &&
|
access = Gitlab::UserAccess.new(current_user, project: project)
|
||||||
if branch?
|
if branch?
|
||||||
access.can_update_branch?(ref)
|
access.can_update_branch?(ref)
|
||||||
elsif tag?
|
elsif tag?
|
||||||
access.can_create_tag?(ref)
|
access.can_create_tag?(ref)
|
||||||
else
|
else
|
||||||
true # Allow it for now and we'll reject when we check ref existence
|
true # Allow it for now and we'll reject when we check ref existence
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def update_merge_requests_head_pipeline
|
def update_merge_requests_head_pipeline
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Decrease Cyclomatic Complexity threshold to 14
|
||||||
|
merge_request: 13972
|
||||||
|
author: Maxim Rydkin
|
||||||
|
type: other
|
|
@ -191,10 +191,31 @@ describe ProjectsHelper do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'link_to_member' do
|
describe '#link_to_member_avatar' do
|
||||||
let(:group) { create(:group) }
|
let(:user) { build_stubbed(:user) }
|
||||||
let(:project) { create(:project, group: group) }
|
let(:expected) { double }
|
||||||
let(:user) { create(:user) }
|
|
||||||
|
before do
|
||||||
|
expect(helper).to receive(:avatar_icon).with(user, 16).and_return(expected)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns image tag for member avatar' do
|
||||||
|
expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16"], alt: "" })
|
||||||
|
|
||||||
|
helper.link_to_member_avatar(user)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns image tag with avatar class' do
|
||||||
|
expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16", "any-avatar-class"], alt: "" })
|
||||||
|
|
||||||
|
helper.link_to_member_avatar(user, avatar_class: "any-avatar-class")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#link_to_member' do
|
||||||
|
let(:group) { build_stubbed(:group) }
|
||||||
|
let(:project) { build_stubbed(:project, group: group) }
|
||||||
|
let(:user) { build_stubbed(:user) }
|
||||||
|
|
||||||
describe 'using the default options' do
|
describe 'using the default options' do
|
||||||
it 'returns an HTML link to the user' do
|
it 'returns an HTML link to the user' do
|
||||||
|
|
|
@ -489,7 +489,7 @@ describe Ci::CreatePipelineService do
|
||||||
|
|
||||||
subject do
|
subject do
|
||||||
described_class.new(project, user, ref: ref)
|
described_class.new(project, user, ref: ref)
|
||||||
.send(:allowed_to_create?, user)
|
.send(:allowed_to_create?)
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when user is a developer' do
|
context 'when user is a developer' do
|
||||||
|
|
Loading…
Reference in a new issue