From a3ffb48a47aca9b1123fb502fe3c259bc3847256 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Mon, 1 Jun 2026 13:34:11 +0530 Subject: [PATCH] refactor(onboarding): use separate onboarding controller (#14507) Depends on: https://github.com/chatwoot/chatwoot/pull/14370 This PR creates a new onboarding controller, this allows more control that the default account update API. Allowing us to spin tasks and update details required specifically during the onboarding flow --- .../api/v1/accounts/onboardings_controller.rb | 36 ++++++ app/controllers/api/v1/accounts_controller.rb | 1 - app/javascript/dashboard/api/onboarding.js | 14 +++ .../dashboard/composables/useAccount.js | 5 + .../routes/dashboard/onboarding/Index.vue | 39 ++++-- .../dashboard/store/modules/accounts.js | 10 ++ config/routes.rb | 1 + .../accounts/onboardings_controller_spec.rb | 114 ++++++++++++++++++ .../api/v1/accounts_controller_spec.rb | 10 -- 9 files changed, 207 insertions(+), 23 deletions(-) create mode 100644 app/controllers/api/v1/accounts/onboardings_controller.rb create mode 100644 app/javascript/dashboard/api/onboarding.js create mode 100644 spec/controllers/api/v1/accounts/onboardings_controller_spec.rb diff --git a/app/controllers/api/v1/accounts/onboardings_controller.rb b/app/controllers/api/v1/accounts/onboardings_controller.rb new file mode 100644 index 000000000..77c6245ea --- /dev/null +++ b/app/controllers/api/v1/accounts/onboardings_controller.rb @@ -0,0 +1,36 @@ +class Api::V1::Accounts::OnboardingsController < Api::V1::Accounts::BaseController + before_action :check_admin_authorization? + + def update + @account = Current.account + finalize = finalizing_account_details? + + @account.assign_attributes(account_params) + @account.custom_attributes.merge!(custom_attributes_params) + @account.custom_attributes.delete('onboarding_step') if finalize + @account.save! + + # TODO: re-enable when the help center generation UI is ready to surface progress + # Onboarding::HelpCenterCreationService.new(@account, Current.user).perform if finalize && website.present? + + render 'api/v1/accounts/update', format: :json + end + + private + + def finalizing_account_details? + @account.custom_attributes['onboarding_step'] == 'account_details' + end + + def website + custom_attributes_params[:website] + end + + def account_params + params.permit(:name, :locale) + end + + def custom_attributes_params + params.permit(:industry, :company_size, :timezone, :referral_source, :user_role, :website) + end +end diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 865b387b9..fb991949a 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -58,7 +58,6 @@ class Api::V1::AccountsController < Api::BaseController @account.assign_attributes(account_params.slice(:name, :locale, :domain, :support_email)) @account.custom_attributes.merge!(custom_attributes_params) @account.settings.merge!(settings_params) - @account.custom_attributes.delete('onboarding_step') if @account.custom_attributes['onboarding_step'] == 'account_details' @account.custom_attributes['onboarding_step'] = 'invite_team' if @account.custom_attributes['onboarding_step'] == 'account_update' @account.save! end diff --git a/app/javascript/dashboard/api/onboarding.js b/app/javascript/dashboard/api/onboarding.js new file mode 100644 index 000000000..2bc098eef --- /dev/null +++ b/app/javascript/dashboard/api/onboarding.js @@ -0,0 +1,14 @@ +/* global axios */ +import ApiClient from './ApiClient'; + +class OnboardingAPI extends ApiClient { + constructor() { + super('onboarding', { accountScoped: true }); + } + + update(data) { + return axios.patch(this.url, data); + } +} + +export default new OnboardingAPI(); diff --git a/app/javascript/dashboard/composables/useAccount.js b/app/javascript/dashboard/composables/useAccount.js index 5a8441d33..c6149044f 100644 --- a/app/javascript/dashboard/composables/useAccount.js +++ b/app/javascript/dashboard/composables/useAccount.js @@ -52,6 +52,10 @@ export function useAccount() { }); }; + const finishOnboarding = async data => { + await store.dispatch('accounts/finishOnboarding', data); + }; + return { accountId, route, @@ -61,5 +65,6 @@ export function useAccount() { isCloudFeatureEnabled, isOnChatwootCloud, updateAccount, + finishOnboarding, }; } diff --git a/app/javascript/dashboard/routes/dashboard/onboarding/Index.vue b/app/javascript/dashboard/routes/dashboard/onboarding/Index.vue index 592672462..3a1d65bc6 100644 --- a/app/javascript/dashboard/routes/dashboard/onboarding/Index.vue +++ b/app/javascript/dashboard/routes/dashboard/onboarding/Index.vue @@ -28,7 +28,7 @@ import { const { t } = useI18n(); const router = useRouter(); const store = useStore(); -const { accountId, currentAccount, updateAccount } = useAccount(); +const { accountId, currentAccount, finishOnboarding } = useAccount(); const { enabledLanguages } = useConfig(); const currentUser = useMapGetter('getCurrentUser'); @@ -195,6 +195,12 @@ const handleWebsiteEnter = () => { websiteInput.value?.blur(); }; +const normalizeWebsiteUrl = raw => { + const trimmed = (raw || '').trim(); + if (!trimmed) return ''; + return /^https?:\/\//i.test(trimmed) ? trimmed : `https://${trimmed}`; +}; + const handleSubmit = async () => { // Block submit while enrichment is still running so users can't bypass // the form with empty values — the controller would otherwise clear @@ -211,9 +217,27 @@ const handleSubmit = async () => { return; } + // Detect which enrichable fields the user actually edited *before* + // normalizing — otherwise an untouched auto-filled domain + // (acme.com -> https://acme.com) compares unequal against the raw snapshot + // and gets falsely reported as changed, skewing onboarding telemetry. + const init = initialValues.value; + const enrichableFields = { + website: website.value, + company_size: companySize.value, + industry: industry.value, + }; + const fieldsChanged = Object.entries(enrichableFields) + .filter(([key, val]) => val !== init[key]) + .map(([key]) => key); + + // Persist with a scheme so downstream consumers (Firecrawl, portal + // homepage_link) get a fully-qualified URL regardless of what the user typed. + website.value = normalizeWebsiteUrl(website.value); + isSubmitting.value = true; try { - await updateAccount({ + await finishOnboarding({ name: accountName.value, locale: locale.value, website: website.value, @@ -224,20 +248,11 @@ const handleSubmit = async () => { user_role: userRole.value, }); - const init = initialValues.value; - const enrichableFields = { - website: website.value, - company_size: companySize.value, - industry: industry.value, - }; - useTrack(ONBOARDING_EVENTS.ACCOUNT_DETAILS_COMPLETED, { has_enriched_data: Boolean( currentAccount.value?.custom_attributes?.brand_info ), - fields_changed: Object.entries(enrichableFields) - .filter(([key, val]) => val !== init[key]) - .map(([key]) => key), + fields_changed: fieldsChanged, user_role: userRole.value, company_size: companySize.value, industry: industry.value, diff --git a/app/javascript/dashboard/store/modules/accounts.js b/app/javascript/dashboard/store/modules/accounts.js index 52e7b8213..68fd37010 100644 --- a/app/javascript/dashboard/store/modules/accounts.js +++ b/app/javascript/dashboard/store/modules/accounts.js @@ -1,6 +1,7 @@ import * as MutationHelpers from 'shared/helpers/vuex/mutationHelpers'; import * as types from '../mutation-types'; import AccountAPI from '../../api/account'; +import OnboardingAPI from '../../api/onboarding'; import { differenceInDays } from 'date-fns'; import EnterpriseAccountAPI from '../../api/enterprise/account'; import { throwErrorMessage } from '../utils/api'; @@ -83,6 +84,15 @@ export const actions = { throw new Error(error); } }, + finishOnboarding: async ({ commit }, payload) => { + commit(types.default.SET_ACCOUNT_UI_FLAG, { isUpdating: true }); + try { + const response = await OnboardingAPI.update(payload); + commit(types.default.EDIT_ACCOUNT, response.data); + } finally { + commit(types.default.SET_ACCOUNT_UI_FLAG, { isUpdating: false }); + } + }, delete: async ({ commit }, { id }) => { commit(types.default.SET_ACCOUNT_UI_FLAG, { isUpdating: true }); try { diff --git a/config/routes.rb b/config/routes.rb index 004a940a3..aeb7fe525 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,6 +54,7 @@ Rails.application.routes.draw do resource :contact_merge, only: [:create] end resource :bulk_actions, only: [:create] + resource :onboarding, only: [:update] resources :agents, only: [:index, :create, :update, :destroy] do post :bulk_create, on: :collection end diff --git a/spec/controllers/api/v1/accounts/onboardings_controller_spec.rb b/spec/controllers/api/v1/accounts/onboardings_controller_spec.rb new file mode 100644 index 000000000..ec2b4dcaa --- /dev/null +++ b/spec/controllers/api/v1/accounts/onboardings_controller_spec.rb @@ -0,0 +1,114 @@ +require 'rails_helper' + +RSpec.describe 'Onboarding API', type: :request do + let(:account) { create(:account, domain: 'example.com') } + let(:admin) { create(:user, account: account, role: :administrator) } + + describe 'PATCH /api/v1/accounts/{account.id}/onboarding' do + context 'when unauthenticated' do + it 'returns unauthorized' do + patch "/api/v1/accounts/#{account.id}/onboarding", params: { website: 'acme.com' }, as: :json + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when authenticated as an agent (non-admin)' do + let(:agent) { create(:user, account: account, role: :agent) } + + it 'returns unauthorized and does not change the account' do + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { name: 'Hijacked', website: 'attacker.com' }, + headers: agent.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unauthorized) + expect(account.reload.name).not_to eq('Hijacked') + end + + it 'does not create a help center portal' do + account.update!(custom_attributes: { 'onboarding_step' => 'account_details' }) + + expect do + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { website: 'attacker.com' }, + headers: agent.create_new_auth_token, as: :json + end.not_to change(account.portals, :count) + end + end + + context 'when finalizing account_details' do + before { account.update!(custom_attributes: { 'onboarding_step' => 'account_details' }) } + + it 'saves name and locale' do + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { name: 'Acme Inc', locale: 'fr' }, + headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:success) + expect(account.reload.name).to eq('Acme Inc') + expect(account.locale).to eq('fr') + end + + it 'merges custom_attributes' do + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { website: 'acme.com', industry: 'tech', company_size: '10-50' }, + headers: admin.create_new_auth_token, as: :json + + attrs = account.reload.custom_attributes + expect(attrs['website']).to eq('acme.com') + expect(attrs['industry']).to eq('tech') + expect(attrs['company_size']).to eq('10-50') + end + + it 'clears onboarding_step' do + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { website: 'acme.com' }, + headers: admin.create_new_auth_token, as: :json + + expect(account.reload.custom_attributes).not_to have_key('onboarding_step') + end + + it 'invokes HelpCenterCreationService when website is present', skip: 'help center generation wiring disabled until UI is ready' do + service = instance_double(Onboarding::HelpCenterCreationService, perform: nil) + allow(Onboarding::HelpCenterCreationService).to receive(:new).and_return(service) + + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { website: 'acme.com' }, + headers: admin.create_new_auth_token, as: :json + + expect(Onboarding::HelpCenterCreationService).to have_received(:new) do |arg_account, arg_user| + expect(arg_account.id).to eq(account.id) + expect(arg_user.id).to eq(admin.id) + end + expect(service).to have_received(:perform) + end + + it 'does not create a help center portal when website is blank' do + expect do + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { name: 'Acme Inc' }, + headers: admin.create_new_auth_token, as: :json + end.not_to change(account.portals, :count) + end + end + + context 'when onboarding_step is not account_details' do + before { account.update!(custom_attributes: { 'onboarding_step' => 'invite_team' }) } + + it 'does not clear onboarding_step' do + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { website: 'acme.com' }, + headers: admin.create_new_auth_token, as: :json + + expect(account.reload.custom_attributes['onboarding_step']).to eq('invite_team') + end + + it 'does not create a help center portal' do + expect do + patch "/api/v1/accounts/#{account.id}/onboarding", + params: { website: 'acme.com' }, + headers: admin.create_new_auth_token, as: :json + end.not_to change(account.portals, :count) + end + end + end +end diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 340803a9b..d93503418 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -302,16 +302,6 @@ RSpec.describe 'Accounts API', type: :request do expect(account.reload.custom_attributes['onboarding_step']).to eq('invite_team') end - it 'clears onboarding step when current value is account_details' do - account.update(custom_attributes: { onboarding_step: 'account_details' }) - patch "/api/v1/accounts/#{account.id}", - params: params, - headers: admin.create_new_auth_token, - as: :json - - expect(account.reload.custom_attributes).not_to have_key('onboarding_step') - end - it 'will not update onboarding step if onboarding step is not present in account custom attributes' do patch "/api/v1/accounts/#{account.id}", params: params,