From 713fdb44ee09e7f16ffb83d71c5faf7036d6d807 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Mon, 13 Jun 2022 20:18:38 +0530 Subject: [PATCH] feat (ee): APIs to configure an auto assignment limit for inboxes (#4672) Co-authored-by: Pranav Raj S --- .../api/v1/accounts/inboxes_controller.rb | 30 +++------- app/controllers/application_controller.rb | 2 +- app/controllers/dashboard_controller.rb | 3 +- app/helpers/api/v1/inboxes_helper.rb | 18 ++++++ .../dashboard/i18n/locale/en/inboxMgmt.json | 5 ++ .../inbox/settingsPage/CollaboratorsPage.vue | 56 ++++++++++++++++++- app/javascript/shared/mixins/configMixin.js | 3 + app/models/account.rb | 3 +- app/models/concerns/round_robin_handler.rb | 2 +- app/models/inbox.rb | 12 ++++ app/services/round_robin/manage_service.rb | 33 ++++++----- app/views/api/v1/models/_inbox.json.jbuilder | 1 + app/views/layouts/vueapp.html.erb | 1 + ...uto_assignment_configuration_to_inboxes.rb | 5 ++ db/schema.rb | 1 + .../api/v1/accounts/inboxes_controller.rb | 9 +++ enterprise/app/models/enterprise/inbox.rb | 20 +++++++ .../v1/accounts/inboxes_controller_spec.rb | 46 +++++++++++++++ spec/enterprise/models/inbox_spec.rb | 38 +++++++++++++ .../round_robin/manage_service_spec.rb | 12 ++-- 20 files changed, 255 insertions(+), 45 deletions(-) create mode 100644 db/migrate/20220527040433_add_auto_assignment_configuration_to_inboxes.rb create mode 100644 enterprise/app/controllers/enterprise/api/v1/accounts/inboxes_controller.rb create mode 100644 enterprise/app/models/enterprise/inbox.rb create mode 100644 spec/enterprise/controllers/api/v1/accounts/inboxes_controller_spec.rb create mode 100644 spec/enterprise/models/inbox_spec.rb diff --git a/app/controllers/api/v1/accounts/inboxes_controller.rb b/app/controllers/api/v1/accounts/inboxes_controller.rb index 4bc85546a..ded4db598 100644 --- a/app/controllers/api/v1/accounts/inboxes_controller.rb +++ b/app/controllers/api/v1/accounts/inboxes_controller.rb @@ -42,7 +42,7 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController end def update - @inbox.update(permitted_params.except(:channel)) + @inbox.update!(permitted_params.except(:channel)) @inbox.update_working_hours(params.permit(working_hours: Inbox::OFFISABLE_ATTRS)[:working_hours]) if params[:working_hours] channel_attributes = get_channel_attributes(@inbox.channel_type) @@ -109,10 +109,14 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController @inbox.channel.save! end + def inbox_attributes + [:name, :avatar, :greeting_enabled, :greeting_message, :enable_email_collect, :csat_survey_enabled, + :enable_auto_assignment, :working_hours_enabled, :out_of_office_message, :timezone, :allow_messages_after_resolved] + end + def permitted_params(channel_attributes = []) params.permit( - :name, :avatar, :greeting_enabled, :greeting_message, :enable_email_collect, :csat_survey_enabled, - :enable_auto_assignment, :working_hours_enabled, :out_of_office_message, :timezone, :allow_messages_after_resolved, + *inbox_attributes, channel: [:type, *channel_attributes] ) end @@ -129,18 +133,6 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController }[permitted_params[:channel][:type]] end - def account_channels_method - { - 'web_widget' => Current.account.web_widgets, - 'api' => Current.account.api_channels, - 'email' => Current.account.email_channels, - 'line' => Current.account.line_channels, - 'telegram' => Current.account.telegram_channels, - 'whatsapp' => Current.account.whatsapp_channels, - 'sms' => Current.account.sms_channels - }[permitted_params[:channel][:type]] - end - def get_channel_attributes(channel_type) if channel_type.constantize.const_defined?(:EDITABLE_ATTRS) channel_type.constantize::EDITABLE_ATTRS.presence @@ -148,10 +140,6 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController [] end end - - def validate_limit - return unless Current.account.inboxes.count >= Current.account.usage_limits[:inboxes] - - render_payment_required('Account limit exceeded. Upgrade to a higher plan') - end end + +Api::V1::Accounts::InboxesController.prepend_mod_with('Api::V1::Accounts::InboxesController') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 0a1c672ec..0762c3d91 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,7 +1,7 @@ class ApplicationController < ActionController::Base include DeviseTokenAuth::Concerns::SetUserByToken include RequestExceptionHandler - include Pundit + include Pundit::Authorization include SwitchLocale skip_before_action :verify_authenticity_token diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 243fe00b2..36c789b52 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -43,7 +43,8 @@ class DashboardController < ActionController::Base VAPID_PUBLIC_KEY: VapidService.public_key, ENABLE_ACCOUNT_SIGNUP: GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false'), FB_APP_ID: GlobalConfigService.load('FB_APP_ID', ''), - FACEBOOK_API_VERSION: 'v14.0' + FACEBOOK_API_VERSION: 'v14.0', + IS_ENTERPRISE: ChatwootApp.enterprise? } end end diff --git a/app/helpers/api/v1/inboxes_helper.rb b/app/helpers/api/v1/inboxes_helper.rb index 56fb79908..82644e62f 100644 --- a/app/helpers/api/v1/inboxes_helper.rb +++ b/app/helpers/api/v1/inboxes_helper.rb @@ -74,4 +74,22 @@ module Api::V1::InboxesHelper context.verify_mode = openssl_verify_mode context end + + def account_channels_method + { + 'web_widget' => Current.account.web_widgets, + 'api' => Current.account.api_channels, + 'email' => Current.account.email_channels, + 'line' => Current.account.line_channels, + 'telegram' => Current.account.telegram_channels, + 'whatsapp' => Current.account.whatsapp_channels, + 'sms' => Current.account.sms_channels + }[permitted_params[:channel][:type]] + end + + def validate_limit + return unless Current.account.inboxes.count >= Current.account.usage_limits[:inboxes] + + render_payment_required('Account limit exceeded. Upgrade to a higher plan') + end end diff --git a/app/javascript/dashboard/i18n/locale/en/inboxMgmt.json b/app/javascript/dashboard/i18n/locale/en/inboxMgmt.json index 252a041bc..6bc5603b0 100644 --- a/app/javascript/dashboard/i18n/locale/en/inboxMgmt.json +++ b/app/javascript/dashboard/i18n/locale/en/inboxMgmt.json @@ -421,6 +421,11 @@ "ALLOW_MESSAGES_AFTER_RESOLVED": "Allow messages after conversation resolved", "ALLOW_MESSAGES_AFTER_RESOLVED_SUB_TEXT": "Allow the end-users to send messages even after the conversation is resolved." }, + "AUTO_ASSIGNMENT":{ + "MAX_ASSIGNMENT_LIMIT": "Auto assignment limit", + "MAX_ASSIGNMENT_LIMIT_RANGE_ERROR": "Please enter a value greater than 0", + "MAX_ASSIGNMENT_LIMIT_SUB_TEXT": "Limit the maximum number of conversations from this inbox that can be auto assigned to an agent" + }, "FACEBOOK_REAUTHORIZE": { "TITLE": "Reauthorize", "SUBTITLE": "Your Facebook connection has expired, please reconnect your Facebook page to continue services", diff --git a/app/javascript/dashboard/routes/dashboard/settings/inbox/settingsPage/CollaboratorsPage.vue b/app/javascript/dashboard/routes/dashboard/settings/inbox/settingsPage/CollaboratorsPage.vue index 2b3fa413f..68f5bc5e3 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/inbox/settingsPage/CollaboratorsPage.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/inbox/settingsPage/CollaboratorsPage.vue @@ -48,20 +48,47 @@ {{ $t('INBOX_MGMT.SETTINGS_POPUP.AUTO_ASSIGNMENT_SUB_TEXT') }}

+ + +
+ + +

+ {{ $t('INBOX_MGMT.AUTO_ASSIGNMENT.MAX_ASSIGNMENT_LIMIT_SUB_TEXT') }} +

+ + +
+ + diff --git a/app/javascript/shared/mixins/configMixin.js b/app/javascript/shared/mixins/configMixin.js index acbe98511..7561f08df 100644 --- a/app/javascript/shared/mixins/configMixin.js +++ b/app/javascript/shared/mixins/configMixin.js @@ -9,5 +9,8 @@ export default { enabledLanguages() { return window.chatwootConfig.enabledLanguages; }, + isEnterprise() { + return window.chatwootConfig.isEnterprise === 'true'; + }, }, }; diff --git a/app/models/account.rb b/app/models/account.rb index 7dd1cc229..62af85e86 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -20,7 +20,6 @@ class Account < ApplicationRecord include FlagShihTzu include Reportable include Featurable - prepend_mod_with('Account') DEFAULT_QUERY_SETTING = { flag_query_mode: :bit_operator, @@ -146,3 +145,5 @@ class Account < ApplicationRecord ActiveRecord::Base.connection.exec_query("drop sequence IF EXISTS conv_dpid_seq_#{id}") end end + +Account.prepend_mod_with('Account') diff --git a/app/models/concerns/round_robin_handler.rb b/app/models/concerns/round_robin_handler.rb index 80865acd7..f543f8e65 100644 --- a/app/models/concerns/round_robin_handler.rb +++ b/app/models/concerns/round_robin_handler.rb @@ -14,7 +14,7 @@ module RoundRobinHandler return unless conversation_status_changed_to_open? return unless should_round_robin? - ::RoundRobin::AssignmentService.new(conversation: self).perform + ::RoundRobin::AssignmentService.new(conversation: self, allowed_member_ids: inbox.member_ids_with_assignment_capacity).perform end def should_round_robin? diff --git a/app/models/inbox.rb b/app/models/inbox.rb index 2801fa834..b58e6870a 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -6,6 +6,7 @@ # # id :integer not null, primary key # allow_messages_after_resolved :boolean default(TRUE) +# auto_assignment_config :jsonb # channel_type :string # csat_survey_enabled :boolean default(FALSE) # email_address :string @@ -35,6 +36,7 @@ class Inbox < ApplicationRecord validates :name, presence: true validates :account_id, presence: true validates :timezone, inclusion: { in: TZInfo::Timezone.all_identifiers } + validate :ensure_valid_max_assignment_limit belongs_to :account @@ -118,9 +120,19 @@ class Inbox < ApplicationRecord end end + def member_ids_with_assignment_capacity + members.ids + end + private + def ensure_valid_max_assignment_limit + # overridden in enterprise/app/models/enterprise/inbox.rb + end + def delete_round_robin_agents ::RoundRobin::ManageService.new(inbox: self).clear_queue end end + +Inbox.prepend_mod_with('Inbox') diff --git a/app/services/round_robin/manage_service.rb b/app/services/round_robin/manage_service.rb index 9b3a60b53..3b5fd1660 100644 --- a/app/services/round_robin/manage_service.rb +++ b/app/services/round_robin/manage_service.rb @@ -1,3 +1,6 @@ +# NOTE: available agent method now expect allowed_member_ids to be passed in always because of inbox limits feature +# need to refactor this class and split the queue managment into a seperate class + # If allowed_member_ids are supplied round robin service will only fetch a member from member id # This is used in case of team assignment class RoundRobin::ManageService @@ -18,6 +21,13 @@ class RoundRobin::ManageService ::Redis::Alfred.lrem(round_robin_key, user_id) end + def reset_queue + clear_queue + add_agent_to_queue(inbox.inbox_members.map(&:user_id)) + end + + # end of queue management functions + def available_agent(priority_list: []) reset_queue unless validate_queue? user_id = get_member_via_priority_list(priority_list) @@ -26,29 +36,22 @@ class RoundRobin::ManageService inbox.inbox_members.find_by(user_id: user_id)&.user if user_id.present? end - def reset_queue - clear_queue - add_agent_to_queue(inbox.inbox_members.map(&:user_id)) - end - private def fetch_user_id - if allowed_member_ids_in_str.present? - user_id = queue.intersection(allowed_member_ids_in_str).pop - pop_push_to_queue(user_id) - user_id - else - ::Redis::Alfred.rpoplpush(round_robin_key, round_robin_key) - end + return nil if allowed_member_ids_in_str.blank? + + user_id = queue.intersection(allowed_member_ids_in_str).pop + pop_push_to_queue(user_id) + user_id end - # priority list is usually the members who are online passed from assignmebt service + # priority list is usually the members who are online passed from assignment service def get_member_via_priority_list(priority_list) return if priority_list.blank? - # when allowed member ids is passed we will be looking to get members from that list alone - priority_list = priority_list.intersection(allowed_member_ids_in_str) if allowed_member_ids_in_str.present? + # When allowed member ids is passed we will be looking to get members from that list alone + priority_list = priority_list.intersection(allowed_member_ids_in_str) return if priority_list.blank? user_id = queue.intersection(priority_list.map(&:to_s)).pop diff --git a/app/views/api/v1/models/_inbox.json.jbuilder b/app/views/api/v1/models/_inbox.json.jbuilder index 18b737da3..b91b04549 100644 --- a/app/views/api/v1/models/_inbox.json.jbuilder +++ b/app/views/api/v1/models/_inbox.json.jbuilder @@ -9,6 +9,7 @@ json.working_hours_enabled resource.working_hours_enabled json.enable_email_collect resource.enable_email_collect json.csat_survey_enabled resource.csat_survey_enabled json.enable_auto_assignment resource.enable_auto_assignment +json.auto_assignment_config resource.auto_assignment_config json.out_of_office_message resource.out_of_office_message json.working_hours resource.weekly_schedule json.timezone resource.timezone diff --git a/app/views/layouts/vueapp.html.erb b/app/views/layouts/vueapp.html.erb index 3f91ac042..ae7393ee6 100644 --- a/app/views/layouts/vueapp.html.erb +++ b/app/views/layouts/vueapp.html.erb @@ -36,6 +36,7 @@ fbAppId: '<%= ENV.fetch('FB_APP_ID', nil) %>', fbApiVersion: '<%= @global_config['FACEBOOK_API_VERSION'] %>', signupEnabled: '<%= @global_config['ENABLE_ACCOUNT_SIGNUP'] %>', + isEnterprise: '<%= @global_config['IS_ENTERPRISE'] %>', <% if @global_config['VAPID_PUBLIC_KEY'] %> vapidPublicKey: new Uint8Array(<%= Base64.urlsafe_decode64(@global_config['VAPID_PUBLIC_KEY']).bytes %>), <% end %> diff --git a/db/migrate/20220527040433_add_auto_assignment_configuration_to_inboxes.rb b/db/migrate/20220527040433_add_auto_assignment_configuration_to_inboxes.rb new file mode 100644 index 000000000..31e324ce9 --- /dev/null +++ b/db/migrate/20220527040433_add_auto_assignment_configuration_to_inboxes.rb @@ -0,0 +1,5 @@ +class AddAutoAssignmentConfigurationToInboxes < ActiveRecord::Migration[6.1] + def change + add_column :inboxes, :auto_assignment_config, :jsonb, default: {} + end +end diff --git a/db/schema.rb b/db/schema.rb index 6f1ffe408..6ab2179d3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -513,6 +513,7 @@ ActiveRecord::Schema.define(version: 2022_06_10_091206) do t.boolean "enable_email_collect", default: true t.boolean "csat_survey_enabled", default: false t.boolean "allow_messages_after_resolved", default: true + t.jsonb "auto_assignment_config", default: {} t.index ["account_id"], name: "index_inboxes_on_account_id" end diff --git a/enterprise/app/controllers/enterprise/api/v1/accounts/inboxes_controller.rb b/enterprise/app/controllers/enterprise/api/v1/accounts/inboxes_controller.rb new file mode 100644 index 000000000..b39db609d --- /dev/null +++ b/enterprise/app/controllers/enterprise/api/v1/accounts/inboxes_controller.rb @@ -0,0 +1,9 @@ +module Enterprise::Api::V1::Accounts::InboxesController + def inbox_attributes + super + ee_inbox_attributes + end + + def ee_inbox_attributes + [auto_assignment_config: [:max_assignment_limit]] + end +end diff --git a/enterprise/app/models/enterprise/inbox.rb b/enterprise/app/models/enterprise/inbox.rb new file mode 100644 index 000000000..6fe3ff2f2 --- /dev/null +++ b/enterprise/app/models/enterprise/inbox.rb @@ -0,0 +1,20 @@ +module Enterprise::Inbox + def member_ids_with_assignment_capacity + max_assignment_limit = auto_assignment_config['max_assignment_limit'] + overloaded_agent_ids = max_assignment_limit.present? ? get_agent_ids_over_assignment_limit(max_assignment_limit) : [] + super - overloaded_agent_ids + end + + private + + def get_agent_ids_over_assignment_limit(limit) + conversations.open.select(:assignee_id).group(:assignee_id).having("count(*) >= #{limit.to_i}").filter_map(&:assignee_id) + end + + def ensure_valid_max_assignment_limit + return if auto_assignment_config['max_assignment_limit'].blank? + return if auto_assignment_config['max_assignment_limit'].to_i.positive? + + errors.add(:auto_assignment_config, 'max_assignment_limit must be greater than 0') + end +end diff --git a/spec/enterprise/controllers/api/v1/accounts/inboxes_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/inboxes_controller_spec.rb new file mode 100644 index 000000000..724a7b0cb --- /dev/null +++ b/spec/enterprise/controllers/api/v1/accounts/inboxes_controller_spec.rb @@ -0,0 +1,46 @@ +require 'rails_helper' + +RSpec.describe 'Enterprise Inboxes API', type: :request do + let(:account) { create(:account) } + let(:admin) { create(:user, account: account, role: :administrator) } + + describe 'POST /api/v1/accounts/{account.id}/inboxes' do + let(:inbox) { create(:inbox, account: account) } + + context 'when it is an authenticated user' do + let(:admin) { create(:user, account: account, role: :administrator) } + let(:valid_params) do + { name: 'test', auto_assignment_config: { max_assignment_limit: 10 }, channel: { type: 'web_widget', website_url: 'test.com' } } + end + + it 'creates a webwidget inbox with auto assignment config' do + post "/api/v1/accounts/#{account.id}/inboxes", + headers: admin.create_new_auth_token, + params: valid_params, + as: :json + + expect(response).to have_http_status(:success) + expect(JSON.parse(response.body)['auto_assignment_config']['max_assignment_limit']).to eq 10 + end + end + end + + describe 'PATCH /api/v1/accounts/{account.id}/inboxes/:id' do + let(:inbox) { create(:inbox, account: account, auto_assignment_config: { max_assignment_limit: 5 }) } + + context 'when it is an authenticated user' do + let(:admin) { create(:user, account: account, role: :administrator) } + let(:valid_params) { { name: 'new test inbox', auto_assignment_config: { max_assignment_limit: 10 } } } + + it 'updates inbox with auto assignment config' do + patch "/api/v1/accounts/#{account.id}/inboxes/#{inbox.id}", + headers: admin.create_new_auth_token, + params: valid_params, + as: :json + + expect(response).to have_http_status(:success) + expect(JSON.parse(response.body)['auto_assignment_config']['max_assignment_limit']).to eq 10 + end + end + end +end diff --git a/spec/enterprise/models/inbox_spec.rb b/spec/enterprise/models/inbox_spec.rb new file mode 100644 index 000000000..26dbcef16 --- /dev/null +++ b/spec/enterprise/models/inbox_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Inbox do + describe 'member_ids_with_assignment_capacity' do + let!(:inbox) { create(:inbox) } + let!(:inbox_member_1) { create(:inbox_member, inbox: inbox) } + let!(:inbox_member_2) { create(:inbox_member, inbox: inbox) } + let!(:inbox_member_3) { create(:inbox_member, inbox: inbox) } + let!(:inbox_member_4) { create(:inbox_member, inbox: inbox) } + + before do + create(:conversation, inbox: inbox, assignee: inbox_member_1.user) + # to test conversations in other inboxes won't impact + create_list(:conversation, 3, assignee: inbox_member_1.user) + create_list(:conversation, 2, inbox: inbox, assignee: inbox_member_2.user) + create_list(:conversation, 3, inbox: inbox, assignee: inbox_member_3.user) + end + + it 'validated max_assignment_limit' do + account = create(:account) + expect(build(:inbox, account: account, auto_assignment_config: { max_assignment_limit: 0 })).not_to be_valid + expect(build(:inbox, account: account, auto_assignment_config: {})).to be_valid + expect(build(:inbox, account: account, auto_assignment_config: { max_assignment_limit: 1 })).to be_valid + end + + it 'returns member ids with assignment capacity with inbox max_assignment_limit is configured' do + # agent 1 has 1 conversations, agent 2 has 2 conversations, agent 3 has 3 conversations and agent 4 with none + inbox.update(auto_assignment_config: { max_assignment_limit: 2 }) + expect(inbox.member_ids_with_assignment_capacity).to contain_exactly(inbox_member_1.user_id, inbox_member_4.user_id) + end + + it 'returns all member ids when inbox max_assignment_limit is not configured' do + expect(inbox.member_ids_with_assignment_capacity).to eq(inbox.members.ids) + end + end +end diff --git a/spec/services/round_robin/manage_service_spec.rb b/spec/services/round_robin/manage_service_spec.rb index 75110d325..84cca45d1 100644 --- a/spec/services/round_robin/manage_service_spec.rb +++ b/spec/services/round_robin/manage_service_spec.rb @@ -8,10 +8,14 @@ describe RoundRobin::ManageService do let!(:inbox_members) { create_list(:inbox_member, 5, inbox: inbox) } describe '#available_agent' do - it 'gets the first available agent and move agent to end of the list' do + it 'returns nil if allowed_member_ids is empty' do + expect(described_class.new(inbox: inbox, allowed_member_ids: []).available_agent).to eq nil + end + + it 'gets the first available agent in allowed_member_ids and move agent to end of the list' do expected_queue = [inbox_members[0].user_id, inbox_members[4].user_id, inbox_members[3].user_id, inbox_members[2].user_id, inbox_members[1].user_id].map(&:to_s) - round_robin_manage_service.available_agent + described_class.new(inbox: inbox, allowed_member_ids: [inbox_members[0].user_id, inbox_members[4].user_id]).available_agent expect(round_robin_manage_service.send(:queue)).to eq(expected_queue) end @@ -19,8 +23,8 @@ describe RoundRobin::ManageService do expected_queue = [inbox_members[2].user_id, inbox_members[4].user_id, inbox_members[3].user_id, inbox_members[1].user_id, inbox_members[0].user_id].map(&:to_s) # prority list will be ids in string, since thats what redis supplies to us - expect(round_robin_manage_service.available_agent(priority_list: [inbox_members[3].user_id.to_s, - inbox_members[2].user_id.to_s])).to eq inbox_members[2].user + expect(described_class.new(inbox: inbox, allowed_member_ids: [inbox_members[2].user_id]) + .available_agent(priority_list: [inbox_members[3].user_id.to_s, inbox_members[2].user_id.to_s])).to eq inbox_members[2].user expect(round_robin_manage_service.send(:queue)).to eq(expected_queue) end