Commit 64e589c5 authored by Tim Zallmann's avatar Tim Zallmann Committed by Douwe Maan

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

parent 86fc4a1f
...@@ -322,7 +322,7 @@ import PerformanceBar from './performance_bar'; ...@@ -322,7 +322,7 @@ import PerformanceBar from './performance_bar';
new gl.Members(); new gl.Members();
new UsersSelect(); new UsersSelect();
break; break;
case 'projects:settings:members:show': case 'projects:project_members:index':
new gl.MemberExpirationDate('.js-access-expiration-date-groups'); new gl.MemberExpirationDate('.js-access-expiration-date-groups');
new GroupsSelect(); new GroupsSelect();
new gl.MemberExpirationDate(); new gl.MemberExpirationDate();
......
...@@ -70,7 +70,7 @@ module MembershipActions ...@@ -70,7 +70,7 @@ module MembershipActions
def members_page_url def members_page_url
if membershipable.is_a?(Project) if membershipable.is_a?(Project)
project_settings_members_path(membershipable) project_project_members_path(membershipable)
else else
polymorphic_url([membershipable, :members]) polymorphic_url([membershipable, :members])
end end
......
...@@ -22,7 +22,7 @@ class Projects::GroupLinksController < Projects::ApplicationController ...@@ -22,7 +22,7 @@ class Projects::GroupLinksController < Projects::ApplicationController
flash[:alert] = 'Please select a group.' flash[:alert] = 'Please select a group.'
end end
redirect_to project_settings_members_path(project) redirect_to project_project_members_path(project)
end end
def update def update
...@@ -36,7 +36,7 @@ class Projects::GroupLinksController < Projects::ApplicationController ...@@ -36,7 +36,7 @@ class Projects::GroupLinksController < Projects::ApplicationController
respond_to do |format| respond_to do |format|
format.html do format.html do
redirect_to project_settings_members_path(project), status: 302 redirect_to project_project_members_path(project), status: 302
end end
format.js { head :ok } format.js { head :ok }
end end
......
...@@ -6,8 +6,23 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -6,8 +6,23 @@ class Projects::ProjectMembersController < Projects::ApplicationController
before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access] before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access]
def index def index
sort = params[:sort].presence || sort_value_name @sort = params[:sort].presence || sort_value_name
redirect_to project_settings_members_path(@project, sort: sort) @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
def update def update
...@@ -19,7 +34,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -19,7 +34,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
end end
def resend_invite 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]) @project_member = @project.project_members.find(params[:id])
...@@ -42,7 +57,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -42,7 +57,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
return render_404 return render_404
end end
redirect_to(project_settings_members_path(project), redirect_to(project_project_members_path(project),
notice: notice) notice: notice)
end end
......
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
...@@ -97,7 +97,7 @@ module GitlabRoutingHelper ...@@ -97,7 +97,7 @@ module GitlabRoutingHelper
## Members ## Members
def project_members_url(project, *args) def project_members_url(project, *args)
project_project_members_url(project) project_project_members_url(project, *args)
end end
def project_member_path(project_member, *args) def project_member_path(project_member, *args)
......
...@@ -267,15 +267,15 @@ module ProjectsHelper ...@@ -267,15 +267,15 @@ module ProjectsHelper
def tab_ability_map def tab_ability_map
{ {
environments: :read_environment, environments: :read_environment,
milestones: :read_milestone, milestones: :read_milestone,
snippets: :read_project_snippet, snippets: :read_project_snippet,
settings: :admin_project, settings: :admin_project,
builds: :read_build, builds: :read_build,
labels: :read_label, labels: :read_label,
issues: :read_issue, issues: :read_issue,
team: :read_project_member, project_members: :read_project_member,
wiki: :read_wiki wiki: :read_wiki
} }
end end
......
...@@ -75,7 +75,7 @@ module SearchHelper ...@@ -75,7 +75,7 @@ module SearchHelper
{ category: "Current Project", label: "Merge Requests", url: project_merge_requests_path(@project) }, { 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: "Milestones", url: project_milestones_path(@project) },
{ category: "Current Project", label: "Snippets", url: project_snippets_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) } { category: "Current Project", label: "Wiki", url: project_wikis_path(@project) }
] ]
else else
......
...@@ -57,16 +57,17 @@ ...@@ -57,16 +57,17 @@
%span %span
Snippets 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 - 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 = 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 = link_to edit_project_path(@project), title: 'Settings', class: 'shortcuts-tree' do
%span %span
Settings 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 -# Shortcut to Project > Activity
%li.hidden %li.hidden
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
%strong %strong
#{@project.name} #{@project.name}
%span.badge= @project_members.total_count %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 .form-group
= search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false } = 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" } %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" }
......
...@@ -12,4 +12,4 @@ ...@@ -12,4 +12,4 @@
.form-actions .form-actions
= button_tag 'Import project members', class: "btn btn-create" = 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"
- page_title "Members"
.row.prepend-top-default .row.prepend-top-default
.col-lg-12 .col-lg-12
%h4 %h4
......
...@@ -9,10 +9,6 @@ ...@@ -9,10 +9,6 @@
= link_to edit_project_path(@project), title: 'General' do = link_to edit_project_path(@project), title: 'General' do
%span %span
General General
= nav_link(controller: :members) do
= link_to project_settings_members_path(@project), title: 'Members' do
%span
Members
- if can_edit - if can_edit
= nav_link(controller: [:integrations, :services, :hooks, :hook_logs]) do = nav_link(controller: [:integrations, :services, :hooks, :hook_logs]) do
= link_to project_settings_integrations_path(@project), title: 'Integrations' do = link_to project_settings_integrations_path(@project), title: 'Integrations' do
......
---
title: Moved "Members in a project" menu entry and path locations
merge_request: 11560
...@@ -386,7 +386,7 @@ constraints(ProjectUrlConstrainer.new) do ...@@ -386,7 +386,7 @@ constraints(ProjectUrlConstrainer.new) do
end end
end end
namespace :settings do 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 :ci_cd, only: [:show], controller: 'ci_cd'
resource :integrations, only: [:show] resource :integrations, only: [:show]
resource :repository, only: [:show], controller: :repository resource :repository, only: [:show], controller: :repository
......
...@@ -31,6 +31,11 @@ Feature: Project Active Tab ...@@ -31,6 +31,11 @@ Feature: Project Active Tab
Then the active main tab should be Wiki Then the active main tab should be Wiki
And no other main tabs should be active 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 # Sub Tabs: Home
Scenario: On Project Home/Show Scenario: On Project Home/Show
...@@ -63,12 +68,6 @@ Feature: Project Active Tab ...@@ -63,12 +68,6 @@ Feature: Project Active Tab
And no other sub tabs should be active And no other sub tabs should be active
And the active main tab should be Settings 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 # Sub Tabs: Repository
Scenario: On Project Repository/Files Scenario: On Project Repository/Files
......
...@@ -32,6 +32,10 @@ module SharedProjectTab ...@@ -32,6 +32,10 @@ module SharedProjectTab
ensure_active_main_tab('Wiki') ensure_active_main_tab('Wiki')
end 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 step 'the active main tab should be Settings' do
ensure_active_main_tab('Settings') ensure_active_main_tab('Settings')
end end
......
...@@ -34,7 +34,7 @@ describe Projects::GroupLinksController do ...@@ -34,7 +34,7 @@ describe Projects::GroupLinksController do
it 'redirects to project group links page' do it 'redirects to project group links page' do
expect(response).to redirect_to( expect(response).to redirect_to(
project_settings_members_path(project) project_project_members_path(project)
) )
end end
end end
...@@ -65,7 +65,7 @@ describe Projects::GroupLinksController do ...@@ -65,7 +65,7 @@ describe Projects::GroupLinksController do
it 'redirects to project group links page' do it 'redirects to project group links page' do
expect(response).to redirect_to( expect(response).to redirect_to(
project_settings_members_path(project) project_project_members_path(project)
) )
end end
end end
...@@ -79,7 +79,7 @@ describe Projects::GroupLinksController do ...@@ -79,7 +79,7 @@ describe Projects::GroupLinksController do
it 'redirects to project group links page' do it 'redirects to project group links page' do
expect(response).to redirect_to( expect(response).to redirect_to(
project_settings_members_path(project) project_project_members_path(project)
) )
expect(flash[:alert]).to eq('Please select a group.') expect(flash[:alert]).to eq('Please select a group.')
end end
......
...@@ -5,11 +5,10 @@ describe Projects::ProjectMembersController do ...@@ -5,11 +5,10 @@ describe Projects::ProjectMembersController do
let(:project) { create(:empty_project, :public, :access_requestable) } let(:project) { create(:empty_project, :public, :access_requestable) }
describe 'GET index' do 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 get :index, namespace_id: project.namespace, project_id: project
expect(response).to have_http_status(302) expect(response).to have_http_status(200)
expect(response.location).to include project_settings_members_path(project)
end end
end end
...@@ -50,7 +49,7 @@ describe Projects::ProjectMembersController do ...@@ -50,7 +49,7 @@ describe Projects::ProjectMembersController do
access_level: Gitlab::Access::GUEST access_level: Gitlab::Access::GUEST
expect(response).to set_flash.to 'Users were successfully added.' 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 end
it 'adds no user to members' do it 'adds no user to members' do
...@@ -62,7 +61,7 @@ describe Projects::ProjectMembersController do ...@@ -62,7 +61,7 @@ describe Projects::ProjectMembersController do
access_level: Gitlab::Access::GUEST access_level: Gitlab::Access::GUEST
expect(response).to set_flash.to 'Message' 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 end
end end
...@@ -111,7 +110,7 @@ describe Projects::ProjectMembersController do ...@@ -111,7 +110,7 @@ describe Projects::ProjectMembersController do
id: member id: member
expect(response).to redirect_to( expect(response).to redirect_to(
project_settings_members_path(project) project_project_members_path(project)
) )
expect(project.members).not_to include member expect(project.members).not_to include member
end end
...@@ -253,7 +252,7 @@ describe Projects::ProjectMembersController do ...@@ -253,7 +252,7 @@ describe Projects::ProjectMembersController do
id: member id: member
expect(response).to redirect_to( expect(response).to redirect_to(
project_settings_members_path(project) project_project_members_path(project)
) )
expect(project.members).to include member expect(project.members).to include member
end end
...@@ -290,7 +289,7 @@ describe Projects::ProjectMembersController do ...@@ -290,7 +289,7 @@ describe Projects::ProjectMembersController do
expect(project.team_members).to include member expect(project.team_members).to include member
expect(response).to set_flash.to 'Successfully imported' expect(response).to set_flash.to 'Successfully imported'
expect(response).to redirect_to( expect(response).to redirect_to(
project_settings_members_path(project) project_project_members_path(project)
) )
end end
end end
......
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
...@@ -11,10 +11,10 @@ feature 'Projects > Members > Anonymous user sees members', feature: true do ...@@ -11,10 +11,10 @@ feature 'Projects > Members > Anonymous user sees members', feature: true do
end end
scenario "anonymous user visits the project's members page and sees the list of members" do 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( expect(current_path).to eq(
project_settings_members_path(project)) project_project_members_path(project))
expect(page).to have_content(user.name) expect(page).to have_content(user.name)
end end
end end
...@@ -46,10 +46,11 @@ feature 'Projects > Members > User requests access', feature: true do ...@@ -46,10 +46,11 @@ feature 'Projects > Members > User requests access', feature: true do
expect(project.requesters.exists?(user_id: user)).to be_truthy expect(project.requesters.exists?(user_id: user)).to be_truthy
open_project_settings_menu page.within('.layout-nav .nav-links') do
click_link 'Members' click_link('Members')
end
visit project_settings_members_path(project) visit project_project_members_path(project)
page.within('.content') do page.within('.content') do
expect(page).not_to have_content(user.name) expect(page).not_to have_content(user.name)
end end
......
...@@ -2,12 +2,6 @@ require 'spec_helper' ...@@ -2,12 +2,6 @@ require 'spec_helper'
describe GitlabRoutingHelper do describe GitlabRoutingHelper do
describe 'Project URL helpers' 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 describe '#project_member_path' do
let(:project_member) { create(:project_member) } let(:project_member) { create(:project_member) }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment