Resolve outdated diff discussions on push

7 files changed, 40 insertions, 24 deletions

diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 8c5e258f519..083ace65b09 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -146,4 +146,8 @@ module NotesHelper
       autocomplete: autocomplete
     }
   end
+
+  def discussion_resolved_intro(discussion)
+    discussion.resolved_by_push? ? 'Automatically resolved' : 'Resolved'
+  end
 end
diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb
index dd979e7bb17..f006a271327 100644
--- a/app/models/concerns/resolvable_discussion.rb
+++ b/app/models/concerns/resolvable_discussion.rb
@@ -24,6 +24,7 @@ module ResolvableDiscussion
 
     delegate  :resolved_at,
               :resolved_by,
+              :resolved_by_push?,
 
               to: :last_resolved_note,
               allow_nil: true
diff --git a/app/models/concerns/resolvable_note.rb b/app/models/concerns/resolvable_note.rb
index 05eb6f86704..6f39029ff5a 100644
--- a/app/models/concerns/resolvable_note.rb
+++ b/app/models/concerns/resolvable_note.rb
@@ -51,22 +51,30 @@ module ResolvableNote
   end
 
   # If you update this method remember to also update `.resolve!`
-  def resolve!(current_user)
-    return unless resolvable?
-    return if resolved?
+  def resolve_without_save(current_user, resolved_by_push: false)
+    return false unless resolvable?
+    return false if resolved?
 
     self.resolved_at = Time.now
     self.resolved_by = current_user
-    save!
+    self.resolved_by_push = resolved_by_push
   end
 
   # If you update this method remember to also update `.unresolve!`
-  def unresolve!
-    return unless resolvable?
-    return unless resolved?
+  def unresolve_without_save(current_user)
+    return false unless resolvable?
+    return false unless resolved?
 
     self.resolved_at = nil
     self.resolved_by = nil
-    save!
+  end
+
+  def resolve!(current_user, resolved_by_push: false)
+    resolve_without_save(current_user, resolved_by_push: resolved_by_push) &&
+      save!
+  end
+
+  def unresolve!
+    unresolve_without_save(current_user) && save
   end
 end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 724fb4ccef1..b82f49d7073 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -918,6 +918,12 @@ class MergeRequest < ActiveRecord::Base
     active_diff_discussions.each do |discussion|
       service.execute(discussion)
     end
+
+    if project.resolve_outdated_diff_discussions?
+      MergeRequests::ResolvedDiscussionNotificationService
+        .new(project, current_user)
+        .execute(self)
+    end
   end
 
   def keep_around_commit
diff --git a/app/services/discussions/update_diff_position_service.rb b/app/services/discussions/update_diff_position_service.rb
index 1ef8d9edbe1..746f209e20f 100644
--- a/app/services/discussions/update_diff_position_service.rb
+++ b/app/services/discussions/update_diff_position_service.rb
@@ -10,6 +10,10 @@ module Discussions
       discussion.notes.each do |note|
         if outdated
           note.change_position = position
+
+          if project.resolve_outdated_diff_discussions?
+            note.resolve_without_save(current_user, resolved_by_push: true)
+          end
         else
           note.position = position
           note.change_position = nil
diff --git a/app/views/discussions/_headline.html.haml b/app/views/discussions/_headline.html.haml
index a9fe805bfa2..b865eb815f0 100644
--- a/app/views/discussions/_headline.html.haml
+++ b/app/views/discussions/_headline.html.haml
@@ -1,19 +1,12 @@
 - if discussion.resolved?
-  - if project.resolve_outdated_diff_discussions
-    .discussion-headline-light.js-discussion-headline
-      Automatically resolved
-      - if discussion.resolved_by
-        by
-        = link_to_member(@project, discussion.resolved_by, avatar: false)
-        with a push
-      = time_ago_with_tooltip(discussion.resolved_at, placement: "bottom")
-  - else
-    .discussion-headline-light.js-discussion-headline
-      Resolved
-      - if discussion.resolved_by
-        by
-        = link_to_member(@project, discussion.resolved_by, avatar: false)
-      = time_ago_with_tooltip(discussion.resolved_at, placement: "bottom")
+  .discussion-headline-light.js-discussion-headline
+    = discussion_resolved_intro(discussion)
+    - if discussion.resolved_by
+      by
+      = link_to_member(@project, discussion.resolved_by, avatar: false)
+    - if discussion.resolved_by_push?
+      with a push
+    = time_ago_with_tooltip(discussion.resolved_at, placement: "bottom")
 - elsif discussion.updated?
   .discussion-headline-light.js-discussion-headline
     Last updated
diff --git a/app/views/projects/_merge_request_merge_settings.html.haml b/app/views/projects/_merge_request_merge_settings.html.haml
index c1cb3b07d8e..e3effe45d6c 100644
--- a/app/views/projects/_merge_request_merge_settings.html.haml
+++ b/app/views/projects/_merge_request_merge_settings.html.haml
@@ -16,7 +16,7 @@
   .checkbox
     = form.label :resolve_outdated_diff_discussions do
       = form.check_box :resolve_outdated_diff_discussions
-      %strong Automatically resolve merge request diffs discussions on lines changed with a push
+      %strong Automatically resolve merge request diff discussions when they become outdated
   .checkbox
     = form.label :printing_merge_request_link_enabled do
       = form.check_box :printing_merge_request_link_enabled