From 468c8c5f0a66a9ebf1489926ba32c19db71d821a Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 13:15:56 +0400 Subject: [PATCH 1/4] A little bit of codestyle improvments --- app/observers/system_hook_observer.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app/observers/system_hook_observer.rb b/app/observers/system_hook_observer.rb index 312cd2b3622..195a6b127ee 100644 --- a/app/observers/system_hook_observer.rb +++ b/app/observers/system_hook_observer.rb @@ -1,8 +1,9 @@ class SystemHookObserver < ActiveRecord::Observer observe :user, :project, :users_project - + def after_create(model) - if model.kind_of? Project + case model + when Project SystemHook.all_hooks_fire({ event_name: "project_create", name: model.name, @@ -12,15 +13,14 @@ class SystemHookObserver < ActiveRecord::Observer owner_email: model.owner.email, created_at: model.created_at }) - elsif model.kind_of? User + when 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 + when UsersProject SystemHook.all_hooks_fire({ event_name: "user_add_to_team", project_name: model.project.name, @@ -31,12 +31,12 @@ class SystemHookObserver < ActiveRecord::Observer project_access: model.repo_access_human, created_at: model.created_at }) - end end def after_destroy(model) - if model.kind_of? Project + case model + when Project SystemHook.all_hooks_fire({ event_name: "project_destroy", name: model.name, @@ -45,14 +45,13 @@ class SystemHookObserver < ActiveRecord::Observer owner_name: model.owner.name, owner_email: model.owner.email, }) - elsif model.kind_of? User + when User SystemHook.all_hooks_fire({ event_name: "user_destroy", name: model.name, email: model.email }) - - elsif model.kind_of? UsersProject + when UsersProject SystemHook.all_hooks_fire({ event_name: "user_remove_from_team", project_name: model.project.name, From aa1780d03c7ceb916c2e122b841d3d4ebc5ce597 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 14:53:15 +0400 Subject: [PATCH 2/4] System hooks execution moved to System hook service --- app/models/system_hook.rb | 6 --- app/observers/system_hook_observer.rb | 59 +--------------------- app/services/system_hooks_service.rb | 55 ++++++++++++++++++++ spec/services/system_hooks_service_spec.rb | 41 +++++++++++++++ 4 files changed, 98 insertions(+), 63 deletions(-) create mode 100644 app/services/system_hooks_service.rb create mode 100644 spec/services/system_hooks_service_spec.rb diff --git a/app/models/system_hook.rb b/app/models/system_hook.rb index 5f1bd6477c4..3914a915aab 100644 --- a/app/models/system_hook.rb +++ b/app/models/system_hook.rb @@ -12,12 +12,6 @@ # 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 diff --git a/app/observers/system_hook_observer.rb b/app/observers/system_hook_observer.rb index 195a6b127ee..be2594b4916 100644 --- a/app/observers/system_hook_observer.rb +++ b/app/observers/system_hook_observer.rb @@ -2,65 +2,10 @@ class SystemHookObserver < ActiveRecord::Observer observe :user, :project, :users_project def after_create(model) - case model - when 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 - }) - when User - SystemHook.all_hooks_fire({ - event_name: "user_create", - name: model.name, - email: model.email, - created_at: model.created_at - }) - when 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) - case model - when 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, - }) - when User - SystemHook.all_hooks_fire({ - event_name: "user_destroy", - name: model.name, - email: model.email - }) - when 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 diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb new file mode 100644 index 00000000000..1d53f3baf33 --- /dev/null +++ b/app/services/system_hooks_service.rb @@ -0,0 +1,55 @@ +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| + sh.async_execute data + end + 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 diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb new file mode 100644 index 00000000000..7f1590f559e --- /dev/null +++ b/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 From 99760edc757b24796d5db1f5328d55f483e4c33c Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 15:33:03 +0400 Subject: [PATCH 3/4] Method moved to service --- app/models/system_hook.rb | 3 --- app/services/system_hooks_service.rb | 6 +++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/models/system_hook.rb b/app/models/system_hook.rb index 3914a915aab..5cdf046644f 100644 --- a/app/models/system_hook.rb +++ b/app/models/system_hook.rb @@ -12,7 +12,4 @@ # class SystemHook < WebHook - def async_execute(data) - Sidekiq::Client.enqueue(SystemHookWorker, id, data) - end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 1d53f3baf33..6043bac672c 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -7,10 +7,14 @@ class SystemHooksService def self.execute_hooks(data) SystemHook.all.each do |sh| - sh.async_execute data + async_execute_hook sh, data end end + def self.async_execute_hook(hook, data) + Sidekiq::Client.enqueue(SystemHookWorker, hook, data) + end + def self.build_event_data(model, event) data = { event_name: build_event_name(model, event), From c77730dd71b31d70a19f1729c795a44d7e609eb0 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Thu, 21 Feb 2013 14:04:06 +0400 Subject: [PATCH 4/4] An Id must be sended to queue --- app/services/system_hooks_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 6043bac672c..132bb14a675 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -12,7 +12,7 @@ class SystemHooksService end def self.async_execute_hook(hook, data) - Sidekiq::Client.enqueue(SystemHookWorker, hook, data) + Sidekiq::Client.enqueue(SystemHookWorker, hook.id, data) end def self.build_event_data(model, event)