Merge branch '31358-ce-step-to-16-perc-compl' into 'master'

Decrease Perceived and Cyclomatic Complexity

See merge request !2874

app/helpers/ee/lock_helper.rb

+module EE
+  module LockHelper
+    def lock_file_link(project = @project, path = @path, html_options: {})
+      return unless project.feature_available?(:file_locks) && current_user
+      return if path.blank?
+
+      path_lock = project.find_path_lock(path, downstream: true)
+
+      if path_lock
+        locker = path_lock.user.name
+
+        if path_lock.exact?(path)
+          exact_lock_file_link(path_lock, html_options, locker)
+        elsif path_lock.upstream?(path)
+          upstream_lock_file_link(path_lock, html_options, locker)
+        elsif path_lock.downstream?(path)
+          downstream_lock_file_link(path_lock, html_options, locker)
+        end
+      else
+        _lock_link(current_user, project, html_options: html_options)
+      end
+    end
+
+    def exact_lock_file_link(path_lock, html_options, locker)
+      if can_unlock?(path_lock)
+        html_options[:data] = { state: :unlock }
+        tooltip = path_lock.user == current_user ? '' : "Locked by #{locker}"
+        enabled_lock_link("Unlock", tooltip, html_options)
+      else
+        disabled_lock_link("Unlock", "Locked by #{locker}. You do not have permission to unlock this", html_options)
+      end
+    end
+
+    def upstream_lock_file_link(path_lock, html_options, locker)
+      additional_phrase = can_unlock?(path_lock) ? 'Unlock that directory in order to unlock this' : 'You do not have permission to unlock it'
+      disabled_lock_link("Unlock", "#{locker} has a lock on \"#{path_lock.path}\". #{additional_phrase}", html_options)
+    end
+
+    def downstream_lock_file_link(path_lock, html_options, locker)
+      additional_phrase = can_unlock?(path_lock) ? 'Unlock this in order to proceed' : 'You do not have permission to unlock it'
+      disabled_lock_link("Lock", "This directory cannot be locked while #{locker} has a lock on \"#{path_lock.path}\". #{additional_phrase}", html_options)
+    end
+
+    def _lock_link(user, project, html_options: {})
+      if can?(current_user, :push_code, project)
+        html_options[:data] = { state: :lock }
+        enabled_lock_link("Lock", '', html_options)
+      else
+        disabled_lock_link("Lock", "You do not have permission to lock this", html_options)
+      end
+    end
+
+    def disabled_lock_link(label, title, html_options)
+      html_options['data-toggle'] = 'tooltip'
+      html_options[:title] = title
+      html_options[:class] = "#{html_options[:class]} disabled has-tooltip"
+
+      content_tag :span, label, html_options
+    end
+
+    def enabled_lock_link(label, title, html_options)
+      html_options['data-toggle'] = 'tooltip'
+      html_options[:title] = title
+      html_options[:class] = "#{html_options[:class]} has-tooltip"
+
+      link_to label, '#', html_options
+    end
+
+    def render_lock_icon(path)
+      return unless @project.feature_available?(:file_locks)
+      return unless @project.root_ref?(@ref)
+
+      if file_lock = @project.find_path_lock(path, exact_match: true)
+        content_tag(
+          :i,
+          nil,
+          class: "fa fa-lock prepend-left-5 append-right-5",
+          title: text_label_for_lock(file_lock, path),
+          'data-toggle' => 'tooltip'
+        )
+      end
+    end
+  end
+end

app/helpers/license_helper.rb

@@ -20,23 +20,14 @@ module LicenseHelper
     is_trial = current_license.trial?
     message = ["Your Enterprise Edition #{'trial ' if is_trial}license"]
 
-    message << if current_license.expired?
-                 "expired on #{current_license.expires_at}."
-               else
-                 "will expire in #{pluralize(current_license.remaining_days, 'day')}."
-               end
+    message << expiration_message
 
     message << link_to('Buy now!', Gitlab::SUBSCRIPTIONS_PLANS_URL, target: '_blank') if is_trial
 
     if current_license.expired? && current_license.will_block_changes?
       message << 'Pushing code and creation of issues and merge requests'
 
-      message <<
-        if current_license.block_changes?
-          'has been disabled.'
-        else
-          "will be disabled on #{current_license.block_changes_at}."
-        end
+      message << block_changes_message
 
       message <<
         if is_admin
@@ -53,6 +44,22 @@ module LicenseHelper
     message.join(' ').html_safe
   end
 
