From a30257c0321e872646fe945739482fdeadbba4c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Tue, 15 Aug 2017 12:03:54 -0500
Subject: [PATCH 01/32] Add some data attributes for options in namespace
 selector

---
 app/helpers/namespaces_helper.rb | 35 ++++++++++++++++----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb
index 7f656b8caae..df8b247aff5 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -4,7 +4,9 @@ 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]
+    options = []
 
     unless extra_group.nil? || extra_group.is_a?(Group)
       extra_group = Group.find(extra_group) if Namespace.find(extra_group).kind == 'group'
@@ -14,22 +16,8 @@ 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)
+    options << options_for_group(users, display_path)
 
     if selected == :current_user && current_user.namespace
       selected = current_user.namespace.id
@@ -45,4 +33,17 @@ module NamespacesHelper
       avatar_icon(namespace.owner.email, size)
     end
   end
+
+  private
+
+  def options_for_group(namespaces, display_path)
+    type = namespaces.first.is_a?(Group) ? 'group' : 'users'
+
+    elements = namespaces.sort_by(&:human_name).map! do |n|
+      [display_path ? n.full_path : n.human_name, n.id,
+       data: { options_parent: type, visibility_level: n.visibility_level_value, name: n.human_name }]
+    end
+
+    [type.camelize, elements]
+  end
 end

From d413f8e4e426e2cb2dc61d5a72d84a7dc67a28c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Tue, 15 Aug 2017 20:25:47 -0500
Subject: [PATCH 02/32] Add validation for visibility level of sub groups

Sub groups should not have a visibility level higher than its parent.
---
 app/models/group.rb       |  9 +++++++++
 spec/models/group_spec.rb | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/app/models/group.rb b/app/models/group.rb
index 2816a68257c..15355418d05 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_parent
 
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
@@ -102,6 +103,14 @@ class Group < Namespace
     full_name
   end
 
+  def visibility_level_allowed_by_parent
+    return if parent_id.blank?
+
+    if parent && (visibility_level > parent.visibility_level)
+      errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.")
+    end
+  end
+
   def visibility_level_allowed_by_projects
     allowed_by_projects = self.projects.where('visibility_level > ?', self.visibility_level).none?
 
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index c5bfae47606..a3310cf1dce 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -84,6 +84,39 @@ 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
   end
 
   describe '.visible_to_user' do

From b2b9d63f9b7301cbef9d1e1b8d4ad3cefeacf35d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Wed, 23 Aug 2017 11:51:11 -0500
Subject: [PATCH 03/32] Add validation to check visibility level of sub groups.

---
 app/models/group.rb       | 20 ++++++++++++++----
 spec/models/group_spec.rb | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/app/models/group.rb b/app/models/group.rb
index 15355418d05..fdd175341b3 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
 
-    unless allowed_by_projects
+  def visibility_level_allowed_by_sub_groups
+    check_visibility_level_for(:children)
+  end
+
+  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 a3310cf1dce..c3342afb7fd 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

From af6968a15859a309cbb93a0327fc1d4be36041bc Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Thu, 24 Aug 2017 16:21:10 -0500
Subject: [PATCH 04/32] separate visibility_level_allowed logic from model
 validators

---
 app/models/group.rb | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/app/models/group.rb b/app/models/group.rb
index fdd175341b3..b093e0b200c 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -105,33 +105,35 @@ class Group < Namespace
   end
 
   def visibility_level_allowed_by_parent
-    return if parent_id.blank?
+    return if visibility_level_allowed_by_parent?
 
-    if parent && (visibility_level > parent.visibility_level)
-      errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.")
-    end
+    errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.")
   end
 
   def visibility_level_allowed_by_projects
-    check_visibility_level_for(: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
-    check_visibility_level_for(:children)
+    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
 
-  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?
+  def visibility_level_allowed_by_parent?(level = self.visibility_level)
+    return true unless parent_id.present? || parent
 
-    if children_have_higher_visibility
-      children_label = children_type == :projects ? 'projects' : 'sub groups'
-      level_name = Gitlab::VisibilityLevel.level_name(visibility_level).downcase
+    level <= parent.visibility_level
+  end
 
-      self.errors.add(:visibility_level, "#{level_name} is not allowed since there are #{children_label} with higher visibility.")
-    end
+  def visibility_level_allowed_by_projects?(level = self.visibility_level)
+    projects.where('visibility_level > ?', level).none?
+  end
 
-    children_have_higher_visibility
+  def visibility_level_allowed_by_sub_groups?(level = self.visibility_level)
+    children.where('visibility_level > ?', level).none?
   end
 
   def avatar_url(**args)

From 37edce19dd2f006ef2117ca8a9f05398e704a11c Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Thu, 24 Aug 2017 17:19:49 -0500
Subject: [PATCH 05/32] recognize instances where group visibility levels are
 unavailable

---
 app/helpers/visibility_level_helper.rb | 3 ++-
 app/models/group.rb                    | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index 35755bc149b..ecc554f85d0 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -96,6 +96,7 @@ module VisibilityLevelHelper
             to: :current_application_settings
 
   def skip_level?(form_model, level)
-    form_model.is_a?(Project) && !form_model.visibility_level_allowed?(level)
+    return false unless form_model.respond_to?(:visibility_level_allowed?)
+    !form_model.visibility_level_allowed?(level)
   end
 end
diff --git a/app/models/group.rb b/app/models/group.rb
index b093e0b200c..257a5075d2c 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -136,6 +136,12 @@ class Group < Namespace
     children.where('visibility_level > ?', level).none?
   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)
     # We use avatar_path instead of overriding avatar_url because of carrierwave.
     # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864

From 852f50977111fc11511682217e81cdce908c318f Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Thu, 24 Aug 2017 17:24:29 -0500
Subject: [PATCH 06/32] rename skip_level? to disallowed_visibitility_level?

---
 app/helpers/visibility_level_helper.rb        |  2 +-
 app/views/shared/_visibility_radios.html.haml |  2 +-
 spec/helpers/visibility_level_helper_spec.rb  | 26 +++++++++----------
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index ecc554f85d0..c8401b784f6 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -95,7 +95,7 @@ module VisibilityLevelHelper
             :default_group_visibility,
             to: :current_application_settings
 
-  def skip_level?(form_model, 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
diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml
index 182c4eebd50..7b9453943d1 100644
--- a/app/views/shared/_visibility_radios.html.haml
+++ b/app/views/shared/_visibility_radios.html.haml
@@ -1,5 +1,5 @@
 - Gitlab::VisibilityLevel.values.each do |level|
-  - next if skip_level?(form_model, level)
+  - next if disallowed_visibility_level?(form_model, level)
   .radio
     - restricted = restricted_visibility_levels.include?(level)
     = form.label "#{model_method}_#{level}" do
diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb
index c3cccbb0d95..428eb664138 100644
--- a/spec/helpers/visibility_level_helper_spec.rb
+++ b/spec/helpers/visibility_level_helper_spec.rb
@@ -58,35 +58,35 @@ 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
       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

From 96c2672da8edff081702702a4a499bae14d27ff6 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Thu, 24 Aug 2017 17:55:43 -0500
Subject: [PATCH 07/32] add tests for groups within visibility level helper

---
 spec/helpers/visibility_level_helper_spec.rb | 23 +++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb
index 428eb664138..9a85fbe5258 100644
--- a/spec/helpers/visibility_level_helper_spec.rb
+++ b/spec/helpers/visibility_level_helper_spec.rb
@@ -80,7 +80,28 @@ describe VisibilityLevelHelper do
       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 "disallows levels" do

From d6d713f6bac9cd1840e45ed43911b99adba9c07b Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 11:57:16 -0500
Subject: [PATCH 08/32] fix failing tests due to new group visibility
 restrictions

---
 spec/features/projects/new_project_spec.rb | 2 +-
 spec/models/group_spec.rb                  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb
index 22fb1223739..9f8b1996249 100644
--- a/spec/features/projects/new_project_spec.rb
+++ b/spec/features/projects/new_project_spec.rb
@@ -87,7 +87,7 @@ feature 'New project' do
     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
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index c3342afb7fd..f9cd12c0ff3 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -127,7 +127,7 @@ describe Group 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.')
+          expect(internal_group.errors[:visibility_level]).to include('private is not allowed since this group contains projects with higher visibility.')
         end
       end
 
@@ -149,7 +149,7 @@ describe Group 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.')
+          expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub-groups with higher visibility.')
         end
       end
 

From 69f679ed3476a887e67a591114c50ebcd1efa1a6 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 13:00:01 -0500
Subject: [PATCH 09/32] rename .project-visibility-level-holder to
 .visibility-level-setting and move from projects.scss to settings.scss

---
 app/assets/stylesheets/pages/projects.scss    | 22 -------------------
 app/assets/stylesheets/pages/settings.scss    | 22 +++++++++++++++++++
 .../application_settings/_form.html.haml      |  6 ++---
 app/views/projects/new.html.haml              |  2 +-
 app/views/shared/_visibility_level.html.haml  |  2 +-
 5 files changed, 27 insertions(+), 27 deletions(-)

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..9a142f338e8 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/app/assets/stylesheets/pages/settings.scss
@@ -143,6 +143,28 @@
   }
 }
 
+.visibility-level-setting {
+  .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;
+    }
+  }
+}
+
 .prometheus-metrics-monitoring {
   .panel {
     .panel-toggle {
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 647e0a772b1..e2ea96b855d 100644
--- a/app/views/projects/new.html.haml
+++ b/app/views/projects/new.html.haml
@@ -111,7 +111,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

From 10a7478ed0b9cbc4b3d4a316f4e124796dbac495 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 13:46:49 -0500
Subject: [PATCH 10/32] fix disabled state style for visibility level options

---
 app/assets/stylesheets/pages/settings.scss    | 8 +++++++-
 app/views/shared/_visibility_radios.html.haml | 9 +++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss
index 9a142f338e8..f7f8119994b 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/app/assets/stylesheets/pages/settings.scss
@@ -147,7 +147,7 @@
   .radio {
     margin-bottom: 10px;
 
-    i {
+    i.fa {
       margin: 2px 0;
       font-size: 20px;
     }
@@ -162,6 +162,12 @@
       margin-left: 29px;
       color: $project-option-descr-color;
     }
+
+    &.disabled {
+      i.fa {
+        opacity: 0.5;
+      }
+    }
   }
 }
 
diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml
index 7b9453943d1..d8d1093d4e3 100644
--- a/app/views/shared/_visibility_radios.html.haml
+++ b/app/views/shared/_visibility_radios.html.haml
@@ -1,9 +1,10 @@
 - Gitlab::VisibilityLevel.values.each do |level|
-  - next if disallowed_visibility_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)

From 04e3e609daa54a62b0caf5a5ed5ade1cdb8c5eae Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 13:48:15 -0500
Subject: [PATCH 11/32] show alternate description text for disabled visibility
 settings

---
 app/assets/stylesheets/pages/settings.scss    | 15 ++++++++++++++-
 app/views/shared/_visibility_radios.html.haml | 11 ++++++-----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss
index f7f8119994b..41a6ba2023a 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/app/assets/stylesheets/pages/settings.scss
@@ -158,15 +158,28 @@
       color: $gl-text-color;
     }
 
-    .option-descr {
+    .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;
+      }
     }
   }
 }
diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml
index d8d1093d4e3..8a80ccd4030 100644
--- a/app/views/shared/_visibility_radios.html.haml
+++ b/app/views/shared/_visibility_radios.html.haml
@@ -8,9 +8,10 @@
       = 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
+          This visibility level has been restricted by the administrator.
+        - elsif disallowed
+          This option is not available the visibility of parent or child items prevents it.

From 8cf504a71c326033a5a0885fa950a7d2c37ca93c Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 14:07:44 -0500
Subject: [PATCH 12/32] don't check disabled radio inputs

---
 app/views/shared/_visibility_radios.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml
index 8a80ccd4030..13eeaeefa9f 100644
--- a/app/views/shared/_visibility_radios.html.haml
+++ b/app/views/shared/_visibility_radios.html.haml
@@ -4,7 +4,7 @@
   - 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: disabled
+      = form.radio_button model_method, level, checked: (selected_level == level && !disabled), disabled: disabled
       = visibility_level_icon(level)
       .option-title
         = visibility_level_label(level)

From fc95395c5dc8b297e831a51bcec04c0644eb06d2 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 15:01:56 -0500
Subject: [PATCH 13/32] display specific reasons when visibility options are
 disabled

---
 app/helpers/visibility_level_helper.rb        | 50 +++++++++++++++++++
 app/views/shared/_visibility_radios.html.haml |  4 +-
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index c8401b784f6..347f796fdc1 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -63,6 +63,56 @@ module VisibilityLevelHelper
     end
   end
 
+  def restricted_visibility_level_description(level)
+    level_name = Gitlab::VisibilityLevel.level_name(level)
+    "#{level_name.capitalize} visibilitiy 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
+
+  def disallowed_project_visibility_level_description(level, project)
+    level_name = Gitlab::VisibilityLevel.level_name(level).downcase
+    reasons = []
+
+    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)
+      reasons << "the visibility of #{project.group.name} is #{project.group.visibility}"
+    end
+
+    reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
+    "This project cannot be #{level_name}#{reasons}."
+  end
+
+  def disallowed_group_visibility_level_description(level, group)
+    level_name = Gitlab::VisibilityLevel.level_name(level).downcase
+    reasons = []
+
+    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)
+      reasons << "the visibility of its parent group is #{group.parent.visibility}"
+    end
+
+    reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
+    "This group cannot be #{level_name}#{reasons}."
+  end
+
   def visibility_icon_description(form_model)
     case form_model
     when Project
diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml
index 13eeaeefa9f..4a656ccfeac 100644
--- a/app/views/shared/_visibility_radios.html.haml
+++ b/app/views/shared/_visibility_radios.html.haml
@@ -12,6 +12,6 @@
         = visibility_level_description(level, form_model)
       .option-disabled-reason
         - if restricted
-          This visibility level has been restricted by the administrator.
+          = restricted_visibility_level_description(level)
         - elsif disallowed
-          This option is not available the visibility of parent or child items prevents it.
+          = disallowed_visibility_level_description(level, form_model)

From aff72ece729ab33afa8c3a4a83e97c5383ce5ed8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Fri, 25 Aug 2017 15:03:16 -0500
Subject: [PATCH 14/32] Fix broken spec.

parent_id is being set to 0 by RSpec.
---
 app/models/group.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/group.rb b/app/models/group.rb
index 257a5075d2c..78084f76869 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -123,7 +123,7 @@ class Group < Namespace
   end
 
   def visibility_level_allowed_by_parent?(level = self.visibility_level)
-    return true unless parent_id.present? || parent
+    return true unless parent_id && parent_id.nonzero?
 
     level <= parent.visibility_level
   end

From 1c3b3facb7a2296ca10dcb57b4f3b8ba416ffd3b Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 15:40:21 -0500
Subject: [PATCH 15/32] add tests for new visibility_level_helper methods

---
 spec/helpers/visibility_level_helper_spec.rb | 26 ++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb
index 9a85fbe5258..bd15abf5469 100644
--- a/spec/helpers/visibility_level_helper_spec.rb
+++ b/spec/helpers/visibility_level_helper_spec.rb
@@ -111,4 +111,30 @@ describe VisibilityLevelHelper do
       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(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(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, subgroup))
+          .to include "the visibility of its parent group is internal"
+      end
+    end
+  end
 end

From b7b49c4a15e592904a41df72aa5ab9b791abc0ef Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 15:54:05 -0500
Subject: [PATCH 16/32] prefer !x.exists? instead of x.none? to prevent
 unnecessary instantiation of records

---
 app/models/group.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/group.rb b/app/models/group.rb
index 78084f76869..d4d201f05b7 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -129,11 +129,11 @@ class Group < Namespace
   end
 
   def visibility_level_allowed_by_projects?(level = self.visibility_level)
-    projects.where('visibility_level > ?', level).none?
+    !projects.where('visibility_level > ?', level).exists?
   end
 
   def visibility_level_allowed_by_sub_groups?(level = self.visibility_level)
-    children.where('visibility_level > ?', level).none?
+    !children.where('visibility_level > ?', level).exists?
   end
 
   def visibility_level_allowed?(level = self.visibility_level)

From b63c08b2638b01bdbb09ed26c9cce6f330f4a97e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Fri, 25 Aug 2017 17:09:15 -0500
Subject: [PATCH 17/32] Build Project in context of Namespace if available

---
 app/controllers/projects_controller.rb | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 1d24563a6a6..8ad99c01e15 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -3,6 +3,7 @@ class ProjectsController < Projects::ApplicationController
   include ExtractsPath
 
   before_action :authenticate_user!, except: [:index, :show, :activity, :refs]
+  before_action :namespace, only: [:new]
   before_action :project, except: [:index, :new, :create]
   before_action :repository, except: [:index, :new, :create]
   before_action :assign_ref_vars, only: [:show], if: :repo_exists?
@@ -20,7 +21,7 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def new
-    @project = Project.new
+    build_project
   end
 
   def edit
@@ -395,4 +396,12 @@ class ProjectsController < Projects::ApplicationController
   def project_export_enabled
     render_404 unless current_application_settings.project_export_enabled?
   end
+
+  def namespace
+    @namespace ||= Namespace.find(params[:namespace_id]) if params[:namespace_id].present?
+  end
+
+  def build_project
+    @project ||= namespace ? namespace.projects.new : Project.new
+  end
 end

From 77a5d9db83ac54980eccfa57735af1ed01ba702c Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 19:42:33 -0500
Subject: [PATCH 18/32] add polyfill for NodeList.prototype.forEach
 (unsupported in Internet Explorer)

---
 app/assets/javascripts/commons/polyfills.js          | 1 +
 app/assets/javascripts/commons/polyfills/nodelist.js | 7 +++++++
 2 files changed, 8 insertions(+)
 create mode 100644 app/assets/javascripts/commons/polyfills/nodelist.js

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);
+    }
+  };
+}

From 62be748ef81a9cd1d4e075940da2df20251605e2 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 19:43:33 -0500
Subject: [PATCH 19/32] dynamically disable/enable visibility options when
 changing namespaces in new project form

---
 app/assets/javascripts/dispatcher.js         |  2 ++
 app/assets/javascripts/project_visibility.js | 37 ++++++++++++++++++++
 app/helpers/namespaces_helper.rb             |  2 +-
 3 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 app/assets/javascripts/project_visibility.js

diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index 2bba7f55de1..feb8735752b 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..b43c8ba56e2
--- /dev/null
+++ b/app/assets/javascripts/project_visibility.js
@@ -0,0 +1,37 @@
+function setVisibilityOptions(namespaceSelector) {
+  if (!namespaceSelector || !('selectedIndex' in namespaceSelector)) {
+    return;
+  }
+  const selectedNamespace = namespaceSelector.options[namespaceSelector.selectedIndex];
+  const { name, visibility, visibilityLevel } = 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.innerText = `This project cannot be ${optionName} because the visibility of ${name} is ${visibility}.`;
+        }
+      } 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/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb
index df8b247aff5..ca149ac2c20 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -41,7 +41,7 @@ module NamespacesHelper
 
     elements = namespaces.sort_by(&:human_name).map! do |n|
       [display_path ? n.full_path : n.human_name, n.id,
-       data: { options_parent: type, visibility_level: n.visibility_level_value, name: n.human_name }]
+       data: { options_parent: type, visibility_level: n.visibility_level_value, visibility: n.visibility, name: n.name }]
     end
 
     [type.camelize, elements]

From f82ec4a7709c73b843b72f69e70201917366db75 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Fri, 25 Aug 2017 19:56:11 -0500
Subject: [PATCH 20/32] add CHANGELOG.md entry for !13442

---
 ...ternal-sub-group-gives-the-option-to-set-it-a-public.yml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 changelogs/unreleased/31273-creating-an-project-within-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml

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

From 3488e8f011e493a89df60cd0db0fff4086f42bd5 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Sat, 26 Aug 2017 01:50:12 -0500
Subject: [PATCH 21/32] add tests for dynamically changing namespace selector
 within new project form

---
 spec/features/projects/new_project_spec.rb | 53 +++++++++++++++-------
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb
index 9f8b1996249..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,22 +70,6 @@ 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
@@ -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

From 0a8d0924fe9a1525b92423411dc1bfcdc9760833 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Sat, 26 Aug 2017 02:43:45 -0500
Subject: [PATCH 22/32] expand the help text with links and additional
 instructions

---
 app/assets/javascripts/project_visibility.js | 8 ++++++--
 app/helpers/namespaces_helper.rb             | 9 ++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/project_visibility.js b/app/assets/javascripts/project_visibility.js
index b43c8ba56e2..c3f5e8cb907 100644
--- a/app/assets/javascripts/project_visibility.js
+++ b/app/assets/javascripts/project_visibility.js
@@ -3,7 +3,7 @@ function setVisibilityOptions(namespaceSelector) {
     return;
   }
   const selectedNamespace = namespaceSelector.options[namespaceSelector.selectedIndex];
