diff --git a/db/migrate/20211202120030_add_unique_index_to_slack_thread_ts.rb b/db/migrate/20211202120030_add_unique_index_to_slack_thread_ts.rb new file mode 100644 index 0000000..15a6af1 --- /dev/null +++ b/db/migrate/20211202120030_add_unique_index_to_slack_thread_ts.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddUniqueIndexToSlackThreadTs < ActiveRecord::Migration[6.1] + def up + add_index :topic_custom_fields, [:topic_id, :name], + unique: true, + where: "(name LIKE 'slack_thread_id_%')", + name: "index_topic_custom_fields_on_topic_id_and_slack_thread_id" + end +end diff --git a/lib/discourse_chat_integration/provider/slack/slack_provider.rb b/lib/discourse_chat_integration/provider/slack/slack_provider.rb index d38a41a..be129eb 100644 --- a/lib/discourse_chat_integration/provider/slack/slack_provider.rb +++ b/lib/discourse_chat_integration/provider/slack/slack_provider.rb @@ -1,8 +1,15 @@ # frozen_string_literal: true module DiscourseChatIntegration::Provider::SlackProvider - PROVIDER_NAME = "slack".freeze - THREAD = "thread".freeze + PROVIDER_NAME = "slack" + THREAD_CUSTOM_FIELD_PREFIX = "slack_thread_id_" + + # In the past, only one thread_ts was stored for each topic. + # Now, we store one thread_ts per Slack channel. + # Data will be automatically migrated when the next message is sent to the channel + # This logic could be removed after 2022-12 - it's unlikely people will care about + # threading messages to more-than-1-year-old Slack threads. + THREAD_LEGACY = "thread" PROVIDER_ENABLED_SETTING = :chat_integration_slack_enabled @@ -11,16 +18,7 @@ module DiscourseChatIntegration::Provider::SlackProvider ] require_dependency 'topic' - ::Topic.register_custom_field_type(DiscourseChatIntegration::Provider::SlackProvider::THREAD, :text) - - class ::Topic - def slack_thread_id=(ts) - self.custom_fields[DiscourseChatIntegration::Provider::SlackProvider::THREAD] = ts - end - def slack_thread_id - self.custom_fields[DiscourseChatIntegration::Provider::SlackProvider::THREAD] - end - end + ::Topic.register_custom_field_type(DiscourseChatIntegration::Provider::SlackProvider::THREAD_LEGACY, :string) def self.excerpt(post, max_length = SiteSetting.chat_integration_slack_excerpt_length) doc = Nokogiri::HTML5.fragment(post.excerpt(max_length, @@ -69,8 +67,8 @@ module DiscourseChatIntegration::Provider::SlackProvider attachments: [] } - if filter == "thread" && thread_ts = topic.slack_thread_id - message[:thread_ts] = thread_ts if not thread_ts.nil? + if filter == "thread" && thread_ts = get_slack_thread_ts(topic, channel) + message[:thread_ts] = thread_ts end summary = { @@ -97,7 +95,7 @@ module DiscourseChatIntegration::Provider::SlackProvider uri = "" # - slack_thread_regex = // + slack_thread_regex = // req = Net::HTTP::Post.new(URI('https://slack.com/api/chat.postMessage')) @@ -110,10 +108,9 @@ module DiscourseChatIntegration::Provider::SlackProvider } if message.key?(:thread_ts) data[:thread_ts] = message[:thread_ts] - elsif match = slack_thread_regex.match(post.raw) + elsif (match = slack_thread_regex.match(post.raw)) && match.captures[0] == channel data[:thread_ts] = match.captures[1] - post.topic.slack_thread_id = match.captures[1] - post.topic.save_custom_fields + set_slack_thread_ts(post.topic, channel, match.captures[1]) end req.set_form_data(data) @@ -136,10 +133,7 @@ module DiscourseChatIntegration::Provider::SlackProvider end ts = json["ts"] - if !ts.nil? && post.topic.slack_thread_id.nil? - post.topic.slack_thread_id = ts - post.topic.save_custom_fields - end + set_slack_thread_ts(post.topic, channel, ts) if !ts.nil? response end @@ -183,6 +177,21 @@ module DiscourseChatIntegration::Provider::SlackProvider http.read_timeout = 5 # seconds http end + + def self.get_slack_thread_ts(topic, channel) + field = TopicCustomField.where(topic: topic, name: "#{THREAD_CUSTOM_FIELD_PREFIX}#{channel}") + field.pluck_first(:value) || topic.custom_fields[THREAD_LEGACY] + end + + def self.set_slack_thread_ts(topic, channel, value) + TopicCustomField.upsert({ + topic_id: topic.id, + name: "#{THREAD_CUSTOM_FIELD_PREFIX}#{channel}", + value: value, + created_at: Time.zone.now, + updated_at: Time.zone.now + }, unique_by: [:topic_id, :name]) + end end require_relative "slack_message_formatter" diff --git a/spec/lib/discourse_chat_integration/provider/slack/slack_provider_spec.rb b/spec/lib/discourse_chat_integration/provider/slack/slack_provider_spec.rb index 792fbbf..2a3c425 100644 --- a/spec/lib/discourse_chat_integration/provider/slack/slack_provider_spec.rb +++ b/spec/lib/discourse_chat_integration/provider/slack/slack_provider_spec.rb @@ -90,10 +90,11 @@ RSpec.describe DiscourseChatIntegration::Provider::SlackProvider do before do SiteSetting.chat_integration_slack_access_token = "magic" @ts = "#{Time.now.to_i}.012345" + @ts2 = "#{Time.now.to_i}.012346" @stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(body: "success") - @thread_stub = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).with(body: hash_including("thread_ts" => @ts)).to_return(body: "{\"ok\":true, \"ts\": \"12345.67890\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }", headers: { 'Content-Type' => 'application/json' }) @stub2 = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).to_return(body: "{\"ok\":true, \"ts\": \"#{@ts}\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }", headers: { 'Content-Type' => 'application/json' }) - @channel = DiscourseChatIntegration::Channel.create(provider: 'dummy') + @thread_stub = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).with(body: hash_including("thread_ts" => @ts)).to_return(body: "{\"ok\":true, \"ts\": \"#{@ts}\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }", headers: { 'Content-Type' => 'application/json' }) + @thread_stub2 = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).with(body: hash_including("thread_ts" => @ts2)).to_return(body: "{\"ok\":true, \"ts\": \"#{@ts2}\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }", headers: { 'Content-Type' => 'application/json' }) end it 'sends an api request' do @@ -103,32 +104,50 @@ RSpec.describe DiscourseChatIntegration::Provider::SlackProvider do described_class.trigger_notification(post, chan1, nil) expect(@stub1).to have_been_requested.times(0) expect(@stub2).to have_been_requested.once - expect(post.topic.slack_thread_id).to eq(@ts) + expect(described_class.get_slack_thread_ts(post.topic, chan1.data["identifier"])).to eq(@ts) expect(@thread_stub).to have_been_requested.times(0) end it 'sends thread id for thread' do expect(@thread_stub).to have_been_requested.times(0) - rule = DiscourseChatIntegration::Rule.create(channel: @channel, filter: "thread") - post.topic.slack_thread_id = @ts + rule = DiscourseChatIntegration::Rule.create(channel: chan1, filter: "thread") + described_class.set_slack_thread_ts(post.topic, chan1.data["identifier"], @ts) described_class.trigger_notification(post, chan1, rule) expect(@thread_stub).to have_been_requested.once end + it 'tracks threading in different channels separately' do + expect(@thread_stub).to have_been_requested.times(0) + chan2 = DiscourseChatIntegration::Channel.create(provider: 'dummy2', data: { "identifier" => "#random" }) + + rule = DiscourseChatIntegration::Rule.create(channel: chan1, filter: "thread") + rule2 = DiscourseChatIntegration::Rule.create(channel: chan2, filter: "thread") + described_class.set_slack_thread_ts(post.topic, chan1.data["identifier"], @ts) + described_class.set_slack_thread_ts(post.topic, chan2.data["identifier"], @ts2) + + described_class.trigger_notification(post, chan1, rule) + described_class.trigger_notification(post, chan2, rule2) + expect(@thread_stub).to have_been_requested.once + expect(@thread_stub2).to have_been_requested.once + + post.topic.reload + expect(described_class.get_slack_thread_ts(post.topic, "#general")).to eq(@ts) + expect(described_class.get_slack_thread_ts(post.topic, "#random")).to eq(@ts2) + end + it 'recognizes slack thread ts in comment' do post.update!(cooked: "cooked", raw: <<~RAW My fingers are typing words that improve `raw_quality` - + RAW ) - rule = DiscourseChatIntegration::Rule.create(channel: @channel, filter: "thread") - post.topic.slack_thread_id = nil + rule = DiscourseChatIntegration::Rule.create(channel: chan1, filter: "thread") described_class.trigger_notification(post, chan1, rule) - expect(post.topic.slack_thread_id).to eq(@ts) + expect(described_class.get_slack_thread_ts(post.topic, chan1.data["identifier"])).to eq(@ts) expect(@thread_stub).to have_been_requested.times(1) end