Commit 29b485a0 authored by Doug Stull's avatar Doug Stull Committed by Robert May

Consolidate project and group can admin members helpers

parent f07a46cf
...@@ -13,16 +13,8 @@ class Groups::ApplicationController < ApplicationController ...@@ -13,16 +13,8 @@ class Groups::ApplicationController < ApplicationController
before_action :set_sorting before_action :set_sorting
requires_cross_project_access requires_cross_project_access
helper_method :can_manage_members?
private private
def can_manage_members?(group = @group)
strong_memoize(:can_manage_members) do
can?(current_user, :admin_group_member, group)
end
end
def group def group
@group ||= find_routable!(Group, params[:group_id] || params[:id], request.path_info) @group ||= find_routable!(Group, params[:group_id] || params[:id], request.path_info)
end end
......
...@@ -29,7 +29,7 @@ class Groups::GroupMembersController < Groups::ApplicationController ...@@ -29,7 +29,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
.new(@group, current_user, params: filter_params) .new(@group, current_user, params: filter_params)
.execute(include_relations: requested_relations) .execute(include_relations: requested_relations)
if can_manage_members? if can?(current_user, :admin_group_member, @group)
@skip_groups = @group.related_group_ids @skip_groups = @group.related_group_ids
@invited_members = @members.invite @invited_members = @members.invite
......
...@@ -57,7 +57,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -57,7 +57,7 @@ class Projects::IssuesController < Projects::ApplicationController
push_frontend_feature_flag(:labels_widget, @project, default_enabled: :yaml) push_frontend_feature_flag(:labels_widget, @project, default_enabled: :yaml)
experiment(:invite_members_in_comment, namespace: @project.root_ancestor) do |experiment_instance| experiment(:invite_members_in_comment, namespace: @project.root_ancestor) do |experiment_instance|
experiment_instance.exclude! unless helpers.can_import_members? experiment_instance.exclude! unless helpers.can_admin_project_member?(@project)
experiment_instance.use {} experiment_instance.use {}
experiment_instance.try(:invite_member_link) {} experiment_instance.try(:invite_member_link) {}
......
...@@ -47,7 +47,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -47,7 +47,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:diff_searching_usage_data, @project, default_enabled: :yaml) push_frontend_feature_flag(:diff_searching_usage_data, @project, default_enabled: :yaml)
experiment(:invite_members_in_comment, namespace: @project.root_ancestor) do |experiment_instance| experiment(:invite_members_in_comment, namespace: @project.root_ancestor) do |experiment_instance|
experiment_instance.exclude! unless helpers.can_import_members? experiment_instance.exclude! unless helpers.can_admin_project_member?(@project)
experiment_instance.use {} experiment_instance.use {}
experiment_instance.try(:invite_member_link) {} experiment_instance.try(:invite_member_link) {}
......
...@@ -23,7 +23,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -23,7 +23,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
.new(@project, current_user, params: filter_params) .new(@project, current_user, params: filter_params)
.execute(include_relations: requested_relations) .execute(include_relations: requested_relations)
if helpers.can_manage_project_members?(@project) if can?(current_user, :admin_project_member, @project)
@invited_members = present_members(project_members.invite) @invited_members = present_members(project_members.invite)
@requesters = present_members(AccessRequestsFinder.new(@project).execute(current_user)) @requesters = present_members(AccessRequestsFinder.new(@project).execute(current_user))
end end
......
...@@ -29,6 +29,10 @@ module GroupsHelper ...@@ -29,6 +29,10 @@ module GroupsHelper
can?(current_user, :set_emails_disabled, group) && !group.parent&.emails_disabled? can?(current_user, :set_emails_disabled, group) && !group.parent&.emails_disabled?
end end
def can_admin_group_member?(group)
Ability.allowed?(current_user, :admin_group_member, group)
end
def group_issues_count(state:) def group_issues_count(state:)
IssuesFinder IssuesFinder
.new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true) .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true)
......
...@@ -4,17 +4,17 @@ module InviteMembersHelper ...@@ -4,17 +4,17 @@ module InviteMembersHelper
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
def can_invite_members_for_project?(project) def can_invite_members_for_project?(project)
Feature.enabled?(:invite_members_group_modal, project.group) && can_manage_project_members?(project) # do not use the can_admin_project_member? helper here due to structure of the view and how membership_locked?
# is leveraged for inviting groups
Feature.enabled?(:invite_members_group_modal, project.group) && can?(current_user, :admin_project_member, project)
end end
def can_invite_group_for_project?(project) def can_invite_group_for_project?(project)
Feature.enabled?(:invite_members_group_modal, project.group) && can_manage_project_members?(project) && project.allowed_to_share_with_group? # do not use the can_admin_project_member? helper here due to structure of the view and how membership_locked?
end # is leveraged for inviting groups
Feature.enabled?(:invite_members_group_modal, project.group) &&
def directly_invite_members? can?(current_user, :admin_project_member, project) &&
strong_memoize(:directly_invite_members) do project.allowed_to_share_with_group?
can_import_members?
end
end end
def invite_accepted_notice(member) def invite_accepted_notice(member)
......
...@@ -101,7 +101,7 @@ module Nav ...@@ -101,7 +101,7 @@ module Nav
) )
end end
if Gitlab::Experimentation.active?(:invite_members_new_dropdown) && can_import_members? if Gitlab::Experimentation.active?(:invite_members_new_dropdown) && can_admin_project_member?(project)
menu_items.push( menu_items.push(
invite_members_menu_item( invite_members_menu_item(
href: project_project_members_path(project) href: project_project_members_path(project)
......
# frozen_string_literal: true # frozen_string_literal: true
module Projects::ProjectMembersHelper module Projects::ProjectMembersHelper
def can_manage_project_members?(project)
can?(current_user, :admin_project_member, project)
end
def show_groups?(group_links)
group_links.exists? || groups_tab_active?
end
def show_invited_members?(project, invited_members)
can_manage_project_members?(project) && invited_members.exists?
end
def show_access_requests?(project, requesters)
can_manage_project_members?(project) && requesters.exists?
end
def groups_tab_active?
params[:search_groups].present?
end
def current_user_is_group_owner?(project)
return false if project.group.nil?
project.group.has_owner?(current_user)
end
def project_members_app_data_json(project, members:, group_links:, invited:, access_requests:) def project_members_app_data_json(project, members:, group_links:, invited:, access_requests:)
{ {
user: project_members_list_data(project, members, { param_name: :page, params: { search_groups: nil } }), user: project_members_list_data(project, members, { param_name: :page, params: { search_groups: nil } }),
...@@ -34,7 +8,7 @@ module Projects::ProjectMembersHelper ...@@ -34,7 +8,7 @@ module Projects::ProjectMembersHelper
invite: project_members_list_data(project, invited.nil? ? [] : invited), invite: project_members_list_data(project, invited.nil? ? [] : invited),
access_request: project_members_list_data(project, access_requests.nil? ? [] : access_requests), access_request: project_members_list_data(project, access_requests.nil? ? [] : access_requests),
source_id: project.id, source_id: project.id,
can_manage_members: can_manage_project_members?(project) can_manage_members: Ability.allowed?(current_user, :admin_project_member, project)
}.to_json }.to_json
end end
......
...@@ -291,8 +291,8 @@ module ProjectsHelper ...@@ -291,8 +291,8 @@ module ProjectsHelper
) % { default_label: default_label } ) % { default_label: default_label }
end end
def can_import_members? def can_admin_project_member?(project)
Ability.allowed?(current_user, :admin_project_member, @project) Ability.allowed?(current_user, :admin_project_member, project) && !membership_locked?
end end
def project_can_be_shared? def project_can_be_shared?
......
...@@ -3,14 +3,13 @@ ...@@ -3,14 +3,13 @@
module GroupLink module GroupLink
class ProjectGroupLinkEntity < GroupLink::GroupLinkEntity class ProjectGroupLinkEntity < GroupLink::GroupLinkEntity
include RequestAwareEntity include RequestAwareEntity
include Projects::ProjectMembersHelper
expose :can_update do |group_link| expose :can_update do |group_link|
can_manage_project_members?(group_link.project) can?(current_user, :admin_project_member, group_link.project)
end end
expose :can_remove do |group_link| expose :can_remove do |group_link|
can_manage_project_members?(group_link.project) can?(current_user, :admin_project_member, group_link.project)
end end
private private
......
- return unless can_manage_members?(group) - return unless can_admin_group_member?(group)
.js-invite-members-modal{ data: { is_project: 'false', .js-invite-members-modal{ data: { is_project: 'false',
access_levels: GroupMember.access_level_roles.to_json, access_levels: GroupMember.access_level_roles.to_json,
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
.row.gl-mt-3 .row.gl-mt-3
.col-lg-12 .col-lg-12
.gl-display-flex.gl-flex-wrap .gl-display-flex.gl-flex-wrap
- if can_manage_members? - if can_admin_group_member?(@group)
.gl-w-half.gl-xs-w-full .gl-w-half.gl-xs-w-full
%h4 %h4
= _('Group members') = _('Group members')
...@@ -20,7 +20,7 @@ ...@@ -20,7 +20,7 @@
trigger_source: 'group-members-page', trigger_source: 'group-members-page',
display_text: _('Invite members') } } display_text: _('Invite members') } }
= render 'groups/invite_members_modal', group: @group = render 'groups/invite_members_modal', group: @group
- if can_manage_members? && Feature.disabled?(:invite_members_group_modal, @group) - if can_admin_group_member?(@group) && Feature.disabled?(:invite_members_group_modal, @group)
%hr.gl-mt-4 %hr.gl-mt-4
%ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' } %ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' }
%li.nav-tab{ role: 'presentation' } %li.nav-tab{ role: 'presentation' }
......
- return unless can_import_members? - return unless can_admin_project_member?(project)
.js-invite-members-modal{ data: { is_project: 'true', .js-invite-members-modal{ data: { is_project: 'true',
access_levels: ProjectMember.access_level_roles.to_json, access_levels: ProjectMember.access_level_roles.to_json,
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
= render "home_panel" = render "home_panel"
= render "archived_notice", project: @project = render "archived_notice", project: @project
= render 'invite_members_empty_project' if can_import_members? = render "invite_members_empty_project" if can_admin_project_member?(@project)
%h4.gl-mt-0.gl-mb-3 %h4.gl-mt-0.gl-mb-3
= _('The repository for this project is empty') = _('The repository for this project is empty')
......
...@@ -10,21 +10,21 @@ ...@@ -10,21 +10,21 @@
%h4 %h4
= _("Project members") = _("Project members")
.gl-justify-content-bottom.gl-display-flex.align-items-center .gl-justify-content-bottom.gl-display-flex.align-items-center
- if can_manage_project_members?(@project) - if can?(current_user, :admin_project_member, @project)
%p= share_project_description(@project) %p= share_project_description(@project)
- else - else
%p %p
= html_escape(_("Members can be added by project %{i_open}Maintainers%{i_close} or %{i_open}Owners%{i_close}")) % { i_open: '<i>'.html_safe, i_close: '</i>'.html_safe } = html_escape(_("Members can be added by project %{i_open}Maintainers%{i_close} or %{i_open}Owners%{i_close}")) % { i_open: '<i>'.html_safe, i_close: '</i>'.html_safe }
.col-md-12.col-lg-6 .col-md-12.col-lg-6
.gl-display-flex.gl-flex-wrap.gl-justify-content-end .gl-display-flex.gl-flex-wrap.gl-justify-content-end
- if can_import_members? - if can_admin_project_member?(@project)
= link_to _("Import a project"), = link_to _("Import a project"),
import_project_project_members_path(@project), import_project_project_members_path(@project),
class: "btn btn-default btn-md gl-button gl-mt-3 gl-sm-w-auto gl-w-full", class: "btn btn-default btn-md gl-button gl-mt-3 gl-sm-w-auto gl-w-full",
title: _("Import members from another project") title: _("Import members from another project")
- if @project.allowed_to_share_with_group? - if @project.allowed_to_share_with_group?
.js-invite-group-trigger{ data: { classes: 'gl-mt-3 gl-sm-w-auto gl-w-full gl-sm-ml-3', display_text: _('Invite a group') } } .js-invite-group-trigger{ data: { classes: 'gl-mt-3 gl-sm-w-auto gl-w-full gl-sm-ml-3', display_text: _('Invite a group') } }
- if can_manage_project_members?(@project) && !membership_locked? - if can_admin_project_member?(@project)
.js-invite-members-trigger{ data: { variant: 'success', .js-invite-members-trigger{ data: { variant: 'success',
classes: 'gl-mt-3 gl-sm-w-auto gl-w-full gl-sm-ml-3', classes: 'gl-mt-3 gl-sm-w-auto gl-w-full gl-sm-ml-3',
trigger_source: 'project-members-page', trigger_source: 'project-members-page',
...@@ -35,13 +35,13 @@ ...@@ -35,13 +35,13 @@
- if project_can_be_shared? - if project_can_be_shared?
%h4 %h4
= _("Project members") = _("Project members")
- if can_manage_project_members?(@project) - if can?(current_user, :admin_project_member, @project)
%p= share_project_description(@project) %p= share_project_description(@project)
- else - else
%p %p
= html_escape(_("Members can be added by project %{i_open}Maintainers%{i_close} or %{i_open}Owners%{i_close}")) % { i_open: '<i>'.html_safe, i_close: '</i>'.html_safe } = html_escape(_("Members can be added by project %{i_open}Maintainers%{i_close} or %{i_open}Owners%{i_close}")) % { i_open: '<i>'.html_safe, i_close: '</i>'.html_safe }
- if Feature.disabled?(:invite_members_group_modal, @project.group) && can_manage_project_members?(@project) && project_can_be_shared? - if Feature.disabled?(:invite_members_group_modal, @project.group) && can?(current_user, :admin_project_member, @project) && project_can_be_shared?
- if !membership_locked? && @project.allowed_to_share_with_group? - if !membership_locked? && @project.allowed_to_share_with_group?
%ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' } %ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' }
%li.nav-tab{ role: 'presentation' } %li.nav-tab{ role: 'presentation' }
...@@ -51,13 +51,37 @@ ...@@ -51,13 +51,37 @@
.tab-content.gitlab-tab-content .tab-content.gitlab-tab-content
.tab-pane.active{ id: 'invite-member-pane', role: 'tabpanel' } .tab-pane.active{ id: 'invite-member-pane', role: 'tabpanel' }
= render 'shared/members/invite_member', submit_url: project_project_members_path(@project), access_levels: ProjectMember.access_level_roles, default_access_level: @project_member.access_level, can_import_members?: can_import_members?, import_path: import_project_project_members_path(@project) = render 'shared/members/invite_member',
submit_url: project_project_members_path(@project),
access_levels: ProjectMember.access_level_roles,
default_access_level: @project_member.access_level,
can_import_members?: can_admin_project_member?(@project),
import_path: import_project_project_members_path(@project)
.tab-pane{ id: 'invite-group-pane', role: 'tabpanel', class: ('active' if membership_locked?) } .tab-pane{ id: 'invite-group-pane', role: 'tabpanel', class: ('active' if membership_locked?) }
= render 'shared/members/invite_group', submit_url: project_group_links_path(@project), access_levels: ProjectGroupLink.access_options, default_access_level: ProjectGroupLink.default_access, group_link_field: 'link_group_id', group_access_field: 'link_group_access', groups_select_tag_data: { skip_groups: @skip_groups } = render 'shared/members/invite_group',
submit_url: project_group_links_path(@project),
access_levels: ProjectGroupLink.access_options,
default_access_level: ProjectGroupLink.default_access,
group_link_field: 'link_group_id',
group_access_field: 'link_group_access',
groups_select_tag_data: { skip_groups: @skip_groups }
- elsif !membership_locked? - elsif !membership_locked?
.invite-member= render 'shared/members/invite_member', submit_url: project_project_members_path(@project), access_levels: ProjectMember.access_level_roles, default_access_level: @project_member.access_level, can_import_members?: can_import_members?, import_path: import_project_project_members_path(@project) .invite-member
= render 'shared/members/invite_member',
submit_url: project_project_members_path(@project),
access_levels: ProjectMember.access_level_roles,
default_access_level: @project_member.access_level,
can_import_members?: can_admin_project_member?(@project),
import_path: import_project_project_members_path(@project)
- elsif @project.allowed_to_share_with_group? - elsif @project.allowed_to_share_with_group?
.invite-group= render 'shared/members/invite_group', access_levels: ProjectGroupLink.access_options, default_access_level: ProjectGroupLink.default_access, submit_url: project_group_links_path(@project), group_link_field: 'link_group_id', group_access_field: 'link_group_access', groups_select_tag_data: { skip_groups: @skip_groups } .invite-group
= render 'shared/members/invite_group',
access_levels: ProjectGroupLink.access_options,
default_access_level: ProjectGroupLink.default_access,
submit_url: project_group_links_path(@project),
group_link_field: 'link_group_id',
group_access_field: 'link_group_access',
groups_select_tag_data: { skip_groups: @skip_groups }
.js-project-members-list-app{ data: { members_data: project_members_app_data_json(@project, .js-project-members-list-app{ data: { members_data: project_members_app_data_json(@project,
members: @project_members, members: @project_members,
group_links: @group_links, group_links: @group_links,
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
#js-vue-sidebar-assignees{ data: { field: issuable_type, #js-vue-sidebar-assignees{ data: { field: issuable_type,
signed_in: signed_in, signed_in: signed_in,
max_assignees: dropdown_options[:data][:"max-select"], max_assignees: dropdown_options[:data][:"max-select"],
directly_invite_members: directly_invite_members? } } directly_invite_members: can_admin_project_member?(@project) } }
.title.hide-collapsed .title.hide-collapsed
= _('Assignee') = _('Assignee')
= loading_icon(css_class: 'gl-vertical-align-text-bottom') = loading_icon(css_class: 'gl-vertical-align-text-bottom')
......
- options = local_assigns.fetch(:options) - options = local_assigns.fetch(:options)
- data = options[:data] - data = options[:data]
- if directly_invite_members? - if can_admin_project_member?(@project)
- options[:dropdown_class] += ' dropdown-extended-height' - options[:dropdown_class] += ' dropdown-extended-height'
- options[:footer_content] = true - options[:footer_content] = true
- options[:wrapper_class] = local_assigns.fetch(:wrapper_class) - options[:wrapper_class] = local_assigns.fetch(:wrapper_class)
......
...@@ -241,11 +241,6 @@ module EE ...@@ -241,11 +241,6 @@ module EE
can?(current_user, :admin_namespace, project.root_ancestor) can?(current_user, :admin_namespace, project.root_ancestor)
end end
override :can_import_members?
def can_import_members?
super && !membership_locked?
end
def show_compliance_framework_badge?(project) def show_compliance_framework_badge?(project)
project&.compliance_framework_setting&.compliance_management_framework.present? project&.compliance_framework_setting&.compliance_management_framework.present?
end end
......
...@@ -88,7 +88,7 @@ RSpec.describe 'Project > Members > Invite group and members' do ...@@ -88,7 +88,7 @@ RSpec.describe 'Project > Members > Invite group and members' do
context 'when the group has "Share with group lock" enabled' do context 'when the group has "Share with group lock" enabled' do
before do before do
project.namespace.update_column(:share_with_group_lock, true) project.namespace.update!(share_with_group_lock: true)
end end
it_behaves_like 'the project cannot be shared with groups' it_behaves_like 'the project cannot be shared with groups'
...@@ -96,7 +96,7 @@ RSpec.describe 'Project > Members > Invite group and members' do ...@@ -96,7 +96,7 @@ RSpec.describe 'Project > Members > Invite group and members' do
context 'when the group has membership lock enabled' do context 'when the group has membership lock enabled' do
before do before do
project.namespace.update_column(:membership_lock, true) project.namespace.update!(membership_lock: true)
end end
it_behaves_like 'the project cannot be shared with members' it_behaves_like 'the project cannot be shared with members'
...@@ -128,7 +128,7 @@ RSpec.describe 'Project > Members > Invite group and members' do ...@@ -128,7 +128,7 @@ RSpec.describe 'Project > Members > Invite group and members' do
context 'when the subgroup has "Share with group lock" enabled' do context 'when the subgroup has "Share with group lock" enabled' do
before do before do
subgroup.update_column(:share_with_group_lock, true) subgroup.update!(share_with_group_lock: true)
end end
it_behaves_like 'the project cannot be shared with groups' it_behaves_like 'the project cannot be shared with groups'
...@@ -136,7 +136,7 @@ RSpec.describe 'Project > Members > Invite group and members' do ...@@ -136,7 +136,7 @@ RSpec.describe 'Project > Members > Invite group and members' do
context 'when the subgroup has membership lock enabled' do context 'when the subgroup has membership lock enabled' do
before do before do
subgroup.update_column(:membership_lock, true) subgroup.update!(membership_lock: true)
end end
it_behaves_like 'the project cannot be shared with members' it_behaves_like 'the project cannot be shared with members'
...@@ -173,7 +173,7 @@ RSpec.describe 'Project > Members > Invite group and members' do ...@@ -173,7 +173,7 @@ RSpec.describe 'Project > Members > Invite group and members' do
# See: https://gitlab.com/gitlab-org/gitlab-foss/issues/42093 # See: https://gitlab.com/gitlab-org/gitlab-foss/issues/42093
context 'when the subgroup has membership lock enabled (parent overridden)' do context 'when the subgroup has membership lock enabled (parent overridden)' do
before do before do
subgroup.update_column(:membership_lock, true) subgroup.update!(membership_lock: true)
end end
it_behaves_like 'the project cannot be shared with groups and members' it_behaves_like 'the project cannot be shared with groups and members'
...@@ -181,7 +181,7 @@ RSpec.describe 'Project > Members > Invite group and members' do ...@@ -181,7 +181,7 @@ RSpec.describe 'Project > Members > Invite group and members' do
context 'when the subgroup has "Share with group lock" enabled (parent overridden)' do context 'when the subgroup has "Share with group lock" enabled (parent overridden)' do
before do before do
subgroup.update_column(:share_with_group_lock, true) subgroup.update!(share_with_group_lock: true)
end end
it_behaves_like 'the project cannot be shared with groups' it_behaves_like 'the project cannot be shared with groups'
......
...@@ -18,6 +18,7 @@ RSpec.describe Projects::ProjectMembersHelper do ...@@ -18,6 +18,7 @@ RSpec.describe Projects::ProjectMembersHelper do
before do before do
project.add_developer(user) project.add_developer(user)
create_schedule_with_user(project, user) create_schedule_with_user(project, user)
allow(helper).to receive(:can_admin_project_member?).and_return(true)
end end
it 'does not execute N+1' do it 'does not execute N+1' do
......
...@@ -38,21 +38,34 @@ RSpec.describe ProjectsHelper do ...@@ -38,21 +38,34 @@ RSpec.describe ProjectsHelper do
end end
end end
describe '#can_import_members?' do describe '#can_admin_project_member?' do
let(:owner) { project.owner } let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, group: group) }
before do before do
allow(helper).to receive(:current_user) { owner } allow(helper).to receive(:current_user) { user }
project.add_maintainer(user)
end end
it 'returns false if membership is locked' do context 'when membership is not locked' do
allow(helper).to receive(:membership_locked?) { true } before do
expect(helper.can_import_members?).to eq false group.membership_lock = false
end
it 'returns true when membership is not locked' do
expect(helper.can_admin_project_member?(project)).to be(true)
end
end end
it 'returns true if membership is not locked' do context 'when membership is locked' do
allow(helper).to receive(:membership_locked?) { false } before do
expect(helper.can_import_members?).to eq true group.membership_lock = true
end
it 'returns false when membership is locked' do
expect(helper.can_admin_project_member?(project)).to be(false)
end
end end
end end
......
...@@ -93,13 +93,12 @@ RSpec.describe Projects::ProjectMembersController do ...@@ -93,13 +93,12 @@ RSpec.describe Projects::ProjectMembersController do
let_it_be(:invited_member) { create(:project_member, :invited, project: project) } let_it_be(:invited_member) { create(:project_member, :invited, project: project) }
before do before do
project.add_maintainer(user)
sign_in(user) sign_in(user)
end end
context 'when user has `admin_project_member` permissions' do context 'when user has `admin_project_member` permissions' do
before do before do
allow(controller.helpers).to receive(:can_manage_project_members?).with(project).and_return(true) project.add_maintainer(user)
end end
it 'lists invited members' do it 'lists invited members' do
...@@ -110,10 +109,6 @@ RSpec.describe Projects::ProjectMembersController do ...@@ -110,10 +109,6 @@ RSpec.describe Projects::ProjectMembersController do
end end
context 'when user does not have `admin_project_member` permissions' do context 'when user does not have `admin_project_member` permissions' do
before do
allow(controller.helpers).to receive(:can_manage_project_members?).with(project).and_return(false)
end
it 'does not list invited members' do it 'does not list invited members' do
get :index, params: { namespace_id: project.namespace, project_id: project } get :index, params: { namespace_id: project.namespace, project_id: project }
...@@ -127,13 +122,12 @@ RSpec.describe Projects::ProjectMembersController do ...@@ -127,13 +122,12 @@ RSpec.describe Projects::ProjectMembersController do
before do before do
project.request_access(access_requester_user) project.request_access(access_requester_user)
project.add_maintainer(user)
sign_in(user) sign_in(user)
end end
context 'when user has `admin_project_member` permissions' do context 'when user has `admin_project_member` permissions' do
before do before do
allow(controller.helpers).to receive(:can_manage_project_members?).with(project).and_return(true) project.add_maintainer(user)
end end
it 'lists access requests' do it 'lists access requests' do
...@@ -144,10 +138,6 @@ RSpec.describe Projects::ProjectMembersController do ...@@ -144,10 +138,6 @@ RSpec.describe Projects::ProjectMembersController do
end end
context 'when user does not have `admin_project_member` permissions' do context 'when user does not have `admin_project_member` permissions' do
before do
allow(controller.helpers).to receive(:can_manage_project_members?).with(project).and_return(false)
end
it 'does not list access requests' do it 'does not list access requests' do
get :index, params: { namespace_id: project.namespace, project_id: project } get :index, params: { namespace_id: project.namespace, project_id: project }
......
...@@ -522,4 +522,23 @@ RSpec.describe GroupsHelper do ...@@ -522,4 +522,23 @@ RSpec.describe GroupsHelper do
expect(helper.render_setting_to_allow_project_access_token_creation?(group)).to be_falsy expect(helper.render_setting_to_allow_project_access_token_creation?(group)).to be_falsy
end end
end end
describe '#can_admin_group_member?' do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
before do
allow(helper).to receive(:current_user) { user }
end
it 'returns true when current_user can admin members' do
group.add_owner(user)
expect(helper.can_admin_group_member?(group)).to be(true)
end
it 'returns false when current_user can not admin members' do
expect(helper.can_admin_group_member?(group)).to be(false)
end
end
end end
...@@ -71,14 +71,14 @@ RSpec.describe InviteMembersHelper do ...@@ -71,14 +71,14 @@ RSpec.describe InviteMembersHelper do
end end
describe "#can_invite_members_for_project?" do describe "#can_invite_members_for_project?" do
context 'when the user can_manage_project_members' do context 'when the user can_admin_project_member' do
before do before do
allow(helper).to receive(:can_manage_project_members?).and_return(true) allow(helper).to receive(:can?).with(owner, :admin_project_member, project).and_return(true)
end end
it 'returns true' do it 'returns true', :aggregate_failures do
expect(helper.can_invite_members_for_project?(project)).to eq true expect(helper.can_invite_members_for_project?(project)).to eq true
expect(helper).to have_received(:can_manage_project_members?) expect(helper).to have_received(:can?).with(owner, :admin_project_member, project)
end end
context 'when feature flag is disabled' do context 'when feature flag is disabled' do
...@@ -86,16 +86,16 @@ RSpec.describe InviteMembersHelper do ...@@ -86,16 +86,16 @@ RSpec.describe InviteMembersHelper do
stub_feature_flags(invite_members_group_modal: false) stub_feature_flags(invite_members_group_modal: false)
end end
it 'returns false' do it 'returns false', :aggregate_failures do
expect(helper.can_invite_members_for_project?(project)).to eq false expect(helper.can_invite_members_for_project?(project)).to eq false
expect(helper).not_to have_received(:can_manage_project_members?) expect(helper).not_to have_received(:can?).with(owner, :admin_project_member, project)
end end
end end
end end
context 'when the user can not manage project members' do context 'when the user can not manage project members' do
before do before do
expect(helper).to receive(:can_manage_project_members?).and_return(false) expect(helper).to receive(:can?).with(owner, :admin_project_member, project).and_return(false)
end end
it 'returns false' do it 'returns false' do
...@@ -103,27 +103,5 @@ RSpec.describe InviteMembersHelper do ...@@ -103,27 +103,5 @@ RSpec.describe InviteMembersHelper do
end end
end end
end end
describe "#directly_invite_members?" do
context 'when the user is an owner' do
before do
allow(helper).to receive(:current_user) { owner }
end
it 'returns true' do
expect(helper.directly_invite_members?).to eq true
end
end
context 'when the user is a developer' do
before do
allow(helper).to receive(:current_user) { developer }
end
it 'returns false' do
expect(helper.directly_invite_members?).to eq false
end
end
end
end end
end end
...@@ -221,13 +221,13 @@ RSpec.describe Nav::NewDropdownHelper do ...@@ -221,13 +221,13 @@ RSpec.describe Nav::NewDropdownHelper do
let(:with_show_new_issue_link) { false } let(:with_show_new_issue_link) { false }
let(:with_merge_project) { nil } let(:with_merge_project) { nil }
let(:with_can_create_snippet_in_project) { false } let(:with_can_create_snippet_in_project) { false }
let(:with_can_import_members) { false } let(:with_can_admin_project_member) { false }
before do before do
allow(helper).to receive(:show_new_issue_link?).with(project) { with_show_new_issue_link } allow(helper).to receive(:show_new_issue_link?).with(project) { with_show_new_issue_link }
allow(helper).to receive(:merge_request_source_project_for_project).with(project) { with_merge_project } allow(helper).to receive(:merge_request_source_project_for_project).with(project) { with_merge_project }
allow(helper).to receive(:can?).with(user, :create_snippet, project) { with_can_create_snippet_in_project } allow(helper).to receive(:can?).with(user, :create_snippet, project) { with_can_create_snippet_in_project }
allow(helper).to receive(:can_import_members?) { with_can_import_members } allow(helper).to receive(:can_admin_project_member?) { with_can_admin_project_member }
end end
it 'has no menu sections' do it 'has no menu sections' do
...@@ -290,7 +290,7 @@ RSpec.describe Nav::NewDropdownHelper do ...@@ -290,7 +290,7 @@ RSpec.describe Nav::NewDropdownHelper do
context 'when invite members experiment' do context 'when invite members experiment' do
let(:with_invite_members_experiment) { true } let(:with_invite_members_experiment) { true }
let(:with_can_import_members) { true } let(:with_can_admin_project_member) { true }
let(:expected_title) { 'This project' } let(:expected_title) { 'This project' }
let(:expected_href) { "/#{project.path_with_namespace}/-/project_members" } let(:expected_href) { "/#{project.path_with_namespace}/-/project_members" }
......
...@@ -8,142 +8,8 @@ RSpec.describe Projects::ProjectMembersHelper do ...@@ -8,142 +8,8 @@ RSpec.describe Projects::ProjectMembersHelper do
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:allow_admin_project) { nil }
before do before do
allow(helper).to receive(:current_user).and_return(current_user) allow(helper).to receive(:current_user).and_return(current_user)
allow(helper).to receive(:can?).with(current_user, :admin_project_member, project).and_return(allow_admin_project)
end
shared_examples 'when `current_user` does not have `admin_project_member` permissions' do
let(:allow_admin_project) { false }
it { is_expected.to be(false) }
end
describe '#can_manage_project_members?' do
subject { helper.can_manage_project_members?(project) }
context 'when `current_user` has `admin_project_member` permissions' do
let(:allow_admin_project) { true }
it { is_expected.to be(true) }
end
include_examples 'when `current_user` does not have `admin_project_member` permissions'
end
describe '#show_groups?' do
subject { helper.show_groups?(project.project_group_links) }
context 'when group links exist' do
let!(:project_group_link) { create(:project_group_link, project: project) }
it { is_expected.to be(true) }
end
context 'when `search_groups` param is set' do
before do
allow(helper).to receive(:params).and_return({ search_groups: 'foo' })
end
it { is_expected.to be(true) }
end
context 'when `search_groups` param is not set and group links do not exist' do
it { is_expected.to be(false) }
end
end
describe '#show_invited_members?' do
subject { helper.show_invited_members?(project, project.project_members.invite) }
context 'when `current_user` has `admin_project_member` permissions' do
let(:allow_admin_project) { true }
context 'when invited members exist' do
let!(:invite) { create(:project_member, :invited, project: project) }
it { is_expected.to be(true) }
end
context 'when invited members do not exist' do
it { is_expected.to be(false) }
end
end
include_examples 'when `current_user` does not have `admin_project_member` permissions'
end
describe '#show_access_requests?' do
subject { helper.show_access_requests?(project, project.requesters) }
context 'when `current_user` has `admin_project_member` permissions' do
let(:allow_admin_project) { true }
context 'when access requests exist' do
let!(:access_request) { create(:project_member, :access_request, project: project) }
it { is_expected.to be(true) }
end
context 'when access requests do not exist' do
it { is_expected.to be(false) }
end
end
include_examples 'when `current_user` does not have `admin_project_member` permissions'
end
describe '#groups_tab_active?' do
subject { helper.groups_tab_active? }
context 'when `search_groups` param is set' do
before do
allow(helper).to receive(:params).and_return({ search_groups: 'foo' })
end
it { is_expected.to be(true) }
end
context 'when `search_groups` param is not set' do
it { is_expected.to be(false) }
end
end
describe '#current_user_is_group_owner?' do
let(:group) { create(:group) }
subject { helper.current_user_is_group_owner?(project2) }
describe "when current user is the owner of the project's parent group" do
let(:project2) { create(:project, namespace: group) }
before do
group.add_owner(current_user)
end
it { is_expected.to be(true) }
end
describe "when current user is not the owner of the project's parent group" do
let_it_be(:user) { create(:user) }
let(:project2) { create(:project, namespace: group) }
before do
group.add_owner(user)
end
it { is_expected.to be(false) }
end
describe "when project does not have a parent group" do
let(:user) { create(:user) }
let(:project2) { create(:project, namespace: user.namespace) }
it { is_expected.to be(false) }
end
end end
describe 'project members' do describe 'project members' do
...@@ -155,8 +21,6 @@ RSpec.describe Projects::ProjectMembersHelper do ...@@ -155,8 +21,6 @@ RSpec.describe Projects::ProjectMembersHelper do
let(:members_collection) { members } let(:members_collection) { members }
describe '#project_members_app_data_json' do describe '#project_members_app_data_json' do
let(:allow_admin_project) { true }
subject do subject do
Gitlab::Json.parse( Gitlab::Json.parse(
helper.project_members_app_data_json( helper.project_members_app_data_json(
...@@ -171,6 +35,7 @@ RSpec.describe Projects::ProjectMembersHelper do ...@@ -171,6 +35,7 @@ RSpec.describe Projects::ProjectMembersHelper do
before do before do
allow(helper).to receive(:project_project_member_path).with(project, ':id').and_return('/foo-bar/-/project_members/:id') allow(helper).to receive(:project_project_member_path).with(project, ':id').and_return('/foo-bar/-/project_members/:id')
project.add_maintainer(current_user)
end end
it 'returns expected json' do it 'returns expected json' do
......
...@@ -720,21 +720,21 @@ RSpec.describe ProjectsHelper do ...@@ -720,21 +720,21 @@ RSpec.describe ProjectsHelper do
end end
end end
describe '#can_import_members?' do describe '#can_admin_project_member?' do
context 'when user is project owner' do context 'when user is project owner' do
before do before do
allow(helper).to receive(:current_user) { project.owner } allow(helper).to receive(:current_user) { project.owner }
end end
it 'returns true for owner of project' do it 'returns true for owner of project' do
expect(helper.can_import_members?).to eq true expect(helper.can_admin_project_member?(project)).to eq true
end end
end end
context 'when user is not a project owner' do context 'when user is not a project owner' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:user_project_role, :can_import) do where(:user_project_role, :can_admin) do
:maintainer | true :maintainer | true
:developer | false :developer | false
:reporter | false :reporter | false
...@@ -748,7 +748,7 @@ RSpec.describe ProjectsHelper do ...@@ -748,7 +748,7 @@ RSpec.describe ProjectsHelper do
end end
it 'resolves if the user can import members' do it 'resolves if the user can import members' do
expect(helper.can_import_members?).to eq can_import expect(helper.can_admin_project_member?(project)).to eq can_admin
end end
end end
end end
......
...@@ -11,6 +11,7 @@ RSpec.shared_context 'merge request show action' do ...@@ -11,6 +11,7 @@ RSpec.shared_context 'merge request show action' do
before do before do
allow(view).to receive(:experiment_enabled?).and_return(false) allow(view).to receive(:experiment_enabled?).and_return(false)
allow(view).to receive(:current_user).and_return(user) allow(view).to receive(:current_user).and_return(user)
allow(view).to receive(:can_admin_project_member?)
assign(:project, project) assign(:project, project)
assign(:merge_request, merge_request) assign(:merge_request, merge_request)
assign(:note, note) assign(:note, note)
......
...@@ -12,5 +12,6 @@ RSpec.shared_context 'project show action' do ...@@ -12,5 +12,6 @@ RSpec.shared_context 'project show action' do
stub_template 'shared/issuable/_sidebar' => '' stub_template 'shared/issuable/_sidebar' => ''
stub_template 'projects/issues/_discussion' => '' stub_template 'projects/issues/_discussion' => ''
allow(view).to receive(:user_status).and_return('') allow(view).to receive(:user_status).and_return('')
allow(view).to receive(:can_admin_project_member?)
end end
end end
...@@ -71,7 +71,7 @@ RSpec.describe 'layouts/header/_new_dropdown' do ...@@ -71,7 +71,7 @@ RSpec.describe 'layouts/header/_new_dropdown' do
before do before do
allow(view).to receive(:can?).with(user, :create_projects, group).and_return(true) allow(view).to receive(:can?).with(user, :create_projects, group).and_return(true)
allow(view).to receive(:can?).with(user, :admin_group_member, group).and_return(invite_member) allow(view).to receive(:can?).with(user, :admin_group_member, group).and_return(invite_member)
allow(view).to receive(:can_import_members?).and_return(invite_member) allow(view).to receive(:can_admin_project_member?).and_return(invite_member)
allow(view).to receive(:experiment_enabled?) allow(view).to receive(:experiment_enabled?)
end end
...@@ -142,7 +142,7 @@ RSpec.describe 'layouts/header/_new_dropdown' do ...@@ -142,7 +142,7 @@ RSpec.describe 'layouts/header/_new_dropdown' do
let(:href) { project_project_members_path(project) } let(:href) { project_project_members_path(project) }
before do before do
allow(view).to receive(:can_import_members?).and_return(invite_member) allow(view).to receive(:can_admin_project_member?).and_return(invite_member)
stub_current_user(user) stub_current_user(user)
allow(view).to receive(:experiment_enabled?) allow(view).to receive(:experiment_enabled?)
end end
......
...@@ -6,7 +6,10 @@ RSpec.describe 'projects/empty' do ...@@ -6,7 +6,10 @@ RSpec.describe 'projects/empty' do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { ProjectPresenter.new(create(:project, :empty_repo), current_user: user) } let_it_be(:project) { ProjectPresenter.new(create(:project, :empty_repo), current_user: user) }
let(:can_admin_project_member) { true }
before do before do
allow(view).to receive(:can_admin_project_member?).and_return(can_admin_project_member)
allow(view).to receive(:experiment_enabled?).and_return(true) allow(view).to receive(:experiment_enabled?).and_return(true)
allow(view).to receive(:current_user).and_return(user) allow(view).to receive(:current_user).and_return(user)
assign(:project, project) assign(:project, project)
...@@ -47,12 +50,6 @@ RSpec.describe 'projects/empty' do ...@@ -47,12 +50,6 @@ RSpec.describe 'projects/empty' do
end end
context 'with invite button on empty projects' do context 'with invite button on empty projects' do
let(:can_import_members) { true }
before do
allow(view).to receive(:can_import_members?).and_return(can_import_members)
end
it 'shows invite members info', :aggregate_failures do it 'shows invite members info', :aggregate_failures do
render render
...@@ -68,7 +65,7 @@ RSpec.describe 'projects/empty' do ...@@ -68,7 +65,7 @@ RSpec.describe 'projects/empty' do
end end
context 'when user does not have permissions to invite members' do context 'when user does not have permissions to invite members' do
let(:can_import_members) { false } let(:can_admin_project_member) { false }
it 'does not show invite member info', :aggregate_failures do it 'does not show invite member info', :aggregate_failures do
render render
......
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