From c504b88f07f03d381e7b2e22a129413319508413 Mon Sep 17 00:00:00 2001
From: Alexander Randa <randa.alex@gmail.com>
Date: Thu, 20 Apr 2017 08:31:37 +0000
Subject: [PATCH] Implement ability to update hooks

---
 app/controllers/admin/hooks_controller.rb     |  26 ++-
 app/controllers/projects/hooks_controller.rb  |  13 ++
 app/views/admin/hooks/_form.html.haml         |  40 ++++
 app/views/admin/hooks/edit.html.haml          |  14 ++
 app/views/admin/hooks/index.html.haml         |  55 +-----
 app/views/projects/hooks/_index.html.haml     |  24 ++-
 app/views/projects/hooks/edit.html.haml       |  14 ++
 app/views/projects/settings/_head.html.haml   |   2 +-
 .../integrations/_project_hook.html.haml      |   1 +
 app/views/shared/web_hooks/_form.html.haml    | 182 ++++++++----------
 changelogs/unreleased/19364-webhook-edit.yml  |   4 +
 config/routes/admin.rb                        |   6 +-
 config/routes/project.rb                      |   2 +-
 spec/factories/project_hooks.rb               |   2 +
 spec/features/admin/admin_hooks_spec.rb       |  43 ++++-
 .../settings/integration_settings_spec.rb     |  94 +++++++++
 spec/routing/admin_routing_spec.rb            |  14 +-
 spec/routing/project_routing_spec.rb          |   6 +-
 18 files changed, 372 insertions(+), 170 deletions(-)
 create mode 100644 app/views/admin/hooks/_form.html.haml
 create mode 100644 app/views/admin/hooks/edit.html.haml
 create mode 100644 app/views/projects/hooks/edit.html.haml
 create mode 100644 changelogs/unreleased/19364-webhook-edit.yml
 create mode 100644 spec/features/projects/settings/integration_settings_spec.rb

diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb
index cbfc4581411..a119934febc 100644
--- a/app/controllers/admin/hooks_controller.rb
+++ b/app/controllers/admin/hooks_controller.rb
@@ -1,4 +1,6 @@
 class Admin::HooksController < Admin::ApplicationController
+  before_action :hook, only: :edit
+
   def index
     @hooks = SystemHook.all
     @hook = SystemHook.new
@@ -15,15 +17,25 @@ class Admin::HooksController < Admin::ApplicationController
     end
   end
 
+  def edit
+  end
+
+  def update
+    if hook.update_attributes(hook_params)
+      flash[:notice] = 'System hook was successfully updated.'
+      redirect_to admin_hooks_path
+    else
+      render 'edit'
+    end
+  end
+
   def destroy
-    @hook = SystemHook.find(params[:id])
-    @hook.destroy
+    hook.destroy
 
     redirect_to admin_hooks_path
   end
 
   def test
-    @hook = SystemHook.find(params[:hook_id])
     data = {
       event_name: "project_create",
       name: "Ruby",
@@ -32,11 +44,17 @@ class Admin::HooksController < Admin::ApplicationController
       owner_name: "Someone",
       owner_email: "example@gitlabhq.com"
     }
-    @hook.execute(data, 'system_hooks')
+    hook.execute(data, 'system_hooks')
 
     redirect_back_or_default
   end
 
