From 26e8877cd967315ee6dd6937ae306998ded66750 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Sun, 8 Oct 2023 17:55:03 +0530 Subject: [PATCH] feat: Support link unfurling for all the channels within the same connected channel account. (#8033) --- Gemfile | 2 +- Gemfile.lock | 7 +- app/jobs/slack_unfurl_job.rb | 46 +++++++++- .../slack/incoming_message_builder.rb | 4 +- spec/jobs/slack_unfurl_job_spec.rb | 91 ++++++++++++++++--- .../slack/incoming_message_builder_spec.rb | 2 +- 6 files changed, 130 insertions(+), 22 deletions(-) diff --git a/Gemfile b/Gemfile index 72cd28ecd..f20569173 100644 --- a/Gemfile +++ b/Gemfile @@ -92,7 +92,7 @@ gem 'twitty', '~> 0.1.5' # facebook client gem 'koala' # slack client -gem 'slack-ruby-client', '~> 2.0.0' +gem 'slack-ruby-client', '~> 2.2.0' # for dialogflow integrations gem 'google-cloud-dialogflow-v2' gem 'grpc' diff --git a/Gemfile.lock b/Gemfile.lock index 78dcee76e..f18848492 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -273,7 +273,7 @@ GEM googleauth (~> 1.0) grpc (~> 1.36) geocoder (1.8.1) - gli (2.21.0) + gli (2.21.1) globalid (1.2.1) activesupport (>= 6.1) gmail_xoauth (0.4.2) @@ -732,13 +732,12 @@ GEM json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) - slack-ruby-client (2.0.0) + slack-ruby-client (2.2.0) faraday (>= 2.0) faraday-mashify faraday-multipart gli hashie - websocket-driver snaky_hash (2.0.1) hashie version_gem (~> 1.1, >= 1.1.1) @@ -935,7 +934,7 @@ DEPENDENCIES sidekiq (>= 7.1.3) sidekiq-cron (>= 1.10.1) simplecov (= 0.17.1) - slack-ruby-client (~> 2.0.0) + slack-ruby-client (~> 2.2.0) spring spring-watcher-listen squasher diff --git a/app/jobs/slack_unfurl_job.rb b/app/jobs/slack_unfurl_job.rb index 6829f7f49..c8d50ca2f 100644 --- a/app/jobs/slack_unfurl_job.rb +++ b/app/jobs/slack_unfurl_job.rb @@ -1,7 +1,49 @@ class SlackUnfurlJob < ApplicationJob queue_as :low - def perform(params, integration_hook) - Integrations::Slack::SlackLinkUnfurlService.new(params: params, integration_hook: integration_hook).perform + def perform(params) + @params = params + set_integration_hook + + return unless channel_has_access + + Integrations::Slack::SlackLinkUnfurlService.new(params: @params, integration_hook: @integration_hook).perform + end + + private + + # Find the integration hook by taking first link from array of links + # Assume that all the links are from the same account, how ever there is a possibility that the links are from different accounts. + # TODO: Fix this edge case later + def set_integration_hook + url = extract_url + return unless url + + account_id = extract_account_id(url) + @integration_hook = Integrations::Hook.find_by(account_id: account_id, app_id: 'slack') + end + + def extract_url + @params.dig(:event, :links)&.first&.[](:url) + end + + def extract_account_id(url) + account_id_regex = %r{/accounts/(\d+)} + match_data = url.match(account_id_regex) + match_data[1] if match_data + end + + # Check the channel has access to the bot to unfurl the links + def channel_has_access + return if @integration_hook.blank? + + slack_client = Slack::Web::Client.new(token: @integration_hook.access_token) + response = slack_client.conversations_members(channel: @params.dig(:event, :channel)) + response['ok'] + rescue Slack::Web::Api::Errors::ChannelNotFound => e + # The link unfurl event will not work for private channels and other accounts channels + # So we can ignore the error + Rails.logger.error "Exception in SlackUnfurlJob: #{e.message}" + false end end diff --git a/lib/integrations/slack/incoming_message_builder.rb b/lib/integrations/slack/incoming_message_builder.rb index 09d137a4a..97728d848 100644 --- a/lib/integrations/slack/incoming_message_builder.rb +++ b/lib/integrations/slack/incoming_message_builder.rb @@ -19,7 +19,7 @@ class Integrations::Slack::IncomingMessageBuilder elsif create_message? create_message elsif link_shared? - SlackUnfurlJob.perform_later(params, integration_hook) + SlackUnfurlJob.perform_later(params) end end @@ -72,7 +72,7 @@ class Integrations::Slack::IncomingMessageBuilder end def link_shared? - params[:event][:type] == 'link_shared' && integration_hook + params[:event][:type] == 'link_shared' end def message diff --git a/spec/jobs/slack_unfurl_job_spec.rb b/spec/jobs/slack_unfurl_job_spec.rb index e60f370ad..b9be64461 100644 --- a/spec/jobs/slack_unfurl_job_spec.rb +++ b/spec/jobs/slack_unfurl_job_spec.rb @@ -7,15 +7,22 @@ RSpec.describe SlackUnfurlJob do let(:hook) { create(:integrations_hook, account: account) } let(:inbox) { create(:inbox, account: account) } let!(:conversation) { create(:conversation, inbox: inbox) } - + let(:slack_client) { double } let(:link_shared) do { team_id: 'TLST3048H', api_app_id: 'A012S5UETV4', - event: link_shared_event.merge(links: [{ - url: "https://qa.chatwoot.com/app/accounts/1/conversations/#{conversation.display_id}", - domain: 'qa.chatwoot.com' - }]), + event: { + type: 'link_shared', + user: 'ULYPAKE5S', + source: 'conversations_history', + unfurl_id: 'C7NQEAE5Q.1695111587.937099.7e240338c6d2053fb49f56808e7c1f619f6ef317c39ebc59fc4af1cc30dce49b', + channel: 'G01354F6A6Q', + links: [{ + url: "https://qa.chatwoot.com/app/accounts/#{hook.account_id}/conversations/#{conversation.display_id}", + domain: 'qa.chatwoot.com' + }] + }, type: 'event_callback', event_time: 1_588_623_033 } @@ -26,13 +33,73 @@ RSpec.describe SlackUnfurlJob do .on_queue('low') end - it 'calls the SlackLinkUnfurlService' do - slack_link_unfurl_service = instance_double(Integrations::Slack::SlackLinkUnfurlService) + context 'when the calls the slack unfurl job' do + let(:slack_link_unfurl_service) { instance_double(Integrations::Slack::SlackLinkUnfurlService) } - expect(Integrations::Slack::SlackLinkUnfurlService).to receive(:new) - .with(params: link_shared, integration_hook: hook) - .and_return(slack_link_unfurl_service) - expect(slack_link_unfurl_service).to receive(:perform) - described_class.perform_now(link_shared, hook) + before do + allow(Integrations::Slack::SlackLinkUnfurlService).to receive(:new) + .with(params: link_shared, integration_hook: hook) + .and_return(slack_link_unfurl_service) + end + + context 'when the URL is shared in the channel' do + let(:expected_body) { { channel: link_shared[:event][:channel] } } + + before do + stub_request(:post, 'https://slack.com/api/conversations.members') + .with(body: expected_body) + .to_return(status: 200, body: { 'ok' => true }.to_json, headers: {}) + end + + it 'does the unfurl' do + expect(slack_link_unfurl_service).to receive(:perform) + described_class.perform_now(link_shared) + end + end + + context 'when the URL is shared in a different channel under the same account' do + let(:expected_body) { { channel: 'XSDSFSFS' } } + + before do + link_shared[:event][:channel] = 'XSDSFSFS' + stub_request(:post, 'https://slack.com/api/conversations.members') + .with(body: expected_body) + .to_return(status: 200, body: { 'ok' => true }.to_json, headers: {}) + end + + it 'does the unfurl' do + expect(slack_link_unfurl_service).to receive(:perform) + described_class.perform_now(link_shared) + end + end + + context 'when another account URL is shared' do + before do + link_shared[:event][:links][0][:url] = 'https://qa.chatwoot.com/app/accounts/123/conversations/123' + end + + it 'does not unfurl' do + expect(Integrations::Slack::SlackLinkUnfurlService).not_to receive(:new) + expect(slack_link_unfurl_service).not_to receive(:perform) + described_class.perform_now(link_shared) + end + end + + context 'when the URL is shared in a channel under a different account' do + let(:expected_body) { { channel: 'GDF4F6A6Q' } } + + before do + link_shared[:event][:channel] = 'GDF4F6A6Q' + stub_request(:post, 'https://slack.com/api/conversations.members') + .with(body: expected_body) + .to_return(status: 404, body: { 'ok' => false }.to_json, headers: {}) + end + + it 'does not unfurl' do + expect(Integrations::Slack::SlackLinkUnfurlService).not_to receive(:new) + expect(slack_link_unfurl_service).not_to receive(:perform) + described_class.perform_now(link_shared) + end + end end end diff --git a/spec/lib/integrations/slack/incoming_message_builder_spec.rb b/spec/lib/integrations/slack/incoming_message_builder_spec.rb index f1c1de7a5..5bb04c582 100644 --- a/spec/lib/integrations/slack/incoming_message_builder_spec.rb +++ b/spec/lib/integrations/slack/incoming_message_builder_spec.rb @@ -163,7 +163,7 @@ describe Integrations::Slack::IncomingMessageBuilder do it 'unfurls link' do builder = described_class.new(link_shared) - expect(SlackUnfurlJob).to receive(:perform_later).with(link_shared, hook) + expect(SlackUnfurlJob).to receive(:perform_later).with(link_shared) builder.perform end end