From 148dd8df6be474b63e0efdcfcf28db298f506020 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 21 Aug 2017 15:28:37 +0100 Subject: [PATCH] Tidy up Slack provider implementation --- config/locales/server.en.yml | 2 +- config/settings.yml | 1 - .../provider/slack/slack_provider.rb | 55 ++++++++----------- .../provider/slack/slack_provider_spec.rb | 12 ---- 4 files changed, 23 insertions(+), 47 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c9cb581..78b1416 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -13,7 +13,7 @@ en: chat_integration_slack_incoming_webhook_token: 'The verification token used to authenticate incoming requests' chat_integration_slack_excerpt_length: 'Slack post excerpt length' chat_integration_slack_icon_url: 'Icon to post to slack with (defaults to forum logo)' - chat_integration_slack_outbound_webhook_url: 'URL for outbound slack requests' + chat_integration_slack_outbound_webhook_url: "For using Slack's 'Incoming Webhook' system instead of of the OAuth API. Not recommended." errors: chat_integration_slack_api_configs_are_empty: "You must enter either an outbound webhook URL, or an access token" diff --git a/config/settings.yml b/config/settings.yml index 4be025a..d93f411 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -22,7 +22,6 @@ plugins: chat_integration_slack_outbound_webhook_url: default: '' regex: '^https:\/\/hooks\.slack\.com\/services\/.+$' - hidden: true chat_integration_slack_icon_url: default: '' hidden: true diff --git a/lib/discourse_chat/provider/slack/slack_provider.rb b/lib/discourse_chat/provider/slack/slack_provider.rb index 9e08c75..844dfee 100644 --- a/lib/discourse_chat/provider/slack/slack_provider.rb +++ b/lib/discourse_chat/provider/slack/slack_provider.rb @@ -51,19 +51,15 @@ module DiscourseChat::Provider::SlackProvider author_icon: post.user.small_avatar_url, color: topic.category ? "##{topic.category.color}" : nil, text: excerpt(post), - mrkdwn_in: ["text"] + mrkdwn_in: ["text"], + title: "#{topic.title} #{(category == '[uncategorized]') ? '' : category} #{topic.tags.present? ? topic.tags.map(&:name).join(', ') : ''}", + title_link: post.full_url, + thumb_url: post.full_url } - record = DiscourseChat.pstore_get("topic_#{post.topic.id}_#{channel}") - - if (SiteSetting.chat_integration_slack_access_token.empty? || post.is_first_post? || record.blank? || (record.present? && ((Time.now.to_i - record[:ts].split('.')[0].to_i) / 60) >= 5)) - summary[:title] = "#{topic.title} #{(category == '[uncategorized]') ? '' : category} #{topic.tags.present? ? topic.tags.map(&:name).join(', ') : ''}" - summary[:title_link] = post.full_url - summary[:thumb_url] = post.full_url - end - message[:attachments].push(summary) - message + + return message end def self.send_via_api(post, channel, message) @@ -74,29 +70,23 @@ module DiscourseChat::Provider::SlackProvider uri = "" record = DiscourseChat.pstore_get("slack_topic_#{post.topic.id}_#{channel}") - if (record.present? && ((Time.now.to_i - record[:ts].split('.')[0].to_i) / 60) < 5 && record[:message][:attachments].length < 5) - attachments = record[:message][:attachments] - attachments.concat message[:attachments] + data = { + token: SiteSetting.chat_integration_slack_access_token, + } - uri = URI("https://slack.com/api/chat.update" + - "?token=#{SiteSetting.chat_integration_slack_access_token}" + - "&username=#{CGI::escape(record[:message][:username])}" + - "&text=#{CGI::escape(record[:message][:text])}" + - "&channel=#{record[:channel]}" + - "&attachments=#{CGI::escape(attachments.to_json)}" + - "&ts=#{record[:ts]}" - ) - else - uri = URI("https://slack.com/api/chat.postMessage" + - "?token=#{SiteSetting.chat_integration_slack_access_token}" + - "&username=#{CGI::escape(message[:username])}" + - "&icon_url=#{CGI::escape(message[:icon_url])}" + - "&channel=#{ message[:channel].gsub('#', '') }" + - "&attachments=#{CGI::escape(message[:attachments].to_json)}" - ) - end + req = Net::HTTP::Post.new(URI('https://slack.com/api/chat.postMessage')) - response = http.request(Net::HTTP::Post.new(uri)) + data = { + token: SiteSetting.chat_integration_slack_access_token, + username: message[:username], + icon_url: message[:icon_url], + channel: message[:channel].gsub('#', ''), + attachments: message[:attachments].to_json + } + + req.set_form_data(data) + + response = http.request(req) unless response.kind_of? Net::HTTPSuccess raise ::DiscourseChat::ProviderError.new info: { request: uri, response_code: response.code, response_body: response.body } @@ -108,12 +98,11 @@ module DiscourseChat::Provider::SlackProvider if json.key?("error") && (json["error"] == ('channel_not_found') || json["error"] == ('is_archived')) error_key = 'chat_integration.provider.slack.errors.channel_not_found' else - error_key = json.to_s + error_key = nil end raise ::DiscourseChat::ProviderError.new info: { error_key: error_key, request: uri, response_code: response.code, response_body: response.body } end - DiscourseChat.pstore_set("slack_topic_#{post.topic.id}_#{channel}", JSON.parse(response.body)) response end diff --git a/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb b/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb index a3767ce..7cd773e 100644 --- a/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb @@ -94,18 +94,6 @@ RSpec.describe DiscourseChat::Provider::SlackProvider do expect(@stub2).to have_been_requested.once end - it 'correctly merges replies' do - second_post = Fabricate(:post, topic: post.topic, post_number: 2) - expect(@stub2).to have_been_requested.times(0) - expect(@stub3).to have_been_requested.times(0) - - described_class.trigger_notification(post, chan1) - described_class.trigger_notification(second_post, chan1) - expect(@stub1).to have_been_requested.times(0) - expect(@stub2).to have_been_requested.once # Initial creation of message - expect(@stub3).to have_been_requested.once # Requests to update the existing message - end - end end