From 657843960cf5656e93a3e4fcea7b954cbc398971 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Mon, 12 Feb 2024 23:21:42 +0530 Subject: [PATCH] feat: account onboarding with clearbit (#8857) * feat: add clearbit lookup * chore: fix typo in .env.example * refactor: split lookup to reduce cognitive complexity * feat: add more fields to lookup * feat: extend accounts controller * feat: save extra data to custom_attributes * feat: allow v2 update with custom_attributes * feat: add update route * refactor: reduce complexity * feat: move update to v1 controller * test: add locale test * feat: remove update from routes * test: update API for custom attributes * test: all custom attributes * fix: v2 tests * test: enterprise accounts controller * fix: clearbit payload * fix: with modified env * feat: allow custom attributes updates to profile * refactor: reduce complexity * feat: allow clearbit api key in installation config * refactor: move clearbit to internal * feat: allow clearbit * chore: add display_title for June * feat: allow more internal options * refactor: use globalconfig to fetch clearbit token * test: move response body to a factory * refactor: update ops * chore: remove clearbit from .env.example * chore: apply suggestions from code review Co-authored-by: sojan-official --------- Co-authored-by: sojan-official --- .env.example | 1 - app/controllers/api/v1/accounts_controller.rb | 8 +- app/controllers/api/v1/profiles_controller.rb | 8 +- app/controllers/api/v2/accounts_controller.rb | 8 ++ config/installation_config.yml | 24 +++-- .../enterprise/api/v2/accounts_controller.rb | 35 +++++++ .../super_admin/app_configs_controller.rb | 34 ++++--- .../enterprise/clearbit_lookup_service.rb | 92 +++++++++++++++++++ .../api/v1/accounts_controller_spec.rb | 9 +- .../api/v1/profiles_controller_spec.rb | 12 +++ .../api/v2/accounts_controller_spec.rb | 8 +- .../api/v2/accounts_controller_spec.rb | 72 +++++++++++++++ .../clearbit_lookup_service_spec.rb | 60 ++++++++++++ spec/factories/clearbit_response.rb | 29 ++++++ 14 files changed, 367 insertions(+), 33 deletions(-) create mode 100644 enterprise/app/controllers/enterprise/api/v2/accounts_controller.rb create mode 100644 enterprise/app/services/enterprise/clearbit_lookup_service.rb create mode 100644 spec/enterprise/controllers/enterprise/api/v2/accounts_controller_spec.rb create mode 100644 spec/enterprise/services/enterprise/clearbit_lookup_service_spec.rb create mode 100644 spec/factories/clearbit_response.rb diff --git a/.env.example b/.env.example index d76338e28..177cb7fc8 100644 --- a/.env.example +++ b/.env.example @@ -254,7 +254,6 @@ AZURE_APP_SECRET= # Sentiment analysis model file path SENTIMENT_FILE_PATH= - # Housekeeping/Performance related configurations # Set to true if you want to remove stale contact inboxes # contact_inboxes with no conversation older than 90 days will be removed diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 0e5615bae..481594ee4 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -44,7 +44,9 @@ class Api::V1::AccountsController < Api::BaseController end def update - @account.update!(account_params.slice(:name, :locale, :domain, :support_email, :auto_resolve_duration)) + @account.assign_attributes(account_params.slice(:name, :locale, :domain, :support_email, :auto_resolve_duration)) + @account.custom_attributes.merge!(custom_attributes_params) + @account.save! end def update_active_at @@ -83,6 +85,10 @@ class Api::V1::AccountsController < Api::BaseController params.permit(:account_name, :email, :name, :password, :locale, :domain, :support_email, :auto_resolve_duration, :user_full_name) end + def custom_attributes_params + params.permit(:industry, :company_size, :timezone) + end + def check_signup_enabled raise ActionController::RoutingError, 'Not Found' if GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') == 'false' end diff --git a/app/controllers/api/v1/profiles_controller.rb b/app/controllers/api/v1/profiles_controller.rb index a77652ff8..bd3c2673c 100644 --- a/app/controllers/api/v1/profiles_controller.rb +++ b/app/controllers/api/v1/profiles_controller.rb @@ -10,7 +10,9 @@ class Api::V1::ProfilesController < Api::BaseController @user.update!(password_params.except(:current_password)) end - @user.update!(profile_params) + @user.assign_attributes(profile_params) + @user.custom_attributes.merge!(custom_attributes_params) + @user.save! end def avatar @@ -62,6 +64,10 @@ class Api::V1::ProfilesController < Api::BaseController ) end + def custom_attributes_params + params.require(:profile).permit(:phone_number) + end + def password_params params.require(:profile).permit( :current_password, diff --git a/app/controllers/api/v2/accounts_controller.rb b/app/controllers/api/v2/accounts_controller.rb index e38863c07..ea19727ad 100644 --- a/app/controllers/api/v2/accounts_controller.rb +++ b/app/controllers/api/v2/accounts_controller.rb @@ -17,8 +17,12 @@ class Api::V2::AccountsController < Api::BaseController @user, @account = AccountBuilder.new( email: account_params[:email], user_password: account_params[:password], + locale: account_params[:locale], user: current_user ).perform + + fetch_account_and_user_info + if @user send_auth_headers(@user) render 'api/v1/accounts/create', format: :json, locals: { resource: @user } @@ -29,6 +33,8 @@ class Api::V2::AccountsController < Api::BaseController private + def fetch_account_and_user_info; end + def fetch_account @account = current_user.accounts.find(params[:id]) @current_account_user = @account.account_users.find_by(user_id: current_user.id) @@ -46,3 +52,5 @@ class Api::V2::AccountsController < Api::BaseController raise ActionController::InvalidAuthenticityToken, 'Invalid Captcha' unless ChatwootCaptcha.new(params[:h_captcha_client_response]).valid? end end + +Api::V2::AccountsController.prepend_mod_with('Api::V2::AccountsController') diff --git a/config/installation_config.yml b/config/installation_config.yml index 2643461e2..60eae3bc2 100644 --- a/config/installation_config.yml +++ b/config/installation_config.yml @@ -7,13 +7,11 @@ # value: the value of the config # display_title: the title of the config displayed in the dashboard UI # description: the description of the config displayed in the dashboard UI -# locked: if you don't specify locked attribute in yaml, the default value will be true, +# locked: if you don't specify locked attribute in yaml, the default value will be true, # which means the particular config will be locked and won't be available in `super_admin/installation_configs` # premium: These values get overwritten unless the user is on a premium plan # type: The type of the config. Default is text, boolean is also supported - - # ------- Branding Related Config ------- # - name: INSTALLATION_NAME value: 'Chatwoot' @@ -58,8 +56,6 @@ type: boolean # ------- End of Branding Related Config ------- # - - # ------- Signup & Account Related Config ------- # - name: ENABLE_ACCOUNT_SIGNUP display_title: 'Enable Account Signup' @@ -89,8 +85,6 @@ locked: false # ------- End of Account Related Config ------- # - - # ------- Email Related Config ------- # - name: MAILER_INBOUND_EMAIL_DOMAIN value: @@ -101,12 +95,11 @@ locked: false # ------- End of Email Related Config ------- # - # ------- Facebook Channel Related Config ------- # -- name: FB_APP_ID +- name: FB_APP_ID display_title: 'Facebook App ID' locked: false -- name: FB_VERIFY_TOKEN +- name: FB_VERIFY_TOKEN display_title: 'Facebook Verify Token' description: 'The verify token used for Facebook Messenger Webhook' locked: false @@ -128,10 +121,12 @@ # ------- Chatwoot Internal Config for Cloud ----# - name: CHATWOOT_INBOX_TOKEN value: + display_title: 'Inbox Token' description: 'The Chatwoot Inbox Token for Contact Support in Cloud' locked: false - name: CHATWOOT_INBOX_HMAC_KEY value: + display_title: 'Inbox HMAC Key' description: 'The Chatwoot Inbox HMAC Key for Contact Support in Cloud' locked: false - name: CHATWOOT_CLOUD_PLANS @@ -142,10 +137,14 @@ description: 'The deployment environment of the installation, to differentiate between Chatwoot cloud and self-hosted' - name: ANALYTICS_TOKEN value: + display_title: 'Analytics Token' description: 'The June.so analytics token for Chatwoot cloud' +- name: CLEARBIT_API_KEY + value: + display_title: 'Clearbit API Key' + description: 'This API key is used for onboarding the users, to pre-fill account data.' # ------- End of Chatwoot Internal Config for Cloud ----# - # ------- Chatwoot Internal Config for Self Hosted ----# - name: INSTALLATION_PRICING_PLAN value: 'community' @@ -179,5 +178,4 @@ value: false locked: false description: 'Disable rendering profile update page for users' - -## ------ End of Configs added for enterprise clients ------ ## \ No newline at end of file +## ------ End of Configs added for enterprise clients ------ ## diff --git a/enterprise/app/controllers/enterprise/api/v2/accounts_controller.rb b/enterprise/app/controllers/enterprise/api/v2/accounts_controller.rb new file mode 100644 index 000000000..d0d76c1ae --- /dev/null +++ b/enterprise/app/controllers/enterprise/api/v2/accounts_controller.rb @@ -0,0 +1,35 @@ +module Enterprise::Api::V2::AccountsController + private + + def fetch_account_and_user_info + data = fetch_from_clearbit + + return if data.blank? + + update_user_info(data) + update_account_info(data) + end + + def fetch_from_clearbit + Enterprise::ClearbitLookupService.lookup(@user.email) + rescue StandardError => e + Rails.logger.error "Error fetching data from clearbit: #{e}" + nil + end + + def update_user_info(data) + @user.update!(name: data[:name]) + end + + def update_account_info(data) + @account.update!( + name: data[:company_name], + custom_attributes: @account.custom_attributes.merge( + 'industry' => data[:industry], + 'company_size' => data[:company_size], + 'timezone' => data[:timezone], + 'logo' => data[:logo] + ) + ) + end +end diff --git a/enterprise/app/controllers/enterprise/super_admin/app_configs_controller.rb b/enterprise/app/controllers/enterprise/super_admin/app_configs_controller.rb index 106dbb4a8..ae1394a24 100644 --- a/enterprise/app/controllers/enterprise/super_admin/app_configs_controller.rb +++ b/enterprise/app/controllers/enterprise/super_admin/app_configs_controller.rb @@ -6,20 +6,30 @@ module Enterprise::SuperAdmin::AppConfigsController case @config when 'custom_branding' - @allowed_configs = %w[ - LOGO_THUMBNAIL - LOGO - LOGO_DARK - BRAND_NAME - INSTALLATION_NAME - BRAND_URL - WIDGET_BRAND_URL - TERMS_URL - PRIVACY_URL - DISPLAY_MANIFEST - ] + @allowed_configs = custom_branding_options + when 'internal' + @allowed_configs = internal_config_options else super end end + + def custom_branding_options + %w[ + LOGO_THUMBNAIL + LOGO + LOGO_DARK + BRAND_NAME + INSTALLATION_NAME + BRAND_URL + WIDGET_BRAND_URL + TERMS_URL + PRIVACY_URL + DISPLAY_MANIFEST + ] + end + + def internal_config_options + %w[CHATWOOT_INBOX_TOKEN CHATWOOT_INBOX_HMAC_KEY ANALYTICS_TOKEN CLEARBIT_API_KEY] + end end diff --git a/enterprise/app/services/enterprise/clearbit_lookup_service.rb b/enterprise/app/services/enterprise/clearbit_lookup_service.rb new file mode 100644 index 000000000..c108ca153 --- /dev/null +++ b/enterprise/app/services/enterprise/clearbit_lookup_service.rb @@ -0,0 +1,92 @@ +# The Enterprise::ClearbitLookupService class is responsible for interacting with the Clearbit API. +# It provides methods to lookup a person's information using their email. +# Clearbit API documentation: {https://dashboard.clearbit.com/docs?ruby#api-reference} +# We use the combined API which returns both the person and comapnies together +# Combined API: {https://dashboard.clearbit.com/docs?ruby=#enrichment-api-combined-api} +# Persons API: {https://dashboard.clearbit.com/docs?ruby=#enrichment-api-person-api} +# Companies API: {https://dashboard.clearbit.com/docs?ruby=#enrichment-api-company-api} +# +# Note: The Clearbit gem is not used in this service, since it is not longer maintained +# GitHub: {https://github.com/clearbit/clearbit-ruby} +# +# @example +# Enterprise::ClearbitLookupService.lookup('test@example.com') +class Enterprise::ClearbitLookupService + # Clearbit API endpoint for combined lookup + CLEARBIT_ENDPOINT = 'https://person.clearbit.com/v2/combined/find'.freeze + + # Performs a lookup on the Clearbit API using the provided email. + # + # @param email [String] The email address to lookup. + # @return [Hash, nil] A hash containing the person's full name, company name, and company timezone, or nil if an error occurs. + def self.lookup(email) + return nil unless clearbit_enabled? + + response = perform_request(email) + process_response(response) + rescue StandardError => e + Rails.logger.error "[ClearbitLookup] #{e.message}" + nil + end + + # Performs a request to the Clearbit API using the provided email. + # + # @param email [String] The email address to lookup. + # @return [HTTParty::Response] The response from the Clearbit API. + def self.perform_request(email) + options = { + headers: { 'Authorization' => "Bearer #{clearbit_token}" }, + query: { email: email } + } + + HTTParty.get(CLEARBIT_ENDPOINT, options) + end + + # Handles an error response from the Clearbit API. + # + # @param response [HTTParty::Response] The response from the Clearbit API. + # @return [nil] Always returns nil. + def self.handle_error(response) + Rails.logger.error "[ClearbitLookup] API Error: #{response.message} (Status: #{response.code})" + nil + end + + # Checks if Clearbit is enabled by checking for the presence of the CLEARBIT_API_KEY environment variable. + # + # @return [Boolean] True if Clearbit is enabled, false otherwise. + def self.clearbit_enabled? + clearbit_token.present? + end + + def self.clearbit_token + GlobalConfigService.load('CLEARBIT_API_KEY', '') + end + + # Processes the response from the Clearbit API. + # + # @param response [HTTParty::Response] The response from the Clearbit API. + # @return [Hash, nil] A hash containing the person's full name, company name, and company timezone, or nil if an error occurs. + def self.process_response(response) + return handle_error(response) unless response.success? + + format_response(response) + end + + # Formats the response data from the Clearbit API. + # + # @param data [Hash] The raw data from the Clearbit API. + # @return [Hash] A hash containing the person's full name, company name, and company timezone. + def self.format_response(response) + data = response.parsed_response + + { + name: data.dig('person', 'name', 'fullName'), + avatar: data.dig('person', 'avatar'), + company_name: data.dig('company', 'name'), + timezone: data.dig('company', 'timeZone'), + logo: data.dig('company', 'logo'), + industry: data.dig('company', 'category', 'industry'), + company_size: data.dig('company', 'metrics', 'employees') + } + end +end diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 060e241a7..95fa2ae7f 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -189,7 +189,10 @@ RSpec.describe 'Accounts API', type: :request do locale: 'en', domain: 'example.com', support_email: 'care@example.com', - auto_resolve_duration: 40 + auto_resolve_duration: 40, + timezone: 'Asia/Kolkata', + industry: 'Technology', + company_size: '1-10' } it 'modifies an account' do @@ -204,6 +207,10 @@ RSpec.describe 'Accounts API', type: :request do expect(account.reload.domain).to eq(params[:domain]) expect(account.reload.support_email).to eq(params[:support_email]) expect(account.reload.auto_resolve_duration).to eq(params[:auto_resolve_duration]) + + %w[timezone industry company_size].each do |attribute| + expect(account.reload.custom_attributes[attribute]).to eq(params[attribute.to_sym]) + end end it 'Throws error 422' do diff --git a/spec/controllers/api/v1/profiles_controller_spec.rb b/spec/controllers/api/v1/profiles_controller_spec.rb index 80f6833f1..28b4c5ed5 100644 --- a/spec/controllers/api/v1/profiles_controller_spec.rb +++ b/spec/controllers/api/v1/profiles_controller_spec.rb @@ -57,6 +57,18 @@ RSpec.describe 'Profile API', type: :request do expect(agent.name).to eq('test') end + it 'updates custom attributes' do + put '/api/v1/profile', + params: { profile: { phone_number: '+123456789' } }, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + agent.reload + + expect(agent.custom_attributes['phone_number']).to eq('+123456789') + end + it 'updates the message_signature' do put '/api/v1/profile', params: { profile: { name: 'test', message_signature: 'Thanks\nMy Signature' } }, diff --git a/spec/controllers/api/v2/accounts_controller_spec.rb b/spec/controllers/api/v2/accounts_controller_spec.rb index e0181a282..82693ed2b 100644 --- a/spec/controllers/api/v2/accounts_controller_spec.rb +++ b/spec/controllers/api/v2/accounts_controller_spec.rb @@ -17,7 +17,7 @@ RSpec.describe 'Accounts API', type: :request 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!' } + params = { email: email, user: nil, locale: nil, password: 'Password1!' } post api_v2_accounts_url, params: params, @@ -37,7 +37,7 @@ RSpec.describe 'Accounts API', type: :request do 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' } + params = { email: email, user: nil, password: 'Password1!', locale: nil, h_captcha_client_response: '123' } post api_v2_accounts_url, params: params, @@ -53,7 +53,7 @@ RSpec.describe 'Accounts API', type: :request do with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do allow(account_builder).to receive(:perform).and_return(nil) - params = { email: nil, user: nil } + params = { email: nil, user: nil, locale: nil } post api_v2_accounts_url, params: params, @@ -89,7 +89,7 @@ RSpec.describe 'Accounts API', type: :request 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!' } + params = { email: email, user: nil, password: 'Password1!', locale: nil } with_modified_env ENABLE_ACCOUNT_SIGNUP: 'api_only' do post api_v2_accounts_url, params: params, diff --git a/spec/enterprise/controllers/enterprise/api/v2/accounts_controller_spec.rb b/spec/enterprise/controllers/enterprise/api/v2/accounts_controller_spec.rb new file mode 100644 index 000000000..8ad75b42f --- /dev/null +++ b/spec/enterprise/controllers/enterprise/api/v2/accounts_controller_spec.rb @@ -0,0 +1,72 @@ +require 'rails_helper' + +RSpec.describe Enterprise::Api::V2::AccountsController, type: :request do + let(:email) { Faker::Internet.email } + let(:user) { create(:user) } + let(:account) { create(:account) } + let(:clearbit_data) do + { + name: 'John Doe', + company_name: 'Acme Inc', + industry: 'Software', + company_size: '51-200', + timezone: 'America/Los_Angeles', + logo: 'https://logo.clearbit.com/acme.com' + } + end + + before do + allow(Enterprise::ClearbitLookupService).to receive(:lookup).and_return(clearbit_data) + end + + describe 'POST /api/v1/accounts' 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 'fetches data from clearbit and updates user and account info' do + with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do + allow(account_builder).to receive(:perform).and_return([user, account]) + + params = { email: email, user: nil, locale: 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(Enterprise::ClearbitLookupService).to have_received(:lookup).with(email) + + custom_attributes = account.custom_attributes + + expect(account.name).to eq('Acme Inc') + expect(custom_attributes['industry']).to eq('Software') + expect(custom_attributes['company_size']).to eq('51-200') + expect(custom_attributes['timezone']).to eq('America/Los_Angeles') + end + end + + it 'handles errors when fetching data from clearbit' do + with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do + allow(account_builder).to receive(:perform).and_return([user, account]) + allow(Enterprise::ClearbitLookupService).to receive(:lookup).and_raise(StandardError) + params = { email: email, user: nil, locale: 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(Enterprise::ClearbitLookupService).to have_received(:lookup).with(email) + + expect(response).to have_http_status(:success) + end + end + end +end diff --git a/spec/enterprise/services/enterprise/clearbit_lookup_service_spec.rb b/spec/enterprise/services/enterprise/clearbit_lookup_service_spec.rb new file mode 100644 index 000000000..725aa6523 --- /dev/null +++ b/spec/enterprise/services/enterprise/clearbit_lookup_service_spec.rb @@ -0,0 +1,60 @@ +require 'rails_helper' + +RSpec.describe Enterprise::ClearbitLookupService do + describe '.lookup' do + let(:email) { 'test@example.com' } + let(:api_key) { 'clearbit_api_key' } + let(:clearbit_endpoint) { described_class::CLEARBIT_ENDPOINT } + let(:response_body) { build(:clearbit_combined_response) } + + context 'when Clearbit is enabled' do + before do + stub_request(:get, "#{clearbit_endpoint}?email=#{email}") + .with(headers: { 'Authorization' => "Bearer #{api_key}" }) + .to_return(status: 200, body: response_body, headers: { 'content-type' => ['application/json'] }) + end + + context 'when the API is working as expected' do + it 'returns the person and company information' do + with_modified_env CLEARBIT_API_KEY: api_key do + result = described_class.lookup(email) + + expect(result).to eq({ + :avatar => 'https://example.com/avatar.png', + :company_name => 'Doe Inc.', + :company_size => '1-10', + :industry => 'Software', + :logo => nil, + :name => 'John Doe', + :timezone => 'Asia/Kolkata' + }) + end + end + end + + context 'when the API returns an error' do + before do + stub_request(:get, "#{clearbit_endpoint}?email=#{email}") + .with(headers: { 'Authorization' => "Bearer #{api_key}" }) + .to_return(status: 404, body: '', headers: {}) + end + + it 'logs the error and returns nil' do + with_modified_env CLEARBIT_API_KEY: api_key do + expect(Rails.logger).to receive(:error) + expect(described_class.lookup(email)).to be_nil + end + end + end + end + + context 'when Clearbit is not enabled' do + it 'returns nil without making an API call' do + with_modified_env CLEARBIT_API_KEY: nil do + expect(Net::HTTP).not_to receive(:start) + expect(described_class.lookup(email)).to be_nil + end + end + end + end +end diff --git a/spec/factories/clearbit_response.rb b/spec/factories/clearbit_response.rb new file mode 100644 index 000000000..fbe8e9a5a --- /dev/null +++ b/spec/factories/clearbit_response.rb @@ -0,0 +1,29 @@ +# spec/factories/response_bodies.rb +FactoryBot.define do + factory :clearbit_combined_response, class: Hash do + skip_create + + initialize_with do + { + 'person' => { + 'name' => { + 'fullName' => 'John Doe' + }, + 'avatar' => 'https://example.com/avatar.png' + }, + 'company' => { + 'name' => 'Doe Inc.', + 'timeZone' => 'Asia/Kolkata', + 'category' => { + 'sector' => 'Technology', + 'industryGroup' => 'Software', + 'industry' => 'Software' + }, + 'metrics' => { + 'employees' => '1-10' + } + } + }.to_json + end + end +end