Merge branch 'change-role-system-hook' into 'master'

Add system hooks for project/group membership updates

Closes gitlab-ee#12252

See merge request gitlab-org/gitlab-ce!32371

app/models/member.rb

@@ -399,7 +399,7 @@ class Member < ApplicationRecord
   end
 
   def post_update_hook
-    # override in sub class
+    system_hook_service.execute_hooks_for(self, :update)
   end
 
   def post_destroy_hook

app/services/system_hooks_service.rb

@@ -74,9 +74,11 @@ class SystemHooksService
     when ProjectMember
       return "user_add_to_team"      if event == :create
       return "user_remove_from_team" if event == :destroy
+      return "user_update_for_team"  if event == :update
     when GroupMember
       return 'user_add_to_group'      if event == :create
       return 'user_remove_from_group' if event == :destroy
+      return 'user_update_for_group'  if event == :update
     else
       "#{model.class.name.downcase}_#{event}"
     end

changelogs/unreleased/change-role-system-hook.yml

+---
+title: Add system hooks for project/group membership updates
+merge_request: 32371
+author: Brandon Williams
+type: added

doc/system_hooks/system_hooks.md

@@ -13,6 +13,7 @@ Your GitLab instance can perform HTTP POST requests on the following events:
 - `project_update`
 - `user_add_to_team`
 - `user_remove_from_team`
+- `user_update_for_team`
 - `user_create`
 - `user_destroy`
 - `user_failed_login`
@@ -24,6 +25,7 @@ Your GitLab instance can perform HTTP POST requests on the following events:
 - `group_rename`
 - `user_add_to_group`
 - `user_remove_from_group`
+- `user_update_for_group`
 
 The triggers for most of these are self-explanatory, but `project_update` and `project_rename` deserve some clarification: `project_update` is fired any time an attribute of a project is changed (name, description, tags, etc.) *unless* the `path` attribute is also changed. In that case, a `project_rename` is triggered instead (so that, for instance, if all you care about is the repo URL, you can just listen for `project_rename`).
 
@@ -173,6 +175,26 @@ Please refer to `group_rename` and `user_rename` for that case.
 }
 ```
 
+**Team Member Updated:**
+
+```json
+{
+                  "created_at": "2012-07-21T07:30:56Z",
+                  "updated_at": "2012-07-21T07:38:22Z",
+                  "event_name": "user_update_for_team",
+                "access_level": "Maintainer",
+                  "project_id": 74,
+                "project_name": "StoreCloud",
+                "project_path": "storecloud",
+ "project_path_with_namespace": "jsmith/storecloud",
+                  "user_email": "johnsmith@gmail.com",
+                   "user_name": "John Smith",
+               "user_username": "johnsmith",
+                     "user_id": 41,
+          "project_visibility": "visibilitylevel|private"
+}
+```
+
 **User created:**
 
 ```json
@@ -349,6 +371,24 @@ If the user is blocked via LDAP, `state` will be `ldap_blocked`.
 }
 ```
 
+**Group Member Updated:**
+
+```json
+{
+    "created_at": "2012-07-21T07:30:56Z",
+    "updated_at": "2012-07-21T07:38:22Z",
+    "event_name": "user_update_for_group",
+  "group_access": "Maintainer",
+      "group_id": 78,
+    "group_name": "StoreCloud",
+    "group_path": "storecloud",
+    "user_email": "johnsmith@gmail.com",
+     "user_name": "John Smith",
+ "user_username": "johnsmith",
+       "user_id": 41
+}
+```
+
 ## Push events
 
 Triggered when you push to the repository, except when pushing tags.

spec/models/hooks/system_hook_spec.rb

@@ -64,7 +64,7 @@ describe SystemHook do
       ).once
     end
 
-    it "project_create hook" do
+    it "project member create hook" do
       project.add_maintainer(user)
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
@@ -73,7 +73,7 @@ describe SystemHook do
       ).once
     end
 
-    it "project_destroy hook" do
+    it "project member destroy hook" do
       project.add_maintainer(user)
       project.project_members.destroy_all # rubocop: disable DestroyAll
 
@@ -83,6 +83,15 @@ describe SystemHook do
       ).once
     end
 
+    it "project member update hook" do
+      project.add_guest(user)
+
+      expect(WebMock).to have_requested(:post, system_hook.url).with(
+        body: /user_update_for_team/,
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
+      ).once
+    end
+
     it 'group create hook' do
       create(:group)
 
@@ -119,6 +128,16 @@ describe SystemHook do
         headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
+
+    it 'group member update hook' do
+      group.add_guest(user)
+      group.add_maintainer(user)
+
+      expect(WebMock).to have_requested(:post, system_hook.url).with(
+        body: /user_update_for_group/,
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
+      ).once
+    end
   end
 
   describe '.repository_update_hooks' do

spec/services/system_hooks_service_spec.rb

@@ -19,6 +19,7 @@ describe SystemHooksService do
     it { expect(event_data(project, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) }
     it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) }
     it { expect(event_data(project_member, :destroy)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) }
+    it { expect(event_data(project_member, :update)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) }
     it { expect(event_data(key, :create)).to include(:username, :key, :id) }
     it { expect(event_data(key, :destroy)).to include(:username, :key, :id) }
     it { expect(event_data(deploy_key, :create)).to include(:key, :id) }
@@ -70,6 +71,13 @@ describe SystemHooksService do
       )
     end
 
+    it do
+      expect(event_data(group_member, :update)).to include(
+        :event_name, :created_at, :updated_at, :group_name, :group_path,
+        :group_id, :user_id, :user_username, :user_name, :user_email, :group_access
+      )
+    end
+
     it 'includes the correct project visibility level' do
       data = event_data(project, :create)
 
@@ -145,6 +153,7 @@ describe SystemHooksService do
     it { expect(event_name(project, :update)).to eq "project_update" }
     it { expect(event_name(project_member, :create)).to eq "user_add_to_team" }
     it { expect(event_name(project_member, :destroy)).to eq "user_remove_from_team" }
+    it { expect(event_name(project_member, :update)).to eq "user_update_for_team" }
     it { expect(event_name(key, :create)).to eq 'key_create' }
     it { expect(event_name(key, :destroy)).to eq 'key_destroy' }
     it { expect(event_name(group, :create)).to eq 'group_create' }
@@ -152,6 +161,7 @@ describe SystemHooksService do
     it { expect(event_name(group, :rename)).to eq 'group_rename' }
     it { expect(event_name(group_member, :create)).to eq 'user_add_to_group' }
     it { expect(event_name(group_member, :destroy)).to eq 'user_remove_from_group' }
+    it { expect(event_name(group_member, :update)).to eq 'user_update_for_group' }
   end
 
   def event_data(*args)