Merge branch '29893-change-menu-locations' into 'master'

Resolve "Inconsistent location of members page between groups and projects"

Closes #29893

See merge request !11560

23 files changed, 73 insertions, 100 deletions

diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index 8f716696605..ae19592ecbe 100644
--- a/app/assets/javascripts/dispatcher.js
+++ b/app/assets/javascripts/dispatcher.js
@@ -322,7 +322,7 @@ import PerformanceBar from './performance_bar';
           new gl.Members();
           new UsersSelect();
           break;
-        case 'projects:settings:members:show':
+        case 'projects:project_members:index':
           new gl.MemberExpirationDate('.js-access-expiration-date-groups');
           new GroupsSelect();
           new gl.MemberExpirationDate();
diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb
index 47d9ae350ae..c6b1e443de6 100644
--- a/app/controllers/concerns/membership_actions.rb
+++ b/app/controllers/concerns/membership_actions.rb
@@ -70,7 +70,7 @@ module MembershipActions
 
   def members_page_url
     if membershipable.is_a?(Project)
-      project_settings_members_path(membershipable)
+      project_project_members_path(membershipable)
     else
       polymorphic_url([membershipable, :members])
     end
diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb
index 8fc614b414d..f59200d3b1f 100644
--- a/app/controllers/projects/group_links_controller.rb
+++ b/app/controllers/projects/group_links_controller.rb
@@ -22,7 +22,7 @@ class Projects::GroupLinksController < Projects::ApplicationController
       flash[:alert] = 'Please select a group.'
     end
 
-    redirect_to project_settings_members_path(project)
+    redirect_to project_project_members_path(project)
   end
 
   def update
@@ -36,7 +36,7 @@ class Projects::GroupLinksController < Projects::ApplicationController
 
     respond_to do |format|
       format.html do
-        redirect_to project_settings_members_path(project), status: 302
+        redirect_to project_project_members_path(project), status: 302
       end
       format.js { head :ok }
     end
diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index 57a6686f66c..f8ff7413b53 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -6,8 +6,23 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access]
 
   def index
-    sort = params[:sort].presence || sort_value_name
-    redirect_to project_settings_members_path(@project, sort: sort)
+    @sort = params[:sort].presence || sort_value_name
+    @group_links = @project.project_group_links
+
+    @skip_groups = @group_links.pluck(:group_id)
+    @skip_groups << @project.namespace_id unless @project.personal?
+    @skip_groups += @project.group.ancestors.pluck(:id) if @project.group
+
+    @project_members = MembersFinder.new(@project, current_user).execute
+
+    if params[:search].present?
+      @project_members = @project_members.joins(:user).merge(User.search(params[:search]))
+      @group_links = @group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id))
+    end
+
+    @project_members = @project_members.sort(@sort).page(params[:page])
+    @requesters = AccessRequestsFinder.new(@project).execute(current_user)
+    @project_member = @project.project_members.new
   end
 
   def update
@@ -19,7 +34,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   def resend_invite
-    redirect_path = project_settings_members_path(@project)
+    redirect_path = project_project_members_path(@project)
 
     @project_member = @project.project_members.find(params[:id])
 
@@ -42,7 +57,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
       return render_404
     end
 
-    redirect_to(project_settings_members_path(project),
+    redirect_to(project_project_members_path(project),
                 notice: notice)
   end
 
diff --git a/app/controllers/projects/settings/members_controller.rb b/app/controllers/projects/settings/members_controller.rb
deleted file mode 100644
index 54f9dceddef..00000000000
--- a/app/controllers/projects/settings/members_controller.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-module Projects
-  module Settings
-    class MembersController < Projects::ApplicationController
-      include SortingHelper
-
-      def show
-        @sort = params[:sort].presence || sort_value_name
-        @group_links = @project.project_group_links
-
-        @skip_groups = @group_links.pluck(:group_id)
-        @skip_groups << @project.namespace_id unless @project.personal?
-        @skip_groups += @project.group.ancestors.pluck(:id) if @project.group
-
-        @project_members = MembersFinder.new(@project, current_user).execute
-
-        if params[:search].present?
-          @project_members = @project_members.joins(:user).merge(User.search(params[:search]))
-          @group_links = @group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id))
-        end
-
-        @project_members = @project_members.sort(@sort).page(params[:page])
-        @requesters = AccessRequestsFinder.new(@project).execute(current_user)
-        @project_member = @project.project_members.new
-      end
-    end
-  end
-end
diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb
index b5f4bbe97dc..2e36d0fdb5a 100644
--- a/app/helpers/gitlab_routing_helper.rb
+++ b/app/helpers/gitlab_routing_helper.rb
@@ -97,7 +97,7 @@ module GitlabRoutingHelper
 
   ## Members
   def project_members_url(project, *args)
