Merge branch '224701-fj-move-members-to-group-sidebar' into 'master'

Move Members menu item to group information menu

See merge request gitlab-org/gitlab!61464

app/helpers/groups_helper.rb

@@ -8,7 +8,9 @@ module GroupsHelper
       groups#activity
       groups#subgroups
     ].tap do |paths|
-      paths << 'labels#index' if Feature.enabled?(:sidebar_refactor, current_user, default_enabled: :yaml)
+      break paths if Feature.disabled?(:sidebar_refactor, current_user, default_enabled: :yaml)
+
+      paths.concat(['labels#index', 'group_members#index'])
     end
   end
 

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

@@ -48,6 +48,13 @@
                   %span
                     = _('Labels')
 
+            - if Feature.enabled?(:sidebar_refactor, current_user, default_enabled: :yaml)
+              - if group_sidebar_link?(:group_members)
+                = nav_link(path: 'group_members#index') do
+                  = link_to group_group_members_path(@group), title: _('Members'), data: { qa_selector: 'group_members_item' } do
+                    %span
+                      = _('Members')
+
       = render_if_exists "layouts/nav/ee/epic_link", group: @group
 
       - if group_sidebar_link?(:issues)
@@ -131,18 +138,19 @@
       - if group_sidebar_link?(:wiki)
         = render 'layouts/nav/sidebar/wiki_link', wiki_url: @group.wiki.web_url
 
-      - if group_sidebar_link?(:group_members)
-        = nav_link(path: 'group_members#index') do
-          = link_to group_group_members_path(@group) do
-            .nav-icon-container
-              = sprite_icon('users')
-            %span.nav-item-name.qa-group-members-item
-              = _('Members')
-          %ul.sidebar-sub-level-items.is-fly-out-only
-            = nav_link(path: 'group_members#index', html_options: { class: "fly-out-top-item" } ) do
-              = link_to group_group_members_path(@group) do
-                %strong.fly-out-top-item-name
-                  = _('Members')
+      - if Feature.disabled?(:sidebar_refactor, current_user, default_enabled: :yaml)
+        - if group_sidebar_link?(:group_members)
+          = nav_link(path: 'group_members#index') do
+            = link_to group_group_members_path(@group) do
+              .nav-icon-container
+                = sprite_icon('users')
+              %span.nav-item-name.qa-group-members-item
+                = _('Members')
+            %ul.sidebar-sub-level-items.is-fly-out-only
+              = nav_link(path: 'group_members#index', html_options: { class: "fly-out-top-item" } ) do
+                = link_to group_group_members_path(@group) do
+                  %strong.fly-out-top-item-name
+                    = _('Members')
 
       - if group_sidebar_link?(:settings)
         = nav_link(path: group_settings_nav_link_paths) do

ee/spec/features/groups/navbar_spec.rb

@@ -127,7 +127,7 @@ RSpec.describe 'Group navbar' do
     before do
       group.add_owner(user)
 
-      insert_after_nav_item(_('Members'), new_nav_item: settings_nav_item)
+      insert_after_nav_item(_('Analytics'), new_nav_item: settings_nav_item)
       insert_after_nav_item(_('Settings'), new_nav_item: administration_nav_item)
 
       visit group_path(group)
@@ -142,7 +142,7 @@ RSpec.describe 'Group navbar' do
 
       group.add_owner(user)
 
-      insert_after_nav_item(_('Members'), new_nav_item: settings_nav_item)
+      insert_after_nav_item(_('Analytics'), new_nav_item: settings_nav_item)
       insert_after_nav_item(
         _('Settings'),
         new_nav_item: {
@@ -178,7 +178,7 @@ RSpec.describe 'Group navbar' do
 
       stub_licensed_features(security_dashboard: true, group_level_compliance_dashboard: true)
 
-      insert_after_nav_item(_('Members'), new_nav_item: settings_nav_item)
+      insert_after_nav_item(_('Analytics'), new_nav_item: settings_nav_item)
       insert_after_nav_item(_('Settings'), new_nav_item: administration_nav_item)
 
       visit group_path(group)

spec/features/groups/navbar_spec.rb

@@ -16,11 +16,7 @@ RSpec.describe 'Group navbar' do
       group_information_nav_item,
       {
         nav_item: _('Issues'),
-        nav_sub_items: [
-          _('List'),
-          _('Board'),
-          _('Milestones')
-        ]
+        nav_sub_items: issues_nav_items
       },
       {
         nav_item: _('Merge requests'),
@@ -33,11 +29,12 @@ RSpec.describe 'Group navbar' do
         nav_sub_items: []
       },
       (analytics_nav_item if Gitlab.ee?),
-      {
-        nav_item: _('Members'),
-        nav_sub_items: []
-      }
-    ]
+      members_nav_item
+    ].compact
+  end
+
+  let(:members_nav_item) do
+    nil
   end
 
   before do
