Added tests for the repository_controller and repository_helper

Added specs for the deploy_keys_presenter and added a new method in the presenter
called #key_available?

Fixed some minor UX inconsistencies and added a concern to handle
redirection

8 files changed, 41 insertions, 26 deletions

diff --git a/app/controllers/concerns/redirect_request.rb b/app/controllers/concerns/redirect_request.rb
new file mode 100644
index 00000000000..acab5a8446a
--- /dev/null
+++ b/app/controllers/concerns/redirect_request.rb
@@ -0,0 +1,7 @@
+module RedirectRequest
+  extend ActiveSupport::Concern
+
+  def redirect_to_repository_settings(project)
+    redirect_to namespace_project_settings_repository_path(project.namespace, project)
+  end
+end
diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb
index 0d60e782dfb..465757e07b1 100644
--- a/app/controllers/projects/deploy_keys_controller.rb
+++ b/app/controllers/projects/deploy_keys_controller.rb
@@ -1,4 +1,5 @@
 class Projects::DeployKeysController < Projects::ApplicationController
+  include RedirectRequest
   respond_to :html
 
   # Authorize
@@ -7,32 +8,32 @@ class Projects::DeployKeysController < Projects::ApplicationController
   layout "project_settings"
 
   def index
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def new
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def create
     @key = DeployKey.new(deploy_key_params.merge(user: current_user))
 
     unless @key.valid? && @project.deploy_keys << @key
-      flash[:alert] = @key.errors.full_messages.join(',').html_safe      
+      flash[:alert] = @key.errors.full_messages.join(', ').html_safe      
     end
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def enable
     Projects::EnableDeployKeyService.new(@project, current_user, params).execute
 
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def disable
     @project.deploy_keys_projects.find_by(deploy_key_id: params[:id]).destroy
 
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   protected
diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb
index 1c22b9c2ca8..b7817a71e0d 100644
--- a/app/controllers/projects/protected_branches_controller.rb
+++ b/app/controllers/projects/protected_branches_controller.rb
@@ -1,4 +1,5 @@
 class Projects::ProtectedBranchesController < Projects::ApplicationController
+  include RedirectRequest
   # Authorize
   before_action :require_non_empty_project
   before_action :authorize_admin_project!
@@ -7,15 +8,15 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
   layout "project_settings"
 
   def index
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def create
     @protected_branch = ::ProtectedBranches::CreateService.new(@project, current_user, protected_branch_params).execute
     unless @protected_branch.persisted?
-      flash[:alert] = @protected_branches.errors.full_messages.join(',').html_safe
+      flash[:alert] = @protected_branches.errors.full_messages.join(', ').html_safe
     end
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def show
@@ -40,7 +41,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
     @protected_branch.destroy
 
     respond_to do |format|
-      format.html { redirect_to namespace_project_settings_repository_path }
+      format.html { redirect_to_repository_settings(@project) }
       format.js { head :ok }
     end
   end
@@ -56,8 +57,4 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
                                              merge_access_levels_attributes: [:access_level, :id],
                                              push_access_levels_attributes: [:access_level, :id])
   end
-
-  def load_protected_branches
-    @protected_branches = @project.protected_branches.order(:name).page(params[:page])
-  end
 end
diff --git a/app/controllers/projects/settings/repository_controller.rb b/app/controllers/projects/settings/repository_controller.rb
index a8c6a1b419c..3c9417b9270 100644
--- a/app/controllers/projects/settings/repository_controller.rb
+++ b/app/controllers/projects/settings/repository_controller.rb
@@ -4,25 +4,25 @@ module Projects
       include RepositoryHelper
 
       before_action :authorize_admin_project!
-      before_action :load_protected_branches, only: [:show]
 
       def show
         @deploy_keys = DeployKeysPresenter
           .new(@project, current_user: @current_user)
 
-        define_protected_branches_controller
-      end
-
-      def load_protected_branches
-        @protected_branches = @project.protected_branches.order(:name).page(params[:page])
+        define_protected_branches
       end
 
       private
 
-      def define_protected_branches_controller
+      def define_protected_branches
+        load_protected_branches
         @protected_branch = @project.protected_branches.new
         load_gon_index
       end
+
+      def load_protected_branches
+        @protected_branches = @project.protected_branches.order(:name).page(params[:page])
+      end
     end
   end
 end
diff --git a/app/helpers/repository_helper.rb b/app/helpers/repository_helper.rb
index a079f1a889c..e34f9338b54 100644
--- a/app/helpers/repository_helper.rb
+++ b/app/helpers/repository_helper.rb
@@ -2,16 +2,22 @@ module RepositoryHelper
   def access_levels_options
     {
       push_access_levels: {
-        "Roles" => ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } },
+        "Roles" => ProtectedBranch::PushAccessLevel.human_access_levels.map do |id, text| 
+          { id: id, text: text, before_divider: true } 
+        end
       },
       merge_access_levels: {
-        "Roles" => ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } }
+        "Roles" => ProtectedBranch::MergeAccessLevel.human_access_levels.map do |id, text| 
+          { id: id, text: text, before_divider: true } 
+        end
       }
     }
   end
 
   def load_gon_index
-    params = { open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } } }
+    params = { open_branches: @project.open_branches.map do |br| 
+      { text: br.name, id: br.name, title: br.name } 
+    end }
     gon.push(params.merge(access_levels_options))
   end
 end
diff --git a/app/presenters/projects/settings/deploy_keys_presenter.rb b/app/presenters/projects/settings/deploy_keys_presenter.rb
index 703d93708f6..25be14bd28b 100644
--- a/app/presenters/projects/settings/deploy_keys_presenter.rb
+++ b/app/presenters/projects/settings/deploy_keys_presenter.rb
@@ -35,6 +35,10 @@ module Projects
         available_project_keys.size
       end
 
+      def key_available?(deploy_key)
+        available_keys.include?(deploy_key)
+      end
+
       def available_public_keys
         return @available_public_keys if defined?(@available_public_keys)
 
diff --git a/app/views/projects/deploy_keys/_deploy_key.html.haml b/app/views/projects/deploy_keys/_deploy_key.html.haml
index 337f8c38a93..ec8fc4c9ee8 100644
--- a/app/views/projects/deploy_keys/_deploy_key.html.haml
+++ b/app/views/projects/deploy_keys/_deploy_key.html.haml
@@ -18,7 +18,7 @@
     %span.key-created-at
       created #{time_ago_with_tooltip(deploy_key.created_at)}
     .visible-xs-block.visible-sm-block
-    - if @deploy_keys.available_keys.include?(deploy_key)
+    - if @deploy_keys.key_available?(deploy_key)
       = link_to enable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), class: "btn btn-sm prepend-left-10", method: :put do
         Enable
     - else
diff --git a/app/views/projects/deploy_keys/_index.html.haml b/app/views/projects/deploy_keys/_index.html.haml
index 91acad83bf3..9f351d3b90e 100644
--- a/app/views/projects/deploy_keys/_index.html.haml
+++ b/app/views/projects/deploy_keys/_index.html.haml
@@ -19,7 +19,7 @@
           = render partial: 'projects/deploy_keys/deploy_key', locals: {deploy_key: enabled_key}
     - else
       .settings-message.text-center
-        No deploy keys found. Create one with the form above or add existing one below.
+        No deploy keys found. Create one with the form above.
     %h5.prepend-top-default
       Deploy keys from projects you have access to (#{@deploy_keys.available_project_keys_size})
     - if @deploy_keys.any_available_project_keys_enabled?