Merge branch '26138-combine-webhooks-and-services-settings-pages' into 'master'

Moved the webhooks and services gear options to a single one called integrations

See merge request !8380

20 files changed, 71 insertions, 45 deletions

diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 0ae8ff98009..b668a9331e7 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -6,21 +6,15 @@ class Projects::HooksController < Projects::ApplicationController
 
   layout "project_settings"
 
-  def index
-    @hooks = @project.hooks
-    @hook = ProjectHook.new
-  end
-
   def create
     @hook = @project.hooks.new(hook_params)
     @hook.save
 
-    if @hook.valid?
-      redirect_to namespace_project_hooks_path(@project.namespace, @project)
-    else
+    unless @hook.valid?      
       @hooks = @project.hooks.select(&:persisted?)
-      render :index
+      flash[:alert] = @hook.errors.full_messages.join.html_safe
     end
+    redirect_to namespace_project_settings_integrations_path(@project.namespace, @project)
   end
 
   def test
@@ -44,7 +38,7 @@ class Projects::HooksController < Projects::ApplicationController
   def destroy
     hook.destroy
 
-    redirect_to namespace_project_hooks_path(@project.namespace, @project)
+    redirect_to namespace_project_settings_integrations_path(@project.namespace, @project)
   end
 
   private
diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index 30c2a5d9982..17cb1d5be24 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -9,10 +9,6 @@ class Projects::ServicesController < Projects::ApplicationController
 
   layout "project_settings"
 
-  def index
-    @services = @project.find_or_initialize_services
-  end
-
   def edit
   end
 
diff --git a/app/controllers/projects/settings/integrations_controller.rb b/app/controllers/projects/settings/integrations_controller.rb
new file mode 100644
index 00000000000..fb2a4837735
--- /dev/null
+++ b/app/controllers/projects/settings/integrations_controller.rb
@@ -0,0 +1,18 @@
+module Projects
+  module Settings
+    class IntegrationsController < Projects::ApplicationController
+      include ServiceParams
+
+      before_action :authorize_admin_project!
+      layout "project_settings"
+      
+      def show
+        @hooks = @project.hooks
+        @hook = ProjectHook.new
+
+        # Services
+        @services = @project.find_or_initialize_services
+      end
+    end
+  end
+end
diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb
index 5742fec4458..2159e4ce21a 100644
--- a/app/helpers/gitlab_routing_helper.rb
+++ b/app/helpers/gitlab_routing_helper.rb
@@ -208,6 +208,10 @@ module GitlabRoutingHelper
   end
 
   # Settings
+  def project_settings_integrations_path(project, *args)
+    namespace_project_settings_integrations_path(project.namespace, project, *args)
+  end
+  
   def project_settings_members_path(project, *args)
     namespace_project_settings_members_path(project.namespace, project, *args)
   end
diff --git a/app/views/layouts/nav/_project_settings.html.haml b/app/views/layouts/nav/_project_settings.html.haml
index 0fb2bb460cb..c6df66d2c3c 100644
--- a/app/views/layouts/nav/_project_settings.html.haml
+++ b/app/views/layouts/nav/_project_settings.html.haml
@@ -8,14 +8,10 @@
     = link_to namespace_project_deploy_keys_path(@project.namespace, @project), title: 'Deploy Keys' do
       %span
         Deploy Keys
-  = nav_link(controller: :hooks) do
-    = link_to namespace_project_hooks_path(@project.namespace, @project), title: 'Webhooks' do
+  = nav_link(controller: :integrations) do
+    = link_to namespace_project_settings_integrations_path(@project.namespace, @project), title: 'Integrations' do
       %span
-        Webhooks
-  = nav_link(controller: :services) do
-    = link_to namespace_project_services_path(@project.namespace, @project), title: 'Services' do
-      %span
-        Services
+        Integrations
   = nav_link(controller: :protected_branches) do
     = link_to namespace_project_protected_branches_path(@project.namespace, @project), title: 'Protected Branches' do
       %span
diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/_index.html.haml
index 8faad351463..8faad351463 100644
--- a/app/views/projects/hooks/index.html.haml
+++ b/app/views/projects/hooks/_index.html.haml
diff --git a/app/views/projects/services/index.html.haml b/app/views/projects/services/_index.html.haml
index 66fd3029dc9..964133504e6 100644
--- a/app/views/projects/services/index.html.haml
+++ b/app/views/projects/services/_index.html.haml
@@ -1,5 +1,3 @@
-- page_title "Services"
-
 .row.prepend-top-default.append-bottom-default
   .col-lg-3
     %h4.prepend-top-0
diff --git a/app/views/projects/hooks/_project_hook.html.haml b/app/views/projects/settings/integrations/_project_hook.html.haml
index ceabe2eab3d..ceabe2eab3d 100644
--- a/app/views/projects/hooks/_project_hook.html.haml
+++ b/app/views/projects/settings/integrations/_project_hook.html.haml
diff --git a/app/views/projects/settings/integrations/show.html.haml b/app/views/projects/settings/integrations/show.html.haml
new file mode 100644
index 00000000000..aa38a889cdd
--- /dev/null
+++ b/app/views/projects/settings/integrations/show.html.haml
@@ -0,0 +1,3 @@
+- page_title 'Integrations'
+= render 'projects/hooks/index'
+= render 'projects/services/index'
diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml
index 5d659eb83a9..13586a5a12a 100644
--- a/app/views/shared/web_hooks/_form.html.haml
+++ b/app/views/shared/web_hooks/_form.html.haml
@@ -1,6 +1,3 @@
-- page_title "Webhooks"
-- context_title = @project ? 'project' : 'group'
-
 .row.prepend-top-default
   .col-lg-3
     %h4.prepend-top-0
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 1fc6ed28c74..6620b765e02 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -307,9 +307,9 @@ constraints(ProjectUrlConstrainer.new) do
           end
         end
       end
-
       namespace :settings do
         resource :members, only: [:show]
+        resource :integrations, only: [:show]
       end
 
       # Since both wiki and repository routing contains wildcard characters
diff --git a/features/project/active_tab.feature b/features/project/active_tab.feature
index 57dda9c2234..d033e6b167b 100644
--- a/features/project/active_tab.feature
+++ b/features/project/active_tab.feature
@@ -39,10 +39,10 @@ Feature: Project Active Tab
 
   # Sub Tabs: Settings
 
-  Scenario: On Project Settings/Hooks
+  Scenario: On Project Settings/Integrations
     Given I visit my project's settings page
-    And I click the "Hooks" tab
-    Then the active sub nav should be Hooks
+    And I click the "Integrations" tab
+    Then the active sub nav should be Integrations
     And no other sub navs should be active
     And the active main tab should be Settings
 
diff --git a/features/steps/project/active_tab.rb b/features/steps/project/active_tab.rb
index 58225032859..9f701840f1d 100644
--- a/features/steps/project/active_tab.rb
+++ b/features/steps/project/active_tab.rb
@@ -27,8 +27,8 @@ 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
+    click_link('Integrations')
   end
 
   step 'I click the "Deploy Keys" tab' do
@@ -39,8 +39,8 @@ class Spinach::Features::ProjectActiveTab < Spinach::FeatureSteps
     ensure_active_sub_nav('Members')
   end
 
-  step 'the active sub nav should be Hooks' do
-    ensure_active_sub_nav('Webhooks')
+  step 'the active sub nav should be Integrations' do
+    ensure_active_sub_nav('Integrations')
   end
 
   step 'the active sub nav should be Deploy Keys' do
diff --git a/features/steps/project/hooks.rb b/features/steps/project/hooks.rb
index 13c0713669a..37b608ffbd3 100644
--- a/features/steps/project/hooks.rb
+++ b/features/steps/project/hooks.rb
@@ -36,12 +36,12 @@ class Spinach::Features::ProjectHooks < Spinach::FeatureSteps
   end
 
   step 'I should see newly created hook' do
-    expect(current_path).to eq namespace_project_hooks_path(current_project.namespace, current_project)
+    expect(current_path).to eq namespace_project_settings_integrations_path(current_project.namespace, current_project)
     expect(page).to have_content(@url)
   end
 
   step 'I should see newly created hook with SSL verification enabled' do
