From a812ed8f41748fe568932c818c3cfeb43e67f138 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 21 Jun 2025 09:11:09 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/concerns/diff_positionable_note.rb | 22 +++++++++++++++ app/models/diff_note.rb | 22 --------------- app/models/draft_note.rb | 12 ++++++++ app/services/draft_notes/create_service.rb | 2 ++ spec/models/draft_note_spec.rb | 28 +++++++++++++++++++ .../draft_notes/create_service_spec.rb | 16 +++++++++++ 6 files changed, 80 insertions(+), 22 deletions(-) diff --git a/app/models/concerns/diff_positionable_note.rb b/app/models/concerns/diff_positionable_note.rb index 8d10d098e54..ebba12bfa41 100644 --- a/app/models/concerns/diff_positionable_note.rb +++ b/app/models/concerns/diff_positionable_note.rb @@ -102,4 +102,26 @@ module DiffPositionableNote errors.add(:commit_id, 'does not match the diff refs') end + + def keep_around_commits + repository.keep_around(*shas, source: "#{noteable_type}/#{self.class.name}") + end + + def repository + noteable.respond_to?(:repository) ? noteable.repository : project.repository + end + + def shas + [ + original_position.base_sha, + original_position.start_sha, + original_position.head_sha + ].tap do |a| + if position != original_position + a << position.base_sha + a << position.start_sha + a << position.head_sha + end + end + end end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index e0a2b6e3f14..cdba51e3a0e 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -143,20 +143,6 @@ class DiffNote < Note position&.multiline? end - def shas - [ - self.original_position.base_sha, - self.original_position.start_sha, - self.original_position.head_sha - ].tap do |a| - if self.position != self.original_position - a << self.position.base_sha - a << self.position.start_sha - a << self.position.head_sha - end - end - end - def latest_diff_file_path latest_diff_file.file_path end @@ -224,12 +210,4 @@ class DiffNote < Note errors.add(:position, "is incomplete") end - - def keep_around_commits - repository.keep_around(*shas, source: "#{noteable_type}/#{self.class.name}") - end - - def repository - noteable.respond_to?(:repository) ? noteable.repository : project.repository - end end diff --git a/app/models/draft_note.rb b/app/models/draft_note.rb index 1ff0c4caf75..1c5aeff131d 100644 --- a/app/models/draft_note.rb +++ b/app/models/draft_note.rb @@ -47,6 +47,18 @@ class DraftNote < ApplicationRecord .map(&:position) end + def self.bulk_insert_and_keep_commits!(items, **options) + inserted_records = bulk_insert!(items, **options) + + keep_commits_for_records(items) + + inserted_records + end + + def self.keep_commits_for_records(records) + records.find(&:on_diff?)&.keep_around_commits + end + def project merge_request.target_project end diff --git a/app/services/draft_notes/create_service.rb b/app/services/draft_notes/create_service.rb index 40d8edae2d0..5dbc61a18dc 100644 --- a/app/services/draft_notes/create_service.rb +++ b/app/services/draft_notes/create_service.rb @@ -36,6 +36,8 @@ module DraftNotes merge_request_activity_counter.track_create_review_note_action(user: current_user) end + draft_note.keep_around_commits if draft_note.on_diff? + after_execute draft_note diff --git a/spec/models/draft_note_spec.rb b/spec/models/draft_note_spec.rb index 785ba1c99c8..efc0e65ed3c 100644 --- a/spec/models/draft_note_spec.rb +++ b/spec/models/draft_note_spec.rb @@ -119,4 +119,32 @@ RSpec.describe DraftNote, feature_category: :code_review_workflow do end end end + + describe '.bulk_insert_and_keep_commits!' do + let_it_be(:user) { create(:user) } + let(:drafts) do + [ + build(:draft_note, merge_request: merge_request, author: user), + build(:draft_note_on_text_diff, merge_request: merge_request, author: user), + build(:draft_note_on_text_diff, merge_request: merge_request, author: user) + ] + end + + it 'inserts items in the given number of batches' do + expect(described_class) + .to receive(:bulk_insert!) + .with(drafts, batch_size: 5) + .and_call_original + + described_class.bulk_insert_and_keep_commits!(drafts, batch_size: 5) + end + + it 'calls keep_around_commits on the first draft note on diff' do + expect(drafts[0]).not_to receive(:keep_around_commits) + expect(drafts[1]).to receive(:keep_around_commits) + expect(drafts[2]).not_to receive(:keep_around_commits) # This tests that we only keep around the first commit + + described_class.bulk_insert_and_keep_commits!(drafts, batch_size: 10) + end + end end diff --git a/spec/services/draft_notes/create_service_spec.rb b/spec/services/draft_notes/create_service_spec.rb index c0753a2e674..dd598b06350 100644 --- a/spec/services/draft_notes/create_service_spec.rb +++ b/spec/services/draft_notes/create_service_spec.rb @@ -20,6 +20,22 @@ RSpec.describe DraftNotes::CreateService, feature_category: :code_review_workflo expect(draft.discussion_id).to be_nil end + it 'creates keep-around refs' do + diff_refs = project.commit(RepoHelpers.sample_commit.id).try(:diff_refs) + + position = Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: diff_refs + ) + + expect(project.repository).to receive(:keep_around) + + create_draft(note: 'Comment on diff', position: position.to_json) + end + it 'tracks the start event when the draft is persisted' do expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) .to receive(:track_create_review_note_action)