Improve SystemNote interface for branch add/restore case

4 files changed, 19 insertions, 15 deletions

diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 8139aef2bb9..fd15889343e 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -7,11 +7,11 @@ module MergeRequests
       @branch_name = Gitlab::Git.ref_name(ref)
       @fork_merge_requests = @project.fork_merge_requests.opened
       @commits = []
-      @merge_requests = merge_requests_for_branch
+      @source_merge_requests = merge_requests_for_source_branch
 
       # Leave a system note if a branch were deleted/added
       if Gitlab::Git.blank_ref?(oldrev) or Gitlab::Git.blank_ref?(newrev)
-        presence = Gitlab::Git.blank_ref?(oldrev) ? 'added' : 'deleted'
+        presence = Gitlab::Git.blank_ref?(oldrev) ? :add : :delete
         comment_mr_branch_presence_changed(presence)
       else
         @commits = @project.repository.commits_between(oldrev, newrev)
@@ -81,18 +81,16 @@ module MergeRequests
 
     # Add comment about branches being deleted or added to merge requests
     def comment_mr_branch_presence_changed(presence)
-      merge_requests = merge_requests_for_branch
-
-      merge_requests.each do |merge_request|
+      @source_merge_requests.each do |merge_request|
         SystemNoteService.change_branch_presence(
             merge_request, merge_request.project, @current_user,
-            'source', @branch_name, presence)
+            :source, @branch_name, presence)
       end
     end
 
     # Add comment about pushing new commits to merge requests
     def comment_mr_with_commits
-      @merge_requests.each do |merge_request|
+      @source_merge_requests.each do |merge_request|
         mr_commit_ids = Set.new(merge_request.commits.map(&:id))
 
         new_commits, existing_commits = @commits.partition do |commit|
@@ -107,7 +105,7 @@ module MergeRequests
 
     # Call merge request webhook with update branches
     def execute_mr_web_hooks
-      @merge_requests.each do |merge_request|
+      @source_merge_requests.each do |merge_request|
         execute_hooks(merge_request, 'update')
       end
     end
@@ -116,7 +114,7 @@ module MergeRequests
       merge_requests.uniq.select(&:source_project)
     end
 
-    def merge_requests_for_branch
+    def merge_requests_for_source_branch
       merge_requests = @project.origin_merge_requests.opened.where(source_branch: @branch_name).to_a
       merge_requests += @fork_merge_requests.where(source_branch: @branch_name).to_a
       filter_merge_requests(merge_requests)
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 24e06504078..c0a5c3aeb53 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -173,9 +173,9 @@ class SystemNoteService
   # noteable    - Noteable object
   # project     - Project owning noteable
   # author      - User performing the change
-  # branch_type - 'source' or 'target'
+  # branch_type - :source or :target
   # branch      - branch name
-  # presence    - 'deleted' or 'created'
+  # presence    - :add or :delete
   #
   # Example Note text:
   #
@@ -183,7 +183,13 @@ class SystemNoteService
   #
   # Returns the created Note object
   def self.change_branch_presence(noteable, project, author, branch_type, branch, presence)
-    body = "#{branch_type} branch `#{branch}` #{presence}".capitalize
+    verb =
+      if presence == :add
+        'restored'
+      else
+        'deleted'
+      end
+    body = "#{branch_type.to_s} branch `#{branch}` #{verb}".capitalize
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
 
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index edb9fd0b43c..41eb5d41b2e 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -121,7 +121,7 @@ describe MergeRequests::RefreshService do
 
       it { expect(@merge_request.notes).to be_empty }
       it { expect(@merge_request).to be_open }
-      it { expect(@fork_merge_request.notes.last.note).to include('Source branch `master` added') }
+      it { expect(@fork_merge_request.notes.last.note).to include('Source branch `master` restored') }
       it { expect(@fork_merge_request).to be_open }
     end
 
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 108bc5995df..16b1c66ff9a 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -229,7 +229,7 @@ describe SystemNoteService do
   end
 
   describe '.change_branch' do
-    subject { described_class.change_branch(noteable, project, author, 'target', old_branch, new_branch) }
+    subject { described_class.change_branch(noteable, project, author, :target, old_branch, new_branch) }
     let(:old_branch) { 'old_branch'}
     let(:new_branch) { 'new_branch'}
 
@@ -243,7 +243,7 @@ describe SystemNoteService do
   end
 
   describe '.change_branch_presence' do
-    subject { described_class.change_branch_presence(noteable, project, author, 'source', 'feature', 'deleted') }
+    subject { described_class.change_branch_presence(noteable, project, author, 'source', 'feature', :delete) }
 
     it_behaves_like 'a system note'