Merge branch 'feature/event_hooks' of /home/git/repositories/gitlab/gitlabhq

CHANGELOG

@@ -7,6 +7,7 @@ v 6.4.0
   - Side-by-side diff view (Steven Thonus)
   - Internal projects (Jason Hollingsworth)
   - Allow removal of avatar (Drew Blessing)
+  - Project web hooks now support issues and merge request events
 
 v 6.3.0
   - API for adding gitlab-ci service

app/assets/stylesheets/gitlab_bootstrap/forms.scss

@@ -13,6 +13,13 @@ form {
         margin-top: 1px !important;
       }
     }
+
+    &.list-label {
+      float: none;
+      padding: 0 !important;
+      margin: 0;
+      text-align: left;
+    }
   }
 }
 

app/helpers/application_helper.rb

@@ -207,4 +207,12 @@ module ApplicationHelper
   def broadcast_message
     BroadcastMessage.current
   end
+
+  def highlight_js(&block)
+    string = capture(&block)
+
+    content_tag :div, class: user_color_scheme_class do
+      Pygments::Lexer[:js].highlight(string).html_safe
+    end
+  end
 end

app/models/concerns/issuable.rb

@@ -111,4 +111,11 @@ module Issuable
     end
     users.concat(mentions.reduce([], :|)).uniq
   end
+
+  def to_hook_data
+    {
+      object_kind: self.class.name.underscore,
+      object_attributes: self.attributes
+    }
+  end
 end

app/models/project.rb

@@ -298,8 +298,10 @@ class Project < ActiveRecord::Base
     ProjectTransferService.new.transfer(self, new_namespace)
   end
 
-  def execute_hooks(data)
-    hooks.each { |hook| hook.async_execute(data) }
+  def execute_hooks(data, hooks_scope = :push_hooks)
+    hooks.send(hooks_scope).each do |hook|
+      hook.async_execute(data)
+    end
   end
 
   def execute_services(data)

app/models/project_hook.rb

@@ -9,8 +9,17 @@
 #  updated_at            :datetime         not null
 #  type                  :string(255)      default("ProjectHook")
 #  service_id            :integer
+#  push_events           :boolean          default(TRUE), not null
+#  issues_events         :boolean          default(FALSE), not null
+#  merge_requests_events :boolean          default(FALSE), not null
 #
 
 class ProjectHook < WebHook
   belongs_to :project
+
+  attr_accessible :push_events, :issues_events, :merge_requests_events
+
+  scope :push_hooks, -> { where(push_events: true) }
+  scope :issue_hooks, -> { where(issues_events: true) }
+  scope :merge_request_hooks, -> { where(merge_requests_events: true) }
 end

app/models/service_hook.rb

@@ -9,6 +9,9 @@
 #  updated_at            :datetime         not null
 #  type                  :string(255)      default("ProjectHook")
 #  service_id            :integer
+#  push_events           :boolean          default(TRUE), not null
+#  issues_events         :boolean          default(FALSE), not null
+#  merge_requests_events :boolean          default(FALSE), not null
 #
 
 class ServiceHook < WebHook

app/models/system_hook.rb

@@ -9,6 +9,9 @@
 #  updated_at            :datetime         not null
 #  type                  :string(255)      default("ProjectHook")
 #  service_id            :integer
+#  push_events           :boolean          default(TRUE), not null
+#  issues_events         :boolean          default(FALSE), not null
+#  merge_requests_events :boolean          default(FALSE), not null
 #
 
 class SystemHook < WebHook

app/models/web_hook.rb

@@ -9,6 +9,9 @@
 #  updated_at            :datetime         not null
 #  type                  :string(255)      default("ProjectHook")
 #  service_id            :integer
+#  push_events           :boolean          default(TRUE), not null
+#  issues_events         :boolean          default(FALSE), not null
+#  merge_requests_events :boolean          default(FALSE), not null
 #
 
 class WebHook < ActiveRecord::Base

app/observers/issue_observer.rb

 class IssueObserver < BaseObserver
   def after_create(issue)
     notification.new_issue(issue, current_user)
-
     issue.create_cross_references!(issue.project, current_user)
+    execute_hooks(issue)
   end
 
   def after_close(issue, transition)
     notification.close_issue(issue, current_user)
