Skip to content

Commit

Permalink
Backend support for group pms
Browse files Browse the repository at this point in the history
  • Loading branch information
davidtaylorhq committed Jul 31, 2017
1 parent f1df1f5 commit 4b2e8af
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 22 deletions.
4 changes: 2 additions & 2 deletions app/controllers/chat_controller.rb
Expand Up @@ -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)

Expand All @@ -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'
Expand Down
22 changes: 18 additions & 4 deletions app/models/rule.rb
Expand Up @@ -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

Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion 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
21 changes: 13 additions & 8 deletions app/services/manager.rb
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions config/locales/server.en.yml
Expand Up @@ -60,6 +60,7 @@ en:

all_categories: "(all categories)"
deleted_category: "(deleted category)"
deleted_group: "(deleted group)"

provider:

Expand Down
46 changes: 39 additions & 7 deletions spec/models/rule_spec.rb
Expand Up @@ -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
Expand All @@ -28,7 +30,7 @@

rule = DiscourseChat::Rule.create({
channel: channel,
category_id: 1,
category_id: category.id,
tags: [tag1.name, tag2.name],
filter: 'watch'
})
Expand All @@ -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')

Expand All @@ -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
Expand Down Expand Up @@ -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')
Expand All @@ -128,7 +144,7 @@
DiscourseChat::Rule.create({
filter: 'watch',
channel: channel,
category_id: 1,
category_id: category.id,
})
end

Expand All @@ -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

Expand Down
27 changes: 27 additions & 0 deletions spec/services/manager_spec.rb
Expand Up @@ -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)}
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 4b2e8af

Please sign in to comment.