From a1c8f4ee9df918fd1deaefa492ece12edc6ae5c5 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Tue, 27 Aug 2024 16:24:07 -0300 Subject: [PATCH] FIX: revert chat-integration move to discourse-automation (#214) * FIX: revert chat-integration move to discourse-automation this reverses the changes made in 57b460737c90af7a9f445b3e5bd4e8cbc09aa298 without deleting the migration. * Revert "DEV: removing create post for category and tag changes setting (#210)" This reverts commit 35925c4dac08638ecba1da588310719c7983763d. * DEV: merge both chat-integration rules and automation rules --- admin/assets/javascripts/admin/models/rule.js | 7 +- ...admin-plugins-chat-integration-provider.js | 15 +-- app/services/manager.rb | 35 +++++- config/locales/client.en.yml | 1 + spec/services/manager_spec.rb | 108 ++++++++++++++++++ 5 files changed, 150 insertions(+), 16 deletions(-) diff --git a/admin/assets/javascripts/admin/models/rule.js b/admin/assets/javascripts/admin/models/rule.js index 34890dc5..4262ed31 100644 --- a/admin/assets/javascripts/admin/models/rule.js +++ b/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"), diff --git a/admin/assets/javascripts/admin/routes/admin-plugins-chat-integration-provider.js b/admin/assets/javascripts/admin/routes/admin-plugins-chat-integration-provider.js index d1446560..dce0a5a2 100644 --- a/admin/assets/javascripts/admin/routes/admin-plugins-chat-integration-provider.js +++ b/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,18 +13,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 - .filter((rule) => enabledFilters.includes(rule.filter)) - .map((rule) => { - rule = this.store.createRecord("rule", rule); - rule.set("channel", channel); - return rule; - }) + channel.rules.map((rule) => { + rule = this.store.createRecord("rule", rule); + rule.set("channel", channel); + return rule; + }) ); }); diff --git a/app/services/manager.rb b/app/services/manager.rb index e19d4a5e..590a25ff 100644 --- a/app/services/manager.rb +++ b/app/services/manager.rb @@ -15,11 +15,11 @@ def self.trigger_notifications(post_id) # 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 @@ def self.trigger_notifications(post_id) 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 @@ def self.trigger_notifications(post_id) # 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) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d9641d57..220cd1b7 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/spec/services/manager_spec.rb b/spec/services/manager_spec.rb index c749cd9a..878a84c7 100644 --- a/spec/services/manager_spec.rb +++ b/spec/services/manager_spec.rb @@ -316,6 +316,38 @@ 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 @@ 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