+  private
+
+  def hook
+    @hook ||= SystemHook.find(params[:id])
+  end
+
   def hook_params
     params.require(:hook).permit(
       :enable_ssl_verification,
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 1e41f980f31..86d13a0d222 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -1,6 +1,7 @@
 class Projects::HooksController < Projects::ApplicationController
   # Authorize
   before_action :authorize_admin_project!
+  before_action :hook, only: :edit
 
   respond_to :html
 
@@ -17,6 +18,18 @@ class Projects::HooksController < Projects::ApplicationController
     redirect_to namespace_project_settings_integrations_path(@project.namespace, @project)
   end
 
+  def edit
+  end
+
+  def update
+    if hook.update_attributes(hook_params)
+      flash[:notice] = 'Hook was successfully updated.'
+      redirect_to namespace_project_settings_integrations_path(@project.namespace, @project)
+    else
+      render 'edit'
+    end
+  end
+
   def test
     if !@project.empty_repo?
       status, message = TestHookService.new.execute(hook, current_user)
diff --git a/app/views/admin/hooks/_form.html.haml b/app/views/admin/hooks/_form.html.haml
new file mode 100644
index 00000000000..6217d5fb135
--- /dev/null
+++ b/app/views/admin/hooks/_form.html.haml
@@ -0,0 +1,40 @@
+= form_errors(hook)
+
+.form-group
+  = form.label :url, 'URL', class: 'control-label'
+  .col-sm-10
+    = form.text_field :url, class: 'form-control'
+.form-group
+  = form.label :token, 'Secret Token', class: 'control-label'
+  .col-sm-10
+    = form.text_field :token, class: 'form-control'
+    %p.help-block
+      Use this token to validate received payloads
+.form-group
+  = form.label :url, 'Trigger', class: 'control-label'
+  .col-sm-10.prepend-top-10
+    %div
+      System hook will be triggered on set of events like creating project
+      or adding ssh key. But you can also enable extra triggers like Push events.
+
+    .prepend-top-default
+      = form.check_box :push_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :push_events, class: 'list-label' do
+          %strong Push events
+        %p.light
+          This url will be triggered by a push to the repository
+    %div
+      = form.check_box :tag_push_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :tag_push_events, class: 'list-label' do
+          %strong Tag push events
+        %p.light
+          This url will be triggered when a new tag is pushed to the repository
+.form-group
+  = form.label :enable_ssl_verification, 'SSL verification', class: 'control-label checkbox'
+  .col-sm-10
+    .checkbox
+      = form.label :enable_ssl_verification do
+        = form.check_box :enable_ssl_verification
+        %strong Enable SSL verification
diff --git a/app/views/admin/hooks/edit.html.haml b/app/views/admin/hooks/edit.html.haml
new file mode 100644
index 00000000000..0777f5e2629
--- /dev/null
+++ b/app/views/admin/hooks/edit.html.haml
@@ -0,0 +1,14 @@
+- page_title 'Edit System Hook'
+%h3.page-title
+  Edit System Hook
+
+%p.light
+  #{link_to 'System hooks ', help_page_path('system_hooks/system_hooks'), class: 'vlink'} can be
+  used for binding events when GitLab creates a User or Project.
+
+%hr
+
+= form_for @hook, as: :hook, url: admin_hook_path, html: { class: 'form-horizontal' } do |f|
+  = render partial: 'form', locals: { form: f, hook: @hook }
+  .form-actions
+    = f.submit 'Save changes', class: 'btn btn-create'
diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml
index d9c7948763a..71117758921 100644
--- a/app/views/admin/hooks/index.html.haml
+++ b/app/views/admin/hooks/index.html.haml
@@ -1,57 +1,17 @@
-- page_title "System Hooks"
+- page_title 'System Hooks'
 %h3.page-title
   System hooks
 
 %p.light
-  #{link_to "System hooks ", help_page_path("system_hooks/system_hooks"), class: "vlink"} can be
+  #{link_to 'System hooks ', help_page_path('system_hooks/system_hooks'), class: 'vlink'} can be
   used for binding events when GitLab creates a User or Project.
 
 %hr
 
-
 = form_for @hook, as: :hook, url: admin_hooks_path, html: { class: 'form-horizontal' } do |f|
-  = form_errors(@hook)
-
-  .form-group
-    = f.label :url, 'URL', class: 'control-label'
-    .col-sm-10
-      = f.text_field :url, class: 'form-control'
-  .form-group
-    = f.label :token, 'Secret Token', class: 'control-label'
-    .col-sm-10
-      = f.text_field :token, class: 'form-control'
-      %p.help-block
-        Use this token to validate received payloads
-  .form-group
-    = f.label :url, "Trigger", class: 'control-label'
-    .col-sm-10.prepend-top-10
-      %div
-        System hook will be triggered on set of events like creating project
-        or adding ssh key. But you can also enable extra triggers like Push events.
-
-      .prepend-top-default
-        = f.check_box :push_events, class: 'pull-left'
-        .prepend-left-20
-          = f.label :push_events, class: 'list-label' do
-            %strong Push events
-          %p.light
-            This url will be triggered by a push to the repository
-      %div
-        = f.check_box :tag_push_events, class: 'pull-left'
-        .prepend-left-20
-          = f.label :tag_push_events, class: 'list-label' do
-            %strong Tag push events
-          %p.light
-            This url will be triggered when a new tag is pushed to the repository
-  .form-group
-    = f.label :enable_ssl_verification, "SSL verification", class: 'control-label checkbox'
-    .col-sm-10
-      .checkbox
-        = f.label :enable_ssl_verification do
-          = f.check_box :enable_ssl_verification
-          %strong Enable SSL verification
+  = render partial: 'form', locals: { form: f, hook: @hook }
   .form-actions
-    = f.submit "Add system hook", class: "btn btn-create"
+    = f.submit 'Add system hook', class: 'btn btn-create'
 %hr
 
 - if @hooks.any?
@@ -62,11 +22,12 @@
       - @hooks.each do |hook|
         %li
           .controls
-            = link_to 'Test hook', admin_hook_test_path(hook), class: "btn btn-sm"
-            = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-remove btn-sm"
+            = link_to 'Test hook', test_admin_hook_path(hook), class: 'btn btn-sm'
+            = link_to 'Edit', edit_admin_hook_path(hook), class: 'btn btn-sm'
+            = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm'
           .monospace= hook.url
           %div
             - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger|
               - if hook.send(trigger)
                 %span.label.label-gray= trigger.titleize
-            %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? "enabled" : "disabled"}
+            %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'}
diff --git a/app/views/projects/hooks/_index.html.haml b/app/views/projects/hooks/_index.html.haml
index 8faad351463..676b7c345bc 100644
--- a/app/views/projects/hooks/_index.html.haml
+++ b/app/views/projects/hooks/_index.html.haml
@@ -1 +1,23 @@
-= render 'shared/web_hooks/form', hook: @hook, hooks: @hooks, url_components: [@project.namespace.becomes(Namespace), @project]
+.row.prepend-top-default
+  .col-lg-3
+    %h4.prepend-top-0
+      = page_title
+    %p
+      #{link_to 'Webhooks', help_page_path('user/project/integrations/webhooks')} can be
+      used for binding events when something is happening within the project.
+
+  .col-lg-9.append-bottom-default
+    = form_for @hook, as: :hook, url: polymorphic_path([@project.namespace.becomes(Namespace), @project, :hooks]) do |f|
+      = render partial: 'shared/web_hooks/form', locals: { form: f, hook: @hook }
+      = f.submit 'Add webhook', class: 'btn btn-create'
+
+    %hr
+    %h5.prepend-top-default
+      Webhooks (#{@hooks.count})
+    - if @hooks.any?
+      %ul.well-list
+        - @hooks.each do |hook|
+          = render 'project_hook', hook: hook
+    - else
+      %p.settings-message.text-center.append-bottom-0
+        No webhooks found, add one in the form above.
diff --git a/app/views/projects/hooks/edit.html.haml b/app/views/projects/hooks/edit.html.haml
new file mode 100644
index 00000000000..7998713be1f
--- /dev/null
+++ b/app/views/projects/hooks/edit.html.haml
@@ -0,0 +1,14 @@
+= render 'projects/settings/head'
+
+.row.prepend-top-default
+  .col-lg-3
+    %h4.prepend-top-0
+      = page_title
+    %p
+      #{link_to 'Webhooks', help_page_path('user/project/integrations/webhooks')} can be
+      used for binding events when something is happening within the project.
+  .col-lg-9.append-bottom-default
+    = form_for [@project.namespace.becomes(Namespace), @project, @hook], as: :hook, url: namespace_project_hook_path do |f|
+      = render partial: 'shared/web_hooks/form', locals: { form: f, hook: @hook }
+      = f.submit 'Save changes', class: 'btn btn-create'
+
diff --git a/app/views/projects/settings/_head.html.haml b/app/views/projects/settings/_head.html.haml
index e50a543ffa8..5a5ade03624 100644
--- a/app/views/projects/settings/_head.html.haml
+++ b/app/views/projects/settings/_head.html.haml
@@ -14,7 +14,7 @@
             %span
               Members
         - if can_edit
-          = nav_link(controller: [:integrations, :services]) do
+          = nav_link(controller: [:integrations, :services, :hooks]) do
             = link_to project_settings_integrations_path(@project), title: 'Integrations' do
               %span
                 Integrations
diff --git a/app/views/projects/settings/integrations/_project_hook.html.haml b/app/views/projects/settings/integrations/_project_hook.html.haml
index ceabe2eab3d..8dc276a3bec 100644
--- a/app/views/projects/settings/integrations/_project_hook.html.haml
+++ b/app/views/projects/settings/integrations/_project_hook.html.haml
@@ -9,6 +9,7 @@
     .col-md-4.col-lg-5.text-right-lg.prepend-top-5
       %span.append-right-10.inline
         SSL Verification: #{hook.enable_ssl_verification ? "enabled" : "disabled"}
+      = link_to "Edit", edit_namespace_project_hook_path(@project.namespace, @project, hook), class: "btn btn-sm"
       = link_to "Test", test_namespace_project_hook_path(@project.namespace, @project, hook), class: "btn btn-sm"
       = link_to namespace_project_hook_path(@project.namespace, @project, hook), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-transparent" do
         %span.sr-only Remove
diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml
index ee3be3c789a..37c3e61912c 100644
--- a/app/views/shared/web_hooks/_form.html.haml
+++ b/app/views/shared/web_hooks/_form.html.haml
@@ -1,102 +1,82 @@
-.row.prepend-top-default
-  .col-lg-3
-    %h4.prepend-top-0
-      = page_title
-    %p
-      #{link_to "Webhooks", help_page_path("user/project/integrations/webhooks")} can be
-      used for binding events when something is happening within the project.
-  .col-lg-9.append-bottom-default
-    = form_for hook, as: :hook, url: polymorphic_path(url_components + [:hooks]) do |f|
-      = form_errors(hook)
+= form_errors(hook)
 
-      .form-group
-        = f.label :url, "URL", class: 'label-light'
-        = f.text_field :url, class: "form-control", placeholder: 'http://example.com/trigger-ci.json'
-      .form-group
-        = f.label :token, "Secret Token", class: 'label-light'
-        = f.text_field :token, class: "form-control", placeholder: ''
-        %p.help-block
-          Use this token to validate received payloads. It will be sent with the request in the X-Gitlab-Token HTTP header.
-      .form-group
-        = f.label :url, "Trigger", class: 'label-light'
-        %ul.list-unstyled
-          %li
-            = f.check_box :push_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :push_events, class: 'list-label' do
-                %strong Push events
-              %p.light
-                This URL will be triggered by a push to the repository
-          %li
-            = f.check_box :tag_push_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :tag_push_events, class: 'list-label' do
-                %strong Tag push events
-              %p.light
-                This URL will be triggered when a new tag is pushed to the repository
-          %li
-            = f.check_box :note_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :note_events, class: 'list-label' do
-                %strong Comments
-              %p.light
-                This URL will be triggered when someone adds a comment
-          %li
-            = f.check_box :issues_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :issues_events, class: 'list-label' do
-                %strong Issues events
-              %p.light
-                This URL will be triggered when an issue is created/updated/merged
-          %li
-            = f.check_box :confidential_issues_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :confidential_issues_events, class: 'list-label' do
-                %strong Confidential Issues events
-              %p.light
-                This URL will be triggered when a confidential issue is created/updated/merged
-          %li
-            = f.check_box :merge_requests_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :merge_requests_events, class: 'list-label' do
-                %strong Merge Request events
-              %p.light
-                This URL will be triggered when a merge request is created/updated/merged
-          %li
-            = f.check_box :build_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :build_events, class: 'list-label' do
-                %strong Jobs events
-              %p.light
-                This URL will be triggered when the job status changes
-          %li
-            = f.check_box :pipeline_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :pipeline_events, class: 'list-label' do
-                %strong Pipeline events
-              %p.light
-                This URL will be triggered when the pipeline status changes
-          %li
-            = f.check_box :wiki_page_events, class: 'pull-left'
-            .prepend-left-20
-              = f.label :wiki_page_events, class: 'list-label' do
-                %strong Wiki Page events
-              %p.light
-                This URL will be triggered when a wiki page is created/updated
-      .form-group
-        = f.label :enable_ssl_verification, "SSL verification", class: 'label-light checkbox'
-        .checkbox
-          = f.label :enable_ssl_verification do
-            = f.check_box :enable_ssl_verification
-            %strong Enable SSL verification
-      = f.submit "Add webhook", class: "btn btn-create"
-    %hr
-    %h5.prepend-top-default
-      Webhooks (#{hooks.count})
-    - if hooks.any?
-      %ul.well-list
-        - hooks.each do |hook|
-          = render "project_hook", hook: hook
-    - else
-      %p.settings-message.text-center.append-bottom-0
-        No webhooks found, add one in the form above.
+.form-group
+  = form.label :url, 'URL', class: 'label-light'
+  = form.text_field :url, class: 'form-control', placeholder: 'http://example.com/trigger-ci.json'
+.form-group
+  = form.label :token, 'Secret Token', class: 'label-light'
+  = form.text_field :token, class: 'form-control', placeholder: ''
+  %p.help-block
+    Use this token to validate received payloads. It will be sent with the request in the X-Gitlab-Token HTTP header.
+.form-group
+  = form.label :url, 'Trigger', class: 'label-light'
+  %ul.list-unstyled
+    %li
+      = form.check_box :push_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :push_events, class: 'list-label' do
+          %strong Push events
+        %p.light
+          This URL will be triggered by a push to the repository
+    %li
+      = form.check_box :tag_push_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :tag_push_events, class: 'list-label' do
+          %strong Tag push events
+        %p.light
+          This URL will be triggered when a new tag is pushed to the repository
+    %li
+      = form.check_box :note_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :note_events, class: 'list-label' do
+          %strong Comments
+        %p.light
+          This URL will be triggered when someone adds a comment
+    %li
+      = form.check_box :issues_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :issues_events, class: 'list-label' do
+          %strong Issues events
+        %p.light
+          This URL will be triggered when an issue is created/updated/merged
+    %li
+      = form.check_box :confidential_issues_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :confidential_issues_events, class: 'list-label' do
+          %strong Confidential Issues events
+        %p.light
+          This URL will be triggered when a confidential issue is created/updated/merged
+    %li
+      = form.check_box :merge_requests_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :merge_requests_events, class: 'list-label' do
+          %strong Merge Request events
+        %p.light
+          This URL will be triggered when a merge request is created/updated/merged
+    %li
+      = form.check_box :build_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :build_events, class: 'list-label' do
+          %strong Jobs events
+        %p.light
+          This URL will be triggered when the job status changes
+    %li
+      = form.check_box :pipeline_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :pipeline_events, class: 'list-label' do
+          %strong Pipeline events
+        %p.light
+          This URL will be triggered when the pipeline status changes
+    %li
+      = form.check_box :wiki_page_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :wiki_page_events, class: 'list-label' do
+          %strong Wiki Page events
+        %p.light
+          This URL will be triggered when a wiki page is created/updated
+.form-group
+  = form.label :enable_ssl_verification, 'SSL verification', class: 'label-light checkbox'
+  .checkbox
+    = form.label :enable_ssl_verification do
+      = form.check_box :enable_ssl_verification
+      %strong Enable SSL verification
diff --git a/changelogs/unreleased/19364-webhook-edit.yml b/changelogs/unreleased/19364-webhook-edit.yml
new file mode 100644
index 00000000000..60e154b8b83
--- /dev/null
+++ b/changelogs/unreleased/19364-webhook-edit.yml
@@ -0,0 +1,4 @@
+---
+title: Implement ability to edit hooks
+merge_request: 10816
+author: Alexander Randa
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index 52ba10604d4..48993420ed9 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -50,8 +50,10 @@ namespace :admin do
 
   resources :deploy_keys, only: [:index, :new, :create, :destroy]
 
-  resources :hooks, only: [:index, :create, :destroy] do
-    get :test
+  resources :hooks, only: [:index, :create, :edit, :update, :destroy] do
+    member do
+      get :test
+    end
   end
 
   resources :broadcast_messages, only: [:index, :edit, :create, :update, :destroy] do
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 115ae2324b3..71fc84a7a12 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -185,7 +185,7 @@ constraints(ProjectUrlConstrainer.new) do
         end
       end
 
-      resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ } do
+      resources :hooks, only: [:index, :create, :edit, :update, :destroy], constraints: { id: /\d+/ } do
         member do
           get :test
         end
diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb
index 39c2a9dd1fb..0210e871a63 100644
--- a/spec/factories/project_hooks.rb
+++ b/spec/factories/project_hooks.rb
@@ -1,6 +1,7 @@
 FactoryGirl.define do
   factory :project_hook do
     url { generate(:url) }
+    enable_ssl_verification false
 
     trait :token do
       token { SecureRandom.hex(10) }
@@ -11,6 +12,7 @@ FactoryGirl.define do
       merge_requests_events true
       tag_push_events true
       issues_events true
+      confidential_issues_events true
       note_events true
       build_events true
       pipeline_events true
diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb
index fb519a9bf12..c5f24d412d7 100644
--- a/spec/features/admin/admin_hooks_spec.rb
+++ b/spec/features/admin/admin_hooks_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe "Admin::Hooks", feature: true do
+describe 'Admin::Hooks', feature: true do
   before do
     @project = create(:project)
     login_as :admin
@@ -8,24 +8,24 @@ describe "Admin::Hooks", feature: true do
     @system_hook = create(:system_hook)
   end
 
-  describe "GET /admin/hooks" do
-    it "is ok" do
+  describe 'GET /admin/hooks' do
+    it 'is ok' do
       visit admin_root_path
 
-      page.within ".layout-nav" do
-        click_on "Hooks"
+      page.within '.layout-nav' do
+        click_on 'Hooks'
       end
 
       expect(current_path).to eq(admin_hooks_path)
     end
 
-    it "has hooks list" do
+    it 'has hooks list' do
       visit admin_hooks_path
       expect(page).to have_content(@system_hook.url)
     end
   end
 
-  describe "New Hook" do
+  describe 'New Hook' do
     let(:url) { generate(:url) }
 
     it 'adds new hook' do
@@ -40,11 +40,36 @@ describe "Admin::Hooks", feature: true do
     end
   end
 
-  describe "Test" do
+  describe 'Update existing hook' do
+    let(:new_url) { generate(:url) }
+
+    it 'updates existing hook' do
+      visit admin_hooks_path
+
+      click_link 'Edit'
+      fill_in 'hook_url', with: new_url
+      check 'Enable SSL verification'
+      click_button 'Save changes'
+
+      expect(page).to have_content 'SSL Verification: enabled'
+      expect(current_path).to eq(admin_hooks_path)
+      expect(page).to have_content(new_url)
+    end
+  end
+
+  describe 'Remove existing hook' do
+    it 'remove existing hook' do
+      visit admin_hooks_path
+
+      expect { click_link 'Remove' }.to change(SystemHook, :count).by(-1)
+    end
+  end
+
+  describe 'Test' do
     before do
       WebMock.stub_request(:post, @system_hook.url)
       visit admin_hooks_path
-      click_link "Test hook"
+      click_link 'Test hook'
     end
 
     it { expect(current_path).to eq(admin_hooks_path) }
diff --git a/spec/features/projects/settings/integration_settings_spec.rb b/spec/features/projects/settings/integration_settings_spec.rb
new file mode 100644
index 00000000000..7909234556e
--- /dev/null
+++ b/spec/features/projects/settings/integration_settings_spec.rb
@@ -0,0 +1,94 @@
+require 'spec_helper'
+
+feature 'Integration settings', feature: true do
+  let(:project) { create(:empty_project) }
+  let(:user) { create(:user) }
+  let(:role) { :developer }
+  let(:integrations_path) { namespace_project_settings_integrations_path(project.namespace, project) }
+
+  background do
+    login_as(user)
+    project.team << [user, role]
+  end
+
+  context 'for developer' do
+    given(:role) { :developer }
+
+    scenario 'to be disallowed to view' do
+      visit integrations_path
+
+      expect(page.status_code).to eq(404)
+    end
+  end
+
+  context 'for master' do
+    given(:role) { :master }
+
+    context 'Webhooks' do
+      let(:hook) { create(:project_hook, :all_events_enabled, enable_ssl_verification: true, project: project) }
+      let(:url) { generate(:url) }
+
+      scenario 'show list of webhooks' do
+        hook
+
+        visit integrations_path
+
+        expect(page.status_code).to eq(200)
+        expect(page).to have_content(hook.url)
+        expect(page).to have_content('SSL Verification: enabled')
+        expect(page).to have_content('Push Events')
+        expect(page).to have_content('Tag Push Events')
+        expect(page).to have_content('Issues Events')
+        expect(page).to have_content('Confidential Issues Events')
+        expect(page).to have_content('Note Events')
+        expect(page).to have_content('Merge Requests  Events')
+        expect(page).to have_content('Pipeline Events')
+        expect(page).to have_content('Wiki Page Events')
+      end
+
+      scenario 'create webhook' do
+        visit integrations_path
+
+        fill_in 'hook_url', with: url
+        check 'Tag push events'
+        check 'Enable SSL verification'
+
+        click_button 'Add webhook'
+
+        expect(page).to have_content(url)
+        expect(page).to have_content('SSL Verification: enabled')
+        expect(page).to have_content('Push Events')
+        expect(page).to have_content('Tag Push Events')
+      end
+
+      scenario 'edit existing webhook' do
+        hook
+        visit integrations_path
+
+        click_link 'Edit'
+        fill_in 'hook_url', with: url
+        check 'Enable SSL verification'
+        click_button 'Save changes'
+
+        expect(page).to have_content 'SSL Verification: enabled'
+        expect(page).to have_content(url)
+      end
+
+      scenario 'test existing webhook' do
+        WebMock.stub_request(:post, hook.url)
+        visit integrations_path
+
+        click_link 'Test'
+
+        expect(current_path).to eq(integrations_path)
+      end
+
+      scenario 'remove existing webhook' do
+        hook
+        visit integrations_path
+
+        expect { click_link 'Remove' }.to change(ProjectHook, :count).by(-1)
+      end
+    end
+  end
+end
diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb
index 99c44bde151..e5fc0b676af 100644
--- a/spec/routing/admin_routing_spec.rb
+++ b/spec/routing/admin_routing_spec.rb
@@ -71,13 +71,15 @@ describe Admin::ProjectsController, "routing" do
   end
 end
 
-# admin_hook_test GET    /admin/hooks/:hook_id/test(.:format) admin/hooks#test
+# admin_hook_test GET    /admin/hooks/:id/test(.:format)      admin/hooks#test
 #     admin_hooks GET    /admin/hooks(.:format)               admin/hooks#index
 #                 POST   /admin/hooks(.:format)               admin/hooks#create
 #      admin_hook DELETE /admin/hooks/:id(.:format)           admin/hooks#destroy
+#                 PUT    /admin/hooks/:id(.:format)           admin/hooks#update
+# edit_admin_hook GET    /admin/hooks/:id(.:format)           admin/hooks#edit
 describe Admin::HooksController, "routing" do
   it "to #test" do
-    expect(get("/admin/hooks/1/test")).to route_to('admin/hooks#test', hook_id: '1')
+    expect(get("/admin/hooks/1/test")).to route_to('admin/hooks#test', id: '1')
   end
 
   it "to #index" do
@@ -88,6 +90,14 @@ describe Admin::HooksController, "routing" do
     expect(post("/admin/hooks")).to route_to('admin/hooks#create')
   end
 
+  it "to #edit" do
+    expect(get("/admin/hooks/1/edit")).to route_to('admin/hooks#edit', id: '1')
+  end
+
+  it "to #update" do
+    expect(put("/admin/hooks/1")).to route_to('admin/hooks#update', id: '1')
+  end
+
   it "to #destroy" do
     expect(delete("/admin/hooks/1")).to route_to('admin/hooks#destroy', id: '1')
   end
diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb
index a3de022d242..163df072cf6 100644
--- a/spec/routing/project_routing_spec.rb
+++ b/spec/routing/project_routing_spec.rb
@@ -340,14 +340,16 @@ describe 'project routing' do
   # test_project_hook GET    /:project_id/hooks/:id/test(.:format) hooks#test
   #     project_hooks GET    /:project_id/hooks(.:format)          hooks#index
   #                   POST   /:project_id/hooks(.:format)          hooks#create
-  #      project_hook DELETE /:project_id/hooks/:id(.:format)      hooks#destroy
+  # edit_project_hook GET    /:project_id/hooks/:id/edit(.:format) hooks#edit
+  #      project_hook PUT    /:project_id/hooks/:id(.:format)      hooks#update
+  #                   DELETE /:project_id/hooks/:id(.:format)      hooks#destroy
   describe Projects::HooksController, 'routing' do
     it 'to #test' do
       expect(get('/gitlab/gitlabhq/hooks/1/test')).to route_to('projects/hooks#test', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
     end
 
     it_behaves_like 'RESTful project resources' do
-      let(:actions)    { [:index, :create, :destroy] }
+      let(:actions)    { [:index, :create, :destroy, :edit, :update] }
       let(:controller) { 'hooks' }
     end
   end