diff --git a/app/assets/javascripts/commons/polyfills.js b/app/assets/javascripts/commons/polyfills.js index bc3e741f524..b78089525cc 100644 --- a/app/assets/javascripts/commons/polyfills.js +++ b/app/assets/javascripts/commons/polyfills.js @@ -12,3 +12,4 @@ import 'core-js/fn/symbol'; // Browser polyfills import './polyfills/custom_event'; import './polyfills/element'; +import './polyfills/nodelist'; diff --git a/app/assets/javascripts/commons/polyfills/nodelist.js b/app/assets/javascripts/commons/polyfills/nodelist.js new file mode 100644 index 00000000000..3772c94b900 --- /dev/null +++ b/app/assets/javascripts/commons/polyfills/nodelist.js @@ -0,0 +1,7 @@ +if (window.NodeList && !NodeList.prototype.forEach) { + NodeList.prototype.forEach = function forEach(callback, thisArg = window) { + for (let i = 0; i < this.length; i += 1) { + callback.call(thisArg, this[i], i, this); + } + }; +} diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index b71c449090e..c70a17104fd 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -74,6 +74,7 @@ import PerformanceBar from './performance_bar'; import initNotes from './init_notes'; import initLegacyFilters from './init_legacy_filters'; import initIssuableSidebar from './init_issuable_sidebar'; +import initProjectVisibilitySelector from './project_visibility'; import GpgBadges from './gpg_badges'; import UserFeatureHelper from './helpers/user_feature_helper'; import initChangesDropdown from './init_changes_dropdown'; @@ -575,6 +576,7 @@ import initChangesDropdown from './init_changes_dropdown'; break; case 'new': new ProjectNew(); + initProjectVisibilitySelector(); break; case 'show': new Star(); diff --git a/app/assets/javascripts/project_visibility.js b/app/assets/javascripts/project_visibility.js new file mode 100644 index 00000000000..c3f5e8cb907 --- /dev/null +++ b/app/assets/javascripts/project_visibility.js @@ -0,0 +1,41 @@ +function setVisibilityOptions(namespaceSelector) { + if (!namespaceSelector || !('selectedIndex' in namespaceSelector)) { + return; + } + const selectedNamespace = namespaceSelector.options[namespaceSelector.selectedIndex]; + const { name, visibility, visibilityLevel, showPath, editPath } = selectedNamespace.dataset; + + document.querySelectorAll('.visibility-level-setting .radio').forEach((option) => { + const optionInput = option.querySelector('input[type=radio]'); + const optionValue = optionInput ? optionInput.value : 0; + const optionTitle = option.querySelector('.option-title'); + const optionName = optionTitle ? optionTitle.innerText.toLowerCase() : ''; + + // don't change anything if the option is restricted by admin + if (!option.classList.contains('restricted')) { + if (visibilityLevel < optionValue) { + option.classList.add('disabled'); + optionInput.disabled = true; + const reason = option.querySelector('.option-disabled-reason'); + if (reason) { + reason.innerHTML = + `This project cannot be ${optionName} because the visibility of + ${name} is ${visibility}. To make this project + ${optionName}, you must first change the visibility + of the parent group.`; + } + } else { + option.classList.remove('disabled'); + optionInput.disabled = false; + } + } + }); +} + +export default function initProjectVisibilitySelector() { + const namespaceSelector = document.querySelector('select.js-select-namespace'); + if (namespaceSelector) { + $('.select2.js-select-namespace').on('change', () => setVisibilityOptions(namespaceSelector)); + setVisibilityOptions(namespaceSelector); + } +} diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 39c4264e496..19caefa1961 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -299,28 +299,6 @@ } } -.project-visibility-level-holder { - .radio { - margin-bottom: 10px; - - i { - margin: 2px 0; - font-size: 20px; - } - - .option-title { - font-weight: $gl-font-weight-normal; - display: inline-block; - color: $gl-text-color; - } - - .option-descr { - margin-left: 29px; - color: $project-option-descr-color; - } - } -} - .save-project-loader { margin-top: 50px; margin-bottom: 50px; diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 15df51e9c69..41a6ba2023a 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -143,6 +143,47 @@ } } +.visibility-level-setting { + .radio { + margin-bottom: 10px; + + i.fa { + margin: 2px 0; + font-size: 20px; + } + + .option-title { + font-weight: $gl-font-weight-normal; + display: inline-block; + color: $gl-text-color; + } + + .option-description, + .option-disabled-reason { + margin-left: 29px; + color: $project-option-descr-color; + } + + .option-disabled-reason { + display: none; + } + + &.disabled { + i.fa { + opacity: 0.5; + } + + .option-description { + display: none; + } + + .option-disabled-reason { + display: block; + } + } + } +} + .prometheus-metrics-monitoring { .panel { .panel-toggle { diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 1d24563a6a6..ed17b3b4689 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -20,7 +20,10 @@ class ProjectsController < Projects::ApplicationController end def new - @project = Project.new + namespace = Namespace.find_by(id: params[:namespace_id]) if params[:namespace_id] + return access_denied! if namespace && !can?(current_user, :create_projects, namespace) + + @project = Project.new(namespace_id: namespace&.id) end def edit diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index 7f656b8caae..d7df9bb06d2 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -4,7 +4,8 @@ module NamespacesHelper end def namespaces_options(selected = :current_user, display_path: false, extra_group: nil) - groups = current_user.owned_groups + current_user.masters_groups + groups = current_user.owned_groups + current_user.masters_groups + users = [current_user.namespace] unless extra_group.nil? || extra_group.is_a?(Group) extra_group = Group.find(extra_group) if Namespace.find(extra_group).kind == 'group' @@ -14,22 +15,9 @@ module NamespacesHelper groups |= [extra_group] end - users = [current_user.namespace] - - data_attr_group = { 'data-options-parent' => 'groups' } - data_attr_users = { 'data-options-parent' => 'users' } - - group_opts = [ - "Groups", groups.sort_by(&:human_name).map { |g| [display_path ? g.full_path : g.human_name, g.id, data_attr_group] } - ] - - users_opts = [ - "Users", users.sort_by(&:human_name).map { |u| [display_path ? u.path : u.human_name, u.id, data_attr_users] } - ] - options = [] - options << group_opts - options << users_opts + options << options_for_group(groups, display_path: display_path, type: 'group') + options << options_for_group(users, display_path: display_path, type: 'user') if selected == :current_user && current_user.namespace selected = current_user.namespace.id @@ -45,4 +33,23 @@ module NamespacesHelper avatar_icon(namespace.owner.email, size) end end + + private + + def options_for_group(namespaces, display_path:, type:) + group_label = type.pluralize + elements = namespaces.sort_by(&:human_name).map! do |n| + [display_path ? n.full_path : n.human_name, n.id, + data: { + options_parent: group_label, + visibility_level: n.visibility_level_value, + visibility: n.visibility, + name: n.name, + show_path: (type == 'group') ? group_path(n) : user_path(n), + edit_path: (type == 'group') ? edit_group_path(n) : nil + }] + end + + [group_label.camelize, elements] + end end diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 35755bc149b..46867d2d974 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -63,6 +63,68 @@ module VisibilityLevelHelper end end + def restricted_visibility_level_description(level) + level_name = Gitlab::VisibilityLevel.level_name(level) + "#{level_name.capitalize} visibility has been restricted by the administrator." + end + + def disallowed_visibility_level_description(level, form_model) + case form_model + when Project + disallowed_project_visibility_level_description(level, form_model) + when Group + disallowed_group_visibility_level_description(level, form_model) + end + end + + # Note: these messages closely mirror the form validation strings found in the project + # model and any changes or additons to these may also need to be made there. + def disallowed_project_visibility_level_description(level, project) + level_name = Gitlab::VisibilityLevel.level_name(level).downcase + reasons = [] + instructions = '' + + unless project.visibility_level_allowed_as_fork?(level) + reasons << "the fork source project has lower visibility" + end + + unless project.visibility_level_allowed_by_group?(level) + errors = visibility_level_errors_for_group(project.group, level_name) + + reasons << errors[:reason] + instructions << errors[:instruction] + end + + reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' + "This project cannot be #{level_name}#{reasons}.#{instructions}".html_safe + end + + # Note: these messages closely mirror the form validation strings found in the group + # model and any changes or additons to these may also need to be made there. + def disallowed_group_visibility_level_description(level, group) + level_name = Gitlab::VisibilityLevel.level_name(level).downcase + reasons = [] + instructions = '' + + unless group.visibility_level_allowed_by_projects?(level) + reasons << "it contains projects with higher visibility" + end + + unless group.visibility_level_allowed_by_sub_groups?(level) + reasons << "it contains sub-groups with higher visibility" + end + + unless group.visibility_level_allowed_by_parent?(level) + errors = visibility_level_errors_for_group(group.parent, level_name) + + reasons << errors[:reason] + instructions << errors[:instruction] + end + + reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' + "This group cannot be #{level_name}#{reasons}.#{instructions}".html_safe + end + def visibility_icon_description(form_model) case form_model when Project @@ -95,7 +157,18 @@ module VisibilityLevelHelper :default_group_visibility, to: :current_application_settings - def skip_level?(form_model, level) - form_model.is_a?(Project) && !form_model.visibility_level_allowed?(level) + def disallowed_visibility_level?(form_model, level) + return false unless form_model.respond_to?(:visibility_level_allowed?) + !form_model.visibility_level_allowed?(level) + end + + private + + def visibility_level_errors_for_group(group, level_name) + group_name = link_to group.name, group_path(group) + change_visiblity = link_to 'change the visibility', edit_group_path(group) + + { reason: "the visibility of #{group_name} is #{group.visibility}", + instruction: " To make this group #{level_name}, you must first #{change_visiblity} of the parent group." } end end diff --git a/app/models/group.rb b/app/models/group.rb index cb3ee032f69..190b27cf66b 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -26,6 +26,8 @@ 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 + validate :visibility_level_allowed_by_parent validates :avatar, file_size: { maximum: 200.kilobytes.to_i } @@ -102,15 +104,24 @@ class Group < Namespace full_name end - def visibility_level_allowed_by_projects - allowed_by_projects = self.projects.where('visibility_level > ?', self.visibility_level).none? + def visibility_level_allowed_by_parent?(level = self.visibility_level) + return true unless parent_id && parent_id.nonzero? - unless allowed_by_projects - 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.") - end + level <= parent.visibility_level + end - allowed_by_projects + def visibility_level_allowed_by_projects?(level = self.visibility_level) + !projects.where('visibility_level > ?', level).exists? + end + + def visibility_level_allowed_by_sub_groups?(level = self.visibility_level) + !children.where('visibility_level > ?', level).exists? + end + + def visibility_level_allowed?(level = self.visibility_level) + visibility_level_allowed_by_parent?(level) && + visibility_level_allowed_by_projects?(level) && + visibility_level_allowed_by_sub_groups?(level) end def avatar_url(**args) @@ -275,11 +286,29 @@ class Group < Namespace list_of_ids.reverse.map { |group| variables[group.id] }.compact.flatten end - protected + private def update_two_factor_requirement return unless require_two_factor_authentication_changed? || two_factor_grace_period_changed? users.find_each(&:update_two_factor_requirement) end + + def visibility_level_allowed_by_parent + return if visibility_level_allowed_by_parent? + + errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.") + end + + def visibility_level_allowed_by_projects + return if visibility_level_allowed_by_projects? + + errors.add(:visibility_level, "#{visibility} is not allowed since this group contains projects with higher visibility.") + end + + def visibility_level_allowed_by_sub_groups + return if visibility_level_allowed_by_sub_groups? + + errors.add(:visibility_level, "#{visibility} is not allowed since there are sub-groups with higher visibility.") + end end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 959af5c0d13..734a08c61fa 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -7,15 +7,15 @@ = f.label :default_branch_protection, class: 'control-label col-sm-2' .col-sm-10 = f.select :default_branch_protection, options_for_select(Gitlab::Access.protection_options, @application_setting.default_branch_protection), {}, class: 'form-control' - .form-group.project-visibility-level-holder + .form-group.visibility-level-setting = f.label :default_project_visibility, class: 'control-label col-sm-2' .col-sm-10 = render('shared/visibility_radios', model_method: :default_project_visibility, form: f, selected_level: @application_setting.default_project_visibility, form_model: Project.new) - .form-group.project-visibility-level-holder + .form-group.visibility-level-setting = f.label :default_snippet_visibility, class: 'control-label col-sm-2' .col-sm-10 = render('shared/visibility_radios', model_method: :default_snippet_visibility, form: f, selected_level: @application_setting.default_snippet_visibility, form_model: ProjectSnippet.new) - .form-group.project-visibility-level-holder + .form-group.visibility-level-setting = f.label :default_group_visibility, class: 'control-label col-sm-2' .col-sm-10 = render('shared/visibility_radios', model_method: :default_group_visibility, form: f, selected_level: @application_setting.default_group_visibility, form_model: Group.new) diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index 5698bb281b4..adffd67029a 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -112,7 +112,7 @@ %span.light (optional) = f.text_area :description, placeholder: 'Description format', class: "form-control", rows: 3, maxlength: 250 - .form-group.project-visibility-level-holder + .form-group.visibility-level-setting = f.label :visibility_level, class: 'label-light' do Visibility Level = link_to icon('question-circle'), help_page_path("public_access/public_access"), aria: { label: 'Documentation for Visibility Level' } diff --git a/app/views/shared/_visibility_level.html.haml b/app/views/shared/_visibility_level.html.haml index 73efec88bb1..192d2502aaf 100644 --- a/app/views/shared/_visibility_level.html.haml +++ b/app/views/shared/_visibility_level.html.haml @@ -1,6 +1,6 @@ - with_label = local_assigns.fetch(:with_label, true) -.form-group.project-visibility-level-holder +.form-group.visibility-level-setting - if with_label = f.label :visibility_level, class: 'control-label' do Visibility Level diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml index 182c4eebd50..0ec7677a566 100644 --- a/app/views/shared/_visibility_radios.html.haml +++ b/app/views/shared/_visibility_radios.html.haml @@ -1,15 +1,17 @@ - Gitlab::VisibilityLevel.values.each do |level| - - next if skip_level?(form_model, level) - .radio - - restricted = restricted_visibility_levels.include?(level) + - disallowed = disallowed_visibility_level?(form_model, level) + - restricted = restricted_visibility_levels.include?(level) + - disabled = disallowed || restricted + .radio{ class: [('disabled' if disabled), ('restricted' if restricted)] } = form.label "#{model_method}_#{level}" do - = form.radio_button model_method, level, checked: (selected_level == level), disabled: restricted + = form.radio_button model_method, level, checked: (selected_level == level), disabled: disabled = visibility_level_icon(level) .option-title = visibility_level_label(level) - .option-descr + .option-description = visibility_level_description(level, form_model) -- unless restricted_visibility_levels.empty? - %div - %span.info - Some visibility level settings have been restricted by the administrator. + .option-disabled-reason + - if restricted + = restricted_visibility_level_description(level) + - elsif disallowed + = disallowed_visibility_level_description(level, form_model) diff --git a/changelogs/unreleased/31273-creating-an-project-within-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml b/changelogs/unreleased/31273-creating-an-project-within-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml new file mode 100644 index 00000000000..4d21717e161 --- /dev/null +++ b/changelogs/unreleased/31273-creating-an-project-within-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml @@ -0,0 +1,6 @@ +--- +title: Ensure correct visibility level options shown on all Project, Group, and Snippets + forms +merge_request: 13442 +author: +type: fixed diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index c0e48046937..4459e227fb3 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -7,6 +7,38 @@ describe ProjectsController do let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') } let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } + describe 'GET new' do + context 'with an authenticated user' do + let(:group) { create(:group) } + + before do + sign_in(user) + end + + context 'when namespace_id param is present' do + context 'when user has access to the namespace' do + it 'renders the template' do + group.add_owner(user) + + get :new, namespace_id: group.id + + expect(response).to have_http_status(200) + expect(response).to render_template('new') + end + end + + context 'when user does not have access to the namespace' do + it 'responds with status 404' do + get :new, namespace_id: group.id + + expect(response).to have_http_status(404) + expect(response).not_to render_template('new') + end + end + end + end + end + describe 'GET index' do context 'as a user' do it 'redirects to root page' do diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index 22fb1223739..cd3dc72d3c6 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'New project' do + include Select2Helper + let(:user) { create(:admin) } before do @@ -68,26 +70,10 @@ feature 'New project' do expect(namespace.text).to eq group.name end - - context 'on validation error' do - before do - fill_in('project_path', with: 'private-group-project') - choose('Internal') - click_button('Create project') - - expect(page).to have_css '.project-edit-errors .alert.alert-danger' - end - - it 'selects the group namespace' do - namespace = find('#project_namespace_id option[selected]') - - expect(namespace.text).to eq group.name - end - end end context 'with subgroup namespace' do - let(:group) { create(:group, :private, owner: user) } + let(:group) { create(:group, owner: user) } let(:subgroup) { create(:group, parent: group) } before do @@ -101,6 +87,41 @@ feature 'New project' do expect(namespace.text).to eq subgroup.full_path end end + + context 'when changing namespaces dynamically', :js do + let(:public_group) { create(:group, :public) } + let(:internal_group) { create(:group, :internal) } + let(:private_group) { create(:group, :private) } + + before do + public_group.add_owner(user) + internal_group.add_owner(user) + private_group.add_owner(user) + visit new_project_path(namespace_id: public_group.id) + end + + it 'enables the correct visibility options' do + select2(user.namespace_id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).not_to be_disabled + + select2(public_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).not_to be_disabled + + select2(internal_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).to be_disabled + + select2(private_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).to be_disabled + end + end end context 'Import project options' do diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index c3cccbb0d95..5077c89d7b4 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -58,35 +58,82 @@ describe VisibilityLevelHelper do end end - describe "skip_level?" do + describe "disallowed_visibility_level?" do describe "forks" do let(:project) { create(:project, :internal) } let(:fork_project) { create(:project, forked_from_project: project) } - it "skips levels" do - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey end end describe "non-forked project" do let(:project) { create(:project, :internal) } - it "skips levels" do - expect(skip_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey - expect(skip_level?(project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey end end - describe "Snippet" do + describe "group" do + let(:group) { create(:group, :internal) } + + it "disallows levels" do + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + end + end + + describe "sub-group" do + let(:group) { create(:group, :private) } + let(:subgroup) { create(:group, :private, parent: group) } + + it "disallows levels" do + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::INTERNAL)).to be_truthy + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + end + end + + describe "snippet" do let(:snippet) { create(:snippet, :internal) } - it "skips levels" do - expect(skip_level?(snippet, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey - expect(skip_level?(snippet, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(snippet, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + end + end + end + + describe "disallowed_visibility_level_description" do + let(:group) { create(:group, :internal) } + let!(:subgroup) { create(:group, :internal, parent: group) } + let!(:project) { create(:project, :internal, group: group) } + + describe "project" do + it "provides correct description for disabled levels" do + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(strip_tags disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, project)) + .to include "the visibility of #{project.group.name} is internal" + end + end + + describe "group" do + it "provides correct description for disabled levels" do + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PRIVATE)).to be_truthy + expect(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, group)) + .to include "it contains projects with higher visibility", "it contains sub-groups with higher visibility" + + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(strip_tags disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, subgroup)) + .to include "the visibility of #{group.name} is internal" end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c5bfae47606..f9cd12c0ff3 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -84,6 +84,83 @@ describe Group do expect(group).not_to be_valid end end + + describe '#visibility_level_allowed_by_parent' do + let(:parent) { create(:group, :internal) } + let(:sub_group) { build(:group, parent_id: parent.id) } + + context 'without a parent' do + it 'is valid' do + sub_group.parent_id = nil + + expect(sub_group).to be_valid + end + end + + context 'with a parent' do + context 'when visibility of sub group is greater than the parent' do + it 'is invalid' do + sub_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(sub_group).to be_invalid + end + end + + context 'when visibility of sub group is lower or equal to the parent' do + [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PRIVATE].each do |level| + it 'is valid' do + sub_group.visibility_level = level + + expect(sub_group).to be_valid + end + end + 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 this group contains 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