diff --git a/app/models/group.rb b/app/models/group.rb index 15355418d05b0eaac8fec6c0bccf6ac7a7f564f2..fdd175341b31ba08b9ce192276d44f455df81398 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -26,6 +26,7 @@ class Group < Namespace validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :visibility_level_allowed_by_projects + validate :visibility_level_allowed_by_sub_groups, if: :visibility_level_changed? validate :visibility_level_allowed_by_parent validates :avatar, file_size: { maximum: 200.kilobytes.to_i } @@ -112,14 +113,25 @@ class Group < Namespace end def visibility_level_allowed_by_projects - allowed_by_projects = self.projects.where('visibility_level > ?', self.visibility_level).none? + check_visibility_level_for(:projects) + end + + def visibility_level_allowed_by_sub_groups + check_visibility_level_for(:children) + end - unless allowed_by_projects + def check_visibility_level_for(children_type) + base_query = public_send(children_type) + children_have_higher_visibility = base_query.where('visibility_level > ?', visibility_level).exists? + + if children_have_higher_visibility + children_label = children_type == :projects ? 'projects' : 'sub groups' level_name = Gitlab::VisibilityLevel.level_name(visibility_level).downcase - self.errors.add(:visibility_level, "#{level_name} is not allowed since there are projects with higher visibility.") + + self.errors.add(:visibility_level, "#{level_name} is not allowed since there are #{children_label} with higher visibility.") end - allowed_by_projects + children_have_higher_visibility end def avatar_url(**args) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index a3310cf1dce1b16ac4ac6fb660c162c7f06cd125..c3342afb7fd772654e2c7422d5aba458171b76f3 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -117,6 +117,50 @@ describe Group do end end end + + describe '#visibility_level_allowed_by_projects' do + let!(:internal_group) { create(:group, :internal) } + let!(:internal_project) { create(:project, :internal, group: internal_group) } + + context 'when group has a lower visibility' do + it 'is invalid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_invalid + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are projects with higher visibility.') + end + end + + context 'when group has a higher visibility' do + it 'is valid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(internal_group).to be_valid + end + end + end + + describe '#visibility_level_allowed_by_sub_groups' do + let!(:internal_group) { create(:group, :internal) } + let!(:internal_sub_group) { create(:group, :internal, parent: internal_group) } + + context 'when parent group has a lower visibility' do + it 'is invalid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_invalid + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub groups with higher visibility.') + end + end + + context 'when parent group has a higher visibility' do + it 'is valid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(internal_group).to be_valid + end + end + end end describe '.visible_to_user' do