Commit 12b84576 authored by Etienne Baqué's avatar Etienne Baqué

Merge branch '296640-engineering-add-invite-option-to-the-comment-component' into 'master'

Refactor invite member api service

See merge request gitlab-org/gitlab!55241
parents 5382795f 025197ea
......@@ -2,112 +2,97 @@
module Members
class InviteService < Members::BaseService
DEFAULT_LIMIT = 100
BlankEmailsError = Class.new(StandardError)
TooManyEmailsError = Class.new(StandardError)
attr_reader :errors
def initialize(*args)
super
def initialize(current_user, params)
@current_user, @params = current_user, params.dup
@errors = {}
@emails = params[:email]&.split(',')&.uniq&.flatten
end
def execute(source)
return error(s_('Email cannot be blank')) if params[:email].blank?
validate_emails!
emails = params[:email].split(',').uniq.flatten
return error(s_("Too many users specified (limit is %{user_limit})") % { user_limit: user_limit }) if
user_limit && emails.size > user_limit
emails.each do |email|
next if existing_member?(source, email)
next if existing_invite?(source, email)
next if existing_request?(source, email)
if existing_user?(email)
add_existing_user_as_member(current_user, source, params, email)
next
end
invite_new_member_and_user(current_user, source, params, email)
end
return success unless errors.any?
error(errors)
@source = source
emails.each(&method(:process_email))
result
rescue BlankEmailsError, TooManyEmailsError => e
error(e.message)
end
private
def invite_new_member_and_user(current_user, source, params, email)
new_member = (source.class.name + 'Member').constantize.create(source_id: source.id,
user_id: nil,
access_level: params[:access_level],
invite_email: email,
created_by_id: current_user.id,
expires_at: params[:expires_at])
attr_reader :source, :errors, :emails
unless new_member.valid? && new_member.persisted?
errors[params[:email]] = new_member.errors.full_messages.to_sentence
end
end
def add_existing_user_as_member(current_user, source, params, email)
new_member = create_member(current_user, existing_user(email), source, params.merge({ invite_email: email }))
def validate_emails!
raise BlankEmailsError, s_('AddMember|Email cannot be blank') if emails.blank?
unless new_member.valid? && new_member.persisted?
errors[email] = new_member.errors.full_messages.to_sentence
if user_limit && emails.size > user_limit
raise TooManyEmailsError, s_("AddMember|Too many users specified (limit is %{user_limit})") % { user_limit: user_limit }
end
end
def create_member(current_user, user, source, params)
source.add_user(user, params[:access_level], current_user: current_user, expires_at: params[:expires_at])
def user_limit
limit = params.fetch(:limit, Members::CreateService::DEFAULT_LIMIT)
limit < 0 ? nil : limit
end
def user_limit
limit = params.fetch(:limit, DEFAULT_LIMIT)
def process_email(email)
return if existing_member?(email)
return if existing_invite?(email)
return if existing_request?(email)
limit && limit < 0 ? nil : limit
add_member(email)
end
def existing_member?(source, email)
def existing_member?(email)
existing_member = source.members.with_user_by_email(email).exists?
if existing_member
errors[email] = "Already a member of #{source.name}"
errors[email] = s_("AddMember|Already a member of %{source_name}") % { source_name: source.name }
return true
end
false
end
def existing_invite?(source, email)
def existing_invite?(email)
existing_invite = source.members.search_invite_email(email).exists?
if existing_invite
errors[email] = "Member already invited to #{source.name}"
errors[email] = s_("AddMember|Member already invited to %{source_name}") % { source_name: source.name }
return true
end
false
end
def existing_request?(source, email)
def existing_request?(email)
existing_request = source.requesters.with_user_by_email(email).exists?
if existing_request
errors[email] = "Member cannot be invited because they already requested to join #{source.name}"
errors[email] = s_("AddMember|Member cannot be invited because they already requested to join %{source_name}") % { source_name: source.name }
return true
end
false
end
def existing_user(email)
User.find_by_email(email)
def add_member(email)
new_member = source.add_user(email, params[:access_level], current_user: current_user, expires_at: params[:expires_at])
errors[email] = new_member.errors.full_messages.to_sentence if new_member.invalid?
end
def existing_user?(email)
existing_user(email).present?
def result
if errors.any?
error(errors)
else
success
end
end
end
end
......@@ -1965,9 +1965,21 @@ msgstr ""
msgid "AddContextCommits|Add/remove"
msgstr ""
msgid "AddMember|Already a member of %{source_name}"
msgstr ""
msgid "AddMember|Email cannot be blank"
msgstr ""
msgid "AddMember|Invite limit of %{daily_invites} per day exceeded"
msgstr ""
msgid "AddMember|Member already invited to %{source_name}"
msgstr ""
msgid "AddMember|Member cannot be invited because they already requested to join %{source_name}"
msgstr ""
msgid "AddMember|No users specified."
msgstr ""
......@@ -11111,9 +11123,6 @@ msgstr ""
msgid "Email address to use for Support Desk"
msgstr ""
msgid "Email cannot be blank"
msgstr ""
msgid "Email could not be sent"
msgstr ""
......@@ -31415,9 +31424,6 @@ msgstr ""
msgid "Too many projects enabled. You will need to manage them via the console or the API."
msgstr ""
msgid "Too many users specified (limit is %{user_limit})"
msgstr ""
msgid "Too much data"
msgstr ""
......
......@@ -3,14 +3,14 @@
require 'spec_helper'
RSpec.describe API::Invitations do
let(:maintainer) { create(:user, username: 'maintainer_user') }
let(:developer) { create(:user) }
let(:access_requester) { create(:user) }
let(:stranger) { create(:user) }
let_it_be(:maintainer) { create(:user, username: 'maintainer_user') }
let_it_be(:developer) { create(:user) }
let_it_be(:access_requester) { create(:user) }
let_it_be(:stranger) { create(:user) }
let(:email) { 'email1@example.com' }
let(:email2) { 'email2@example.com' }
let(:project) do
let_it_be(:project) do
create(:project, :public, creator_id: maintainer.id, namespace: maintainer.namespace) do |project|
project.add_developer(developer)
project.add_maintainer(maintainer)
......@@ -18,7 +18,7 @@ RSpec.describe API::Invitations do
end
end
let!(:group) do
let_it_be(:group, reload: true) do
create(:group, :public) do |group|
group.add_developer(developer)
group.add_owner(maintainer)
......
......@@ -2,76 +2,155 @@
require 'spec_helper'
RSpec.describe Members::InviteService do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:project_user) { create(:user) }
RSpec.describe Members::InviteService, :aggregate_failures do
let_it_be(:project) { create(:project) }
let_it_be(:user) { project.owner }
let_it_be(:project_user) { create(:user) }
let(:params) { {} }
let(:base_params) { { access_level: Gitlab::Access::GUEST } }
before do
project.add_maintainer(user)
subject(:result) { described_class.new(user, base_params.merge(params)).execute(project) }
context 'when email is previously unused by current members' do
let(:params) { { email: 'email@example.org' } }
it 'successfully creates a member' do
expect { result }.to change(ProjectMember, :count).by(1)
expect(result[:status]).to eq(:success)
end
end
it 'adds an existing user to members' do
params = { email: project_user.email.to_s, access_level: Gitlab::Access::GUEST }
result = described_class.new(user, params).execute(project)
context 'when emails are passed as an array' do
let(:params) { { email: %w[email@example.org email2@example.org] } }
it 'successfully creates members' do
expect { result }.to change(ProjectMember, :count).by(2)
expect(result[:status]).to eq(:success)
expect(project.users).to include project_user
end
end
it 'creates a new user for an unknown email address' do
params = { email: 'email@example.org', access_level: Gitlab::Access::GUEST }
result = described_class.new(user, params).execute(project)
context 'when emails are passed as an empty string' do
let(:params) { { email: '' } }
it 'returns an error' do
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Email cannot be blank')
end
end
context 'when email param is not included' do
it 'returns an error' do
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Email cannot be blank')
end
end
context 'when email is not a valid email' do
let(:params) { { email: '_bogus_' } }
it 'returns an error' do
expect { result }.not_to change(ProjectMember, :count)
expect(result[:status]).to eq(:error)
expect(result[:message]['_bogus_']).to eq("Invite email is invalid")
end
end
context 'when duplicate email addresses are passed' do
let(:params) { { email: 'email@example.org,email@example.org' } }
it 'only creates one member per unique address' do
expect { result }.to change(ProjectMember, :count).by(1)
expect(result[:status]).to eq(:success)
end
end
it 'limits the number of emails to 100' do
emails = Array.new(101).map { |n| "email#{n}@example.com" }
params = { email: emails, access_level: Gitlab::Access::GUEST }
context 'when observing email limits' do
let_it_be(:emails) { Array(1..101).map { |n| "email#{n}@example.com" } }
result = described_class.new(user, params).execute(project)
context 'when over the allowed default limit of emails' do
let(:params) { { email: emails } }
it 'limits the number of emails to 100' do
expect { result }.not_to change(ProjectMember, :count)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Too many users specified (limit is 100)')
end
end
it 'does not invite an invalid email' do
params = { email: project_user.id.to_s, access_level: Gitlab::Access::GUEST }
result = described_class.new(user, params).execute(project)
context 'when over the allowed custom limit of emails' do
let(:params) { { email: 'email@example.org,email2@example.org', limit: 1 } }
it 'limits the number of emails to the limit supplied' do
expect { result }.not_to change(ProjectMember, :count)
expect(result[:status]).to eq(:error)
expect(result[:message][project_user.id.to_s]).to eq("Invite email is invalid")
expect(project.users).not_to include project_user
expect(result[:message]).to eq('Too many users specified (limit is 1)')
end
end
context 'when limit allowed is disabled via limit param' do
let(:params) { { email: emails, limit: -1 } }
it 'does not limit number of emails' do
expect { result }.to change(ProjectMember, :count).by(101)
expect(result[:status]).to eq(:success)
end
end
end
it 'does not invite to an invalid access level' do
params = { email: project_user.email, access_level: -1 }
result = described_class.new(user, params).execute(project)
context 'when email belongs to an existing user' do
let(:params) { { email: project_user.email } }
it 'adds an existing user to members' do
expect { result }.to change(ProjectMember, :count).by(1)
expect(result[:status]).to eq(:success)
expect(project.users).to include project_user
end
end
context 'when access level is not valid' do
let(:params) { { email: project_user.email, access_level: -1 } }
it 'returns an error' do
expect { result }.not_to change(ProjectMember, :count)
expect(result[:status]).to eq(:error)
expect(result[:message][project_user.email]).to eq("Access level is not included in the list")
end
end
it 'does not add a member with an existing invite' do
invited_member = create(:project_member, :invited, project: project)
params = { email: invited_member.invite_email,
access_level: Gitlab::Access::GUEST }
result = described_class.new(user, params).execute(project)
context 'when invite already exists for an included email' do
let!(:invited_member) { create(:project_member, :invited, project: project) }
let(:params) { { email: "#{invited_member.invite_email},#{project_user.email}" } }
it 'adds new email and returns an error for the already invited email' do
expect { result }.to change(ProjectMember, :count).by(1)
expect(result[:status]).to eq(:error)
expect(result[:message][invited_member.invite_email]).to eq("Member already invited to #{project.name}")
expect(project.users).to include project_user
end
end
it 'does not add a member with an access_request' do
requested_member = create(:project_member, :access_request, project: project)
context 'when access request already exists for an included email' do
let!(:requested_member) { create(:project_member, :access_request, project: project) }
let(:params) { { email: "#{requested_member.user.email},#{project_user.email}" } }
it 'adds new email and returns an error for the already invited email' do
expect { result }.to change(ProjectMember, :count).by(1)
expect(result[:status]).to eq(:error)
expect(result[:message][requested_member.user.email])
.to eq("Member cannot be invited because they already requested to join #{project.name}")
expect(project.users).to include project_user
end
end
params = { email: requested_member.user.email,
access_level: Gitlab::Access::GUEST }
result = described_class.new(user, params).execute(project)
context 'when email is already a member on the project' do
let!(:existing_member) { create(:project_member, :guest, project: project) }
let(:params) { { email: "#{existing_member.user.email},#{project_user.email}" } }
it 'adds new email and returns an error for the already invited email' do
expect { result }.to change(ProjectMember, :count).by(1)
expect(result[:status]).to eq(:error)
expect(result[:message][requested_member.user.email]).to eq("Member cannot be invited because they already requested to join #{project.name}")
expect(result[:message][existing_member.user.email]).to eq("Already a member of #{project.name}")
expect(project.users).to include project_user
end
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