@@ -80,4 +77,40 @@ RSpec.describe 'Group navbar' do
 
     it_behaves_like 'verified navigation bar'
   end
+
+  context 'when feature flag :sidebar_refactor is disabled' do
+    let(:group_information_nav_item) do
+      {
+        nav_item: _('Group overview'),
+        nav_sub_items: [
+          _('Details'),
+          _('Activity')
+        ]
+      }
+    end
+
+    let(:members_nav_item) do
+      {
+        nav_item: _('Members'),
+        nav_sub_items: []
+      }
+    end
+
+    let(:issues_nav_items) do
+      [
+        _('List'),
+        _('Board'),
+        _('Labels'),
+        _('Milestones')
+      ]
+    end
+
+    before do
+      stub_feature_flags(sidebar_refactor: false)
+
+      visit group_path(group)
+    end
+
+    it_behaves_like 'verified navigation bar'
+  end
 end

spec/support/shared_contexts/navbar_structure_context.rb

@@ -182,21 +182,26 @@ RSpec.shared_context 'group navbar structure' do
       nav_item: _('Group information'),
       nav_sub_items: [
         _('Activity'),
-        _('Labels')
+        _('Labels'),
+        _('Members')
       ]
     }
   end
 
+  let(:issues_nav_items) do
+    [
+      _('List'),
+      _('Board'),
+      _('Milestones')
+    ]
+  end
+
   let(:structure) do
     [
       group_information_nav_item,
       {
         nav_item: _('Issues'),
-        nav_sub_items: [
-          _('List'),
-          _('Board'),
-          _('Milestones')
-        ]
+        nav_sub_items: issues_nav_items
       },
       {
         nav_item: _('Merge requests'),
@@ -208,11 +213,7 @@ RSpec.shared_context 'group navbar structure' do
         nav_item: _('Kubernetes'),
         nav_sub_items: []
       },
-      (analytics_nav_item if Gitlab.ee?),
-      {
-        nav_item: _('Members'),
-        nav_sub_items: []
-      }
+      (analytics_nav_item if Gitlab.ee?)
     ]
   end
 end

spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb

@@ -25,6 +25,13 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
       expect(rendered).not_to have_link('Details', href: details_group_path(group))
     end
 
+    it 'has a link to the members page' do
+      render
+
+      expect(rendered).to have_selector('.sidebar-top-level-items > li.home a[title="Members"]')
+      expect(rendered).to have_link('Members', href: group_group_members_path(group))
+    end
+
     context 'when feature flag :sidebar_refactor is disabled' do
       before do
         stub_feature_flags(sidebar_refactor: false)
@@ -41,6 +48,33 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
 
         expect(rendered).to have_link('Details', href: details_group_path(group))
       end
+
+      it 'does not have a link to the members page' do
+        render
+
+        expect(rendered).not_to have_selector('.sidebar-top-level-items > li.home a[title="Members"]')
+      end
+    end
+  end
+
+  describe 'Members' do
+    it 'does not have a Members menu' do
+      render
+
+      expect(rendered).not_to have_selector('.nav-item-name', text: 'Members')
+    end
+
+    context 'when feature flag :sidebar_refactor is disabled' do
+      before do
+        stub_feature_flags(sidebar_refactor: false)
+      end
+
+      it 'has a Member menu' do
+        render
+
+        expect(rendered).to have_selector('.nav-item-name', text: 'Members')
+        expect(rendered).to have_link('Members', href: group_group_members_path(group))
+      end
     end
   end
 end