-
     create_note(issue)
+    execute_hooks(issue)
   end
 
   def after_reopen(issue, transition)
@@ -29,4 +29,8 @@ class IssueObserver < BaseObserver
   def create_note(issue)
     Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit)
   end
+
+  def execute_hooks(issue)
+    issue.project.execute_hooks(issue.to_hook_data, :issue_hooks)
+  end
 end

app/observers/merge_request_observer.rb

@@ -7,15 +7,15 @@ class MergeRequestObserver < ActivityObserver
     end
 
     notification.new_merge_request(merge_request, current_user)
-
     merge_request.create_cross_references!(merge_request.project, current_user)
+    execute_hooks(merge_request)
   end
 
   def after_close(merge_request, transition)
     create_event(merge_request, Event::CLOSED)
-    Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
-
     notification.close_mr(merge_request, current_user)
+    create_note(merge_request)
+    execute_hooks(merge_request)
   end
 
   def after_merge(merge_request, transition)
@@ -31,11 +31,13 @@ class MergeRequestObserver < ActivityObserver
       action: Event::MERGED,
       author_id: merge_request.author_id_of_changes
     )
+
+    execute_hooks(merge_request)
   end
 
   def after_reopen(merge_request, transition)
     create_event(merge_request, Event::REOPENED)
-    Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
+    create_note(merge_request)
   end
 
   def after_update(merge_request)
@@ -53,4 +55,17 @@ class MergeRequestObserver < ActivityObserver
       author_id: current_user.id
     )
   end
+
+  private
+
+  # Create merge request note with service comment like 'Status changed to closed'
+  def create_note(merge_request)
+    Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
+  end
+
+  def execute_hooks(merge_request)
+    if merge_request.project
+      merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks)
+    end
+  end
 end

app/services/git_push_service.rb

@@ -32,7 +32,7 @@ class GitPushService
     end
 
     if push_to_branch?(ref)
-      project.execute_hooks(@push_data.dup)
+      project.execute_hooks(@push_data.dup, :push_hooks)
       project.execute_services(@push_data.dup)
     end
 

app/views/help/web_hooks.html.haml

 = render layout: 'help/layout' do
-  %h3.page-title Web hooks
+  %h3.page-title Project web hooks
+  %p.light
+    Project web hooks allow you to trigger url if new code is pushed or new issue is created
+  %hr
 
   %p.slead
-    Every GitLab project can trigger a web server whenever the repo is pushed to.
+    You can configure web hook to listen for specific events like pushes, issues, merge requests.
+    %br
+    GitLab will send POST request with data to web hook url.
     %br
     Web Hooks can be used to update an external issue tracker, trigger CI builds, update a backup mirror, or even deploy to your production server.
+  %hr
+
+  %h4 Push events
+  %p.light
+    Triggered when you push to the repository except pushing tags.
     %br
-    GitLab will send POST request with commits information on every push.
-  %h5 Hooks request example:
-  = render "projects/hooks/data_ex"
+    Request body:
+  = highlight_js do
+    :erb
+      {
+        "before": "95790bf891e76fee5e1747ab589903a6a1f80f22",
+        "after": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+        "ref": "refs/heads/master",
+        "user_id": 4,
+        "user_name": "John Smith",
+        "project_id": 15,
+        "repository": {
+          "name": "Diaspora",
+          "url": "git@localhost:diaspora.git",
+          "description": "",
+          "homepage": "http://localhost/diaspora",
+        },
+        "commits": [
+          {
+            "id": "b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327",
+            "message": "Update Catalan translation to e38cb41.",
+            "timestamp": "2011-12-12T14:27:31+02:00",
+            "url": "http://localhost/diaspora/commits/b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327",
+            "author": {
+              "name": "Jordi Mallach",
+              "email": "jordi@softcatala.org",
+            }
+          },
+          // ...
+          {
+            "id": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+            "message": "fixed readme",
+            "timestamp": "2012-01-03T23:36:29+02:00",
+            "url": "http://localhost/diaspora/commits/da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+            "author": {
+              "name": "GitLab dev user",
+              "email": "gitlabdev@dv6700.(none)",
+            },
+          },
+        ],
+        "total_commits_count": 4,
+      };
+
 
