Merge branch 'add-already-exists-back-to-members' into 'master'

Revert user already exists to members API

See merge request gitlab-org/gitlab!80672

lib/api/helpers/members_helpers.rb

@@ -62,10 +62,13 @@ module API
       end
 
       def add_single_member_by_user_id(create_service_params)
+        source = create_service_params[:source]
         user_id = create_service_params[:user_ids]
         user = User.find_by(id: user_id) # rubocop: disable CodeReuse/ActiveRecord
 
         if user
+          conflict!('Member already exists') if member_already_exists?(source, user_id)
+
           instance = ::Members::CreateService.new(current_user, create_service_params)
           instance.execute
 
@@ -87,6 +90,12 @@ module API
       def add_single_member?(user_id)
         user_id.present?
       end
+
+      private
+
+      def member_already_exists?(source, user_id)
+        source.members.exists?(user_id: user_id) # rubocop: disable CodeReuse/ActiveRecord
+      end
     end
   end
 end

spec/requests/api/members_spec.rb

@@ -291,25 +291,6 @@ RSpec.describe API::Members do
               user: maintainer
             )
           end
-
-          context 'with an already existing member' do
-            before do
-              source.add_developer(stranger)
-            end
-
-            it 'tracks the invite source from params' do
-              post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
-                   params: params.merge(invite_source: '_invite_source_')
-
-              expect_snowplow_event(
-                category: 'Members::CreateService',
-                action: 'create_member',
-                label: '_invite_source_',
-                property: 'existing_user',
-                user: maintainer
-              )
-            end
-          end
         end
 
         context 'when executing the Members::CreateService for multiple user_ids' do
@@ -418,49 +399,6 @@ RSpec.describe API::Members do
             expect(member.tasks_to_be_done).to match_array([:code, :ci])
             expect(member.member_task.project_id).to eq(project_id)
           end
-
-          context 'with already existing member' do
-            before do
-              source.add_developer(stranger)
-            end
-
-            it 'does not update tasks to be done if tasks already exist', :aggregate_failures do
-              member = source.members.find_by(user_id: stranger.id)
-              create(:member_task, member: member, project_id: project_id, tasks_to_be_done: %w(code ci))
-
-              expect do
-                post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
-                     params: {
-                       user_id: stranger.id,
-                       access_level: Member::DEVELOPER,
-                       tasks_to_be_done: %w(issues),
-                       tasks_project_id: project_id
-                     }
-              end.not_to change(MemberTask, :count)
-
-              member.reset
-              expect(response).to have_gitlab_http_status(:created)
-              expect(member.tasks_to_be_done).to match_array([:code, :ci])
-              expect(member.member_task.project_id).to eq(project_id)
-            end
-
-            it 'adds tasks to be done if they do not exist', :aggregate_failures do
-              expect do
-                post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
-                     params: {
-                       user_id: stranger.id,
-                       access_level: Member::DEVELOPER,
-                       tasks_to_be_done: %w(issues),
-                       tasks_project_id: project_id
-                     }
-              end.to change(MemberTask, :count).by(1)
-
-              member = source.members.find_by(user_id: stranger.id)
-              expect(response).to have_gitlab_http_status(:created)
-              expect(member.tasks_to_be_done).to match_array([:issues])
-              expect(member.member_task.project_id).to eq(project_id)
-            end
-          end
         end
 
         context 'when there are multiple users to add' do
@@ -474,68 +412,16 @@ RSpec.describe API::Members do
               expect(member.member_task.project_id).to eq(project_id)
             end
           end
-
-          context 'with already existing members' do
-            before do
-              source.add_developer(stranger)
-              source.add_developer(developer)
-            end
-
-            it 'does not update tasks to be done if tasks already exist', :aggregate_failures do
-              members = source.members.where(user_id: [developer.id, stranger.id])
-              members.each do |member|
-                create(:member_task, member: member, project_id: project_id, tasks_to_be_done: %w(code ci))
-              end
-
-              expect do
-                post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
-                     params: {
-                       user_id: [developer.id, stranger.id].join(','),
-                       access_level: Member::DEVELOPER,
-                       tasks_to_be_done: %w(issues),
-                       tasks_project_id: project_id
-                     }
-              end.not_to change(MemberTask, :count)
-
-              expect(response).to have_gitlab_http_status(:created)
-              members.each do |member|
-                member.reset
-                expect(member.tasks_to_be_done).to match_array([:code, :ci])
-                expect(member.member_task.project_id).to eq(project_id)
-              end
-            end
-
-            it 'adds tasks to be done if they do not exist', :aggregate_failures do
-              expect do
-                post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
-                     params: {
-                       user_id: [developer.id, stranger.id].join(','),
-                       access_level: Member::DEVELOPER,
-                       tasks_to_be_done: %w(issues),
-                       tasks_project_id: project_id
-                     }
-              end.to change(MemberTask, :count).by(2)
-
-              expect(response).to have_gitlab_http_status(:created)
-              members = source.members.where(user_id: [developer.id, stranger.id])
-              members.each do |member|
-                expect(member.tasks_to_be_done).to match_array([:issues])
-                expect(member.member_task.project_id).to eq(project_id)
-              end
-            end
-          end
         end
       end
 
-      it "updates a current member" do
+      it "returns 409 if member already exists" do
         source.add_guest(stranger)
 
         post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
-             params: { user_id: stranger.id, access_level: Member::MAINTAINER }
+             params: { user_id: maintainer.id, access_level: Member::MAINTAINER }
 
-        expect(response).to have_gitlab_http_status(:created)
-        expect(json_response['id']).to eq(stranger.id)
-        expect(json_response['access_level']).to eq(Member::MAINTAINER)
+        expect(response).to have_gitlab_http_status(:conflict)
       end
 
       it 'returns 404 when the user_id is not valid' do