Commit 69b089c6 authored by Nick Thomas's avatar Nick Thomas Committed by Yorick Peterse

Remove the approvers_left helper

parent 471e76d9
...@@ -123,7 +123,6 @@ class ApprovalState ...@@ -123,7 +123,6 @@ class ApprovalState
filter_approvers(rules.flat_map(&target), unactioned: unactioned) filter_approvers(rules.flat_map(&target), unactioned: unactioned)
end end
# approvers_left
def unactioned_approvers def unactioned_approvers
strong_memoize(:unactioned_approvers) { approvers - approved_approvers } strong_memoize(:unactioned_approvers) { approvers - approved_approvers }
end end
......
...@@ -5,12 +5,6 @@ ...@@ -5,12 +5,6 @@
module VisibleApprovable module VisibleApprovable
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
# Users in the list of approvers who have not already approved this MR.
#
def approvers_left
approval_state.unactioned_approvers
end
# The list of approvers from either this MR (if they've been set on the MR) or the # The list of approvers from either this MR (if they've been set on the MR) or the
# target project. Excludes the author if 'self-approval' isn't explicitly # target project. Excludes the author if 'self-approval' isn't explicitly
# enabled on project settings. # enabled on project settings.
...@@ -42,7 +36,6 @@ module VisibleApprovable ...@@ -42,7 +36,6 @@ module VisibleApprovable
approved_by_users.reset approved_by_users.reset
approval_rules.reset approval_rules.reset
clear_memoization(:approvers_left)
clear_memoization(:all_approvers_including_groups) clear_memoization(:all_approvers_including_groups)
clear_memoization(:approval_state) clear_memoization(:approval_state)
end end
......
...@@ -548,45 +548,6 @@ describe MergeRequest do ...@@ -548,45 +548,6 @@ describe MergeRequest do
end end
end end
describe "#approvers_left" do
let(:merge_request) {create :merge_request}
it "returns correct value" do
user = create(:user)
user1 = create(:user)
create(:approver, target: merge_request, user: user)
create(:approver, target: merge_request, user: user1)
merge_request.approvals.create(user_id: user1.id)
expect(merge_request.approvers_left).to eq [user]
end
it "returns correct value when there is a group approver" do
user = create(:user)
user1 = create(:user)
user2 = create(:user)
group = create(:group)
group.add_developer(user2)
create(:approver_group, target: merge_request, group: group)
create(:approver, target: merge_request, user: user)
create(:approver, target: merge_request, user: user1)
merge_request.approvals.create(user_id: user1.id)
expect(merge_request.approvers_left).to match_array [user, user2]
end
it "returns correct value when there is only a group approver" do
user = create(:user)
group = create(:group)
group.add_developer(user)
merge_request.approver_groups.create(group: group)
expect(merge_request.approvers_left).to eq [user]
end
end
describe '#all_approvers_including_groups' do describe '#all_approvers_including_groups' do
it 'returns correct set of users' do it 'returns correct set of users' do
user = create :user user = create :user
......
...@@ -7,31 +7,6 @@ describe VisibleApprovable do ...@@ -7,31 +7,6 @@ describe VisibleApprovable do
let!(:project) { create(:project, :repository) } let!(:project) { create(:project, :repository) }
let!(:user) { project.creator } let!(:user) { project.creator }
describe '#approvers_left' do
let!(:private_group) { create(:group_with_members, :private) }
let!(:public_group) { create(:group_with_members) }
let!(:approver) { create(:user) }
let!(:rule) { create(:approval_project_rule, project: project, groups: [public_group, private_group], users: [approver])}
before do
project.add_developer(approver)
end
subject { resource.approvers_left }
it 'avoids N+1 queries' do
control = ActiveRecord::QueryRecorder.new { subject }
expect { subject }.not_to exceed_query_limit(control)
end
it 'returns all approvers left' do
resource.approvals.create!(user: approver)
is_expected.to match_array(public_group.users + private_group.users)
end
end
describe '#overall_approvers' do describe '#overall_approvers' do
let(:approver) { create(:user) } let(:approver) { create(:user) }
let(:code_owner) { build(:user) } let(:code_owner) { build(:user) }
...@@ -134,15 +109,6 @@ describe VisibleApprovable do ...@@ -134,15 +109,6 @@ describe VisibleApprovable do
subject { resource.reset_approval_cache! } subject { resource.reset_approval_cache! }
it 'clears the cache of approvers left' do
user_can_approve = resource.approvers_left.first
resource.approvals.create!(user: user_can_approve)
subject
expect(resource.approvers_left).to be_empty
end
it 'clears the all_approvers_including_groups cache' do it 'clears the all_approvers_including_groups cache' do
resource.all_approvers_including_groups.first.destroy! resource.all_approvers_including_groups.first.destroy!
......
...@@ -83,25 +83,6 @@ describe MergeRequestPresenter do ...@@ -83,25 +83,6 @@ describe MergeRequestPresenter do
it { is_expected.to eq(expose_path("/api/v4/projects/#{merge_request.project.id}/merge_requests/#{merge_request.iid}/unapprove")) } it { is_expected.to eq(expose_path("/api/v4/projects/#{merge_request.project.id}/merge_requests/#{merge_request.iid}/unapprove")) }
end end
describe '#approvers_left' do
let!(:private_group) { create(:group_with_members, :private) }
let!(:public_group) { create(:group_with_members) }
let!(:approver) { create(:user) }
let!(:approval_rule) { create(:approval_merge_request_rule, merge_request: merge_request, users: [approver], groups: [private_group, public_group]) }
before do
merge_request.approvals.create!(user: approver)
end
subject { described_class.new(merge_request, current_user: user).approvers_left }
it 'contains all approvers' do
approvers = public_group.users + private_group.users - [user]
is_expected.to match_array(approvers)
end
end
describe '#all_approvers_including_groups with approval_rule enabled' do describe '#all_approvers_including_groups with approval_rule enabled' do
let!(:private_group) { create(:group_with_members, :private) } let!(:private_group) { create(:group_with_members, :private) }
let!(:public_group) { create(:group_with_members) } let!(:public_group) { create(:group_with_members) }
......
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