Added tests for the repository_controller

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

.vscode/settings.json

-{}
\ No newline at end of file

app/controllers/concerns/redirect_request.rb

+module RedirectRequest
+  extend ActiveSupport::Concern
+
+  def redirect_to_repository_settings(project)
+    redirect_to namespace_project_settings_repository_path(project.namespace, project)
+  end
+end

app/controllers/projects/deploy_keys_controller.rb

 class Projects::DeployKeysController < Projects::ApplicationController
+  include RedirectRequest
   respond_to :html
 
   # Authorize
@@ -7,22 +8,22 @@ 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
     else
       log_audit_event(@key.title, action: :create)
     end
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def enable
@@ -30,7 +31,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
     Projects::EnableDeployKeyService.new(@project, current_user, params).execute
     log_audit_event(@key.title, action: :create)
 
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def disable
@@ -38,7 +39,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
     @project.deploy_keys_projects.find_by(deploy_key_id: params[:id]).destroy
     log_audit_event(@key.title, action: :destroy)
 
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   protected

app/controllers/projects/mirrors_controller.rb

 class Projects::MirrorsController < Projects::ApplicationController
+  include RedirectRequest
   # Authorize
   before_action :authorize_admin_project!, except: [:update_now]
   before_action :authorize_push_code!, only: [:update_now]
@@ -7,7 +8,7 @@ class Projects::MirrorsController < Projects::ApplicationController
   layout "project_settings"
 
   def show
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def update
@@ -22,10 +23,10 @@ class Projects::MirrorsController < Projects::ApplicationController
         flash[:notice] = "Mirroring settings were successfully updated."
       end
     else
-      flash[:alert] = @project.errors.full_messages.join(',').html_safe
+      flash[:alert] = @project.errors.full_messages.join(', ').html_safe
     end
 
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   def update_now
@@ -36,7 +37,7 @@ class Projects::MirrorsController < Projects::ApplicationController
       @project.update_mirror
       flash[:notice] = "The repository is being updated..."
     end
-    redirect_to namespace_project_settings_repository_path(@project.namespace, @project)
+    redirect_to_repository_settings(@project)
   end
 
   private

app/controllers/projects/protected_branches_controller.rb

 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
@@ -76,7 +77,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
 
   def load_gon_index
     params = { open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } } }
-    params[:current_project_id] = @project.id if @project
+    params.merge!(current_project_id: @project.id) if @project
     gon.push(params.merge(access_levels_options))
   end
 end

app/controllers/projects/push_rules_controller.rb

 class Projects::PushRulesController < Projects::ApplicationController
+  include RedirectRequest
   # Authorize
   before_action :authorize_admin_project!
 
@@ -17,10 +18,11 @@ class Projects::PushRulesController < Projects::ApplicationController
     @push_rule.update_attributes(push_rule_params)
 
     if @push_rule.valid?
-      redirect_to namespace_project_push_rules_path(@project.namespace, @project), notice: 'Push Rules updated successfully.'
+      flash[:notice] = 'Push Rules updated successfully.'
     else
-      render :index
+      flash[:alert] = @push_rule.errors.full_messages.join(', ').html_safe
     end
+    redirect_to_repository_settings(@project)
   end
 
   private

app/controllers/projects/settings/repository_controller.rb

@@ -4,7 +4,6 @@ module Projects
       include RepositoryHelper
 
       before_action :authorize_admin_project!
-      before_action :load_protected_branches, only: [:show]
       before_action :push_rule, only: [:show]
       before_action :remote_mirror, only: [:show]
 
@@ -12,16 +11,13 @@ module Projects
         @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])
-      end
+        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
@@ -33,6 +29,10 @@ module Projects
       def remote_mirror
         @remote_mirror = @project.remote_mirrors.first_or_initialize
       end
+
+      def load_protected_branches
+        @protected_branches = @project.protected_branches.order(:name).page(params[:page])
+      end
     end
   end
 end

