Move queries to models

- Fix rubocop offences

app/mailers/emails/reviews.rb

@@ -22,8 +22,7 @@ module Emails
       review = Review.find_by_id(review_id)
 
       @notes = review.notes
-      discussion_ids = @notes.pluck(:discussion_id)
-      @discussions = discussions(discussion_ids)
+      @discussions = Discussion.build_discussions(review.discussion_ids, preload_note_diff_file: true)
       @author = review.author
       @author_name = review.author_name
       @project = review.project
@@ -31,11 +30,5 @@ module Emails
       @target_url = project_merge_request_url(@project, @merge_request)
       @sent_notification = SentNotification.record(@merge_request, recipient_id, reply_key)
     end
-
-    def discussions(discussion_ids)
-      notes = Note.where(discussion_id: discussion_ids).inc_note_diff_file.fresh
-      grouped_notes = notes.group_by { |n| n.discussion_id }
-      grouped_notes.transform_values { |notes| Discussion.build(notes) }
-    end
   end
 end

app/models/discussion.rb

@@ -47,6 +47,14 @@ class Discussion
     grouped_notes.values.map { |notes| build(notes, context_noteable) }
   end
 
+  def self.build_discussions(discussion_ids, context_noteable = nil, preload_note_diff_file: false)
+    notes = Note.where(discussion_id: discussion_ids).fresh
+    notes = notes.inc_note_diff_file if preload_note_diff_file
+
+    grouped_notes = notes.group_by { |n| n.discussion_id }
+    grouped_notes.transform_values { |notes| Discussion.build(notes, context_noteable) }
+  end
+
   def self.lazy_find(discussion_id)
     BatchLoader.for(discussion_id).batch do |discussion_ids, loader|
       results = Note.where(discussion_id: discussion_ids).fresh.to_a.group_by(&:discussion_id)

app/models/review.rb

@@ -14,6 +14,10 @@ class Review < ApplicationRecord
 
   participant :author
 
+  def discussion_ids
+    notes.pluck(:discussion_id)
+  end
+
   def all_references(current_user = nil, extractor: nil)
     ext = super
 

app/views/notify/_note_email.html.haml

@@ -4,7 +4,7 @@
 - note_style = local_assigns.fetch(:note_style, "")
 - skip_stylesheet_link = local_assigns.fetch(:skip_stylesheet_link, false)
 
-- discussion = local_assigns.fetch(:discussion){ note.discussion } if note.part_of_discussion?
+- discussion = local_assigns.fetch(:discussion) { note.discussion } if note.part_of_discussion?
 
 %p{ style: "color: #777777;" }
   = succeed ':' do

app/views/notify/_note_email.text.erb

 <% note = local_assigns.fetch(:note, @note) -%>
 <% diff_limit = local_assigns.fetch(:diff_limit, nil) -%>
 <% target_url = local_assigns.fetch(:target_url, @target_url) -%>
-<% discussion = local_assigns.fetch(:discussion){ note.discussion } if note.part_of_discussion? -%>
+<% discussion = local_assigns.fetch(:discussion) { note.discussion } if note.part_of_discussion? -%>
 
 <%= sanitize_name(note.author_name) -%>
 <%  if discussion.nil? -%>

spec/mailers/notify_spec.rb

@@ -2182,7 +2182,6 @@ RSpec.describe Notify do
       let!(:notes) { create_list(:diff_note_on_merge_request, 3, review: review, project: project, author: review.author, noteable: merge_request) }
 
       it 'links to notes and discussions', :aggregate_failures do
-
         reply_note = create(:diff_note_on_merge_request, review: review, project: project, author: review.author, noteable: merge_request, in_reply_to: notes.first)
 
         review.notes.each do |note|