Merge pull request #3052 from Undev/system-hook-observer-improves

System hooks code & logic refactoring

app/models/system_hook.rb

@@ -12,13 +12,4 @@
 #
 
 class SystemHook < WebHook
-  def self.all_hooks_fire(data)
-    SystemHook.all.each do |sh|
-      sh.async_execute data
-    end
-  end
-
-  def async_execute(data)
-    Sidekiq::Client.enqueue(SystemHookWorker, id, data)
-  end
 end

app/observers/system_hook_observer.rb

@@ -1,67 +1,11 @@
 class SystemHookObserver < ActiveRecord::Observer
   observe :user, :project, :users_project
-  
+
   def after_create(model)
-    if model.kind_of? Project
-      SystemHook.all_hooks_fire({
-        event_name: "project_create",
-        name: model.name,
-        path: model.path,
-        project_id: model.id,
-        owner_name: model.owner.name,
-        owner_email: model.owner.email,
-        created_at: model.created_at
-      })
-    elsif model.kind_of? User 
-      SystemHook.all_hooks_fire({
-        event_name: "user_create",
-        name: model.name,
-        email: model.email,
-        created_at: model.created_at
-      })
-
-    elsif model.kind_of? UsersProject
-      SystemHook.all_hooks_fire({
-        event_name: "user_add_to_team",
-        project_name: model.project.name,
-        project_path: model.project.path,
-        project_id: model.project_id,
-        user_name: model.user.name,
-        user_email: model.user.email,
-        project_access: model.repo_access_human,
-        created_at: model.created_at
-      })
-
-    end
+    SystemHooksService.execute_hooks_for(model, :create)
   end
 
   def after_destroy(model)
-    if model.kind_of? Project
-      SystemHook.all_hooks_fire({
-        event_name: "project_destroy",
-        name: model.name,
-        path: model.path,
-        project_id: model.id,
-        owner_name: model.owner.name,
-        owner_email: model.owner.email,
-      })
-    elsif model.kind_of? User
-      SystemHook.all_hooks_fire({
-        event_name: "user_destroy",
-        name: model.name,
-        email: model.email
-      })
-
-    elsif model.kind_of? UsersProject
-      SystemHook.all_hooks_fire({
-        event_name: "user_remove_from_team",
-        project_name: model.project.name,
-        project_path: model.project.path,
-        project_id: model.project_id,
-        user_name: model.user.name,
-        user_email: model.user.email,
-        project_access: model.repo_access_human
-      })
-    end
+    SystemHooksService.execute_hooks_for(model, :destroy)
   end
 end

app/services/system_hooks_service.rb

@@ -0,0 +1,59 @@
+class SystemHooksService
+  def self.execute_hooks_for(model, event)
+    execute_hooks(build_event_data(model, event))
+  end
+
+  private
+
+  def self.execute_hooks(data)
+    SystemHook.all.each do |sh|
+      async_execute_hook sh, data
+    end
+  end
+
+  def self.async_execute_hook(hook, data)
+    Sidekiq::Client.enqueue(SystemHookWorker, hook.id, data)
+  end
+
+  def self.build_event_data(model, event)
+    data = {
+      event_name: build_event_name(model, event),
+      created_at: model.created_at
+    }
+
+    case model
+    when Project
+      data.merge!({
+        name: model.name,
+        path: model.path,
+        project_id: model.id,
+        owner_name: model.owner.name,
+        owner_email: model.owner.email
+      })
+    when User
+      data.merge!({
+        name: model.name,
+        email: model.email
+      })
+    when UsersProject
+      data.merge!({
+        project_name: model.project.name,
+        project_path: model.project.path,
+        project_id: model.project_id,
+        user_name: model.user.name,
+        user_email: model.user.email,
+        project_access: model.repo_access_human
+      })
+    end
+  end
+
+  def self.build_event_name(model, event)
+    case model
+    when UsersProject
+      return "user_add_to_team"      if event == :create
+      return "user_remove_from_team" if event == :destroy
+    else
+      "#{model.class.name.downcase}_#{event.to_s}"
+    end
+  end
+end

spec/services/system_hooks_service_spec.rb

@@ -0,0 +1,41 @@
+require 'spec_helper'
+
+describe SystemHooksService do
+  let (:user)          { create :user }
+  let (:project)       { create :project }
+  let (:users_project) { create :users_project }
+
+  context 'it should build event data' do
+    it 'should build event data for user' do
+      SystemHooksService.build_event_data(user, :create).should include(:event_name, :name, :created_at, :email)
+    end
+
+    it 'should build event data for project' do
+      SystemHooksService.build_event_data(project, :create).should include(:event_name, :name, :created_at, :path, :project_id, :owner_name, :owner_email)
+    end
+
+    it 'should build event data for users project' do
+      SystemHooksService.build_event_data(users_project, :create).should include(:event_name, :created_at, :project_name, :project_path, :project_id, :user_name, :user_email, :project_access)
+    end
+  end
+
+  context 'it should build event names' do
+    it 'should build event names for user' do
+      SystemHooksService.build_event_name(user, :create).should eq "user_create"
+
+      SystemHooksService.build_event_name(user, :destroy).should eq "user_destroy"
+    end
+
+    it 'should build event names for project' do
+      SystemHooksService.build_event_name(project, :create).should eq "project_create"
+
+      SystemHooksService.build_event_name(project, :destroy).should eq "project_destroy"
+    end
+
+    it 'should build event names for users project' do
+      SystemHooksService.build_event_name(users_project, :create).should eq "user_add_to_team"
+
+      SystemHooksService.build_event_name(users_project, :destroy).should eq "user_remove_from_team"
+    end
+  end
+end