From 4b2e8af7114ee56fe030ebdf92dbc440e34fce47 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 28 Jul 2017 20:27:49 +0100 Subject: [PATCH] Backend support for group pms --- app/controllers/chat_controller.rb | 4 +-- app/models/rule.rb | 22 +++++++++++--- app/serializers/rule_serializer.rb | 13 ++++++++- app/services/manager.rb | 21 ++++++++------ config/locales/server.en.yml | 1 + spec/models/rule_spec.rb | 46 +++++++++++++++++++++++++----- spec/services/manager_spec.rb | 27 ++++++++++++++++++ 7 files changed, 112 insertions(+), 22 deletions(-) diff --git a/app/controllers/chat_controller.rb b/app/controllers/chat_controller.rb index 42879f80..82126ea2 100644 --- a/app/controllers/chat_controller.rb +++ b/app/controllers/chat_controller.rb @@ -118,7 +118,7 @@ def destroy_channel def create_rule begin - hash = params.require(:rule).permit(:channel_id, :filter, :category_id, tags:[]) + hash = params.require(:rule).permit(:channel_id, :filter, :group_id, :category_id, tags:[]) rule = DiscourseChat::Rule.new(hash) @@ -135,7 +135,7 @@ def create_rule def update_rule begin rule = DiscourseChat::Rule.find(params[:id].to_i) - hash = params.require(:rule).permit(:filter, :category_id, tags:[]) + hash = params.require(:rule).permit(:filter, :group_id, :category_id, tags:[]) if not rule.update(hash) raise Discourse::InvalidParameters, 'Rule is not valid' diff --git a/app/models/rule.rb b/app/models/rule.rb index 6ccd4f47..e485deb6 100644 --- a/app/models/rule.rb +++ b/app/models/rule.rb @@ -2,7 +2,7 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel KEY_PREFIX = 'rule:' # Setup ActiveRecord::Store to use the JSON field to read/write these values - store :value, accessors: [ :channel_id, :category_id, :tags, :filter ], coder: JSON + store :value, accessors: [ :channel_id, :group_id, :category_id, :tags, :filter ], coder: JSON after_initialize :init_filter @@ -13,16 +13,29 @@ def init_filter validates :filter, :inclusion => { :in => %w(watch follow mute), :message => "%{value} is not a valid filter" } - validate :channel_valid?, :category_valid?, :tags_valid? + validate :channel_valid?, :category_and_group_valid?, :tags_valid? def channel_valid? - # Validate category + # Validate channel if not (DiscourseChat::Channel.where(id: channel_id).exists?) errors.add(:channel_id, "#{channel_id} is not a valid channel id") end end - def category_valid? + def category_and_group_valid? + if category_id and group_id + errors.add(:category_id, "cannot be specified in addition to group_id") + return + end + + if group_id + # Validate group + if not Group.where(id: group_id).exists? + errors.add(:group_id, "#{group_id} is not a valid group id") + end + return + end + # Validate category if not (category_id.nil? or Category.where(id: category_id).exists?) errors.add(:category_id, "#{category_id} is not a valid category id") @@ -79,6 +92,7 @@ def channel=(val) scope :with_channel_id, ->(channel_id) { where("value::json->>'channel_id'=?", channel_id.to_s)} scope :with_category_id, ->(category_id) { category_id.nil? ? where("(value::json->'category_id') IS NULL OR json_typeof(value::json->'category_id')='null'") : where("value::json->>'category_id'=?", category_id.to_s)} + scope :with_group_ids, ->(group_id) { where("value::json->>'group_id' IN (?)", group_id.map(&:to_s))} scope :order_by_precedence, ->{ order("CASE WHEN value::json->>'filter' = 'mute' THEN 1 diff --git a/app/serializers/rule_serializer.rb b/app/serializers/rule_serializer.rb index 57a05811..e3b9cb68 100644 --- a/app/serializers/rule_serializer.rb +++ b/app/serializers/rule_serializer.rb @@ -1,3 +1,14 @@ class DiscourseChat::RuleSerializer < ApplicationSerializer - attributes :id, :channel_id, :category_id, :tags, :filter + attributes :id, :channel_id, :group_id, :group_name, :category_id, :tags, :filter + + def group_name + if object.group_id + groups = Group.where(id:object.group_id) + if groups.exists? + return groups.first.name + else + return I18n.t("chat_integration.deleted_group") + end + end + end end \ No newline at end of file diff --git a/app/services/manager.rb b/app/services/manager.rb index 78bee222..d4fd9b9f 100644 --- a/app/services/manager.rb +++ b/app/services/manager.rb @@ -18,14 +18,19 @@ def self.trigger_notifications(post_id) 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 = DiscourseChat::Rule.with_category_id(topic.category_id) - - if topic.category # Also load the rules for the wildcard category - matching_rules += DiscourseChat::Rule.with_category_id(nil) + # Abort if topic is blank... this should never be the case + return if topic.blank? + + # If it's a private message, filter rules by groups, otherwise filter rules by category + if topic.archetype == Archetype.private_message + group_ids_with_access = topic.topic_allowed_groups.pluck(:group_id) + return if group_ids_with_access.empty? + matching_rules = DiscourseChat::Rule.with_group_ids(group_ids_with_access) + else + matching_rules = DiscourseChat::Rule.with_category_id(topic.category_id) + if topic.category # Also load the rules for the wildcard category + matching_rules += DiscourseChat::Rule.with_category_id(nil) + end end # If tagging is enabled, thow away rules that don't apply to this topic diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 559b4914..7b889d16 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -60,6 +60,7 @@ en: all_categories: "(all categories)" deleted_category: "(deleted category)" + deleted_group: "(deleted group)" provider: diff --git a/spec/models/rule_spec.rb b/spec/models/rule_spec.rb index f2927419..bace0c62 100644 --- a/spec/models/rule_spec.rb +++ b/spec/models/rule_spec.rb @@ -8,6 +8,8 @@ let(:tag2){Fabricate(:tag)} let(:channel){DiscourseChat::Channel.create(provider:'dummy')} + let(:category) {Fabricate(:category)} + let(:group) {Fabricate(:group)} describe '.alloc_key' do it 'should return sequential numbers' do @@ -28,7 +30,7 @@ rule = DiscourseChat::Rule.create({ channel: channel, - category_id: 1, + category_id: category.id, tags: [tag1.name, tag2.name], filter: 'watch' }) @@ -38,7 +40,7 @@ loadedRule = DiscourseChat::Rule.find(rule.id) expect(loadedRule.channel.id).to eq(channel.id) - expect(loadedRule.category_id).to eq(1) + expect(loadedRule.category_id).to eq(category.id) expect(loadedRule.tags).to contain_exactly(tag1.name,tag2.name) expect(loadedRule.filter).to eq('watch') @@ -48,7 +50,7 @@ before do rule = DiscourseChat::Rule.create({ channel: channel, - category_id: 1, + category_id: category.id, tags: [tag1.name, tag2.name] }) end @@ -102,15 +104,29 @@ end it 'can be filtered by category' do - rule2 = DiscourseChat::Rule.create(channel:channel, category_id: 1) + rule2 = DiscourseChat::Rule.create(channel:channel, category_id: category.id) rule3 = DiscourseChat::Rule.create(channel:channel, category_id: nil) expect(DiscourseChat::Rule.all.length).to eq(3) - expect(DiscourseChat::Rule.with_category_id(1).length).to eq(2) + expect(DiscourseChat::Rule.with_category_id(category.id).length).to eq(2) expect(DiscourseChat::Rule.with_category_id(nil).length).to eq(1) end + it 'can be filtered by group' do + group1 = Fabricate(:group) + group2 = Fabricate(:group) + rule2 = DiscourseChat::Rule.create(channel:channel, group_id: group1.id) + rule3 = DiscourseChat::Rule.create(channel:channel, group_id: group2.id) + + expect(DiscourseChat::Rule.all.length).to eq(3) + + expect(DiscourseChat::Rule.with_category_id(category.id).length).to eq(1) + expect(DiscourseChat::Rule.with_group_ids([group1.id,group2.id]).length).to eq(2) + expect(DiscourseChat::Rule.with_group_ids([group1.id]).length).to eq(1) + expect(DiscourseChat::Rule.with_group_ids([group2.id]).length).to eq(1) + end + it 'can be sorted by precedence' do rule2 = DiscourseChat::Rule.create(channel:channel, filter:'mute') rule3 = DiscourseChat::Rule.create(channel:channel, filter:'follow') @@ -128,7 +144,7 @@ DiscourseChat::Rule.create({ filter: 'watch', channel: channel, - category_id: 1, + category_id: category.id, }) end @@ -140,9 +156,25 @@ expect(rule.valid?).to eq(false) end + it "doesn't allow both category and group to be set" do + expect(rule.valid?).to eq(true) + rule.group_id = group.id + expect(rule.valid?).to eq(false) + rule.category_id = nil + expect(rule.valid?).to eq(true) + end + + it 'validates group correctly' do + rule.category_id = nil + rule.group_id = group.id + expect(rule.valid?).to eq(true) + rule.group_id = -99 + expect(rule.valid?).to eq(false) + end + it 'validates category correctly' do expect(rule.valid?).to eq(true) - rule.category_id = 99 + rule.category_id = -99 expect(rule.valid?).to eq(false) end diff --git a/spec/services/manager_spec.rb b/spec/services/manager_spec.rb index 2180c048..e35cb09a 100644 --- a/spec/services/manager_spec.rb +++ b/spec/services/manager_spec.rb @@ -6,6 +6,7 @@ let(:manager) {::DiscourseChat::Manager} let(:category) {Fabricate(:category)} + let(:group) {Fabricate(:group)} 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)} @@ -107,6 +108,32 @@ expect(provider.sent_to_channel_ids).to contain_exactly() end + it "should work for group pms" do + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch' ) # Wildcard watch + DiscourseChat::Rule.create!(channel: chan2, filter: 'watch', group_id: group.id ) # Group watch + + private_post = Fabricate(:private_message_post) + private_post.topic.invite_group(Fabricate(:user), group) + + manager.trigger_notifications(private_post.id) + + expect(provider.sent_to_channel_ids).to contain_exactly(chan2.id) + end + + it "should work for pms with multiple groups" do + group2 = Fabricate(:group) + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', group_id: group.id ) + DiscourseChat::Rule.create!(channel: chan2, filter: 'watch', group_id: group2.id ) + + private_post = Fabricate(:private_message_post) + private_post.topic.invite_group(Fabricate(:user), group) + private_post.topic.invite_group(Fabricate(:user), group2) + + manager.trigger_notifications(private_post.id) + + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id, chan2.id) + end + it "should not notify about posts the chat_user cannot see" do DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: nil ) # Wildcard watch