Skip to content

G
gitlab-ce
Commit 33b013e8 authored by Ethan Urie's avatar Ethan Urie
Browse files
Options

Merge branch '352526-feature-flag-cleanup-invite_members_group_modal2' into 'master' 

Cleanup invite_members_group_modal - remove controller paths

See merge request gitlab-org/gitlab!82411
parents 53ed766d 0f375f69
Show whitespace changes
Inline Side-by-side
Showing with 66 additions and 340 deletions
app/controllers/concerns/membership_actions.rb
View file @ 33b013e8
......@@ -4,17 +4,6 @@ module MembershipActions
include MembersPresentation
extend ActiveSupport::Concern
def create
create_params = params.permit(:user_ids, :access_level, :expires_at)
result = Members::CreateService.new(current_user, create_params.merge({ source: membershipable, invite_source: "#{plain_source_type}-members-page" })).execute
if result[:status] == :success
redirect_to members_page_url, notice: _('Users were successfully added.')
else
redirect_to members_page_url, alert: result[:message]
end
end
def update
update_params = params.require(root_params_key).permit(:access_level, :expires_at)
member = membershipable.members_and_requesters.find(params[:id])
......
app/controllers/groups/group_members_controller.rb
View file @ 33b013e8
......@@ -16,7 +16,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
before_action :authorize_admin_group_member!, except: admin_not_required_endpoints
skip_before_action :check_two_factor_requirement, only: :leave
skip_cross_project_access_check :index, :create, :update, :destroy, :request_access,
skip_cross_project_access_check :index, :update, :destroy, :request_access,
:approve_access_request, :leave, :resend_invite,
:override
......
config/routes/group.rb
View file @ 33b013e8
......@@ -94,7 +94,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
concerns :clusterable
resources :group_members, only: [:index, :create, :update, :destroy], concerns: :access_requestable do
resources :group_members, only: [:index, :update, :destroy], concerns: :access_requestable do
post :resend_invite, on: :member
delete :leave, on: :collection
end
......
config/routes/project.rb
View file @ 33b013e8
......@@ -163,7 +163,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
resources :project_members, except: [:show, :new, :edit], constraints: { id: %r{[a-zA-Z./0-9_\-#%+:]+} }, concerns: :access_requestable do
resources :project_members, except: [:show, :new, :create, :edit], constraints: { id: %r{[a-zA-Z./0-9_\-#%+:]+} }, concerns: :access_requestable do
collection do
delete :leave
......
ee/app/controllers/ee/projects/project_members_controller.rb
View file @ 33b013e8
......@@ -7,7 +7,7 @@ module EE
extend ::Gitlab::Utils::Override
prepended do
before_action :check_membership_lock!, only: [:create, :import, :apply_import]
before_action :check_membership_lock!, only: [:import, :apply_import]
end
def check_membership_lock!
......
ee/spec/controllers/groups/group_members_controller_spec.rb
View file @ 33b013e8
......@@ -45,16 +45,6 @@ RSpec.describe Groups::GroupMembersController do
end
end
describe 'POST #create' do
it 'creates an audit event' do
expect do
post :create, params: { group_id: group,
user_ids: user.id,
access_level: Gitlab::Access::GUEST }
end.to change(AuditEvent, :count).by(1)
end
end
describe 'DELETE #leave' do
context 'when member is not an owner' do
it 'creates an audit event' do
......
ee/spec/controllers/projects/project_members_controller_spec.rb
View file @ 33b013e8
......@@ -44,37 +44,6 @@ RSpec.describe Projects::ProjectMembersController do
end
end
describe 'POST create' do
let(:stranger) { create(:user) }
subject(:create_member) do
post :create, params: {
user_ids: stranger.id,
namespace_id: project.namespace,
access_level: access_level,
project_id: project
}
end
let(:access_level) { nil }
before do
project.add_maintainer(user)
sign_in(user)
end
context 'when project group has membership lock enabled' do
before do
project.namespace.update!(membership_lock: true)
end
it 'responds with 403' do
create_member
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
describe 'GET import' do
subject(:import) do
get :import, params: {
......
ee/spec/requests/api/invitations_spec.rb
View file @ 33b013e8
......@@ -6,7 +6,7 @@ RSpec.describe API::Invitations, 'EE Invitations' do
include GroupAPIHelpers
let_it_be(:admin) { create(:user, :admin, email: 'admin@example.com') }
let_it_be(:group) { create(:group) }
let_it_be(:group, reload: true) { create(:group) }
let(:url) { "/groups/#{group.id}/invitations" }
let(:invite_email) { 'restricted@example.org' }
......@@ -47,8 +47,32 @@ RSpec.describe API::Invitations, 'EE Invitations' do
it_behaves_like 'restricted email error', message, code
end
shared_examples 'member creation audit event' do
it 'creates an audit event while creating a new member' do
params = { email: 'example1@example.com', access_level: Member::DEVELOPER }
expect do
post api(url, admin), params: params
expect(response).to have_gitlab_http_status(:created)
end.to change { AuditEvent.count }.by(1)
end
it 'does not create audit event if creating a new member fails' do
params = { email: '_bogus_', access_level: Member::DEVELOPER }
expect do
post api(url, admin), params: params
expect(response).to have_gitlab_http_status(:bad_request)
end.not_to change { AuditEvent.count }
end
end
describe 'POST /groups/:id/invitations' do
it_behaves_like 'member creation audit event'
it_behaves_like 'admin signup restrictions email error - denylist', "The member's email address is not allowed for this group. Go to the &#39;Admin area &gt; Sign-up restrictions&#39;, and check the &#39;Domain denylist&#39;.", :created
context 'when the group is restricted by admin signup restrictions' do
it_behaves_like 'admin signup restrictions email error - allowlist', "The member's email address is not allowed for this group. Go to the &#39;Admin area &gt; Sign-up restrictions&#39;, and check &#39;Allowed domains for sign-ups&#39;.", :created
it_behaves_like 'admin signup restrictions email error - email restrictions', "The member's email address is not allowed for this group. Go to the &#39;Admin area &gt; Sign-up restrictions&#39;, and check &#39;Email restrictions for sign-ups&#39;.", :created
......@@ -69,6 +93,23 @@ RSpec.describe API::Invitations, 'EE Invitations' do
let(:url) { "/projects/#{project.id}/invitations" }
it_behaves_like 'member creation audit event'
context 'with group membership locked' do
before do
group.update!(membership_lock: true)
end
it 'returns an error and exception message when group membership lock is enabled' do
params = { email: 'example1@example.com', access_level: Member::DEVELOPER }
post api(url, admin), params: params
expect(json_response['message']).to eq 'Members::CreateService::MembershipLockedError'
expect(json_response['status']).to eq 'error'
end
end
context 'when the project is restricted by admin signup restrictions' do
it_behaves_like 'admin signup restrictions email error - denylist', "The member's email address is not allowed for this project. Go to the &#39;Admin area &gt; Sign-up restrictions&#39;, and check the &#39;Domain denylist&#39;.", :created
context 'when the group is restricted by admin signup restrictions' do
......
spec/controllers/groups/group_members_controller_spec.rb
View file @ 33b013e8
......@@ -99,107 +99,6 @@ RSpec.describe Groups::GroupMembersController do
end
end
describe 'POST create' do
let_it_be(:group_user) { create(:user) }
before do
sign_in(user)
end
context 'when user does not have enough rights' do
before do
group.add_developer(user)
end
it 'returns 403', :aggregate_failures do
post :create, params: {
group_id: group,
user_ids: group_user.id,
access_level: Gitlab::Access::GUEST
}
expect(response).to have_gitlab_http_status(:forbidden)
expect(group.users).not_to include group_user
end
end
context 'when user has enough rights' do
before do
group.add_owner(user)
end
it 'adds user to members', :aggregate_failures, :snowplow do
post :create, params: {
group_id: group,
user_ids: group_user.id,
access_level: Gitlab::Access::GUEST
}
expect(controller).to set_flash.to 'Users were successfully added.'
expect(response).to redirect_to(group_group_members_path(group))
expect(group.users).to include group_user
expect_snowplow_event(
category: 'Members::CreateService',
action: 'create_member',
label: 'group-members-page',
property: 'existing_user',
user: user
)
end
it 'adds no user to members', :aggregate_failures do
post :create, params: {
group_id: group,
user_ids: '',
access_level: Gitlab::Access::GUEST
}
expect(controller).to set_flash.to 'No users specified.'
expect(response).to redirect_to(group_group_members_path(group))
expect(group.users).not_to include group_user
end
end
context 'access expiry date' do
before do
group.add_owner(user)
end
subject do
post :create, params: {
group_id: group,
user_ids: group_user.id,
access_level: Gitlab::Access::GUEST,
expires_at: expires_at
}
end
context 'when set to a date in the past' do
let(:expires_at) { 2.days.ago }
it 'does not add user to members', :aggregate_failures do
subject
expect(flash[:alert]).to include('Expires at cannot be a date in the past')
expect(response).to redirect_to(group_group_members_path(group))
expect(group.users).not_to include group_user
end
end
context 'when set to a date in the future' do
let(:expires_at) { 5.days.from_now }
it 'adds user to members', :aggregate_failures do
subject
expect(controller).to set_flash.to 'Users were successfully added.'
expect(response).to redirect_to(group_group_members_path(group))
expect(group.users).to include group_user
end
end
end
end
describe 'PUT update' do
let_it_be(:requester) { create(:group_member, :access_request, group: group) }
......@@ -508,14 +407,6 @@ RSpec.describe Groups::GroupMembersController do
end
end
describe 'POST #create' do
it 'is successful' do
post :create, params: { group_id: group, users: user, access_level: Gitlab::Access::GUEST }
expect(response).to have_gitlab_http_status(:found)
end
end
describe 'PUT #update' do
it 'is successful' do
put :update,
......
spec/controllers/projects/project_members_controller_spec.rb
View file @ 33b013e8
......@@ -147,137 +147,6 @@ RSpec.describe Projects::ProjectMembersController do
end
end
describe 'POST create' do
let_it_be(:project_user) { create(:user) }
before do
sign_in(user)
end
context 'when user does not have enough rights' do
before do
project.add_developer(user)
end
it 'returns 404', :aggregate_failures do
post :create, params: {
namespace_id: project.namespace,
project_id: project,
user_ids: project_user.id,
access_level: Gitlab::Access::GUEST
}
expect(response).to have_gitlab_http_status(:not_found)
expect(project.users).not_to include project_user
end
end
context 'when user has enough rights' do
before do
project.add_maintainer(user)
end
it 'adds user to members', :aggregate_failures, :snowplow do
post :create, params: {
namespace_id: project.namespace,
project_id: project,
user_ids: project_user.id,
access_level: Gitlab::Access::GUEST
}
expect(controller).to set_flash.to 'Users were successfully added.'
expect(response).to redirect_to(project_project_members_path(project))
expect(project.users).to include project_user
expect_snowplow_event(
category: 'Members::CreateService',
action: 'create_member',
label: 'project-members-page',
property: 'existing_user',
user: user
)
end
it 'adds no user to members', :aggregate_failures do
expect_next_instance_of(Members::CreateService) do |instance|
expect(instance).to receive(:execute).and_return(status: :failure, message: 'Message')
end
post :create, params: {
namespace_id: project.namespace,
project_id: project,
user_ids: '',
access_level: Gitlab::Access::GUEST
}
expect(controller).to set_flash.to 'Message'
expect(response).to redirect_to(project_project_members_path(project))
end
end
context 'adding project bot' do
let_it_be(:project_bot) { create(:user, :project_bot) }
before do
project.add_maintainer(user)
unrelated_project = create(:project)
unrelated_project.add_maintainer(project_bot)
end
it 'returns error', :aggregate_failures do
post :create, params: {
namespace_id: project.namespace,
project_id: project,
user_ids: project_bot.id,
access_level: Gitlab::Access::GUEST
}
expect(flash[:alert]).to include('project bots cannot be added to other groups / projects')
expect(response).to redirect_to(project_project_members_path(project))
end
end
context 'access expiry date' do
before do
project.add_maintainer(user)
end
subject do
post :create, params: {
namespace_id: project.namespace,
project_id: project,
user_ids: project_user.id,
access_level: Gitlab::Access::GUEST,
expires_at: expires_at
}
end
context 'when set to a date in the past' do
let(:expires_at) { 2.days.ago }
it 'does not add user to members', :aggregate_failures do
subject
expect(flash[:alert]).to include('Expires at cannot be a date in the past')
expect(response).to redirect_to(project_project_members_path(project))
expect(project.users).not_to include project_user
end
end
context 'when set to a date in the future' do
let(:expires_at) { 5.days.from_now }
it 'adds user to members', :aggregate_failures do
subject
expect(controller).to set_flash.to 'Users were successfully added.'
expect(response).to redirect_to(project_project_members_path(project))
expect(project.users).to include project_user
end
end
end
end
describe 'PUT update' do
let_it_be(:requester) { create(:project_member, :access_request, project: project) }
......@@ -656,48 +525,6 @@ RSpec.describe Projects::ProjectMembersController do
end
end
describe 'POST create' do
let_it_be(:stranger) { create(:user) }
context 'when creating owner' do
before do
project.add_maintainer(user)
sign_in(user)
end
it 'creates a member' do
expect do
post :create, params: {
user_ids: stranger.id,
namespace_id: project.namespace,
access_level: Member::OWNER,
project_id: project
}
end.to change { project.members.count }.by(1)
expect(project.team_members).to include(user)
end
end
context 'when create maintainer' do
before do
project.add_maintainer(user)
sign_in(user)
end
it 'creates a member' do
expect do
post :create, params: {
user_ids: stranger.id,
namespace_id: project.namespace,
access_level: Member::MAINTAINER,
project_id: project
}
end.to change { project.members.count }.by(1)
end
end
end
describe 'POST resend_invite' do
let_it_be(:member) { create(:project_member, project: project) }
......
spec/requests/api/invitations_spec.rb
View file @ 33b013e8
......@@ -208,6 +208,25 @@ RSpec.describe API::Invitations do
end
end
context 'when adding project bot' do
let_it_be(:project_bot) { create(:user, :project_bot) }
before do
unrelated_project = create(:project)
unrelated_project.add_maintainer(project_bot)
end
it 'returns error' do
expect do
post invitations_url(source, maintainer),
params: { email: project_bot.email, access_level: Member::DEVELOPER }
expect(json_response['status']).to eq 'error'
expect(json_response['message'][project_bot.email]).to include('User project bots cannot be added to other groups / projects')
end.not_to change { source.members.count }
end
end
it "returns a message if member already exists" do
post invitations_url(source, maintainer),
params: { email: developer.email, access_level: Member::MAINTAINER }
......
spec/routing/project_routing_spec.rb
View file @ 33b013e8
......@@ -395,7 +395,7 @@ RSpec.describe 'project routing' do
# DELETE /:project_id/project_members/:id(.:format) project_members#destroy
describe Projects::ProjectMembersController, 'routing' do
it_behaves_like 'resource routing' do
let(:actions) { %i[index create update destroy] }
let(:actions) { %i[index update destroy] }
let(:base_path) { '/gitlab/gitlabhq/-/project_members' }
end
end
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7