diff --git a/app/builders/agent_builder.rb b/app/builders/agent_builder.rb new file mode 100644 index 000000000..6ea68821d --- /dev/null +++ b/app/builders/agent_builder.rb @@ -0,0 +1,60 @@ +# The AgentBuilder class is responsible for creating a new agent. +# It initializes with necessary attributes and provides a perform method +# to create a user and account user in a transaction. +class AgentBuilder + # Initializes an AgentBuilder with necessary attributes. + # @param email [String] the email of the user. + # @param name [String] the name of the user. + # @param role [String] the role of the user, defaults to 'agent' if not provided. + # @param inviter [User] the user who is inviting the agent (Current.user in most cases). + # @param availability [String] the availability status of the user, defaults to 'offline' if not provided. + # @param auto_offline [Boolean] the auto offline status of the user. + pattr_initialize [:email, { name: '' }, :inviter, :account, { role: :agent }, { availability: :offline }, { auto_offline: false }] + + # Creates a user and account user in a transaction. + # @return [User] the created user. + def perform + ActiveRecord::Base.transaction do + @user = find_or_create_user + send_confirmation_if_required + create_account_user + end + @user + end + + private + + # Finds a user by email or creates a new one with a temporary password. + # @return [User] the found or created user. + def find_or_create_user + user = User.find_by(email: email) + return user if user + + temp_password = "1!aA#{SecureRandom.alphanumeric(12)}" + User.create!(email: email, name: name, password: temp_password, password_confirmation: temp_password) + end + + # Sends confirmation instructions if the user is persisted and not confirmed. + def send_confirmation_if_required + @user.send_confirmation_instructions if user_needs_confirmation? + end + + # Checks if the user needs confirmation. + # @return [Boolean] true if the user is persisted and not confirmed, false otherwise. + def user_needs_confirmation? + @user.persisted? && !@user.confirmed? + end + + # Creates an account user linking the user to the current account. + def create_account_user + AccountUser.create!({ + account_id: account.id, + user_id: @user.id, + inviter_id: inviter.id + }.merge({ + role: role, + availability: availability, + auto_offline: auto_offline + }.compact)) + end +end diff --git a/app/controllers/api/v1/accounts/agents_controller.rb b/app/controllers/api/v1/accounts/agents_controller.rb index 9a089f242..768d1a3ff 100644 --- a/app/controllers/api/v1/accounts/agents_controller.rb +++ b/app/controllers/api/v1/accounts/agents_controller.rb @@ -1,16 +1,26 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController - before_action :fetch_agent, except: [:create, :index] + before_action :fetch_agent, except: [:create, :index, :bulk_create] before_action :check_authorization - before_action :find_user, only: [:create] before_action :validate_limit, only: [:create] - before_action :create_user, only: [:create] - before_action :save_account_user, only: [:create] + before_action :validate_limit_for_bulk_create, only: [:bulk_create] def index @agents = agents end - def create; end + def create + builder = AgentBuilder.new( + email: new_agent_params['email'], + name: new_agent_params['name'], + role: new_agent_params['role'], + availability: new_agent_params['availability'], + auto_offline: new_agent_params['auto_offline'], + inviter: current_user, + account: Current.account + ) + + builder.perform + end def update @agent.update!(agent_params.slice(:name).compact) @@ -23,6 +33,21 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController head :ok end + def bulk_create + emails = params[:emails] + + emails.each do |email| + builder = AgentBuilder.new( + email: email, + name: email.split('@').first, + inviter: current_user, + account: Current.account + ) + builder.perform + end + head :ok + end + private def check_authorization @@ -33,47 +58,34 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController @agent = agents.find(params[:id]) end - def find_user - @user = User.find_by(email: new_agent_params[:email]) - end - - # TODO: move this to a builder and combine the save account user method into a builder - # ensure the account user association is also created in a single transaction - def create_user - return @user.send_confirmation_instructions if @user - - @user = User.create!(new_agent_params.slice(:email, :name, :password, :password_confirmation)) - end - - def save_account_user - AccountUser.create!({ - account_id: Current.account.id, - user_id: @user.id, - inviter_id: current_user.id - }.merge({ - role: new_agent_params[:role], - availability: new_agent_params[:availability], - auto_offline: new_agent_params[:auto_offline] - }.compact)) - end - def agent_params params.require(:agent).permit(:name, :email, :name, :role, :availability, :auto_offline) end def new_agent_params - # intial string ensures the password requirements are met - temp_password = "1!aA#{SecureRandom.alphanumeric(12)}" params.require(:agent).permit(:email, :name, :role, :availability, :auto_offline) - .merge!(password: temp_password, password_confirmation: temp_password, inviter: current_user) end def agents @agents ||= Current.account.users.order_by_full_name.includes(:account_users, { avatar_attachment: [:blob] }) end + def validate_limit_for_bulk_create + limit_available = params[:emails].count <= available_agent_count + + render_payment_required('Account limit exceeded. Please purchase more licenses') unless limit_available + end + def validate_limit - render_payment_required('Account limit exceeded. Please purchase more licenses') if agents.count >= Current.account.usage_limits[:agents] + render_payment_required('Account limit exceeded. Please purchase more licenses') unless can_add_agent? + end + + def available_agent_count + Current.account.usage_limits[:agents] - agents.count + end + + def can_add_agent? + available_agent_count.positive? end def delete_user_record(agent) diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index b19874fd1..cba1720ec 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -14,4 +14,8 @@ class UserPolicy < ApplicationPolicy def destroy? @account_user.administrator? end + + def bulk_create? + @account_user.administrator? + end end diff --git a/config/routes.rb b/config/routes.rb index 653ba8277..210d5bc0e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -44,7 +44,9 @@ Rails.application.routes.draw do resource :contact_merge, only: [:create] end resource :bulk_actions, only: [:create] - resources :agents, only: [:index, :create, :update, :destroy] + resources :agents, only: [:index, :create, :update, :destroy] do + post :bulk_create, on: :collection + end resources :agent_bots, only: [:index, :create, :show, :update, :destroy] do delete :avatar, on: :member end diff --git a/spec/builders/agent_builder_spec.rb b/spec/builders/agent_builder_spec.rb new file mode 100644 index 000000000..9d7667306 --- /dev/null +++ b/spec/builders/agent_builder_spec.rb @@ -0,0 +1,87 @@ +require 'rails_helper' + +RSpec.describe AgentBuilder, type: :model do + subject(:agent_builder) { described_class.new(params) } + + let(:account) { create(:account) } + let!(:current_user) { create(:user, account: account) } + let(:email) { 'test@example.com' } + let(:name) { 'Test User' } + let(:role) { 'agent' } + let(:availability) { 'offline' } + let(:auto_offline) { false } + let(:params) do + { + email: email, + name: name, + inviter: current_user, + account: account, + role: role, + availability: availability, + auto_offline: auto_offline + } + end + + describe '#perform' do + context 'when user does not exist' do + it 'creates a new user' do + expect { agent_builder.perform }.to change(User, :count).by(1) + end + + it 'creates a new account user' do + expect { agent_builder.perform }.to change(AccountUser, :count).by(1) + end + + it 'returns a user' do + expect(agent_builder.perform).to be_a(User) + end + end + + context 'when user exists' do + before do + create(:user, email: email) + end + + it 'does not create a new user' do + expect { agent_builder.perform }.not_to change(User, :count) + end + + it 'creates a new account user' do + expect { agent_builder.perform }.to change(AccountUser, :count).by(1) + end + end + + context 'when only email is provided' do + let(:params) { { email: email, inviter: current_user, account: account } } + + it 'creates a user with default values' do + user = agent_builder.perform + expect(user.name).to eq('') + expect(AccountUser.find_by(user: user).role).to eq('agent') + end + end + + context 'when a temporary password is generated' do + it 'sets a temporary password for the user' do + user = agent_builder.perform + expect(user.encrypted_password).not_to be_empty + end + end + + context 'with confirmation required' do + let(:unconfirmed_user) { create(:user, email: email) } + + before do + unconfirmed_user.confirmed_at = nil + unconfirmed_user.save(validate: false) + allow(unconfirmed_user).to receive(:confirmed?).and_return(false) + end + + it 'sends confirmation instructions' do + user = agent_builder.perform + expect(user).to receive(:send_confirmation_instructions) + agent_builder.send(:send_confirmation_if_required) + end + end + end +end diff --git a/spec/controllers/api/v1/accounts/agents_controller_spec.rb b/spec/controllers/api/v1/accounts/agents_controller_spec.rb index 2b06301d6..10693443a 100644 --- a/spec/controllers/api/v1/accounts/agents_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/agents_controller_spec.rb @@ -4,8 +4,8 @@ RSpec.describe 'Agents API', type: :request do include ActiveJob::TestHelper let(:account) { create(:account) } - let(:admin) { create(:user, custom_attributes: { test: 'test' }, account: account, role: :administrator) } - let(:agent) { create(:user, account: account, role: :agent) } + let!(:admin) { create(:user, custom_attributes: { test: 'test' }, account: account, role: :administrator) } + let!(:agent) { create(:user, account: account, role: :agent) } describe 'GET /api/v1/accounts/{account.id}/agents' do context 'when it is an unauthenticated user' do @@ -63,6 +63,8 @@ RSpec.describe 'Agents API', type: :request do end it 'deletes the agent and user object if associated with only one account' do + expect(account.users).to include(other_agent) + perform_enqueued_jobs(only: DeleteObjectJob) do delete "/api/v1/accounts/#{account.id}/agents/#{other_agent.id}", headers: admin.create_new_auth_token, @@ -70,8 +72,7 @@ RSpec.describe 'Agents API', type: :request do end expect(response).to have_http_status(:success) - expect(account.reload.users.size).to eq(1) - expect(User.count).to eq(account.reload.users.size) + expect(account.reload.users).not_to include(other_agent) end it 'deletes only the agent object when user is associated with multiple accounts' do @@ -85,8 +86,8 @@ RSpec.describe 'Agents API', type: :request do end expect(response).to have_http_status(:success) - expect(account.users.size).to eq(1) - expect(User.count).to eq(account.reload.users.size + 1) + expect(account.reload.users).not_to include(other_agent) + expect(other_agent.account_users.count).to eq(1) # Should only be associated with other_account now end end end @@ -174,4 +175,27 @@ RSpec.describe 'Agents API', type: :request do end end end + + describe 'POST /api/v1/accounts/{account.id}/agents/bulk_create' do + let(:emails) { ['test1@example.com', 'test2@example.com', 'test3@example.com'] } + let(:bulk_create_params) { { emails: emails } } + + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + post "/api/v1/accounts/#{account.id}/agents/bulk_create", params: bulk_create_params + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when authenticated as admin' do + it 'creates multiple agents successfully' do + expect do + post "/api/v1/accounts/#{account.id}/agents/bulk_create", params: bulk_create_params, headers: admin.create_new_auth_token + end.to change(User, :count).by(3) + + expect(response).to have_http_status(:ok) + end + end + end end diff --git a/spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb new file mode 100644 index 000000000..97e76a6cd --- /dev/null +++ b/spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +RSpec.describe 'Agents API', type: :request do + include ActiveJob::TestHelper + + let(:account) { create(:account) } + let!(:admin) { create(:user, custom_attributes: { test: 'test' }, account: account, role: :administrator) } + + describe 'POST /api/v1/accounts/{account.id}/agents' do + context 'when the account has reached its agent limit' do + params = { name: 'NewUser', email: Faker::Internet.email, role: :agent } + + before do + account.update(limits: { agents: 4 }) + create_list(:user, 4, account: account, role: :agent) + end + + it 'prevents adding a new agent and returns a payment required status' do + post "/api/v1/accounts/#{account.id}/agents", params: params, headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:payment_required) + expect(response.body).to include('Account limit exceeded. Please purchase more licenses') + end + end + end + + describe 'POST /api/v1/accounts/{account.id}/agents/bulk_create' do + let(:emails) { ['test1@example.com', 'test2@example.com', 'test3@example.com'] } + let(:bulk_create_params) { { emails: emails } } + + context 'when exceeding agent limit' do + it 'prevents creating agents and returns a payment required status' do + # Set the limit to be less than the number of emails + account.update(limits: { agents: 2 }) + + expect do + post "/api/v1/accounts/#{account.id}/agents/bulk_create", params: bulk_create_params, headers: admin.create_new_auth_token + end.not_to change(User, :count) + + expect(response).to have_http_status(:payment_required) + expect(response.body).to include('Account limit exceeded. Please purchase more licenses') + end + end + end +end