+  def expiration_message
+    if current_license.expired?
+      "expired on #{current_license.expires_at}."
+    else
+      "will expire in #{pluralize(current_license.remaining_days, 'day')}."
+    end
+  end
+
+  def block_changes_message
+    if current_license.block_changes?
+      'has been disabled.'
+    else
+      "will be disabled on #{current_license.block_changes_at}."
+    end
+  end
+
   def current_license
     return @current_license if defined?(@current_license)
 

app/helpers/tree_helper.rb

@@ -109,79 +109,4 @@ module TreeHelper
       return tree.name
     end
   end
-
-  def lock_file_link(project = @project, path = @path, html_options: {})
-    return unless project.feature_available?(:file_locks) && current_user
-    return if path.blank?
-
-    path_lock = project.find_path_lock(path, downstream: true)
-
-    if path_lock
-      locker = path_lock.user.name
-
-      if path_lock.exact?(path)
-        if can_unlock?(path_lock)
-          html_options[:data] = { state: :unlock }
-          tooltip = path_lock.user == current_user ? '' : "Locked by #{locker}"
-          enabled_lock_link("Unlock", tooltip, html_options)
-        else
-          disabled_lock_link("Unlock", "Locked by #{locker}. You do not have permission to unlock this", html_options)
-        end
-      elsif path_lock.upstream?(path)
-        if can_unlock?(path_lock)
-          disabled_lock_link("Unlock", "#{locker} has a lock on \"#{path_lock.path}\". Unlock that directory in order to unlock this", html_options)
-        else
-          disabled_lock_link("Unlock", "#{locker} has a lock on \"#{path_lock.path}\". You do not have permission to unlock it", html_options)
-        end
-      elsif path_lock.downstream?(path)
-        if can_unlock?(path_lock)
-          disabled_lock_link("Lock", "This directory cannot be locked while #{locker} has a lock on \"#{path_lock.path}\". Unlock this in order to proceed", html_options)
-        else
-          disabled_lock_link("Lock", "This directory cannot be locked while #{locker} has a lock on \"#{path_lock.path}\". You do not have permission to unlock it", html_options)
-        end
-      end
-    else
-      _lock_link(current_user, project, html_options: html_options)
-    end
-  end
-
-  def _lock_link(user, project, html_options: {})
-    if can?(current_user, :push_code, project)
-      html_options[:data] = { state: :lock }
-      enabled_lock_link("Lock", '', html_options)
-    else
-      disabled_lock_link("Lock", "You do not have permission to lock this", html_options)
-    end
-  end
-
-  def disabled_lock_link(label, title, html_options)
-    html_options['data-toggle'] = 'tooltip'
-    html_options[:title] = title
-    html_options[:class] = "#{html_options[:class]} disabled has-tooltip"
-
-    content_tag :span, label, html_options
-  end
-
-  def enabled_lock_link(label, title, html_options)
-    html_options['data-toggle'] = 'tooltip'
-    html_options[:title] = title
-    html_options[:class] = "#{html_options[:class]} has-tooltip"
-
-    link_to label, '#', html_options
-  end
-
-  def render_lock_icon(path)
-    return unless @project.feature_available?(:file_locks)
-    return unless @project.root_ref?(@ref)
-
-    if file_lock = @project.find_path_lock(path, exact_match: true)
-      content_tag(
-        :i,
-        nil,
-        class: "fa fa-lock prepend-left-5 append-right-5",
-        title: text_label_for_lock(file_lock, path),
-        'data-toggle' => 'tooltip'
-      )
-    end
-  end
 end

spec/helpers/ee/lock_helper_spec.rb

+require "spec_helper"
+
+describe EE::LockHelper do
+  describe '#lock_file_link' do
+    let!(:path_lock) { create :path_lock, path: 'app/models' }
+    let(:path) { path_lock.path }
+    let(:user) { path_lock.user }
+    let(:project) { path_lock.project }
+
+    before do
+      allow(helper).to receive(:can?).and_return(true)
+      allow(helper).to receive(:current_user).and_return(user)
+      allow(project).to receive(:feature_available?).with(:file_locks) { true }
+
+      project.reload
+    end
+
+    context "there is no locks" do
+      it "returns Lock with no toltip" do
+        expect(helper.lock_file_link(project, '.gitignore')).to match('Lock')
+      end
+
+      it "returns Lock button with tooltip" do
+        allow(helper).to receive(:can?).and_return(false)
+        expect(helper.lock_file_link(project, '.gitignore')).to match('You do not have permission to lock this')
+      end
+    end
+
+    context "exact lock" do
+      it "returns Unlock with no toltip" do
+        expect(helper.lock_file_link(project, path)).to match('Unlock')
+      end
+
+      it "returns Lock button with tooltip" do
+        allow(helper).to receive(:can?).and_return(false)
+        expect(helper.lock_file_link(project, path)).to match('Unlock')
+        expect(helper.lock_file_link(project, path)).to match("Locked by #{user.name}. You do not have permission to unlock this.")
+      end
+    end
+
+    context "upstream lock" do
+      let(:requested_path) { 'app/models/user.rb' }
+
+      it "returns Lock with no toltip" do
+        expect(helper.lock_file_link(project, requested_path)).to match('Unlock')
+        expect(helper.lock_file_link(project, requested_path)).to match(html_escape("#{user.name} has a lock on \"app/models\". Unlock that directory in order to unlock this"))
+      end
+
+      it "returns Lock button with tooltip" do
+        allow(helper).to receive(:can?).and_return(false)
+        expect(helper.lock_file_link(project, requested_path)).to match('Unlock')
+        expect(helper.lock_file_link(project, requested_path)).to match(html_escape("#{user.name} has a lock on \"app/models\". You do not have permission to unlock it"))
+      end
+    end
+
+    context "downstream lock" do
+      it "returns Lock with no toltip" do
+        expect(helper.lock_file_link(project, 'app')).to match(html_escape("This directory cannot be locked while #{user.name} has a lock on \"app/models\". Unlock this in order to proceed"))
+      end
+
+      it "returns Lock button with tooltip" do
+        allow(helper).to receive(:can?).and_return(false)
+        expect(helper.lock_file_link(project, 'app')).to match('Lock')
+        expect(helper.lock_file_link(project, 'app')).to match(html_escape("This directory cannot be locked while #{user.name} has a lock on \"app/models\". You do not have permission to unlock it"))
+      end
+    end
+  end
+end

spec/helpers/tree_helper_spec.rb

@@ -35,69 +35,4 @@ describe TreeHelper do
       end
     end
   end
-
-  describe '#lock_file_link' do
-    let!(:path_lock) { create :path_lock, path: 'app/models' }
-    let(:path) { path_lock.path }
-    let(:user) { path_lock.user }
-    let(:project) { path_lock.project }
-
-    before do
-      allow(helper).to receive(:can?).and_return(true)
-      allow(helper).to receive(:current_user).and_return(user)
-      allow(project).to receive(:feature_available?).with(:file_locks) { true }
-
-      project.reload
-    end
-
-    context "there is no locks" do
-      it "returns Lock with no toltip" do
-        expect(helper.lock_file_link(project, '.gitignore')).to match('Lock')
-      end
-
-      it "returns Lock button with tooltip" do
-        allow(helper).to receive(:can?).and_return(false)
-        expect(helper.lock_file_link(project, '.gitignore')).to match('You do not have permission to lock this')
-      end
-    end
-
-    context "exact lock" do
-      it "returns Unlock with no toltip" do
-        expect(helper.lock_file_link(project, path)).to match('Unlock')
-      end
-
-      it "returns Lock button with tooltip" do
-        allow(helper).to receive(:can?).and_return(false)
-        expect(helper.lock_file_link(project, path)).to match('Unlock')
-        expect(helper.lock_file_link(project, path)).to match("Locked by #{user.name}. You do not have permission to unlock this.")
-      end
-    end
-
-    context "upstream lock" do
-      let(:requested_path) { 'app/models/user.rb' }
-
-      it "returns Lock with no toltip" do
-        expect(helper.lock_file_link(project, requested_path)).to match('Unlock')
-        expect(helper.lock_file_link(project, requested_path)).to match(html_escape("#{user.name} has a lock on \"app/models\". Unlock that directory in order to unlock this"))
-      end
-
-      it "returns Lock button with tooltip" do
-        allow(helper).to receive(:can?).and_return(false)
-        expect(helper.lock_file_link(project, requested_path)).to match('Unlock')
-        expect(helper.lock_file_link(project, requested_path)).to match(html_escape("#{user.name} has a lock on \"app/models\". You do not have permission to unlock it"))
-      end
-    end
-
-    context "downstream lock" do
-      it "returns Lock with no toltip" do
-        expect(helper.lock_file_link(project, 'app')).to match(html_escape("This directory cannot be locked while #{user.name} has a lock on \"app/models\". Unlock this in order to proceed"))
-      end
-
-      it "returns Lock button with tooltip" do
-        allow(helper).to receive(:can?).and_return(false)
-        expect(helper.lock_file_link(project, 'app')).to match('Lock')
-        expect(helper.lock_file_link(project, 'app')).to match(html_escape("This directory cannot be locked while #{user.name} has a lock on \"app/models\". You do not have permission to unlock it"))
-      end
-    end
-  end
 end