Remove member create controller code

- no longer needed since we now create members via API only

app/controllers/concerns/membership_actions.rb

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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 'Admin area > Sign-up restrictions', and check the 'Domain denylist'.", :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 'Admin area > Sign-up restrictions', and check 'Allowed domains for sign-ups'.", :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 'Admin area > Sign-up restrictions', and check 'Email restrictions for sign-ups'.", :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 'Admin area > Sign-up restrictions', and check the 'Domain denylist'.", :created
       context 'when the group is restricted by admin signup restrictions' do

spec/controllers/groups/group_members_controller_spec.rb

@@ -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

@@ -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

@@ -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

@@ -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