webhooks: include old revision in MR update events

4 files changed, 11 insertions, 7 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 39b77460deb..4285a548e64 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')