From a7ca55c0801561bbd0c4a9e21a775569914a7d6c Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 21 Jul 2021 22:02:43 +0530 Subject: [PATCH] chore: Change the conversation `bot` status to `pending` (#2677) fixes: #2649 --- .../v1/accounts/conversations_controller.rb | 6 +++- .../components/buttons/ResolveAction.vue | 16 ++++----- app/javascript/dashboard/constants.js | 2 +- .../dashboard/i18n/locale/en/chatlist.json | 4 +-- .../i18n/locale/en/conversation.json | 2 +- .../conversationAttributes/actions.spec.js | 8 ++--- .../conversationAttributes/getters.spec.js | 4 +-- .../conversationAttributes/mutations.spec.js | 6 ++-- app/listeners/notification_listener.rb | 4 +-- app/models/conversation.rb | 11 +++--- config/locales/en.yml | 2 +- .../dialogflow/processor_service.rb | 2 +- spec/controllers/api/base_controller_spec.rb | 2 +- .../accounts/conversations_controller_spec.rb | 36 ++++++++++++++++--- .../dialogflow/processor_service_spec.rb | 2 +- .../concerns/round_robin_handler_spec.rb | 2 +- spec/models/conversation_spec.rb | 8 ++--- swagger/definitions/resource/conversation.yml | 2 +- swagger/paths/conversation/create.yml | 2 +- swagger/paths/conversation/index.yml | 6 ++-- swagger/paths/conversation/toggle_status.yml | 2 +- swagger/swagger.json | 12 +++---- 22 files changed, 87 insertions(+), 54 deletions(-) diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 9f4f0b45d..8d719e46c 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -49,7 +49,8 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro def toggle_status if params[:status] - @conversation.status = params[:status] + status = params[:status] == 'bot' ? 'pending' : params[:status] + @conversation.status = status @status = @conversation.save else @status = @conversation.toggle_status @@ -106,6 +107,9 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro def conversation_params additional_attributes = params[:additional_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, diff --git a/app/javascript/dashboard/components/buttons/ResolveAction.vue b/app/javascript/dashboard/components/buttons/ResolveAction.vue index 5c429d237..2ab3ad83b 100644 --- a/app/javascript/dashboard/components/buttons/ResolveAction.vue +++ b/app/javascript/dashboard/components/buttons/ResolveAction.vue @@ -24,7 +24,7 @@ {{ this.$t('CONVERSATION.HEADER.REOPEN_ACTION') }} - + - {{ this.$t('CONVERSATION.RESOLVE_DROPDOWN.OPEN_BOT') }} + {{ this.$t('CONVERSATION.RESOLVE_DROPDOWN.MARK_PENDING') }} @@ -91,20 +91,20 @@ export default { isOpen() { return this.currentChat.status === wootConstants.STATUS_TYPE.OPEN; }, - isBot() { - return this.currentChat.status === wootConstants.STATUS_TYPE.BOT; + isPending() { + return this.currentChat.status === wootConstants.STATUS_TYPE.PENDING; }, isResolved() { return this.currentChat.status === wootConstants.STATUS_TYPE.RESOLVED; }, buttonClass() { - if (this.isBot) return 'primary'; + if (this.isPending) return 'primary'; if (this.isOpen) return 'success'; if (this.isResolved) return 'warning'; return ''; }, showDropDown() { - return !this.isBot; + return !this.isPending; }, }, methods: { diff --git a/app/javascript/dashboard/constants.js b/app/javascript/dashboard/constants.js index 34f4b9845..b981fd31d 100644 --- a/app/javascript/dashboard/constants.js +++ b/app/javascript/dashboard/constants.js @@ -8,7 +8,7 @@ export default { STATUS_TYPE: { OPEN: 'open', RESOLVED: 'resolved', - BOT: 'bot', + PENDING: 'pending', }, }; export const DEFAULT_REDIRECT_URL = '/app/'; diff --git a/app/javascript/dashboard/i18n/locale/en/chatlist.json b/app/javascript/dashboard/i18n/locale/en/chatlist.json index ef4e0629a..a8847e758 100644 --- a/app/javascript/dashboard/i18n/locale/en/chatlist.json +++ b/app/javascript/dashboard/i18n/locale/en/chatlist.json @@ -47,8 +47,8 @@ "VALUE": "resolved" }, { - "TEXT": "Bot", - "VALUE": "bot" + "TEXT": "Pending", + "VALUE": "pending" } ], "ATTACHMENTS": { diff --git a/app/javascript/dashboard/i18n/locale/en/conversation.json b/app/javascript/dashboard/i18n/locale/en/conversation.json index 81c023aa5..831bad781 100644 --- a/app/javascript/dashboard/i18n/locale/en/conversation.json +++ b/app/javascript/dashboard/i18n/locale/en/conversation.json @@ -41,7 +41,7 @@ "DETAILS": "details" }, "RESOLVE_DROPDOWN": { - "OPEN_BOT": "Open with bot" + "MARK_PENDING": "Mark as pending" }, "FOOTER": { "MSG_INPUT": "Shift + enter for new line. Start with '/' to select a Canned Response.", diff --git a/app/javascript/widget/store/modules/specs/conversationAttributes/actions.spec.js b/app/javascript/widget/store/modules/specs/conversationAttributes/actions.spec.js index 51c864ad2..241e620cf 100644 --- a/app/javascript/widget/store/modules/specs/conversationAttributes/actions.spec.js +++ b/app/javascript/widget/store/modules/specs/conversationAttributes/actions.spec.js @@ -7,10 +7,10 @@ jest.mock('widget/helpers/axios'); describe('#actions', () => { describe('#get attributes', () => { it('sends mutation if api is success', async () => { - API.get.mockResolvedValue({ data: { id: 1, status: 'bot' } }); + API.get.mockResolvedValue({ data: { id: 1, status: 'pending' } }); await actions.getAttributes({ commit }); expect(commit.mock.calls).toEqual([ - ['SET_CONVERSATION_ATTRIBUTES', { id: 1, status: 'bot' }], + ['SET_CONVERSATION_ATTRIBUTES', { id: 1, status: 'pending' }], ['conversation/setMetaUserLastSeenAt', undefined, { root: true }], ]); }); @@ -23,10 +23,10 @@ describe('#actions', () => { describe('#update attributes', () => { it('sends correct mutations', () => { - actions.update({ commit }, { id: 1, status: 'bot' }); + actions.update({ commit }, { id: 1, status: 'pending' }); expect(commit).toBeCalledWith('UPDATE_CONVERSATION_ATTRIBUTES', { id: 1, - status: 'bot', + status: 'pending', }); }); }); diff --git a/app/javascript/widget/store/modules/specs/conversationAttributes/getters.spec.js b/app/javascript/widget/store/modules/specs/conversationAttributes/getters.spec.js index 478c97d2c..0fe40bf11 100644 --- a/app/javascript/widget/store/modules/specs/conversationAttributes/getters.spec.js +++ b/app/javascript/widget/store/modules/specs/conversationAttributes/getters.spec.js @@ -4,11 +4,11 @@ describe('#getters', () => { it('getConversationParams', () => { const state = { id: 1, - status: 'bot', + status: 'pending', }; expect(getters.getConversationParams(state)).toEqual({ id: 1, - status: 'bot', + status: 'pending', }); }); }); diff --git a/app/javascript/widget/store/modules/specs/conversationAttributes/mutations.spec.js b/app/javascript/widget/store/modules/specs/conversationAttributes/mutations.spec.js index 2b2bd62fc..ede3c3a8f 100644 --- a/app/javascript/widget/store/modules/specs/conversationAttributes/mutations.spec.js +++ b/app/javascript/widget/store/modules/specs/conversationAttributes/mutations.spec.js @@ -14,7 +14,7 @@ describe('#mutations', () => { describe('#UPDATE_CONVERSATION_ATTRIBUTES', () => { it('update status if it is same conversation', () => { - const state = { id: 1, status: 'bot' }; + const state = { id: 1, status: 'pending' }; mutations.UPDATE_CONVERSATION_ATTRIBUTES(state, { id: 1, status: 'open', @@ -22,12 +22,12 @@ describe('#mutations', () => { expect(state).toEqual({ id: 1, status: 'open' }); }); it('doesnot update status if it is not the same conversation', () => { - const state = { id: 1, status: 'bot' }; + const state = { id: 1, status: 'pending' }; mutations.UPDATE_CONVERSATION_ATTRIBUTES(state, { id: 2, status: 'open', }); - expect(state).toEqual({ id: 1, status: 'bot' }); + expect(state).toEqual({ id: 1, status: 'pending' }); }); }); diff --git a/app/listeners/notification_listener.rb b/app/listeners/notification_listener.rb index ad7bf2d14..b619dfa88 100644 --- a/app/listeners/notification_listener.rb +++ b/app/listeners/notification_listener.rb @@ -1,7 +1,7 @@ class NotificationListener < BaseListener def conversation_created(event) conversation, account = extract_conversation_and_account(event) - return if conversation.bot? + return if conversation.pending? conversation.inbox.members.each do |agent| NotificationBuilder.new( @@ -17,7 +17,7 @@ class NotificationListener < BaseListener conversation, account = extract_conversation_and_account(event) assignee = conversation.assignee return unless conversation.notifiable_assignee_change? - return if conversation.bot? + return if conversation.pending? NotificationBuilder.new( notification_type: 'conversation_assignment', diff --git a/app/models/conversation.rb b/app/models/conversation.rb index e0760deea..865ff0b56 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -45,7 +45,7 @@ class Conversation < ApplicationRecord validates :inbox_id, presence: true before_validation :validate_additional_attributes - enum status: { open: 0, resolved: 1, bot: 2 } + enum status: { open: 0, resolved: 1, pending: 2 } scope :latest, -> { order(last_activity_at: :desc) } scope :unassigned, -> { where(assignee_id: nil) } @@ -64,7 +64,7 @@ class Conversation < ApplicationRecord has_one :csat_survey_response, dependent: :destroy has_many :notifications, as: :primary_actor, dependent: :destroy - before_create :set_bot_conversation + before_create :mark_conversation_pending_if_bot # wanted to change this to after_update commit. But it ended up creating a loop # reinvestigate in future and identity the implications @@ -91,7 +91,7 @@ class Conversation < ApplicationRecord def toggle_status # FIXME: implement state machine with aasm self.status = open? ? :resolved : :open - self.status = :open if bot? + self.status = :open if pending? save end @@ -144,8 +144,9 @@ class Conversation < ApplicationRecord self.additional_attributes = {} unless additional_attributes.is_a?(Hash) end - def set_bot_conversation - self.status = :bot if inbox.agent_bot_inbox&.active? || inbox.hooks.pluck(:app_id).include?('dialogflow') + def mark_conversation_pending_if_bot + # TODO: make this an inbox config instead of assuming bot conversations should start as pending + self.status = :pending if inbox.agent_bot_inbox&.active? || inbox.hooks.pluck(:app_id).include?('dialogflow') end def notify_conversation_creation diff --git a/config/locales/en.yml b/config/locales/en.yml index 2e7e85d03..cae92afb8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -63,7 +63,7 @@ en: status: resolved: "Conversation was marked resolved by %{user_name}" open: "Conversation was reopened by %{user_name}" - bot: "Conversation was transferred to bot by %{user_name}" + pending: "Conversation was marked as pending by %{user_name}" auto_resolved: "Conversation was marked resolved by system due to %{duration} days of inactivity" assignee: self_assigned: "%{user_name} self-assigned this conversation" diff --git a/lib/integrations/dialogflow/processor_service.rb b/lib/integrations/dialogflow/processor_service.rb index 7a425be68..4c490d6b4 100644 --- a/lib/integrations/dialogflow/processor_service.rb +++ b/lib/integrations/dialogflow/processor_service.rb @@ -5,7 +5,7 @@ class Integrations::Dialogflow::ProcessorService message = event_data[:message] return if message.private? return unless processable_message?(message) - return unless message.conversation.bot? + return unless message.conversation.pending? response = get_dialogflow_response(message.conversation.contact_inbox.source_id, message_content(message)) process_response(message, response) diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb index 4ee5e7abe..969b83f68 100644 --- a/spec/controllers/api/base_controller_spec.rb +++ b/spec/controllers/api/base_controller_spec.rb @@ -32,7 +32,7 @@ RSpec.describe 'API Base', type: :request do describe 'request with api_access_token for bot' do let!(:agent_bot) { create(:agent_bot) } let!(:inbox) { create(:inbox, account: account) } - let!(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user, status: 'bot') } + let!(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user, status: 'pending') } context 'when it is an unauthorized url' do it 'returns unauthorized' do diff --git a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb index 82beb18cc..bb75ba54e 100644 --- a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -200,6 +200,20 @@ RSpec.describe 'Conversations API', type: :request do end it 'creates a conversation in specificed status' 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: 'pending' }, + 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 + + # 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, @@ -208,7 +222,7 @@ RSpec.describe 'Conversations API', type: :request do expect(response).to have_http_status(:success) response_data = JSON.parse(response.body, symbolize_names: true) - expect(response_data[:status]).to eq('bot') + expect(response_data[:status]).to eq('pending') end it 'creates a new conversation with message when message is passed' do @@ -269,8 +283,8 @@ RSpec.describe 'Conversations API', type: :request do expect(conversation.reload.status).to eq('resolved') end - it 'toggles the conversation status to open from bot' do - conversation.update!(status: 'bot') + it 'toggles the conversation status to open from pending' do + conversation.update!(status: 'pending') post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/toggle_status", headers: agent.create_new_auth_token, @@ -283,13 +297,27 @@ RSpec.describe 'Conversations API', type: :request do it 'toggles the conversation status to specific status when parameter 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: 'pending' }, + as: :json + + expect(response).to have_http_status(:success) + expect(conversation.reload.status).to eq('pending') + end + + # 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') + 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('bot') + expect(conversation.reload.status).to eq('pending') end end end diff --git a/spec/lib/integrations/dialogflow/processor_service_spec.rb b/spec/lib/integrations/dialogflow/processor_service_spec.rb index c0c6ac795..a0bf083f9 100644 --- a/spec/lib/integrations/dialogflow/processor_service_spec.rb +++ b/spec/lib/integrations/dialogflow/processor_service_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe Integrations::Dialogflow::ProcessorService do let(:account) { create(:account) } let(:hook) { create(:integrations_hook, :dialogflow, account: account) } - let(:conversation) { create(:conversation, account: account, status: :bot) } + let(:conversation) { create(:conversation, account: account, status: :pending) } let(:message) { create(:message, account: account, conversation: conversation) } let(:event_name) { 'message.created' } let(:event_data) { { message: message } } diff --git a/spec/models/concerns/round_robin_handler_spec.rb b/spec/models/concerns/round_robin_handler_spec.rb index 4c4c654cc..9ec499ed2 100644 --- a/spec/models/concerns/round_robin_handler_spec.rb +++ b/spec/models/concerns/round_robin_handler_spec.rb @@ -40,7 +40,7 @@ shared_examples_for 'round_robin_handler' do account: account, contact: create(:contact, account: account), inbox: inbox, - status: 'bot', + status: 'pending', assignee: nil ) diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index cbbd97855..c21db3044 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -345,8 +345,8 @@ RSpec.describe Conversation, type: :model do let!(:bot_inbox) { create(:agent_bot_inbox) } let(:conversation) { create(:conversation, inbox: bot_inbox.inbox) } - it 'returns conversation status as bot' do - expect(conversation.status).to eq('bot') + it 'returns conversation status as pending' do + expect(conversation.status).to eq('pending') end end @@ -354,8 +354,8 @@ RSpec.describe Conversation, type: :model do let(:hook) { create(:integrations_hook, :dialogflow) } let(:conversation) { create(:conversation, inbox: hook.inbox) } - it 'returns conversation status as bot' do - expect(conversation.status).to eq('bot') + it 'returns conversation status as pending' do + expect(conversation.status).to eq('pending') end end diff --git a/swagger/definitions/resource/conversation.yml b/swagger/definitions/resource/conversation.yml index 46c2efd2e..9de4dafb8 100644 --- a/swagger/definitions/resource/conversation.yml +++ b/swagger/definitions/resource/conversation.yml @@ -13,7 +13,7 @@ properties: description: ID of the inbox status: type: string - enum: ['open', 'resolved', 'bot'] + enum: ['open', 'resolved', 'pending'] description: The status of the conversation timestamp: type: string diff --git a/swagger/paths/conversation/create.yml b/swagger/paths/conversation/create.yml index e13d17cfb..104460bae 100644 --- a/swagger/paths/conversation/create.yml +++ b/swagger/paths/conversation/create.yml @@ -14,7 +14,7 @@ get: - name: status in: query type: string - enum: ['open', 'resolved', 'bot'] + enum: ['open', 'resolved', 'pending'] required: true - name: page in: query diff --git a/swagger/paths/conversation/index.yml b/swagger/paths/conversation/index.yml index 35707b65a..7c690fe56 100644 --- a/swagger/paths/conversation/index.yml +++ b/swagger/paths/conversation/index.yml @@ -15,7 +15,7 @@ get: - name: status in: query type: string - enum: ['open', 'resolved', 'bot'] + enum: ['open', 'resolved', 'pending'] - name: page in: query type: integer @@ -71,8 +71,8 @@ post: description: Lets you specify attributes like browser information status: type: string - enum: ['open', 'resolved', 'bot'] - description: Specify the conversation whether it's bot, open, closed + enum: ['open', 'resolved', 'pending'] + description: Specify the conversation whether it's pending, open, closed responses: 200: diff --git a/swagger/paths/conversation/toggle_status.yml b/swagger/paths/conversation/toggle_status.yml index 250e050e5..a737281a4 100644 --- a/swagger/paths/conversation/toggle_status.yml +++ b/swagger/paths/conversation/toggle_status.yml @@ -15,7 +15,7 @@ parameters: properties: status: type: string - enum: ["open", "resolved", "bot"] + enum: ["open", "resolved", "pending"] required: true description: The status of the conversation responses: diff --git a/swagger/swagger.json b/swagger/swagger.json index 561a1bd66..935044734 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -1417,7 +1417,7 @@ "enum": [ "open", "resolved", - "bot" + "pending" ] }, { @@ -1508,9 +1508,9 @@ "enum": [ "open", "resolved", - "bot" + "pending" ], - "description": "Specify the conversation whether it's bot, open, closed" + "description": "Specify the conversation whether it's pending, open, closed" } } } @@ -1574,7 +1574,7 @@ "enum": [ "open", "resolved", - "bot" + "pending" ], "required": true }, @@ -1688,7 +1688,7 @@ "enum": [ "open", "resolved", - "bot" + "pending" ], "required": true, "description": "The status of the conversation" @@ -2850,7 +2850,7 @@ "enum": [ "open", "resolved", - "bot" + "pending" ], "description": "The status of the conversation" },