From b05d06a28a055ac2e4fdabd2dc1398ea98e5876c Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Fri, 25 Nov 2022 13:01:04 +0300 Subject: [PATCH] feat: Ability to lock to single conversation (#5881) Adds the ability to lock conversation to a single thread for Whatsapp and Sms Inboxes when using outbound messages. demo: https://www.loom.com/share/c9e1e563c8914837a4139dfdd2503fef fixes: #4975 Co-authored-by: Nithin David <1277421+nithindavid@users.noreply.github.com> --- .rubocop.yml | 1 - app/builders/conversation_builder.rb | 40 ++++++++++++++++ .../v1/accounts/conversations_controller.rb | 28 ++--------- .../api/v1/accounts/inboxes_controller.rb | 3 +- .../dashboard/i18n/locale/en/inboxMgmt.json | 6 +++ .../dashboard/settings/inbox/Settings.vue | 28 +++++++++++ .../store/modules/contactConversations.js | 14 +++++- app/models/inbox.rb | 1 + app/views/api/v1/models/_inbox.json.jbuilder | 5 +- ..._add_lock_conversation_to_single_thread.rb | 5 ++ db/schema.rb | 1 + spec/builders/conversation_builder_spec.rb | 46 +++++++++++++++++++ .../accounts/conversations_controller_spec.rb | 40 ++++++++-------- 13 files changed, 171 insertions(+), 47 deletions(-) create mode 100644 app/builders/conversation_builder.rb create mode 100644 db/migrate/20221109065043_add_lock_conversation_to_single_thread.rb create mode 100644 spec/builders/conversation_builder_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index 3665ad2e3..dafd9a620 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,7 +16,6 @@ Metrics/ClassLength: - 'app/models/message.rb' - 'app/builders/messages/facebook/message_builder.rb' - 'app/controllers/api/v1/accounts/contacts_controller.rb' - - 'app/controllers/api/v1/accounts/conversations_controller.rb' - 'app/listeners/action_cable_listener.rb' - 'app/models/conversation.rb' RSpec/ExampleLength: diff --git a/app/builders/conversation_builder.rb b/app/builders/conversation_builder.rb new file mode 100644 index 000000000..6a995b188 --- /dev/null +++ b/app/builders/conversation_builder.rb @@ -0,0 +1,40 @@ +class ConversationBuilder + pattr_initialize [:params!, :contact_inbox!] + + def perform + look_up_exising_conversation || create_new_conversation + end + + private + + def look_up_exising_conversation + return unless @contact_inbox.inbox.lock_to_single_conversation? + + @contact_inbox.conversations.last + end + + def create_new_conversation + ::Conversation.create!(conversation_params) + end + + def conversation_params + additional_attributes = params[:additional_attributes]&.permit! || {} + custom_attributes = params[:custom_attributes]&.permit! || {} + status = params[:status].present? ? { status: params[:status] } : {} + + # TODO: temporary fallback for the old bot status in conversation, we will remove after couple of releases + # commenting this out to see if there are any errors, if not we can remove this in subsequent releases + # status = { status: 'pending' } if status[:status] == 'bot' + { + account_id: @contact_inbox.inbox.account_id, + inbox_id: @contact_inbox.inbox_id, + contact_id: @contact_inbox.contact_id, + contact_inbox_id: @contact_inbox.id, + additional_attributes: additional_attributes, + custom_attributes: custom_attributes, + snoozed_until: params[:snoozed_until], + assignee_id: params[:assignee_id], + team_id: params[:team_id] + }.merge(status) + end +end diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 3a86a3c42..ec107dfff 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -24,7 +24,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro def create ActiveRecord::Base.transaction do - @conversation = ::Conversation.create!(conversation_params) + @conversation = ConversationBuilder.new(params: params, contact_inbox: @contact_inbox).perform Messages::MessageBuilder.new(Current.user, @conversation, params[:message]).perform if params[:message].present? end end @@ -99,8 +99,10 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro end def set_conversation_status - status = params[:status] == 'bot' ? 'pending' : params[:status] - @conversation.status = status + # TODO: temporary fallback for the old bot status in conversation, we will remove after couple of releases + # commenting this out to see if there are any errors, if not we can remove this in subsequent releases + # status = params[:status] == 'bot' ? 'pending' : params[:status] + @conversation.status = params[:status] @conversation.snoozed_until = parse_date_time(params[:snoozed_until].to_s) if params[:snoozed_until] end @@ -152,26 +154,6 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro ).perform end - def conversation_params - additional_attributes = params[:additional_attributes]&.permit! || {} - custom_attributes = params[:custom_attributes]&.permit! || {} - status = params[:status].present? ? { status: params[:status] } : {} - - # TODO: temporary fallback for the old bot status in conversation, we will remove after couple of releases - status = { status: 'pending' } if status[:status] == 'bot' - { - account_id: Current.account.id, - inbox_id: @contact_inbox.inbox_id, - contact_id: @contact_inbox.contact_id, - contact_inbox_id: @contact_inbox.id, - additional_attributes: additional_attributes, - custom_attributes: custom_attributes, - snoozed_until: params[:snoozed_until], - assignee_id: params[:assignee_id], - team_id: params[:team_id] - }.merge(status) - end - def conversation_finder @conversation_finder ||= ConversationFinder.new(Current.user, params) end diff --git a/app/controllers/api/v1/accounts/inboxes_controller.rb b/app/controllers/api/v1/accounts/inboxes_controller.rb index 95662e29b..24507977e 100644 --- a/app/controllers/api/v1/accounts/inboxes_controller.rb +++ b/app/controllers/api/v1/accounts/inboxes_controller.rb @@ -113,7 +113,8 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController 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] + :enable_auto_assignment, :working_hours_enabled, :out_of_office_message, :timezone, :allow_messages_after_resolved, + :lock_to_single_conversation] end def permitted_params(channel_attributes = []) diff --git a/app/javascript/dashboard/i18n/locale/en/inboxMgmt.json b/app/javascript/dashboard/i18n/locale/en/inboxMgmt.json index b624cce11..d06907233 100644 --- a/app/javascript/dashboard/i18n/locale/en/inboxMgmt.json +++ b/app/javascript/dashboard/i18n/locale/en/inboxMgmt.json @@ -388,6 +388,10 @@ "ENABLED": "Enabled", "DISABLED": "Disabled" }, + "LOCK_TO_SINGLE_CONVERSATION": { + "ENABLED": "Enabled", + "DISABLED": "Disabled" + }, "ENABLE_HMAC": { "LABEL": "Enable" } @@ -441,6 +445,8 @@ "ENABLE_CSAT_SUB_TEXT": "Enable/Disable CSAT(Customer satisfaction) survey after resolving a conversation", "ENABLE_CONTINUITY_VIA_EMAIL": "Enable conversation continuity via email", "ENABLE_CONTINUITY_VIA_EMAIL_SUB_TEXT": "Conversations will continue over email if the contact email address is available.", + "LOCK_TO_SINGLE_CONVERSATION": "Lock to single conversation", + "LOCK_TO_SINGLE_CONVERSATION_SUB_TEXT": "Enable or disable multiple conversations for the same contact in this inbox", "INBOX_UPDATE_TITLE": "Inbox Settings", "INBOX_UPDATE_SUB_TEXT": "Update your inbox settings", "AUTO_ASSIGNMENT_SUB_TEXT": "Enable or disable the automatic assignment of new conversations to the agents added to this inbox.", diff --git a/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue b/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue index 8790ef57a..2f963a44d 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue @@ -258,6 +258,28 @@

+ + @@ -380,6 +402,7 @@ export default { greetingMessage: '', emailCollectEnabled: false, csatSurveyEnabled: false, + locktoSingleConversation: false, allowMessagesAfterResolved: true, continuityViaEmail: true, selectedInboxName: '', @@ -496,6 +519,9 @@ export default { } return this.inbox.name; }, + canLocktoSingleConversation() { + return this.isASmsInbox || this.isAWhatsAppChannel; + }, inboxNameLabel() { if (this.isAWebWidgetInbox) { return this.$t('INBOX_MGMT.ADD.WEBSITE_NAME.LABEL'); @@ -567,6 +593,7 @@ export default { this.channelWelcomeTagline = this.inbox.welcome_tagline; this.selectedFeatureFlags = this.inbox.selected_feature_flags || []; this.replyTime = this.inbox.reply_time; + this.locktoSingleConversation = this.inbox.lock_to_single_conversation; }); }, async updateInbox() { @@ -579,6 +606,7 @@ export default { allow_messages_after_resolved: this.allowMessagesAfterResolved, greeting_enabled: this.greetingEnabled, greeting_message: this.greetingMessage || '', + lock_to_single_conversation: this.locktoSingleConversation, channel: { widget_color: this.inbox.widget_color, website_url: this.channelWebsiteUrl, diff --git a/app/javascript/dashboard/store/modules/contactConversations.js b/app/javascript/dashboard/store/modules/contactConversations.js index 9c9f03016..a696a3e75 100644 --- a/app/javascript/dashboard/store/modules/contactConversations.js +++ b/app/javascript/dashboard/store/modules/contactConversations.js @@ -89,7 +89,19 @@ export const mutations = { }, [types.default.ADD_CONTACT_CONVERSATION]: ($state, { id, data }) => { const conversations = $state.records[id] || []; - Vue.set($state.records, id, [...conversations, data]); + + const updatedConversations = [...conversations]; + const index = conversations.findIndex( + conversation => conversation.id === data.id + ); + + if (index !== -1) { + updatedConversations[index] = { ...conversations[index], ...data }; + } else { + updatedConversations.push(data); + } + + Vue.set($state.records, id, updatedConversations); }, [types.default.DELETE_CONTACT_CONVERSATION]: ($state, id) => { Vue.delete($state.records, id); diff --git a/app/models/inbox.rb b/app/models/inbox.rb index 0a24466fb..98419e9ab 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -14,6 +14,7 @@ # enable_email_collect :boolean default(TRUE) # greeting_enabled :boolean default(FALSE) # greeting_message :string +# lock_to_single_conversation :boolean default(FALSE), not null # name :string not null # out_of_office_message :string # timezone :string default("UTC") diff --git a/app/views/api/v1/models/_inbox.json.jbuilder b/app/views/api/v1/models/_inbox.json.jbuilder index 73b240a2a..414dc250a 100644 --- a/app/views/api/v1/models/_inbox.json.jbuilder +++ b/app/views/api/v1/models/_inbox.json.jbuilder @@ -15,12 +15,13 @@ json.working_hours resource.weekly_schedule json.timezone resource.timezone json.callback_webhook_url resource.callback_webhook_url json.allow_messages_after_resolved resource.allow_messages_after_resolved - -json.tweets_enabled resource.channel.try(:tweets_enabled) if resource.twitter? +json.lock_to_single_conversation resource.lock_to_single_conversation ## Channel specific settings ## TODO : Clean up and move the attributes into channel sub section +json.tweets_enabled resource.channel.try(:tweets_enabled) if resource.twitter? + ## WebWidget Attributes json.widget_color resource.channel.try(:widget_color) json.website_url resource.channel.try(:website_url) diff --git a/db/migrate/20221109065043_add_lock_conversation_to_single_thread.rb b/db/migrate/20221109065043_add_lock_conversation_to_single_thread.rb new file mode 100644 index 000000000..9a0dbd450 --- /dev/null +++ b/db/migrate/20221109065043_add_lock_conversation_to_single_thread.rb @@ -0,0 +1,5 @@ +class AddLockConversationToSingleThread < ActiveRecord::Migration[6.1] + def change + add_column :inboxes, :lock_to_single_conversation, :boolean, null: false, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index ea56820ba..60cfb93e6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -535,6 +535,7 @@ ActiveRecord::Schema.define(version: 2022_11_16_000514) do t.boolean "csat_survey_enabled", default: false t.boolean "allow_messages_after_resolved", default: true t.jsonb "auto_assignment_config", default: {} + t.boolean "lock_to_single_conversation", default: false, null: false t.index ["account_id"], name: "index_inboxes_on_account_id" end diff --git a/spec/builders/conversation_builder_spec.rb b/spec/builders/conversation_builder_spec.rb new file mode 100644 index 000000000..f0e0ada06 --- /dev/null +++ b/spec/builders/conversation_builder_spec.rb @@ -0,0 +1,46 @@ +require 'rails_helper' + +describe ::ConversationBuilder do + let(:account) { create(:account) } + let!(:sms_channel) { create(:channel_sms, account: account) } + let!(:sms_inbox) { create(:inbox, channel: sms_channel, account: account) } + let(:contact) { create(:contact, account: account) } + let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: sms_inbox) } + + describe '#perform' do + it 'creates conversation' do + conversation = described_class.new( + contact_inbox: contact_inbox, + params: {} + ).perform + + expect(conversation.contact_inbox_id).to eq(contact_inbox.id) + end + + context 'when lock_to_single_conversation is true for inbox' do + before do + sms_inbox.update!(lock_to_single_conversation: true) + end + + it 'creates conversation when existing conversation is not present' do + conversation = described_class.new( + contact_inbox: contact_inbox, + params: {} + ).perform + + expect(conversation.contact_inbox_id).to eq(contact_inbox.id) + end + + it 'returns last from existing conversations when existing conversation is not present' do + create(:conversation, contact_inbox: contact_inbox) + existing_conversation = create(:conversation, contact_inbox: contact_inbox) + conversation = described_class.new( + contact_inbox: contact_inbox, + params: {} + ).perform + + expect(conversation.id).to eq(existing_conversation.id) + end + end + end +end diff --git a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb index b0c2681a6..690a70c53 100644 --- a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -265,17 +265,18 @@ RSpec.describe 'Conversations API', type: :request do # TODO: remove this spec when we remove the condition check in controller # Added for backwards compatibility for bot status - it 'creates a conversation as pending if status is specified as bot' do - allow(Rails.configuration.dispatcher).to receive(:dispatch) - post "/api/v1/accounts/#{account.id}/conversations", - headers: agent.create_new_auth_token, - params: { source_id: contact_inbox.source_id, status: 'bot' }, - as: :json + # remove this in subsequent release + # it 'creates a conversation as pending if status is specified as bot' do + # allow(Rails.configuration.dispatcher).to receive(:dispatch) + # post "/api/v1/accounts/#{account.id}/conversations", + # headers: agent.create_new_auth_token, + # params: { source_id: contact_inbox.source_id, status: 'bot' }, + # as: :json - expect(response).to have_http_status(:success) - response_data = JSON.parse(response.body, symbolize_names: true) - expect(response_data[:status]).to eq('pending') - end + # expect(response).to have_http_status(:success) + # response_data = JSON.parse(response.body, symbolize_names: true) + # expect(response_data[:status]).to eq('pending') + # end it 'creates a new conversation with message when message is passed' do allow(Rails.configuration.dispatcher).to receive(:dispatch) @@ -408,17 +409,18 @@ RSpec.describe 'Conversations API', type: :request do # TODO: remove this spec when we remove the condition check in controller # Added for backwards compatibility for bot status - it 'toggles the conversation status to pending status when parameter bot is passed' do - expect(conversation.status).to eq('open') + # remove in next release + # it 'toggles the conversation status to pending status when parameter bot is passed' do + # expect(conversation.status).to eq('open') - post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/toggle_status", - headers: agent.create_new_auth_token, - params: { status: 'bot' }, - as: :json + # post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/toggle_status", + # headers: agent.create_new_auth_token, + # params: { status: 'bot' }, + # as: :json - expect(response).to have_http_status(:success) - expect(conversation.reload.status).to eq('pending') - end + # expect(response).to have_http_status(:success) + # expect(conversation.reload.status).to eq('pending') + # end end end