Port of 26138-combine-webhooks-and-services-settings-pages to EE

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

config/routes/project.rb

@@ -361,6 +361,7 @@ constraints(ProjectUrlConstrainer.new) do
 
       namespace :settings do
         resource :members, only: [:show]
+        resource :integrations, only: [:show]
       end
 
       # Since both wiki and repository routing contains wildcard characters

config/routes/project.rb.orig

+require 'constraints/project_url_constrainer'
+
+resources :projects, only: [:index, :new, :create]
+
+draw :git_http
+
+constraints(ProjectUrlConstrainer.new) do
+  scope(path: '*namespace_id', as: :namespace) do
+    scope(path: ':project_id',
+          constraints: { project_id: Gitlab::Regex.project_route_regex },
+          module: :projects,
+          as: :project) do
+
+      resources :autocomplete_sources, only: [] do
+        collection do
+          get 'emojis'
+          get 'members'
+          get 'issues'
+          get 'merge_requests'
+          get 'labels'
+          get 'milestones'
+          get 'commands'
+        end
+      end
+
+      #
+      # Templates
+      #
+      get '/templates/:template_type/:key' => 'templates#show', as: :template
+
+      resource  :avatar, only: [:show, :destroy]
+      resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
+        member do
+          get :branches
+          get :pipelines
+          post :revert
+          post :cherry_pick
+          get :diff_for_path
+        end
+      end
+
+      ## EE-specific
+      resource :pages, only: [:show, :destroy] do
+        resources :domains, only: [:show, :new, :create, :destroy], controller: 'pages_domains', constraints: { id: /[^\/]+/ }
+      end
+      ## EE-specific
+
+      resources :compare, only: [:index, :create] do
+        collection do
+          get :diff_for_path
+        end
+      end
+
+      get '/compare/:from...:to', to: 'compare#show', as: 'compare', constraints: { from: /.+/, to: /.+/ }
+
+      # Don't use format parameter as file extension (old 3.0.x behavior)
+      # See http://guides.rubyonrails.org/routing.html#route-globbing-and-wildcard-segments
+      scope format: false do
+        resources :network, only: [:show], constraints: { id: Gitlab::Regex.git_reference_regex }
+
+        resources :graphs, only: [:show], constraints: { id: Gitlab::Regex.git_reference_regex } do
+          member do
+            get :commits
+            get :ci
+            get :languages
+          end
+        end
+      end
+
+      resources :snippets, concerns: :awardable, constraints: { id: /\d+/ } do
+        member do
+          get 'raw'
+        end
+      end
+
+      resources :services, constraints: { id: /[^\/]+/ }, only: [:index, :edit, :update] do
+        member do
+          get :test
+        end
+      end
+
+      resource :mattermost, only: [:new, :create]
+
+      resources :deploy_keys, constraints: { id: /\d+/ }, only: [:index, :new, :create] do
+        member do
+          put :enable
+          put :disable
+        end
+      end
+
+      resources :forks, only: [:index, :new, :create]
+      resource :import, only: [:new, :create, :show]
+
+      resources :merge_requests, concerns: :awardable, constraints: { id: /\d+/ } do
+        member do
+          get :commits
+          get :diffs
+          get :conflicts
+          get :conflict_for_path
+          get :pipelines
+          get :merge_check
+          post :merge
+          post :cancel_merge_when_build_succeeds
+          get :ci_status
+          get :ci_environments_status
+          post :toggle_subscription
+
+          ## EE-specific
+          get :approvals
+          post :approvals, action: :approve
+          delete :approvals, action: :unapprove
+
+          post :rebase
+          ## EE-specific
+
+          post :remove_wip
+          get :diff_for_path
+          post :resolve_conflicts
+          post :assign_related_issues
+        end
+
+        collection do
+          get :branch_from
+          get :branch_to
+          get :update_branches
+          get :diff_for_path
+          post :bulk_update
+          get :new_diffs, path: 'new/diffs'
+        end
+
+        ## EE-specific
+        resources :approvers, only: :destroy
+        resources :approver_groups, only: :destroy
+        ## EE-specific
+
+        resources :discussions, only: [], constraints: { id: /\h{40}/ } do
+          member do
+            post :resolve
+            delete :resolve, action: :unresolve
+          end
+        end
+      end
+
+      resources :branches, only: [:index, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
+      delete :merged_branches, controller: 'branches', action: :destroy_all_merged
+      resources :tags, only: [:index, :show, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } do
+        resource :release, only: [:edit, :update]
+      end
+
+      ## EE-specific
+      resources :path_locks, only: [:index, :destroy] do
+        collection do
+          post :toggle
+        end
+      end
+
+      resources :protected_branches, only: [:index, :show, :create, :update, :destroy, :patch], constraints: { id: Gitlab::Regex.git_reference_regex } do
+        scope module: :protected_branches do
+          resources :merge_access_levels, only: [:destroy]
+          resources :push_access_levels, only: [:destroy]
+        end
+      end
+      ## EE-specific
+
+      resources :variables, only: [:index, :show, :update, :create, :destroy]
+      resources :triggers, only: [:index, :create, :destroy]
+
+      ## EE-specific
+      resource :mirror, only: [:show, :update] do
+        member do
+          post :update_now
+        end
+      end
+      resources :push_rules, constraints: { id: /\d+/ }
+      ## EE-specific
+
+      resources :pipelines, only: [:index, :new, :create, :show] do
+        collection do
+          resource :pipelines_settings, path: 'settings', only: [:show, :update]
+        end
+
+        member do
+          get :stage
+          post :cancel
+          post :retry
+          get :builds
+        end
+      end
+
+      resources :environments, except: [:destroy] do
+        member do
+          post :stop
+          get :terminal
+          get '/terminal.ws/authorize', to: 'environments#terminal_websocket_authorize', constraints: { format: nil }
+        end
+      end
+
+      resource :cycle_analytics, only: [:show]
+
+      namespace :cycle_analytics do
+        scope :events, controller: 'events' do
+          get :issue
+          get :plan
+          get :code
+          get :test
+          get :review
+          get :staging
+          get :production
+        end
+      end
+
+      resources :builds, only: [:index, :show], constraints: { id: /\d+/ } do
+        collection do
+          post :cancel_all
+
+          resources :artifacts, only: [] do
+            collection do
+              get :latest_succeeded,
+                path: '*ref_name_and_path',
+                format: false
+            end
+          end
+        end
+
+        member do
+          get :status
+          post :cancel
+          post :retry
+          post :play
+          post :erase
+          get :trace
+          get :raw
+        end
+
+        resource :artifacts, only: [] do
+          get :download
+          get :browse, path: 'browse(/*path)', format: false
+          get :file, path: 'file/*path', format: false
+          post :keep
+        end
+      end
+
+      resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ } do
+        member do
+          get :test
+        end
+      end
+
+      resources :container_registry, only: [:index, :destroy], constraints: { id: Gitlab::Regex.container_registry_reference_regex }
+
+      resources :milestones, constraints: { id: /\d+/ } do
+        member do
+          put :sort_issues
+          put :sort_merge_requests
+        end
+      end
+
+      resources :labels, except: [:show], constraints: { id: /\d+/ } do
+        collection do
+          post :generate
+          post :set_priorities
+        end
+
+        member do
+          post :toggle_subscription
+          delete :remove_priority
+        end
+      end
+
+      resources :issues, concerns: :awardable, constraints: { id: /\d+/ } do
+        member do
+          post :toggle_subscription
+          post :mark_as_spam
+          get :referenced_merge_requests
+          get :related_branches
+          get :can_create_branch
+        end
+        collection do
+          post  :bulk_update
+        end
+      end
+
+      resources :project_members, except: [:show, :new, :edit], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ }, concerns: :access_requestable do
+        collection do
+          delete :leave
+
+          # Used for import team
+          # from another project
+          get :import
+          post :apply_import
+        end
+
+        member do
+          post :resend_invite
+        end
+      end
+
+      resources :group_links, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ }
+
+      resources :notes, only: [:index, :create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } do
+        member do
+          delete :delete_attachment
+          post :resolve
+          delete :resolve, action: :unresolve
+        end
+      end
+
+      resources :boards, only: [:index, :show, :create, :update, :destroy] do
+        scope module: :boards do
+          resources :issues, only: [:update]
+
+          resources :lists, only: [:index, :create, :update, :destroy] do
+            collection do
+              post :generate
+            end
+
+            resources :issues, only: [:index, :create]
+          end
+        end
+      end
+
+      resources :todos, only: [:create]
+
+      resources :uploads, only: [:create] do
+        collection do
+          get ":secret/:filename", action: :show, as: :show, constraints: { filename: /[^\/]+/ }
+        end
+      end
+
+      resources :runners, only: [:index, :edit, :update, :destroy, :show] do
+        member do
+          get :resume
+          get :pause
+        end
+
+        collection do
+          post :toggle_shared_runners
+        end
+      end
+
+      ## EE-specific
+      resources :approvers, only: :destroy
+      resources :approver_groups, only: :destroy
+      ## EE-specific
+
+      resources :runner_projects, only: [:create, :destroy]
+      resources :badges, only: [:index] do
+        collection do
+          scope '*ref', constraints: { ref: Gitlab::Regex.git_reference_regex } do
+            constraints format: /svg/ do
+              get :build
+              get :coverage
+            end
+          end
+        end
+      end
+<<<<<<< HEAD
+
+      ## EE-specific
+      resources :audit_events, only: [:index]
+      ## EE-specific
+
+=======
+>>>>>>> 9f0d794513... Moved the webhooks and services gear options to a single one called integrations
+      namespace :settings do
+        resource :members, only: [:show]
+        resource :integrations, only: [:show]
+      end
+
+      # Since both wiki and repository routing contains wildcard characters
+      # its preferable to keep it below all other project routes
+      draw :wiki
+      draw :repository
+    end
+
+    resources(:projects,
+              path: '/',
+              constraints: { id: Gitlab::Regex.project_route_regex },
+              only: [:edit, :show, :update, :destroy]) do
+      member do
+        put :transfer
+        delete :remove_fork
+        post :archive
+        post :unarchive
+        post :housekeeping
+        post :toggle_star
+        post :preview_markdown
+        post :export
+        post :remove_export
+        post :generate_new_export
+        get :download_export
+        get :activity
+        get :refs
+        put :new_issue_address
+      end
+    end
+  end
+end

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
@@ -43,8 +43,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

@@ -276,7 +276,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 group hooks 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) }