diff --git a/app/models/todo.rb b/app/models/todo.rb
index 2f2e731fc7ef771dde091ff340a9f28abcb2f501..d159b51a0eb5f20e641d030a1cdc889d12c3b28e 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -225,6 +225,10 @@ def member_access_requested?
action == MEMBER_ACCESS_REQUESTED
end
+ def review_submitted?
+ action == REVIEW_SUBMITTED
+ end
+
def member_access_type
target.class.name.downcase
end
diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml
index e20fccc218a32ecdb946797bb3ee12320251b9b5..1cd8015934e5f819cd3a0e6bf6d2b540c450c76f 100644
--- a/app/views/dashboard/todos/_todo.html.haml
+++ b/app/views/dashboard/todos/_todo.html.haml
@@ -48,6 +48,7 @@
= first_line_in_markdown(todo, :body, 125, is_todo: true, project: todo.project, group: todo.group)
= render_if_exists "dashboard/todos/diff_summary", local_assigns: { todo: todo }
+ = render_if_exists "dashboard/todos/review_summary", local_assigns: { todo: todo }
.todo-timestamp.gl-white-space-nowrap.gl-sm-ml-3.gl-mt-2.gl-mb-2.gl-sm-my-0.gl-px-2.gl-sm-px-0
%span.todo-timestamp.gl-font-sm.gl-text-secondary
diff --git a/ee/app/helpers/ee/merge_requests_helper.rb b/ee/app/helpers/ee/merge_requests_helper.rb
index b9e265a0bd5f2b4300a7c07dfc6e738059c64826..7065ef71abccb13c8c786d5595f4f5f2f0d1ae95 100644
--- a/ee/app/helpers/ee/merge_requests_helper.rb
+++ b/ee/app/helpers/ee/merge_requests_helper.rb
@@ -42,5 +42,13 @@ def review_bar_data(merge_request, user)
def diff_llm_summary(merge_request)
merge_request.latest_merge_request_diff&.merge_request_diff_llm_summary
end
+
+ def review_llm_summary_allowed?(merge_request, user)
+ Ability.allowed?(user, :summarize_submitted_review, merge_request)
+ end
+
+ def review_llm_summary(merge_request, reviewer)
+ merge_request.latest_merge_request_diff&.latest_review_summary_from_reviewer(reviewer)
+ end
end
end
diff --git a/ee/app/models/ee/merge_request_diff.rb b/ee/app/models/ee/merge_request_diff.rb
index d1b9ff80fde2514cad48658fab172c867bc54deb..9fa544c040aac274914116dce10d1f62e46ba3bf 100644
--- a/ee/app/models/ee/merge_request_diff.rb
+++ b/ee/app/models/ee/merge_request_diff.rb
@@ -104,5 +104,9 @@ def log_geo_deleted_event
# Keep empty for now. Should be addressed in future
# by https://gitlab.com/gitlab-org/gitlab/issues/33817
end
+
+ def latest_review_summary_from_reviewer(reviewer)
+ merge_request_review_llm_summaries.from_reviewer(reviewer).last
+ end
end
end
diff --git a/ee/app/models/merge_request/review_llm_summary.rb b/ee/app/models/merge_request/review_llm_summary.rb
index 3480730975e0a872e69d3653a5bba67df2075e57..b08098438c3cb01b21f075539d1f17366e7894d6 100644
--- a/ee/app/models/merge_request/review_llm_summary.rb
+++ b/ee/app/models/merge_request/review_llm_summary.rb
@@ -11,6 +11,8 @@ class MergeRequest::ReviewLlmSummary < ApplicationRecord
enum provider: { open_ai: 0, vertex_ai: 1 }
+ scope :from_reviewer, ->(reviewer) { joins(:review).where(review: { author_id: reviewer.id }) }
+
def reviewer
review.author
end
diff --git a/ee/app/views/dashboard/todos/_diff_summary.html.haml b/ee/app/views/dashboard/todos/_diff_summary.html.haml
index c3a8407c3f5f5836b1ced28e1eb15e0c76b1b1c0..5ea6e71320895700a630e1e68eb1309e1f2838ea 100644
--- a/ee/app/views/dashboard/todos/_diff_summary.html.haml
+++ b/ee/app/views/dashboard/todos/_diff_summary.html.haml
@@ -4,9 +4,4 @@
- merge_request = todo.target
- diff_llm_summary = diff_llm_summary(merge_request)
- if summarize_llm_enabled?(merge_request.project, current_user) && diff_llm_summary.present?
- .todo-diff-summary.gl-mt-3.gl-py-3.gl-pl-6.gl-inset-border-l-4-gray-100
- %p.gl-m-0{ class: "gl-text-gray-600!" }= diff_llm_summary.content
- .gl-text-gray-400.gl-mt-4
- = sprite_icon('tanuki-ai', size: 16, css_class: 'gl-text-gray-600!')
- = _('Summary generated by AI')
- = render Pajamas::BadgeComponent.new(_('Experiment'), size: 'sm', variant: 'neutral', class: 'gl-ml-3')
+ = render 'llm_summary', local_assigns: { content: diff_llm_summary.content }
diff --git a/ee/app/views/dashboard/todos/_llm_summary.html.haml b/ee/app/views/dashboard/todos/_llm_summary.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..f23d9d1aa2569093ee14241f136899855bc2db7a
--- /dev/null
+++ b/ee/app/views/dashboard/todos/_llm_summary.html.haml
@@ -0,0 +1,8 @@
+- content = local_assigns.fetch(:content)
+
+.todo-llm-summary.gl-mt-3.gl-py-3.gl-pl-6.gl-inset-border-l-4-gray-100
+ %p.gl-m-0{ class: "gl-text-gray-600!" }= content
+ .gl-text-gray-400.gl-mt-4
+ = sprite_icon('tanuki-ai', size: 16, css_class: 'gl-text-gray-600!')
+ = _('Summary generated by AI')
+ = render Pajamas::BadgeComponent.new(_('Experiment'), size: 'sm', variant: 'neutral', class: 'gl-ml-3')
diff --git a/ee/app/views/dashboard/todos/_review_summary.html.haml b/ee/app/views/dashboard/todos/_review_summary.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..15efe15558f00497b39428eb683bec8824033f3b
--- /dev/null
+++ b/ee/app/views/dashboard/todos/_review_summary.html.haml
@@ -0,0 +1,6 @@
+- todo = local_assigns.fetch(:todo)
+
+- if todo.review_submitted? && review_llm_summary_allowed?(todo.target, todo.user)
+ - review_summary = review_llm_summary(todo.target, todo.author)
+ - if review_summary
+ = render 'llm_summary', local_assigns: { content: review_summary.content }
diff --git a/ee/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review.rb b/ee/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review.rb
index b1c110b913aa630e67de7de3abbaef121854cb93..c7f60fb233065422e280705cbad6bf6d7939d766 100644
--- a/ee/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review.rb
+++ b/ee/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review.rb
@@ -42,6 +42,12 @@ def store_response(response_modifier, review, mr_diff)
provider: MergeRequest::ReviewLlmSummary.providers[:vertex_ai],
content: response_modifier.response_body
)
+
+ create_todo(review)
+ end
+
+ def create_todo(review)
+ TodoService.new.review_submitted(review)
end
end
end
diff --git a/ee/spec/features/dashboards/todos_spec.rb b/ee/spec/features/dashboards/todos_spec.rb
index 7639f37f7499bc9f4f797c38a6d90edc183b12a6..f0251e72a62b5cead218d357cad1648190a931b3 100644
--- a/ee/spec/features/dashboards/todos_spec.rb
+++ b/ee/spec/features/dashboards/todos_spec.rb
@@ -110,7 +110,7 @@
visit page_path
page.within('.js-todos-all') do
- expect(page).not_to have_selector('.todo-diff-summary')
+ expect(page).not_to have_selector('.todo-llm-summary')
end
end
@@ -125,8 +125,74 @@
it 'shows the todo with diff summary' do
visit page_path
- page.within('.js-todos-all .todo-diff-summary') do
+ page.within('.js-todos-all .todo-llm-summary') do
expect(page).to have_content(diff_summary.content)
+ expect(page).to have_content('Summary generated by AI')
+ end
+ end
+ end
+ end
+
+ context 'when user has review submitted todo', :saas do
+ let_it_be(:namespace) { create(:group_with_plan, plan: :ultimate_plan) }
+ let_it_be(:project) { create(:project, group: namespace) }
+ let_it_be(:merge_request) { create(:merge_request, :skip_diff_creation, source_project: project) }
+ let_it_be(:merge_request_diff) { create(:merge_request_diff, merge_request: merge_request) }
+ let_it_be(:review) { create(:review, merge_request: merge_request) }
+ let_it_be(:todo) do
+ create(
+ :todo,
+ :review_submitted,
+ author: review.author,
+ user: user,
+ project: project,
+ target: merge_request
+ )
+ end
+
+ before_all do
+ project.add_developer(user)
+ end
+
+ before do
+ stub_ee_application_setting(should_check_namespace_plan: true)
+
+ stub_licensed_features(
+ summarize_submitted_review: true,
+ ai_features: true
+ )
+
+ project.reload.root_ancestor.namespace_settings.update!(
+ experiment_features_enabled: true,
+ third_party_ai_features_enabled: true
+ )
+
+ sign_in(user)
+ end
+
+ it 'does not show todo with review summary' do
+ visit page_path
+
+ page.within('.js-todos-all') do
+ expect(page).not_to have_selector('.todo-llm-summary')
+ end
+ end
+
+ context 'when merge request has review summary' do
+ let!(:review_summary) do
+ create(
+ :merge_request_review_llm_summary,
+ merge_request_diff: merge_request_diff,
+ review: review
+ )
+ end
+
+ it 'shows the todo with review summary' do
+ visit page_path
+
+ page.within('.js-todos-all .todo-llm-summary') do
+ expect(page).to have_content(review_summary.content)
+ expect(page).to have_content('Summary generated by AI')
end
end
end
diff --git a/ee/spec/helpers/merge_requests_helper_spec.rb b/ee/spec/helpers/merge_requests_helper_spec.rb
index b7e7e589cb37675c2642de93e9b64e54a434a88c..195500de9957054d95129793221c885c7c736871 100644
--- a/ee/spec/helpers/merge_requests_helper_spec.rb
+++ b/ee/spec/helpers/merge_requests_helper_spec.rb
@@ -108,4 +108,49 @@
it { expect(helper.diff_llm_summary(merge_request)).to eq(nil) }
end
end
+
+ describe '#review_llm_summary_allowed?' do
+ let(:user) { build_stubbed(:user) }
+ let(:merge_request) { build_stubbed(:merge_request) }
+
+ it 'calls Ability.allowed? with summarize_submitted_review' do
+ expect(Ability)
+ .to receive(:allowed?)
+ .with(user, :summarize_submitted_review, merge_request)
+ .and_return(true)
+
+ expect(review_llm_summary_allowed?(merge_request, user)).to eq(true)
+ end
+ end
+
+ describe '#review_llm_summary' do
+ let_it_be(:merge_request) { build_stubbed(:merge_request) }
+ let_it_be(:reviewer) { build_stubbed(:user) }
+ let(:latest_merge_request_diff) { instance_double(MergeRequestDiff) }
+
+ before do
+ allow(merge_request)
+ .to receive(:latest_merge_request_diff)
+ .and_return(latest_merge_request_diff)
+ end
+
+ it 'returns latest review summary from reviewer' do
+ latest_review_summary = instance_double(MergeRequest::ReviewLlmSummary)
+
+ expect(latest_merge_request_diff)
+ .to receive(:latest_review_summary_from_reviewer)
+ .with(reviewer)
+ .and_return(latest_review_summary)
+
+ expect(review_llm_summary(merge_request, reviewer)).to eq(latest_review_summary)
+ end
+
+ context 'when merge request has no latest merge request diff' do
+ let(:latest_merge_request_diff) { nil }
+
+ it 'returns nil' do
+ expect(review_llm_summary(merge_request, reviewer)).to be_nil
+ end
+ end
+ end
end
diff --git a/ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review_spec.rb b/ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review_spec.rb
index 957faca3d125fefd45ed63302850fa1e5f8423bf..2ab037ab52a30cebf609cae46eece40f4d347fea 100644
--- a/ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review_spec.rb
+++ b/ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review_spec.rb
@@ -68,6 +68,14 @@
expect(latest_review_llm_summary.content).to eq('Summary generated by AI')
end
end
+
+ it 'create a todo' do
+ expect_next_instance_of(TodoService) do |svc|
+ expect(svc).to receive(:review_submitted).with(review)
+ end
+
+ subject.execute(user, merge_request, options)
+ end
end
context 'when the chat client returns an unsuccessful response' do
@@ -83,6 +91,12 @@
expect { subject.execute(user, merge_request, options) }
.not_to change { mr_diff.merge_request_review_llm_summaries.count }
end
+
+ it 'does not create a todo' do
+ expect(TodoService).not_to receive(:new)
+
+ subject.execute(user, merge_request, options)
+ end
end
end
end
diff --git a/ee/spec/models/ee/merge_request_diff_spec.rb b/ee/spec/models/ee/merge_request_diff_spec.rb
index aaa9445a799a1f292e6292acd96abdedb3278f73..92821c1aab119f3db0b7d9a026a9ce897d336cf6 100644
--- a/ee/spec/models/ee/merge_request_diff_spec.rb
+++ b/ee/spec/models/ee/merge_request_diff_spec.rb
@@ -323,4 +323,24 @@
end
end
end
+
+ describe '#latest_review_summary_from_reviewer' do
+ let(:merge_request) { create(:merge_request, :skip_diff_creation) }
+ let(:merge_request_2) { create(:merge_request, :skip_diff_creation) }
+ let(:reviewer) { merge_request.author }
+ let(:mr_diff_1) { create(:merge_request_diff, merge_request: merge_request) }
+ let(:mr_diff_2) { create(:merge_request_diff, merge_request: merge_request_2) }
+ let(:review_1) { create(:review, merge_request: merge_request, author: reviewer) }
+ let(:review_2) { create(:review, merge_request: merge_request, author: reviewer) }
+ let(:review_3) { create(:review, merge_request: merge_request) }
+ let(:review_4) { create(:review, merge_request: merge_request_2, author: reviewer) }
+ let!(:review_summary_1) { create(:merge_request_review_llm_summary, merge_request_diff: mr_diff_1, review: review_1) }
+ let!(:review_summary_2) { create(:merge_request_review_llm_summary, merge_request_diff: mr_diff_1, review: review_2) }
+ let!(:review_summary_3) { create(:merge_request_review_llm_summary, merge_request_diff: mr_diff_1, review: review_3) }
+ let!(:review_summary_4) { create(:merge_request_review_llm_summary, merge_request_diff: mr_diff_2, review: review_4) }
+
+ it 'returns the latest review summary from reviewer' do
+ expect(mr_diff_1.latest_review_summary_from_reviewer(reviewer)).to eq(review_summary_2)
+ end
+ end
end
diff --git a/ee/spec/models/merge_request/review_llm_summary_spec.rb b/ee/spec/models/merge_request/review_llm_summary_spec.rb
index 0f971accad930e98a75841e841ab446b25f7cbdc..282a95dd9579452938dad69df678f260cbcb3d40 100644
--- a/ee/spec/models/merge_request/review_llm_summary_spec.rb
+++ b/ee/spec/models/merge_request/review_llm_summary_spec.rb
@@ -14,6 +14,17 @@
it { is_expected.to validate_presence_of(:provider) }
end
+ describe '.from_reviewer' do
+ let(:review_1) { create(:review) }
+ let(:review_2) { create(:review) }
+ let!(:review_llm_summary_1) { create(:merge_request_review_llm_summary, review: review_1) }
+ let!(:review_llm_summary_2) { create(:merge_request_review_llm_summary, review: review_2) }
+
+ it 'returns review LLM summaries that were generated for the reviews from the reviewer' do
+ expect(described_class.from_reviewer(review_1.author)).to eq([review_llm_summary_1])
+ end
+ end
+
describe '#reviewer' do
it 'returns author of associated review' do
expect(merge_request_review_llm_summary.reviewer).to eq(merge_request_review_llm_summary.review.author)