app/helpers/repository_helper.rb

 module RepositoryHelper
   def access_levels_options
     {
-      push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text } },
-      merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text } },
-      selected_merge_access_levels: @protected_branch.merge_access_levels.map { |access_level| access_level.user_id || access_level.access_level },
-      selected_push_access_levels: @protected_branch.push_access_levels.map { |access_level| access_level.user_id || access_level.access_level }
+      push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map do |id, text| 
+        { id: id, text: text } 
+      end,
+      merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map do |id, text| 
+        { id: id, text: text } 
+      end,
+      selected_merge_access_levels: @protected_branch.merge_access_levels.map do |access_level| 
+        access_level.user_id || access_level.access_level 
+      end,
+      selected_push_access_levels: @protected_branch.push_access_levels.map do |access_level| 
+        access_level.user_id || access_level.access_level 
+      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 }
     params.merge!(current_project_id: @project.id) if @project
     gon.push(params.merge(access_levels_options))
   end

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)
 

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

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?

app/views/projects/push_rules/_index.html.haml

-- page_title "Push Rules"
 
 .row.prepend-top-default.append-bottom-default
   .col-lg-3
     %h4.prepend-top-0
-      = page_title
+      Push Rules
     %p.light
       Push Rules outline what is accepted for this project. You can request new rules (for free) by creating an issue on our
       = succeed '.' do

changelogs/unreleased-ee/26732-combine-deploy-keys-and-push-rules-and-mirror-repository-and-protect-branches-settings-pages-ee.yml

 ---
-title: Combined deploy keys and push rules settings options into a single one called
+title: Combined deploy keys, push rules, protect branches and mirror repository settings options into a single one called
   Repository
 merge_request:
 author:

spec/controllers/projects/settings/repository_controller_spec.rb

+require 'spec_helper'
+
+describe Projects::Settings::IntegrationsController do
+  let(:project) { create(:empty_project, :public) }
+  let(:user) { create(:user) }
+
+  before do
+    project.team << [user, :master]
+    sign_in(user)
+  end
+
+  describe 'GET show' do
+    it 'renders show with 200 status code' do
+      get :show, namespace_id: project.namespace, project_id: project
+
+      expect(response).to have_http_status(200)
+      expect(response).to render_template(:show)
+    end
+  end
+end

spec/presenters/projects/settings/deploy_keys_presenter_spec.rb

@@ -3,6 +3,11 @@ require 'spec_helper'
 describe Projects::Settings::DeployKeysPresenter do
   let(:project) { create(:empty_project) }
   let(:user) { create(:user) }
+  let(:deploy_key)  { create(:deploy_key, public: true) }
+
+  let!(:deploy_keys_project) do
+    create(:deploy_keys_project, project: project, deploy_key: deploy_key)
+  end
 
   subject(:presenter) do
     described_class.new(project, current_user: user)
@@ -13,16 +18,42 @@ describe Projects::Settings::DeployKeysPresenter do
   end
 
   describe '#enabled_keys' do
-    let(:deploy_key) do
-      create(:deploy_keys_project, project: project).deploy_key
-    end
-
-    it 'returns project keys' do
-      expect(presenter.enabled_keys).to eq [deploy_key]
+    it 'returns currently enabled keys' do
+      expect(presenter.enabled_keys).to eq [deploy_keys_project.deploy_key]
     end
 
     it 'does not contain enabled_keys inside available_keys' do
       expect(presenter.available_keys).not_to include deploy_key
     end
+
+    it 'returns the enabled_keys size' do
+      expect(presenter.enabled_keys_size).to eq(1)
+    end
+
+    it 'returns true if there is any enabled_keys' do
+      expect(presenter.any_keys_enabled?).to eq(true)
+    end
+  end
+
+  describe '#available_keys/#available_project_keys' do
+    it 'returns the current available_keys' do
+      expect(presenter.available_keys).to be_empty
+    end
+
+    it 'returns the current available_project_keys' do
+      expect(presenter.available_project_keys).to be_empty
+    end
+
+    it 'returns if any available_project_keys are enabled' do
+      expect(presenter.any_available_project_keys_enabled?).to eq(false)
+    end
+
+    it 'returns the available_project_keys size' do
+      expect(presenter.available_project_keys_size).to eq(0)
+    end
+
+    it 'shows if there is an available key' do
+      expect(presenter.key_available?(deploy_key)).to eq(false)
+    end
   end
 end