Fixed protected brancheslukehowell/gitlab-ce-23007-replace-settings-gear-with-tab

Fixed pipeline badges ref bug

Fixed failing features

11 files changed, 66 insertions, 52 deletions

diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb
index db2c8ae6b53..755bd30d04b 100644
--- a/app/controllers/projects/deploy_keys_controller.rb
+++ b/app/controllers/projects/deploy_keys_controller.rb
@@ -1,4 +1,6 @@
 class Projects::DeployKeysController < Projects::ApplicationController
+  include ProtectedBranchesHelper
+
   respond_to :html
 
   # Authorize
@@ -11,6 +13,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
     @protected_branch = @project.protected_branches.new
     @protected_branches = @project.protected_branches.order(:name).page(params[:page])
     set_index_vars
+    load_gon_index(@project)
   end
 
   def new
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 76e74e79659..c2173a54b9e 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -10,8 +10,7 @@ class Projects::HooksController < Projects::ApplicationController
     @hooks = @project.hooks
     @hook = ProjectHook.new
 
-    @project.build_missing_services
-    @services = @project.services.visible.reload
+    @services = @project.find_or_initialize_services
   end
 
   def create
diff --git a/app/controllers/projects/pipelines_settings_controller.rb b/app/controllers/projects/pipelines_settings_controller.rb
index c7eb459e3d0..31f086af9e2 100644
--- a/app/controllers/projects/pipelines_settings_controller.rb
+++ b/app/controllers/projects/pipelines_settings_controller.rb
@@ -2,7 +2,7 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController
   before_action :authorize_admin_pipeline!
 
   def show
-    redirect_to namespace_project_runners_path(@project.namespace, @project)
+    redirect_to namespace_project_runners_path(@project.namespace, @project, params)
   end
 
   def update
diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb
index 9a438d5512c..ae360e4f26e 100644
--- a/app/controllers/projects/protected_branches_controller.rb
+++ b/app/controllers/projects/protected_branches_controller.rb
@@ -1,4 +1,6 @@
 class Projects::ProtectedBranchesController < Projects::ApplicationController
+  include ProtectedBranchesHelper
+
   # Authorize
   before_action :require_non_empty_project
   before_action :authorize_admin_project!
@@ -9,16 +11,16 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
 
   def index
     @protected_branch = @project.protected_branches.new
-    load_gon_index
+    load_gon_index(@project)
   end
 
   def create
     @protected_branch = ::ProtectedBranches::CreateService.new(@project, current_user, protected_branch_params).execute
     if @protected_branch.persisted?
-      redirect_to namespace_project_protected_branches_path(@project.namespace, @project)
+      redirect_to_protected_branches
     else
       load_protected_branches
-      load_gon_index
+      load_gon_index(@project)
       render :index
     end
   end
@@ -45,7 +47,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
     @protected_branch.destroy
 
     respond_to do |format|
-      format.html { redirect_to namespace_project_protected_branches_path }
+      format.html { redirect_to_protected_branches }
       format.js { head :ok }
     end
   end
@@ -66,15 +68,12 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
     @protected_branches = @project.protected_branches.order(:name).page(params[:page])
   end
 
-  def access_levels_options
-    {
-      push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } },
-      merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } }
-    }
-  end
-
-  def load_gon_index
-    params = { open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } } }
-    gon.push(params.merge(access_levels_options))
+  def redirect_to_protected_branches
+    if Rails.application.routes.recognize_path(request.referer)[:controller] == 'projects/deploy-keys'
+      path = namespace_project_protected_branches_path(@project.namespace, @project)
+    else
+      path = namespace_project_deploy_keys_path(@project.namespace, @project)
+    end
+    redirect_to path
   end
 end
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 05696f8b169..a8a18b4fa16 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -26,10 +26,6 @@ class ProjectsController < Projects::ApplicationController
     render 'edit'
   end
 
-  def integrations
-    render 'settings/integrations'
-  end
-
   def create
     @project = ::Projects::CreateService.new(current_user, project_params).execute
 
diff --git a/app/helpers/protected_branches_helper.rb b/app/helpers/protected_branches_helper.rb
new file mode 100644
index 00000000000..7f476807996
--- /dev/null
+++ b/app/helpers/protected_branches_helper.rb
@@ -0,0 +1,13 @@
+module ProtectedBranchesHelper
+  def access_levels_options
+    {
+      push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } },
+      merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } }
+    }
+  end
+
+  def load_gon_index(project)
+    params = { open_branches: project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } } }
+    gon.push(params.merge(access_levels_options))
+  end
+end
diff --git a/features/project/active_tab.feature b/features/project/active_tab.feature
index 57dda9c2234..dd59519e100 100644
--- a/features/project/active_tab.feature
+++ b/features/project/active_tab.feature
@@ -41,22 +41,22 @@ Feature: Project Active Tab
 
   Scenario: On Project Settings/Hooks
     Given I visit my project's settings page
-    And I click the "Hooks" tab
-    Then the active sub nav should be Hooks
-    And no other sub navs should be active
+    And I click the "Integrations" tab
+    Then the active sub tab should be Integrations
+    And no other sub tabs should be active
     And the active main tab should be Settings
 
   Scenario: On Project Settings/Deploy Keys
     Given I visit my project's settings page
-    And I click the "Deploy Keys" tab
-    Then the active sub nav should be Deploy Keys
-    And no other sub navs should be active
+    And I click the "Repository" tab
+    Then the active sub tab should be Repository
+    And no other sub tabs should be active
     And the active main tab should be Settings
 
   Scenario: On Project Members
     Given I visit my project's members page
     Then the active sub nav should be Members
-    And no other sub navs should be active
+    And no other sub tabs should be active
     And the active main tab should be Settings
 
   # Sub Tabs: Repository
