Added filtering logic for multiple providers, along with many RSpec tests.

General structure of the filter rules is taken from discourse-slack-official, but re-written to be more robust and easier to understand.
This commit is contained in:
David Taylor 2017-06-27 19:21:27 +01:00
parent 3967e2cd91
commit 02692cf100
11 changed files with 339 additions and 34 deletions

View File

@ -2,7 +2,7 @@ import { ajax } from 'discourse/lib/ajax';
export default Discourse.Route.extend({
model() {
return ajax("/chat/list-integrations.json").then(result => {
return ajax("/chat/list-providers.json").then(result => {
return result.chat;
});
}

View File

@ -1,3 +1,5 @@
plugins:
discourse_chat_enabled:
chat_enabled:
default: false
chat_discourse_username:
default: system

View File

@ -1,12 +0,0 @@
module DiscourseChat
module Integration
def self.integrations
constants.select do |constant|
constant.to_s =~ /Integration$/
end.map(&method(:const_get))
end
end
end
require_relative "integration/slack/slack_integration.rb"
require_relative "integration/telegram/telegram_integration.rb"

View File

@ -1,7 +0,0 @@
module DiscourseChat
module Integration
module SlackIntegration
INTEGRATION_NAME = "slack".freeze
end
end
end

View File

@ -1,7 +0,0 @@
module DiscourseChat
module Integration
module TelegramIntegration
INTEGRATION_NAME = "telegram".freeze
end
end
end

97
lib/manager.rb Normal file
View File

@ -0,0 +1,97 @@
module DiscourseChat
module Manager
KEY_PREFIX = 'category_'.freeze
def self.guardian
Guardian.new(User.find_by(username: SiteSetting.chat_discourse_username))
end
def self.get_store_key(cat_id = nil)
"#{KEY_PREFIX}#{cat_id.present? ? cat_id : '*'}"
end
def self.get_rules_for_category(cat_id = nil)
PluginStore.get(DiscourseChat::PLUGIN_NAME, get_store_key(cat_id)) || []
end
def self.trigger_notifications(post_id)
Rails.logger.info("Triggering chat notifications for post #{post_id}")
post = Post.find_by(id: post_id)
# Abort if the chat_user doesn't have permission to see the post
return if !guardian.can_see?(post)
# Abort if the post is blank, or is non-regular (e.g. a "topic closed" notification)
return if post.blank? || post.post_type != Post.types[:regular]
topic = post.topic
# Abort if a private message (possible TODO: Add support for notifying about group PMs)
return if topic.blank? || topic.archetype == Archetype.private_message
# Load all the rules that apply to this topic's category
matching_rules = get_rules_for_category(topic.category_id)
if topic.category # Also load the rules for the wildcard category
matching_rules += get_rules_for_category(nil)
end
# If tagging is enabled, thow away rules that don't apply to this topic
if SiteSetting.tagging_enabled
topic_tags = topic.tags.present? ? topic.tags.pluck(:name) : []
matching_rules = matching_rules.select do |rule|
next true if rule[:tags].nil? or rule[:tags].empty? # Filter has no tags specified
any_tags_match = !((rule[:tags] & topic_tags).empty?)
next any_tags_match # If any tags match, keep this filter, otherwise throw away
end
end
# Sort by order of precedence (mute always wins; watch beats follow)
precedence = { 'mute' => 0, 'watch' => 1, 'follow' => 2}
sort_func = proc { |a, b| precedence[a[:filter]] <=> precedence[b[:filter]] }
matching_rules = matching_rules.sort(&sort_func)
# Take the first rule for each channel
uniq_func = proc { |rule| rule.values_at(:provider, :channel) }
matching_rules = matching_rules.uniq(&uniq_func)
# If a matching rule is set to mute, we can discard it now
matching_rules = matching_rules.select { |rule| rule[:filter] != "mute" }
# If this is not the first post, discard all "follow" rules
if not post.is_first_post?
matching_rules = matching_rules.select { |rule| rule[:filter] != "follow" }
end
# All remaining rules now require a notification to be sent
# If there are none left, abort
return false if matching_rules.empty?
# Loop through each rule, and trigger appropriate notifications
matching_rules.each do |rule|
Rails.logger.info("Sending notification to provider #{rule[:provider]}, channel #{rule[:channel]}")
provider = ::DiscourseChat::Provider.get_by_name(rule[:provider])
if provider
provider.trigger_notification(post, rule[:channel])
else
puts "Can't find provider"
# TODO: Handle when the provider does not exist
end
end
end
def self.create_rule(provider, channel, filter, category_id, tags)
raise "Invalid filter" if !['mute','follow','watch'].include?(filter)
data = get_rules_for_category(category_id)
tags = Tag.where(name: tags).pluck(:name)
tags = nil if tags.blank?
data.push(provider: provider, channel: channel, filter: filter, tags: tags)
PluginStore.set(DiscourseChat::PLUGIN_NAME, get_store_key(category_id), data)
end
end
end

17
lib/provider.rb Normal file
View File

@ -0,0 +1,17 @@
module DiscourseChat
module Provider
def self.providers
constants.select do |constant|
constant.to_s =~ /Provider$/
end.map(&method(:const_get))
end
def self.get_by_name(name)
self.providers.find{|p| p::PROVIDER_NAME == name}
end
end
end
require_relative "provider/slack/slack_provider.rb"
require_relative "provider/telegram/telegram_provider.rb"

View File

@ -0,0 +1,7 @@
module DiscourseChat
module Provider
module SlackProvider
PROVIDER_NAME = "slack".freeze
end
end
end

View File

@ -0,0 +1,7 @@
module DiscourseChat
module Provider
module TelegramProvider
PROVIDER_NAME = "telegram".freeze
end
end
end

View File

@ -3,7 +3,7 @@
# version: 0.1
# url: https://github.com/discourse/discourse-chat
enabled_site_setting :discourse_chat_enabled
enabled_site_setting :chat_enabled
after_initialize do
@ -16,13 +16,20 @@ after_initialize do
end
end
require_relative "lib/integration"
require_relative "lib/provider"
require_relative "lib/manager"
DiscourseEvent.on(:post_created) do |post|
if SiteSetting.chat_enabled?
::DiscourseChat::Manager.trigger_notifications(post.id)
end
end
class ::DiscourseChat::ChatController < ::ApplicationController
requires_plugin DiscourseChat::PLUGIN_NAME
def list_integrations
render json: ::DiscourseChat::Integration.integrations.map {|x| x::INTEGRATION_NAME}
def list_providers
render json: ::DiscourseChat::Provider.providers.map {|x| x::PROVIDER_NAME}
end
end
@ -33,7 +40,7 @@ after_initialize do
add_admin_route 'chat.menu_title', 'chat'
DiscourseChat::Engine.routes.draw do
get "/list-integrations" => "chat#list_integrations", constraints: AdminConstraint.new
get "/list-providers" => "chat#list_providers", constraints: AdminConstraint.new
end
Discourse::Application.routes.prepend do

194
spec/lib/manager_spec.rb Normal file
View File

@ -0,0 +1,194 @@
require 'rails_helper'
require_dependency 'post_creator'
RSpec.describe DiscourseChat::Manager do
let(:manager) {::DiscourseChat::Manager}
let(:category) {Fabricate(:category)}
let(:topic){Fabricate(:topic, category_id: category.id )}
let(:first_post) {Fabricate(:post, topic: topic)}
let(:second_post) {Fabricate(:post, topic: topic, post_number:2)}
describe '.trigger_notifications' do
before(:each) do
module ::DiscourseChat::Provider::DummyProvider
PROVIDER_NAME = "dummy".freeze
@@sent_messages = []
def self.trigger_notification(post, channel)
@@sent_messages.push(post: post.id, channel: channel)
end
def self.sent_messages
@@sent_messages
end
end
end
after(:each) do
::DiscourseChat::Provider.send(:remove_const, :DummyProvider)
end
let(:provider) {::DiscourseChat::Provider::DummyProvider}
it "should send a notification to watched and following channels for new topic" do
manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
manager.create_rule('dummy', 'chan2', 'follow', category.id, nil)
manager.create_rule('dummy', 'chan3', 'mute', category.id, nil)
manager.trigger_notifications(first_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1', 'chan2')
end
it "should send a notification only to watched for reply" do
manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
manager.create_rule('dummy', 'chan2', 'follow', category.id, nil)
manager.create_rule('dummy', 'chan3', 'mute', category.id, nil)
manager.trigger_notifications(second_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1')
end
it "should respect wildcard category settings" do
manager.create_rule('dummy', 'chan1', 'watch', nil, nil)
manager.trigger_notifications(first_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1')
end
it "should respect mute over watch" do
manager.create_rule('dummy', 'chan1', 'watch', nil, nil) # Wildcard watch
manager.create_rule('dummy', 'chan1', 'mute', category.id, nil) # Specific mute
manager.trigger_notifications(first_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly()
end
it "should respect watch over follow" do
manager.create_rule('dummy', 'chan1', 'follow', nil, nil)
manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
manager.trigger_notifications(second_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1')
end
it "should not notify about private messages" do
manager.create_rule('dummy', 'chan1', 'watch', nil, nil)
private_post = Fabricate(:private_message_post)
manager.trigger_notifications(private_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly()
end
it "should not notify about private messages" do
manager.create_rule('dummy', 'chan1', 'watch', nil, nil)
private_post = Fabricate(:private_message_post)
manager.trigger_notifications(private_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly()
end
it "should not notify about posts the chat_user cannot see" do
manager.create_rule('dummy', 'chan1', 'watch', nil, nil)
# Create a group & user
group = Fabricate(:group, name: "friends")
user = Fabricate(:user, username: 'david')
group.add(user)
# Set the chat_user to the newly created non-admin user
SiteSetting.chat_discourse_username = 'david'
# Create a category
category = Fabricate(:category, name: "Test category")
topic.category = category
topic.save!
# Restrict category to admins only
category.set_permissions(Group[:admins] => :full)
category.save!
# Check no notification sent
manager.trigger_notifications(first_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly()
# Now expose category to new user
category.set_permissions(Group[:friends] => :full)
category.save!
# Check notification sent
manager.trigger_notifications(first_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1')
end
describe 'with tags enabled' do
let(:tag){Fabricate(:tag, name:'gsoc')}
let(:tagged_topic){Fabricate(:topic, category_id: category.id, tags: [tag])}
let(:tagged_first_post) {Fabricate(:post, topic: tagged_topic)}
before(:each) do
SiteSetting.tagging_enabled = true
end
it 'should still work for rules without any tags specified' do
manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
manager.trigger_notifications(first_post.id)
manager.trigger_notifications(tagged_first_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1','chan1')
end
it 'should only match tagged topics when rule has tags' do
manager.create_rule('dummy', 'chan1', 'watch', category.id, [tag.name])
manager.trigger_notifications(first_post.id)
manager.trigger_notifications(tagged_first_post.id)
expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1')
end
end
end
describe '.create_rule' do
it 'should add new rule correctly' do
expect do
manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
end.to change { manager.get_rules_for_category(category.id).length }.by(1)
expect do
manager.create_rule('dummy', 'chan2', 'follow', category.id, nil)
end.to change { manager.get_rules_for_category(category.id).length }.by(1)
end
it 'should accept tags correctly' do
tag = Fabricate(:tag)
expect do
manager.create_rule('dummy', 'chan1', 'watch', category.id, [tag.name, 'faketag'])
end.to change { manager.get_rules_for_category(category.id).length }.by(1)
expect(manager.get_rules_for_category(category.id).first[:tags]).to contain_exactly(tag.name)
end
it 'should error on invalid filter strings' do
expect do
manager.create_rule('dummy', 'chan1', 'invalid_filter', category.id, nil)
end.to raise_error(RuntimeError, "Invalid filter")
end
end
end