Tidy up Slack provider implementation

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"

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

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)
+    data = {
-      attachments = record[:message][:attachments]
+      token: SiteSetting.chat_integration_slack_access_token,
-      attachments.concat message[:attachments]
+    }
 
-      uri = URI("https://slack.com/api/chat.update" +
+    req = Net::HTTP::Post.new(URI('https://slack.com/api/chat.postMessage'))
-        "?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
 
-    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
 

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