+  %h4.prepend-top-20 Issues events
+  %p.light
+    Triggered when new issue created or existing issue was closed.
+    %br
+    Request body:
+  = highlight_js do
+    :erb
+      {
+        "object_kind":"issue",
+        "object_attributes":{
+          "id":301,
+          "title":"New API: create/update/delete file",
+          "assignee_id":51,
+          "author_id":51,
+          "project_id":14,
+          "created_at":"2013-12-03T17:15:43Z",
+          "updated_at":"2013-12-03T17:15:43Z",
+          "position":0,
+          "branch_name":null,
+          "description":"Create new API for manipulations with repository",
+          "milestone_id":null,
+          "state":"opened",
+          "iid":23
+        }
+      }
+  %h4.prepend-top-20 Merge request events
+  %p.light
+    Triggered when new merge request created or existing merge request was merged/closed.
+    %br
+    Request body:
+  = highlight_js do
+    :erb
+      {
+        "object_kind":"merge_request",
+        "object_attributes":{
+          "id":99,
+          "target_branch":"master",
+          "source_branch":"ms-viewport",
+          "source_project_id":14,
+          "author_id":51,
+          "assignee_id":6,
+          "title":"MS-Viewport",
+          "created_at":"2013-12-03T17:23:34Z",
+          "updated_at":"2013-12-03T17:23:34Z",
+          "st_commits":null,
+          "st_diffs":null,
+          "milestone_id":null,
+          "state":"opened",
+          "merge_status":"unchecked",
+          "target_project_id":14,
+          "iid":1,
+          "description":""
+        }
+      }

app/views/projects/hooks/_data_ex.html.erb

-<% data_ex_str = <<eos
-{
-  "before": "95790bf891e76fee5e1747ab589903a6a1f80f22",
-  "after": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
-  "ref": "refs/heads/master",
-  "user_id": 4,
-  "user_name": "John Smith",
-  "project_id": 15,
-  "repository": {
-    "name": "Diaspora",
-    "url": "git@localhost:diaspora.git",
-    "description": "",
-    "homepage": "http://localhost/diaspora",
-  },
-  "commits": [
-    {
-      "id": "b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327",
-      "message": "Update Catalan translation to e38cb41.",
-      "timestamp": "2011-12-12T14:27:31+02:00",
-      "url": "http://localhost/diaspora/commits/b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327",
-      "author": {
-        "name": "Jordi Mallach",
-        "email": "jordi@softcatala.org",
-      }
-    },
-    // ...
-    {
-      "id": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
-      "message": "fixed readme",
-      "timestamp": "2012-01-03T23:36:29+02:00",
-      "url": "http://localhost/diaspora/commits/da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
-      "author": {
-        "name": "GitLab dev user",
-        "email": "gitlabdev@dv6700.(none)",
-      },
-    },
-  ],
-  "total_commits_count": 4,
-};
-eos
-%>
-<div class="<%= user_color_scheme_class%>">
-  <%= raw Pygments::Lexer[:js].highlight(data_ex_str) %>
-</div>

app/views/projects/hooks/index.html.haml

 %h3.page-title
-  Post-receive hooks
+  Web hooks
 
 %p.light
-  #{link_to "Post-receive hooks ", help_web_hooks_path, class: "vlink"} can be
-  used for binding events when someone pushes to the repository.
+  #{link_to "Web hooks ", help_web_hooks_path, class: "vlink"} can be
+  used for binding events when something happends to the the project.
 
 %hr.clearfix
 
@@ -13,23 +13,50 @@
       - @hook.errors.full_messages.each do |msg|
         %p= msg
   .control-group
-    = f.label :url, "URL:"
+    = f.label :url, "URL"
     .controls
       = f.text_field :url, class: "text_field input-xxlarge input-xpadding", placeholder: 'http://example.com/trigger-ci.json'
        
       = f.submit "Add Web Hook", class: "btn btn-create"
+  .control-group
+    = f.label :url, "Trigger"
+    .controls
+      %div
+        = 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 in case of push to repository
+      %div
+        = 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 for created issues
+      %div
+        = 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 for created merge requests
 %hr
 
 -if @hooks.any?
   .ui-box
     .title
