Commit 0e5dbaf8 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Do not show system notes on commits in the MR page

parent 723f936f
...@@ -858,15 +858,6 @@ class MergeRequest < ApplicationRecord ...@@ -858,15 +858,6 @@ class MergeRequest < ApplicationRecord
end end
def related_notes 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 # 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 # 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 # used won't produce any duplicates (e.g. a note for a commit can't also be
...@@ -878,6 +869,16 @@ class MergeRequest < ApplicationRecord ...@@ -878,6 +869,16 @@ class MergeRequest < ApplicationRecord
alias_method :discussion_notes, :related_notes 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? def mergeable_discussions_state?
return true unless project.only_allow_merge_if_all_discussions_are_resolved? return true unless project.only_allow_merge_if_all_discussions_are_resolved?
......
---
title: Exclude system notes from commits in merge request discussions
merge_request: 26396
author:
type: fixed
...@@ -805,6 +805,14 @@ describe MergeRequest do ...@@ -805,6 +805,14 @@ describe MergeRequest do
expect(merge_request.commits).not_to be_empty expect(merge_request.commits).not_to be_empty
expect(merge_request.related_notes.count).to eq(3) expect(merge_request.related_notes.count).to eq(3)
end 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 end
describe '#for_fork?' do describe '#for_fork?' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment