Commit 07422cb2 authored by Doug Stull's avatar Doug Stull Committed by Thong Kuah

Allow invitation service to receive user_ids

- enhance to allow eventual deprecation of the members API use
  in the invite modal

Changelog: added
parent d74bef76
......@@ -14,7 +14,7 @@ module Members
super
@errors = []
@invites = invites_from_params&.split(',')&.uniq&.flatten
@invites = invites_from_params
@source = params[:source]
end
......@@ -43,7 +43,9 @@ module Members
attr_reader :source, :errors, :invites, :member_created_namespace_id, :members
def invites_from_params
params[:user_ids]
return params[:user_ids] if params[:user_ids].is_a?(Array)
params[:user_ids]&.to_s&.split(',')&.uniq&.flatten || []
end
def validate_invite_source!
......@@ -87,6 +89,7 @@ module Members
end
end
# overridden
def add_error_for_member(member)
prefix = "#{member.user.username}: " if member.user.present?
......@@ -117,7 +120,8 @@ module Members
def create_tasks_to_be_done
return if params[:tasks_to_be_done].blank? || params[:tasks_project_id].blank?
valid_members = members.select { |member| member.valid? && member.member_task.valid? }
# Only create task issues for existing users. Tasks for new users are created when they signup.
valid_members = members.select { |member| member.valid? && member.member_task.valid? && member.user.present? }
return unless valid_members.present?
# We can take the first `member_task` here, since all tasks will have the same attributes needed
......
......@@ -7,6 +7,8 @@ module Members
def initialize(*args)
super
@invites += parsed_emails
@errors = {}
end
......@@ -14,38 +16,63 @@ module Members
alias_method :formatted_errors, :errors
def invites_from_params
params[:email]
def parsed_emails
# can't put this in the initializer since `invites_from_params` is called in super class
# and needs it
@parsed_emails ||= (formatted_param(params[:email]) || [])
end
def formatted_param(parameter)
parameter&.split(',')&.uniq&.flatten
end
def validate_invitable!
super
return if params[:email].blank?
# we need the below due to add_users hitting Members::CreatorService.parse_users_list and ignoring invalid emails
# ideally we wouldn't need this, but we can't really change the add_users method
valid, invalid = invites.partition { |email| Member.valid_email?(email) }
@invites = valid
invalid_emails.each { |email| errors[email] = s_('AddMember|Invite email is invalid') }
end
def invalid_emails
parsed_emails.each_with_object([]) do |email, invalid|
next if Member.valid_email?(email)
invalid.each { |email| errors[email] = s_('AddMember|Invite email is invalid') }
invalid << email
@invites.delete(email)
end
end
override :blank_invites_message
def blank_invites_message
s_('AddMember|Emails cannot be blank')
s_('AddMember|Invites cannot be blank')
end
override :add_error_for_member
def add_error_for_member(member)
errors[invite_email(member)] = member.errors.full_messages.to_sentence
errors[invited_object(member)] = member.errors.full_messages.to_sentence
end
override :create_tasks_to_be_done
def create_tasks_to_be_done
# Only create task issues for existing users. Tasks for new users are created when they signup.
end
def invited_object(member)
return member.invite_email if member.invite_email
def invite_email(member)
member.invite_email || member.user.email
# There is a case where someone was invited by email, but the `user` record exists.
# The member record returned will not have an invite_email attribute defined since
# the CreatorService finds `user` record sometimes by email.
# At that point we lose the info of whether this invite was done by `user` or by email.
# Here we will give preference to check invites by user_id first.
# There is also a case where a user could be invited by their email and
# at the same time via the API in the same request.
# This would would mean the same user is invited as user_id and email.
# However, that isn't as likely from the UI at least since the token generator checks
# for that case and doesn't allow email being used if the user exists as a record already.
if member.user_id.to_s.in?(invites)
member.user.username
else
member.user.all_emails.detect { |email| email.in?(invites) }
end
end
end
end
......@@ -2334,15 +2334,15 @@ msgstr ""
msgid "AddContextCommits|Add/remove"
msgstr ""
msgid "AddMember|Emails cannot be blank"
msgstr ""
msgid "AddMember|Invite email is invalid"
msgstr ""
msgid "AddMember|Invite limit of %{daily_invites} per day exceeded"
msgstr ""
msgid "AddMember|Invites cannot be blank"
msgstr ""
msgid "AddMember|No invite source provided."
msgstr ""
......
......@@ -240,7 +240,7 @@ RSpec.describe API::Invitations do
params: { email: '', access_level: Member::MAINTAINER }
expect(response).to have_gitlab_http_status(:created)
expect(json_response['message']).to eq('Emails cannot be blank')
expect(json_response['message']).to eq('Invites cannot be blank')
end
it 'returns 404 when the email list is not a valid format' do
......
......@@ -6,6 +6,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
let_it_be(:source, reload: true) { create(:project) }
let_it_be(:user) { create(:user) }
let_it_be(:member) { create(:user) }
let_it_be(:user_invited_by_id) { create(:user) }
let_it_be(:user_ids) { member.id.to_s }
let_it_be(:access_level) { Gitlab::Access::GUEST }
......@@ -49,6 +50,36 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(true)
end
context 'when user_id is passed as an integer' do
let(:user_ids) { member.id }
it 'successfully creates member' do
expect(execute_service[:status]).to eq(:success)
expect(source.users).to include member
expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(true)
end
end
context 'with user_ids as an array of integers' do
let(:user_ids) { [member.id, user_invited_by_id.id] }
it 'successfully creates members' do
expect(execute_service[:status]).to eq(:success)
expect(source.users).to include(member, user_invited_by_id)
expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(true)
end
end
context 'with user_ids as an array of strings' do
let(:user_ids) { [member.id.to_s, user_invited_by_id.id.to_s] }
it 'successfully creates members' do
expect(execute_service[:status]).to eq(:success)
expect(source.users).to include(member, user_invited_by_id)
expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(true)
end
end
context 'when executing on a group' do
let_it_be(:source) { create(:group) }
......@@ -191,6 +222,15 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
)
end
context 'when it is an invite by email passed to user_ids' do
let(:user_ids) { 'email@example.org' }
it 'does not create task issues' do
expect(TasksToBeDone::CreateWorker).not_to receive(:perform_async)
execute_service
end
end
context 'when passing many user ids' do
before do
stub_licensed_features(multiple_issue_assignees: false)
......
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