Merge branch '209020-add-administration-menu-for-group-owners' into 'master'

Move settings only relevant to the parent group into an "Administration" menu

See merge request gitlab-org/gitlab!28057

app/views/layouts/nav/sidebar/_group.html.haml

@@ -162,4 +162,6 @@
 
             = render_if_exists "groups/ee/settings_nav"
 
+        = render_if_exists "groups/ee/administration_nav"
+
     = render 'shared/sidebar_toggle_button'

doc/subscriptions/index.md

@@ -173,7 +173,7 @@ To see the status of your GitLab.com subscription, log into GitLab.com and go to
   1. Go to **User Avatar > Settings**.
   1. Click **Billing**.
 - For groups:
-  1. From the group page (*not* from a project within the group), go to **Settings > Billing**.
+  1. From the group page (*not* from a project within the group), go to **Administration > Billing**.
 
 The following table describes details of your subscription for groups:
 
@@ -427,7 +427,7 @@ CI pipeline minutes are the execution time for your [pipelines](../ci/pipelines/
 
 Quotas apply to:
 
-- Groups, where the minutes are shared across all members of the group, its subgroups, and nested projects. To view the group's usage, navigate to the group, then **{settings}** **Settings > Usage Quotas**.
+- Groups, where the minutes are shared across all members of the group, its subgroups, and nested projects. To view the group's usage, navigate to the group, then **{settings}** **Administration > Usage Quotas**.
 - Your personal account, where the minutes are available for your personal projects. To view and buy personal minutes, click your avatar, then **{settings}** **Settings > Pipeline quota**.
 
 Only pipeline minutes for GitLab shared runners are restricted. If you have a specific runner set up for your projects, there is no limit to your build time on GitLab.com.
@@ -448,10 +448,10 @@ main quota. Additional minutes:
 
 To purchase additional minutes for your group on GitLab.com:
 
-1. From your group, go to **{settings}** **Settings > Usage Quotas**.
+1. From your group, go to **{settings}** **Administration > Usage Quotas**.
 1. Locate the subscription card that's linked to your group on GitLab.com, click **Buy more CI minutes**, and complete the details about the transaction.
 1. Once we have processed your payment, the extra CI minutes will be synced to your group.
-1. To confirm the available CI minutes, go to your group, then **{settings}** **Settings > Usage Quotas**.
+1. To confirm the available CI minutes, go to your group, then **{settings}** **Administration > Usage Quotas**.
    The **Additional minutes** displayed now includes the purchased additional CI minutes, plus any minutes rolled over from last month.
 
 To purchase additional minutes for your personal namespace:

doc/user/group/saml_sso/index.md

@@ -24,7 +24,7 @@ Note the following:
 
 ## Configuring your Identity Provider
 
-1. Navigate to the group and click **Settings > SAML SSO**.
+1. Navigate to the group and click **Administration > SAML SSO**.
 1. Configure your SAML server using the **Assertion consumer service URL** and **Identifier**. Alternatively GitLab provides [metadata XML configuration](#metadata-configuration). See [your identity provider's documentation](#providers) for more details.
 1. Configure the SAML response to include a NameID that uniquely identifies each user.
 1. Configure required assertions using the [table below](#assertions).
@@ -116,7 +116,7 @@ This feature is similar to the [Credentials inventory for self-managed instances
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/34648) in GitLab 12.9.
 
 Groups with group-managed accounts can disallow forking of projects to destinations outside the group.
-To do so, enable the "Prohibit outer forks" option in **Settings > SAML SSO**.
+To do so, enable the "Prohibit outer forks" option in **Administration > SAML SSO**.
 When enabled, projects within the group can only be forked to other destinations within the group (including its subgroups).
 
 ##### Other restrictions for Group-managed accounts
@@ -146,7 +146,7 @@ assertions to be able to create a user.
 
 GitLab provides metadata XML that can be used to configure your Identity Provider.
 
-1. Navigate to the group and click **Settings > SAML SSO**.
+1. Navigate to the group and click **Administration > SAML SSO**.
 1. Copy the provided **GitLab metadata URL**.
 1. Follow your Identity Provider's documentation and paste the metadata URL when it is requested.
 
@@ -154,7 +154,7 @@ GitLab provides metadata XML that can be used to configure your Identity Provide
 
 Once you've set up your identity provider to work with GitLab, you'll need to configure GitLab to use it for authentication:
 
-1. Navigate to the group's **Settings > SAML SSO**.
+1. Navigate to the group's **Administration > SAML SSO**.
 1. Find the SSO URL from your Identity Provider and enter it the **Identity provider single sign on URL** field.
 1. Find and enter the fingerprint for the SAML token signing certificate in the **Certificate** field.
 1. Click the **Enable SAML authentication for this group** toggle switch.
@@ -281,14 +281,14 @@ If the information information you need isn't listed above you may wish to check
 To link SAML to your existing GitLab.com account:
 
 1. Sign in to your GitLab.com account.
-1. Locate the SSO URL for the group you are signing in to. A group Admin can find this on the group's **Settings > SAML SSO** page.
+1. Locate the SSO URL for the group you are signing in to. A group Admin can find this on the group's **Administration > SAML SSO** page.
 1. Visit the SSO URL and click **Authorize**.
 1. Enter your credentials on the Identity Provider if prompted.
 1. You will be redirected back to GitLab.com and should now have access to the group. In the future, you can use SAML to sign in to GitLab.com.
 
 ## Signing in to GitLab.com with SAML
 
-1. Locate the SSO URL for the group you are signing in to. A group Admin can find this on a group's **Settings > SAML SSO** page. If configured, it might also be possible to sign in to GitLab starting from your Identity Provider.
+1. Locate the SSO URL for the group you are signing in to. A group Admin can find this on a group's **Administration > SAML SSO** page. If configured, it might also be possible to sign in to GitLab starting from your Identity Provider.
 1. Visit the SSO URL and click the **Sign in with Single Sign-On** button.
 1. Enter your credentials on the Identity Provider if prompted.
 1. You will be signed in to GitLab.com and redirected to the group.

doc/user/group/saml_sso/scim_setup.md

@@ -30,7 +30,7 @@ The following identity providers are supported:
 
 Once [Single sign-on](index.md) has been configured, we can:
 
-1. Navigate to the group and click **Settings > SAML SSO**.
+1. Navigate to the group and click **Administration > SAML SSO**.
 1. Click on the **Generate a SCIM token** button.
 1. Save the token and URL so they can be used in the next step.
 

ee/app/helpers/ee/groups_helper.rb

@@ -11,13 +11,8 @@ module EE
         .count
     end
 
-    override :group_nav_link_paths
-    def group_nav_link_paths
-      if ::Gitlab::CurrentSettings.should_check_namespace_plan? && can?(current_user, :admin_group, @group)
-        super + %w[billings#index saml_providers#show]
-      else
-        super
-      end
+    def group_administration_nav_link_paths
+      %w[saml_providers#show usage_quotas#index billings#index]
     end
 
     def size_limit_message_for_group(group)
@@ -86,6 +81,28 @@ module EE
       can?(current_user, :read_group_activity_analytics, @group)
     end
 
+    def show_usage_quotas_in_sidebar?
+      License.feature_available?(:usage_quotas)
+    end
+
+    def show_billing_in_sidebar?
+      ::Gitlab::CurrentSettings.should_check_namespace_plan?
+    end
+
+    def show_administration_nav?(group)
+      group.parent.nil? && can?(current_user, :admin_group, @group)
+    end
+
+    def administration_nav_path(group)
+      if show_saml_in_sidebar?(group)
+        group_saml_providers_path(group)
+      elsif show_usage_quotas_in_sidebar?
+        group_usage_quotas_path(group)
+      elsif show_billing_in_sidebar?
+        group_billings_path(group)
+      end
+    end
+
     private
 
     def get_group_sidebar_links

ee/app/views/groups/ee/_administration_nav.html.haml

+- return unless show_administration_nav?(@group)
+
+= nav_link(path: group_administration_nav_link_paths) do
+  = link_to administration_nav_path(@group) do
+    .nav-icon-container
+      = sprite_icon('admin')
+    %span.nav-item-name
+      = _('Administration')
+  %ul.sidebar-sub-level-items
+    = nav_link(path: group_administration_nav_link_paths, html_options: { class: "fly-out-top-item" } ) do
+      = link_to administration_nav_path(@group) do
+        %strong.fly-out-top-item-name
+          = _('Administration')
+    %li.divider.fly-out-top-item
+    - if show_saml_in_sidebar?(@group)
+      = nav_link(path: 'saml_providers#show') do
+        = link_to group_saml_providers_path(@group), title: _('SAML SSO') do
+          %span
+            = _('SAML SSO')
+
+    - if show_usage_quotas_in_sidebar?
+      = nav_link(path: 'usage_quotas#index') do
+        = link_to group_usage_quotas_path(@group), title: s_('UsageQuota|Usage Quotas') do
+          %span
+            = s_('UsageQuota|Usage Quotas')
+
+    - if show_billing_in_sidebar?
+      = nav_link(path: 'billings#index') do
+        = link_to group_billings_path(@group), title: _('Billing') do
+          %span
+            = _('Billing')

ee/app/views/groups/ee/_settings_nav.html.haml

@@ -4,12 +4,6 @@
       %span
         LDAP Synchronization
 
-- if show_saml_in_sidebar?(@group)
-  = nav_link(path: 'saml_providers#show') do
-    = link_to group_saml_providers_path(@group), title: 'SAML SSO', class: 'qa-group-saml-sso-link' do
-      %span
-        SAML SSO
-
 - if @group.feature_available?(:group_webhooks) || show_promotions?
   = nav_link(path: 'hooks#index') do
     = link_to group_hooks_path(@group), title: 'Webhooks' do
@@ -21,16 +15,3 @@
     = link_to group_audit_events_path(@group), title: 'Audit Events', data: { qa_selector: 'audit_events_settings_link' } do
       %span
         Audit Events
-
--# Check if this is not a sub group
-- if License.feature_available?(:usage_quotas) && @group.parent.nil?
-  = nav_link(path: 'usage_quota#index') do
-    = link_to group_usage_quotas_path(@group), title: s_('UsageQuota|Usage Quotas') do
-      %span
-        = s_('UsageQuota|Usage Quotas')
-
-- if Gitlab::CurrentSettings.should_check_namespace_plan?
-  = nav_link(path: 'billings#index') do
-    = link_to group_billings_path(@group), title: 'Billing' do
-      %span
-        Billing

ee/app/views/groups/settings/_allowed_email_domain.html.haml

-- return unless group.feature_available?(:group_allowed_email_domains)
-
-- read_only = group.parent_id.present?
+- return if !group.feature_available?(:group_allowed_email_domains) || group.parent_id.present?
 
 %h5= _('Restrict membership by email')
 
 = f.fields_for :allowed_email_domain do |allowed_email_domain_form|
   .form-group
-    - if read_only
-      = allowed_email_domain_form.text_field :domain, value: group.root_ancestor_allowed_email_domain&.domain, class: 'form-control', disabled: true, placeholder: _('No value set by top-level parent group.')
-      .form-text.text-muted
-        = _('Email domain is not editable in subgroups. Value inherited from top-level parent group.')
-    - else
     = allowed_email_domain_form.text_field :domain, class: 'form-control', placeholder: _('Enter domain')
     .form-text.text-muted
       - read_more_link = link_to(_('Read more'), help_page_path('user/group/index', anchor: 'allowed-domain-restriction-premium-only'))

ee/app/views/groups/settings/_ip_restriction.html.haml

-- return unless group.feature_available?(:group_ip_restriction)
-
-- read_only = group.parent_id.present?
+- return if !group.feature_available?(:group_ip_restriction) || group.parent_id.present?
 
 %h5= _('Restrict access by IP address')
 
 .form-group
-  - if read_only
-    = f.text_field :ip_restriction_ranges, value: group.root_ancestor&.ip_restriction_ranges, class: 'form-control', disabled: true, placeholder: _('No value set by top-level parent group.')
-    .form-text.text-muted
-      = _('IP address restriction is not editable in subgroups. Value inherited from top-level parent group.')
-  - else
   = f.text_field :ip_restriction_ranges, class: 'form-control', data: { qa_selector: 'ip_restriction_field' }, placeholder: _('Enter IP address range')
   .form-text.text-muted
     - read_more_link = link_to(_('Read more'), help_page_path('user/group/index', anchor: 'ip-access-restriction-ultimate'))

ee/changelogs/unreleased/209020-add-administration-menu-for-group-owners.yml

+---
+title: Move settings only relevant to the parent group into an "Administration" menu
+merge_request: 28057
+author:
+type: changed

ee/spec/features/groups/navbar_spec.rb

@@ -73,6 +73,31 @@ describe 'Group navbar' do
       group.add_owner(user)
 
       insert_after_nav_item(_('Members'), new_nav_item: settings_nav_item)
+      insert_after_nav_item(_('Settings'), new_nav_item: administration_nav_item)
+
+      visit group_path(group)
+    end
+
+    it_behaves_like 'verified navigation bar'
+  end
+
+  context 'when SAML SSO is available' do
+    before do
+      stub_licensed_features(group_saml: true)
+
+      group.add_owner(user)
+
+      insert_after_nav_item(_('Members'), new_nav_item: settings_nav_item)
+      insert_after_nav_item(
+        _('Settings'),
+        new_nav_item: {
+          nav_item: _('Administration'),
+          nav_sub_items: [
+            _('SAML SSO'),
+            s_('UsageQuota|Usage Quotas')
+          ]
+        }
+      )
 
       visit group_path(group)
     end
@@ -98,6 +123,7 @@ describe 'Group navbar' do
       )
 
       insert_after_nav_item(_('Members'), new_nav_item: settings_nav_item)
+      insert_after_nav_item(_('Settings'), new_nav_item: administration_nav_item)
 
       visit group_path(group)
     end

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe GroupsHelper do
+  using RSpec::Parameterized::TableSyntax
+
   let(:owner) { create(:user, group_view: :security_dashboard) }
   let(:current_user) { owner }
   let(:group) { create(:group, :private) }
@@ -171,4 +173,110 @@ describe GroupsHelper do
       end
     end
   end
+
+  describe '#show_usage_quotas_in_sidebar?' do
+    where(:usage_quotas_feature_available?, :expected) do
+      true  | true
+      false | false
+    end
+
+    with_them do
+      it do
+        stub_licensed_features(usage_quotas: usage_quotas_feature_available?)
+
+        expect(helper.show_usage_quotas_in_sidebar?).to eq(expected)
+      end
+    end
+  end
+
+  describe '#show_billing_in_sidebar?' do
+    where(:should_check_namespace_plan_return_value, :expected) do
+      true  | true
+      false | false
+    end
+
+    with_them do
+      it do
+        allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(should_check_namespace_plan_return_value)
+
+        expect(helper.show_billing_in_sidebar?).to eq(expected)
+      end
+    end
+  end
+
+  describe '#show_administration_nav?' do
+    context 'when user does not have admin_group permissions' do
+      before do
+        allow(helper).to receive(:can?).and_return(true)
+        allow(helper).to receive(:can?).with(current_user, :admin_group, group).and_return(false)
+      end
+
+      it 'returns false' do
+        expect(helper.show_administration_nav?(group)).to be false
+      end
+    end
+
+    context 'when user has admin_group permissions' do
+      before do
+        allow(helper).to receive(:can?).and_return(false)
+        allow(helper).to receive(:can?).with(current_user, :admin_group, group).and_return(true)
+      end
+
+      it 'returns true' do
+        allow(helper).to receive(:show_saml_in_sidebar?).with(group).and_return(true)
+
+        expect(helper.show_administration_nav?(group)).to be true
+      end
+
+      it 'returns false for a subgroup' do
+        subgroup = create(:group, :private, parent: group)
+
+        expect(helper.show_administration_nav?(subgroup)).to be false
+      end
+    end
+  end
+
+  describe '#administration_nav_path' do
+    context 'when SAML providers feature is available' do
+      before do
+        allow(helper).to receive(:show_saml_in_sidebar?).with(group).and_return(true)
+      end
+
+      it 'returns path to SAML providers' do
+        expect(helper.administration_nav_path(group)).to eq(group_saml_providers_path(group))
+      end
+    end
+
+    context 'when SAML providers feature is not available' do
+      before do
+        allow(helper).to receive(:show_saml_in_sidebar?).with(group).and_return(false)
+      end
+
+      context 'and usage quotas feature is available' do
+        before do
+          allow(helper).to receive(:show_usage_quotas_in_sidebar?).and_return(true)
+        end
+
+        it 'returns path to usage quotas' do
+          expect(helper.administration_nav_path(group)).to eq(group_usage_quotas_path(group))
+        end
+      end
+
+      context 'and usage quotas feature is not available' do
+        before do
+          allow(helper).to receive(:show_usage_quotas_in_sidebar?).and_return(false)
+        end
+
+        context 'and billing feature is available' do
+          before do
+            allow(helper).to receive(:show_billing_in_sidebar?).and_return(true)
+          end
+
+          it 'returns path to billing' do
+            expect(helper.administration_nav_path(group)).to eq(group_billings_path(group))
+          end
+        end
+      end
+    end
+  end
 end

ee/spec/views/groups/edit.html.haml_spec.rb

@@ -50,35 +50,13 @@ describe 'groups/edit.html.haml' do
     end
 
     context 'subgroup' do
-      shared_examples 'renders read-only ip_restriction setting of root ancestor' do
-        it 'renders disabled ranges of root ancestor in comma separated format' do
-          render
-
-          expect(rendered).to render_template('groups/settings/_ip_restriction')
-          expect(rendered).to(have_field('group_ip_restriction_ranges',
-                                         { disabled: true,
-                                           with: ranges.join(",") }))
-        end
-      end
-
       let(:group) { create(:group, :nested) }
 
-      before do
-        ranges.each do |range|
-          create(:ip_restriction, group: group.parent, range: range)
-        end
-      end
-
-      context 'with single subnet' do
-        let(:ranges) { ['192.168.0.0/24'] }
-
-        it_behaves_like 'renders read-only ip_restriction setting of root ancestor'
-      end
-
-      context 'with multiple subnets' do
-        let(:ranges) { ['192.168.0.0/24', '192.168.1.0/8'] }
+      it 'does not show ip_restriction setting' do
+        render
 
-        it_behaves_like 'renders read-only ip_restriction setting of root ancestor'
+        expect(rendered).to render_template('groups/settings/_ip_restriction')
+        expect(rendered).not_to have_field('group_ip_restriction_attributes_range')
       end
     end
 
@@ -120,18 +98,11 @@ describe 'groups/edit.html.haml' do
     context 'subgroup' do
       let(:group) { create(:group, :nested) }
 
-      before do
-        create(:allowed_email_domain, group: group.parent)
-        group.build_allowed_email_domain
-      end
-
-      it 'show read-only allowed_email_domain setting of root ancestor' do
+      it 'does not show allowed_email_domain setting' do
         render
 
         expect(rendered).to render_template('groups/settings/_allowed_email_domain')
-        expect(rendered).to(have_field('group_allowed_email_domain_attributes_domain',
-                                       { disabled: true,
-                                         with: 'gitlab.com' }))
+        expect(rendered).not_to have_field('group_allowed_email_domain_attributes_domain')
       end
     end
 

locale/gitlab.pot

@@ -1584,6 +1584,9 @@ msgstr ""
 msgid "AdminUsers|You are about to permanently delete the user %{username}. This will delete all of the issues, merge requests, and groups linked to them. To avoid data loss, consider using the %{strong_start}block user%{strong_end} feature instead. Once you %{strong_start}Delete user%{strong_end}, it cannot be undone or recovered."
 msgstr ""
 
+msgid "Administration"
+msgstr ""
+
 msgid "Advanced"
 msgstr ""
 
@@ -7364,9 +7367,6 @@ msgstr ""
 msgid "Email display name"
 msgstr ""
 
-msgid "Email domain is not editable in subgroups. Value inherited from top-level parent group."
-msgstr ""
-
 msgid "Email not verified. Please verify your email in Salesforce."
 msgstr ""
 
@@ -10695,9 +10695,6 @@ msgstr ""
 msgid "IP Address"
 msgstr ""
 
-msgid "IP address restriction is not editable in subgroups. Value inherited from top-level parent group."
-msgstr ""
-
 msgid "IP subnet restriction only allowed for top-level groups"
 msgstr ""
 
@@ -13511,9 +13508,6 @@ msgstr ""
 msgid "No thanks, don't show this again"
 msgstr ""
 
-msgid "No value set by top-level parent group."
-msgstr ""
-
 msgid "No vulnerabilities found for this group"
 msgstr ""
 

spec/support/shared_contexts/navbar_structure_context.rb

@@ -121,8 +121,16 @@ RSpec.shared_context 'group navbar structure' do
         _('Projects'),
         _('CI / CD'),
         _('Webhooks'),
-        _('Audit Events'),
-        _('Usage Quotas')
+        _('Audit Events')
+      ]
+    }
+  end
+
+  let(:administration_nav_item) do
+    {
+      nav_item: _('Administration'),
+      nav_sub_items: [
+        s_('UsageQuota|Usage Quotas')
       ]
     }
   end