Commit efa40191 authored by Stan Hu's avatar Stan Hu

Fix second 500 error with NULL restricted visibility levels

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/30313 fixed one
500 error when `application_settings.restricted_visibility_levels` is
NULL, but https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22522
introduced the public visiblity check in two places.

To fix this problem and reduce code duplication, move this check into
`Gitlab::VisibilityLevel`, where NULL values of
`restricted_visibility_levels` was already handled.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/215638
parent d8dfa19f
...@@ -497,7 +497,7 @@ class ApplicationController < ActionController::Base ...@@ -497,7 +497,7 @@ class ApplicationController < ActionController::Base
end end
def public_visibility_restricted? def public_visibility_restricted?
Gitlab::CurrentSettings.restricted_visibility_levels.include? Gitlab::VisibilityLevel::PUBLIC Gitlab::VisibilityLevel.public_visibility_restricted?
end end
def set_usage_stats_consent_flag def set_usage_stats_consent_flag
......
...@@ -52,7 +52,7 @@ module ExploreHelper ...@@ -52,7 +52,7 @@ module ExploreHelper
end end
def public_visibility_restricted? def public_visibility_restricted?
Gitlab::CurrentSettings.restricted_visibility_levels&.include? Gitlab::VisibilityLevel::PUBLIC Gitlab::VisibilityLevel.public_visibility_restricted?
end end
private private
......
---
title: Fix second 500 error with NULL restricted visibility levels
merge_request: 30414
author:
type: fixed
...@@ -82,15 +82,23 @@ module Gitlab ...@@ -82,15 +82,23 @@ module Gitlab
end end
def non_restricted_level?(level) def non_restricted_level?(level)
!restricted_level?(level)
end
def restricted_level?(level)
restricted_levels = Gitlab::CurrentSettings.restricted_visibility_levels restricted_levels = Gitlab::CurrentSettings.restricted_visibility_levels
if restricted_levels.nil? if restricted_levels.nil?
true false
else else
!restricted_levels.include?(level) restricted_levels.include?(level)
end end
end end
def public_visibility_restricted?
restricted_level?(PUBLIC)
end
def valid_level?(level) def valid_level?(level)
options.value?(level) options.value?(level)
end end
......
...@@ -19,23 +19,10 @@ describe ExploreHelper do ...@@ -19,23 +19,10 @@ describe ExploreHelper do
end end
describe '#public_visibility_restricted?' do describe '#public_visibility_restricted?' do
using RSpec::Parameterized::TableSyntax it 'delegates to Gitlab::VisibilityLevel' do
expect(Gitlab::VisibilityLevel).to receive(:public_visibility_restricted?).and_call_original
where(:visibility_levels, :expected_status) do helper.public_visibility_restricted?
nil | nil
[Gitlab::VisibilityLevel::PRIVATE] | false
[Gitlab::VisibilityLevel::PRIVATE, Gitlab::VisibilityLevel::INTERNAL] | false
[Gitlab::VisibilityLevel::PUBLIC] | true
end
with_them do
before do
stub_application_setting(restricted_visibility_levels: visibility_levels)
end
it 'returns the expected status' do
expect(helper.public_visibility_restricted?).to eq(expected_status)
end
end end
end end
end end
...@@ -96,6 +96,30 @@ describe Gitlab::VisibilityLevel do ...@@ -96,6 +96,30 @@ describe Gitlab::VisibilityLevel do
end end
end end
describe '.restricted_level?, .non_restricted_level?, and .public_level_restricted?' do
using RSpec::Parameterized::TableSyntax
where(:visibility_levels, :expected_status) do
nil | false
[Gitlab::VisibilityLevel::PRIVATE] | false
[Gitlab::VisibilityLevel::PRIVATE, Gitlab::VisibilityLevel::INTERNAL] | false
[Gitlab::VisibilityLevel::PUBLIC] | true
[Gitlab::VisibilityLevel::PUBLIC, Gitlab::VisibilityLevel::INTERNAL] | true
end
with_them do
before do
stub_application_setting(restricted_visibility_levels: visibility_levels)
end
it 'returns the expected status' do
expect(described_class.restricted_level?(Gitlab::VisibilityLevel::PUBLIC)).to eq(expected_status)
expect(described_class.non_restricted_level?(Gitlab::VisibilityLevel::PUBLIC)).to eq(!expected_status)
expect(described_class.public_visibility_restricted?).to eq(expected_status)
end
end
end
describe '#visibility_level_decreased?' do describe '#visibility_level_decreased?' do
let(:project) { create(:project, :internal) } let(:project) { create(:project, :internal) }
......
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