Commit 0b6d6f9d authored by Kerri Miller's avatar Kerri Miller

Cast predicate methods to boolean

These predicate alias methods have nil as a possible return value, which
can cause potential confusion if we attempt to use their actual return
value rather than testing for its boolean nature. This raises problems
specifically in APIs where we expose the return value as a string, for
instance.
parent b742f30f
...@@ -534,7 +534,10 @@ module EE ...@@ -534,7 +534,10 @@ module EE
def require_password_to_approve def require_password_to_approve
super && password_authentication_enabled_for_web? super && password_authentication_enabled_for_web?
end end
alias_method :require_password_to_approve?, :require_password_to_approve
def require_password_to_approve?
!!require_password_to_approve
end
def find_path_lock(path, exact_match: false, downstream: false) def find_path_lock(path, exact_match: false, downstream: false)
path_lock_finder = strong_memoize(:path_lock_finder) do path_lock_finder = strong_memoize(:path_lock_finder) do
...@@ -723,7 +726,10 @@ module EE ...@@ -723,7 +726,10 @@ module EE
::Gitlab::CurrentSettings.disable_overriding_approvers_per_merge_request? || super ::Gitlab::CurrentSettings.disable_overriding_approvers_per_merge_request? || super
end end
end end
alias_method :disable_overriding_approvers_per_merge_request?, :disable_overriding_approvers_per_merge_request
def disable_overriding_approvers_per_merge_request?
!!disable_overriding_approvers_per_merge_request
end
def merge_requests_author_approval def merge_requests_author_approval
strong_memoize(:merge_requests_author_approval) do strong_memoize(:merge_requests_author_approval) do
...@@ -733,7 +739,10 @@ module EE ...@@ -733,7 +739,10 @@ module EE
super super
end end
end end
alias_method :merge_requests_author_approval?, :merge_requests_author_approval
def merge_requests_author_approval?
!!merge_requests_author_approval
end
def merge_requests_disable_committers_approval def merge_requests_disable_committers_approval
strong_memoize(:merge_requests_disable_committers_approval) do strong_memoize(:merge_requests_disable_committers_approval) do
...@@ -742,7 +751,10 @@ module EE ...@@ -742,7 +751,10 @@ module EE
::Gitlab::CurrentSettings.prevent_merge_requests_committers_approval? || super ::Gitlab::CurrentSettings.prevent_merge_requests_committers_approval? || super
end end
end end
alias_method :merge_requests_disable_committers_approval?, :merge_requests_disable_committers_approval
def merge_requests_disable_committers_approval?
!!merge_requests_disable_committers_approval
end
def license_compliance(pipeline = latest_pipeline_with_reports(::Ci::JobArtifact.license_scanning_reports)) def license_compliance(pipeline = latest_pipeline_with_reports(::Ci::JobArtifact.license_scanning_reports))
SCA::LicenseCompliance.new(self, pipeline) SCA::LicenseCompliance.new(self, pipeline)
......
...@@ -677,6 +677,30 @@ RSpec.describe Project do ...@@ -677,6 +677,30 @@ RSpec.describe Project do
end end
end end
shared_examples 'a predicate wrapper method' do
where(:wrapped_method_return, :subject_return) do
true | true
false | false
nil | false
end
with_them do
it 'returns the expected boolean value' do
expect(project)
.to receive(wrapped_method)
.and_return(wrapped_method_return)
expect(project.send("#{wrapped_method}?")).to be(subject_return)
end
end
end
describe '#disable_overriding_approvers_per_merge_request?' do
it_behaves_like 'a predicate wrapper method' do
let(:wrapped_method) { :disable_overriding_approvers_per_merge_request }
end
end
describe '#merge_requests_disable_committers_approval' do describe '#merge_requests_disable_committers_approval' do
it_behaves_like 'setting modified by application setting' do it_behaves_like 'setting modified by application setting' do
let(:setting) { :merge_requests_disable_committers_approval } let(:setting) { :merge_requests_disable_committers_approval }
...@@ -684,6 +708,18 @@ RSpec.describe Project do ...@@ -684,6 +708,18 @@ RSpec.describe Project do
end end
end end
describe '#merge_requests_disable_committers_approval?' do
it_behaves_like 'a predicate wrapper method' do
let(:wrapped_method) { :merge_requests_disable_committers_approval }
end
end
describe '#require_password_to_approve?' do
it_behaves_like 'a predicate wrapper method' do
let(:wrapped_method) { :require_password_to_approve }
end
end
describe '#merge_requests_author_approval' do describe '#merge_requests_author_approval' do
let(:setting) { :merge_requests_author_approval } let(:setting) { :merge_requests_author_approval }
let(:application_setting) { :prevent_merge_requests_author_approval } let(:application_setting) { :prevent_merge_requests_author_approval }
...@@ -715,6 +751,12 @@ RSpec.describe Project do ...@@ -715,6 +751,12 @@ RSpec.describe Project do
end end
end end
end end
describe '#merge_requests_author_approval?' do
it_behaves_like 'a predicate wrapper method' do
let(:wrapped_method) { :merge_requests_author_approval }
end
end
end end
describe '#has_active_hooks?' do describe '#has_active_hooks?' 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