From 88c4b63eec18bc26f3bdd5e18c7012f2e465b1c7 Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Thu, 10 Dec 2020 22:53:49 +0530 Subject: [PATCH] chore: Add slack external_source_id for outgoing messages (#1503) --- .../accounts/integrations/slack_controller.rb | 16 ++-- lib/integrations/slack/channel_builder.rb | 4 +- .../slack/send_on_slack_service.rb | 10 ++ .../slack/send_on_slack_service_spec.rb | 96 ++++++++++++------- 4 files changed, 83 insertions(+), 43 deletions(-) diff --git a/app/controllers/api/v1/accounts/integrations/slack_controller.rb b/app/controllers/api/v1/accounts/integrations/slack_controller.rb index 1002f5752..5ded9f512 100644 --- a/app/controllers/api/v1/accounts/integrations/slack_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/slack_controller.rb @@ -2,13 +2,15 @@ class Api::V1::Accounts::Integrations::SlackController < Api::V1::Accounts::Base before_action :fetch_hook, only: [:update, :destroy] def create - builder = Integrations::Slack::HookBuilder.new( - account: Current.account, - code: params[:code], - inbox_id: params[:inbox_id] - ) - @hook = builder.perform - create_chatwoot_slack_channel + ActiveRecord::Base.transaction do + builder = Integrations::Slack::HookBuilder.new( + account: Current.account, + code: params[:code], + inbox_id: params[:inbox_id] + ) + @hook = builder.perform + create_chatwoot_slack_channel + end end def update diff --git a/lib/integrations/slack/channel_builder.rb b/lib/integrations/slack/channel_builder.rb index aa2aa992a..2092f3594 100644 --- a/lib/integrations/slack/channel_builder.rb +++ b/lib/integrations/slack/channel_builder.rb @@ -21,8 +21,8 @@ class Integrations::Slack::ChannelBuilder end def find_or_create_channel - exisiting_channel = slack_client.conversations_list.channels.find { |channel| channel['name'] == params[:channel] } - @channel = exisiting_channel || slack_client.conversations_create(name: params[:channel])['channel'] + existing_channel = slack_client.conversations_list.channels.find { |channel| channel['name'] == params[:channel] } + @channel = existing_channel || slack_client.conversations_create(name: params[:channel])['channel'] end def update_reference_id diff --git a/lib/integrations/slack/send_on_slack_service.rb b/lib/integrations/slack/send_on_slack_service.rb index 89c6d0b32..87040c04a 100644 --- a/lib/integrations/slack/send_on_slack_service.rb +++ b/lib/integrations/slack/send_on_slack_service.rb @@ -24,7 +24,11 @@ class Integrations::Slack::SendOnSlackService < Base::SendOnChannelService def perform_reply send_message + + return unless @slack_message + update_reference_id + update_external_source_id_slack end def message_content @@ -68,6 +72,12 @@ class Integrations::Slack::SendOnSlackService < Base::SendOnChannelService conversation.update!(identifier: @slack_message['ts']) end + def update_external_source_id_slack + return unless @slack_message['message'] + + message.update!(external_source_id_slack: "cw-origin-#{@slack_message['message']['ts']}") + end + def slack_client @slack_client ||= Slack::Web::Client.new(token: hook.access_token) end diff --git a/spec/lib/integrations/slack/send_on_slack_service_spec.rb b/spec/lib/integrations/slack/send_on_slack_service_spec.rb index dc36de65f..234a2180b 100644 --- a/spec/lib/integrations/slack/send_on_slack_service_spec.rb +++ b/spec/lib/integrations/slack/send_on_slack_service_spec.rb @@ -1,50 +1,78 @@ require 'rails_helper' describe Integrations::Slack::SendOnSlackService do - let(:account) { create(:account) } - let!(:inbox) { create(:inbox, account: account) } let!(:contact) { create(:contact) } - + let!(:conversation) { create(:conversation, contact: contact, identifier: nil) } + let(:account) { conversation.account } let!(:hook) { create(:integrations_hook, account: account) } - let!(:conversation) { create(:conversation, account: account, inbox: inbox, contact: contact) } - let!(:message) { create(:message, account: account, inbox: inbox, conversation: conversation) } + let!(:message) do + create(:message, account: conversation.account, inbox: conversation.inbox, conversation: conversation) + end + let(:slack_message) { double } + let(:slack_message_content) { double } + let(:slack_client) { double } + let(:builder) { described_class.new(message: message, hook: hook) } + + before do + allow(builder).to receive(:slack_client).and_return(slack_client) + allow(slack_message).to receive(:[]).with('ts').and_return('12345.6789') + allow(slack_message).to receive(:[]).with('message').and_return(slack_message_content) + allow(slack_message_content).to receive(:[]).with('ts').and_return('6789.12345') + end describe '#perform' do - it 'sent message to slack' do - builder = described_class.new(message: message, hook: hook) - stub_request(:post, 'https://slack.com/api/chat.postMessage') - .to_return(status: 200, body: '', headers: {}) - slack_client = double - expect(builder).to receive(:slack_client).and_return(slack_client) + context 'without identifier' do + it 'updates slack thread id in conversation' do + inbox = conversation.inbox - expect(slack_client).to receive(:chat_postMessage).with( - channel: hook.reference_id, - text: message.content, - username: "Contact: #{message.sender.name}", - thread_ts: conversation.identifier, - icon_url: anything - ) + expect(slack_client).to receive(:chat_postMessage).with( + channel: hook.reference_id, + text: "*Inbox: #{inbox.name} [#{inbox.inbox_type}]* \n\n #{message.content}", + username: "Contact: #{message.sender.name}", + thread_ts: nil, + icon_url: anything + ).and_return(slack_message) - builder.perform + builder.perform + + expect(conversation.reload.identifier).to eq '12345.6789' + end end - it 'disables hook on Slack AccountInactive error' do - builder = described_class.new(message: message, hook: hook) - slack_client = double - expect(builder).to receive(:slack_client).and_return(slack_client) - expect(slack_client).to receive(:chat_postMessage).with( - channel: hook.reference_id, - text: message.content, - username: "Contact: #{message.sender.name}", - thread_ts: conversation.identifier, - icon_url: anything - ).and_raise(Slack::Web::Api::Errors::AccountInactive.new('Account disconnected')) + context 'with identifier' do + before do + conversation.update!(identifier: 'random_slack_thread_ts') + end - allow(hook).to receive(:authorization_error!) + it 'sent message to slack' do + expect(slack_client).to receive(:chat_postMessage).with( + channel: hook.reference_id, + text: message.content, + username: "Contact: #{message.sender.name}", + thread_ts: conversation.identifier, + icon_url: anything + ).and_return(slack_message) - builder.perform - expect(hook).to be_disabled - expect(hook).to have_received(:authorization_error!) + builder.perform + + expect(message.external_source_id_slack).to eq 'cw-origin-6789.12345' + end + + it 'disables hook on Slack AccountInactive error' do + expect(slack_client).to receive(:chat_postMessage).with( + channel: hook.reference_id, + text: message.content, + username: "Contact: #{message.sender.name}", + thread_ts: conversation.identifier, + icon_url: anything + ).and_raise(Slack::Web::Api::Errors::AccountInactive.new('Account disconnected')) + + allow(hook).to receive(:authorization_error!) + + builder.perform + expect(hook).to be_disabled + expect(hook).to have_received(:authorization_error!) + end end end end