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

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

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
 

app/controllers/projects/settings/integrations_controller.rb

+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

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

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

app/views/projects/services/index.html.haml → app/views/projects/services/_index.html.haml

-- page_title "Services"
-
 .row.prepend-top-default.append-bottom-default
   .col-lg-3
     %h4.prepend-top-0

app/views/projects/settings/integrations/show.html.haml

+- page_title 'Integrations'
+= render 'projects/hooks/index'
+= render 'projects/services/index'

app/views/shared/web_hooks/_form.html.haml

-- page_title "Webhooks"
-- context_title = @project ? 'project' : 'group'
-
 .row.prepend-top-default
   .col-lg-3
     %h4.prepend-top-0

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

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
 

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

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

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

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

spec/controllers/projects/settings/integrations_controller_spec.rb

+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

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) }

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) }

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) }