From ac7202754664986e115329813b6f23298c271c6f Mon Sep 17 00:00:00 2001
From: nmilojevic1 <nmilojevic@gitlab.com>
Date: Fri, 1 Apr 2022 11:35:56 +0200
Subject: [PATCH] Move queries to models
- Fix rubocop offences
---
app/mailers/emails/reviews.rb | 9 +--------
app/models/discussion.rb | 8 ++++++++
app/models/review.rb | 4 ++++
app/views/notify/_note_email.html.haml | 2 +-
app/views/notify/_note_email.text.erb | 2 +-
spec/mailers/notify_spec.rb | 1 -
6 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/app/mailers/emails/reviews.rb b/app/mailers/emails/reviews.rb
index 5bbea1a5fe0c1..48c1b5b057cc5 100644
--- a/app/mailers/emails/reviews.rb
+++ b/app/mailers/emails/reviews.rb
@@ -22,8 +22,7 @@ def setup_review_email(review_id, recipient_id)
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 @@ def setup_review_email(review_id, recipient_id)
@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
diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 8a167034629fe..9eb3308b9012b 100644
--- a/app/models/discussion.rb
+++ b/app/models/discussion.rb
@@ -47,6 +47,14 @@ def self.build_collection(notes, context_noteable = nil)
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)
diff --git a/app/models/review.rb b/app/models/review.rb
index 5a30e2963c8f0..6456951347d96 100644
--- a/app/models/review.rb
+++ b/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
diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml
index 7b7a7d5b995cc..d3d19c3388eb2 100644
--- a/app/views/notify/_note_email.html.haml
+++ b/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
diff --git a/app/views/notify/_note_email.text.erb b/app/views/notify/_note_email.text.erb
index 8b8b75cb40dca..ab510f4993dae 100644
--- a/app/views/notify/_note_email.text.erb
+++ b/app/views/notify/_note_email.text.erb
@@ -1,7 +1,7 @@
<% 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? -%>
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 8aefaadae2e78..b6ad66f41b50b 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -2182,7 +2182,6 @@ def invite_to_group(group, inviter:, user: nil, tasks_to_be_done: [])
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|
--
GitLab