Merge branch '31362_decrease_cyclomatic_complexity_threshold_step3' into 'master'

Decrease Cyclomatic Complexity threshold to 14

See merge request !13972

6 files changed, 60 insertions, 28 deletions

diff --git a/.rubocop.yml b/.rubocop.yml
index 16f2e4484fc..4640681379a 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -643,7 +643,7 @@ Metrics/ClassLength:
 # of test cases needed to validate a method.
 Metrics/CyclomaticComplexity:
   Enabled: true
-  Max: 15
+  Max: 14
 
 # Limit lines to 80 characters.
 Metrics/LineLength:
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 86665ea2aec..51c625ede4b 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -15,9 +15,13 @@ module ProjectsHelper
   end
 
   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)
-    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
 
   def link_to_member(project, author, opts = {}, &block)
@@ -29,7 +33,7 @@ module ProjectsHelper
     author_html = ""
 
     # 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
     if opts[:by_username]
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 414c01b2546..d20de9b16a4 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -16,9 +16,9 @@ module Ci
         protected: project.protected_for?(ref)
       )
 
-      result = validate(current_user,
-                        ignore_skip_ci: ignore_skip_ci,
-                        save_on_errors: save_on_errors)
+      result = validate_project_and_git_items ||
+        validate_pipeline(ignore_skip_ci: ignore_skip_ci,
+                          save_on_errors: save_on_errors)
 
       return result if result
 
@@ -47,13 +47,13 @@ module Ci
 
     private
 
-    def validate(triggering_user, ignore_skip_ci:, save_on_errors:)
+    def validate_project_and_git_items
       unless project.builds_enabled?
         return error('Pipeline is disabled')
       end
 
-      unless allowed_to_trigger_pipeline?(triggering_user)
-        if can?(triggering_user, :create_pipeline, project)
+      unless allowed_to_trigger_pipeline?
+        if can?(current_user, :create_pipeline, project)
           return error("Insufficient permissions for protected ref '#{ref}'")
         else
           return error('Insufficient permissions to create a new pipeline')
@@ -67,7 +67,9 @@ module Ci
       unless commit
         return error('Commit not found')
       end
+    end
 
+    def validate_pipeline(ignore_skip_ci:, save_on_errors:)
       unless pipeline.config_processor
         unless pipeline.ci_yaml_file
           return error("Missing #{pipeline.ci_yaml_file_path} file")
@@ -85,25 +87,25 @@ module Ci
       end
     end
 
-    def allowed_to_trigger_pipeline?(triggering_user)
-      if triggering_user
-        allowed_to_create?(triggering_user)
+    def allowed_to_trigger_pipeline?
+      if current_user
+        allowed_to_create?
       else # legacy triggers don't have a corresponding user
         !project.protected_for?(ref)
       end
     end
 
-    def allowed_to_create?(triggering_user)
-      access = Gitlab::UserAccess.new(triggering_user, project: project)
+    def allowed_to_create?
+      return unless can?(current_user, :create_pipeline, project)
 
-      can?(triggering_user, :create_pipeline, project) &&
-        if branch?
-          access.can_update_branch?(ref)
-        elsif tag?
-          access.can_create_tag?(ref)
-        else
-          true # Allow it for now and we'll reject when we check ref existence
-        end
+      access = Gitlab::UserAccess.new(current_user, project: project)
+      if branch?
+        access.can_update_branch?(ref)
+      elsif tag?
+        access.can_create_tag?(ref)
+      else
+        true # Allow it for now and we'll reject when we check ref existence
+      end
     end
 
     def update_merge_requests_head_pipeline
diff --git a/changelogs/unreleased/31362_decrease_cyclomatic_complexity_threshold_step3.yml b/changelogs/unreleased/31362_decrease_cyclomatic_complexity_threshold_step3.yml
new file mode 100644
index 00000000000..4a8d8097169
--- /dev/null
+++ b/changelogs/unreleased/31362_decrease_cyclomatic_complexity_threshold_step3.yml
@@ -0,0 +1,5 @@
+---
+title: Decrease Cyclomatic Complexity threshold to 14
+merge_request: 13972
+author: Maxim Rydkin
+type: other
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index d1efa318d14..49cb7c954b4 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -191,10 +191,31 @@ describe ProjectsHelper do
     end
   end
 
-  describe 'link_to_member' do
-    let(:group)   { create(:group) }
-    let(:project) { create(:project, group: group) }
-    let(:user)    { create(:user) }
+  describe '#link_to_member_avatar' do
+    let(:user) { build_stubbed(:user) }
+    let(:expected) { double }
+
+    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
       it 'returns an HTML link to the user' do
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 49d7c663128..009d67a3fbe 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -489,7 +489,7 @@ describe Ci::CreatePipelineService do
 
     subject do
       described_class.new(project, user, ref: ref)
-        .send(:allowed_to_create?, user)
+        .send(:allowed_to_create?)
     end
 
     context 'when user is a developer' do