FIX: revert chat-integration move to discourse-automation (#219)

* Reapply "FIX: revert chat-integration move to discourse-automation (#214)" (#218)

This reverts commit f15c0fddf3.

* DEV: added more logging to the plugin

* DEV: update error logging with `.inspect` for better debugging

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

@@ -23,8 +23,6 @@ export default class Rule extends RestModel {
     },
   ];
 
-  possible_filters_id = ["thread", "watch", "follow", "mute"];
-
   get available_filters() {
     const available = [];
     const provider = this.channel.provider;
@@ -48,6 +46,11 @@ 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,5 +1,4 @@
 import { action } from "@ember/object";
-import { getOwner } from "@ember/owner";
 import Group from "discourse/models/group";
 import DiscourseRoute from "discourse/routes/discourse";
 
@@ -14,14 +13,10 @@ 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
+        channel.rules.map((rule) => {
-          .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 'category_changed' small action
+      # Abort if post is not either regular, or a 'tags_changed'/'category_changed' small action
       if (post.post_type != Post.types[:regular]) &&
            !(
              post.post_type == Post.types[:small_action] &&
-               %w[category_changed].include?(post.action_code)
+               %w[tags_changed category_changed].include?(post.action_code)
            )
         return
       end
@@ -55,7 +55,34 @@ module DiscourseChatIntegration
         end
       end
 
-      matching_rules = matching_rules.select { |rule| rule.filter != "tag_added" } # ignore tag_added rules, now uses Automation
+      if post.action_code == "tags_changed"
+        # 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
@@ -70,7 +97,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 } #(mute always wins; thread beats watch beats follow)
+      f_prec = { "mute" => 0, "thread" => 1, "watch" => 2, "follow" => 3, "tag_added" => 4 } #(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,6 +37,7 @@ 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"

plugin.rb

@@ -127,8 +127,11 @@ after_initialize do
 
         begin
           provider.trigger_notification(post, channel, nil)
-        rescue StandardError => _
+        rescue StandardError => e
           Rails.logger.warn "[discourse-automation] Error while sending chat integration message. Automation ID: #{automation.id}"
+          Rails.logger.warn "[discourse-chat-integration] Error: #{e.inspect}"
+          Rails.logger.warn "[discourse-chat-integration] Channel: #{channel_name}"
+          Rails.logger.warn "[discourse-chat-integration] Reference post: #{post.inspect}"
         end
       end
     end

spec/services/manager_spec.rb

@@ -316,6 +316,38 @@ RSpec.describe DiscourseChatIntegration::Manager do
       expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id)
     end
 
+    describe "With `create_post_for_category_and_tag_changes` enabled" do
+      before(:each) { SiteSetting.create_post_for_category_and_tag_changes = true }
+
+      let(:admin) { Fabricate(:admin) }
+      let(:other_topic) { Fabricate(:topic) }
+      let(:other_topic_post) { Fabricate(:post, topic: topic) }
+
+      it "should trigger follow rules for specific categories when topic category changes" do
+        DiscourseChatIntegration::Rule.create!(
+          channel: chan1,
+          filter: "follow",
+          category_id: category.id,
+        )
+
+        PostRevisor.new(other_topic_post).revise!(admin, category_id: category.id)
+
+        manager.trigger_notifications(topic.ordered_posts.last.id)
+
+        expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id)
+      end
+
+      it "shouldn't trigger follow rules with wildcard category match" do
+        DiscourseChatIntegration::Rule.create!(channel: chan1, filter: "follow", category_id: nil)
+
+        PostRevisor.new(other_topic_post).revise!(admin, category_id: category.id)
+
+        manager.trigger_notifications(topic.ordered_posts.last.id)
+
+        expect(provider.sent_to_channel_ids).to contain_exactly
+      end
+    end
+
     describe "with tags enabled" do
       let(:tag) { Fabricate(:tag, name: "gsoc") }
       let(:tagged_topic) { Fabricate(:topic, category_id: category.id, tags: [tag]) }
@@ -345,6 +377,82 @@ 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