Commit c7df1373 authored by Igor Drozdov's avatar Igor Drozdov

Rename has_approved? methods to approved_by?

Remove duplicated definitions of the method as well
parent 59b0dcc4
...@@ -8,7 +8,7 @@ module ApprovableBase ...@@ -8,7 +8,7 @@ module ApprovableBase
has_many :approved_by_users, through: :approvals, source: :user has_many :approved_by_users, through: :approvals, source: :user
end end
def has_approved?(user) def approved_by?(user)
return false unless user return false unless user
approved_by_users.include?(user) approved_by_users.include?(user)
......
...@@ -4,7 +4,7 @@ module MergeRequests ...@@ -4,7 +4,7 @@ module MergeRequests
class RemoveApprovalService < MergeRequests::BaseService class RemoveApprovalService < MergeRequests::BaseService
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def execute(merge_request) def execute(merge_request)
return unless merge_request.has_approved?(current_user) return unless merge_request.approved_by?(current_user)
# paranoid protection against running wrong deletes # paranoid protection against running wrong deletes
return unless merge_request.id && current_user.id return unless merge_request.id && current_user.id
......
...@@ -45,7 +45,7 @@ module EE ...@@ -45,7 +45,7 @@ module EE
end end
def unapprove def unapprove
if merge_request.has_approved?(current_user) if merge_request.approved_by?(current_user)
::MergeRequests::RemoveApprovalService ::MergeRequests::RemoveApprovalService
.new(project, current_user) .new(project, current_user)
.execute(merge_request) .execute(merge_request)
......
...@@ -128,12 +128,6 @@ class ApprovalState ...@@ -128,12 +128,6 @@ class ApprovalState
true true
end end
def has_approved?(user)
return false unless user
approved_approvers.include?(user)
end
def authors_can_approve? def authors_can_approve?
project.merge_requests_author_approval? project.merge_requests_author_approval?
end end
......
...@@ -13,7 +13,6 @@ module Approvable ...@@ -13,7 +13,6 @@ module Approvable
approvals_left approvals_left
approvals_required approvals_required
can_approve? can_approve?
has_approved?
authors_can_approve? authors_can_approve?
committers_can_approve? committers_can_approve?
approvers_overwritten? approvers_overwritten?
......
- if merge_request.approval_needed? - if merge_request.approval_needed?
- approved = merge_request.approved? - approved = merge_request.approved?
- self_approved = merge_request.has_approved?(current_user) - self_approved = merge_request.approved_by?(current_user)
- given = merge_request.approvals_given - given = merge_request.approvals_given
- total = merge_request.total_approvals_count - total = merge_request.total_approvals_count
......
...@@ -51,7 +51,7 @@ module EE ...@@ -51,7 +51,7 @@ module EE
end end
expose :user_has_approved do |approval_state, options| expose :user_has_approved do |approval_state, options|
approval_state.has_approved?(options[:current_user]) approval_state.merge_request.approved_by?(options[:current_user])
end end
expose :user_can_approve do |approval_state, options| expose :user_can_approve do |approval_state, options|
......
...@@ -744,7 +744,7 @@ RSpec.describe ApprovalState do ...@@ -744,7 +744,7 @@ RSpec.describe ApprovalState do
end end
it 'requires 1 approval' do it 'requires 1 approval' do
expect(subject.has_approved?(approver)).to eq(true) expect(merge_request.approved_by?(approver)).to eq(true)
expect(subject.approvals_left).to eq(1) expect(subject.approvals_left).to eq(1)
end end
...@@ -754,7 +754,7 @@ RSpec.describe ApprovalState do ...@@ -754,7 +754,7 @@ RSpec.describe ApprovalState do
end end
it 'becomes approved' do it 'becomes approved' do
expect(subject.has_approved?(approver1)).to eq(true) expect(merge_request.approved_by?(approver1)).to eq(true)
expect(subject.approved?).to eq(true) expect(subject.approved?).to eq(true)
end end
end end
...@@ -762,19 +762,6 @@ RSpec.describe ApprovalState do ...@@ -762,19 +762,6 @@ RSpec.describe ApprovalState do
end end
end end
describe '#has_approved?' do
it 'returns false if user is nil' do
expect(subject.has_approved?(nil)).to eq(false)
end
it 'returns true if user has approved' do
create(:approval, merge_request: merge_request, user: approver1)
expect(subject.has_approved?(approver1)).to eq(true)
expect(subject.has_approved?(approver2)).to eq(false)
end
end
describe '#authors_can_approve?' do describe '#authors_can_approve?' do
context 'when project allows author approval' do context 'when project allows author approval' do
before do before do
...@@ -1353,19 +1340,6 @@ RSpec.describe ApprovalState do ...@@ -1353,19 +1340,6 @@ RSpec.describe ApprovalState do
end end
end end
describe '#has_approved?' do
it 'returns false if user is nil' do
expect(subject.has_approved?(nil)).to eq(false)
end
it 'returns true if user has approved' do
create(:approval, merge_request: merge_request, user: approver1)
expect(subject.has_approved?(approver1)).to eq(true)
expect(subject.has_approved?(approver2)).to eq(false)
end
end
describe '#authors_can_approve?' do describe '#authors_can_approve?' do
context 'when project allows author approval' do context 'when project allows author approval' do
before do before do
......
...@@ -4,11 +4,11 @@ module API ...@@ -4,11 +4,11 @@ module API
module Entities module Entities
class MergeRequestApprovals < Grape::Entity class MergeRequestApprovals < Grape::Entity
expose :user_has_approved do |merge_request, options| expose :user_has_approved do |merge_request, options|
merge_request.has_approved?(options[:current_user]) merge_request.approved_by?(options[:current_user])
end end
expose :user_can_approve do |merge_request, options| expose :user_can_approve do |merge_request, options|
!merge_request.has_approved?(options[:current_user]) && !merge_request.approved_by?(options[:current_user]) &&
options[:current_user].can?(:approve_merge_request, merge_request) options[:current_user].can?(:approve_merge_request, merge_request)
end end
......
...@@ -3,11 +3,11 @@ ...@@ -3,11 +3,11 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe ApprovableBase do RSpec.describe ApprovableBase do
describe '#has_approved?' do describe '#approved_by?' do
let(:merge_request) { create(:merge_request) } let(:merge_request) { create(:merge_request) }
let(:user) { create(:user) } let(:user) { create(:user) }
subject { merge_request.has_approved?(user) } subject { merge_request.approved_by?(user) }
context 'when a user has not approved' do context 'when a user has not approved' do
it 'returns false' do it 'returns false' 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