diff options
-rw-r--r-- | CHANGELOG | 2 | ||||
-rw-r--r-- | app/services/merge_requests/base_service.rb | 5 | ||||
-rw-r--r-- | app/services/merge_requests/close_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/reopen_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/update_service.rb | 2 | ||||
-rw-r--r-- | doc/web_hooks/web_hooks.md | 4 | ||||
-rw-r--r-- | spec/services/merge_requests/close_service_spec.rb | 15 | ||||
-rw-r--r-- | spec/services/merge_requests/create_service_spec.rb | 19 | ||||
-rw-r--r-- | spec/services/merge_requests/merge_service_spec.rb | 44 | ||||
-rw-r--r-- | spec/services/merge_requests/reopen_service_spec.rb | 45 | ||||
-rw-r--r-- | spec/services/merge_requests/update_service_spec.rb | 18 |
12 files changed, 141 insertions, 19 deletions
diff --git a/CHANGELOG b/CHANGELOG index 9eb68042554..6ac6e5b4fb9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -42,7 +42,7 @@ v 7.8.0 - - - - - + - Add action property to merge request hook (Julien Bianchi) - - - diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 7f3421b8e4b..b4199d1c800 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -5,9 +5,12 @@ module MergeRequests Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) end - def execute_hooks(merge_request) + def execute_hooks(merge_request, action = 'open') if merge_request.project hook_data = merge_request.to_hook_data(current_user) + merge_request_url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id) + hook_data[:object_attributes][:url] = merge_request_url + hook_data[:object_attributes][:action] = action merge_request.project.execute_hooks(hook_data, :merge_request_hooks) end end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index 64e37a23e6b..4249a84f382 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -9,7 +9,7 @@ module MergeRequests event_service.close_mr(merge_request, current_user) notification_service.close_mr(merge_request, current_user) create_note(merge_request) - execute_hooks(merge_request) + execute_hooks(merge_request, 'close') end merge_request diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 5de7247d617..1e1614028f7 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -12,7 +12,7 @@ module MergeRequests notification_service.merge_mr(merge_request, current_user) create_merge_event(merge_request, current_user) create_note(merge_request) - execute_hooks(merge_request) + execute_hooks(merge_request, 'merge') true rescue diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index bd68919a550..a2a9c933f63 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -5,7 +5,7 @@ module MergeRequests event_service.reopen_mr(merge_request, current_user) notification_service.reopen_mr(merge_request, current_user) create_note(merge_request) - execute_hooks(merge_request) + execute_hooks(merge_request, 'reopen') merge_request.reload_code merge_request.mark_as_unchecked end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index fc26619cd17..56c8510e0ae 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -38,7 +38,7 @@ module MergeRequests end merge_request.notice_added_references(merge_request.project, current_user) - execute_hooks(merge_request) + execute_hooks(merge_request, 'update') end merge_request diff --git a/doc/web_hooks/web_hooks.md b/doc/web_hooks/web_hooks.md index e17d21b990d..e3399e5f1b8 100644 --- a/doc/web_hooks/web_hooks.md +++ b/doc/web_hooks/web_hooks.md @@ -166,7 +166,9 @@ Triggered when a new merge request is created or an existing merge request was u "name": "GitLab dev user", "email": "gitlabdev@dv6700.(none)" } - } + }, + "url": "http://example.com/diaspora/merge_requests/1", + "action": "open" } } ``` diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index a504f916b08..5060a67bebf 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -12,14 +12,23 @@ describe MergeRequests::CloseService do end describe :execute do - context "valid params" do + context 'valid params' do + let(:service) { MergeRequests::CloseService.new(project, user, {}) } + before do - @merge_request = MergeRequests::CloseService.new(project, user, {}).execute(merge_request) + service.stub(:execute_hooks) + + @merge_request = service.execute(merge_request) end it { @merge_request.should be_valid } it { @merge_request.should be_closed } + it 'should execute hooks with close action' do + expect(service).to have_received(:execute_hooks). + with(@merge_request, 'close') + end + it 'should send email to user2 about assign of new merge_request' do email = ActionMailer::Base.deliveries.last email.to.first.should == user2.email @@ -28,7 +37,7 @@ describe MergeRequests::CloseService do it 'should create system note about merge_request reassign' do note = @merge_request.notes.last - note.note.should include "Status changed to closed" + note.note.should include 'Status changed to closed' end end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index cebeb0644d0..dbd21143690 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -5,21 +5,30 @@ describe MergeRequests::CreateService do let(:user) { create(:user) } describe :execute do - context "valid params" do - before do - project.team << [user, :master] - opts = { + context 'valid params' do + let(:opts) do + { title: 'Awesome merge_request', description: 'please fix', source_branch: 'stable', target_branch: 'master' } + end + let(:service) { MergeRequests::CreateService.new(project, user, opts) } + + before do + project.team << [user, :master] + service.stub(:execute_hooks) - @merge_request = MergeRequests::CreateService.new(project, user, opts).execute + @merge_request = service.execute end it { @merge_request.should be_valid } it { @merge_request.title.should == 'Awesome merge_request' } + + it 'should execute hooks with default action' do + expect(service).to have_received(:execute_hooks).with(@merge_request) + end end end end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb new file mode 100644 index 00000000000..5f61fd3187b --- /dev/null +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe MergeRequests::MergeService do + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:merge_request) { create(:merge_request, assignee: user2) } + let(:project) { merge_request.project } + + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + + describe :execute do + context 'valid params' do + let(:service) { MergeRequests::MergeService.new(project, user, {}) } + + before do + service.stub(:execute_hooks) + + service.execute(merge_request, 'Awesome message') + end + + it { merge_request.should be_valid } + it { merge_request.should be_merged } + + it 'should execute hooks with merge action' do + expect(service).to have_received(:execute_hooks). + with(merge_request, 'merge') + end + + it 'should send email to user2 about merge of new merge_request' do + email = ActionMailer::Base.deliveries.last + email.to.first.should == user2.email + email.subject.should include(merge_request.title) + end + + it 'should create system note about merge_request merge' do + note = merge_request.notes.last + note.note.should include 'Status changed to merged' + end + end + end +end diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb new file mode 100644 index 00000000000..2a7066124dc --- /dev/null +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe MergeRequests::ReopenService do + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:merge_request) { create(:merge_request, assignee: user2) } + let(:project) { merge_request.project } + + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + + describe :execute do + context 'valid params' do + let(:service) { MergeRequests::ReopenService.new(project, user, {}) } + + before do + service.stub(:execute_hooks) + + merge_request.state = :closed + service.execute(merge_request) + end + + it { merge_request.should be_valid } + it { merge_request.should be_reopened } + + it 'should execute hooks with reopen action' do + expect(service).to have_received(:execute_hooks). + with(merge_request, 'reopen') + end + + it 'should send email to user2 about reopen of merge_request' do + email = ActionMailer::Base.deliveries.last + email.to.first.should == user2.email + email.subject.should include(merge_request.title) + end + + it 'should create system note about merge_request reopen' do + note = merge_request.notes.last + note.note.should include 'Status changed to reopened' + end + end + end +end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index af5d3a3dc81..c8f40f48bab 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -12,16 +12,21 @@ describe MergeRequests::UpdateService do end describe :execute do - context "valid params" do - before do - opts = { + context 'valid params' do + let(:opts) do + { title: 'New title', description: 'Also please fix', assignee_id: user2.id, state_event: 'close' } + end + let(:service) { MergeRequests::UpdateService.new(project, user, opts) } + + before do + service.stub(:execute_hooks) - @merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + @merge_request = service.execute(merge_request) end it { @merge_request.should be_valid } @@ -29,6 +34,11 @@ describe MergeRequests::UpdateService do it { @merge_request.assignee.should == user2 } it { @merge_request.should be_closed } + it 'should execute hooks with update action' do + expect(service).to have_received(:execute_hooks). + with(@merge_request, 'update') + end + it 'should send email to user2 about assign of new merge_request' do email = ActionMailer::Base.deliveries.last email.to.first.should == user2.email |