FEATURE: add topic tags changed trigger to chat integration (#208)

* FEATURE: add topic tags changed trigger to chat integration

* FEATURE: add placeholder for reply to topic trigger

add description on how to use the placeholder

* DEV: Move slack message creation to provider

Add tests to new method

* FEATURE: add ${URL} to placeholder replacements and added tags link

If triggered when a topic tag is changed, message behavior will follow what user defined in message.

* DEV: Update tests with tags

* DEV: add post to topic for testing

* DEV: update test strings

* DEV: add early return for topic tags changed trigger

* DEV: move early return to use try/catch

* DEV: update `create_slack_message` to not send a tuple of values

* DEV: refactor method to be more readable

* FEATURE: add `${ADDED_AND_REMOVED}` for default texts

* DEV: Update typo in test

* DEV: Add tests to check when if `create_slack_message` raises an error

* DEV: Remove the `tag_added` from chat-integration filter

Added migration to handle the migration of the `tag_added` filter from the chat-integration plugin.

Only removed the logic from the plugin, data removal will happen in a future PR

* DEV: lint migration file

* DEV: update chat-integration to not show "tag_added" rules

* DEV: update added and missing tags logic

* DEV: update context variable name

* DEV: update migration to include `begin/rescue` block and added a list with available filters

admin/assets/javascripts/admin/models/rule.js

@@ -23,6 +23,8 @@ export default class Rule extends RestModel {
     },
   ];
 
+  possible_filters_id = ["thread", "watch", "follow", "mute"];
+
   get available_filters() {
     const available = [];
     const provider = this.channel.provider;
@@ -46,11 +48,6 @@ export default class Rule extends RestModel {
         name: I18n.t("chat_integration.filter.follow"),
         icon: "circle",
       },
-      {
-        id: "tag_added",
-        name: I18n.t("chat_integration.filter.tag_added"),
-        icon: "tag",
-      },
       {
         id: "mute",
         name: I18n.t("chat_integration.filter.mute"),

admin/assets/javascripts/admin/routes/admin-plugins-chat-integration-provider.js

@@ -1,4 +1,5 @@
 import { action } from "@ember/object";
+import { getOwner } from "@ember/owner";
 import Group from "discourse/models/group";
 import DiscourseRoute from "discourse/routes/discourse";
 
@@ -13,10 +14,14 @@ export default class AdminPluginsChatIntegrationProvider extends DiscourseRoute
       Group.findAll(),
     ]);
 
