Commit 61611ad2 authored by Douwe Maan's avatar Douwe Maan

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
parents 7e04ee9a af7ce322
...@@ -20,6 +20,7 @@ v 8.11.0 (unreleased) ...@@ -20,6 +20,7 @@ v 8.11.0 (unreleased)
- Optimize checking if a user has read access to a list of issues !5370 - Optimize checking if a user has read access to a list of issues !5370
- Nokogiri's various parsing methods are now instrumented - 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 - 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) - Add build event color in HipChat messages (David Eisner)
- Make fork counter always clickable. !5463 (winniehell) - Make fork counter always clickable. !5463 (winniehell)
- All created issues, API or WebUI, can be submitted to Akismet for spam check !5333 - All created issues, API or WebUI, can be submitted to Akismet for spam check !5333
......
...@@ -17,16 +17,19 @@ module MergeRequests ...@@ -17,16 +17,19 @@ module MergeRequests
end end
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 = merge_request.to_hook_data(current_user)
hook_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(merge_request) hook_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(merge_request)
hook_data[:object_attributes][:action] = action hook_data[:object_attributes][:action] = action
if oldrev && !Gitlab::Git.blank_ref?(oldrev)
hook_data[:object_attributes][:oldrev] = oldrev
end
hook_data hook_data
end end
def execute_hooks(merge_request, action = 'open') def execute_hooks(merge_request, action = 'open', oldrev = nil)
if merge_request.project 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_hooks(merge_data, :merge_request_hooks)
merge_request.project.execute_services(merge_data, :merge_request_hooks) merge_request.project.execute_services(merge_data, :merge_request_hooks)
end end
......
...@@ -137,7 +137,7 @@ module MergeRequests ...@@ -137,7 +137,7 @@ module MergeRequests
# Call merge request webhook with update branches # Call merge request webhook with update branches
def execute_mr_web_hooks def execute_mr_web_hooks
merge_requests_for_source_branch.each do |merge_request| merge_requests_for_source_branch.each do |merge_request|
execute_hooks(merge_request, 'update') execute_hooks(merge_request, 'update', @oldrev)
end end
end end
......
...@@ -57,7 +57,7 @@ describe MergeRequests::RefreshService, services: true do ...@@ -57,7 +57,7 @@ describe MergeRequests::RefreshService, services: true do
it 'should execute hooks with update action' do it 'should execute hooks with update action' do
expect(refresh_service).to have_received(:execute_hooks). expect(refresh_service).to have_received(:execute_hooks).
with(@merge_request, 'update') with(@merge_request, 'update', @oldrev)
end end
it { expect(@merge_request.notes).not_to be_empty } it { expect(@merge_request.notes).not_to be_empty }
...@@ -113,7 +113,7 @@ describe MergeRequests::RefreshService, services: true do ...@@ -113,7 +113,7 @@ describe MergeRequests::RefreshService, services: true do
it 'should execute hooks with update action' do it 'should execute hooks with update action' do
expect(refresh_service).to have_received(:execute_hooks). expect(refresh_service).to have_received(:execute_hooks).
with(@fork_merge_request, 'update') with(@fork_merge_request, 'update', @oldrev)
end end
it { expect(@merge_request.notes).to be_empty } it { expect(@merge_request.notes).to be_empty }
...@@ -158,7 +158,7 @@ describe MergeRequests::RefreshService, services: true do ...@@ -158,7 +158,7 @@ describe MergeRequests::RefreshService, services: true do
it 'refreshes the merge request' do it 'refreshes the merge request' do
expect(refresh_service).to receive(:execute_hooks). 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) allow_any_instance_of(Repository).to receive(:merge_base).and_return(@oldrev)
refresh_service.execute(Gitlab::Git::BLANK_SHA, @newrev, 'refs/heads/master') refresh_service.execute(Gitlab::Git::BLANK_SHA, @newrev, 'refs/heads/master')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment