From 37ef33cba18f947699fc3f285397be34861ab51e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Jan 2014 14:26:05 +0200 Subject: Use instance methods for system_hooks_service Signed-off-by: Dmitriy Zaporozhets --- app/observers/system_hook_observer.rb | 10 ++++++++-- app/services/system_hooks_service.rb | 10 +++++----- spec/services/system_hooks_service_spec.rb | 4 ++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/observers/system_hook_observer.rb b/app/observers/system_hook_observer.rb index 3a649fd590d..80de177b9a2 100644 --- a/app/observers/system_hook_observer.rb +++ b/app/observers/system_hook_observer.rb @@ -2,10 +2,16 @@ class SystemHookObserver < BaseObserver observe :user, :project, :users_project def after_create(model) - SystemHooksService.execute_hooks_for(model, :create) + system_hook_service.execute_hooks_for(model, :create) end def after_destroy(model) - SystemHooksService.execute_hooks_for(model, :destroy) + system_hook_service.execute_hooks_for(model, :destroy) + end + + private + + def system_hook_service + SystemHooksService.new end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index de4fa9b79e7..4969198b8c2 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -1,21 +1,21 @@ class SystemHooksService - def self.execute_hooks_for(model, event) + def execute_hooks_for(model, event) execute_hooks(build_event_data(model, event)) end private - def self.execute_hooks(data) + def execute_hooks(data) SystemHook.all.each do |sh| async_execute_hook sh, data end end - def self.async_execute_hook(hook, data) + def async_execute_hook(hook, data) Sidekiq::Client.enqueue(SystemHookWorker, hook.id, data) end - def self.build_event_data(model, event) + def build_event_data(model, event) data = { event_name: build_event_name(model, event), created_at: model.created_at @@ -51,7 +51,7 @@ class SystemHooksService end end - def self.build_event_name(model, event) + def build_event_name(model, event) case model when UsersProject return "user_add_to_team" if event == :create diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index ebc1ed51d2e..f1df7e55dd0 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -24,10 +24,10 @@ describe SystemHooksService do end def event_data(*args) - SystemHooksService.build_event_data(*args) + SystemHooksService.new.send :build_event_data, *args end def event_name(*args) - SystemHooksService.build_event_name(*args) + SystemHooksService.new.send :build_event_name, *args end end -- cgit v1.2.1 From 8bfc62fb8b02bde7da97958deb8aeda63581bfce Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Jan 2014 14:38:50 +0200 Subject: Convert TestHookContext into TestHookService. Added tests Signed-off-by: Dmitriy Zaporozhets --- app/contexts/test_hook_context.rb | 7 ------- app/controllers/projects/hooks_controller.rb | 11 ++++++++--- app/services/test_hook_service.rb | 6 ++++++ spec/services/test_hook_service_spec.rb | 14 ++++++++++++++ 4 files changed, 28 insertions(+), 10 deletions(-) delete mode 100644 app/contexts/test_hook_context.rb create mode 100644 app/services/test_hook_service.rb create mode 100644 spec/services/test_hook_service_spec.rb diff --git a/app/contexts/test_hook_context.rb b/app/contexts/test_hook_context.rb deleted file mode 100644 index 63eda6c7d06..00000000000 --- a/app/contexts/test_hook_context.rb +++ /dev/null @@ -1,7 +0,0 @@ -class TestHookContext < BaseContext - def execute - hook = project.hooks.find(params[:id]) - data = GitPushService.new.sample_data(project, current_user) - hook.execute(data) - end -end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 314d87df034..a863b318324 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -24,15 +24,20 @@ class Projects::HooksController < Projects::ApplicationController end def test - TestHookContext.new(project, current_user, params).execute + TestHookService.new.execute(hook, current_user) redirect_to :back end def destroy - @hook = @project.hooks.find(params[:id]) - @hook.destroy + hook.destroy redirect_to project_hooks_path(@project) end + + private + + def hook + @hook ||= @project.hooks.find(params[:id]) + end end diff --git a/app/services/test_hook_service.rb b/app/services/test_hook_service.rb new file mode 100644 index 00000000000..17d86a7a274 --- /dev/null +++ b/app/services/test_hook_service.rb @@ -0,0 +1,6 @@ +class TestHookService + def execute(hook, current_user) + data = GitPushService.new.sample_data(hook.project, current_user) + hook.execute(data) + end +end diff --git a/spec/services/test_hook_service_spec.rb b/spec/services/test_hook_service_spec.rb new file mode 100644 index 00000000000..fbe9066096d --- /dev/null +++ b/spec/services/test_hook_service_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe TestHookService do + let (:user) { create :user } + let (:project) { create :project_with_code } + let (:hook) { create :project_hook, project: project } + + describe :execute do + it "should execute successfully" do + stub_request(:post, hook.url).to_return(status: 200) + TestHookService.new.execute(hook, user).should be_true + end + end +end -- cgit v1.2.1 From f19cdee8cc1e85066154d008ee2653d845c9f3cd Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Jan 2014 15:06:12 +0200 Subject: Remove commit_load_context.rb Signed-off-by: Dmitriy Zaporozhets --- app/contexts/commit_load_context.rb | 34 -------------------------- app/controllers/projects/commit_controller.rb | 35 +++++++++++++++------------ lib/api/repositories.rb | 6 ++--- 3 files changed, 23 insertions(+), 52 deletions(-) delete mode 100644 app/contexts/commit_load_context.rb diff --git a/app/contexts/commit_load_context.rb b/app/contexts/commit_load_context.rb deleted file mode 100644 index 0c684976b64..00000000000 --- a/app/contexts/commit_load_context.rb +++ /dev/null @@ -1,34 +0,0 @@ -class CommitLoadContext < BaseContext - def execute - result = { - commit: nil, - suppress_diff: false, - line_notes: [], - notes_count: 0, - note: nil, - status: :ok - } - - commit = project.repository.commit(params[:id]) - - if commit - line_notes = project.notes.for_commit_id(commit.id).inline - - result[:commit] = commit - result[:note] = project.build_commit_note(commit) - result[:line_notes] = line_notes - result[:notes_count] = project.notes.for_commit_id(commit.id).count - result[:branches] = project.repository.branch_names_contains(commit.id) - - begin - result[:suppress_diff] = true if commit.diff_suppress? && !params[:force_show_diff] - result[:force_suppress_diff] = commit.diff_force_suppress? - rescue Grit::Git::GitTimeout - result[:suppress_diff] = true - result[:status] = :huge_commit - end - end - - result - end -end diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 65b8a7283a7..687026ed360 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -6,30 +6,31 @@ class Projects::CommitController < Projects::ApplicationController before_filter :authorize_read_project! before_filter :authorize_code_access! before_filter :require_non_empty_project + before_filter :commit def show - result = CommitLoadContext.new(project, current_user, params).execute + return git_not_found! unless @commit - @commit = result[:commit] + @line_notes = project.notes.for_commit_id(commit.id).inline + @branches = project.repository.branch_names_contains(commit.id) - if @commit.nil? - git_not_found! - return + begin + @suppress_diff = true if commit.diff_suppress? && !params[:force_show_diff] + @force_suppress_diff = commit.diff_force_suppress? + rescue Grit::Git::GitTimeout + @suppress_diff = true + @status = :huge_commit end - @suppress_diff = result[:suppress_diff] - @force_suppress_diff = result[:force_suppress_diff] - - @note = result[:note] - @line_notes = result[:line_notes] - @branches = result[:branches] - @notes_count = result[:notes_count] + @note = project.build_commit_note(commit) + @notes_count = project.notes.for_commit_id(commit.id).count @notes = project.notes.for_commit_id(@commit.id).not_inline.fresh @noteable = @commit - @comments_allowed = @reply_allowed = true - @comments_target = { noteable_type: 'Commit', - commit_id: @commit.id } + @comments_target = { + noteable_type: 'Commit', + commit_id: @commit.id + } respond_to do |format| format.html do @@ -42,4 +43,8 @@ class Projects::CommitController < Projects::ApplicationController format.patch { render text: @commit.to_patch } end end + + def commit + @commit ||= project.repository.commit(params[:id]) + end end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index c99c8f7bdfb..af958f06c64 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -124,9 +124,9 @@ module API # GET /projects/:id/repository/commits/:sha/diff get ":id/repository/commits/:sha/diff" do sha = params[:sha] - result = CommitLoadContext.new(user_project, current_user, {id: sha}).execute - not_found! "Commit" unless result[:commit] - result[:commit].diffs + commit = user_project.repository.commit(sha) + not_found! "Commit" unless commit + commit.diffs end # Get a project repository tree -- cgit v1.2.1 From 7bcd112a297fede6ce58801ae462664a246d0442 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Jan 2014 15:31:49 +0200 Subject: Fix commit page Signed-off-by: Dmitriy Zaporozhets --- app/controllers/projects/commit_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 687026ed360..c56df65dcba 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -34,7 +34,7 @@ class Projects::CommitController < Projects::ApplicationController respond_to do |format| format.html do - if result[:status] == :huge_commit + if @status == :huge_commit render "huge_commit" and return end end -- cgit v1.2.1 From 4e2c34d81b46b97929ca1db3fe1c2a1f77b03552 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Jan 2014 16:38:48 +0200 Subject: Test hook properly Signed-off-by: Dmitriy Zaporozhets --- features/steps/project/project_hooks.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/features/steps/project/project_hooks.rb b/features/steps/project/project_hooks.rb index 36555fb8e8c..19ff3244543 100644 --- a/features/steps/project/project_hooks.rb +++ b/features/steps/project/project_hooks.rb @@ -1,9 +1,12 @@ +require 'webmock' + class ProjectHooks < Spinach::FeatureSteps include SharedAuthentication include SharedProject include SharedPaths include RSpec::Matchers include RSpec::Mocks::ExampleMethods + include WebMock::API Given 'project has hook' do @hook = create(:project_hook, project: current_project) @@ -25,8 +28,7 @@ class ProjectHooks < Spinach::FeatureSteps end When 'I click test hook button' do - test_hook_context = double(execute: true) - TestHookContext.should_receive(:new).and_return(test_hook_context) + stub_request(:post, @hook.url).to_return(status: 200) click_link 'Test Hook' end -- cgit v1.2.1