-  const { name, visibility, visibilityLevel } = selectedNamespace.dataset;
+  const { name, visibility, visibilityLevel, showPath, editPath } = selectedNamespace.dataset;
 
   document.querySelectorAll('.visibility-level-setting .radio').forEach((option) => {
     const optionInput = option.querySelector('input[type=radio]');
@@ -18,7 +18,11 @@ function setVisibilityOptions(namespaceSelector) {
         optionInput.disabled = true;
         const reason = option.querySelector('.option-disabled-reason');
         if (reason) {
-          reason.innerText = `This project cannot be ${optionName} because the visibility of ${name} is ${visibility}.`;
+          reason.innerHTML =
+            `This project cannot be ${optionName} because the visibility of
+            <a href="${showPath}">${name}</a> is ${visibility}. To make this project
+            ${optionName}, you must first <a href="${editPath}">change the visibility</a>
+            of the parent group.`;
         }
       } else {
         option.classList.remove('disabled');
diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb
index ca149ac2c20..33d6875a704 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -41,7 +41,14 @@ module NamespacesHelper
 
     elements = namespaces.sort_by(&:human_name).map! do |n|
       [display_path ? n.full_path : n.human_name, n.id,
-       data: { options_parent: type, visibility_level: n.visibility_level_value, visibility: n.visibility, name: n.name }]
+       data: {
+         options_parent: type,
+         visibility_level: n.visibility_level_value,
+         visibility: n.visibility,
+         name: n.name,
+         show_path: n.is_a?(Group) ? group_path(n) : user_path(n),
+         edit_path: n.is_a?(Group) ? edit_group_path(n) : nil
+       }]
     end
 
     [type.camelize, elements]

From 6f03ddcdc3af1fbb840498a0e4765458079f0b0f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Tue, 29 Aug 2017 00:49:01 -0500
Subject: [PATCH 23/32] Address some suggestions from first code review

---
 app/controllers/projects_controller.rb | 11 +------
 app/helpers/namespaces_helper.rb       | 15 +++++-----
 app/helpers/visibility_level_helper.rb | 12 ++++----
 app/models/group.rb                    | 40 +++++++++++++-------------
 app/models/project.rb                  |  6 ++--
 lib/gitlab/visibility_level.rb         | 18 ++++++++++++
 6 files changed, 56 insertions(+), 46 deletions(-)

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 8ad99c01e15..51cf37b9438 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -3,7 +3,6 @@ class ProjectsController < Projects::ApplicationController
   include ExtractsPath
 
   before_action :authenticate_user!, except: [:index, :show, :activity, :refs]
-  before_action :namespace, only: [:new]
   before_action :project, except: [:index, :new, :create]
   before_action :repository, except: [:index, :new, :create]
   before_action :assign_ref_vars, only: [:show], if: :repo_exists?
@@ -21,7 +20,7 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def new
-    build_project
+    @project ||= Project.new(params.permit(:namespace_id))
   end
 
   def edit
@@ -396,12 +395,4 @@ class ProjectsController < Projects::ApplicationController
   def project_export_enabled
     render_404 unless current_application_settings.project_export_enabled?
   end
-
-  def namespace
-    @namespace ||= Namespace.find(params[:namespace_id]) if params[:namespace_id].present?
-  end
-
-  def build_project
-    @project ||= namespace ? namespace.projects.new : Project.new
-  end
 end
diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb
index 33d6875a704..3c784272df2 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -6,7 +6,6 @@ module NamespacesHelper
   def namespaces_options(selected = :current_user, display_path: false, extra_group: nil)
     groups  = current_user.owned_groups + current_user.masters_groups
     users   = [current_user.namespace]
-    options = []
 
     unless extra_group.nil? || extra_group.is_a?(Group)
       extra_group = Group.find(extra_group) if Namespace.find(extra_group).kind == 'group'
@@ -16,8 +15,9 @@ module NamespacesHelper
       groups |= [extra_group]
     end
 
-    options << options_for_group(groups, display_path)
-    options << options_for_group(users, display_path)
+    options = []
+    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
@@ -36,13 +36,12 @@ module NamespacesHelper
 
   private
 
-  def options_for_group(namespaces, display_path)
-    type = namespaces.first.is_a?(Group) ? 'group' : 'users'
-
+  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: type,
+         options_parent: group_label,
          visibility_level: n.visibility_level_value,
          visibility: n.visibility,
          name: n.name,
@@ -51,6 +50,6 @@ module NamespacesHelper
        }]
     end
 
-    [type.camelize, elements]
+    [group_label.camelize, elements]
   end
 end
diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index 347f796fdc1..caadc12019c 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -65,7 +65,7 @@ module VisibilityLevelHelper
 
   def restricted_visibility_level_description(level)
     level_name = Gitlab::VisibilityLevel.level_name(level)
-    "#{level_name.capitalize} visibilitiy has been restricted by the administrator."
+    "#{level_name.capitalize} visibility has been restricted by the administrator."
   end
 
   def disallowed_visibility_level_description(level, form_model)
@@ -82,11 +82,11 @@ module VisibilityLevelHelper
     reasons = []
 
     unless project.visibility_level_allowed_as_fork?(level)
-      reasons << "the fork source project has lower visibility"
+      reasons << project.visibility_error_for(:fork, level: level_name)
     end
 
     unless project.visibility_level_allowed_by_group?(level)
-      reasons << "the visibility of #{project.group.name} is #{project.group.visibility}"
+      reasons << project.visibility_error_for(:group, level: level_name, group_level: project.group.visibility)
     end
 
     reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
