feat: new accounts controller for signup+onboarding (#8804)

* feat: add v2 accounts controller

* feat: allow empty account and user name

* feat: ensure  and  is present for v1 signup

* test: remove validation checks

* chore: apply suggestions

* chore: revert en.yml formatting

* chore: line at EOF

* fix: routes

---------

Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>

app/builders/account_builder.rb

@@ -2,7 +2,7 @@
 
 class AccountBuilder
   include CustomExceptions::Account
-  pattr_initialize [:account_name!, :email!, :confirmed, :user, :user_full_name, :user_password, :super_admin, :locale]
+  pattr_initialize [:account_name, :email!, :confirmed, :user, :user_full_name, :user_password, :super_admin, :locale]
 
   def perform
     if @user.nil?
@@ -21,6 +21,16 @@ class AccountBuilder
 
   private
 
+  def user_full_name
+    # the empty string ensures that not-null constraint is not violated
+    @user_full_name || ''
+  end
+
+  def account_name
+    # the empty string ensures that not-null constraint is not violated
+    @account_name || ''
+  end
+
   def validate_email
     address = ValidEmail2::Address.new(@email)
     if address.valid? # && !address.disposable?
@@ -39,7 +49,7 @@ class AccountBuilder
   end
 
   def create_account
-    @account = Account.create!(name: @account_name, locale: I18n.locale)
+    @account = Account.create!(name: account_name, locale: I18n.locale)
     Current.account = @account
   end
 
@@ -64,7 +74,7 @@ class AccountBuilder
     @user = User.new(email: @email,
                      password: user_password,
                      password_confirmation: user_password,
-                     name: @user_full_name)
+                     name: user_full_name)
     @user.type = 'SuperAdmin' if @super_admin
     @user.confirm if @confirmed
     @user.save!

app/controllers/api/v1/accounts_controller.rb

@@ -5,11 +5,13 @@ class Api::V1::AccountsController < Api::BaseController
   skip_before_action :authenticate_user!, :set_current_user, :handle_with_exception,
                      only: [:create], raise: false
   before_action :check_signup_enabled, only: [:create]
+  before_action :ensure_account_name, only: [:create]
   before_action :validate_captcha, only: [:create]
   before_action :fetch_account, except: [:create]
   before_action :check_authorization, except: [:create]
 
   rescue_from CustomExceptions::Account::InvalidEmail,
+              CustomExceptions::Account::InvalidParams,
               CustomExceptions::Account::UserExists,
               CustomExceptions::Account::UserErrors,
               with: :render_error_response
@@ -53,6 +55,17 @@ class Api::V1::AccountsController < Api::BaseController
 
   private
 
+  def ensure_account_name
+    # ensure that account_name and user_full_name is present
+    # this is becuase the account builder and the models validations are not triggered
+    # this change is to align the behaviour with the v2 accounts controller
+    # since these values are not required directly there
+    return if account_params[:account_name].present?
+    return if account_params[:user_full_name].present?
+
+    raise CustomExceptions::Account::InvalidParams.new({})
+  end
+
   def get_cache_keys
     {
       label: fetch_value_for_key(params[:id], Label.name.underscore),

app/controllers/api/v2/accounts_controller.rb

@@ -0,0 +1,48 @@
+class Api::V2::AccountsController < Api::BaseController
+  include AuthHelper
+
+  skip_before_action :authenticate_user!, :set_current_user, :handle_with_exception,
+                     only: [:create], raise: false
+  before_action :check_signup_enabled, only: [:create]
+  before_action :validate_captcha, only: [:create]
+  before_action :fetch_account, except: [:create]
+  before_action :check_authorization, except: [:create]
+
+  rescue_from CustomExceptions::Account::InvalidEmail,
+              CustomExceptions::Account::UserExists,
+              CustomExceptions::Account::UserErrors,
+              with: :render_error_response
+
+  def create
+    @user, @account = AccountBuilder.new(
+      email: account_params[:email],
+      user_password: account_params[:password],
+      user: current_user
+    ).perform
+    if @user
+      send_auth_headers(@user)
+      render 'api/v1/accounts/create', format: :json, locals: { resource: @user }
+    else
+      render_error_response(CustomExceptions::Account::SignupFailed.new({}))
+    end
+  end
+
+  private
+
+  def fetch_account
+    @account = current_user.accounts.find(params[:id])
+    @current_account_user = @account.account_users.find_by(user_id: current_user.id)
+  end
+
+  def account_params
+    params.permit(:account_name, :email, :name, :password, :locale, :domain, :support_email, :auto_resolve_duration, :user_full_name)
+  end
+
+  def check_signup_enabled
+    raise ActionController::RoutingError, 'Not Found' if GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') == 'false'
+  end
+
+  def validate_captcha
+    raise ActionController::InvalidAuthenticityToken, 'Invalid Captcha' unless ChatwootCaptcha.new(params[:h_captcha_client_response]).valid?
+  end
+end

app/models/account.rb

@@ -32,7 +32,6 @@ class Account < ApplicationRecord
     check_for_column: false
   }.freeze
 
-  validates :name, presence: true
   validates :auto_resolve_duration, numericality: { greater_than_or_equal_to: 1, less_than_or_equal_to: 999, allow_nil: true }
   validates :domain, length: { maximum: 100 }
 

app/models/user.rb

@@ -68,8 +68,7 @@ class User < ApplicationRecord
   # work because :validatable in devise overrides this.
   # validates_uniqueness_of :email, scope: :account_id
 
-  validates :email, :name, presence: true
-  validates_length_of :name, minimum: 1, maximum: 255
+  validates :email, presence: true
 
   has_many :account_users, dependent: :destroy_async
   has_many :accounts, through: :account_users

config/locales/en.yml

