diff --git a/app/mailers/emails/reviews.rb b/app/mailers/emails/reviews.rb
index b98fa8aa6c9fa734c503085286bed9f547073d49..ed1166509a58bbfbf7fc9dc0c318b1f96e6fcc82 100644
--- a/app/mailers/emails/reviews.rb
+++ b/app/mailers/emails/reviews.rb
@@ -19,17 +19,16 @@ def review_thread_options(recipient_id)
end
def setup_review_email(review_id, recipient_id)
- review = Review.find_by_id(review_id)
-
- @notes = review.notes
- @discussions = Discussion.build_discussions(review.discussion_ids, preload_note_diff_file: true)
+ @review = Review.find_by_id(review_id)
+ @notes = @review.notes
+ @discussions = Discussion.build_discussions(@review.discussion_ids, preload_note_diff_file: true)
@include_diff_discussion_stylesheet = @discussions.values.any? do |discussion|
discussion.diff_discussion? && discussion.on_text?
end
- @author = review.author
- @author_name = review.author_name
- @project = review.project
- @merge_request = review.merge_request
+ @author = @review.author
+ @author_name = @review.author_name
+ @project = @review.project
+ @merge_request = @review.merge_request
@target_url = project_merge_request_url(@project, @merge_request)
@sent_notification = SentNotification.record(@merge_request, recipient_id, reply_key)
end
diff --git a/app/mailers/previews/notify_preview.rb b/app/mailers/previews/notify_preview.rb
index 576dbdd8b52a7b53ff51fc28cba703dfb3500bb3..93d4625c34421186738da8adce70732db6e947a6 100644
--- a/app/mailers/previews/notify_preview.rb
+++ b/app/mailers/previews/notify_preview.rb
@@ -284,6 +284,13 @@ def request_review_merge_request_email
Notify.request_review_merge_request_email(user.id, merge_request.id, user.id).message
end
+ def new_review_email
+ review = Review.last
+ mr_author = review.merge_request.author
+
+ Notify.new_review_email(mr_author.id, review.id).message
+ end
+
def project_was_moved_email
Notify.project_was_moved_email(project.id, user.id, "gitlab/gitlab").message
end
diff --git a/app/models/review.rb b/app/models/review.rb
index c621da3b03c549f17a55ddc072d2add2213e99f0..d47aaf027ce7cbe0360fb0d1a0c64843c97b5c36 100644
--- a/app/models/review.rb
+++ b/app/models/review.rb
@@ -32,3 +32,5 @@ def user_mentions
merge_request.user_mentions.where.not(note_id: nil)
end
end
+
+Review.prepend_mod
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index b93b44ce7970538e63c40dfc765f9abe0d9a59e7..d305c8c03cfd4846bfaf25b9034a71e987431b98 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -723,9 +723,12 @@ def group_was_not_exported(group, current_user, errors)
# Notify users on new review in system
def new_review(review)
recipients = NotificationRecipients::BuildService.build_new_review_recipients(review)
+ deliver_options = new_review_deliver_options(review)
recipients.each do |recipient|
- mailer.new_review_email(recipient.user.id, review.id).deliver_later
+ mailer
+ .new_review_email(recipient.user.id, review.id)
+ .deliver_later(deliver_options)
end
end
@@ -946,6 +949,11 @@ def deliver_access_request_email(recipient, member)
def warn_skipping_notifications(user, object)
Gitlab::AppLogger.warn(message: "Skipping sending notifications", user: user.id, klass: object.class.to_s, object_id: object.id)
end
+
+ def new_review_deliver_options(review)
+ # Overridden in EE
+ {}
+ end
end
NotificationService.prepend_mod_with('NotificationService')
diff --git a/app/views/notify/new_review_email.html.haml b/app/views/notify/new_review_email.html.haml
index afc1bd682156baec211696b2dab4eeec6cf2fb0e..8a184aa9696759480dee2efe6cfe775cb6277efe 100644
--- a/app/views/notify/new_review_email.html.haml
+++ b/app/views/notify/new_review_email.html.haml
@@ -22,3 +22,4 @@
- discussion.first_note.project = @project if discussion&.first_note
- target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{note.id}")
= render 'note_email', note: note, diff_limit: 3, target_url: target_url, note_style: "border-bottom:1px solid #ededed; padding-bottom: 1em;", include_stylesheet_link: false, discussion: discussion, author: @author
+ = render_if_exists 'notify/review_summary'
diff --git a/app/views/notify/new_review_email.text.erb b/app/views/notify/new_review_email.text.erb
index 69cb33b05dfdcd0d30b3ea959be1235b55ce0167..e974c8b6be8710ac0869f64fce4455ba93769120 100644
--- a/app/views/notify/new_review_email.text.erb
+++ b/app/views/notify/new_review_email.text.erb
@@ -12,3 +12,5 @@
--
<% end %>
<% end %>
+
+<%= render_if_exists 'notify/review_summary' %>
diff --git a/ee/app/models/ee/review.rb b/ee/app/models/ee/review.rb
new file mode 100644
index 0000000000000000000000000000000000000000..fa48aa0c9cc5117617b0ee9341a79828e5416a39
--- /dev/null
+++ b/ee/app/models/ee/review.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module EE
+ module Review
+ extend ActiveSupport::Concern
+
+ prepended do
+ has_one :merge_request_review_llm_summary, class_name: 'MergeRequest::ReviewLlmSummary'
+ end
+ end
+end
diff --git a/ee/app/services/ee/notification_service.rb b/ee/app/services/ee/notification_service.rb
index 6ad77f1b66200863daef870275378174cbdaa868..9b6e910e4195ae511bfc3f9cf691dc019a66d5c7 100644
--- a/ee/app/services/ee/notification_service.rb
+++ b/ee/app/services/ee/notification_service.rb
@@ -182,5 +182,17 @@ def send_account_validation_email(pipeline)
email = user.notification_email_or_default
mailer.account_validation_email(pipeline, email).deliver_later
end
+
+ override :new_review_deliver_options
+ def new_review_deliver_options(review)
+ options = super
+
+ # We delay delivery by 2 minutes since we generate review summary asynchronously.
+ # We expect that in 2 minutes, we already have the summary generated so we
+ # can include it in the email.
+ options[:wait] = 2.minutes if Ability.allowed?(review.author, :summarize_submitted_review, review.merge_request)
+
+ options
+ end
end
end
diff --git a/ee/app/views/notify/_review_summary.html.haml b/ee/app/views/notify/_review_summary.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..ac068f1333a874d1f49ea0d88c9f8ed8a4e08341
--- /dev/null
+++ b/ee/app/views/notify/_review_summary.html.haml
@@ -0,0 +1,7 @@
+- review_llm_summary = @review.merge_request_review_llm_summary
+- if review_llm_summary.present?
+ .md{ style: 'border-bottom:1px solid #ededed; padding-bottom: 1em; padding-top: 1em;' }
+ %h4= _('Summary')
+ = markdown(review_llm_summary.content, pipeline: :email, author: review_llm_summary.reviewer, current_user: @recipient, issuable_reference_expansion_enabled: true)
+ %p{ style: 'color: #737278;' }
+ = _('Summary generated by AI (Experiment)')
diff --git a/ee/app/views/notify/_review_summary.text.erb b/ee/app/views/notify/_review_summary.text.erb
new file mode 100644
index 0000000000000000000000000000000000000000..57fff56e3cb9e246d0e7a5c0d2284613628cc4fa
--- /dev/null
+++ b/ee/app/views/notify/_review_summary.text.erb
@@ -0,0 +1,8 @@
+<% review_llm_summary = @review.merge_request_review_llm_summary %>
+<% if review_llm_summary.present? %>
+--
+
+<%= review_llm_summary.content %>
+
+<%= _('Summary generated by AI (Experiment)') %>
+<% end %>
diff --git a/ee/spec/mailers/notify_spec.rb b/ee/spec/mailers/notify_spec.rb
index 01e42dbd30969c63d85c940efb655ad45dc6ad12..0d04c92a8178c22f4a09d394f7a9868eaa01e378 100644
--- a/ee/spec/mailers/notify_spec.rb
+++ b/ee/spec/mailers/notify_spec.rb
@@ -402,6 +402,17 @@
end
end
+ describe 'merge request reviews' do
+ let!(:review) { create(:review, project: project, merge_request: merge_request) }
+ let!(:review_summary) { create(:merge_request_review_llm_summary, review: review) }
+
+ subject { described_class.new_review_email(recipient.id, review.id) }
+
+ it 'includes review summary' do
+ expect(subject.text_part.body.raw_source).to include(review_summary.content)
+ end
+ end
+
def expect_sender(user)
sender = subject.header[:from].addrs[0]
expect(sender.display_name).to eq("#{user.name} (@#{user.username})")
diff --git a/ee/spec/models/ee/review_spec.rb b/ee/spec/models/ee/review_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..fce8a253108ad433b1662f096def28d25fb89fed
--- /dev/null
+++ b/ee/spec/models/ee/review_spec.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Review, feature_category: :code_review_workflow do
+ describe 'associations' do
+ it { is_expected.to have_one(:merge_request_review_llm_summary).class_name('MergeRequest::ReviewLlmSummary') }
+ end
+end
diff --git a/ee/spec/services/ee/notification_service_spec.rb b/ee/spec/services/ee/notification_service_spec.rb
index b34e77ab9d46097614907e917e478ceafda96c09..28959adc4fb0ce61c544893d14324cfb2e2ab037 100644
--- a/ee/spec/services/ee/notification_service_spec.rb
+++ b/ee/spec/services/ee/notification_service_spec.rb
@@ -59,6 +59,35 @@
subject.new_review(review)
end
+ context 'when review author is allowed to summarize_submitted_review' do
+ before do
+ allow(Ability).to receive(:allowed?).and_call_original
+ allow(Ability)
+ .to receive(:allowed?)
+ .with(review.author, :summarize_submitted_review, review.merge_request)
+ .and_return(true)
+ end
+
+ it 'delays the sending of new_review_email by 2 minutes' do
+ new_review_email = double
+
+ expect(Notify).not_to receive(:new_review_email).with(review.author.id, review.id)
+ expect(Notify).not_to receive(:new_review_email).with(@unsubscriber.id, review.id)
+ merge_request.assignee_ids.each do |assignee_id|
+ expect(Notify).to receive(:new_review_email).with(assignee_id, review.id).and_return(new_review_email)
+ end
+ expect(Notify).to receive(:new_review_email).with(merge_request.author.id, review.id).and_return(new_review_email)
+ expect(Notify).to receive(:new_review_email).with(@u_watcher.id, review.id).and_return(new_review_email)
+ expect(Notify).to receive(:new_review_email).with(@u_mentioned.id, review.id).and_return(new_review_email)
+ expect(Notify).to receive(:new_review_email).with(@subscriber.id, review.id).and_return(new_review_email)
+ expect(Notify).to receive(:new_review_email).with(@watcher_and_subscriber.id, review.id).and_return(new_review_email)
+ expect(Notify).to receive(:new_review_email).with(@subscribed_participant.id, review.id).and_return(new_review_email)
+ expect(new_review_email).to receive(:deliver_later).with({ wait: 2.minutes }).exactly(8).times
+
+ subject.new_review(review)
+ end
+ end
+
it_behaves_like 'project emails are disabled' do
let(:notification_target) { review }
let(:notification_trigger) { subject.new_review(review) }
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 1ba69edc3bf1146656f3a05edbffc716102283f2..d39561ec7a69c69780ec4c5477acb73574132d92 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -45170,6 +45170,9 @@ msgstr ""
msgid "Summary generated by AI"
msgstr ""
+msgid "Summary generated by AI (Experiment)"
+msgstr ""
+
msgid "Summary will be generated with the next push to this merge request and will appear here."
msgstr ""
diff --git a/spec/mailers/previews_spec.rb b/spec/mailers/previews_spec.rb
index 14bd56e5d40f24637eb082f41c3b8d47ab8560cd..e1af0d7ef7731fbab9dc05ec8382e56dc33fdbb8 100644
--- a/spec/mailers/previews_spec.rb
+++ b/spec/mailers/previews_spec.rb
@@ -13,6 +13,7 @@
let_it_be(:issue) { create(:issue, project: project, milestone: milestone) }
let_it_be(:remote_mirror) { create(:remote_mirror, project: project) }
let_it_be(:member) { create(:project_member, :maintainer, project: project, created_by: user) }
+ let_it_be(:review) { create(:review, project: project, merge_request: merge_request, author: user) }
Gitlab.ee do
let_it_be(:epic) { create(:epic, group: group) }