-    project_project_members_url(project)
+    project_project_members_url(project, *args)
   end
 
   def project_member_path(project_member, *args)
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 5022b291f7f..25969adb649 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -267,15 +267,15 @@ module ProjectsHelper
 
   def tab_ability_map
     {
-      environments: :read_environment,
-      milestones:   :read_milestone,
-      snippets:     :read_project_snippet,
-      settings:     :admin_project,
-      builds:       :read_build,
-      labels:       :read_label,
-      issues:       :read_issue,
-      team:         :read_project_member,
-      wiki:         :read_wiki
+      environments:     :read_environment,
+      milestones:       :read_milestone,
+      snippets:         :read_project_snippet,
+      settings:         :admin_project,
+      builds:           :read_build,
+      labels:           :read_label,
+      issues:           :read_issue,
+      project_members:  :read_project_member,
+      wiki:             :read_wiki
     }
   end
 
diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb
index 8c44f4b0934..fd7ab59ce64 100644
--- a/app/helpers/search_helper.rb
+++ b/app/helpers/search_helper.rb
@@ -75,7 +75,7 @@ module SearchHelper
         { category: "Current Project", label: "Merge Requests", url: project_merge_requests_path(@project) },
         { category: "Current Project", label: "Milestones",     url: project_milestones_path(@project) },
         { category: "Current Project", label: "Snippets",       url: project_snippets_path(@project) },
-        { category: "Current Project", label: "Members",        url: project_settings_members_path(@project) },
+        { category: "Current Project", label: "Members",        url: project_project_members_path(@project) },
         { category: "Current Project", label: "Wiki",           url: project_wikis_path(@project) }
       ]
     else
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index 14deb46eee3..fb90bb4b472 100644
--- a/app/views/layouts/nav/_project.html.haml
+++ b/app/views/layouts/nav/_project.html.haml
@@ -57,16 +57,17 @@
           %span
             Snippets
 
