Commit bf4e1ff7 authored by charlie ablett's avatar charlie ablett

Do not validate ProjectNamespaces for hierarchy

ProjectNamespaces don't have a hierarchy so they are
excluded from group-only validations that expect to only
receive Group.
parent 7947ae8f
...@@ -93,9 +93,11 @@ class Namespace < ApplicationRecord ...@@ -93,9 +93,11 @@ class Namespace < ApplicationRecord
validates :max_artifacts_size, numericality: { only_integer: true, greater_than: 0, allow_nil: true } validates :max_artifacts_size, numericality: { only_integer: true, greater_than: 0, allow_nil: true }
validate :validate_parent_type, if: -> { Feature.enabled?(:validate_namespace_parent_type, default_enabled: :yaml) } validate :validate_parent_type, if: -> { Feature.enabled?(:validate_namespace_parent_type, default_enabled: :yaml) }
validate :nesting_level_allowed
validate :changing_shared_runners_enabled_is_allowed # ProjectNamespaces excluded as they are not meant to appear in the group hierarchy at the moment.
validate :changing_allow_descendants_override_disabled_shared_runners_is_allowed validate :nesting_level_allowed, unless: -> { project_namespace? }
validate :changing_shared_runners_enabled_is_allowed, unless: -> { project_namespace? }
validate :changing_allow_descendants_override_disabled_shared_runners_is_allowed, unless: -> { project_namespace? }
delegate :name, to: :owner, allow_nil: true, prefix: true delegate :name, to: :owner, allow_nil: true, prefix: true
delegate :avatar_url, to: :owner, allow_nil: true delegate :avatar_url, to: :owner, allow_nil: true
......
...@@ -40,6 +40,30 @@ RSpec.describe Namespace do ...@@ -40,6 +40,30 @@ RSpec.describe Namespace do
end end
end end
shared_examples 'validations called by different namespace types' do |method|
using RSpec::Parameterized::TableSyntax
where(:namespace_type, :call_validation) do
:namespace | true
:group | true
:user_namespace | true
:project_namespace | false
end
with_them do
it 'conditionally runs given validation' do
namespace = build(namespace_type)
if call_validation
expect(namespace).to receive(method)
else
expect(namespace).not_to receive(method)
end
namespace.valid?
end
end
end
describe 'validations' do describe 'validations' do
it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_length_of(:name).is_at_most(255) }
...@@ -112,14 +136,20 @@ RSpec.describe Namespace do ...@@ -112,14 +136,20 @@ RSpec.describe Namespace do
end end
end end
it 'does not allow too deep nesting' do describe '#nesting_level_allowed' do
ancestors = (1..21).to_a context 'for a group' do
group = build(:group) it 'does not allow too deep nesting' do
ancestors = (1..21).to_a
group = build(:group)
allow(group).to receive(:ancestors).and_return(ancestors) allow(group).to receive(:ancestors).and_return(ancestors)
expect(group).not_to be_valid expect(group).not_to be_valid
expect(group.errors[:parent_id].first).to eq('has too deep level of nesting') expect(group.errors[:parent_id].first).to eq('has too deep level of nesting')
end
end
it_behaves_like 'validations called by different namespace types', :nesting_level_allowed
end end
describe 'reserved path validation' do describe 'reserved path validation' do
...@@ -1855,87 +1885,95 @@ RSpec.describe Namespace do ...@@ -1855,87 +1885,95 @@ RSpec.describe Namespace do
end end
context 'with a parent' do context 'with a parent' do
context 'when parent has shared runners disabled' do context 'when namespace is a group' do
let(:parent) { create(:group, :shared_runners_disabled) } context 'when parent has shared runners disabled' do
let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } let(:parent) { create(:group, :shared_runners_disabled) }
let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) }
it 'is invalid' do
expect(group).to be_invalid it 'is invalid' do
expect(group.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled') expect(group).to be_invalid
expect(group.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled')
end
end end
end
context 'when parent has shared runners disabled but allows override' do context 'when parent has shared runners disabled but allows override' do
let(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } let(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) }
let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) }
it 'is valid' do it 'is valid' do
expect(group).to be_valid expect(group).to be_valid
end
end end
end
context 'when parent has shared runners enabled' do context 'when parent has shared runners enabled' do
let(:parent) { create(:group, shared_runners_enabled: true) } let(:parent) { create(:group, shared_runners_enabled: true) }
let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) }
it 'is valid' do it 'is valid' do
expect(group).to be_valid expect(group).to be_valid
end
end end
end end
end end
it_behaves_like 'validations called by different namespace types', :changing_shared_runners_enabled_is_allowed
end end
describe 'validation #changing_allow_descendants_override_disabled_shared_runners_is_allowed' do describe 'validation #changing_allow_descendants_override_disabled_shared_runners_is_allowed' do
context 'without a parent' do context 'when namespace is a group' do
context 'with shared runners disabled' do context 'without a parent' do
let(:namespace) { build(:namespace, :allow_descendants_override_disabled_shared_runners, :shared_runners_disabled) } context 'with shared runners disabled' do
let(:namespace) { build(:group, :allow_descendants_override_disabled_shared_runners, :shared_runners_disabled) }
it 'is valid' do it 'is valid' do
expect(namespace).to be_valid expect(namespace).to be_valid
end
end end
end
context 'with shared runners enabled' do context 'with shared runners enabled' do
let(:namespace) { create(:namespace) } let(:namespace) { create(:namespace) }
it 'is invalid' do it 'is invalid' do
namespace.allow_descendants_override_disabled_shared_runners = true namespace.allow_descendants_override_disabled_shared_runners = true
expect(namespace).to be_invalid expect(namespace).to be_invalid
expect(namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be changed if shared runners are enabled') expect(namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be changed if shared runners are enabled')
end
end end
end end
end
context 'with a parent' do context 'with a parent' do
context 'when parent does not allow shared runners' do context 'when parent does not allow shared runners' do
let(:parent) { create(:group, :shared_runners_disabled) } let(:parent) { create(:group, :shared_runners_disabled) }
let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) }
it 'is invalid' do it 'is invalid' do
expect(group).to be_invalid expect(group).to be_invalid
expect(group.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it') expect(group.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it')
end
end end
end
context 'when parent allows shared runners and setting to true' do context 'when parent allows shared runners and setting to true' do
let(:parent) { create(:group, shared_runners_enabled: true) } let(:parent) { create(:group, shared_runners_enabled: true) }
let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) }
it 'is valid' do it 'is valid' do
expect(group).to be_valid expect(group).to be_valid
end
end end
end
context 'when parent allows shared runners and setting to false' do context 'when parent allows shared runners and setting to false' do
let(:parent) { create(:group, shared_runners_enabled: true) } let(:parent) { create(:group, shared_runners_enabled: true) }
let(:group) { build(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) } let(:group) { build(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) }
it 'is valid' do it 'is valid' do
expect(group).to be_valid expect(group).to be_valid
end
end end
end end
end end
it_behaves_like 'validations called by different namespace types', :changing_allow_descendants_override_disabled_shared_runners_is_allowed
end end
describe '#root?' do describe '#root?' 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