@@ -45,6 +45,7 @@ en:
       disposable_email: We do not allow disposable emails
       invalid_email: You have entered an invalid email
       email_already_exists: "You have already signed up for an account with %{email}"
+      invalid_params: 'Invalid, please check the signup paramters and try again'
       failed: Signup failed
     data_import:
       data_type:

config/routes.rb

@@ -288,16 +288,18 @@ Rails.application.routes.draw do
     end
 
     namespace :v2 do
-      resources :accounts, only: [], module: :accounts do
-        resources :reports, only: [:index] do
-          collection do
-            get :summary
-            get :agents
-            get :inboxes
-            get :labels
-            get :teams
-            get :conversations
-            get :conversation_traffic
+      resources :accounts, only: [:create] do
+        scope module: :accounts do
+          resources :reports, only: [:index] do
+            collection do
+              get :summary
+              get :agents
+              get :inboxes
+              get :labels
+              get :teams
+              get :conversations
+              get :conversation_traffic
+            end
           end
         end
       end

lib/custom_exceptions/account.rb

@@ -17,6 +17,12 @@ module CustomExceptions::Account
     end
   end
 
+  class InvalidParams < CustomExceptions::Base
+    def message
+      I18n.t 'errors.signup.invalid_params'
+    end
+  end
+
   class UserErrors < CustomExceptions::Base
     def message
       @data[:errors].full_messages.join(',')

spec/controllers/api/v1/accounts_controller_spec.rb

@@ -61,10 +61,9 @@ RSpec.describe 'Accounts API', type: :request do
                params: params,
                as: :json
 
-          expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password]))
-          expect(account_builder).to have_received(:perform)
+          expect(AccountBuilder).not_to have_received(:new)
           expect(response).to have_http_status(:forbidden)
-          expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json)
+          expect(response.body).to eq({ message: I18n.t('errors.signup.invalid_params') }.to_json)
         end
       end
     end

spec/controllers/api/v2/accounts_controller_spec.rb

@@ -0,0 +1,104 @@
+require 'rails_helper'
+
+RSpec.describe 'Accounts API', type: :request do
+  describe 'POST /api/v2/accounts' do
+    let(:email) { Faker::Internet.email }
+
+    context 'when posting to accounts with correct parameters' do
+      let(:account_builder) { double }
+      let(:account) { create(:account) }
+      let(:user) { create(:user, email: email, account: account) }
+
+      before do
+        allow(AccountBuilder).to receive(:new).and_return(account_builder)
+      end
+
+      it 'calls account builder' do
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do
+          allow(account_builder).to receive(:perform).and_return([user, account])
+
+          params = { email: email, user: nil, password: 'Password1!' }
+
+          post api_v2_accounts_url,
+               params: params,
+               as: :json
+
+          expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password]))
+          expect(account_builder).to have_received(:perform)
+          expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid')
+          expect(response.body).to include('en')
+        end
+      end
+
+      it 'calls ChatwootCaptcha' do
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do
+          captcha = double
+          allow(account_builder).to receive(:perform).and_return([user, account])
+          allow(ChatwootCaptcha).to receive(:new).and_return(captcha)
+          allow(captcha).to receive(:valid?).and_return(true)
+
+          params = { email: email, user: nil, password: 'Password1!', h_captcha_client_response: '123' }
+
+          post api_v2_accounts_url,
+               params: params,
+               as: :json
+
+          expect(ChatwootCaptcha).to have_received(:new).with('123')
+          expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid')
+          expect(response.body).to include('en')
+        end
+      end
+
+      it 'renders error response on invalid params' do
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do
+          allow(account_builder).to receive(:perform).and_return(nil)
+
+          params = { email: nil, user: nil }
+
+          post api_v2_accounts_url,
+               params: params,
+               as: :json
+
+          expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password]))
+          expect(account_builder).to have_received(:perform)
+          expect(response).to have_http_status(:forbidden)
+          expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json)
+        end
+      end
+    end
+
+    context 'when ENABLE_ACCOUNT_SIGNUP env variable is set to false' do
+      it 'responds 404 on requests' do
+        params = { email: email }
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false' do
+          post api_v2_accounts_url,
+               params: params,
+               as: :json
+
+          expect(response).to have_http_status(:not_found)
+        end
+      end
+    end
+
+    context 'when ENABLE_ACCOUNT_SIGNUP env variable is set to api_only' do
+      let(:account_builder) { double }
+      let(:account) { create(:account) }
+      let(:user) { create(:user, email: email, account: account) }
+
+      it 'does not respond 404 on requests' do
+        allow(AccountBuilder).to receive(:new).and_return(account_builder)
+        allow(account_builder).to receive(:perform).and_return([user, account])
+
+        params = { email: email, user: nil, password: 'Password1!' }
+        with_modified_env ENABLE_ACCOUNT_SIGNUP: 'api_only' do
+          post api_v2_accounts_url,
+               params: params,
+               as: :json
+
+          expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password]))
+          expect(response).to have_http_status(:success)
+        end
+      end
+    end
+  end
+end

spec/models/account_spec.rb

@@ -3,7 +3,6 @@
 require 'rails_helper'
 
 RSpec.describe Account do
-  it { is_expected.to validate_presence_of(:name) }
   it { is_expected.to validate_numericality_of(:auto_resolve_duration).is_greater_than_or_equal_to(1) }
   it { is_expected.to validate_numericality_of(:auto_resolve_duration).is_less_than_or_equal_to(999) }
 

spec/models/user_spec.rb

@@ -9,8 +9,6 @@ RSpec.describe User do
 
   context 'validations' do
     it { is_expected.to validate_presence_of(:email) }
-    it { is_expected.to validate_presence_of(:name) }
-    it { is_expected.to validate_length_of(:name).is_at_least(1) }
   end
 
   context 'associations' do