diff --git a/features/steps/project/active_tab.rb b/features/steps/project/active_tab.rb
index 58225032859..aad195b3110 100644
--- a/features/steps/project/active_tab.rb
+++ b/features/steps/project/active_tab.rb
@@ -27,24 +27,28 @@ class Spinach::Features::ProjectActiveTab < Spinach::FeatureSteps
     end
   end
 
-  step 'I click the "Hooks" tab' do
-    click_link('Webhooks')
+  step 'I click the "Integrations" tab' do
+    page.within '.sub-nav' do
+      click_link('Integrations')
+    end
   end
 
-  step 'I click the "Deploy Keys" tab' do
-    click_link('Deploy Keys')
+  step 'I click the "Repository" tab' do
+    page.within '.sub-nav' do
+      click_link('Repository')
+    end
   end
 
-  step 'the active sub nav should be Members' do
-    ensure_active_sub_nav('Members')
+  step 'the active sub tab should be Members' do
+    ensure_active_sub_tab('Members')
   end
 
-  step 'the active sub nav should be Hooks' do
-    ensure_active_sub_nav('Webhooks')
+  step 'the active sub tab should be Integrations' do
+    ensure_active_sub_tab('Integrations')
   end
 
-  step 'the active sub nav should be Deploy Keys' do
-    ensure_active_sub_nav('Deploy Keys')
+  step 'the active sub tab should be Repository' do
+    ensure_active_sub_tab('Repository')
   end
 
   # Sub Tabs: Commits
diff --git a/features/steps/shared/project_tab.rb b/features/steps/shared/project_tab.rb
index d6024212601..8c81976fba6 100644
--- a/features/steps/shared/project_tab.rb
+++ b/features/steps/shared/project_tab.rb
@@ -20,8 +20,8 @@ module SharedProjectTab
     ensure_active_main_tab('Issues')
   end
 
-  step 'the active main tab should be Members' do
-    ensure_active_main_tab('Members')
+  step 'the active sub nav should be Members' do
+    ensure_active_sub_tab('Members')
   end
 
   step 'the active main tab should be Merge Requests' do
@@ -37,7 +37,7 @@ module SharedProjectTab
   end
 
   step 'the active main tab should be Settings' do
-    expect(page).to have_selector('.layout-nav .nav-links > li.active', count: 0)
+    ensure_active_main_tab('Settings')
   end
 
   step 'the active main tab should be Activity' do
diff --git a/spec/features/protected_branches/access_control_ce_spec.rb b/spec/features/protected_branches/access_control_ce_spec.rb
index 395c61a4743..7f0c6de1d3e 100644
--- a/spec/features/protected_branches/access_control_ce_spec.rb
+++ b/spec/features/protected_branches/access_control_ce_spec.rb
@@ -1,7 +1,7 @@
 RSpec.shared_examples "protected branches > access control > CE" do
   ProtectedBranch::PushAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
     it "allows creating protected branches that #{access_type_name} can push to" do
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('master')
       within('.new_protected_branch') do
         allowed_to_push_button = find(".js-allowed-to-push")
@@ -18,7 +18,7 @@ RSpec.shared_examples "protected branches > access control > CE" do
     end
 
     it "allows updating protected branches so that #{access_type_name} can push to them" do
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('master')
       click_on "Protect"
 
@@ -36,7 +36,7 @@ RSpec.shared_examples "protected branches > access control > CE" do
 
   ProtectedBranch::MergeAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
     it "allows creating protected branches that #{access_type_name} can merge to" do
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('master')
       within('.new_protected_branch') do
         allowed_to_merge_button = find(".js-allowed-to-merge")
@@ -53,7 +53,7 @@ RSpec.shared_examples "protected branches > access control > CE" do
     end
 
     it "allows updating protected branches so that #{access_type_name} can merge to them" do
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('master')
       click_on "Protect"
 
diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb
index 1a3f7b970f6..8b32f0d0d59 100644
--- a/spec/features/protected_branches_spec.rb
+++ b/spec/features/protected_branches_spec.rb
@@ -17,7 +17,7 @@ feature 'Projected Branches', feature: true, js: true do
 
   describe "explicit protected branches" do
     it "allows creating explicit protected branches" do
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('some-branch')
       click_on "Protect"
 
@@ -30,7 +30,7 @@ feature 'Projected Branches', feature: true, js: true do
       commit = create(:commit, project: project)
       project.repository.add_branch(user, 'some-branch', commit.id)
 
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('some-branch')
       click_on "Protect"
 
@@ -38,7 +38,7 @@ feature 'Projected Branches', feature: true, js: true do
     end
 
     it "displays an error message if the named branch does not exist" do
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('some-branch')
       click_on "Protect"
 
@@ -48,7 +48,7 @@ feature 'Projected Branches', feature: true, js: true do
 
   describe "wildcard protected branches" do
     it "allows creating protected branches with a wildcard" do
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('*-stable')
       click_on "Protect"
 
@@ -61,7 +61,7 @@ feature 'Projected Branches', feature: true, js: true do
       project.repository.add_branch(user, 'production-stable', 'master')
       project.repository.add_branch(user, 'staging-stable', 'master')
 
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('*-stable')
       click_on "Protect"
 
@@ -73,11 +73,11 @@ feature 'Projected Branches', feature: true, js: true do
       project.repository.add_branch(user, 'staging-stable', 'master')
       project.repository.add_branch(user, 'development', 'master')
 
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       set_protected_branch_name('*-stable')
       click_on "Protect"
 
-      visit namespace_project_protected_branches_path(project.namespace, project)
+      visit namespace_project_deploy_keys_path(project.namespace, project)
       click_on "2 matching branches"
 
       within(".protected-branches-list") do