Commit 5ce58821 authored by Kerri Miller's avatar Kerri Miller

Filter commented_approvers for user note authors

Previously, we were pulling authors of _all_ notes, including system
notes (such as cross-MR and Issue mentions).

Changelog: fixed
parent 1cadf027
...@@ -18,7 +18,7 @@ class ApprovalWrappedAnyApproverRule < ApprovalWrappedRule ...@@ -18,7 +18,7 @@ class ApprovalWrappedAnyApproverRule < ApprovalWrappedRule
def commented_approvers def commented_approvers
strong_memoize(:commented_approvers) do strong_memoize(:commented_approvers) do
merge_request.note_authors.select do |user| merge_request.user_note_authors.select do |user|
merge_request.can_approve?(user) merge_request.can_approve?(user)
end end
end end
......
...@@ -80,7 +80,7 @@ class ApprovalWrappedRule ...@@ -80,7 +80,7 @@ class ApprovalWrappedRule
def commented_approvers def commented_approvers
strong_memoize(:commented_approvers) do strong_memoize(:commented_approvers) do
merge_request.note_authors & approvers merge_request.user_note_authors & approvers
end end
end end
......
...@@ -55,10 +55,12 @@ RSpec.describe ApprovalWrappedAnyApproverRule do ...@@ -55,10 +55,12 @@ RSpec.describe ApprovalWrappedAnyApproverRule do
it "returns an array of approvers who have commented" do it "returns an array of approvers who have commented" do
create(:note, project: merge_request.project, noteable: merge_request, author: approver1) create(:note, project: merge_request.project, noteable: merge_request, author: approver1)
create(:system_note, project: merge_request.project, noteable: merge_request, author: approver2)
allow(merge_request).to receive(:can_approve?).with(approver1).and_return(true) allow(merge_request).to receive(:can_approve?).and_return(true)
expect(subject.commented_approvers).to eq([approver1]) expect(subject.commented_approvers).to include(approver1)
expect(subject.commented_approvers).not_to include(approver2)
end end
end end
end end
...@@ -162,9 +162,14 @@ RSpec.describe ApprovalWrappedRule do ...@@ -162,9 +162,14 @@ RSpec.describe ApprovalWrappedRule do
end end
it "returns an array of approvers who have commented" do it "returns an array of approvers who have commented" do
non_approver = create(:user)
create(:note, project: merge_request.project, noteable: merge_request, author: approver1) create(:note, project: merge_request.project, noteable: merge_request, author: approver1)
create(:note, project: merge_request.project, noteable: merge_request, author: non_approver)
create(:system_note, project: merge_request.project, noteable: merge_request, author: approver3)
expect(subject.commented_approvers).to eq([approver1]) expect(subject.commented_approvers).to include(approver1)
expect(subject.commented_approvers).not_to include(non_approver)
expect(subject.commented_approvers).not_to include(approver3)
end end
end end
......
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