@@ -98,15 +98,15 @@ module VisibilityLevelHelper
     reasons = []
 
     unless group.visibility_level_allowed_by_projects?(level)
-      reasons << "it contains projects with higher visibility"
+      reasons << group.visibility_error_for(:projects, level: level_name)
     end
 
     unless group.visibility_level_allowed_by_sub_groups?(level)
-      reasons << "it contains sub-groups with higher visibility"
+      reasons << group.visibility_error_for(:sub_groups, level: level_name)
     end
 
     unless group.visibility_level_allowed_by_parent?(level)
-      reasons << "the visibility of its parent group is #{group.parent.visibility}"
+      reasons << group.visibility_error_for(:parent, level: level_name, parent_level: group.parent.visibility)
     end
 
     reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
diff --git a/app/models/group.rb b/app/models/group.rb
index d4d201f05b7..0e6d88f6a91 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -26,7 +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_sub_groups
   validate :visibility_level_allowed_by_parent
 
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
@@ -104,24 +104,6 @@ class Group < Namespace
     full_name
   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
-
   def visibility_level_allowed_by_parent?(level = self.visibility_level)
     return true unless parent_id && parent_id.nonzero?
 
@@ -304,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_error_for(:parent, level: visibility, parent_level: parent.visibility))
+  end
+
+  def visibility_level_allowed_by_projects
+    return if visibility_level_allowed_by_projects?
+
+    errors.add(:visibility_level, visibility_error_for(:projects, level: visibility))
+  end
+
+  def visibility_level_allowed_by_sub_groups
+    return if visibility_level_allowed_by_sub_groups?
+
+    errors.add(:visibility_level, visibility_error_for(:sub_groups, level: visibility))
+  end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index d5324ceac31..088fe5e1fed 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -671,14 +671,16 @@ class Project < ActiveRecord::Base
 
     level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase
     group_level_name = Gitlab::VisibilityLevel.level_name(self.group.visibility_level).downcase
-    self.errors.add(:visibility_level, "#{level_name} is not allowed in a #{group_level_name} group.")
+
+    self.errors.add(:visibility_level, visibility_error_for(:group, level: level_name, group_level: group_level_name))
   end
 
   def visibility_level_allowed_as_fork
     return if visibility_level_allowed_as_fork?
 
     level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase
-    self.errors.add(:visibility_level, "#{level_name} is not allowed since the fork source project has lower visibility.")
+
+    self.errors.add(:visibility_level, visibility_error_for(:fork, level: level_name))
   end
 
   def check_wiki_path_conflict
diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb
index c60bd91ea6e..ef5c4d9a3b2 100644
--- a/lib/gitlab/visibility_level.rb
+++ b/lib/gitlab/visibility_level.rb
@@ -130,5 +130,23 @@ module Gitlab
     def visibility=(level)
       self[visibility_level_field] = Gitlab::VisibilityLevel.level_value(level)
     end
+
+    def visibility_errors_template
+      @visibility_errors ||= {
+        'Project' => {
+          group: "%{level} is not allowed in a %{group_level} group",
+          fork: "%{level} is not allowed since the fork source project has lower visibility"
+        },
+        'Group' => {
+          parent: "%{level} is not allowed since the parent group has a %{parent_level} visibility",
+          projects: "%{level} is not allowed since this group contains projects with higher visibility",
+          sub_groups: "%{level} is not allowed since there are sub-groups with higher visibility"
+        }
+      }
+    end
+
+    def visibility_error_for(section, variables)
+      visibility_errors_template.dig(model_name.to_s, section) % variables
+    end
   end
 end

From b9b0b37b3695d5925c3ba6cd90cdefcc3c67ba6e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Wed, 30 Aug 2017 12:24:49 -0500
Subject: [PATCH 24/32] Add check for access to Namespace

---
 app/controllers/projects_controller.rb       |  5 ++-
 app/helpers/namespaces_helper.rb             |  4 +--
 spec/controllers/projects_controller_spec.rb | 32 ++++++++++++++++++++
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 51cf37b9438..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(params.permit(:namespace_id))
+    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 3c784272df2..d7df9bb06d2 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -45,8 +45,8 @@ module NamespacesHelper
          visibility_level: n.visibility_level_value,
          visibility: n.visibility,
          name: n.name,
-         show_path: n.is_a?(Group) ? group_path(n) : user_path(n),
-         edit_path: n.is_a?(Group) ? edit_group_path(n) : nil
+         show_path: (type == 'group') ? group_path(n) : user_path(n),
+         edit_path: (type == 'group') ? edit_group_path(n) : nil
        }]
     end
 
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

From 6cad21efbe25ffe1c0a3a153a25ed9601b50c427 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Wed, 30 Aug 2017 14:37:08 -0500
Subject: [PATCH 25/32] revert changes to visibility level helpers from 6f03ddc

---
 app/helpers/visibility_level_helper.rb | 10 +++++-----
 app/models/group.rb                    |  6 +++---
 app/models/project.rb                  |  6 ++----
 lib/gitlab/visibility_level.rb         | 18 ------------------
 4 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index caadc12019c..a13127a6365 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -82,11 +82,11 @@ module VisibilityLevelHelper
     reasons = []
 
     unless project.visibility_level_allowed_as_fork?(level)
-      reasons << project.visibility_error_for(:fork, level: level_name)
+      reasons << "the fork source project has lower visibility"
     end
 
     unless project.visibility_level_allowed_by_group?(level)
