Merge pull request #8652 from bladealslayer/feature/system_hook_for_groups

Added support for firing system hooks on group create/destroy and adding...

CHANGELOG

@@ -59,6 +59,7 @@ v 7.8.0
   - 
   - 
   - 
+  - Added support for firing system hooks on group create/destroy and adding/removing users to group (Boyan Tabakov)
 
 v 7.7.1
   - Improve mention autocomplete performance

app/models/group.rb

@@ -25,6 +25,9 @@ class Group < Namespace
 
   mount_uploader :avatar, AttachmentUploader
 
+  after_create :post_create_hook
+  after_destroy :post_destroy_hook
+
   def human_name
     name
   end
@@ -74,6 +77,18 @@ class Group < Namespace
     projects.public_only.any?
   end
 
+  def post_create_hook
+    system_hook_service.execute_hooks_for(self, :create)
+  end
+
+  def post_destroy_hook
+    system_hook_service.execute_hooks_for(self, :destroy)
+  end
+
+  def system_hook_service
+    SystemHooksService.new
+  end
+
   class << self
     def search(query)
       where("LOWER(namespaces.name) LIKE :query", query: "%#{query.downcase}%")

app/models/members/group_member.rb

@@ -27,8 +27,9 @@ class GroupMember < Member
   scope :with_group, ->(group) { where(source_id: group.id) }
   scope :with_user, ->(user) { where(user_id: user.id) }
 
-  after_create :notify_create
+  after_create :post_create_hook
   after_update :notify_update
+  after_destroy :post_destroy_hook
 
   def self.access_level_roles
     Gitlab::Access.options_with_owner
@@ -42,8 +43,9 @@ class GroupMember < Member
     access_level
   end
 
-  def notify_create
+  def post_create_hook
     notification_service.new_group_member(self)
+    system_hook_service.execute_hooks_for(self, :create)
   end
 
   def notify_update
@@ -52,6 +54,14 @@ class GroupMember < Member
     end
   end
 
+  def post_destroy_hook
+    system_hook_service.execute_hooks_for(self, :destroy)
+  end
+
+  def system_hook_service
+    SystemHooksService.new
+  end
+
   def notification_service
     NotificationService.new
   end

app/services/system_hooks_service.rb

@@ -60,6 +60,26 @@ class SystemHooksService
         access_level: model.human_access,
         project_visibility: Project.visibility_levels.key(model.project.visibility_level_field).downcase
       })
+    when Group
+      owner = model.owner
+
+      data.merge!(
+        name: model.name,
+        path: model.path,
+        group_id: model.id,
+        owner_name: owner.respond_to?(:name) ? owner.name : nil,
+        owner_email: owner.respond_to?(:email) ? owner.email : nil,
+      )
+    when GroupMember
+      data.merge!(
+        group_name: model.group.name,
+        group_path: model.group.path,
+        group_id: model.group.id,
+        user_name: model.user.name,
+        user_email: model.user.email,
+        user_id: model.user.id,
+        group_access: model.human_access,
+      )
     end
   end
 
@@ -68,6 +88,9 @@ class SystemHooksService
     when ProjectMember
       return "user_add_to_team"      if event == :create
       return "user_remove_from_team" if event == :destroy
+    when GroupMember
+      return 'user_add_to_group'      if event == :create
+      return 'user_remove_from_group' if event == :destroy
     else
       "#{model.class.name.downcase}_#{event.to_s}"
     end

doc/system_hooks/system_hooks.md

 # System hooks
 
-Your GitLab instance can perform HTTP POST requests on the following events: `project_create`, `project_destroy`, `user_add_to_team`, `user_remove_from_team`, `user_create`, `user_destroy`, `key_create` and `key_destroy`.
+Your GitLab instance can perform HTTP POST requests on the following events: `project_create`, `project_destroy`, `user_add_to_team`, `user_remove_from_team`, `user_create`, `user_destroy`, `key_create`, `key_destroy`, `group_create`, `group_destroy`, `user_add_to_group` and `user_remove_from_group`.
 
 System hooks can be used, e.g. for logging or changing information in a LDAP server.
 
@@ -50,6 +50,7 @@ System hooks can be used, e.g. for logging or changing information in a LDAP ser
        "project_path": "storecloud",
          "user_email": "johnsmith@gmail.com",
           "user_name": "John Smith",
+            "user_id": 41,
  "project_visibility": "private",
 }
 ```
@@ -66,6 +67,7 @@ System hooks can be used, e.g. for logging or changing information in a LDAP ser
        "project_path": "storecloud",
          "user_email": "johnsmith@gmail.com",
           "user_name": "John Smith",
+            "user_id": 41,
  "project_visibility": "private",
 }
 ```
