From 0522ad64143c9aedb27e00b642e82cad1273c83b Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Thu, 26 Jan 2023 12:18:48 +0000
Subject: [PATCH] FIX: Always use parent thread_ts for slack threads (#159)

Previously we were using the `ts` of the previous message we sent to the thread. While this did work under some situations, it's not recommended in the Slack API docs, and can lead to some unexpected behavior (e.g. when one of the threaded messages is deleted).

This commit updates our logic to always use Slack's returned `thread_ts`, which represents the thread's parent.
---
 .../provider/slack/slack_provider.rb                     | 2 +-
 .../provider/slack/slack_provider_spec.rb                | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/discourse_chat_integration/provider/slack/slack_provider.rb b/lib/discourse_chat_integration/provider/slack/slack_provider.rb
index 4390deb..c80a06d 100644
--- a/lib/discourse_chat_integration/provider/slack/slack_provider.rb
+++ b/lib/discourse_chat_integration/provider/slack/slack_provider.rb
@@ -144,7 +144,7 @@ module DiscourseChatIntegration::Provider::SlackProvider
                                                           }
     end
 
-    ts = json["ts"]
+    ts = json.dig("message", "thread_ts") || json["ts"]
     set_slack_thread_ts(post.topic, channel, ts) if !ts.nil? && !post.nil?
 
     response
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 856c703..04e2fc4 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
@@ -99,6 +99,7 @@ RSpec.describe DiscourseChatIntegration::Provider::SlackProvider do
         SiteSetting.chat_integration_slack_access_token = "magic"
         @ts = "#{Time.now.to_i}.012345"
         @ts2 = "#{Time.now.to_i}.012346"
+        @ts3 = "#{Time.now.to_i}.0123467"
         @stub1 =
           stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(
             body: "success",
@@ -116,17 +117,17 @@ RSpec.describe DiscourseChatIntegration::Provider::SlackProvider do
             body: hash_including("thread_ts" => @ts),
           ).to_return(
             body:
-              "{\"ok\":true, \"ts\": \"#{@ts}\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }",
+              "{\"ok\":true, \"ts\": \"#{@ts}\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\", \"thread_ts\":\"#{@ts}\"} }",
             headers: {
               "Content-Type" => "application/json",
             },
           )
         @thread_stub2 =
           stub_request(:post, %r{https://slack.com/api/chat.postMessage}).with(
-            body: hash_including("thread_ts" => @ts2),
+            body: hash_including("thread_ts" => @ts),
           ).to_return(
             body:
-              "{\"ok\":true, \"ts\": \"#{@ts2}\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }",
+              "{\"ok\":true, \"ts\": \"#{@ts3}\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\", \"thread_ts\":\"#{@ts}\"} }",
             headers: {
               "Content-Type" => "application/json",
             },
@@ -176,7 +177,7 @@ RSpec.describe DiscourseChatIntegration::Provider::SlackProvider do
 
         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)
+        expect(described_class.get_slack_thread_ts(post.topic, "#random")).to eq(@ts)
       end
 
       it "recognizes slack thread ts in comment" do