diff options
| -rw-r--r-- | app/models/merge_request.rb | 19 | ||||
| -rw-r--r-- | changelogs/unreleased/55268-exclude-system-notes-from-commits-in-mr.yml | 5 | ||||
| -rw-r--r-- | spec/models/merge_request_spec.rb | 8 |
3 files changed, 23 insertions, 9 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b780b67492f..458c57c1dc6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -858,15 +858,6 @@ class MergeRequest < ApplicationRecord end def related_notes - # Fetch comments only from last 100 commits - commits_for_notes_limit = 100 - commit_ids = commit_shas.take(commits_for_notes_limit) - - commit_notes = Note - .except(:order) - .where(project_id: [source_project_id, target_project_id]) - .for_commit_id(commit_ids) - # We're using a UNION ALL here since this results in better performance # compared to using OR statements. We're using UNION ALL since the queries # used won't produce any duplicates (e.g. a note for a commit can't also be @@ -878,6 +869,16 @@ class MergeRequest < ApplicationRecord alias_method :discussion_notes, :related_notes + def commit_notes + # Fetch comments only from last 100 commits + commit_ids = commit_shas.take(100) + + Note + .user + .where(project_id: [source_project_id, target_project_id]) + .for_commit_id(commit_ids) + end + def mergeable_discussions_state? return true unless project.only_allow_merge_if_all_discussions_are_resolved? diff --git a/changelogs/unreleased/55268-exclude-system-notes-from-commits-in-mr.yml b/changelogs/unreleased/55268-exclude-system-notes-from-commits-in-mr.yml new file mode 100644 index 00000000000..7af4739136b --- /dev/null +++ b/changelogs/unreleased/55268-exclude-system-notes-from-commits-in-mr.yml @@ -0,0 +1,5 @@ +--- +title: Exclude system notes from commits in merge request discussions +merge_request: 26396 +author: +type: fixed diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 892fdc4e4e9..6f34ef9c1bc 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -805,6 +805,14 @@ describe MergeRequest do expect(merge_request.commits).not_to be_empty expect(merge_request.related_notes.count).to eq(3) end + + it "excludes system notes for commits" do + system_note = create(:note_on_commit, :system, commit_id: merge_request.commits.first.id, + project: merge_request.project) + + expect(merge_request.related_notes.count).to eq(2) + expect(merge_request.related_notes).not_to include(system_note) + end end describe '#for_fork?' do |