@@ -117,3 +119,62 @@ System hooks can be used, e.g. for logging or changing information in a LDAP ser
             "id": 4
 }
 ```
+
+**Group created:**
+
+```json
+{
+   "created_at": "2012-07-21T07:30:54Z",
+   "event_name": "group_create",
+         "name": "StormCloud",
+  "owner_email": "johnsmith@gmail.com",
+   "owner_name": "John Smith",
+         "path": "stormcloud",
+     "group_id": 78
+}
+```
+
+**Group removed:**
+
+```json
+{
+   "created_at": "2012-07-21T07:30:54Z",
+   "event_name": "group_destroy",
+         "name": "StoreCloud",
+  "owner_email": "johnsmith@gmail.com",
+   "owner_name": "John Smith",
+         "path": "storecloud",
+     "group_id": 78
+}
+```
+
+**New Group Member:**
+
+```json
+{
+    "created_at": "2012-07-21T07:30:56Z",
+    "event_name": "user_add_to_group",
+  "group_access": "Master",
+      "group_id": 78,
+    "group_name": "StoreCloud",
+    "group_path": "storecloud",
+    "user_email": "johnsmith@gmail.com",
+     "user_name": "John Smith",
+       "user_id": 41
+}
+```
+**Group Member Removed:**
+
+```json
+{
+    "created_at": "2012-07-21T07:30:56Z",
+    "event_name": "user_remove_from_group",
+  "group_access": "Master",
+      "group_id": 78,
+    "group_name": "StoreCloud",
+    "group_path": "storecloud",
+    "user_email": "johnsmith@gmail.com",
+     "user_name": "John Smith",
+       "user_id": 41
+}
+```

spec/models/system_hook_spec.rb

@@ -61,5 +61,40 @@ describe SystemHook do
       project.project_members.destroy_all
       WebMock.should have_requested(:post, @system_hook.url).with(body: /user_remove_from_team/).once
     end
+
+    it 'group create hook' do
+      create(:group)
+      WebMock.should have_requested(:post, @system_hook.url).with(
+        body: /group_create/
+      ).once
+    end
+
+    it 'group destroy hook' do
+      group = create(:group)
+      group.destroy
+      WebMock.should have_requested(:post, @system_hook.url).with(
+        body: /group_destroy/
+      ).once
+    end
+
+    it 'group member create hook' do
+      group = create(:group)
+      user = create(:user)
+      group.add_user(user, Gitlab::Access::MASTER)
+      WebMock.should have_requested(:post, @system_hook.url).with(
+        body: /user_add_to_group/
+      ).once
+    end
+
+    it 'group member destroy hook' do
+      group = create(:group)
+      user = create(:user)
+      group.add_user(user, Gitlab::Access::MASTER)
+      group.group_members.destroy_all
+      WebMock.should have_requested(:post, @system_hook.url).with(
+        body: /user_remove_from_group/
+      ).once
+    end
+
   end
 end

spec/services/system_hooks_service_spec.rb

@@ -5,6 +5,8 @@ describe SystemHooksService do
   let (:project)       { create :project }
   let (:project_member) { create :project_member }
   let (:key)           { create(:key, user: user) }
+  let (:group)         { create(:group) }
+  let (:group_member)  { create(:group_member) }
 
   context 'event data' do
     it { event_data(user, :create).should include(:event_name, :name, :created_at, :email, :user_id) }
@@ -15,6 +17,31 @@ describe SystemHooksService do
     it { event_data(project_member, :destroy).should include(:event_name, :created_at, :project_name, :project_path, :project_id, :user_name, :user_email, :access_level, :project_visibility) }
     it { event_data(key, :create).should include(:username, :key, :id) }
     it { event_data(key, :destroy).should include(:username, :key, :id) }
+
+    it do
+      event_data(group, :create).should include(
+        :event_name, :name, :created_at, :path, :group_id, :owner_name,
+        :owner_email
+      )
+    end
+    it do
+      event_data(group, :destroy).should include(
+        :event_name, :name, :created_at, :path, :group_id, :owner_name,
+        :owner_email
+      )
+    end
+    it do
+      event_data(group_member, :create).should include(
+        :event_name, :created_at, :group_name, :group_path, :group_id, :user_id,
+        :user_name, :user_email, :group_access
+      )
+    end
+    it do
+      event_data(group_member, :destroy).should include(
+        :event_name, :created_at, :group_name, :group_path, :group_id, :user_id,
+        :user_name, :user_email, :group_access
+      )
+    end
   end
 
   context 'event names' do
@@ -26,6 +53,10 @@ describe SystemHooksService do
     it { event_name(project_member, :destroy).should eq "user_remove_from_team" }
     it { event_name(key, :create).should eq 'key_create' }
     it { event_name(key, :destroy).should eq 'key_destroy' }
+    it { event_name(group, :create).should eq 'group_create' }
+    it { event_name(group, :destroy).should eq 'group_destroy' }
+    it { event_name(group_member, :create).should eq 'user_add_to_group' }
+    it { event_name(group_member, :destroy).should eq 'user_remove_from_group' }
   end
 
   def event_data(*args)