+    const enabledFilters =
+      getOwner(this).lookup("model:rule").possible_filters_id;
     channels.forEach((channel) => {
       channel.set(
         "rules",
-        channel.rules.map((rule) => {
+        channel.rules
+          .filter((rule) => enabledFilters.includes(rule.filter))
+          .map((rule) => {
             rule = this.store.createRecord("rule", rule);
             rule.set("channel", channel);
             return rule;

app/services/manager.rb

@@ -15,11 +15,11 @@ module DiscourseChatIntegration
       # Abort if the post is blank
       return if post.blank?
 
-      # Abort if post is not either regular, or a 'tags_changed'/'category_changed' small action
+      # Abort if post is not either regular or a 'category_changed' small action
       if (post.post_type != Post.types[:regular]) &&
            !(
              post.post_type == Post.types[:small_action] &&
-               %w[tags_changed category_changed].include?(post.action_code)
+               %w[category_changed].include?(post.action_code)
            )
         return
       end
@@ -55,34 +55,7 @@ module DiscourseChatIntegration
         end
       end
 
-      if post.action_code == "tags_changed"
+      matching_rules = matching_rules.select { |rule| rule.filter != "tag_added" } # ignore tag_added rules, now uses Automation
-        # Post is a small_action post regarding tags changing for the topic. Check if any tags were _added_
-        # and if so, corresponding rules with `filter: tag_added`
-        tags_added = post.custom_fields["tags_added"]
-        tags_added = [tags_added].compact if !tags_added.is_a?(Array)
-        return if tags_added.blank?
-
-        tags_removed = post.custom_fields["tags_removed"]
-        tags_removed = [tags_removed].compact if !tags_removed.is_a?(Array)
-
-        unchanged_tags = topic.tags.map(&:name) - tags_added - tags_removed
-
-        matching_rules =
-          matching_rules.select do |rule|
-            # Only rules that match this post, are ones where the filter is "tag_added"
-            next false if rule.filter != "tag_added"
-            next true if rule.tags.blank?
-
-            # Skip if the topic already has one of the tags in the rule, applied
-            next false if unchanged_tags.any? && (unchanged_tags & rule.tags).any?
-
-            # We don't need to do any additional filtering here because topics are filtered
-            # by tag later
-            true
-          end
-      else
-        matching_rules = matching_rules.select { |rule| rule.filter != "tag_added" }
-      end
 
       # If tagging is enabled, thow away rules that don't apply to this topic
       if SiteSetting.tagging_enabled
@@ -97,7 +70,7 @@ module DiscourseChatIntegration
 
       # Sort by order of precedence
       t_prec = { "group_message" => 0, "group_mention" => 1, "normal" => 2 } # Group things win
-      f_prec = { "mute" => 0, "thread" => 1, "watch" => 2, "follow" => 3, "tag_added" => 4 } #(mute always wins; thread beats 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)

config/locales/client.en.yml

@@ -37,7 +37,6 @@ en:
         mute: 'Mute'
         follow: 'First post only'
         watch: 'All posts and replies'
-        tag_added: 'Tag added to topic'
         thread: 'All posts with threaded replies'
       rule_table:
         filter: "Filter"
@@ -256,7 +255,11 @@ en:
           title: Send Slack message
           fields:
             message:
-              label: Message
+              label: Message.
+              description: >-
+                           Use ${TOPIC} for topic name, ${URL} for used URL, ${REMOVED_TAGS} for removed tags, ${ADDED_TAGS} for added tags,
+                           ${ADDED_AND_REMOVED} for default text.
+                           only available if trigger is set to Topic tags changed.
             url:
               label: URL
             channel:

config/locales/server.en.yml

@@ -187,6 +187,11 @@ en:
           error_users: "Error: unable to fetch users from Slack"
           error_history: "Error: unable to fetch channel history from Slack"
           error_ts: "Error: unable to fetch requested message from Slack"
+        messaging:
+          topic_tag_changed:
+            added_and_removed: "Added %{added} and removed %{removed}"
+            added: "Added %{added}"
+            removed: "Removed %{removed}"
 
       #######################################
       ########## TELEGRAM STRINGS ###########

db/migrate/20240808175526_migrate_tag_added_from_filter_to_automation.rb

@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+class MigrateTagAddedFromFilterToAutomation < ActiveRecord::Migration[7.1]
+  def up
+    if defined?(DiscourseAutomation) &&
+         DiscourseChatIntegration::Channel.with_provider("slack").exists?
+      begin
+        DiscourseChatIntegration::Rule
+          .where("value::json->>'filter'=?", "tag_added")
+          .each do |rule|
+            channel_id = rule.value["channel_id"]
+            channel_name =
+              DiscourseChatIntegration::Channel.find(channel_id).value["data"]["identifier"] # it _must_ have a channel_id
+
+            category_id = rule.value["category_id"]
+            tags = rule.value["tags"]
+
+            automation =
+              DiscourseAutomation::Automation.new(
+                script: "send_slack_message",
+                trigger: "topic_tags_changed",
+                name: "When tags change in topic",
+                enabled: true,
+                last_updated_by_id: Discourse.system_user.id,
+              )
+
+            automation.save!
+
+            # Triggers:
+            # Watching categories
+
+            metadata = (category_id ? { "value" => [category_id] } : {})
+
+            automation.upsert_field!(
+              "watching_categories",
+              "categories",
+              metadata,
+              target: "trigger",
+            )
+
+            # Watching tags
+
+            metadata = (tags ? { "value" => tags } : {})
+
+            automation.upsert_field!("watching_tags", "tags", metadata, target: "trigger")
+
+            # Script options:
+            # Message
+            automation.upsert_field!(
+              "message",
+              "message",
+              { "value" => "${ADDED_AND_REMOVED}" },
+              target: "script",
+            )
+
+            # URL
+            automation.upsert_field!(
+              "url",
+              "text",
+              { "value" => Discourse.current_hostname },
+              target: "script",
+            )
+
+            # Channel
+            automation.upsert_field!(
+              "channel",
+              "text",
+              { "value" => channel_name },
+              target: "script",
+            )
+          end
+      rescue StandardError
+        Rails.logger.warn("Failed to migrate tag_added rules to automations")
+      end
+    end
+  end
+
+  def down
+    if defined?(DiscourseAutomation) &&
+         DiscourseChatIntegration::Channel.with_provider("slack").exists?
+      DiscourseAutomation::Automation
+        .where(script: "send_slack_message", trigger: "topic_tags_changed")
+        .each do |automation|
+          # if is the same name as created and message is the same
+          if automation.name == "When tags change in topic" &&
+               automation.fields.where(name: "message").first.metadata["value"] ==
+                 "${ADDED_AND_REMOVED}"
+            automation.destroy!
+          end
+        end
+    end
+  end
+end

lib/discourse_chat_integration/provider/slack/slack_provider.rb

@@ -87,6 +87,74 @@ module DiscourseChatIntegration::Provider::SlackProvider
     message
   end
 
+  def self.create_slack_message(context:, content:, url:, channel_name:)
+    sender = ::DiscourseChatIntegration::Helper.formatted_display_name(Discourse.system_user)
+
+    content = replace_placehoders(content, context) if context["kind"] ==
+      DiscourseAutomation::Triggers::TOPIC_TAGS_CHANGED
+
+    full_content =
+      if context["kind"] == DiscourseAutomation::Triggers::TOPIC_TAGS_CHANGED
+        content
+      else
+        "#{content} - #{url}"
+      end
+
+    icon_url =
+      if SiteSetting.chat_integration_slack_icon_url.present?
+        "#{Discourse.base_url}#{SiteSetting.chat_integration_slack_icon_url}"
+      elsif (url = (SiteSetting.try(:site_logo_small_url) || SiteSetting.logo_small_url)).present?
+        "#{Discourse.base_url}#{url}"
+      end
+
+    slack_username =
+      if SiteSetting.chat_integration_slack_username.present?
+        SiteSetting.chat_integration_slack_username
+      else
+        SiteSetting.title || "Discourse"
+      end
+
+    message = {
+      channel: "##{channel_name}",
+      username: slack_username,
+      icon_url: icon_url,
+      attachments: [],
+    }
+
+    summary = {
+      fallback: content.truncate(100),
+      author_name: sender,
+      color: nil,
+      text: full_content,
+      mrkdwn_in: ["text"],
+      title: content.truncate(100),
+      title_link: url,
+      thumb_url: nil,
+    }
+
+    if context["kind"] == DiscourseAutomation::Triggers::TOPIC_TAGS_CHANGED
+      topic = context["topic"]
+      category =
+        if topic.category&.uncategorized?
+          "[#{I18n.t("uncategorized_category_name")}]"
+        elsif topic.category
+          if (topic.category.parent_category)
+            "[#{topic.category.parent_category.name}/#{topic.category.name}]"
+          else
+            "[#{topic.category.name}]"
+          end
+        end
+      summary[:title_link] = topic.posts.first.full_url
+      summary[
+        :title
+      ] = "#{topic.title} #{category} #{topic.tags.present? ? topic.tags.map(&:name).join(", ") : ""}"
+      summary[:thumb_url]
+    end
+
+    message[:attachments].push(summary)
+    message
+  end
+
   def self.send_via_api(post, channel, message)
     http = slack_api_http
 
@@ -214,6 +282,63 @@ module DiscourseChatIntegration::Provider::SlackProvider
       unique_by: :index_topic_custom_fields_on_topic_id_and_slack_thread_id,
     )
   end
+
+  def self.replace_placehoders(content, context)
+    if context["topic"] && content.include?("${TOPIC}")
+      topic = context["topic"]
+      content = content.gsub("${TOPIC}", topic.title)
+    end
+
+    if content.include?("${REMOVED_TAGS}")
+      if context["removed_tags"].empty?
+        raise StandardError.new "No tags but content includes reference."
+      end
+      removed_tags_names = create_tag_list(context["removed_tags"])
+      content = content.gsub("${REMOVED_TAGS}", removed_tags_names)
+    end
+
+    if content.include?("${ADDED_TAGS}")
+      if context["added_tags"].empty?
+        raise StandardError.new "No tags but content includes reference."
+      end
+      added_tags_names = create_tag_list(context["added_tags"])
+      content = content.gsub("${ADDED_TAGS}", added_tags_names)
+    end
+
+    if content.include?("${ADDED_AND_REMOVED}")
+      added_tags = context["added_tags"]
+      missing_tags = context["removed_tags"]
+
+      text =
+        if !added_tags.empty? && !missing_tags.empty?
+          I18n.t(
+            "chat_integration.provider.slack.messaging.topic_tag_changed.added_and_removed",
+            added: create_tag_list(added_tags),
+            removed: create_tag_list(missing_tags),
+          )
+        elsif !added_tags.empty?
+          I18n.t(
+            "chat_integration.provider.slack.messaging.topic_tag_changed.added",
+            added: create_tag_list(added_tags),
+          )
+        elsif !missing_tags.empty?
+          I18n.t(
+            "chat_integration.provider.slack.messaging.topic_tag_changed.removed",
+            removed: create_tag_list(missing_tags),
+          )
+        end
+
+      content = content.gsub("${ADDED_AND_REMOVED}", text)
+    end
+
+    content = content.gsub("${URL}", url) if content.include?("${URL}")
+
+    content
+  end
+
+  def self.create_tag_list(tag_list)
+    tag_list.map { |tag_name| "<#{Tag.find_by_name(tag_name).full_url}|#{tag_name}>" }.join(", ")
+  end
 end
 
 require_relative "slack_message_formatter"

plugin.rb

@@ -58,14 +58,9 @@ after_initialize do
 
       version 1
 
-      triggerables %i[point_in_time recurring]
+      triggerables %i[point_in_time recurring topic_tags_changed]
 
       script do |context, fields, automation|
-        sender = Discourse.system_user
-
-        content = fields.dig("message", "value")
-        url = fields.dig("url", "value")
-        full_content = "#{content} - #{url}"
         channel_name = fields.dig("channel", "value")
         channel =
           DiscourseChatIntegration::Channel.new(
@@ -75,43 +70,18 @@ after_initialize do
             },
           )
 
-        icon_url =
+        begin
-          if SiteSetting.chat_integration_slack_icon_url.present?
+          message =
-            "#{Discourse.base_url}#{SiteSetting.chat_integration_slack_icon_url}"
+            DiscourseChatIntegration::Provider::SlackProvider.create_slack_message(
-          elsif (
+              context: context,
-                url = (SiteSetting.try(:site_logo_small_url) || SiteSetting.logo_small_url)
+              content: fields.dig("message", "value"),
-              ).present?
+              url: fields.dig("url", "value"),
-            "#{Discourse.base_url}#{url}"
+              channel_name: channel_name,
-          end
+            )
-
-        slack_username =
-          if SiteSetting.chat_integration_slack_username.present?
-            SiteSetting.chat_integration_slack_username
-          else
-            SiteSetting.title || "Discourse"
-          end
-
-        message = {
-          channel: "##{channel_name}",
-          username: slack_username,
-          icon_url: icon_url,
-          attachments: [],
-        }
-
-        summary = {
-          fallback: content.truncate(100),
-          author_name: sender,
-          color: nil,
-          text: full_content,
-          mrkdwn_in: ["text"],
-          title: content.truncate(100),
-          title_link: url,
-          thumb_url: nil,
-        }
-
-        message[:attachments].push(summary)
-
           DiscourseChatIntegration::Provider::SlackProvider.send_via_api(nil, channel, message)
+        rescue StandardError => _
+          # StandardError here is when there are no tags but content includes reference to them.
+        end
       end
     end
   end

spec/lib/discourse_chat_integration/provider/slack/slack_provider_spec.rb

@@ -209,4 +209,141 @@ RSpec.describe DiscourseChatIntegration::Provider::SlackProvider do
       end
     end
   end
+
+  describe ".create_slack_message" do
+    it "should work with a simple message" do
+      content = "Simple message"
+      url = "http://example.com"
+      message = { channel: "#general", username: "Discourse", content: "#{content} - #{url}" }
+      result =
+        described_class.create_slack_message(
+          context: {
+          },
+          content: content,
+          channel_name: "general",
+          url: url,
+        )
+      expect(
+        {
+          channel: result[:channel],
+          username: result[:username],
+          content: result[:attachments][0][:text],
+        },
+      ).to eq(message)
+    end
+
+    it "should do the replacements" do
+      topic = Fabricate(:topic)
+      topic.posts << Fabricate(:post, topic: topic)
+      tag1, tag2, tag3, tag4 = [Fabricate(:tag), Fabricate(:tag), Fabricate(:tag), Fabricate(:tag)]
+      content =
+        "The topic title is: ${TOPIC}
+         removed tags: ${REMOVED_TAGS}
+         added tags: ${ADDED_TAGS}"
+
+      result =
+        described_class.create_slack_message(
+          context: {
+            "topic" => topic,
+            "removed_tags" => [tag1.name, tag2.name],
+            "added_tags" => [tag3.name, tag4.name],
+            "kind" => DiscourseAutomation::Triggers::TOPIC_TAGS_CHANGED,
+          },
+          content: content,
+          channel_name: "general",
+          url: "http://example.com",
+        )
+      text = result[:attachments][0][:text]
+      expect(text).to include(topic.title)
+      expect(text).to include("<#{tag1.full_url}|#{tag1.name}>, <#{tag2.full_url}|#{tag2.name}>")
+      expect(text).to include("<#{tag3.full_url}|#{tag3.name}>, <#{tag4.full_url}|#{tag4.name}>")
+    end
+
+    it "should do the replacements for ${ADDED_AND_REMOVED}" do
+      topic = Fabricate(:topic)
+      topic.posts << Fabricate(:post, topic: topic)
+      tag1, tag2 = [Fabricate(:tag), Fabricate(:tag)]
+      content = "${ADDED_AND_REMOVED}"
+      result =
+        described_class.create_slack_message(
+          context: {
+            "topic" => topic,
+            "added_tags" => [tag2.name],
+            "removed_tags" => [tag1.name],
+            "kind" => DiscourseAutomation::Triggers::TOPIC_TAGS_CHANGED,
+          },
+          content: content,
+          channel_name: "general",
+          url: "http://example.com",
+        )
+      text = result[:attachments][0][:text]
+      expect(text).to include(
+        I18n.t(
+          "chat_integration.provider.slack.messaging.topic_tag_changed.added_and_removed",
+          added: "<#{tag2.full_url}|#{tag2.name}>",
+          removed: "<#{tag1.full_url}|#{tag1.name}>",
+        ),
+      )
+
+      result =
+        described_class.create_slack_message(
+          context: {
+            "topic" => topic,
+            "added_tags" => [],
+            "removed_tags" => [tag1.name],
+            "kind" => DiscourseAutomation::Triggers::TOPIC_TAGS_CHANGED,
+          },
+          content: content,
+          channel_name: "general",
+          url: "http://example.com",
+        )
+      text = result[:attachments][0][:text]
+      expect(text).to include(
+        I18n.t(
+          "chat_integration.provider.slack.messaging.topic_tag_changed.removed",
+          removed: "<#{tag1.full_url}|#{tag1.name}>",
+        ),
+      )
+
+      result =
+        described_class.create_slack_message(
+          context: {
+            "topic" => topic,
+            "added_tags" => [tag2.name],
+            "removed_tags" => [],
+            "kind" => DiscourseAutomation::Triggers::TOPIC_TAGS_CHANGED,
+          },
+          content: content,
+          channel_name: "general",
+          url: "http://example.com",
+        )
+      text = result[:attachments][0][:text]
+      expect(text).to include(
+        I18n.t(
+          "chat_integration.provider.slack.messaging.topic_tag_changed.added",
+          added: "<#{tag2.full_url}|#{tag2.name}>",
+        ),
+      )
+    end
+
+    it "should raise errors if tags are not present but uses in content" do
+      topic = Fabricate(:topic)
+      topic.posts << Fabricate(:post, topic: topic)
+      content = "This should not work ${ADDED_TAGS}"
+
+      expect {
+        described_class.create_slack_message(
+          context: {
+            "topic" => topic,
+            "kind" => DiscourseAutomation::Triggers::TOPIC_TAGS_CHANGED,
+            "added_tags" => [],
+            "removed_tags" => [],
+          },
+          content: content,
+          channel_name: "general",
+          url: "http://example.com",
+        )
+      }.to raise_error(StandardError)
+    end
+  end
 end

spec/services/manager_spec.rb

@@ -377,82 +377,6 @@ RSpec.describe DiscourseChatIntegration::Manager do
 
         expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id)
       end
-
-      describe "with create_small_action_post_for_tag_changes enabled" do
-        fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) }
-        fab!(:additional_tag) { Fabricate(:tag) }
-
-        before { SiteSetting.create_post_for_category_and_tag_changes = true }
-
-        def set_new_tags_and_return_small_action_post(tags)
-          PostRevisor.new(tagged_first_post).revise!(admin, tags: tags)
-
-          tagged_topic.ordered_posts.last
-        end
-
-        it "should notify when rule is set up for tag additions for a category with no tag filter" do
-          post = set_new_tags_and_return_small_action_post([tag.name, additional_tag.name])
-
-          DiscourseChatIntegration::Rule.create!(
-            channel: chan1,
-            filter: "tag_added",
-            category_id: category.id,
-          )
-
-          manager.trigger_notifications(post.id)
-          expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id)
-        end
-
-        it "notifies when topic has a tag added that matches the rule" do
-          post = set_new_tags_and_return_small_action_post([tag.name, additional_tag.name])
-
-          DiscourseChatIntegration::Rule.create!(
-            channel: chan1,
-            filter: "tag_added",
-            category_id: category.id,
-            tags: [additional_tag.name],
-          )
-
-          manager.trigger_notifications(post.id)
-          expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id)
-        end
-
-        it "doesn't notify when a new regular post is created" do
-          DiscourseChatIntegration::Rule.create!(
-            channel: chan1,
-            filter: "tag_added",
-            category_id: nil,
-            tags: [tag.name],
-          )
-
-          post = Fabricate(:post, topic: tagged_topic)
-          manager.trigger_notifications(post.id)
-          expect(provider.sent_to_channel_ids).to contain_exactly
-        end
-
-        it "doesn't notify when topic has an unchanged tag present in the rule, even if a new tag is added" do
-          post = set_new_tags_and_return_small_action_post([tag.name, additional_tag.name])
-
-          DiscourseChatIntegration::Rule.create!(
-            channel: chan1,
-            filter: "tag_added",
-            category_id: category.id,
-            tags: [tag.name],
-          )
-
-          manager.trigger_notifications(post.id)
-          expect(provider.sent_to_channel_ids).to contain_exactly
-        end
-
-        it "doesn't notify for small action 'tags_changed' posts unless a matching rule exists" do
-          post = set_new_tags_and_return_small_action_post([additional_tag.name])
-
-          DiscourseChatIntegration::Rule.create!(channel: chan1, filter: "watch", category_id: nil) # Wildcard watch
-
-          manager.trigger_notifications(post.id)
-          expect(provider.sent_to_channel_ids).to contain_exactly
-        end
-      end
     end
   end
 end