-      reasons << project.visibility_error_for(:group, level: level_name, group_level: project.group.visibility)
+      reasons << "the visibility of #{project.group.name} is #{project.group.visibility}"
     end
 
     reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
@@ -98,15 +98,15 @@ module VisibilityLevelHelper
     reasons = []
 
     unless group.visibility_level_allowed_by_projects?(level)
-      reasons << group.visibility_error_for(:projects, level: level_name)
+      reasons << "it contains projects with higher visibility"
     end
 
     unless group.visibility_level_allowed_by_sub_groups?(level)
-      reasons << group.visibility_error_for(:sub_groups, level: level_name)
+      reasons << "it contains sub-groups with higher visibility"
     end
 
     unless group.visibility_level_allowed_by_parent?(level)
-      reasons << group.visibility_error_for(:parent, level: level_name, parent_level: group.parent.visibility)
+      reasons << "the visibility of its parent group is #{group.parent.visibility}"
     end
 
     reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
diff --git a/app/models/group.rb b/app/models/group.rb
index 0e6d88f6a91..3778b832a47 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -297,18 +297,18 @@ class Group < Namespace
   def visibility_level_allowed_by_parent
     return if visibility_level_allowed_by_parent?
 
-    errors.add(:visibility_level, visibility_error_for(:parent, level: visibility, parent_level: parent.visibility))
+    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_error_for(:projects, level: visibility))
+    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_error_for(:sub_groups, level: visibility))
+    errors.add(:visibility_level, "#{visibility} is not allowed since there are sub-groups with higher visibility.")
   end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index 088fe5e1fed..d5324ceac31 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -671,16 +671,14 @@ class Project < ActiveRecord::Base
 
     level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase
     group_level_name = Gitlab::VisibilityLevel.level_name(self.group.visibility_level).downcase
-
-    self.errors.add(:visibility_level, visibility_error_for(:group, level: level_name, group_level: group_level_name))
+    self.errors.add(:visibility_level, "#{level_name} is not allowed in a #{group_level_name} group.")
   end
 
   def visibility_level_allowed_as_fork
     return if visibility_level_allowed_as_fork?
 
     level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase
-
-    self.errors.add(:visibility_level, visibility_error_for(:fork, level: level_name))
+    self.errors.add(:visibility_level, "#{level_name} is not allowed since the fork source project has lower visibility.")
   end
 
   def check_wiki_path_conflict
diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb
index ef5c4d9a3b2..c60bd91ea6e 100644
--- a/lib/gitlab/visibility_level.rb
+++ b/lib/gitlab/visibility_level.rb
@@ -130,23 +130,5 @@ module Gitlab
     def visibility=(level)
       self[visibility_level_field] = Gitlab::VisibilityLevel.level_value(level)
     end
-
-    def visibility_errors_template
-      @visibility_errors ||= {
-        'Project' => {
-          group: "%{level} is not allowed in a %{group_level} group",
-          fork: "%{level} is not allowed since the fork source project has lower visibility"
-        },
-        'Group' => {
-          parent: "%{level} is not allowed since the parent group has a %{parent_level} visibility",
-          projects: "%{level} is not allowed since this group contains projects with higher visibility",
-          sub_groups: "%{level} is not allowed since there are sub-groups with higher visibility"
-        }
-      }
-    end
-
-    def visibility_error_for(section, variables)
-      visibility_errors_template.dig(model_name.to_s, section) % variables
-    end
   end
 end

From b1a14f5fc94aa900e06aaddb3d13a5775ccffc54 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Wed, 30 Aug 2017 14:40:09 -0500
Subject: [PATCH 26/32] add notes to the disabled visibility setting string
 helper to ensure changes are reflected in the model as well

---
 app/helpers/visibility_level_helper.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index a13127a6365..19550d37b2f 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -77,6 +77,8 @@ module VisibilityLevelHelper
     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 = []
@@ -93,6 +95,8 @@ module VisibilityLevelHelper
     "This project cannot be #{level_name}#{reasons}."
   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 = []

From 6dd69950edf6b98600d0bead148969ef347bc1ec Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Wed, 30 Aug 2017 15:08:30 -0500
Subject: [PATCH 27/32] add links and instructions to disabled visibility
 option help text

---
 app/helpers/visibility_level_helper.rb | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index 19550d37b2f..569872e528b 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -82,17 +82,22 @@ module VisibilityLevelHelper
   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)
-      reasons << "the visibility of #{project.group.name} is #{project.group.visibility}"
+      group = link_to project.group.name, group_path(project.group)
+      change_visiblity = link_to 'change the visibility', edit_group_path(project.group)
+
+      reasons << "the visibility of #{group} is #{project.group.visibility}"
+      instructions << " To make this project #{level_name}, you must first #{change_visiblity} of the parent group."
     end
 
     reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
-    "This project cannot be #{level_name}#{reasons}."
+    "This project cannot be #{level_name}#{reasons}.#{instructions}".html_safe
   end
 
   # Note: these messages closely mirror the form validation strings found in the group

From 8a72203a589bc3b7a74f5dd60a3c607d21436e6a Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Wed, 30 Aug 2017 15:51:03 -0500
Subject: [PATCH 28/32] update test to match new disabled option description

---
 spec/helpers/visibility_level_helper_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb
index bd15abf5469..0375fa600ed 100644
--- a/spec/helpers/visibility_level_helper_spec.rb
+++ b/spec/helpers/visibility_level_helper_spec.rb
@@ -120,7 +120,7 @@ describe VisibilityLevelHelper do
     describe "project" do
       it "provides correct description for disabled levels" do
         expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy
-        expect(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, project))
+        expect(strip_tags disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, project))
           .to include "the visibility of #{project.group.name} is internal"
       end
     end

From 129823664bb2287545b0402823366261151273ca Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Wed, 30 Aug 2017 16:01:57 -0500
Subject: [PATCH 29/32] enhance disabled group visibility options with links
 and instructions

---
 app/helpers/visibility_level_helper.rb       | 9 +++++++--
 spec/helpers/visibility_level_helper_spec.rb | 4 ++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index 569872e528b..e21a7b04bb4 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -105,6 +105,7 @@ module VisibilityLevelHelper
   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"
@@ -115,11 +116,15 @@ module VisibilityLevelHelper
     end
 
     unless group.visibility_level_allowed_by_parent?(level)
-      reasons << "the visibility of its parent group is #{group.parent.visibility}"
+      parent_group = link_to group.parent.name, group_path(group.parent)
+      change_visiblity = link_to 'change the visibility', edit_group_path(group.parent)
+
+      reasons << "the visibility of #{parent_group} is #{group.parent.visibility}"
+      instructions << " To make this group #{level_name}, you must first #{change_visiblity} of the parent group."
     end
 
     reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
-    "This group cannot be #{level_name}#{reasons}."
+    "This group cannot be #{level_name}#{reasons}.#{instructions}".html_safe
   end
 
   def visibility_icon_description(form_model)
diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb
index 0375fa600ed..5077c89d7b4 100644
--- a/spec/helpers/visibility_level_helper_spec.rb
+++ b/spec/helpers/visibility_level_helper_spec.rb
@@ -132,8 +132,8 @@ describe VisibilityLevelHelper do
           .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(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, subgroup))
-          .to include "the visibility of its parent group is internal"
+        expect(strip_tags disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, subgroup))
+          .to include "the visibility of #{group.name} is internal"
       end
     end
   end

From 8ffbab216b90c7743ee15a652bb72eaf460bc3ba Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Wed, 30 Aug 2017 16:27:20 -0500
Subject: [PATCH 30/32] ensure disabled radio options still show up as checked
 when necessary

---
 app/views/shared/_visibility_radios.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml
index 4a656ccfeac..0ec7677a566 100644
--- a/app/views/shared/_visibility_radios.html.haml
+++ b/app/views/shared/_visibility_radios.html.haml
@@ -4,7 +4,7 @@
   - 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), disabled: disabled
+      = form.radio_button model_method, level, checked: (selected_level == level), disabled: disabled
       = visibility_level_icon(level)
       .option-title
         = visibility_level_label(level)

From 68de5dcba28d83089fd563434ba9d1ba1d882b76 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Wed, 30 Aug 2017 18:38:06 -0500
Subject: [PATCH 31/32] Fix error reported by Flay

---
 app/helpers/visibility_level_helper.rb | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index e21a7b04bb4..4b4f7c6a57a 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -89,11 +89,10 @@ module VisibilityLevelHelper
     end
 
     unless project.visibility_level_allowed_by_group?(level)
-      group = link_to project.group.name, group_path(project.group)
-      change_visiblity = link_to 'change the visibility', edit_group_path(project.group)
+      errors = visibility_level_errors_for_group(project.group, level_name)
 
-      reasons << "the visibility of #{group} is #{project.group.visibility}"
-      instructions << " To make this project #{level_name}, you must first #{change_visiblity} of the parent group."
+      reasons << errors[:reason]
+      instructions << errors[:instruction]
     end
 
     reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
@@ -116,11 +115,10 @@ module VisibilityLevelHelper
     end
 
     unless group.visibility_level_allowed_by_parent?(level)
-      parent_group = link_to group.parent.name, group_path(group.parent)
-      change_visiblity = link_to 'change the visibility', edit_group_path(group.parent)
+      errors = visibility_level_errors_for_group(group.parent, level_name)
 
-      reasons << "the visibility of #{parent_group} is #{group.parent.visibility}"
-      instructions << " To make this group #{level_name}, you must first #{change_visiblity} of the parent group."
+      reasons << errors[:reason]
+      instructions << errors[:instruction]
     end
 
     reasons = reasons.any? ? ' because ' + reasons.to_sentence : ''
@@ -163,4 +161,14 @@ module VisibilityLevelHelper
     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 = link_to group.name, group_path(group)
+    change_visiblity = link_to 'change the visibility', edit_group_path(group)
+
+    { reason: "the visibility of #{group} is #{group.visibility}",
+      instruction: " To make this group #{level_name}, you must first #{change_visiblity} of the parent group." }
+  end
 end

From cf37f0b173abacaef36660f1c9875f8fee8b78d8 Mon Sep 17 00:00:00 2001
From: Mike Greiling <mike@pixelcog.com>
Date: Wed, 30 Aug 2017 19:33:24 -0500
Subject: [PATCH 32/32] fix variable naming conflict

---
 app/helpers/visibility_level_helper.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb
index 4b4f7c6a57a..46867d2d974 100644
--- a/app/helpers/visibility_level_helper.rb
+++ b/app/helpers/visibility_level_helper.rb
@@ -165,10 +165,10 @@ module VisibilityLevelHelper
   private
 
   def visibility_level_errors_for_group(group, level_name)
-    group = link_to group.name, group_path(group)
+    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} is #{group.visibility}",
+    { 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