-    expect(current_path).to eq namespace_project_hooks_path(current_project.namespace, current_project)
+    expect(current_path).to eq namespace_project_settings_integrations_path(current_project.namespace, current_project)
     expect(page).to have_content(@url)
     expect(page).to have_content("SSL Verification: enabled")
   end
@@ -57,7 +57,7 @@ class Spinach::Features::ProjectHooks < Spinach::FeatureSteps
   end
 
   step 'hook should be triggered' do
-    expect(current_path).to eq namespace_project_hooks_path(current_project.namespace, current_project)
+    expect(current_path).to eq namespace_project_settings_integrations_path(current_project.namespace, current_project)
     expect(page).to have_selector '.flash-notice',
                               text: 'Hook executed successfully: HTTP 200'
   end
diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb
index a4d29770922..772b07d0ad8 100644
--- a/features/steps/project/services.rb
+++ b/features/steps/project/services.rb
@@ -4,7 +4,7 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
   include SharedPaths
 
   step 'I visit project "Shop" services page' do
-    visit namespace_project_services_path(@project.namespace, @project)
+    visit namespace_project_settings_integrations_path(@project.namespace, @project)
   end
 
   step 'I should see list of available services' do
diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb
index 670e6ca49a3..718cf924729 100644
--- a/features/steps/shared/paths.rb
+++ b/features/steps/shared/paths.rb
@@ -256,7 +256,7 @@ module SharedPaths
   end
 
   step 'I visit project hooks page' do
-    visit namespace_project_hooks_path(@project.namespace, @project)
+    visit namespace_project_settings_integrations_path(@project.namespace, @project)
   end
 
   step 'I visit project deploy keys page' do
diff --git a/spec/controllers/projects/settings/integrations_controller_spec.rb b/spec/controllers/projects/settings/integrations_controller_spec.rb
new file mode 100644
index 00000000000..e0f9a5b24a6
--- /dev/null
+++ b/spec/controllers/projects/settings/integrations_controller_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Projects::Settings::IntegrationsController do
+  let(:project) { create(: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
diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb
index ecebabefff8..92d5a2fbc48 100644
--- a/spec/features/security/project/internal_access_spec.rb
+++ b/spec/features/security/project/internal_access_spec.rb
@@ -262,8 +262,8 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_denied_for(:visitor) }
   end
 
-  describe "GET /:project_path/hooks" do
-    subject { namespace_project_hooks_path(project.namespace, project) }
+  describe "GET /:project_path/settings/integrations" do
+    subject { namespace_project_settings_integrations_path(project.namespace, project) }
 
     it { is_expected.to be_allowed_for(:admin) }
     it { is_expected.to be_allowed_for(:owner).of(project) }
diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb
index 9bc59a7c4f9..b616e488487 100644
--- a/spec/features/security/project/private_access_spec.rb
+++ b/spec/features/security/project/private_access_spec.rb
@@ -234,8 +234,8 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for(:visitor) }
   end
 
-  describe "GET /:project_path/hooks" do
-    subject { namespace_project_hooks_path(project.namespace, project) }
+  describe "GET /:project_path/namespace/hooks" do
+    subject { namespace_project_settings_integrations_path(project.namespace, project) }
 
     it { is_expected.to be_allowed_for(:admin) }
     it { is_expected.to be_allowed_for(:owner).of(project) }
diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb
index a8d43b3d581..ded85e837f4 100644
--- a/spec/features/security/project/public_access_spec.rb
+++ b/spec/features/security/project/public_access_spec.rb
@@ -400,8 +400,8 @@ describe "Public Project Access", feature: true  do
     it { is_expected.to be_allowed_for(:visitor) }
   end
 
-  describe "GET /:project_path/hooks" do
-    subject { namespace_project_hooks_path(project.namespace, project) }
+  describe "GET /:project_path/settings/integrations" do
+    subject { namespace_project_settings_integrations_path(project.namespace, project) }
 
     it { is_expected.to be_allowed_for(:admin) }
     it { is_expected.to be_allowed_for(:owner).of(project) }