-      Hooks (#{@hooks.count})
+      Web Hooks (#{@hooks.count})
     %ul.well-list
       - @hooks.each do |hook|
         %li
-          %span.badge.badge-info POST
-          →
-          %span.monospace= hook.url
           .pull-right
             = link_to 'Test Hook', test_project_hook_path(@project, hook), class: "btn btn-small grouped"
             = link_to 'Remove', project_hook_path(@project, hook), confirm: 'Are you sure?', method: :delete, class: "btn btn-remove btn-small grouped"
+          .clearfix
+            %span.monospace= hook.url
+          %p
+            - %w(push_events issues_events merge_requests_events).each do |trigger|
+              - if hook.send(trigger)
+                %span.label.label-gray= trigger.titleize

db/migrate/20131202192556_add_event_fields_for_web_hook.rb

+class AddEventFieldsForWebHook < ActiveRecord::Migration
+  def change
+    add_column :web_hooks, :push_events, :boolean, default: true, null: false
+    add_column :web_hooks, :issues_events, :boolean, default: false, null: false
+    add_column :web_hooks, :merge_requests_events, :boolean, default: false, null: false
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20131112220935) do
+ActiveRecord::Schema.define(:version => 20131202192556) do
 
   create_table "broadcast_messages", :force => true do |t|
     t.text     "message",    :null => false
@@ -338,6 +338,9 @@ ActiveRecord::Schema.define(:version => 20131112220935) do
     t.datetime "updated_at",                                       :null => false
     t.string   "type",                  :default => "ProjectHook"
     t.integer  "service_id"
+    t.boolean  "push_events",           :default => true,          :null => false
+    t.boolean  "issues_events",         :default => false,         :null => false
+    t.boolean  "merge_requests_events", :default => false,         :null => false
   end
 
   add_index "web_hooks", ["project_id"], :name => "index_web_hooks_on_project_id"

doc/api/projects.md

@@ -402,6 +402,10 @@ Parameters:
 {
   "id": 1,
   "url": "http://example.com/hook",
+  "project_id": 3,
+  "push_events": "true",
+  "issues_events": "true",
+  "merge_requests_events": "true",
   "created_at": "2012-10-12T17:04:47Z"
 }
 ```
@@ -419,6 +423,9 @@ Parameters:
 
 + `id` (required) - The ID or NAME of a project
 + `url` (required) - The hook URL
++ `push_events` - Trigger hook on push events
++ `issues_events` - Trigger hook on issues events
++ `merge_requests_events` - Trigger hook on merge_requests events
 
 
 ### Edit project hook
@@ -434,6 +441,9 @@ Parameters:
 + `id` (required) - The ID or NAME of a project
 + `hook_id` (required) - The ID of a project hook
 + `url` (required) - The hook URL
++ `push_events` - Trigger hook on push events
++ `issues_events` - Trigger hook on issues events
++ `merge_requests_events` - Trigger hook on merge_requests events
 
 
 ### Delete project hook

lib/api/entities.rb

@@ -24,6 +24,10 @@ module API
       expose :id, :url, :created_at
     end
 
+    class ProjectHook < Hook
+      expose :project_id, :push_events, :issues_events, :merge_requests_events
+    end
+
     class ForkedFromProject < Grape::Entity
       expose :id
       expose :name, :name_with_namespace

lib/api/project_hooks.rb

@@ -22,7 +22,7 @@ module API
       #   GET /projects/:id/hooks
       get ":id/hooks" do
         @hooks = paginate user_project.hooks
-        present @hooks, with: Entities::Hook
+        present @hooks, with: Entities::ProjectHook
       end
 
       # Get a project hook
@@ -34,7 +34,7 @@ module API
       #   GET /projects/:id/hooks/:hook_id
       get ":id/hooks/:hook_id" do
         @hook = user_project.hooks.find(params[:hook_id])
-        present @hook, with: Entities::Hook
+        present @hook, with: Entities::ProjectHook
       end
 
 
@@ -47,10 +47,11 @@ module API
       #   POST /projects/:id/hooks
       post ":id/hooks" do
         required_attributes! [:url]
+        attrs = attributes_for_keys [:url, :push_events, :issues_events, :merge_requests_events]
+        @hook = user_project.hooks.new(attrs)
 
-        @hook = user_project.hooks.new({"url" => params[:url]})
         if @hook.save
-          present @hook, with: Entities::Hook
+          present @hook, with: Entities::ProjectHook
         else
           if @hook.errors[:url].present?
             error!("Invalid url given", 422)
@@ -70,10 +71,10 @@ module API
       put ":id/hooks/:hook_id" do
         @hook = user_project.hooks.find(params[:hook_id])
         required_attributes! [:url]
+        attrs = attributes_for_keys [:url, :push_events, :issues_events, :merge_requests_events]
 
-        attrs = attributes_for_keys [:url]
         if @hook.update_attributes attrs
-          present @hook, with: Entities::Hook
+          present @hook, with: Entities::ProjectHook
         else
           if @hook.errors[:url].present?
             error!("Invalid url given", 422)

spec/models/service_hook_spec.rb

@@ -9,6 +9,9 @@
 #  updated_at            :datetime         not null
 #  type                  :string(255)      default("ProjectHook")
 #  service_id            :integer
+#  push_events           :boolean          default(TRUE), not null
+#  issues_events         :boolean          default(FALSE), not null
+#  merge_requests_events :boolean          default(FALSE), not null
 #
 
 require "spec_helper"

spec/models/system_hook_spec.rb

@@ -9,6 +9,9 @@
 #  updated_at            :datetime         not null
 #  type                  :string(255)      default("ProjectHook")
 #  service_id            :integer
+#  push_events           :boolean          default(TRUE), not null
+#  issues_events         :boolean          default(FALSE), not null
+#  merge_requests_events :boolean          default(FALSE), not null
 #
 
 require "spec_helper"

spec/models/web_hook_spec.rb

@@ -9,6 +9,9 @@
 #  updated_at            :datetime         not null
 #  type                  :string(255)      default("ProjectHook")
 #  service_id            :integer
+#  push_events           :boolean          default(TRUE), not null
+#  issues_events         :boolean          default(FALSE), not null
+#  merge_requests_events :boolean          default(FALSE), not null
 #
 
 require 'spec_helper'

spec/observers/merge_request_observer_spec.rb

@@ -4,7 +4,7 @@ describe MergeRequestObserver do
   let(:some_user) { create :user }
   let(:assignee) { create :user }
   let(:author) { create :user }
-  let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author) }
+  let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author).as_null_object }
   let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author, target_project: create(:project)) }
   let(:unassigned_mr) { create(:merge_request, author: author, target_project: create(:project)) }
   let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author, target_project: create(:project)) }

spec/requests/api/project_hooks_spec.rb

+require 'spec_helper'
+
+describe API::API, 'ProjectHooks' do
+  include ApiHelpers
+  before(:each) { enable_observers }
+  after(:each) { disable_observers }
+
+  let(:user) { create(:user) }
+  let(:user3) { create(:user) }
+  let!(:project) { create(:project_with_code, creator_id: user.id, namespace: user.namespace) }
+  let!(:hook) { create(:project_hook, project: project, url: "http://example.com") }
+
+  before do
+    project.team << [user, :master]
+    project.team << [user3, :developer]
+  end
+
+  describe "GET /projects/:id/hooks" do
+    context "authorized user" do
+      it "should return project hooks" do
+        get api("/projects/#{project.id}/hooks", user)
+        response.status.should == 200
+
+        json_response.should be_an Array
+        json_response.count.should == 1
+        json_response.first['url'].should == "http://example.com"
+      end
+    end
+
+    context "unauthorized user" do
+      it "should not access project hooks" do
+        get api("/projects/#{project.id}/hooks", user3)
+        response.status.should == 403
+      end
+    end
+  end
+
+  describe "GET /projects/:id/hooks/:hook_id" do
+    context "authorized user" do
+      it "should return a project hook" do
+        get api("/projects/#{project.id}/hooks/#{hook.id}", user)
+        response.status.should == 200
+        json_response['url'].should == hook.url
+      end
+
+      it "should return a 404 error if hook id is not available" do
+        get api("/projects/#{project.id}/hooks/1234", user)
+        response.status.should == 404
+      end
+    end
+
+    context "unauthorized user" do
+      it "should not access an existing hook" do
+        get api("/projects/#{project.id}/hooks/#{hook.id}", user3)
+        response.status.should == 403
+      end
+    end
+
+    it "should return a 404 error if hook id is not available" do
+      get api("/projects/#{project.id}/hooks/1234", user)
+      response.status.should == 404
+    end
+  end
+
+  describe "POST /projects/:id/hooks" do
+    it "should add hook to project" do
+      expect {
+        post api("/projects/#{project.id}/hooks", user),
+          url: "http://example.com", issues_events: true
+      }.to change {project.hooks.count}.by(1)
+      response.status.should == 201
+    end
+
+    it "should return a 400 error if url not given" do
+      post api("/projects/#{project.id}/hooks", user)
+      response.status.should == 400
+    end
+
+    it "should return a 422 error if url not valid" do
+      post api("/projects/#{project.id}/hooks", user), "url" => "ftp://example.com"
+      response.status.should == 422
+    end
+  end
+
+  describe "PUT /projects/:id/hooks/:hook_id" do
+    it "should update an existing project hook" do
+      put api("/projects/#{project.id}/hooks/#{hook.id}", user),
+        url: 'http://example.org', push_events: false
+      response.status.should == 200
+      json_response['url'].should == 'http://example.org'
+    end
+
+    it "should return 404 error if hook id not found" do
+      put api("/projects/#{project.id}/hooks/1234", user), url: 'http://example.org'
+      response.status.should == 404
+    end
+
+    it "should return 400 error if url is not given" do
+      put api("/projects/#{project.id}/hooks/#{hook.id}", user)
+      response.status.should == 400
+    end
+
+    it "should return a 422 error if url is not valid" do
+      put api("/projects/#{project.id}/hooks/#{hook.id}", user), url: 'ftp://example.com'
+      response.status.should == 422
+    end
+  end
+
+  describe "DELETE /projects/:id/hooks/:hook_id" do
+    it "should delete hook from project" do
+      expect {
+        delete api("/projects/#{project.id}/hooks/#{hook.id}", user)
+      }.to change {project.hooks.count}.by(-1)
+      response.status.should == 200
+    end
+
+    it "should return success when deleting hook" do
+      delete api("/projects/#{project.id}/hooks/#{hook.id}", user)
+      response.status.should == 200
+    end
+
+    it "should return success when deleting non existent hook" do
+      delete api("/projects/#{project.id}/hooks/42", user)
+      response.status.should == 200
+    end
+
+    it "should return a 405 error if hook id not given" do
+      delete api("/projects/#{project.id}/hooks", user)
+      response.status.should == 405
+    end
+  end
+end

spec/requests/api/projects_spec.rb

@@ -10,7 +10,6 @@ describe API::API do
   let(:user3) { create(:user) }
   let(:admin) { create(:admin) }
   let!(:project) { create(:project_with_code, creator_id: user.id, namespace: user.namespace) }
-  let!(:hook) { create(:project_hook, project: project, url: "http://example.com") }
   let!(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') }
   let!(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) }
   let!(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) }
@@ -439,121 +438,6 @@ describe API::API do
     end
   end
 
-  describe "GET /projects/:id/hooks" do
-    context "authorized user" do
-      it "should return project hooks" do
-        get api("/projects/#{project.id}/hooks", user)
-        response.status.should == 200
-
-        json_response.should be_an Array
-        json_response.count.should == 1
-        json_response.first['url'].should == "http://example.com"
-      end
-    end
-
-    context "unauthorized user" do
-      it "should not access project hooks" do
-        get api("/projects/#{project.id}/hooks", user3)
-        response.status.should == 403
-      end
-    end
-  end
-
-  describe "GET /projects/:id/hooks/:hook_id" do
-    context "authorized user" do
-      it "should return a project hook" do
-        get api("/projects/#{project.id}/hooks/#{hook.id}", user)
-        response.status.should == 200
-        json_response['url'].should == hook.url
-      end
-
-      it "should return a 404 error if hook id is not available" do
-        get api("/projects/#{project.id}/hooks/1234", user)
-        response.status.should == 404
-      end
-    end
-
-    context "unauthorized user" do
-      it "should not access an existing hook" do
-        get api("/projects/#{project.id}/hooks/#{hook.id}", user3)
-        response.status.should == 403
-      end
-    end
-
-    it "should return a 404 error if hook id is not available" do
-      get api("/projects/#{project.id}/hooks/1234", user)
-      response.status.should == 404
-    end
-  end
-
-  describe "POST /projects/:id/hooks" do
-    it "should add hook to project" do
-      expect {
-        post api("/projects/#{project.id}/hooks", user),
-          url: "http://example.com"
-      }.to change {project.hooks.count}.by(1)
-      response.status.should == 201
-    end
-
-    it "should return a 400 error if url not given" do
-      post api("/projects/#{project.id}/hooks", user)
-      response.status.should == 400
-    end
-
-    it "should return a 422 error if url not valid" do
-      post api("/projects/#{project.id}/hooks", user), "url" => "ftp://example.com"
-      response.status.should == 422
-    end
-  end
-
-  describe "PUT /projects/:id/hooks/:hook_id" do
-    it "should update an existing project hook" do
-      put api("/projects/#{project.id}/hooks/#{hook.id}", user),
-        url: 'http://example.org'
-      response.status.should == 200
-      json_response['url'].should == 'http://example.org'
-    end
-
-    it "should return 404 error if hook id not found" do
-      put api("/projects/#{project.id}/hooks/1234", user), url: 'http://example.org'
-      response.status.should == 404
-    end
-
-    it "should return 400 error if url is not given" do
-      put api("/projects/#{project.id}/hooks/#{hook.id}", user)
-      response.status.should == 400
-    end
-
-    it "should return a 422 error if url is not valid" do
-      put api("/projects/#{project.id}/hooks/#{hook.id}", user), url: 'ftp://example.com'
-      response.status.should == 422
-    end
-  end
-
-  describe "DELETE /projects/:id/hooks/:hook_id" do
-    it "should delete hook from project" do
-      expect {
-        delete api("/projects/#{project.id}/hooks/#{hook.id}", user)
-      }.to change {project.hooks.count}.by(-1)
-      response.status.should == 200
-    end
-
-    it "should return success when deleting hook" do
-      delete api("/projects/#{project.id}/hooks/#{hook.id}", user)
-      response.status.should == 200
-    end
-
-    it "should return success when deleting non existent hook" do
-      delete api("/projects/#{project.id}/hooks/42", user)
-      response.status.should == 200
-    end
-
-    it "should return a 405 error if hook id not given" do
-      delete api("/projects/#{project.id}/hooks", user)
-      response.status.should == 405
-    end
-  end
-
   describe "GET /projects/:id/snippets" do
     it "should return an array of project snippets" do
       get api("/projects/#{project.id}/snippets", user)

spec/services/git_push_service_spec.rb

@@ -74,38 +74,19 @@ describe GitPushService do
   end
 
   describe "Web Hooks" do
-    context "with web hooks" do
-      before do
-        @project_hook = create(:project_hook)
-        @project_hook_2 = create(:project_hook)
-        project.hooks << [@project_hook, @project_hook_2]
-
-        stub_request(:post, @project_hook.url)
-        stub_request(:post, @project_hook_2.url)
-      end
-
-      it "executes multiple web hook" do
-        @project_hook.should_receive(:async_execute).once
-        @project_hook_2.should_receive(:async_execute).once
-
-        service.execute(project, user, @oldrev, @newrev, @ref)
-      end
-    end
-
     context "execute web hooks" do
-      before do
-        @project_hook = create(:project_hook)
-        project.hooks << [@project_hook]
-        stub_request(:post, @project_hook.url)
-      end
-
       it "when pushing a branch for the first time" do
-        @project_hook.should_receive(:async_execute)
+        project.should_receive(:execute_hooks)
         service.execute(project, user, @blankrev, 'newrev', 'refs/heads/master')
       end
 
+      it "when pushing new commits to existing branch" do
+        project.should_receive(:execute_hooks)
+        service.execute(project, user, 'oldrev', 'newrev', 'refs/heads/master')
+      end
+
       it "when pushing tags" do
-        @project_hook.should_not_receive(:async_execute)
+        project.should_not_receive(:execute_hooks)
         service.execute(project, user, 'newrev', 'newrev', 'refs/tags/v1.0.0')
       end
     end