feat: allow bulk invite create via email (#8853)

* feat: add agent builder

* feat: use new agent builder

* refactor: validate limit

* test: agent limits

* feat: allow bulk create

* feat: allow bulk create

* refactor: rename current_user to inviter in AgentBuilder

* refactor: move limits tests to enterprise

* test: send correct params

* refactor: account builder returns both user and account_user

* chore: Revert "refactor: account builder returns both user and account_user"

This reverts commit 1419789871e8a3b8ff57af27fe53925b1486a839.

* feat: return user as is

* Update agent_builder.rb

- minor update

---------

Co-authored-by: Sojan Jose <sojan@pepalo.com>

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

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)

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

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

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

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

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