From da9106127afea45a72b8dcc8b944b8cbfc1b8fd1 Mon Sep 17 00:00:00 2001 From: Michael K Johnson Date: Mon, 15 Jun 2020 11:45:25 -0400 Subject: [PATCH] FEATURE: Enable optional support for threading slack posts (#38) When creating a new Discourse post from slack with the `post` feature, record the slack `ts` thread ID for the resulting topic post using an HTML comment to pass the `ts` through. When notifying slack of new Discourse posts, record the slack `ts` thread ID in the post's topic if it has not yet been recorded. (Normally, this will be done for the topic post, except where notifications are being posted for old topics before this feature was created.) Add a new rule filter `thread` which posts threaded responses to slack if there is a `ts` recorded for the post topic. Modify the `trigger_notifications` interface to enable other integrations to implement similar functionality. Present the `thread` rule in the help text and admin UI only for the slack providers. https://meta.discourse.org/t/optionally-threading-posts-to-parent-topic-in-slack-integration/150759 --- app/controllers/chat_controller.rb | 2 +- app/helpers/helper.rb | 2 +- app/models/rule.rb | 7 +-- app/services/manager.rb | 4 +- assets/javascripts/admin/models/rule.js.es6 | 47 ++++++++++++------- .../routes/admin-plugins-chat-provider.js.es6 | 2 +- config/locales/client.en.yml | 1 + config/locales/server.en.yml | 3 +- .../provider/discord/discord_provider.rb | 2 +- .../provider/flowdock/flowdock_provider.rb | 2 +- .../provider/gitter/gitter_provider.rb | 2 +- .../provider/groupme/groupme_provider.rb | 2 +- .../provider/matrix/matrix_provider.rb | 2 +- .../mattermost/mattermost_provider.rb | 2 +- .../rocketchat/rocketchat_provider.rb | 2 +- .../provider/slack/slack_provider.rb | 42 ++++++++++++++--- .../provider/slack/slack_transcript.rb | 4 ++ .../provider/telegram/telegram_provider.rb | 2 +- .../provider/zulip/zulip_provider.rb | 2 +- plugin.rb | 1 + spec/dummy_provider.rb | 4 +- spec/helpers/helper_spec.rb | 7 ++- .../provider/discord/discord_provider_spec.rb | 6 +-- .../flowdock/flowdock_provider_spec.rb | 4 +- .../provider/gitter/gitter_provider_spec.rb | 4 +- .../groupme/groupme_provider_spect.rb | 4 +- .../provider/matrix/matrix_provider_spec.rb | 4 +- .../mattermost/mattermost_provider_spec.rb | 4 +- .../rocketchat/rocketchat_provider_spec.rb | 4 +- .../provider/slack/slack_provider_spec.rb | 41 +++++++++++++--- .../provider/slack/slack_transcript_spec.rb | 36 +++++++++++--- .../telegram/telegram_provider_spec.rb | 4 +- .../provider/zulip/zulip_provider_spec.rb | 4 +- spec/models/rule_spec.rb | 9 ++-- spec/services/manager_spec.rb | 11 ++++- 35 files changed, 200 insertions(+), 79 deletions(-) diff --git a/app/controllers/chat_controller.rb b/app/controllers/chat_controller.rb index 1312671..ca29495 100644 --- a/app/controllers/chat_controller.rb +++ b/app/controllers/chat_controller.rb @@ -34,7 +34,7 @@ class DiscourseChat::ChatController < ApplicationController post = Topic.find(topic_id.to_i).posts.first - provider.trigger_notification(post, channel) + provider.trigger_notification(post, channel, nil) render json: success_json rescue Discourse::InvalidParameters, ActiveRecord::RecordNotFound => e diff --git a/app/helpers/helper.rb b/app/helpers/helper.rb index e09cb9c..3625ffb 100644 --- a/app/helpers/helper.rb +++ b/app/helpers/helper.rb @@ -13,7 +13,7 @@ module DiscourseChat error_text = I18n.t("chat_integration.provider.#{provider}.parse_error") case cmd - when "watch", "follow", "mute" + when "thread", "watch", "follow", "mute" return error_text if tokens.empty? # If the first token in the command is a tag, this rule applies to all categories category_name = tokens[0].start_with?('tag:') ? nil : tokens.shift diff --git a/app/models/rule.rb b/app/models/rule.rb index c74edba..cb22cd8 100644 --- a/app/models/rule.rb +++ b/app/models/rule.rb @@ -31,15 +31,16 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel " CASE WHEN value::json->>'filter' = 'mute' THEN 1 - WHEN value::json->>'filter' = 'watch' THEN 2 - WHEN value::json->>'filter' = 'follow' THEN 3 + WHEN value::json->>'filter' = 'thread' THEN 2 + WHEN value::json->>'filter' = 'watch' THEN 3 + WHEN value::json->>'filter' = 'follow' THEN 4 END ") } after_initialize :init_filter - validates :filter, inclusion: { in: %w(watch follow mute), + validates :filter, inclusion: { in: %w(thread watch follow mute), message: "%{value} is not a valid filter" } validates :type, inclusion: { in: %w(normal group_message group_mention), diff --git a/app/services/manager.rb b/app/services/manager.rb index dd52a1b..f398fe6 100644 --- a/app/services/manager.rb +++ b/app/services/manager.rb @@ -52,7 +52,7 @@ module DiscourseChat # Sort by order of precedence t_prec = { 'group_message' => 0, 'group_mention' => 1, 'normal' => 2 } # Group things win - f_prec = { 'mute' => 0, 'watch' => 1, 'follow' => 2 } #(mute always wins; watch beats follow) + f_prec = { 'mute' => 0, 'thread' => 1, 'watch' => 2, 'follow' => 3 } #(mute always wins; thread beats watch beats follow) sort_func = proc { |a, b| [t_prec[a.type], f_prec[a.filter]] <=> [t_prec[b.type], f_prec[b.filter]] } matching_rules = matching_rules.sort(&sort_func) @@ -80,7 +80,7 @@ module DiscourseChat next unless is_enabled = ::DiscourseChat::Provider.is_enabled(provider) begin - provider.trigger_notification(post, channel) + provider.trigger_notification(post, channel, rule) channel.update_attribute('error_key', nil) if channel.error_key rescue => e if e.class == (DiscourseChat::ProviderError) && e.info.key?(:error_key) && !e.info[:error_key].nil? diff --git a/assets/javascripts/admin/models/rule.js.es6 b/assets/javascripts/admin/models/rule.js.es6 index 19346dc..ce976c6 100644 --- a/assets/javascripts/admin/models/rule.js.es6 +++ b/assets/javascripts/admin/models/rule.js.es6 @@ -6,23 +6,38 @@ import { } from "discourse-common/utils/decorators"; export default RestModel.extend({ - available_filters: [ - { - id: "watch", - name: I18n.t("chat_integration.filter.watch"), - icon: "exclamation-circle" - }, - { - id: "follow", - name: I18n.t("chat_integration.filter.follow"), - icon: "circle" - }, - { - id: "mute", - name: I18n.t("chat_integration.filter.mute"), - icon: "times-circle" + @computed("channel.provider") + available_filters(provider) { + const available = []; + + if (provider === "slack") { + available.push({ + id: "thread", + name: I18n.t("chat_integration.filter.thread"), + icon: "chevron-right" + }); } - ], + + available.push( + { + id: "watch", + name: I18n.t("chat_integration.filter.watch"), + icon: "exclamation-circle" + }, + { + id: "follow", + name: I18n.t("chat_integration.filter.follow"), + icon: "circle" + }, + { + id: "mute", + name: I18n.t("chat_integration.filter.mute"), + icon: "times-circle" + } + ); + + return available; + }, available_types: [ { id: "normal", name: I18n.t("chat_integration.type.normal") }, diff --git a/assets/javascripts/admin/routes/admin-plugins-chat-provider.js.es6 b/assets/javascripts/admin/routes/admin-plugins-chat-provider.js.es6 index f4e2452..1993976 100644 --- a/assets/javascripts/admin/routes/admin-plugins-chat-provider.js.es6 +++ b/assets/javascripts/admin/routes/admin-plugins-chat-provider.js.es6 @@ -18,7 +18,7 @@ export default DiscourseRoute.extend({ "rules", channel.rules.map(rule => { rule = this.store.createRecord("rule", rule); - rule.channel = channel; + rule.set("channel", channel); return rule; }) ); diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 7467d83..3f3604d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -36,6 +36,7 @@ en: mute: 'Mute' follow: 'First post only' watch: 'All posts and replies' + thread: 'All posts with threaded replies' rule_table: filter: "Filter" category: "Category" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 860e5d2..bcc9098 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -122,8 +122,9 @@ en: tag: "The *%{name}* tag cannot be found." category: "The *%{name}* category cannot be found. Available categories: *%{list}*" help: | - *New rule:* `/discourse [watch|follow|mute] [category] [tag:name]` + *New rule:* `/discourse [thread|watch|follow|mute] [category] [tag:name]` (you must specify a rule type and at least one category or tag) + - *thread* – notify this channel for new topics, thread replies if possible - *watch* – notify this channel for new topics and new replies - *follow* – notify this channel for new topics - *mute* – block notifications to this channel diff --git a/lib/discourse_chat/provider/discord/discord_provider.rb b/lib/discourse_chat/provider/discord/discord_provider.rb index 58bd3e2..439dbab 100644 --- a/lib/discourse_chat/provider/discord/discord_provider.rb +++ b/lib/discourse_chat/provider/discord/discord_provider.rb @@ -63,7 +63,7 @@ module DiscourseChat message end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) # Adding ?wait=true means that we actually get a success/failure response, rather than returning asynchronously webhook_url = "#{channel.data['webhook_url']}?wait=true" message = generate_discord_message(post) diff --git a/lib/discourse_chat/provider/flowdock/flowdock_provider.rb b/lib/discourse_chat/provider/flowdock/flowdock_provider.rb index d6e031c..89a5005 100644 --- a/lib/discourse_chat/provider/flowdock/flowdock_provider.rb +++ b/lib/discourse_chat/provider/flowdock/flowdock_provider.rb @@ -48,7 +48,7 @@ module DiscourseChat::Provider::FlowdockProvider message end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) flow_token = channel.data["flow_token"] message = generate_flowdock_message(post, flow_token) response = send_message("https://api.flowdock.com/messages", message) diff --git a/lib/discourse_chat/provider/gitter/gitter_provider.rb b/lib/discourse_chat/provider/gitter/gitter_provider.rb index b1e0d3a..50293f2 100644 --- a/lib/discourse_chat/provider/gitter/gitter_provider.rb +++ b/lib/discourse_chat/provider/gitter/gitter_provider.rb @@ -10,7 +10,7 @@ module DiscourseChat { key: "webhook_url", regex: '^https://webhooks\.gitter\.im/e/\S+$', unique: true, hidden: true } ] - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) message = gitter_message(post) response = Net::HTTP.post_form(URI(channel.data['webhook_url']), message: message) unless response.kind_of? Net::HTTPSuccess diff --git a/lib/discourse_chat/provider/groupme/groupme_provider.rb b/lib/discourse_chat/provider/groupme/groupme_provider.rb index 03213da..ecea5ac 100644 --- a/lib/discourse_chat/provider/groupme/groupme_provider.rb +++ b/lib/discourse_chat/provider/groupme/groupme_provider.rb @@ -71,7 +71,7 @@ module DiscourseChat::Provider::GroupmeProvider end end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) data_package = generate_groupme_message(post) self.send_via_webhook(data_package, channel) end diff --git a/lib/discourse_chat/provider/matrix/matrix_provider.rb b/lib/discourse_chat/provider/matrix/matrix_provider.rb index 05e7595..c70af25 100644 --- a/lib/discourse_chat/provider/matrix/matrix_provider.rb +++ b/lib/discourse_chat/provider/matrix/matrix_provider.rb @@ -56,7 +56,7 @@ module DiscourseChat message end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) message = generate_matrix_message(post) response = send_message(channel.data['room_id'], message) diff --git a/lib/discourse_chat/provider/mattermost/mattermost_provider.rb b/lib/discourse_chat/provider/mattermost/mattermost_provider.rb index 724b068..07c65a3 100644 --- a/lib/discourse_chat/provider/mattermost/mattermost_provider.rb +++ b/lib/discourse_chat/provider/mattermost/mattermost_provider.rb @@ -75,7 +75,7 @@ module DiscourseChat message end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) channel_id = channel.data['identifier'] message = mattermost_message(post, channel_id) diff --git a/lib/discourse_chat/provider/rocketchat/rocketchat_provider.rb b/lib/discourse_chat/provider/rocketchat/rocketchat_provider.rb index f35219e..3c7abcf 100644 --- a/lib/discourse_chat/provider/rocketchat/rocketchat_provider.rb +++ b/lib/discourse_chat/provider/rocketchat/rocketchat_provider.rb @@ -68,7 +68,7 @@ module DiscourseChat::Provider::RocketchatProvider end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) channel_id = channel.data['identifier'] message = rocketchat_message(post, channel_id) diff --git a/lib/discourse_chat/provider/slack/slack_provider.rb b/lib/discourse_chat/provider/slack/slack_provider.rb index 802f477..d351354 100644 --- a/lib/discourse_chat/provider/slack/slack_provider.rb +++ b/lib/discourse_chat/provider/slack/slack_provider.rb @@ -2,6 +2,7 @@ module DiscourseChat::Provider::SlackProvider PROVIDER_NAME = "slack".freeze + THREAD = "thread".freeze PROVIDER_ENABLED_SETTING = :chat_integration_slack_enabled @@ -9,6 +10,18 @@ module DiscourseChat::Provider::SlackProvider { key: "identifier", regex: '^[@#]?\S*$', unique: true } ] + require_dependency 'topic' + ::Topic.register_custom_field_type(DiscourseChat::Provider::SlackProvider::THREAD, :text) + + class ::Topic + def slack_thread_id=(ts) + self.custom_fields[DiscourseChat::Provider::SlackProvider::THREAD] = ts + end + def slack_thread_id + self.custom_fields[DiscourseChat::Provider::SlackProvider::THREAD] + end + end + def self.excerpt(post, max_length = SiteSetting.chat_integration_slack_excerpt_length) doc = Nokogiri::HTML.fragment(post.excerpt(max_length, remap_emoji: true, @@ -18,7 +31,7 @@ module DiscourseChat::Provider::SlackProvider SlackMessageFormatter.format(doc.to_html) end - def self.slack_message(post, channel) + def self.slack_message(post, channel, filter) display_name = "@#{post.user.username}" full_name = post.user.name || "" @@ -56,6 +69,10 @@ module DiscourseChat::Provider::SlackProvider attachments: [] } + if filter == "thread" && thread_ts = topic.slack_thread_id + message[:thread_ts] = thread_ts if not thread_ts.nil? + end + summary = { fallback: "#{topic.title} - #{display_name}", author_name: display_name, @@ -79,11 +96,9 @@ module DiscourseChat::Provider::SlackProvider response = nil uri = "" - record = DiscourseChat.pstore_get("slack_topic_#{post.topic.id}_#{channel}") - data = { - token: SiteSetting.chat_integration_slack_access_token, - } + # + slack_thread_regex = // req = Net::HTTP::Post.new(URI('https://slack.com/api/chat.postMessage')) @@ -94,6 +109,12 @@ module DiscourseChat::Provider::SlackProvider channel: message[:channel].gsub('#', ''), attachments: message[:attachments].to_json } + if message.key?(:thread_ts) + data[:thread_ts] = message[:thread_ts] + elsif match = slack_thread_regex.match(post.raw) + post.topic.slack_thread_id = match.captures[1] + post.topic.save_custom_fields + end req.set_form_data(data) @@ -114,6 +135,12 @@ module DiscourseChat::Provider::SlackProvider raise ::DiscourseChat::ProviderError.new info: { error_key: error_key, request: uri, response_code: response.code, response_body: response.body } 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 + response end @@ -137,9 +164,10 @@ module DiscourseChat::Provider::SlackProvider end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) channel_id = channel.data['identifier'] - message = slack_message(post, channel_id) + filter = rule.nil? ? "" : rule.filter + message = slack_message(post, channel_id, filter) if SiteSetting.chat_integration_slack_access_token.empty? self.send_via_webhook(message) diff --git a/lib/discourse_chat/provider/slack/slack_transcript.rb b/lib/discourse_chat/provider/slack/slack_transcript.rb index a69c6a1..803a01f 100644 --- a/lib/discourse_chat/provider/slack/slack_transcript.rb +++ b/lib/discourse_chat/provider/slack/slack_transcript.rb @@ -118,6 +118,10 @@ module DiscourseChat::Provider::SlackProvider post_content << "[#{username}]: #{url}\n" end + if not @requested_thread_ts.nil? + post_content << "" + end + post_content end diff --git a/lib/discourse_chat/provider/telegram/telegram_provider.rb b/lib/discourse_chat/provider/telegram/telegram_provider.rb index 6071fec..c8d4d7e 100644 --- a/lib/discourse_chat/provider/telegram/telegram_provider.rb +++ b/lib/discourse_chat/provider/telegram/telegram_provider.rb @@ -79,7 +79,7 @@ module DiscourseChat end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) chat_id = channel.data['chat_id'] message = { diff --git a/lib/discourse_chat/provider/zulip/zulip_provider.rb b/lib/discourse_chat/provider/zulip/zulip_provider.rb index 1141807..877584d 100644 --- a/lib/discourse_chat/provider/zulip/zulip_provider.rb +++ b/lib/discourse_chat/provider/zulip/zulip_provider.rb @@ -46,7 +46,7 @@ module DiscourseChat } end - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) stream = channel.data['stream'] subject = channel.data['subject'] diff --git a/plugin.rb b/plugin.rb index 664d211..780e557 100644 --- a/plugin.rb +++ b/plugin.rb @@ -11,6 +11,7 @@ enabled_site_setting :chat_integration_enabled register_asset "stylesheets/chat-integration-admin.scss" register_svg_icon "rocket" if respond_to?(:register_svg_icon) +register_svg_icon "fa-arrow-circle-o-right" if respond_to?(:register_svg_icon) # Site setting validators must be loaded before initialize require_relative "lib/discourse_chat/provider/slack/slack_enabled_setting_validator" diff --git a/spec/dummy_provider.rb b/spec/dummy_provider.rb index 67aecea..1b6c1d5 100644 --- a/spec/dummy_provider.rb +++ b/spec/dummy_provider.rb @@ -10,7 +10,7 @@ RSpec.shared_context "dummy provider" do @@sent_messages = [] @@raise_exception = nil - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) if @@raise_exception raise @@raise_exception end @@ -50,7 +50,7 @@ RSpec.shared_context "validated dummy provider" do @@sent_messages = [] - def self.trigger_notification(post, channel) + def self.trigger_notification(post, channel, rule) @@sent_messages.push(post: post.id, channel: channel) end diff --git a/spec/helpers/helper_spec.rb b/spec/helpers/helper_spec.rb index c31a7ca..57eaa4c 100644 --- a/spec/helpers/helper_spec.rb +++ b/spec/helpers/helper_spec.rb @@ -32,7 +32,12 @@ RSpec.describe DiscourseChat::Manager do expect(rule.tags).to eq(nil) end - it 'should work with all three filter types' do + it 'should work with all four filter types' do + response = DiscourseChat::Helper.process_command(chan1, ['thread', category.slug]) + + rule = DiscourseChat::Rule.all.first + expect(rule.filter).to eq('thread') + response = DiscourseChat::Helper.process_command(chan1, ['watch', category.slug]) rule = DiscourseChat::Rule.all.first diff --git a/spec/lib/discourse_chat/provider/discord/discord_provider_spec.rb b/spec/lib/discourse_chat/provider/discord/discord_provider_spec.rb index 5854648..909fbda 100644 --- a/spec/lib/discourse_chat/provider/discord/discord_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/discord/discord_provider_spec.rb @@ -14,7 +14,7 @@ RSpec.describe DiscourseChat::Provider::DiscordProvider do it 'sends a webhook request' do stub1 = stub_request(:post, 'https://discordapp.com/api/webhooks/1234/abcd?wait=true').to_return(status: 200) - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end @@ -22,14 +22,14 @@ RSpec.describe DiscourseChat::Provider::DiscordProvider do stub1 = stub_request(:post, 'https://discordapp.com/api/webhooks/1234/abcd?wait=true') .with(body: hash_including(embeds: [hash_including(author: hash_including(url: /^https?:\/\//))])) .to_return(status: 200) - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, "https://discordapp.com/api/webhooks/1234/abcd?wait=true").to_return(status: 400) expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end diff --git a/spec/lib/discourse_chat/provider/flowdock/flowdock_provider_spec.rb b/spec/lib/discourse_chat/provider/flowdock/flowdock_provider_spec.rb index 4eec7aa..c87b073 100644 --- a/spec/lib/discourse_chat/provider/flowdock/flowdock_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/flowdock/flowdock_provider_spec.rb @@ -14,14 +14,14 @@ RSpec.describe DiscourseChat::Provider::FlowdockProvider do it 'sends a request' do stub1 = stub_request(:post, "https://api.flowdock.com/messages").to_return(status: 200) - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, "https://api.flowdock.com/messages").to_return(status: 404, body: "{ \"error\": \"Not Found\"}") expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end end diff --git a/spec/lib/discourse_chat/provider/gitter/gitter_provider_spec.rb b/spec/lib/discourse_chat/provider/gitter/gitter_provider_spec.rb index 4b821b3..a7e84d2 100644 --- a/spec/lib/discourse_chat/provider/gitter/gitter_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/gitter/gitter_provider_spec.rb @@ -14,14 +14,14 @@ RSpec.describe DiscourseChat::Provider::GitterProvider do it 'sends a webhook request' do stub1 = stub_request(:post, chan1.data['webhook_url']).to_return(body: "OK") - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, chan1.data['webhook_url']).to_return(status: 404, body: "{ \"error\": \"Not Found\"}") expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end end diff --git a/spec/lib/discourse_chat/provider/groupme/groupme_provider_spect.rb b/spec/lib/discourse_chat/provider/groupme/groupme_provider_spect.rb index 19497a8..44db878 100644 --- a/spec/lib/discourse_chat/provider/groupme/groupme_provider_spect.rb +++ b/spec/lib/discourse_chat/provider/groupme/groupme_provider_spect.rb @@ -15,14 +15,14 @@ RSpec.describe DiscourseChat::Provider::GroupmeProvider do it 'sends a request' do stub1 = stub_request(:post, "https://api.groupme.com/v3/bots/post").to_return(status: 200) - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, "https://api.groupme.com/v3/bots/post").to_return(status: 404, body: "{ \"error\": \"Not Found\"}") expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end end diff --git a/spec/lib/discourse_chat/provider/matrix/matrix_provider_spec.rb b/spec/lib/discourse_chat/provider/matrix/matrix_provider_spec.rb index 1bcee40..85d5351 100644 --- a/spec/lib/discourse_chat/provider/matrix/matrix_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/matrix/matrix_provider_spec.rb @@ -15,14 +15,14 @@ RSpec.describe DiscourseChat::Provider::MatrixProvider do it 'sends the message' do stub1 = stub_request(:put, %r{https://matrix.org/_matrix/client/r0/rooms/!blah:matrix.org/send/m.room.message/*}).to_return(status: 200) - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:put, %r{https://matrix.org/_matrix/client/r0/rooms/!blah:matrix.org/send/m.room.message/*}).to_return(status: 400, body: '{"errmsg":"M_UNKNOWN"}') expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end diff --git a/spec/lib/discourse_chat/provider/mattermost/mattermost_provider_spec.rb b/spec/lib/discourse_chat/provider/mattermost/mattermost_provider_spec.rb index cb4f525..2c2419b 100644 --- a/spec/lib/discourse_chat/provider/mattermost/mattermost_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/mattermost/mattermost_provider_spec.rb @@ -18,7 +18,7 @@ RSpec.describe DiscourseChat::Provider::MattermostProvider do it 'sends a webhook request' do stub1 = stub_request(:post, 'https://mattermost.blah/hook/abcd').to_return(status: 200) - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end @@ -40,7 +40,7 @@ RSpec.describe DiscourseChat::Provider::MattermostProvider do it 'handles errors correctly' do stub1 = stub_request(:post, "https://mattermost.blah/hook/abcd").to_return(status: 500, body: "error") expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end diff --git a/spec/lib/discourse_chat/provider/rocketchat/rocketchat_provider_spec.rb b/spec/lib/discourse_chat/provider/rocketchat/rocketchat_provider_spec.rb index 284c662..42ecf31 100644 --- a/spec/lib/discourse_chat/provider/rocketchat/rocketchat_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/rocketchat/rocketchat_provider_spec.rb @@ -15,14 +15,14 @@ RSpec.describe DiscourseChat::Provider::RocketchatProvider do it 'sends a webhook request' do stub1 = stub_request(:post, 'https://example.com/abcd').to_return(body: "{\"success\":true}") - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, 'https://example.com/abcd').to_return(status: 400, body: "{}") expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once 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 843f8da..38d9b8b 100644 --- a/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb @@ -75,36 +75,65 @@ RSpec.describe DiscourseChat::Provider::SlackProvider do it 'sends a webhook request' do stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(body: "success") - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(status: 400, body: "error") expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end describe 'with api token' do before do SiteSetting.chat_integration_slack_access_token = "magic" + @ts = "#{Time.now.to_i}.012345" @stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(body: "success") - @stub2 = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).to_return(body: "{\"ok\":true, \"ts\": \"#{Time.now.to_i}.012345\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }", headers: { 'Content-Type' => 'application/json' }) - @stub3 = stub_request(:post, %r{https://slack.com/api/chat.update}).to_return(body: '{"ok":true, "ts": "some_message_id"}', headers: { 'Content-Type' => 'application/json' }) + @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 = DiscourseChat::Channel.create(provider: 'dummy') end it 'sends an api request' do expect(@stub2).to have_been_requested.times(0) + expect(@thread_stub).to have_been_requested.times(0) - described_class.trigger_notification(post, chan1) + 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(@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 = DiscourseChat::Rule.create(channel: @channel, filter: "thread") + post.topic.slack_thread_id = @ts + + described_class.trigger_notification(post, chan1, rule) + expect(@thread_stub).to have_been_requested.once + 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 = DiscourseChat::Rule.create(channel: @channel, filter: "thread") + post.topic.slack_thread_id = nil + + described_class.trigger_notification(post, chan1, rule) + expect(post.topic.slack_thread_id).to eq('1501801629.052212') end it 'handles errors correctly' do @stub2 = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).to_return(body: "{\"ok\":false }", headers: { 'Content-Type' => 'application/json' }) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(@stub2).to have_been_requested.once end diff --git a/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb b/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb index f011b62..e8e1571 100644 --- a/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb +++ b/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb @@ -22,7 +22,7 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do "type": "message", "user": "U6E2W7R8C", "text": "Which one?", - "ts": "1501801634.053761" + "ts": "1501801635.053761" }, { "type": "message", @@ -32,7 +32,7 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do }, { "type": "message", - "user": "U6E2W7R8C", + "user": "U820GH3LA", "text": "I'm interested!!", "ts": "1501801634.053761", "thread_ts": "1501801629.052212" @@ -70,6 +70,24 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do let(:users_fixture) { [ + { + id: "U6JSSESES", + name: "threader", + profile: { + image_24: "https://example.com/avatar", + display_name: "Threader", + real_name: "A. Threader" + } + }, + { + id: "U820GH3LA", + name: "responder", + profile: { + image_24: "https://example.com/avatar", + display_name: "Responder", + real_name: "A. Responder" + } + }, { id: "U5Z773QLS", name: "awesomeguyemail", @@ -160,18 +178,24 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do let(:thread_transcript) { described_class.new(channel_name: "#general", channel_id: "G1234", requested_thread_ts: "1501801629.052212") } before do + thread_transcript.load_user_data stub_request(:post, "https://slack.com/api/conversations.replies") .with(body: hash_including(token: "abcde", channel: 'G1234', ts: "1501801629.052212")) - .to_return(status: 200, body: { ok: true, messages: messages_fixture }.to_json) + .to_return(status: 200, body: { ok: true, messages: messages_fixture[3..4] }.to_json) thread_transcript.load_chat_history end it 'includes messages in a thread' do - expect(thread_transcript.messages.length).to eq(7) + expect(thread_transcript.messages.length).to eq(2) end it 'loads in chronological order' do # replies API presents messages in actual chronological order - expect(thread_transcript.messages.first.ts).to eq('1501801665.062694') + expect(thread_transcript.messages.first.ts).to eq('1501801629.052212') + end + + it 'includes slack thread identifiers in body' do + text = thread_transcript.build_transcript + expect(text).to include("") end end @@ -249,7 +273,7 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do it 'handles usernames correctly' do expect(transcript.first_message.username).to eq('awesomeguy') # Normal user expect(transcript.messages[1].username).to eq('Test_Community') # Bot user - expect(transcript.messages[2].username).to eq(nil) # Unknown normal user + expect(transcript.messages[3].username).to eq(nil) # Unknown normal user # Normal user, display_name not set (fall back to real_name) expect(transcript.messages[4].username).to eq('another_guy') end diff --git a/spec/lib/discourse_chat/provider/telegram/telegram_provider_spec.rb b/spec/lib/discourse_chat/provider/telegram/telegram_provider_spec.rb index 232bfa8..1c92ea0 100644 --- a/spec/lib/discourse_chat/provider/telegram/telegram_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/telegram/telegram_provider_spec.rb @@ -16,14 +16,14 @@ RSpec.describe DiscourseChat::Provider::TelegramProvider do it 'sends a webhook request' do stub1 = stub_request(:post, 'https://api.telegram.org/botTOKEN/sendMessage').to_return(body: "{\"ok\":true}") - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, 'https://api.telegram.org/botTOKEN/sendMessage').to_return(body: "{\"ok\":false, \"description\":\"chat not found\"}") expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end diff --git a/spec/lib/discourse_chat/provider/zulip/zulip_provider_spec.rb b/spec/lib/discourse_chat/provider/zulip/zulip_provider_spec.rb index 3738b35..c8b2f2b 100644 --- a/spec/lib/discourse_chat/provider/zulip/zulip_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/zulip/zulip_provider_spec.rb @@ -17,14 +17,14 @@ RSpec.describe DiscourseChat::Provider::ZulipProvider do it 'sends a webhook request' do stub1 = stub_request(:post, 'https://hello.world/api/v1/messages').to_return(status: 200) - described_class.trigger_notification(post, chan1) + described_class.trigger_notification(post, chan1, nil) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, 'https://hello.world/api/v1/messages').to_return(status: 400, body: '{}') expect(stub1).to have_been_requested.times(0) - expect { described_class.trigger_notification(post, chan1) }.to raise_exception(::DiscourseChat::ProviderError) + expect { described_class.trigger_notification(post, chan1, nil) }.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end diff --git a/spec/models/rule_spec.rb b/spec/models/rule_spec.rb index 4dd5614..633948e 100644 --- a/spec/models/rule_spec.rb +++ b/spec/models/rule_spec.rb @@ -141,11 +141,12 @@ RSpec.describe DiscourseChat::Rule do it 'can be sorted by precedence' do rule2 = DiscourseChat::Rule.create(channel: channel, filter: 'mute') rule3 = DiscourseChat::Rule.create(channel: channel, filter: 'follow') - rule4 = DiscourseChat::Rule.create(channel: channel, filter: 'mute') + rule4 = DiscourseChat::Rule.create(channel: channel, filter: 'thread') + rule5 = DiscourseChat::Rule.create(channel: channel, filter: 'mute') - expect(DiscourseChat::Rule.all.length).to eq(4) + expect(DiscourseChat::Rule.all.length).to eq(5) - expect(DiscourseChat::Rule.all.order_by_precedence.map(&:filter)).to eq(["mute", "mute", "watch", "follow"]) + expect(DiscourseChat::Rule.all.order_by_precedence.map(&:filter)).to eq(["mute", "mute", "thread", "watch", "follow"]) end end @@ -190,6 +191,8 @@ RSpec.describe DiscourseChat::Rule do end it 'validates filter correctly' do + expect(rule.valid?).to eq(true) + rule.filter = 'thread' expect(rule.valid?).to eq(true) rule.filter = 'follow' expect(rule.valid?).to eq(true) diff --git a/spec/services/manager_spec.rb b/spec/services/manager_spec.rb index 3806fef..ac9125a 100644 --- a/spec/services/manager_spec.rb +++ b/spec/services/manager_spec.rb @@ -93,7 +93,7 @@ RSpec.describe DiscourseChat::Manager do end it "should respect watch over follow" do - DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: nil) # Wildcard watch + DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: nil) # Wildcard follow DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id: category.id) # Specific watch manager.trigger_notifications(second_post.id) @@ -101,6 +101,15 @@ RSpec.describe DiscourseChat::Manager do expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id) end + it "should respect thread over watch" do + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id: nil) # Wildcard watch + DiscourseChat::Rule.create!(channel: chan1, filter: 'thread', category_id: category.id) # Specific thread + + manager.trigger_notifications(second_post.id) + + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id) + end + it "should not notify about private messages" do DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: nil) # Wildcard watch