+    - if project_nav_tab? :project_members
+      = nav_link(controller: :project_members) do
+        = link_to project_project_members_path(@project), title: 'Members', class: 'shortcuts-members' do
+          %span
+            Members
+
     - if project_nav_tab? :settings
       = nav_link(path: %w[projects#edit members#show integrations#show services#edit repository#show ci_cd#show pages#show]) do
         = link_to edit_project_path(@project), title: 'Settings', class: 'shortcuts-tree' do
           %span
             Settings
-    - else
-      = nav_link(path: %w[members#show]) do
-        = link_to project_settings_members_path(@project), title: 'Settings', class: 'shortcuts-tree' do
-          %span
-            Settings
 
     -# Shortcut to Project > Activity
     %li.hidden
diff --git a/app/views/projects/project_members/_team.html.haml b/app/views/projects/project_members/_team.html.haml
index 7ed467c8841..1f18490594b 100644
--- a/app/views/projects/project_members/_team.html.haml
+++ b/app/views/projects/project_members/_team.html.haml
@@ -5,7 +5,7 @@
       %strong
         #{@project.name}
     %span.badge= @project_members.total_count
-    = form_tag project_settings_members_path(@project), method: :get, class: 'form-inline member-search-form flex-project-members-form'  do
+    = form_tag project_project_members_path(@project), method: :get, class: 'form-inline member-search-form flex-project-members-form'  do
       .form-group
         = search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false }
         %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" }
diff --git a/app/views/projects/project_members/import.html.haml b/app/views/projects/project_members/import.html.haml
index 03b33eb2da7..f6ca8d5a921 100644
--- a/app/views/projects/project_members/import.html.haml
+++ b/app/views/projects/project_members/import.html.haml
@@ -12,4 +12,4 @@
 
   .form-actions
     = button_tag 'Import project members', class: "btn btn-create"
-    = link_to "Cancel", project_settings_members_path(@project), class: "btn btn-cancel"
+    = link_to "Cancel", project_project_members_path(@project), class: "btn btn-cancel"
diff --git a/app/views/projects/project_members/_index.html.haml b/app/views/projects/project_members/index.html.haml
index 08e2d6177ab..25153fd0b6f 100644
--- a/app/views/projects/project_members/_index.html.haml
+++ b/app/views/projects/project_members/index.html.haml
@@ -1,3 +1,5 @@
+- page_title "Members"
+
 .row.prepend-top-default
   .col-lg-12
     %h4
diff --git a/app/views/projects/settings/_head.html.haml b/app/views/projects/settings/_head.html.haml
index b5773acb5a4..15ba09b10ba 100644
--- a/app/views/projects/settings/_head.html.haml
+++ b/app/views/projects/settings/_head.html.haml
@@ -9,10 +9,6 @@
             = link_to edit_project_path(@project), title: 'General' do
               %span
                 General
-        = nav_link(controller: :members) do
-          = link_to project_settings_members_path(@project), title: 'Members' do
-            %span
-              Members
         - if can_edit
           = nav_link(controller: [:integrations, :services, :hooks, :hook_logs]) do
             = link_to project_settings_integrations_path(@project), title: 'Integrations' do
diff --git a/changelogs/unreleased/29893-change-menu-locations.yml b/changelogs/unreleased/29893-change-menu-locations.yml
new file mode 100644
index 00000000000..d348adc2d74
--- /dev/null
+++ b/changelogs/unreleased/29893-change-menu-locations.yml
@@ -0,0 +1,3 @@
+---
+title: Moved "Members in a project" menu entry and path locations
+merge_request: 11560
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 8caee302651..62cab25c763 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -386,7 +386,7 @@ constraints(ProjectUrlConstrainer.new) do
         end
       end
       namespace :settings do
-        resource :members, only: [:show]
+        get :members, to: redirect('/%{namespace_id}/%{project_id}/project_members')
         resource :ci_cd, only: [:show], controller: 'ci_cd'
         resource :integrations, only: [:show]
         resource :repository, only: [:show], controller: :repository
diff --git a/features/project/active_tab.feature b/features/project/active_tab.feature
index 34201cd8486..3ea0aab5a67 100644
--- a/features/project/active_tab.feature
+++ b/features/project/active_tab.feature
@@ -31,6 +31,11 @@ Feature: Project Active Tab
     Then the active main tab should be Wiki
     And no other main tabs should be active
 
+  Scenario: On Project Members
+    Given I visit my project's members page
+    Then the active main tab should be Members
+    And no other main tabs should be active
+
   # Sub Tabs: Home
 
   Scenario: On Project Home/Show
@@ -63,12 +68,6 @@ Feature: Project Active Tab
     And no other sub tabs should be active
     And the active main tab should be Settings
 
-  Scenario: On Project Members
-    Given I visit my project's members page
-    Then the active sub tab should be Members
-    And no other sub tabs should be active
-    And the active main tab should be Settings
-
   # Sub Tabs: Repository
 
   Scenario: On Project Repository/Files
diff --git a/features/steps/shared/project_tab.rb b/features/steps/shared/project_tab.rb
index 0cb9229dbae..901f7f76ee9 100644
--- a/features/steps/shared/project_tab.rb
+++ b/features/steps/shared/project_tab.rb
@@ -32,6 +32,10 @@ module SharedProjectTab
     ensure_active_main_tab('Wiki')
   end
 
+  step 'the active main tab should be Members' do
+    ensure_active_main_tab('Members')
+  end
+
   step 'the active main tab should be Settings' do
     ensure_active_main_tab('Settings')
   end
diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb
index 48a2994cbc0..019a50882ab 100644
--- a/spec/controllers/projects/group_links_controller_spec.rb
+++ b/spec/controllers/projects/group_links_controller_spec.rb
@@ -34,7 +34,7 @@ describe Projects::GroupLinksController do
 
       it 'redirects to project group links page' do
         expect(response).to redirect_to(
-          project_settings_members_path(project)
+          project_project_members_path(project)
         )
       end
     end
@@ -65,7 +65,7 @@ describe Projects::GroupLinksController do
 
       it 'redirects to project group links page' do
         expect(response).to redirect_to(
-          project_settings_members_path(project)
+          project_project_members_path(project)
         )
       end
     end
@@ -79,7 +79,7 @@ describe Projects::GroupLinksController do
 
       it 'redirects to project group links page' do
         expect(response).to redirect_to(
-          project_settings_members_path(project)
+          project_project_members_path(project)
         )
         expect(flash[:alert]).to eq('Please select a group.')
       end
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index 548ec8f487f..8671d7a78dd 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -5,11 +5,10 @@ describe Projects::ProjectMembersController do
   let(:project) { create(:empty_project, :public, :access_requestable) }
 
   describe 'GET index' do
-    it 'should have the settings/members address with a 302 status code' do
+    it 'should have the project_members address with a 200 status code' do
       get :index, namespace_id: project.namespace, project_id: project
 
-      expect(response).to have_http_status(302)
-      expect(response.location).to include project_settings_members_path(project)
+      expect(response).to have_http_status(200)
     end
   end
 
@@ -50,7 +49,7 @@ describe Projects::ProjectMembersController do
                       access_level: Gitlab::Access::GUEST
 
         expect(response).to set_flash.to 'Users were successfully added.'
-        expect(response).to redirect_to(project_settings_members_path(project))
+        expect(response).to redirect_to(project_project_members_path(project))
       end
 
       it 'adds no user to members' do
@@ -62,7 +61,7 @@ describe Projects::ProjectMembersController do
                       access_level: Gitlab::Access::GUEST
 
         expect(response).to set_flash.to 'Message'
-        expect(response).to redirect_to(project_settings_members_path(project))
+        expect(response).to redirect_to(project_project_members_path(project))
       end
     end
   end
@@ -111,7 +110,7 @@ describe Projects::ProjectMembersController do
                            id: member
 
           expect(response).to redirect_to(
-            project_settings_members_path(project)
+            project_project_members_path(project)
           )
           expect(project.members).not_to include member
         end
@@ -253,7 +252,7 @@ describe Projects::ProjectMembersController do
                                         id: member
 
           expect(response).to redirect_to(
-            project_settings_members_path(project)
+            project_project_members_path(project)
           )
           expect(project.members).to include member
         end
@@ -290,7 +289,7 @@ describe Projects::ProjectMembersController do
         expect(project.team_members).to include member
         expect(response).to set_flash.to 'Successfully imported'
         expect(response).to redirect_to(
-          project_settings_members_path(project)
+          project_project_members_path(project)
         )
       end
     end
diff --git a/spec/controllers/projects/settings/members_controller_spec.rb b/spec/controllers/projects/settings/members_controller_spec.rb
deleted file mode 100644
index 076d6cd9c6e..00000000000
--- a/spec/controllers/projects/settings/members_controller_spec.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-require('spec_helper')
-
-describe Projects::Settings::MembersController do
-  let(:project) { create(:empty_project, :public, :access_requestable) }
-
-  describe 'GET show' do
-    it 'renders show with 200 status code' do
-      get :show, namespace_id: project.namespace, project_id: project
-
-      expect(response).to have_http_status(200)
-      expect(response).to render_template(:show)
-    end
-  end
-end
diff --git a/spec/features/projects/members/anonymous_user_sees_members_spec.rb b/spec/features/projects/members/anonymous_user_sees_members_spec.rb
index 4958d5594ac..28c8d20aad5 100644
--- a/spec/features/projects/members/anonymous_user_sees_members_spec.rb
+++ b/spec/features/projects/members/anonymous_user_sees_members_spec.rb
@@ -11,10 +11,10 @@ feature 'Projects > Members > Anonymous user sees members', feature: true do
   end
 
   scenario "anonymous user visits the project's members page and sees the list of members" do
-    visit project_settings_members_path(project)
+    visit project_project_members_path(project)
 
     expect(current_path).to eq(
-      project_settings_members_path(project))
+      project_project_members_path(project))
     expect(page).to have_content(user.name)
   end
 end
diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb
index 247cc0e6f2c..75a366a3eb7 100644
--- a/spec/features/projects/members/user_requests_access_spec.rb
+++ b/spec/features/projects/members/user_requests_access_spec.rb
@@ -46,10 +46,11 @@ feature 'Projects > Members > User requests access', feature: true do
 
     expect(project.requesters.exists?(user_id: user)).to be_truthy
 
-    open_project_settings_menu
-    click_link 'Members'
+    page.within('.layout-nav .nav-links') do
+      click_link('Members')
+    end
 
-    visit project_settings_members_path(project)
+    visit project_project_members_path(project)
     page.within('.content') do
       expect(page).not_to have_content(user.name)
     end
diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb
index 7a522487a74..717ac1962d1 100644
--- a/spec/helpers/gitlab_routing_helper_spec.rb
+++ b/spec/helpers/gitlab_routing_helper_spec.rb
@@ -2,12 +2,6 @@ require 'spec_helper'
 
 describe GitlabRoutingHelper do
   describe 'Project URL helpers' do
-    describe '#project_members_url' do
-      let(:project) { build_stubbed(:empty_project) }
-
-      it { expect(project_members_url(project)).to eq project_project_members_url(project) }
-    end
-
     describe '#project_member_path' do
       let(:project_member) { create(:project_member) }