Optimize discussion notes resolving and unresolving21109-discussion-resolve-runs-a-single-update-query-per-note-but-should-run-a-single-update-query-for-all-notes-instead

Use `update_all` to only require one query per discussion to
update the notes resolved status. Some changes had to be made to
the discussion spec to accout for the fact that notes are not
individually updated now

1 files changed, 27 insertions, 12 deletions

diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 9676bc03470..de06c13481a 100644
--- a/app/models/discussion.rb
+++ b/app/models/discussion.rb
@@ -1,7 +1,7 @@
 class Discussion
   NUMBER_OF_TRUNCATED_DIFF_LINES = 16
 
-  attr_reader :first_note, :last_note, :notes
+  attr_reader :notes
 
   delegate  :created_at,
             :project,
@@ -36,8 +36,6 @@ class Discussion
   end
 
   def initialize(notes)
-    @first_note = notes.first
-    @last_note = notes.last
     @notes = notes
   end
 
@@ -70,17 +68,25 @@ class Discussion
   end
 
   def resolvable?
-    return @resolvable if defined?(@resolvable)
+    return @resolvable if @resolvable.present?
 
     @resolvable = diff_discussion? && notes.any?(&:resolvable?)
   end
 
   def resolved?
-    return @resolved if defined?(@resolved)
+    return @resolved if @resolved.present?
 
     @resolved = resolvable? && notes.none?(&:to_be_resolved?)
   end
 
+  def first_note
+    @first_note ||= @notes.first
+  end
+
+  def last_note
+    @last_note ||= @notes.last
+  end
+
   def resolved_notes
     notes.select(&:resolved?)
   end
@@ -100,17 +106,13 @@ class Discussion
   def resolve!(current_user)
     return unless resolvable?
 
-    notes.each do |note|
-      note.resolve!(current_user) if note.resolvable?
-    end
+    update { |notes| notes.resolve!(current_user) }
   end
 
   def unresolve!
     return unless resolvable?
 
-    notes.each do |note|
-      note.unresolve! if note.resolvable?
-    end
+    update { |notes| notes.unresolve! }
   end
 
   def for_target?(target)
@@ -118,7 +120,7 @@ class Discussion
   end
 
   def active?
-    return @active if defined?(@active)
+    return @active if @active.present?
 
     @active = first_note.active?
   end
@@ -174,4 +176,17 @@ class Discussion
 
     prev_lines
   end
+
+  private
+
+  def update
+    notes_relation = DiffNote.where(id: notes.map(&:id)).fresh
+    yield(notes_relation)
+
+    # Set the notes array to the updated notes
+    @notes = notes_relation.to_a
+
+    # Reset the memoized values
+    @last_resolved_note = @resolvable = @resolved = @first_note = @last_note = nil
+  end
 end