summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-08-01 19:58:26 +0000
committerDouwe Maan <douwe@gitlab.com>2016-08-01 19:58:26 +0000
commit61611ad24f6d800ab64fdb4f21125e3dfb2aada5 (patch)
treefb57066529ea28b3e5223a57228514d177bf2dbc
parent7e04ee9af87948eabc9d4f9bb8b738356f111768 (diff)
parentaf7ce322bdbaf74eaf54eac92c2ed5183e0d8e9c (diff)
downloadgitlab-ce-61611ad24f6d800ab64fdb4f21125e3dfb2aada5.tar.gz
Merge branch 'oldrev-in-update-hooks' into 'master'
webhooks: include old revision in MR update events ## What does this MR do? Includes the old revision for an MR in webhooks. ## Are there points in the code the reviewer needs to double check? The docs do not give an example `update` hook; should I add one? ## Why was this MR needed? In order to keep web hook listeners stateless, it helps to include the previous revision of the MR in the hook so that it does not need to remember the last revision (which may also be wrong in case of missed or out-of-order events due to previous errors). ## What are the relevant issue numbers? N/A ## Screenshots (if relevant) N/A ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [x] API support added - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !5535
-rw-r--r--CHANGELOG1
-rw-r--r--app/services/merge_requests/base_service.rb9
-rw-r--r--app/services/merge_requests/refresh_service.rb2
-rw-r--r--spec/services/merge_requests/refresh_service_spec.rb6
4 files changed, 11 insertions, 7 deletions
diff --git a/CHANGELOG b/CHANGELOG
index e46befcec2a..7b5b70911ae 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -20,6 +20,7 @@ v 8.11.0 (unreleased)
- Optimize checking if a user has read access to a list of issues !5370
- Nokogiri's various parsing methods are now instrumented
- Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363
+ - Include old revision in merge request update hooks (Ben Boeckel)
- Add build event color in HipChat messages (David Eisner)
- Make fork counter always clickable. !5463 (winniehell)
- All created issues, API or WebUI, can be submitted to Akismet for spam check !5333
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index bc3606a14c2..ba424b09463 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -17,16 +17,19 @@ module MergeRequests
end
end
- def hook_data(merge_request, action)
+ def hook_data(merge_request, action, oldrev = nil)
hook_data = merge_request.to_hook_data(current_user)
hook_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(merge_request)
hook_data[:object_attributes][:action] = action
+ if oldrev && !Gitlab::Git.blank_ref?(oldrev)
+ hook_data[:object_attributes][:oldrev] = oldrev
+ end
hook_data
end
- def execute_hooks(merge_request, action = 'open')
+ def execute_hooks(merge_request, action = 'open', oldrev = nil)
if merge_request.project
- merge_data = hook_data(merge_request, action)
+ merge_data = hook_data(merge_request, action, oldrev)
merge_request.project.execute_hooks(merge_data, :merge_request_hooks)
merge_request.project.execute_services(merge_data, :merge_request_hooks)
end
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 1daf6bbf553..5cedd6f11d9 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -137,7 +137,7 @@ module MergeRequests
# Call merge request webhook with update branches
def execute_mr_web_hooks
merge_requests_for_source_branch.each do |merge_request|
- execute_hooks(merge_request, 'update')
+ execute_hooks(merge_request, 'update', @oldrev)
end
end
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index ce643b3f860..781ee7ffed3 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -57,7 +57,7 @@ describe MergeRequests::RefreshService, services: true do
it 'should execute hooks with update action' do
expect(refresh_service).to have_received(:execute_hooks).
- with(@merge_request, 'update')
+ with(@merge_request, 'update', @oldrev)
end
it { expect(@merge_request.notes).not_to be_empty }
@@ -113,7 +113,7 @@ describe MergeRequests::RefreshService, services: true do
it 'should execute hooks with update action' do
expect(refresh_service).to have_received(:execute_hooks).
- with(@fork_merge_request, 'update')
+ with(@fork_merge_request, 'update', @oldrev)
end
it { expect(@merge_request.notes).to be_empty }
@@ -158,7 +158,7 @@ describe MergeRequests::RefreshService, services: true do
it 'refreshes the merge request' do
expect(refresh_service).to receive(:execute_hooks).
- with(@fork_merge_request, 'update')
+ with(@fork_merge_request, 'update', Gitlab::Git::BLANK_SHA)
allow_any_instance_of(Repository).to receive(:merge_base).and_return(@oldrev)
refresh_service.execute(Gitlab::Git::BLANK_SHA, @newrev, 'refs/heads/master')