Merge branch 'extra-system-hooks' into 'master'

Added system hooks messages for renaming and transferring a project

This is work in progress but I wanted to get some feedback on it in case there is something I've missed or a better way to do it.

As the title says when a project is renamed or transferred to another group a system hook message is generated.

Currently adds a couple of extra properties to the standard project system hooks messages.

old_path_with_namespace - The old path of the project so we can see what has been renamed / transferred
changed_at - The time that change took place

My main concern is if there is a better way of doing this than passing the extra_data variable into the system hook.  Would it be better to extend the project model with these 2 items as transient properties as I doubt they would need to be saved to the database?  The other 

I've also seen, but can't find, someone talking about the emails sent when a project is renamed or transferred doesn't reference the original project path so if you have similarly named projects then you might not know what has been moved.  I think the extra_data (or however it is best to implement this) would allow the extra information to be sent in emails.

Closes [System hooks for project transfer and repository rename](http://feedback.gitlab.com/forums/176466-general/suggestions/6006233-system-hooks-for-project-transfer-and-repository-r)


See merge request !611

CHANGELOG

@@ -13,6 +13,7 @@ v 8.4.0 (unreleased)
   - Add link to merge request on build detail page
   - Revert back upvote and downvote button to the issue and MR pages
   - Swap position of Assignee and Author selector on Issuables (Zeger-Jan van de Weg)
+  - Add system hook messages for project rename and transfer (Steve Norman)
   - Fix version check image in Safari
 
 v 8.3.3 (unreleased)

app/models/project.rb

@@ -81,6 +81,7 @@ class Project < ActiveRecord::Base
   acts_as_taggable_on :tags
 
   attr_accessor :new_default_branch
+  attr_accessor :old_path_with_namespace
 
   # Relations
   belongs_to :creator, foreign_key: 'creator_id', class_name: 'User'
@@ -701,6 +702,11 @@ class Project < ActiveRecord::Base
         gitlab_shell.mv_repository("#{old_path_with_namespace}.wiki", "#{new_path_with_namespace}.wiki")
         send_move_instructions(old_path_with_namespace)
         reset_events_cache
+
+        @old_path_with_namespace = old_path_with_namespace
+
+        SystemHooksService.new.execute_hooks_for(self, :rename)
+
         @repository = nil
       rescue
         # Returning false does not rollback after_* transaction but gives

app/services/projects/transfer_service.rb

@@ -55,6 +55,9 @@ module Projects
         # Move uploads
         Gitlab::UploadsTransfer.new.move_project(project.path, old_namespace.path, new_namespace.path)
 
+        project.old_path_with_namespace = old_path
+        
+        SystemHooksService.new.execute_hooks_for(project, :transfer)
         true
       end
     end

app/services/system_hooks_service.rb

@@ -18,7 +18,8 @@ class SystemHooksService
   def build_event_data(model, event)
     data = {
       event_name: build_event_name(model, event),
-      created_at: model.created_at.xmlschema
+      created_at: model.created_at.xmlschema,
+      updated_at: model.updated_at.xmlschema
     }
 
     case model
@@ -34,6 +35,14 @@ class SystemHooksService
       end
     when Project
       data.merge!(project_data(model))
+
+      if event == :rename || event == :transfer
+        data.merge!({
+          old_path_with_namespace: model.old_path_with_namespace
+        })
+      end
+
+      data
     when User
       data.merge!({
         name: model.name,

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`, `key_destroy`, `group_create`, `group_destroy`, `user_add_to_group` and `user_remove_from_group`.
+Your GitLab instance can perform HTTP POST requests on the following events: `project_create`, `project_destroy`, `project_rename`, `project_transfer`, `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.
 
@@ -17,6 +17,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
           "created_at": "2012-07-21T07:30:54Z",
+          "updated_at": "2012-07-21T07:38:22Z",
           "event_name": "project_create",
                 "name": "StoreCloud",
          "owner_email": "johnsmith@gmail.com",
@@ -33,6 +34,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
           "created_at": "2012-07-21T07:30:58Z",
+          "updated_at": "2012-07-21T07:38:22Z",
           "event_name": "project_destroy",
                 "name": "Underscore",
          "owner_email": "johnsmith@gmail.com",
@@ -44,11 +46,48 @@ X-Gitlab-Event: System Hook
 }
 ```
 
+**Project renamed:**
+
+```json
+{
+               "created_at": "2012-07-21T07:30:58Z",
+               "updated_at": "2012-07-21T07:38:22Z",
+               "event_name": "project_rename",
+                     "name": "Underscore",
+                     "path": "underscore",
+      "path_with_namespace": "jsmith/underscore",
+               "project_id": 73,
+               "owner_name": "John Smith",
+              "owner_email": "johnsmith@gmail.com",
+       "project_visibility": "internal",
+  "old_path_with_namespace": "jsmith/overscore",
+}
+```
+
+**Project transferred:**
+
+```json
+{
+               "created_at": "2012-07-21T07:30:58Z",
+               "updated_at": "2012-07-21T07:38:22Z",
+               "event_name": "project_transfer",
+                     "name": "Underscore",
+                     "path": "underscore",
+      "path_with_namespace": "scores/underscore",
+               "project_id": 73,
+               "owner_name": "John Smith",
+              "owner_email": "johnsmith@gmail.com",
+       "project_visibility": "internal",
+  "old_path_with_namespace": "jsmith/overscore",
+}
+```
+
 **New Team Member:**
 
 ```json
 {
                   "created_at": "2012-07-21T07:30:56Z",
+                  "updated_at": "2012-07-21T07:38:22Z",
                   "event_name": "user_add_to_team",
               "project_access": "Master",
                   "project_id": 74,
@@ -67,6 +106,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
                   "created_at": "2012-07-21T07:30:56Z",
+                  "updated_at": "2012-07-21T07:38:22Z",
                   "event_name": "user_remove_from_team",
               "project_access": "Master",
                   "project_id": 74,
@@ -85,6 +125,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
    "created_at": "2012-07-21T07:44:07Z",
+   "updated_at": "2012-07-21T07:38:22Z",
         "email": "js@gitlabhq.com",
    "event_name": "user_create",
          "name": "John Smith",
@@ -97,6 +138,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
    "created_at": "2012-07-21T07:44:07Z",
+   "updated_at": "2012-07-21T07:38:22Z",
         "email": "js@gitlabhq.com",
    "event_name": "user_destroy",
          "name": "John Smith",
@@ -110,6 +152,7 @@ X-Gitlab-Event: System Hook
 {
     "event_name": "key_create",
     "created_at": "2014-08-18 18:45:16 UTC",
+    "updated_at": "2012-07-21T07:38:22Z",
       "username": "root",
            "key": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC58FwqHUbebw2SdT7SP4FxZ0w+lAO/erhy2ylhlcW/tZ3GY3mBu9VeeiSGoGz8hCx80Zrz+aQv28xfFfKlC8XQFpCWwsnWnQqO2Lv9bS8V1fIHgMxOHIt5Vs+9CAWGCCvUOAurjsUDoE2ALIXLDMKnJxcxD13XjWdK54j6ZXDB4syLF0C2PnAQSVY9X7MfCYwtuFmhQhKaBussAXpaVMRHltie3UYSBUUuZaB3J4cg/7TxlmxcNd+ppPRIpSZAB0NI6aOnqoBCpimscO/VpQRJMVLr3XiSYeT6HBiDXWHnIVPfQc03OGcaFqOit6p8lYKMaP/iUQLm+pgpZqrXZ9vB john@localhost",
            "id": 4
@@ -122,6 +165,7 @@ X-Gitlab-Event: System Hook
 {
     "event_name": "key_destroy",
     "created_at": "2014-08-18 18:45:16 UTC",
+    "updated_at": "2012-07-21T07:38:22Z",
       "username": "root",
            "key": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC58FwqHUbebw2SdT7SP4FxZ0w+lAO/erhy2ylhlcW/tZ3GY3mBu9VeeiSGoGz8hCx80Zrz+aQv28xfFfKlC8XQFpCWwsnWnQqO2Lv9bS8V1fIHgMxOHIt5Vs+9CAWGCCvUOAurjsUDoE2ALIXLDMKnJxcxD13XjWdK54j6ZXDB4syLF0C2PnAQSVY9X7MfCYwtuFmhQhKaBussAXpaVMRHltie3UYSBUUuZaB3J4cg/7TxlmxcNd+ppPRIpSZAB0NI6aOnqoBCpimscO/VpQRJMVLr3XiSYeT6HBiDXWHnIVPfQc03OGcaFqOit6p8lYKMaP/iUQLm+pgpZqrXZ9vB john@localhost",
             "id": 4
@@ -133,6 +177,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
    "created_at": "2012-07-21T07:30:54Z",
+   "updated_at": "2012-07-21T07:38:22Z",
    "event_name": "group_create",
          "name": "StoreCloud",
   "owner_email": "johnsmith@gmail.com",
@@ -147,6 +192,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
    "created_at": "2012-07-21T07:30:54Z",
+   "updated_at": "2012-07-21T07:38:22Z",
    "event_name": "group_destroy",
          "name": "StoreCloud",
   "owner_email": "johnsmith@gmail.com",
@@ -161,6 +207,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
     "created_at": "2012-07-21T07:30:56Z",
+    "updated_at": "2012-07-21T07:38:22Z",
     "event_name": "user_add_to_group",
   "group_access": "Master",
       "group_id": 78,
@@ -176,6 +223,7 @@ X-Gitlab-Event: System Hook
 ```json
 {
     "created_at": "2012-07-21T07:30:56Z",
+    "updated_at": "2012-07-21T07:38:22Z",
     "event_name": "user_remove_from_group",
   "group_access": "Master",
       "group_id": 78,

spec/services/system_hooks_service_spec.rb

@@ -9,37 +9,54 @@ describe SystemHooksService, services: true do
   let(:group_member)  { create(:group_member) }
 
   context 'event data' do
-    it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :email, :user_id) }
-    it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :email, :user_id) }
-    it { expect(event_data(project, :create)).to include(:event_name, :name, :created_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) }
-    it { expect(event_data(project, :destroy)).to include(:event_name, :name, :created_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) }
-    it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :access_level, :project_visibility) }
-    it { expect(event_data(project_member, :destroy)).to include(:event_name, :created_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_email, :access_level, :project_visibility) }
+    it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id) }
+    it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id) }
+    it { expect(event_data(project, :create)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) }
+    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_email, :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_email, :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 do
+      project.old_path_with_namespace = 'renamed_from_path'
+      expect(event_data(project, :rename)).to include(
+        :event_name, :name, :created_at, :updated_at, :path, :project_id, 
+        :owner_name, :owner_email, :project_visibility, 
+        :old_path_with_namespace
+      ) 
+    end
+    it do
+      project.old_path_with_namespace = 'transfered_from_path'
+      expect(event_data(project, :transfer)).to include(
+        :event_name, :name, :created_at, :updated_at, :path, :project_id, 
+        :owner_name, :owner_email, :project_visibility, 
+        :old_path_with_namespace
+      ) 
+    end
+
     it do
       expect(event_data(group, :create)).to include(
-        :event_name, :name, :created_at, :path, :group_id, :owner_name,
-        :owner_email
+        :event_name, :name, :created_at, :updated_at, :path, :group_id, 
+        :owner_name, :owner_email
       )
     end
     it do
       expect(event_data(group, :destroy)).to include(
-        :event_name, :name, :created_at, :path, :group_id, :owner_name,
-        :owner_email
+        :event_name, :name, :created_at, :updated_at, :path, :group_id, 
+        :owner_name, :owner_email
       )
     end
     it do
       expect(event_data(group_member, :create)).to include(
-        :event_name, :created_at, :group_name, :group_path, :group_id, :user_id,
-        :user_name, :user_email, :group_access
+        :event_name, :created_at, :updated_at, :group_name, :group_path, 
+        :group_id, :user_id, :user_name, :user_email, :group_access
       )
     end
     it do
       expect(event_data(group_member, :destroy)).to include(
-        :event_name, :created_at, :group_name, :group_path, :group_id, :user_id,
-        :user_name, :user_email, :group_access
+        :event_name, :created_at, :updated_at, :group_name, :group_path, 
+        :group_id, :user_id, :user_name, :user_email, :group_access
       )
     end
   end
@@ -49,6 +66,8 @@ describe SystemHooksService, services: true do
     it { expect(event_name(user, :destroy)).to eq "user_destroy" }
     it { expect(event_name(project, :create)).to eq "project_create" }
     it { expect(event_name(project, :destroy)).to eq "project_destroy" }
+    it { expect(event_name(project, :rename)).to eq "project_rename" }
+    it { expect(event_name(project, :transfer)).to eq "project_transfer" }
     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(key, :create)).to eq 'key_create' }