FEATURE: Filter topic and post web hook events by tags (#6726)

approved

#1
FEATURE: Filter topic and post web hook events by tags (#6726)
  • FEATURE: Filter topic and post web hook events by tags

  • Add a spec test with unmatched tags

From d33d0317420dc93d1bdf4420f0916fad5aa8085a Mon Sep 17 00:00:00 2001
From: Vinoth Kannan <vinothkannan@vinkas.com>
Date: Wed, 5 Dec 2018 14:44:06 +0530
Subject: [PATCH] FEATURE: Filter topic and post web hook events by tags
 (#6726)

* FEATURE: Filter topic and post web hook events by tags

* Add a spec test with unmatched tags

diff --git a/app/assets/javascripts/admin/controllers/admin-web-hooks-show.js.es6 b/app/assets/javascripts/admin/controllers/admin-web-hooks-show.js.es6
index 279fc53..55e6b00 100644
--- a/app/assets/javascripts/admin/controllers/admin-web-hooks-show.js.es6
+++ b/app/assets/javascripts/admin/controllers/admin-web-hooks-show.js.es6
@@ -9,6 +9,11 @@ export default Ember.Controller.extend({
   defaultEventTypes: Ember.computed.alias("adminWebHooks.defaultEventTypes"),
   contentTypes: Ember.computed.alias("adminWebHooks.contentTypes"),
 
+  @computed
+  showTagsFilter() {
+    return this.siteSettings.tagging_enabled;
+  },
+
   @computed("model.isSaving", "saved", "saveButtonDisabled")
   savingStatus(isSaving, saved, saveButtonDisabled) {
     if (isSaving) {
diff --git a/app/assets/javascripts/admin/models/web-hook.js.es6 b/app/assets/javascripts/admin/models/web-hook.js.es6
index 142d22b..a2a7ae7 100644
--- a/app/assets/javascripts/admin/models/web-hook.js.es6
+++ b/app/assets/javascripts/admin/models/web-hook.js.es6
@@ -63,6 +63,7 @@ export default RestModel.extend({
   createProperties() {
     const types = this.get("web_hook_event_types");
     const categoryIds = this.get("categories").map(c => c.id);
+    const tagNames = this.get("tag_names");
 
     // Hack as {{group-selector}} accepts a comma-separated string as data source, but
     // we use an array to populate the datasource above.
@@ -81,6 +82,7 @@ export default RestModel.extend({
         ? [null]
         : types.map(type => type.id),
       category_ids: Ember.isEmpty(categoryIds) ? [null] : categoryIds,
+      tag_names: Ember.isEmpty(tagNames) ? [null] : tagNames,
       group_ids:
         Ember.isEmpty(groupNames) || Ember.isEmpty(groupNames[0])
           ? [null]
diff --git a/app/assets/javascripts/admin/routes/admin-web-hooks-show.js.es6 b/app/assets/javascripts/admin/routes/admin-web-hooks-show.js.es6
index f548a4e..611b015 100644
--- a/app/assets/javascripts/admin/routes/admin-web-hooks-show.js.es6
+++ b/app/assets/javascripts/admin/routes/admin-web-hooks-show.js.es6
@@ -19,6 +19,7 @@ export default Discourse.Route.extend({
     }
 
     model.set("category_ids", model.get("category_ids"));
+    model.set("tag_names", model.get("tag_names"));
     model.set("group_ids", model.get("group_ids"));
     controller.setProperties({ model, saved: false });
   },
diff --git a/app/assets/javascripts/admin/templates/web-hooks-show.hbs b/app/assets/javascripts/admin/templates/web-hooks-show.hbs
index 294e6b3..bf408bf 100644
--- a/app/assets/javascripts/admin/templates/web-hooks-show.hbs
+++ b/app/assets/javascripts/admin/templates/web-hooks-show.hbs
@@ -51,6 +51,13 @@
         {{category-selector categories=model.categories}}
         <div class="instructions">{{i18n 'admin.web_hooks.categories_filter_instructions'}}</div>
       </div>
+      {{#if showTagsFilter}}
+        <div class="filter">
+          <label>{{d-icon 'circle' class='tracking'}}{{i18n 'admin.web_hooks.tags_filter'}}</label>
+          {{tag-chooser tags=model.tag_names everyTag=true}}
+          <div class="instructions">{{i18n 'admin.web_hooks.tags_filter_instructions'}}</div>
+        </div>
+      {{/if}}
       <div class="filter">
         <label>{{d-icon 'circle' class='tracking'}}{{i18n 'admin.web_hooks.groups_filter'}}</label>
         {{group-selector groupNames=model.groupsFilterInName groupFinder=model.groupFinder}}
diff --git a/app/controllers/admin/web_hooks_controller.rb b/app/controllers/admin/web_hooks_controller.rb
index 92ec801..1ba3b71 100644
--- a/app/controllers/admin/web_hooks_controller.rb
+++ b/app/controllers/admin/web_hooks_controller.rb
@@ -111,6 +111,7 @@ class Admin::WebHooksController < Admin::AdminController
                                      :wildcard_web_hook, :active, :verify_certificate,
                                      web_hook_event_type_ids: [],
                                      group_ids: [],
+                                     tag_names: [],
                                      category_ids: [])
   end
 
diff --git a/app/jobs/regular/emit_web_hook_event.rb b/app/jobs/regular/emit_web_hook_event.rb
index 5cd4010..643d496 100644
--- a/app/jobs/regular/emit_web_hook_event.rb
+++ b/app/jobs/regular/emit_web_hook_event.rb
@@ -28,6 +28,9 @@ module Jobs
         return if web_hook.category_ids.present? && (!args[:category_id].present? ||
           !web_hook.category_ids.include?(args[:category_id]))
 
+        return if web_hook.tag_ids.present? && (args[:tag_ids].blank? ||
+          (web_hook.tag_ids & args[:tag_ids]).blank?)
+
         raise Discourse::InvalidParameters.new(:payload) unless args[:payload].present?
         args[:payload] = JSON.parse(args[:payload])
       end
diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb
index 518b076..b72a3b7 100644
--- a/app/models/web_hook.rb
+++ b/app/models/web_hook.rb
@@ -2,6 +2,7 @@ class WebHook < ActiveRecord::Base
   has_and_belongs_to_many :web_hook_event_types
   has_and_belongs_to_many :groups
   has_and_belongs_to_many :categories
+  has_and_belongs_to_many :tags
 
   has_many :web_hook_events, dependent: :destroy
 
@@ -15,6 +16,10 @@ class WebHook < ActiveRecord::Base
 
   before_save :strip_url
 
+  def tag_names=(tag_names_arg)
+    DiscourseTagging.add_or_create_tags_by_name(self, tag_names_arg, unlimited: true)
+  end
+
   def self.content_types
     @content_types ||= Enum.new('application/json' => 1,
                                 'application/x-www-form-urlencoded' => 2)
@@ -68,6 +73,7 @@ class WebHook < ActiveRecord::Base
       WebHook.enqueue_hooks(:topic, event,
         id: topic.id,
         category_id: topic&.category_id,
+        tag_ids: topic&.tags.pluck(:id),
         payload: payload
       )
     end
@@ -80,6 +86,7 @@ class WebHook < ActiveRecord::Base
       WebHook.enqueue_hooks(:post, event,
         id: post.id,
         category_id: post&.topic&.category_id,
+        tag_ids: post&.topic&.tags.pluck(:id),
         payload: payload
       )
     end
diff --git a/app/serializers/admin_web_hook_serializer.rb b/app/serializers/admin_web_hook_serializer.rb
index ff74ae3..711674b 100644
--- a/app/serializers/admin_web_hook_serializer.rb
+++ b/app/serializers/admin_web_hook_serializer.rb
@@ -10,6 +10,7 @@ class AdminWebHookSerializer < ApplicationSerializer
              :web_hook_event_types
 
   has_many :categories, serializer: BasicCategorySerializer, embed: :ids, include: false
+  has_many :tags, key: :tag_names, serializer: TagSerializer, embed: :ids, embed_key: :name, include: false
   has_many :groups, serializer: BasicGroupSerializer, embed: :ids, include: false
 
   def web_hook_event_types
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index b062705..de7874f 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3082,6 +3082,8 @@ en:
         active_notice: "We will deliver event details when it happens."
         categories_filter_instructions: "Relevant webhooks will only be triggered if the event is related with specified categories. Leave blank to trigger webhooks for all categories."
         categories_filter: "Triggered Categories"
+        tags_filter_instructions: "Relevant webhooks will only be triggered if the event is related with specified tags. Leave blank to trigger webhooks for all tags."
+        tags_filter: "Triggered Tags"
         groups_filter_instructions: "Relevant webhooks will only be triggered if the event is related with specified groups. Leave blank to trigger webhooks for all groups."
         groups_filter: "Triggered Groups"
         delete_confirm: "Delet

GitHub


#2

There’s a & operator missing. This should be topic&.tags&.pluck(:id)
Line 89 has the same problem.


Follow Up #3

#4

It won’t be a problem. topic&.tags will never return nil value. If tags are not attached to the topic then it will return an empty array. So pluck method will not raise an error there.


#5

But it looks like topic can be nil which would result in tags being nil too.
If topic can be nil, the safe navigation operator should be used everywhere (for topic.id too). If it can’t be nil there’s no need to use the operator.


#6

Ah, I thought & is safe enough to use in continuous calls if topic is nil.


#7

But can tags even theoretically be nil here? I thought it is unconditionally a relation object?

Ahhh I see yeah if topic is nil it is unsafe


#8

I would actually avoid the safe operator in this case, if you have no topic why bother raising a web hook?


#9

Fixed at Add missing safe navigation operator · discourse/discourse@fb78414 · GitHub


#10

I did few more improvements at DEV: remove unnecessary safe nav operators (#6730) · discourse/discourse@57ba4b7 · GitHub


Approved #11

#12