From e07a4da46036f21f12a680b14fb2a7647efab9f6 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 13 Jul 2017 16:09:34 +0100 Subject: [PATCH 01/15] Model changes --- app/models/channel.rb | 32 +++++++++++++- app/models/rule.rb | 43 ++++++++---------- app/services/manager.rb | 4 +- spec/models/channel_spec.rb | 64 +++++++++++++++++++++++++++ spec/models/rule_spec.rb | 87 +++++++++++++++++-------------------- 5 files changed, 154 insertions(+), 76 deletions(-) create mode 100644 spec/models/channel_spec.rb diff --git a/app/models/channel.rb b/app/models/channel.rb index 5077a77..7317836 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -2,6 +2,36 @@ class DiscourseChat::Channel < DiscourseChat::PluginModel KEY_PREFIX = 'channel:' # Setup ActiveRecord::Store to use the JSON field to read/write these values - store :value, accessors: [ :name ], coder: JSON + store :value, accessors: [ :provider, :descriptor ], coder: JSON + + validate :provider_and_descriptor_valid? + + def provider_and_descriptor_valid? + # Validate provider + if not ::DiscourseChat::Provider.provider_names.include? provider + errors.add(:provider, "#{provider} is not a valid provider") + return + end + + # Validate descriptor + if descriptor.blank? + errors.add(:descriptor, "channel descriptor cannot be blank") + return + end + + provider_class = ::DiscourseChat::Provider.get_by_name(provider) + if defined? provider_class::PROVIDER_CHANNEL_REGEX + channel_regex = Regexp.new provider_class::PROVIDER_CHANNEL_REGEX + if not channel_regex.match?(descriptor) + errors.add(:descriptor, "#{descriptor} is not a valid channel descriptor for provider #{provider}") + end + end + end + + def rules + DiscourseChat::Rule.with_channel_id(id) + end + + scope :with_provider, ->(provider) { where("value::json->>'provider'=?", provider)} end \ No newline at end of file diff --git a/app/models/rule.rb b/app/models/rule.rb index b2f3707..20623d9 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: [ :provider, :channel, :category_id, :tags, :filter, :error_key ], coder: JSON + store :value, accessors: [ :channel_id, :category_id, :tags, :filter, :error_key ], coder: JSON after_initialize :init_filter @@ -13,27 +13,12 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel validates :filter, :inclusion => { :in => %w(watch follow mute), :message => "%{value} is not a valid filter" } - validate :provider_and_channel_valid?, :category_valid?, :tags_valid? + validate :channel_valid?, :category_valid?, :tags_valid? - def provider_and_channel_valid? - # Validate provider - if not ::DiscourseChat::Provider.provider_names.include? provider - errors.add(:provider, "#{provider} is not a valid provider") - return - end - - # Validate channel - if channel.blank? - errors.add(:channel, "channel cannot be blank") - return - end - - provider_class = ::DiscourseChat::Provider.get_by_name(provider) - if defined? provider_class::PROVIDER_CHANNEL_REGEX - channel_regex = Regexp.new provider_class::PROVIDER_CHANNEL_REGEX - if not channel_regex.match?(channel) - errors.add(:channel, "#{channel} is not a valid channel for provider #{provider}") - end + def channel_valid? + # Validate category + if not (channel_id.nil? or DiscourseChat::Channel.where(id: channel_id).exists?) + errors.add(:channel_id, "#{channel_id} is not a valid channel id") end end @@ -72,12 +57,18 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel end end - scope :with_provider, ->(provider) { where("value::json->>'provider'=?", provider)} + # Mock foreign key + # Could raise RecordNotFound + def channel + DiscourseChat::Channel.find(channel_id) + end + def channel=(val) + self.channel_id = val.id + end - scope :with_channel, ->(provider, channel) { with_provider(provider).where("value::json->>'channel'=?", channel)} + scope :with_channel, ->(channel) { with_channel_id(channel.id) } + scope :with_channel_id, ->(channel_id) { where("value::json->>'channel_id'=?", channel_id.to_s)} - scope :with_category, ->(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_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)} end \ No newline at end of file diff --git a/app/services/manager.rb b/app/services/manager.rb index 5368acc..1cf5783 100644 --- a/app/services/manager.rb +++ b/app/services/manager.rb @@ -22,10 +22,10 @@ module DiscourseChat 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(topic.category_id) + 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(nil) + matching_rules += DiscourseChat::Rule.with_category_id(nil) end # If tagging is enabled, thow away rules that don't apply to this topic diff --git a/spec/models/channel_spec.rb b/spec/models/channel_spec.rb new file mode 100644 index 0000000..6a424e1 --- /dev/null +++ b/spec/models/channel_spec.rb @@ -0,0 +1,64 @@ +require 'rails_helper' +require_relative '../dummy_provider' + +RSpec.describe DiscourseChat::Channel do + include_context "dummy provider" + + + it 'should save and load successfully' do + expect(DiscourseChat::Channel.all.length).to eq(0) + + chan = DiscourseChat::Channel.create({ + provider:"dummy", + descriptor: "#random", + }) + + expect(DiscourseChat::Channel.all.length).to eq(1) + + loadedChan = DiscourseChat::Channel.find(chan.id) + + expect(loadedChan.provider).to eq('dummy') + expect(loadedChan.descriptor).to eq('#random') + + end + + it 'can be filtered by provider' do + channel1 = DiscourseChat::Channel.create({provider:'dummy', descriptor:'blah'}) + channel2 = DiscourseChat::Channel.create({provider:'slack', descriptor:'#blah'}) + channel3 = DiscourseChat::Channel.create({provider:'slack', descriptor:'#blah'}) + + expect(DiscourseChat::Channel.all.length).to eq(3) + + expect(DiscourseChat::Channel.with_provider('slack').length).to eq(2) + expect(DiscourseChat::Channel.with_provider('dummy').length).to eq(1) + end + + it 'can find its own rules' do + channel = DiscourseChat::Channel.create({provider:'dummy', descriptor:'blah'}) + expect(channel.rules.size).to eq(0) + DiscourseChat::Rule.create(channel: channel) + DiscourseChat::Rule.create(channel: channel) + expect(channel.rules.size).to eq(2) + + end + + describe 'validations' do + let(:channel) { DiscourseChat::Channel.create( + provider:"dummy", + descriptor: "#general" + ) } + + it 'validates provider correctly' do + expect(channel.valid?).to eq(true) + channel.provider = 'somerandomprovider' + expect(channel.valid?).to eq(false) + end + + it 'validates channel correctly' do + expect(channel.valid?).to eq(true) + channel.descriptor = '' + expect(channel.valid?).to eq(false) + end + + end +end \ No newline at end of file diff --git a/spec/models/rule_spec.rb b/spec/models/rule_spec.rb index 595f0c0..27b7369 100644 --- a/spec/models/rule_spec.rb +++ b/spec/models/rule_spec.rb @@ -1,24 +1,33 @@ require 'rails_helper' +require_relative '../dummy_provider' RSpec.describe DiscourseChat::Rule do + include_context "dummy provider" let(:tag1){Fabricate(:tag)} let(:tag2){Fabricate(:tag)} + let(:channel){DiscourseChat::Channel.create(provider:'dummy', descriptor:'#general')} + describe '.alloc_key' do it 'should return sequential numbers' do - expect( DiscourseChat::Rule.create(provider:'slack',channel:'#general').key ).to eq("rule:1") - expect( DiscourseChat::Rule.create(provider:'slack',channel:'#general').key ).to eq("rule:2") - expect( DiscourseChat::Rule.create(provider:'slack',channel:'#general').key ).to eq("rule:3") + expect( DiscourseChat::Rule.create(channel: channel).key ).to eq("rule:1") + expect( DiscourseChat::Rule.create(channel: channel).key ).to eq("rule:2") + expect( DiscourseChat::Rule.create(channel: channel).key ).to eq("rule:3") end end + it 'should convert between channel and channel_id successfully' do + rule = DiscourseChat::Rule.create(channel: channel) + expect( rule.channel_id ).to eq(channel.id) + expect( rule.channel.id ).to eq(channel.id) + end + it 'should save and load successfully' do expect(DiscourseChat::Rule.all.length).to eq(0) rule = DiscourseChat::Rule.create({ - provider:"slack", - channel: "#general", + channel: channel, category_id: 1, tags: [tag1.name, tag2.name], filter: 'watch' @@ -28,8 +37,7 @@ RSpec.describe DiscourseChat::Rule do loadedRule = DiscourseChat::Rule.find(rule.id) - expect(loadedRule.provider).to eq('slack') - expect(loadedRule.channel).to eq('#general') + expect(loadedRule.channel.id).to eq(channel.id) expect(loadedRule.category_id).to eq(1) expect(loadedRule.tags).to contain_exactly(tag1.name,tag2.name) expect(loadedRule.filter).to eq('watch') @@ -39,8 +47,7 @@ RSpec.describe DiscourseChat::Rule do describe 'general operations' do before do rule = DiscourseChat::Rule.create({ - provider:"slack", - channel: "#general", + channel: channel, category_id: 1, tags: [tag1.name, tag2.name] }) @@ -48,16 +55,16 @@ RSpec.describe DiscourseChat::Rule do it 'can be modified' do rule = DiscourseChat::Rule.all.first - rule.channel = "#random" + rule.tags = [tag1.name] rule.save! rule = DiscourseChat::Rule.all.first - expect(rule.channel).to eq('#random') + expect(rule.tags).to contain_exactly(tag1.name) end it 'can be deleted' do - DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save! + DiscourseChat::Rule.new(channel:channel).save! expect(DiscourseChat::Rule.all.length).to eq(2) rule = DiscourseChat::Rule.all.first @@ -67,10 +74,10 @@ RSpec.describe DiscourseChat::Rule do end it 'can delete all' do - DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save! - DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save! - DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save! - DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save! + DiscourseChat::Rule.create({channel:channel}) + DiscourseChat::Rule.create({channel:channel}) + DiscourseChat::Rule.create({channel:channel}) + DiscourseChat::Rule.create({channel:channel}) expect(DiscourseChat::Rule.all.length).to eq(5) @@ -79,36 +86,29 @@ RSpec.describe DiscourseChat::Rule do expect(DiscourseChat::Rule.all.length).to eq(0) end - it 'can be filtered by provider' do - rule2 = DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save! - rule3 = DiscourseChat::Rule.new({provider:'slack', channel:'#blah'}).save! - - expect(DiscourseChat::Rule.all.length).to eq(3) - - expect(DiscourseChat::Rule.with_provider('slack').length).to eq(2) - expect(DiscourseChat::Rule.with_provider('telegram').length).to eq(1) - end - it 'can be filtered by channel' do - rule2 = DiscourseChat::Rule.new({provider:'telegram', channel:'blah'}).save! - rule3 = DiscourseChat::Rule.new({provider:'slack', channel:'#blah'}).save! - rule4 = DiscourseChat::Rule.new({provider:'slack', channel:'#general'}).save! - rule5 = DiscourseChat::Rule.new({provider:'slack', channel:'#general'}).save! + channel2 = DiscourseChat::Channel.create(provider:'dummy', descriptor:'#random') + channel3 = DiscourseChat::Channel.create(provider:'dummy', descriptor:'#another') + rule2 = DiscourseChat::Rule.create(channel:channel) + rule3 = DiscourseChat::Rule.create(channel:channel) + rule4 = DiscourseChat::Rule.create(channel:channel2) + rule5 = DiscourseChat::Rule.create(channel:channel3) + expect(DiscourseChat::Rule.all.length).to eq(5) - expect(DiscourseChat::Rule.with_channel('slack','#general').length).to eq(3) - expect(DiscourseChat::Rule.with_channel('slack', '#blah').length).to eq(1) + expect(DiscourseChat::Rule.with_channel(channel).length).to eq(3) + expect(DiscourseChat::Rule.with_channel(channel2).length).to eq(1) end it 'can be filtered by category' do - rule2 = DiscourseChat::Rule.new({provider:'slack', channel:'#blah', category_id: 1}).save! - rule3 = DiscourseChat::Rule.new({provider:'slack', channel:'#blah', category_id: nil}).save! + rule2 = DiscourseChat::Rule.create(channel:channel, category_id: 1) + rule3 = DiscourseChat::Rule.create(channel:channel, category_id: nil) expect(DiscourseChat::Rule.all.length).to eq(3) - expect(DiscourseChat::Rule.with_category(1).length).to eq(2) - expect(DiscourseChat::Rule.with_category(nil).length).to eq(1) + expect(DiscourseChat::Rule.with_category_id(1).length).to eq(2) + expect(DiscourseChat::Rule.with_category_id(nil).length).to eq(1) end end @@ -117,23 +117,16 @@ RSpec.describe DiscourseChat::Rule do let(:rule) do DiscourseChat::Rule.create({ filter: 'watch', - provider:"slack", - channel: "#general", + channel: channel, category_id: 1, }) end - it 'validates provider correctly' do - expect(rule.valid?).to eq(true) - rule.provider = 'somerandomprovider' - expect(rule.valid?).to eq(false) - end - it 'validates channel correctly' do expect(rule.valid?).to eq(true) - rule.channel = '' + rule.channel_id = 'blahblahblah' expect(rule.valid?).to eq(false) - rule.channel = 'blah' + rule.channel_id = -1 expect(rule.valid?).to eq(false) end @@ -167,7 +160,7 @@ RSpec.describe DiscourseChat::Rule do it "doesn't allow save when invalid" do expect(rule.valid?).to eq(true) - rule.provider = 'somerandomprovider' + rule.filter = 'somerandomfilter' expect(rule.valid?).to eq(false) expect(rule.save).to eq(false) end From 4b25dcec8f277a12c48094e8dc5e5a551a0ef7f6 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 13 Jul 2017 20:50:45 +0100 Subject: [PATCH 02/15] Allow providers to define a data schema for their channel parameters --- app/models/channel.rb | 35 ++++++++++------ .../provider/slack/slack_provider.rb | 2 +- .../provider/telegram/telegram_provider.rb | 4 +- spec/dummy_provider.rb | 32 ++++++++++++++- spec/models/channel_spec.rb | 41 ++++++++++++------- spec/models/rule_spec.rb | 6 +-- 6 files changed, 86 insertions(+), 34 deletions(-) diff --git a/app/models/channel.rb b/app/models/channel.rb index 7317836..00e72f0 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -2,28 +2,39 @@ class DiscourseChat::Channel < DiscourseChat::PluginModel KEY_PREFIX = 'channel:' # Setup ActiveRecord::Store to use the JSON field to read/write these values - store :value, accessors: [ :provider, :descriptor ], coder: JSON + store :value, accessors: [ :provider, :data ], coder: JSON - validate :provider_and_descriptor_valid? + after_initialize :init_data - def provider_and_descriptor_valid? + def init_data + self.data = {} if self.data.nil? + end + + validate :provider_valid?, :data_valid? + + def provider_valid? # Validate provider if not ::DiscourseChat::Provider.provider_names.include? provider errors.add(:provider, "#{provider} is not a valid provider") return end - - # Validate descriptor - if descriptor.blank? - errors.add(:descriptor, "channel descriptor cannot be blank") + end + + def data_valid? + # If provider is invalid, don't try and check data + return unless ::DiscourseChat::Provider.provider_names.include? provider + + params = ::DiscourseChat::Provider.get_by_name(provider)::CHANNEL_PARAMETERS + + unless params.keys.sort == data.keys.sort + errors.add(:data, "data does not match the required structure for provider #{provider}") return end - provider_class = ::DiscourseChat::Provider.get_by_name(provider) - if defined? provider_class::PROVIDER_CHANNEL_REGEX - channel_regex = Regexp.new provider_class::PROVIDER_CHANNEL_REGEX - if not channel_regex.match?(descriptor) - errors.add(:descriptor, "#{descriptor} is not a valid channel descriptor for provider #{provider}") + data.each do |key, value| + regex_string = params[key] + if !Regexp.new(regex_string).match?(value) + errors.add(:data, "data.#{key} is invalid") end end end diff --git a/lib/discourse_chat/provider/slack/slack_provider.rb b/lib/discourse_chat/provider/slack/slack_provider.rb index b4ee843..dbd3afe 100644 --- a/lib/discourse_chat/provider/slack/slack_provider.rb +++ b/lib/discourse_chat/provider/slack/slack_provider.rb @@ -3,7 +3,7 @@ module DiscourseChat::Provider::SlackProvider PROVIDER_ENABLED_SETTING = :chat_integration_slack_enabled - PROVIDER_CHANNEL_REGEX = '^[@#]\S*$' + CHANNEL_PARAMETERS = {"channel" => '^[@#]\S*$'} def self.excerpt(post, max_length = SiteSetting.chat_integration_slack_excerpt_length) doc = Nokogiri::HTML.fragment(post.excerpt(max_length, diff --git a/lib/discourse_chat/provider/telegram/telegram_provider.rb b/lib/discourse_chat/provider/telegram/telegram_provider.rb index d63aee6..8e7afed 100644 --- a/lib/discourse_chat/provider/telegram/telegram_provider.rb +++ b/lib/discourse_chat/provider/telegram/telegram_provider.rb @@ -1,10 +1,10 @@ module DiscourseChat module Provider module TelegramProvider - include Provider - PROVIDER_NAME = "telegram".freeze PROVIDER_ENABLED_SETTING = :chat_integration_telegram_enabled + CHANNEL_PARAMETERS = {} + end end end diff --git a/spec/dummy_provider.rb b/spec/dummy_provider.rb index 92c79f9..709f926 100644 --- a/spec/dummy_provider.rb +++ b/spec/dummy_provider.rb @@ -8,6 +8,8 @@ RSpec.shared_context "dummy provider" do module ::DiscourseChat::Provider::DummyProvider PROVIDER_NAME = "dummy".freeze PROVIDER_ENABLED_SETTING = :chat_integration_enabled # Tie to main plugin enabled setting + CHANNEL_PARAMETERS = {} + @@sent_messages = [] @@raise_exception = nil @@ -30,9 +32,37 @@ RSpec.shared_context "dummy provider" do end let(:provider){::DiscourseChat::Provider::DummyProvider} - +end + +RSpec.shared_context "validated dummy provider" do + before(:each) do + if defined? ::DiscourseChat::Provider::Dummy2Provider + ::DiscourseChat::Provider.send(:remove_const, :Dummy2Provider) + end + + module ::DiscourseChat::Provider::Dummy2Provider + PROVIDER_NAME = "dummy2".freeze + PROVIDER_ENABLED_SETTING = :chat_integration_enabled # Tie to main plugin enabled setting + CHANNEL_PARAMETERS = {"val" => '\S+'} + + @@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 + + let(:provider){::DiscourseChat::Provider::DummyProvider} end RSpec.configure do |rspec| rspec.include_context "dummy provider" + rspec.include_context "validated dummy provider" + end \ No newline at end of file diff --git a/spec/models/channel_spec.rb b/spec/models/channel_spec.rb index 6a424e1..4094bf0 100644 --- a/spec/models/channel_spec.rb +++ b/spec/models/channel_spec.rb @@ -3,6 +3,7 @@ require_relative '../dummy_provider' RSpec.describe DiscourseChat::Channel do include_context "dummy provider" + include_context "validated dummy provider" it 'should save and load successfully' do @@ -10,7 +11,6 @@ RSpec.describe DiscourseChat::Channel do chan = DiscourseChat::Channel.create({ provider:"dummy", - descriptor: "#random", }) expect(DiscourseChat::Channel.all.length).to eq(1) @@ -18,23 +18,22 @@ RSpec.describe DiscourseChat::Channel do loadedChan = DiscourseChat::Channel.find(chan.id) expect(loadedChan.provider).to eq('dummy') - expect(loadedChan.descriptor).to eq('#random') end it 'can be filtered by provider' do - channel1 = DiscourseChat::Channel.create({provider:'dummy', descriptor:'blah'}) - channel2 = DiscourseChat::Channel.create({provider:'slack', descriptor:'#blah'}) - channel3 = DiscourseChat::Channel.create({provider:'slack', descriptor:'#blah'}) + channel1 = DiscourseChat::Channel.create!(provider:'dummy') + channel2 = DiscourseChat::Channel.create!(provider:'dummy2', data:{val:"blah"}) + channel3 = DiscourseChat::Channel.create!(provider:'dummy2', data:{val:"blah"}) expect(DiscourseChat::Channel.all.length).to eq(3) - expect(DiscourseChat::Channel.with_provider('slack').length).to eq(2) + expect(DiscourseChat::Channel.with_provider('dummy2').length).to eq(2) expect(DiscourseChat::Channel.with_provider('dummy').length).to eq(1) end it 'can find its own rules' do - channel = DiscourseChat::Channel.create({provider:'dummy', descriptor:'blah'}) + channel = DiscourseChat::Channel.create({provider:'dummy'}) expect(channel.rules.size).to eq(0) DiscourseChat::Rule.create(channel: channel) DiscourseChat::Rule.create(channel: channel) @@ -43,21 +42,33 @@ RSpec.describe DiscourseChat::Channel do end describe 'validations' do - let(:channel) { DiscourseChat::Channel.create( - provider:"dummy", - descriptor: "#general" - ) } + let(:channel) { } it 'validates provider correctly' do + channel = DiscourseChat::Channel.create!(provider:"dummy") expect(channel.valid?).to eq(true) channel.provider = 'somerandomprovider' expect(channel.valid?).to eq(false) end - it 'validates channel correctly' do - expect(channel.valid?).to eq(true) - channel.descriptor = '' - expect(channel.valid?).to eq(false) + it 'succeeds with valid data' do + channel2 = DiscourseChat::Channel.new(provider:"dummy2", data:{val:"hello"}) + expect(channel2.valid?).to eq(true) + end + + it 'disallows invalid data' do + channel2 = DiscourseChat::Channel.new(provider:"dummy2", data:{val:''}) + expect(channel2.valid?).to eq(false) + end + + it 'disallows unknown keys' do + channel2 = DiscourseChat::Channel.new(provider:"dummy2", data:{val:"hello", unknown:"world"}) + expect(channel2.valid?).to eq(false) + end + + it 'requires all keys' do + channel2 = DiscourseChat::Channel.new(provider:"dummy2", data:{}) + expect(channel2.valid?).to eq(false) end end diff --git a/spec/models/rule_spec.rb b/spec/models/rule_spec.rb index 27b7369..0c218ad 100644 --- a/spec/models/rule_spec.rb +++ b/spec/models/rule_spec.rb @@ -7,7 +7,7 @@ RSpec.describe DiscourseChat::Rule do let(:tag1){Fabricate(:tag)} let(:tag2){Fabricate(:tag)} - let(:channel){DiscourseChat::Channel.create(provider:'dummy', descriptor:'#general')} + let(:channel){DiscourseChat::Channel.create(provider:'dummy')} describe '.alloc_key' do it 'should return sequential numbers' do @@ -87,8 +87,8 @@ RSpec.describe DiscourseChat::Rule do end it 'can be filtered by channel' do - channel2 = DiscourseChat::Channel.create(provider:'dummy', descriptor:'#random') - channel3 = DiscourseChat::Channel.create(provider:'dummy', descriptor:'#another') + channel2 = DiscourseChat::Channel.create(provider:'dummy') + channel3 = DiscourseChat::Channel.create(provider:'dummy') rule2 = DiscourseChat::Rule.create(channel:channel) rule3 = DiscourseChat::Rule.create(channel:channel) From 246c81ce5c1b4c88239557c8ed86a9b2ab80ee9c Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 13 Jul 2017 21:47:15 +0100 Subject: [PATCH 03/15] Update manager to deal with new channel structure --- app/services/manager.rb | 5 ++- spec/dummy_provider.rb | 4 ++ spec/services/manager_spec.rb | 85 ++++++++++++++++------------------- 3 files changed, 45 insertions(+), 49 deletions(-) diff --git a/app/services/manager.rb b/app/services/manager.rb index 1cf5783..ce49d5f 100644 --- a/app/services/manager.rb +++ b/app/services/manager.rb @@ -44,7 +44,7 @@ module DiscourseChat matching_rules = matching_rules.sort(&sort_func) # Take the first rule for each channel - uniq_func = proc { |rule| [rule.provider, rule.channel] } + uniq_func = proc { |rule| [rule.channel_id] } matching_rules = matching_rules.uniq(&uniq_func) # If a matching rule is set to mute, we can discard it now @@ -61,7 +61,8 @@ module DiscourseChat # Loop through each rule, and trigger appropriate notifications matching_rules.each do |rule| - provider = ::DiscourseChat::Provider.get_by_name(rule.provider) + channel = rule.channel + provider = ::DiscourseChat::Provider.get_by_name(channel.provider) is_enabled = ::DiscourseChat::Provider.is_enabled(provider) if provider and is_enabled diff --git a/spec/dummy_provider.rb b/spec/dummy_provider.rb index 709f926..19b5bc8 100644 --- a/spec/dummy_provider.rb +++ b/spec/dummy_provider.rb @@ -24,6 +24,10 @@ RSpec.shared_context "dummy provider" do @@sent_messages end + def self.sent_to_channel_ids + @@sent_messages.map{|x| x[:channel].id} + end + def self.set_raise_exception(bool) @@raise_exception = bool end diff --git a/spec/services/manager_spec.rb b/spec/services/manager_spec.rb index dd0225e..0055960 100644 --- a/spec/services/manager_spec.rb +++ b/spec/services/manager_spec.rb @@ -13,30 +13,30 @@ RSpec.describe DiscourseChat::Manager do describe '.trigger_notifications' do include_context "dummy provider" + let(:chan1){DiscourseChat::Channel.create!(provider:'dummy')} + let(:chan2){DiscourseChat::Channel.create!(provider:'dummy')} + let(:chan3){DiscourseChat::Channel.create!(provider:'dummy')} + before do SiteSetting.chat_integration_enabled = true end - def create_rule(provider, channel, filter, category_id, tags) # Just shorthand for testing purposes - DiscourseChat::Rule.create({provider: provider, channel: channel, filter:filter, category_id:category_id, tags:tags}) - end - it "should fail gracefully when a provider throws an exception" do - create_rule('dummy', 'chan1', 'watch', category.id, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id:category.id ) # Triggering a ProviderError should set the error_key to the error message - ::DiscourseChat::Provider::DummyProvider.set_raise_exception(DiscourseChat::ProviderError.new info: {error_key:"hello"}) + provider.set_raise_exception(DiscourseChat::ProviderError.new info: {error_key:"hello"}) manager.trigger_notifications(first_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly() + expect(provider.sent_to_channel_ids).to contain_exactly() expect(DiscourseChat::Rule.all.first.error_key).to eq('hello') # Triggering a different error should set the error_key to a generic message - ::DiscourseChat::Provider::DummyProvider.set_raise_exception(StandardError.new "hello") + provider.set_raise_exception(StandardError.new "hello") manager.trigger_notifications(first_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly() + expect(provider.sent_to_channel_ids).to contain_exactly() expect(DiscourseChat::Rule.all.first.error_key).to eq('chat_integration.rule_exception') - ::DiscourseChat::Provider::DummyProvider.set_raise_exception(nil) + provider.set_raise_exception(nil) manager.trigger_notifications(first_post.id) expect(DiscourseChat::Rule.all.first.error_key.nil?).to be true @@ -44,80 +44,71 @@ RSpec.describe DiscourseChat::Manager do it "should not send notifications when provider is disabled" do SiteSetting.chat_integration_enabled = false - create_rule('dummy', 'chan1', 'watch', category.id, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id:category.id ) manager.trigger_notifications(first_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly() + expect(provider.sent_to_channel_ids).to contain_exactly() end it "should send a notification to watched and following channels for new topic" do - - create_rule('dummy', 'chan1', 'watch', category.id, nil) - create_rule('dummy', 'chan2', 'follow', category.id, nil) - create_rule('dummy', 'chan3', 'mute', category.id, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id:category.id ) + DiscourseChat::Rule.create!(channel: chan2, filter: 'follow', category_id:category.id ) + DiscourseChat::Rule.create!(channel: chan3, filter: 'mute', category_id:category.id ) manager.trigger_notifications(first_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1', 'chan2') + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id, chan2.id) end it "should send a notification only to watched for reply" do - create_rule('dummy', 'chan1', 'watch', category.id, nil) - create_rule('dummy', 'chan2', 'follow', category.id, nil) - create_rule('dummy', 'chan3', 'mute', category.id, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id:category.id ) + DiscourseChat::Rule.create!(channel: chan2, filter: 'follow', category_id:category.id ) + DiscourseChat::Rule.create!(channel: chan3, filter: 'mute', category_id:category.id ) manager.trigger_notifications(second_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1') + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id) end it "should respect wildcard category settings" do - create_rule('dummy', 'chan1', 'watch', nil, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id: nil ) manager.trigger_notifications(first_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1') + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id) end it "should respect mute over watch" do - create_rule('dummy', 'chan1', 'watch', nil, nil) # Wildcard watch - create_rule('dummy', 'chan1', 'mute', category.id, nil) # Specific mute + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id: nil ) # Wildcard watch + DiscourseChat::Rule.create!(channel: chan1, filter: 'mute', category_id: category.id ) # Specific mute manager.trigger_notifications(first_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly() + expect(provider.sent_to_channel_ids).to contain_exactly() end it "should respect watch over follow" do - create_rule('dummy', 'chan1', 'follow', nil, nil) - create_rule('dummy', 'chan1', 'watch', category.id, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: nil ) # Wildcard watch + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch', category_id: category.id ) # Specific watch manager.trigger_notifications(second_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1') + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id) end it "should not notify about private messages" do - create_rule('dummy', 'chan1', 'watch', nil, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: nil ) # Wildcard watch + 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 - 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() + expect(provider.sent_to_channel_ids).to contain_exactly() end it "should not notify about posts the chat_user cannot see" do - create_rule('dummy', 'chan1', 'watch', nil, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: nil ) # Wildcard watch # Create a group & user group = Fabricate(:group, name: "friends") @@ -138,7 +129,7 @@ RSpec.describe DiscourseChat::Manager do # Check no notification sent manager.trigger_notifications(first_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly() + expect(provider.sent_to_channel_ids).to contain_exactly() # Now expose category to new user category.set_permissions(Group[:friends] => :full) @@ -146,7 +137,7 @@ RSpec.describe DiscourseChat::Manager do # Check notification sent manager.trigger_notifications(first_post.id) - expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly('chan1') + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id) end @@ -160,21 +151,21 @@ RSpec.describe DiscourseChat::Manager do end it 'should still work for rules without any tags specified' do - create_rule('dummy', 'chan1', 'watch', category.id, nil) + DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: nil ) # Wildcard watch 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') + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id, chan1.id) end it 'should only match tagged topics when rule has tags' do - create_rule('dummy', 'chan1', 'watch', category.id, [tag.name]) + DiscourseChat::Rule.create!(channel: chan1, filter: 'follow', category_id: category.id, tags:[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') + expect(provider.sent_to_channel_ids).to contain_exactly(chan1.id) end end From e850fb194b65307fe91bcaff2349c821dc37f9c5 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 13 Jul 2017 22:13:03 +0100 Subject: [PATCH 04/15] Update helper to deal with the new channel structure --- app/helpers/helper.rb | 15 ++-- spec/helpers/helper_spec.rb | 140 ++++++++++++++++-------------------- 2 files changed, 69 insertions(+), 86 deletions(-) diff --git a/app/helpers/helper.rb b/app/helpers/helper.rb index ba96b35..d04b4e9 100644 --- a/app/helpers/helper.rb +++ b/app/helpers/helper.rb @@ -2,8 +2,9 @@ module DiscourseChat module Helper # Produce a string with a list of all rules associated with a channel - def self.status_for_channel(provider, channel) - rules = DiscourseChat::Rule.with_channel(provider, channel) + def self.status_for_channel(channel) + rules = channel.rules + provider = channel.provider text = I18n.t("chat_integration.provider.#{provider}.status.header") + "\n" @@ -43,8 +44,8 @@ module DiscourseChat # Delete a rule based on its (1 based) index as seen in the # status_for_channel function - def self.delete_by_index(provider, channel, index) - rules = DiscourseChat::Rule.with_channel(provider, channel) + def self.delete_by_index(channel, index) + rules = DiscourseChat::Rule.with_channel(channel) return false if index < 1 or index > rules.size @@ -58,8 +59,8 @@ module DiscourseChat # :updated if an existing rule has been updated # :created if a new rule has been created # false if there was an error - def self.smart_create_rule(provider:, channel:, filter:, category_id:, tags:) - existing_rules = DiscourseChat::Rule.with_channel(provider, channel) + def self.smart_create_rule(channel:, filter:, category_id:nil, tags:nil) + existing_rules = DiscourseChat::Rule.with_channel(channel) # Select the ones that have the same category same_category = existing_rules.select { |rule| rule.category_id == category_id } @@ -100,7 +101,7 @@ module DiscourseChat end # This rule is unique! Create a new one: - return :created if Rule.new({provider: provider, channel: channel, filter: filter, category_id: category_id, tags: tags}).save + return :created if Rule.new(channel: channel, filter: filter, category_id: category_id, tags: tags).save return false # Error diff --git a/spec/helpers/helper_spec.rb b/spec/helpers/helper_spec.rb index 6cd73d7..d890b06 100644 --- a/spec/helpers/helper_spec.rb +++ b/spec/helpers/helper_spec.rb @@ -1,6 +1,13 @@ require 'rails_helper' +require_relative '../dummy_provider' RSpec.describe DiscourseChat::Manager do + include_context "dummy provider" + + let(:chan1){DiscourseChat::Channel.create!(provider:'dummy')} + let(:chan2){DiscourseChat::Channel.create!(provider:'dummy')} + + let(:category) {Fabricate(:category)} let(:category) {Fabricate(:category)} let(:tag1){Fabricate(:tag)} @@ -11,45 +18,36 @@ RSpec.describe DiscourseChat::Manager do context 'with no rules' do it 'includes the heading' do - string = DiscourseChat::Helper.status_for_channel('slack','#general') - expect(string).to include('Rules for this channel') + string = DiscourseChat::Helper.status_for_channel(chan1) + expect(string).to include('dummy.status.header') end it 'includes the no_rules string' do - string = DiscourseChat::Helper.status_for_channel('slack','#general') - expect(string).to include('no rules') + string = DiscourseChat::Helper.status_for_channel(chan1) + expect(string).to include('dummy.status.no_rules') end end context 'with some rules' do before do - DiscourseChat::Rule.new({provider: 'slack', channel: '#general', filter:'watch', category_id:category.id, tags:nil}).save! - DiscourseChat::Rule.new({provider: 'slack', channel: '#general', filter:'mute', category_id:nil, tags:nil}).save! - DiscourseChat::Rule.new({provider: 'slack', channel: '#general', filter:'follow', category_id:nil, tags:[tag1.name]}).save! - DiscourseChat::Rule.new({provider: 'slack', channel: '#otherchannel', filter:'watch', category_id:1, tags:nil}).save! + DiscourseChat::Rule.create!(channel: chan1, filter:'watch', category_id:category.id, tags:nil) + DiscourseChat::Rule.create!(channel: chan1, filter:'mute', category_id:nil, tags:nil) + DiscourseChat::Rule.create!(channel: chan1, filter:'follow', category_id:nil, tags:[tag1.name]) + DiscourseChat::Rule.create!(channel: chan2, filter:'watch', category_id:1, tags:nil) end it 'displays the correct rules' do - string = DiscourseChat::Helper.status_for_channel('slack','#general') - expect(string.scan('watch').size).to eq(1) - expect(string.scan('mute').size).to eq(1) - expect(string.scan('follow').size).to eq(1) - end - - it 'enumerates the rules correctly' do - string = DiscourseChat::Helper.status_for_channel('slack','#general') - expect(string.scan('1)').size).to eq(1) - expect(string.scan('2)').size).to eq(1) - expect(string.scan('3)').size).to eq(1) + string = DiscourseChat::Helper.status_for_channel(chan1) + expect(string.scan('status.rule_string').size).to eq(3) end it 'only displays tags for rules with tags' do - string = DiscourseChat::Helper.status_for_channel('slack','#general') - expect(string.scan('with tags').size).to eq(0) + string = DiscourseChat::Helper.status_for_channel(chan1) + expect(string.scan('rule_string_tags_suffix').size).to eq(0) SiteSetting.tagging_enabled = true - string = DiscourseChat::Helper.status_for_channel('slack','#general') - expect(string.scan('with tags').size).to eq(1) + string = DiscourseChat::Helper.status_for_channel(chan1) + expect(string.scan('rule_string_tags_suffix').size).to eq(1) end end @@ -64,46 +62,42 @@ RSpec.describe DiscourseChat::Manager do # Three identical rules, with different categories # Status will be sorted by category id, so they should # be in this order - rule1 = DiscourseChat::Rule.new({provider: 'slack', - channel: '#general', - filter: 'watch', - category_id: category.id, - tags: [tag1.name, tag2.name] - }).save! - rule2 = DiscourseChat::Rule.new({provider: 'slack', - channel: '#general', - filter: 'watch', - category_id: category2.id, - tags: [tag1.name, tag2.name] - }).save! - rule3 = DiscourseChat::Rule.new({provider: 'slack', - channel: '#general', - filter: 'watch', - category_id: category3.id, - tags: [tag1.name, tag2.name] - }).save! + rule1 = DiscourseChat::Rule.create(channel: chan1, + filter: 'watch', + category_id: category.id, + tags: [tag1.name, tag2.name] + ) + rule2 = DiscourseChat::Rule.create(channel: chan1, + filter: 'watch', + category_id: category2.id, + tags: [tag1.name, tag2.name] + ) + rule3 = DiscourseChat::Rule.create(channel: chan1, + filter: 'watch', + category_id: category3.id, + tags: [tag1.name, tag2.name] + ) expect(DiscourseChat::Rule.all.size).to eq(3) - expect(DiscourseChat::Helper.delete_by_index('slack','#general',2)).to eq(:deleted) + expect(DiscourseChat::Helper.delete_by_index(chan1,2)).to eq(:deleted) expect(DiscourseChat::Rule.all.size).to eq(2) expect(DiscourseChat::Rule.all.map(&:category_id)).to contain_exactly(category.id, category3.id) end it 'fails gracefully for out of range indexes' do - rule1 = DiscourseChat::Rule.new({provider: 'slack', - channel: '#general', - filter: 'watch', - category_id: category.id, - tags: [tag1.name, tag2.name] - }).save! + rule1 = DiscourseChat::Rule.create(channel: chan1, + filter: 'watch', + category_id: category.id, + tags: [tag1.name, tag2.name] + ) - expect(DiscourseChat::Helper.delete_by_index('slack','#general',-1)).to eq(false) - expect(DiscourseChat::Helper.delete_by_index('slack','#general',0)).to eq(false) - expect(DiscourseChat::Helper.delete_by_index('slack','#general',2)).to eq(false) + expect(DiscourseChat::Helper.delete_by_index(chan1,-1)).to eq(false) + expect(DiscourseChat::Helper.delete_by_index(chan1,0)).to eq(false) + expect(DiscourseChat::Helper.delete_by_index(chan1,2)).to eq(false) - expect(DiscourseChat::Helper.delete_by_index('slack','#general',1)).to eq(:deleted) + expect(DiscourseChat::Helper.delete_by_index(chan1,1)).to eq(:deleted) end @@ -112,8 +106,7 @@ RSpec.describe DiscourseChat::Manager do describe '.smart_create_rule' do it 'creates a rule when there are none' do - val = DiscourseChat::Helper.smart_create_rule(provider: 'slack', - channel: '#general', + val = DiscourseChat::Helper.smart_create_rule(channel: chan1, filter: 'watch', category_id: category.id, tags: [tag1.name] @@ -121,23 +114,20 @@ RSpec.describe DiscourseChat::Manager do expect(val).to eq(:created) record = DiscourseChat::Rule.all.first - expect(record.provider).to eq('slack') - expect(record.channel).to eq('#general') + expect(record.channel).to eq(chan1) expect(record.filter).to eq('watch') expect(record.category_id).to eq(category.id) expect(record.tags).to eq([tag1.name]) end it 'updates a rule when it has the same category and tags' do - existing = DiscourseChat::Rule.new({provider: 'slack', - channel: '#general', + existing = DiscourseChat::Rule.create!(channel:chan1, filter: 'watch', category_id: category.id, tags: [tag2.name, tag1.name] - }).save! + ) - val = DiscourseChat::Helper.smart_create_rule(provider: 'slack', - channel: '#general', + val = DiscourseChat::Helper.smart_create_rule(channel: chan1, filter: 'mute', category_id: category.id, tags: [tag1.name, tag2.name] @@ -150,15 +140,13 @@ RSpec.describe DiscourseChat::Manager do end it 'updates a rule when it has the same category and filter' do - existing = DiscourseChat::Rule.new({provider: 'slack', - channel: '#general', - filter: 'watch', - category_id: category.id, - tags: [tag1.name, tag2.name] - }).save! + existing = DiscourseChat::Rule.create(channel: chan1, + filter: 'watch', + category_id: category.id, + tags: [tag1.name, tag2.name] + ) - val = DiscourseChat::Helper.smart_create_rule(provider: 'slack', - channel: '#general', + val = DiscourseChat::Helper.smart_create_rule(channel: chan1, filter: 'watch', category_id: category.id, tags: [tag1.name, tag3.name] @@ -171,11 +159,10 @@ RSpec.describe DiscourseChat::Manager do end it 'destroys duplicate rules on save' do - DiscourseChat::Rule.new({provider: 'slack', channel: '#general', filter: 'watch'}).save! - DiscourseChat::Rule.new({provider: 'slack', channel: '#general', filter: 'watch'}).save! + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch') + DiscourseChat::Rule.create!(channel: chan1, filter: 'watch') expect(DiscourseChat::Rule.all.size).to eq(2) - val = DiscourseChat::Helper.smart_create_rule(provider: 'slack', - channel: '#general', + val = DiscourseChat::Helper.smart_create_rule(channel: chan1, filter: 'watch', category_id: nil, tags: nil @@ -185,12 +172,7 @@ RSpec.describe DiscourseChat::Manager do end it 'returns false on error' do - val = DiscourseChat::Helper.smart_create_rule(provider: 'nonexistantprovider', - channel: '#general', - filter: 'watch', - category_id: nil, - tags: nil - ) + val = DiscourseChat::Helper.smart_create_rule(channel: chan1, filter: 'blah') expect(val).to eq(false) end From ab2e4c2de872b478cd3e4facf0862753db4314b2 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 13 Jul 2017 22:50:14 +0100 Subject: [PATCH 05/15] Allow looking up channel by data attributes (nested json values) --- app/models/channel.rb | 2 ++ spec/models/channel_spec.rb | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/models/channel.rb b/app/models/channel.rb index 00e72f0..177e6b2 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -45,4 +45,6 @@ class DiscourseChat::Channel < DiscourseChat::PluginModel scope :with_provider, ->(provider) { where("value::json->>'provider'=?", provider)} + scope :with_data_value, ->(key, value) { where("(value::json->>'data')::json->>?=?", key, value)} + end \ No newline at end of file diff --git a/spec/models/channel_spec.rb b/spec/models/channel_spec.rb index 4094bf0..7670a0d 100644 --- a/spec/models/channel_spec.rb +++ b/spec/models/channel_spec.rb @@ -32,13 +32,24 @@ RSpec.describe DiscourseChat::Channel do expect(DiscourseChat::Channel.with_provider('dummy').length).to eq(1) end + it 'can be filtered by data value' do + channel2 = DiscourseChat::Channel.create!(provider:'dummy2', data:{val:"foo"}) + channel3 = DiscourseChat::Channel.create!(provider:'dummy2', data:{val:"blah"}) + + expect(DiscourseChat::Channel.all.length).to eq(2) + + for_provider = DiscourseChat::Channel.with_provider('dummy2') + expect(for_provider.length).to eq(2) + + expect(DiscourseChat::Channel.with_provider('dummy2').with_data_value('val','blah').length).to eq(1) + end + it 'can find its own rules' do channel = DiscourseChat::Channel.create({provider:'dummy'}) expect(channel.rules.size).to eq(0) DiscourseChat::Rule.create(channel: channel) DiscourseChat::Rule.create(channel: channel) expect(channel.rules.size).to eq(2) - end describe 'validations' do From ebb6fa947def0d82b6ba820014be2cee9e15e337 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 13 Jul 2017 23:21:15 +0100 Subject: [PATCH 06/15] Update slack provider to deal with new channel structure --- .../slack/slack_command_controller.rb | 13 ++++-- .../provider/slack/slack_provider.rb | 7 +-- .../slack/slack_command_controller_spec.rb | 46 +++++++++++++------ .../provider/slack/slack_provider_spec.rb | 14 +++--- 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/lib/discourse_chat/provider/slack/slack_command_controller.rb b/lib/discourse_chat/provider/slack/slack_command_controller.rb index 5975092..7eb8ca3 100644 --- a/lib/discourse_chat/provider/slack/slack_command_controller.rb +++ b/lib/discourse_chat/provider/slack/slack_command_controller.rb @@ -22,7 +22,7 @@ module DiscourseChat::Provider::SlackProvider tokens = params[:text].split(" ") # channel name fix - channel = + channel_id = case params[:channel_name] when 'directmessage' "@#{params[:user_name]}" @@ -34,6 +34,11 @@ module DiscourseChat::Provider::SlackProvider provider = DiscourseChat::Provider::SlackProvider::PROVIDER_NAME + channel = DiscourseChat::Channel.with_provider(provider).with_data_value('identifier',channel_id).first + + # Create channel if doesn't exist + channel ||= DiscourseChat::Channel.create!(provider:provider, data:{identifier: channel_id}) + cmd = tokens.shift if tokens.size >= 1 error_text = I18n.t("chat_integration.provider.slack.parse_error") @@ -72,7 +77,7 @@ module DiscourseChat::Provider::SlackProvider end category_id = category.nil? ? nil : category.id - case DiscourseChat::Helper.smart_create_rule(provider: provider, channel:channel, filter:cmd, category_id: category_id, tags:tags) + case DiscourseChat::Helper.smart_create_rule(channel:channel, filter:cmd, category_id: category_id, tags:tags) when :created return I18n.t("chat_integration.provider.slack.create.created") when :updated @@ -86,13 +91,13 @@ module DiscourseChat::Provider::SlackProvider rule_number = tokens[0].to_i return error_text unless rule_number.to_s == tokens[0] # Check we were given a number - if DiscourseChat::Helper.delete_by_index(provider, channel, rule_number) + if DiscourseChat::Helper.delete_by_index(channel, rule_number) return I18n.t("chat_integration.provider.slack.delete.success") else return I18n.t("chat_integration.provider.slack.delete.error") end when "status" - return DiscourseChat::Helper.status_for_channel(provider, channel) + return DiscourseChat::Helper.status_for_channel(channel) when "help" return I18n.t("chat_integration.provider.slack.help") else diff --git a/lib/discourse_chat/provider/slack/slack_provider.rb b/lib/discourse_chat/provider/slack/slack_provider.rb index dbd3afe..b212739 100644 --- a/lib/discourse_chat/provider/slack/slack_provider.rb +++ b/lib/discourse_chat/provider/slack/slack_provider.rb @@ -3,7 +3,7 @@ module DiscourseChat::Provider::SlackProvider PROVIDER_ENABLED_SETTING = :chat_integration_slack_enabled - CHANNEL_PARAMETERS = {"channel" => '^[@#]\S*$'} + CHANNEL_PARAMETERS = {"identifier" => '^[@#]\S*$'} def self.excerpt(post, max_length = SiteSetting.chat_integration_slack_excerpt_length) doc = Nokogiri::HTML.fragment(post.excerpt(max_length, @@ -137,12 +137,13 @@ module DiscourseChat::Provider::SlackProvider end def self.trigger_notification(post, channel) - message = slack_message(post, channel) + channel_id = channel.data['identifier'] + message = slack_message(post, channel_id) if SiteSetting.chat_integration_slack_access_token.empty? self.send_via_webhook(message) else - self.send_via_api(post, channel, message) + self.send_via_api(post, channel_id, message) end end diff --git a/spec/lib/discourse_chat/provider/slack/slack_command_controller_spec.rb b/spec/lib/discourse_chat/provider/slack/slack_command_controller_spec.rb index 11d00a0..bc4e9f7 100644 --- a/spec/lib/discourse_chat/provider/slack/slack_command_controller_spec.rb +++ b/spec/lib/discourse_chat/provider/slack/slack_command_controller_spec.rb @@ -4,6 +4,7 @@ describe 'Slack Command Controller', type: :request do let(:category) { Fabricate(:category) } let(:tag) { Fabricate(:tag) } let(:tag2) { Fabricate(:tag) } + let!(:chan1){DiscourseChat::Channel.create!(provider:'slack', data:{identifier: '#welcome'})} describe 'with plugin disabled' do it 'should return a 404' do @@ -24,8 +25,6 @@ describe 'Slack Command Controller', type: :request do end end - - describe 'slash commands endpoint' do before do SiteSetting.chat_integration_enabled = true @@ -82,8 +81,7 @@ describe 'Slack Command Controller', type: :request do expect(json["text"]).to eq(I18n.t("chat_integration.provider.slack.create.created")) rule = DiscourseChat::Rule.all.first - expect(rule.provider).to eq('slack') - expect(rule.channel).to eq('#welcome') + expect(rule.channel).to eq(chan1) expect(rule.filter).to eq('watch') expect(rule.category_id).to eq(category.id) expect(rule.tags).to eq(nil) @@ -144,8 +142,7 @@ describe 'Slack Command Controller', type: :request do expect(json["text"]).to eq(I18n.t("chat_integration.provider.slack.create.created")) rule = DiscourseChat::Rule.all.first - expect(rule.provider).to eq('slack') - expect(rule.channel).to eq('#welcome') + expect(rule.channel).to eq(chan1) expect(rule.filter).to eq('watch') expect(rule.category_id).to eq(nil) expect(rule.tags).to eq([tag.name]) @@ -164,8 +161,7 @@ describe 'Slack Command Controller', type: :request do expect(json["text"]).to eq(I18n.t("chat_integration.provider.slack.create.created")) rule = DiscourseChat::Rule.all.first - expect(rule.provider).to eq('slack') - expect(rule.channel).to eq('#welcome') + expect(rule.channel).to eq(chan1) expect(rule.filter).to eq('watch') expect(rule.category_id).to eq(category.id) expect(rule.tags).to contain_exactly(tag.name, tag2.name) @@ -184,16 +180,36 @@ describe 'Slack Command Controller', type: :request do expect(json["text"]).to eq(I18n.t("chat_integration.provider.slack.not_found.tag", name:"blah")) end end + + context 'from an unknown channel' do + it 'creates the channel' do + post "/chat-integration/slack/command.json", + text: "watch #{category.slug}", + channel_name: 'general', + token: token + + json = JSON.parse(response.body) + + expect(json["text"]).to eq(I18n.t("chat_integration.provider.slack.create.created")) + + chan = DiscourseChat::Channel.with_provider('slack').with_data_value('identifier','#general').first + expect(chan.provider).to eq('slack') + + rule = chan.rules.first + expect(rule.filter).to eq('watch') + expect(rule.category_id).to eq(category.id) + expect(rule.tags).to eq(nil) + end + end end describe 'remove rule' do it 'removes the rule' do - rule1 = DiscourseChat::Rule.new({provider: 'slack', - channel: '#welcome', - filter: 'watch', - category_id: category.id, - tags: [tag.name, tag2.name] - }).save! + rule1 = DiscourseChat::Rule.create(channel: chan1, + filter: 'watch', + category_id: category.id, + tags: [tag.name, tag2.name] + ) expect(DiscourseChat::Rule.all.size).to eq(1) post "/chat-integration/slack/command.json", @@ -250,7 +266,7 @@ describe 'Slack Command Controller', type: :request do json = JSON.parse(response.body) - expect(json["text"]).to eq(DiscourseChat::Helper.status_for_channel('slack','#welcome')) + expect(json["text"]).to eq(DiscourseChat::Helper.status_for_channel(chan1)) end end diff --git a/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb b/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb index 7a93862..6295d50 100644 --- a/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb @@ -57,16 +57,18 @@ RSpec.describe DiscourseChat::Provider::SlackProvider do SiteSetting.chat_integration_slack_enabled = true end + let(:chan1){DiscourseChat::Channel.create!(provider:'slack', data:{identifier: '#general'})} + it 'sends a webhook request' do stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(body: "success") - described_class.trigger_notification(post, '#general') + described_class.trigger_notification(post, chan1) expect(stub1).to have_been_requested.once end it 'handles errors correctly' do stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(status: 400, body: "error") expect(stub1).to have_been_requested.times(0) - expect{described_class.trigger_notification(post, '#general')}.to raise_exception(::DiscourseChat::ProviderError) + expect{described_class.trigger_notification(post, chan1)}.to raise_exception(::DiscourseChat::ProviderError) expect(stub1).to have_been_requested.once end @@ -81,14 +83,14 @@ RSpec.describe DiscourseChat::Provider::SlackProvider do it 'sends an api request' do expect(@stub2).to have_been_requested.times(0) - described_class.trigger_notification(post, '#general') + described_class.trigger_notification(post, chan1) expect(@stub1).to have_been_requested.times(0) expect(@stub2).to have_been_requested.once end it 'handles errors correctly' do @stub2 = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).to_return(body: "{\"ok\":false }", headers: {'Content-Type' => 'application/json'}) - expect{described_class.trigger_notification(post, '#general')}.to raise_exception(::DiscourseChat::ProviderError) + expect{described_class.trigger_notification(post, chan1)}.to raise_exception(::DiscourseChat::ProviderError) expect(@stub2).to have_been_requested.once end @@ -97,8 +99,8 @@ RSpec.describe DiscourseChat::Provider::SlackProvider do expect(@stub2).to have_been_requested.times(0) expect(@stub3).to have_been_requested.times(0) - described_class.trigger_notification(post, '#general') - described_class.trigger_notification(second_post, '#general') + described_class.trigger_notification(post, chan1) + described_class.trigger_notification(second_post, chan1) expect(@stub1).to have_been_requested.times(0) expect(@stub2).to have_been_requested.once # Initial creation of message expect(@stub3).to have_been_requested.once # Requests to update the existing message From 1ef9073027836183706750d70109fd20d63ea4f4 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 17 Jul 2017 17:53:32 +0100 Subject: [PATCH 07/15] Make CHANNEL_PARAMETERS a list --- app/controllers/chat_controller.rb | 2 +- app/models/channel.rb | 4 ++-- lib/discourse_chat/provider/slack/slack_provider.rb | 4 +++- lib/discourse_chat/provider/telegram/telegram_provider.rb | 2 +- spec/dummy_provider.rb | 6 ++++-- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/app/controllers/chat_controller.rb b/app/controllers/chat_controller.rb index 21e3cd1..a692a30 100644 --- a/app/controllers/chat_controller.rb +++ b/app/controllers/chat_controller.rb @@ -9,7 +9,7 @@ class DiscourseChat::ChatController < ApplicationController providers = ::DiscourseChat::Provider.enabled_providers.map {|x| { name: x::PROVIDER_NAME, id: x::PROVIDER_NAME, - channel_regex: (defined? x::PROVIDER_CHANNEL_REGEX) ? x::PROVIDER_CHANNEL_REGEX : nil + channel_parameters: (defined? x::CHANNEL_PARAMETERS) ? x::CHANNEL_PARAMETERS : [] }} render json:providers, root: 'providers' diff --git a/app/models/channel.rb b/app/models/channel.rb index 177e6b2..7c3aeaa 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -26,13 +26,13 @@ class DiscourseChat::Channel < DiscourseChat::PluginModel params = ::DiscourseChat::Provider.get_by_name(provider)::CHANNEL_PARAMETERS - unless params.keys.sort == data.keys.sort + unless params.map {|p| p[:key]}.sort == data.keys.sort errors.add(:data, "data does not match the required structure for provider #{provider}") return end data.each do |key, value| - regex_string = params[key] + regex_string = params.find{|p| p[:key] == key}[:regex] if !Regexp.new(regex_string).match?(value) errors.add(:data, "data.#{key} is invalid") end diff --git a/lib/discourse_chat/provider/slack/slack_provider.rb b/lib/discourse_chat/provider/slack/slack_provider.rb index b212739..3146b1f 100644 --- a/lib/discourse_chat/provider/slack/slack_provider.rb +++ b/lib/discourse_chat/provider/slack/slack_provider.rb @@ -3,7 +3,9 @@ module DiscourseChat::Provider::SlackProvider PROVIDER_ENABLED_SETTING = :chat_integration_slack_enabled - CHANNEL_PARAMETERS = {"identifier" => '^[@#]\S*$'} + CHANNEL_PARAMETERS = [ + {key: "identifier", regex: '^[@#]\S*$'} + ] def self.excerpt(post, max_length = SiteSetting.chat_integration_slack_excerpt_length) doc = Nokogiri::HTML.fragment(post.excerpt(max_length, diff --git a/lib/discourse_chat/provider/telegram/telegram_provider.rb b/lib/discourse_chat/provider/telegram/telegram_provider.rb index 8e7afed..a681603 100644 --- a/lib/discourse_chat/provider/telegram/telegram_provider.rb +++ b/lib/discourse_chat/provider/telegram/telegram_provider.rb @@ -3,7 +3,7 @@ module DiscourseChat module TelegramProvider PROVIDER_NAME = "telegram".freeze PROVIDER_ENABLED_SETTING = :chat_integration_telegram_enabled - CHANNEL_PARAMETERS = {} + CHANNEL_PARAMETERS = [] end end diff --git a/spec/dummy_provider.rb b/spec/dummy_provider.rb index 19b5bc8..7a6c521 100644 --- a/spec/dummy_provider.rb +++ b/spec/dummy_provider.rb @@ -8,7 +8,7 @@ RSpec.shared_context "dummy provider" do module ::DiscourseChat::Provider::DummyProvider PROVIDER_NAME = "dummy".freeze PROVIDER_ENABLED_SETTING = :chat_integration_enabled # Tie to main plugin enabled setting - CHANNEL_PARAMETERS = {} + CHANNEL_PARAMETERS = [] @@sent_messages = [] @@raise_exception = nil @@ -47,7 +47,9 @@ RSpec.shared_context "validated dummy provider" do module ::DiscourseChat::Provider::Dummy2Provider PROVIDER_NAME = "dummy2".freeze PROVIDER_ENABLED_SETTING = :chat_integration_enabled # Tie to main plugin enabled setting - CHANNEL_PARAMETERS = {"val" => '\S+'} + CHANNEL_PARAMETERS = [ + {key: "val", regex: '\S+'} + ] @@sent_messages = [] From bdb81191d77560dbab7ccdedbd06200defdadb37 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Jul 2017 16:17:03 +0100 Subject: [PATCH 08/15] New admin interface for improved channel definitions --- app/controllers/chat_controller.rb | 96 +++++++++++++++---- app/models/rule.rb | 2 +- app/routes/discourse_chat.rb | 9 +- app/serializers/channel_serializer.rb | 11 +++ app/serializers/rule_serializer.rb | 4 +- .../javascripts/admin/adapters/channel.js.es6 | 7 ++ .../admin/components/channel-details.js.es6 | 36 +++++++ .../admin/components/rule-row.js.es6 | 38 ++++++++ .../admin-plugins-chat-provider.js.es6 | 38 ++++---- .../admin-plugins-chat-edit-channel.js.es6 | 94 ++++++++++++++++++ .../admin-plugins-chat-edit-rule.js.es6 | 61 ------------ .../modals/admin-plugins-chat-test.js.es6 | 7 +- .../javascripts/admin/models/channel.js.es6 | 14 +++ assets/javascripts/admin/models/rule.js.es6 | 10 +- .../routes/admin-plugins-chat-provider.js.es6 | 12 ++- .../modal/admin-plugins-chat-edit-channel.hbs | 49 ++++++++++ .../modal/admin-plugins-chat-edit-rule.hbs | 87 ----------------- .../modal/admin-plugins-chat-test.hbs | 29 +----- .../templates/admin/plugins-chat-provider.hbs | 67 +------------ .../templates/components/channel-details.hbs | 40 ++++++++ .../templates/components/rule-row.hbs | 53 ++++++++++ .../stylesheets/chat-integration-admin.scss | 28 +++++- config/locales/client.en.yml | 29 +++--- plugin.rb | 1 + 24 files changed, 514 insertions(+), 308 deletions(-) create mode 100644 app/serializers/channel_serializer.rb create mode 100644 assets/javascripts/admin/adapters/channel.js.es6 create mode 100644 assets/javascripts/admin/components/channel-details.js.es6 create mode 100644 assets/javascripts/admin/components/rule-row.js.es6 create mode 100644 assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-channel.js.es6 delete mode 100644 assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-rule.js.es6 create mode 100644 assets/javascripts/admin/models/channel.js.es6 create mode 100644 assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-channel.hbs delete mode 100644 assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-rule.hbs create mode 100644 assets/javascripts/discourse/templates/components/channel-details.hbs create mode 100644 assets/javascripts/discourse/templates/components/rule-row.hbs diff --git a/app/controllers/chat_controller.rb b/app/controllers/chat_controller.rb index a692a30..43fbace 100644 --- a/app/controllers/chat_controller.rb +++ b/app/controllers/chat_controller.rb @@ -15,23 +15,19 @@ class DiscourseChat::ChatController < ApplicationController render json:providers, root: 'providers' end - def test_provider + def test begin - requested_provider = params[:provider] - channel = params[:channel] - topic_id = params[:topic_id] + channel_id = params[:channel_id].to_i + topic_id = params[:topic_id].to_i - provider = ::DiscourseChat::Provider.get_by_name(requested_provider) + channel = DiscourseChat::Channel.find(channel_id) - if provider.nil? or not ::DiscourseChat::Provider.is_enabled(provider) + provider = ::DiscourseChat::Provider.get_by_name(channel.provider) + + if not ::DiscourseChat::Provider.is_enabled(provider) raise Discourse::NotFound end - if defined? provider::PROVIDER_CHANNEL_REGEX - channel_regex = Regexp.new provider::PROVIDER_CHANNEL_REGEX - raise Discourse::InvalidParameters, 'Channel is not valid' if not channel_regex.match?(channel) - end - post = Topic.find(topic_id.to_i).posts.first provider.trigger_notification(post, channel) @@ -48,23 +44,91 @@ class DiscourseChat::ChatController < ApplicationController end end + def list_channels + providers = ::DiscourseChat::Provider.enabled_providers.map {|x| x::PROVIDER_NAME} + + requested_provider = params[:provider] + + if not providers.include? requested_provider + raise Discourse::NotFound + end + + channels = DiscourseChat::Channel.with_provider(requested_provider) + + render_serialized channels, DiscourseChat::ChannelSerializer, root: 'channels' + end + + def create_channel + begin + providers = ::DiscourseChat::Provider.enabled_providers.map {|x| x::PROVIDER_NAME} + + requested_provider = params[:channel][:provider] + + if not providers.include? requested_provider + raise Discourse::InvalidParameters, 'Provider is not valid' + end + + allowed_keys = DiscourseChat::Provider.get_by_name(requested_provider)::CHANNEL_PARAMETERS.map{|p| p[:key].to_sym} + + hash = params.require(:channel).permit(:provider, data:allowed_keys) + + channel = DiscourseChat::Channel.new(hash) + + if not channel.save(hash) + raise Discourse::InvalidParameters, 'Channel is not valid' + end + + render_serialized channel, DiscourseChat::ChannelSerializer, root: 'channel' + rescue Discourse::InvalidParameters => e + render json: {errors: [e.message]}, status: 422 + end + end + + def update_channel + begin + channel = DiscourseChat::Channel.find(params[:id].to_i) + # rule.error_key = nil # Reset any error on the rule + + allowed_keys = DiscourseChat::Provider.get_by_name(channel.provider)::CHANNEL_PARAMETERS.map{|p| p[:key].to_sym} + + hash = params.require(:channel).permit(data:allowed_keys) + + if not channel.update(hash) + raise Discourse::InvalidParameters, 'Channel is not valid' + end + + render_serialized channel, DiscourseChat::ChannelSerializer, root: 'channel' + rescue Discourse::InvalidParameters => e + render json: {errors: [e.message]}, status: 422 + end + end + + def destroy_channel + rule = DiscourseChat::Channel.find(params[:id].to_i) + + rule.destroy + + render json: success_json + end + + def list_rules providers = ::DiscourseChat::Provider.enabled_providers.map {|x| x::PROVIDER_NAME} requested_provider = params[:provider] - if providers.include? requested_provider - rules = DiscourseChat::Rule.with_provider(requested_provider) - else + if not providers.include? requested_provider raise Discourse::NotFound end + rules = DiscourseChat::Rule.with_provider(requested_provider) + render_serialized rules, DiscourseChat::RuleSerializer, root: 'rules' end def create_rule begin - hash = params.require(:rule).permit(:provider, :channel, :filter, :category_id, tags:[]) + hash = params.require(:rule).permit(:channel_id, :filter, :category_id, tags:[]) rule = DiscourseChat::Rule.new(hash) @@ -82,7 +146,7 @@ class DiscourseChat::ChatController < ApplicationController begin rule = DiscourseChat::Rule.find(params[:id].to_i) rule.error_key = nil # Reset any error on the rule - hash = params.require(:rule).permit(:provider, :channel, :filter, :category_id, tags:[]) + hash = params.require(:rule).permit(:filter, :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 20623d9..3300c4e 100644 --- a/app/models/rule.rb +++ b/app/models/rule.rb @@ -17,7 +17,7 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel def channel_valid? # Validate category - if not (channel_id.nil? or DiscourseChat::Channel.where(id: channel_id).exists?) + if not (DiscourseChat::Channel.where(id: channel_id).exists?) errors.add(:channel_id, "#{channel_id} is not a valid channel id") end end diff --git a/app/routes/discourse_chat.rb b/app/routes/discourse_chat.rb index 207b4e1..cf137e6 100644 --- a/app/routes/discourse_chat.rb +++ b/app/routes/discourse_chat.rb @@ -4,10 +4,15 @@ module DiscourseChat AdminEngine.routes.draw do get "" => "chat#respond" get '/providers' => "chat#list_providers" - post '/test' => "chat#test_provider" + post '/test' => "chat#test" + get '/channels' => "chat#list_channels" + post '/channels' => "chat#create_channel" + put '/channels/:id' => "chat#update_channel" + delete '/channels/:id' => "chat#destroy_channel" + get '/rules' => "chat#list_rules" - put '/rules' => "chat#create_rule" + post '/rules' => "chat#create_rule" put '/rules/:id' => "chat#update_rule" delete '/rules/:id' => "chat#destroy_rule" diff --git a/app/serializers/channel_serializer.rb b/app/serializers/channel_serializer.rb new file mode 100644 index 0000000..4022976 --- /dev/null +++ b/app/serializers/channel_serializer.rb @@ -0,0 +1,11 @@ +require_relative './rule_serializer' + +class DiscourseChat::ChannelSerializer < ApplicationSerializer + attributes :id, :provider, :data, :rules + + def rules + object.rules.map do |rule| + DiscourseChat::RuleSerializer.new(rule, root:false) + end + end +end \ No newline at end of file diff --git a/app/serializers/rule_serializer.rb b/app/serializers/rule_serializer.rb index 7ff023b..d8b3f77 100644 --- a/app/serializers/rule_serializer.rb +++ b/app/serializers/rule_serializer.rb @@ -1,3 +1,3 @@ -class DiscourseChat::RuleSerializer < ActiveModel::Serializer - attributes :id, :provider, :channel, :category_id, :tags, :filter, :error_key +class DiscourseChat::RuleSerializer < ApplicationSerializer + attributes :id, :channel_id, :category_id, :tags, :filter, :error_key end \ No newline at end of file diff --git a/assets/javascripts/admin/adapters/channel.js.es6 b/assets/javascripts/admin/adapters/channel.js.es6 new file mode 100644 index 0000000..8287b75 --- /dev/null +++ b/assets/javascripts/admin/adapters/channel.js.es6 @@ -0,0 +1,7 @@ +import buildPluginAdapter from 'admin/adapters/build-plugin'; +import Rule from 'discourse/plugins/discourse-chat-integration/admin/models/rule' + +export default buildPluginAdapter('chat').extend({ + + +}); \ No newline at end of file diff --git a/assets/javascripts/admin/components/channel-details.js.es6 b/assets/javascripts/admin/components/channel-details.js.es6 new file mode 100644 index 0000000..6e3be76 --- /dev/null +++ b/assets/javascripts/admin/components/channel-details.js.es6 @@ -0,0 +1,36 @@ +import { popupAjaxError } from 'discourse/lib/ajax-error'; + +export default Ember.Component.extend({ + classNames: ['channel-details'], + actions: { + refresh: function(){ + this.sendAction('refresh'); + }, + + delete(channel){ + bootbox.confirm(I18n.t("chat_integration.channel_delete_confirm"), I18n.t("no_value"), I18n.t("yes_value"), result => { + if (result) { + channel.destroyRecord().then(() => { + this.send('refresh'); + }).catch(popupAjaxError) + } + }); + }, + + edit(channel){ + this.sendAction('edit', channel) + }, + + test(channel){ + this.sendAction('test', channel) + }, + + createRule(channel){ + var newRule = this.get('store').createRecord('rule',{channel_id: channel.id}); + channel.rules.pushObject(newRule) + } + + + + } +}); \ No newline at end of file diff --git a/assets/javascripts/admin/components/rule-row.js.es6 b/assets/javascripts/admin/components/rule-row.js.es6 new file mode 100644 index 0000000..7622fa1 --- /dev/null +++ b/assets/javascripts/admin/components/rule-row.js.es6 @@ -0,0 +1,38 @@ +import { popupAjaxError } from 'discourse/lib/ajax-error'; + +export default Ember.Component.extend({ + tagName: 'tr', + editing: false, + + autoEdit: function(){ + if(!this.get('rule').id){ + this.set('editing', true); + } + }.on('init'), + + actions: { + edit: function(){ + this.set('editing', true); + }, + + cancel: function(){ + this.send('refresh'); + }, + + save: function(){ + this.get('rule').save().then(result => { + this.send('refresh'); + }).catch(popupAjaxError); + }, + + delete(rule){ + rule.destroyRecord().then(() => { + this.send('refresh'); + }).catch(popupAjaxError) + }, + + refresh: function(){ + this.sendAction('refresh'); + } + } +}); \ No newline at end of file diff --git a/assets/javascripts/admin/controllers/admin-plugins-chat-provider.js.es6 b/assets/javascripts/admin/controllers/admin-plugins-chat-provider.js.es6 index 5fd9341..335d400 100644 --- a/assets/javascripts/admin/controllers/admin-plugins-chat-provider.js.es6 +++ b/assets/javascripts/admin/controllers/admin-plugins-chat-provider.js.es6 @@ -7,42 +7,38 @@ import { popupAjaxError } from 'discourse/lib/ajax-error'; export default Ember.Controller.extend({ modalShowing: false, - + anyErrors: function(){ var anyErrors = false; - this.get('model.rules').forEach(function(rule){ - if(rule.error_key){ + this.get('model.channels').forEach(function(channel){ + if(channel.error_key){ anyErrors = true; } }); return anyErrors; - }.property('model.rules'), + }.property('model.channels'), actions:{ - create(){ + createChannel(){ this.set('modalShowing', true); - var model = {rule: this.store.createRecord('rule',{provider: this.get('model.provider').id}), provider:this.get('model.provider')}; - showModal('admin-plugins-chat-edit-rule', { model: model, admin: true }); + var model = {channel: this.store.createRecord('channel',{provider: this.get('model.provider').id, data:{}},), provider:this.get('model.provider')}; + showModal('admin-plugins-chat-edit-channel', { model: model, admin: true }); }, - edit(rule){ + editChannel(channel){ this.set('modalShowing', true); - var model = {rule: rule, provider:this.get('model.provider')}; - showModal('admin-plugins-chat-edit-rule', { model: model, admin: true }); - }, - delete(rule){ - const self = this; - rule.destroyRecord().then(function() { - self.send('refresh'); - }).catch(popupAjaxError) + var model = {channel: channel, provider: this.get('model.provider')}; + showModal('admin-plugins-chat-edit-channel', { model: model, admin: true }); }, + testChannel(channel){ + this.set('modalShowing', true); + var model = {channel:channel} + showModal('admin-plugins-chat-test', { model: model, admin: true }); + }, showError(error_key){ bootbox.alert(I18n.t(error_key)); }, - test(){ - this.set('modalShowing', true); - var model = {provider:this.get('model.provider'), channel:''} - showModal('admin-plugins-chat-test', { model: model, admin: true }); - } + + } diff --git a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-channel.js.es6 b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-channel.js.es6 new file mode 100644 index 0000000..b215fba --- /dev/null +++ b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-channel.js.es6 @@ -0,0 +1,94 @@ +import Rule from 'discourse/plugins/discourse-chat-integration/admin/models/rule' +import ModalFunctionality from 'discourse/mixins/modal-functionality'; +import { ajax } from 'discourse/lib/ajax'; +import { extractError } from 'discourse/lib/ajax-error'; +import InputValidation from 'discourse/models/input-validation'; + +export default Ember.Controller.extend(ModalFunctionality, { + + initThing: function(){ + console.log("Initialising controller"); + console.log(this.get('model.data')); + }.on('init'), + + // The validation property must be defined at runtime since the possible parameters vary by provider + setupValidations: function(){ + if(this.get('model.provider')){ + var theKeys = this.get('model.provider.channel_parameters').map( ( param ) => param['key'] ); + Ember.defineProperty(this,'paramValidation',Ember.computed('model.channel.data.{' + theKeys.join(',') + '}',this._paramValidation)); + } + }.observes('model'), + + validate(parameter){ + var regString = parameter.regex; + var regex = new RegExp(regString); + var val = this.get('model.channel.data.'+parameter.key); + + if(val == ""){ // Fail silently if field blank + return InputValidation.create({ + failed: true, + }); + }else if(!regString){ // Pass silently if no regex available for provider + return InputValidation.create({ + ok: true, + }); + }else if(regex.test(val)){ // Test against regex + return InputValidation.create({ + ok: true, + reason: I18n.t('chat_integration.edit_channel_modal.channel_validation.ok') + }); + }else{ // Failed regex + return InputValidation.create({ + failed: true, + reason: I18n.t('chat_integration.edit_channel_modal.channel_validation.fail') + }); + } + + }, + + _paramValidation: function(){ + var response = {} + var parameters = this.get('model.provider.channel_parameters'); + parameters.forEach(parameter => { + response[parameter.key] = this.validate(parameter); + }); + return response; + }, + + saveDisabled: function(){ + var validations = this.get('paramValidation'); + + if(!validations){ return true } + + var invalid = false; + + Object.keys(validations).forEach(key =>{ + if(!validations[key]){ + invalid = true; + } + if(!validations[key]['ok']){ + invalid = true; + } + }); + + return invalid; + }.property('paramValidation'), + + actions: { + cancel: function(){ + this.send('closeModal'); + }, + + save: function(){ + + const self = this; + + this.get('model.channel').save().then(function(result) { + self.send('closeModal'); + }).catch(function(error) { + self.flash(extractError(error), 'error'); + }); + + } + } +}); \ No newline at end of file diff --git a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-rule.js.es6 b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-rule.js.es6 deleted file mode 100644 index 2093a03..0000000 --- a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-rule.js.es6 +++ /dev/null @@ -1,61 +0,0 @@ -import Rule from 'discourse/plugins/discourse-chat-integration/admin/models/rule' -import ModalFunctionality from 'discourse/mixins/modal-functionality'; -import { ajax } from 'discourse/lib/ajax'; -import { extractError } from 'discourse/lib/ajax-error'; -import InputValidation from 'discourse/models/input-validation'; - -export default Ember.Controller.extend(ModalFunctionality, { - - model: Rule.create({}), - - channelValidation: function(){ - - var regString = this.get('model.provider.channel_regex'); - var regex = new RegExp(regString); - var val = this.get('model.rule.channel'); - - if(val == ""){ // Fail silently if field blank - return InputValidation.create({ - failed: true, - }); - }else if(!regString){ // Pass silently if no regex available for provider - return InputValidation.create({ - ok: true, - }); - }else if(regex.test(val)){ // Test against regex - return InputValidation.create({ - ok: true, - reason: I18n.t('chat_integration.edit_rule_modal.channel_validation.ok') - }); - }else{ // Failed regex - return InputValidation.create({ - failed: true, - reason: I18n.t('chat_integration.edit_rule_modal.channel_validation.fail') - }); - } - }.property('model.rule.channel'), - - saveDisabled: function(){ - if(this.get('channelValidation.failed')){ return true } - - return false; - }.property('channelValidation.failed'), - - actions: { - cancel: function(){ - this.send('closeModal'); - }, - - save: function(){ - - const self = this; - - this.get('model.rule').update().then(function(result) { - self.send('closeModal'); - }).catch(function(error) { - self.flash(extractError(error), 'error'); - }); - - } - } -}); \ No newline at end of file diff --git a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-test.js.es6 b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-test.js.es6 index 1eacb71..b6a5ba5 100644 --- a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-test.js.es6 +++ b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-test.js.es6 @@ -3,11 +3,11 @@ import { ajax } from 'discourse/lib/ajax'; export default Ember.Controller.extend(ModalFunctionality, { sendDisabled: function(){ - if(this.get('model').topic_id && this.get('model').channel){ + if(this.get('model').topic_id){ return false } return true - }.property('model.topic_id', 'model.channel'), + }.property('model.topic_id'), actions: { @@ -15,8 +15,7 @@ export default Ember.Controller.extend(ModalFunctionality, { self = this; this.set('loading', true); ajax("/admin/plugins/chat/test", { - data: { provider: this.get('model.provider.name'), - channel: this.get('model.channel'), + data: { channel_id: this.get('model.channel.id'), topic_id: this.get('model.topic_id') }, type: 'POST' diff --git a/assets/javascripts/admin/models/channel.js.es6 b/assets/javascripts/admin/models/channel.js.es6 new file mode 100644 index 0000000..b21399c --- /dev/null +++ b/assets/javascripts/admin/models/channel.js.es6 @@ -0,0 +1,14 @@ +import RestModel from 'discourse/models/rest'; + +export default RestModel.extend({ + + updateProperties() { + var prop_names = ['data']; + return this.getProperties(prop_names); + }, + + createProperties() { + var prop_names = ['provider','data']; + return this.getProperties(prop_names); + } +}); diff --git a/assets/javascripts/admin/models/rule.js.es6 b/assets/javascripts/admin/models/rule.js.es6 index 2ee1523..44fe2c8 100644 --- a/assets/javascripts/admin/models/rule.js.es6 +++ b/assets/javascripts/admin/models/rule.js.es6 @@ -11,9 +11,8 @@ export default RestModel.extend({ category_id: null, tags: null, - provider: '', - channel: '', - filter: null, + channel_id: null, + filter: 'watch', error_key: null, @computed('category_id') @@ -31,12 +30,13 @@ export default RestModel.extend({ }, updateProperties() { - var prop_names = ['category_id','provider','channel', 'tags','filter']; + var prop_names = ['category_id','tags','filter']; return this.getProperties(prop_names); }, createProperties() { - return this.updateProperties(); + var prop_names = ['channel_id', 'category_id','tags','filter']; + return this.getProperties(prop_names); } }); diff --git a/assets/javascripts/admin/routes/admin-plugins-chat-provider.js.es6 b/assets/javascripts/admin/routes/admin-plugins-chat-provider.js.es6 index 07b7b0a..7889816 100644 --- a/assets/javascripts/admin/routes/admin-plugins-chat-provider.js.es6 +++ b/assets/javascripts/admin/routes/admin-plugins-chat-provider.js.es6 @@ -1,12 +1,20 @@ -import Rule from 'discourse/plugins/discourse-chat-integration/admin/models/rule' import { ajax } from 'discourse/lib/ajax'; export default Discourse.Route.extend({ model(params, transition) { return Ember.RSVP.hash({ - rules: this.store.find('rule', {provider: params.provider}), + channels: this.store.findAll('channel', {provider: params.provider}), provider: this.modelFor("admin-plugins-chat").findBy('id',params.provider) + }).then(value => { + value.channels.forEach(channel => { + channel.set('rules', channel.rules.map(rule => { + rule = this.store.createRecord('rule', rule); + rule.channel = channel; + return rule; + })); + }); + return value; }); }, diff --git a/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-channel.hbs b/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-channel.hbs new file mode 100644 index 0000000..9c3399d --- /dev/null +++ b/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-channel.hbs @@ -0,0 +1,49 @@ +{{#d-modal-body id="chat_integration_edit_channel_modal" title="chat_integration.edit_channel_modal.title"}} +
+
+ + + + + + + + + + + + {{# each model.provider.channel_parameters as |param|}} + + + + + + + + + {{/each}} +
+ {{i18n (concat 'chat_integration.provider.' model.channel.provider '.title')}} +
+ {{text-field + name=(concat 'param-' param.key) + value=(mut (get model.channel.data param.key)) + }} + +   + {{#if (get model.channel.data param.key)}} + {{input-tip validation=(get paramValidation param.key)}} + {{/if}} +
+ +
+
+{{/d-modal-body}} + +
+ + {{d-button id="save_rule" class='btn-primary btn-large' action="save" title="chat_integration.edit_channel_modal.save" label="chat_integration.edit_channel_modal.save" disabled=saveDisabled}} + + {{d-button class="btn-large" action="cancel" title="chat_integration.edit_channel_modal.cancel" label="chat_integration.edit_channel_modal.cancel"}} + +
\ No newline at end of file diff --git a/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-rule.hbs b/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-rule.hbs deleted file mode 100644 index a428578..0000000 --- a/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-rule.hbs +++ /dev/null @@ -1,87 +0,0 @@ -{{#d-modal-body id="chat_integration_edit_rule_modal" title="chat_integration.edit_rule_modal.title"}} -
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - {{#if siteSettings.tagging_enabled}} - - - - - - - - - {{/if}} - -
- {{i18n (concat 'chat_integration.provider.' model.rule.provider '.title')}} -
- {{text-field - name="channel" - value=model.rule.channel - autofocus="autofocus" - id="channel-field"}} - -  {{input-tip validation=channelValidation}} -
- {{combo-box name="filter" content=model.rule.available_filters value=model.rule.filter}} -
- {{category-chooser - name="category" - value=model.rule.category_id - rootNoneLabel="chat_integration.all_categories" - rootNone=true - overrideWidths=false - }} -
- {{tag-chooser placeholderKey="chat_integration.all_tags" name="tags" tags=model.rule.tags}} -
- -
-
-{{/d-modal-body}} - -
- - {{d-button id="save_rule" class='btn-primary btn-large' action="save" title="chat_integration.edit_rule_modal.save" label="chat_integration.edit_rule_modal.save" disabled=saveDisabled}} - - {{d-button class="btn-large" action="cancel" title="chat_integration.edit_rule_modal.cancel" label="chat_integration.edit_rule_modal.cancel"}} - -
\ No newline at end of file diff --git a/assets/javascripts/admin/templates/modal/admin-plugins-chat-test.hbs b/assets/javascripts/admin/templates/modal/admin-plugins-chat-test.hbs index ef6b755..c4bc8e6 100644 --- a/assets/javascripts/admin/templates/modal/admin-plugins-chat-test.hbs +++ b/assets/javascripts/admin/templates/modal/admin-plugins-chat-test.hbs @@ -3,34 +3,7 @@
- - - - - - - - - - - - - - - - - - + + + + + + +{{#if siteSettings.tagging_enabled}} + +{{/if}} + + + diff --git a/assets/stylesheets/chat-integration-admin.scss b/assets/stylesheets/chat-integration-admin.scss index a048ce9..c9b7a71 100644 --- a/assets/stylesheets/chat-integration-admin.scss +++ b/assets/stylesheets/chat-integration-admin.scss @@ -1,6 +1,7 @@ #admin-plugin-chat{ table{ + margin-top:0; td:last-child{ white-space:nowrap; } @@ -25,10 +26,33 @@ } + div.channel-details{ + margin: 20px 10px; + + border: 1px solid dark-light-diff($primary, $secondary, 90%, -75%); + + div.channel-header{ + background: dark-light-diff($primary, $secondary, 90%, -75%); + padding: 10px; + overflow:auto; + + .channel-title{ + font-weight: bold; + font-size: 1.3em; + } + + + } + + div.channel-footer{ + overflow:auto; + } + } + } -#chat_integration_edit_rule_modal, #chat_integration_test_modal{ +#chat_integration_edit_channel_modal, #chat_integration_test_modal{ table{ width:100%; @@ -66,4 +90,4 @@ } - } \ No newline at end of file + } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index a8224a7..8afedca 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -9,7 +9,11 @@ en: all_categories: "(all categories)" all_tags: "(all tags)" create_rule: "Create Rule" - test_provider: "Test Provider" + create_channel: "Create Channel" + delete_channel: "Delete" + test_channel: "Test" + edit_channel: "Edit" + channel_delete_confirm: "Are you sure you want to delete this channel? All associated rules will be deleted." test_modal: title: "Send a test message" provider: "Provider" @@ -30,23 +34,15 @@ en: tags: "Tags" edit_rule: "Edit" delete_rule: "Delete" - edit_rule_modal: - title: "Edit Rule" - save: "Save Rule" + edit_channel_modal: + title: "Edit Channel" + save: "Save Channel" cancel: "Cancel" provider: "Provider" - category: "Category" - tags: "Tags" - channel: "Channel" - filter: "Filter" channel_validation: ok: "Valid" - fail: "Invalid channel format" - instructions: - filter: "Notification level. Mute overrides other matching rules." - category: "This rule will only apply to topics in the specified category." - tags: "If specified, this rule will only apply to topics which have at least one of these tags." - + fail: "Invalid format" + provider: ####################################### @@ -54,7 +50,10 @@ en: ####################################### slack: title: "Slack" - channel_instructions: "e.g. #channel, @username." + param: + identifier: + title: Channel + help: "e.g. #channel, @username." errors: action_prohibited: "The bot does not have permission to post to that channel" channel_not_found: "The specified channel does not exist on slack" diff --git a/plugin.rb b/plugin.rb index 19f6225..f9580fd 100644 --- a/plugin.rb +++ b/plugin.rb @@ -19,6 +19,7 @@ after_initialize do require_relative "app/models/rule" require_relative "app/models/channel" + require_relative "app/serializers/channel_serializer" require_relative "app/serializers/rule_serializer" require_relative "app/controllers/chat_controller" From 2275b048f9fa986af9ffb0bab25bd783f16072be Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Jul 2017 16:42:05 +0100 Subject: [PATCH 09/15] Destroy associated rules when a channel is destroyed --- app/models/channel.rb | 5 +++++ spec/models/channel_spec.rb | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/app/models/channel.rb b/app/models/channel.rb index 7c3aeaa..a3238d0 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -10,6 +10,11 @@ class DiscourseChat::Channel < DiscourseChat::PluginModel self.data = {} if self.data.nil? end + after_destroy :destroy_rules + def destroy_rules + rules.destroy_all() + end + validate :provider_valid?, :data_valid? def provider_valid? diff --git a/spec/models/channel_spec.rb b/spec/models/channel_spec.rb index 7670a0d..12147e2 100644 --- a/spec/models/channel_spec.rb +++ b/spec/models/channel_spec.rb @@ -52,6 +52,17 @@ RSpec.describe DiscourseChat::Channel do expect(channel.rules.size).to eq(2) end + it 'destroys its rules on destroy' do + channel = DiscourseChat::Channel.create({provider:'dummy'}) + expect(channel.rules.size).to eq(0) + rule1 = DiscourseChat::Rule.create(channel: channel) + rule2 = DiscourseChat::Rule.create(channel: channel) + + expect(DiscourseChat::Rule.with_channel(channel).exists?).to eq(true) + channel.destroy() + expect(DiscourseChat::Rule.with_channel(channel).exists?).to eq(false) + end + describe 'validations' do let(:channel) { } From 0a9ef040a1723220700b46e6deffe796d5e5fc37 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Jul 2017 18:23:20 +0100 Subject: [PATCH 10/15] Update specs for updated controller methods --- app/controllers/chat_controller.rb | 19 +-- app/models/rule.rb | 9 ++ app/routes/discourse_chat.rb | 1 - spec/controllers/chat_controller_spec.rb | 166 +++++++++++++++++------ spec/dummy_provider.rb | 2 +- 5 files changed, 137 insertions(+), 60 deletions(-) diff --git a/app/controllers/chat_controller.rb b/app/controllers/chat_controller.rb index 43fbace..a605a53 100644 --- a/app/controllers/chat_controller.rb +++ b/app/controllers/chat_controller.rb @@ -62,6 +62,10 @@ class DiscourseChat::ChatController < ApplicationController begin providers = ::DiscourseChat::Provider.enabled_providers.map {|x| x::PROVIDER_NAME} + if not defined? params[:channel] and defined? params[:channel][:provider] + raise Discourse::InvalidParameters, 'Provider is not valid' + end + requested_provider = params[:channel][:provider] if not providers.include? requested_provider @@ -111,21 +115,6 @@ class DiscourseChat::ChatController < ApplicationController render json: success_json end - - def list_rules - providers = ::DiscourseChat::Provider.enabled_providers.map {|x| x::PROVIDER_NAME} - - requested_provider = params[:provider] - - if not providers.include? requested_provider - raise Discourse::NotFound - end - - rules = DiscourseChat::Rule.with_provider(requested_provider) - - render_serialized rules, DiscourseChat::RuleSerializer, root: 'rules' - end - def create_rule begin hash = params.require(:rule).permit(:channel_id, :filter, :category_id, tags:[]) diff --git a/app/models/rule.rb b/app/models/rule.rb index 3300c4e..0357b3f 100644 --- a/app/models/rule.rb +++ b/app/models/rule.rb @@ -57,6 +57,15 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel end end + # Don't want this to end up as anything other than an integer + def channel_id=(val) + if val.nil? or val.blank? + super(nil) + else + super(val.to_i) + end + end + # Mock foreign key # Could raise RecordNotFound def channel diff --git a/app/routes/discourse_chat.rb b/app/routes/discourse_chat.rb index cf137e6..1b65c53 100644 --- a/app/routes/discourse_chat.rb +++ b/app/routes/discourse_chat.rb @@ -11,7 +11,6 @@ module DiscourseChat put '/channels/:id' => "chat#update_channel" delete '/channels/:id' => "chat#destroy_channel" - get '/rules' => "chat#list_rules" post '/rules' => "chat#create_rule" put '/rules/:id' => "chat#update_rule" delete '/rules/:id' => "chat#destroy_rule" diff --git a/spec/controllers/chat_controller_spec.rb b/spec/controllers/chat_controller_spec.rb index daf5094..a8f4e08 100644 --- a/spec/controllers/chat_controller_spec.rb +++ b/spec/controllers/chat_controller_spec.rb @@ -6,7 +6,9 @@ describe 'Chat Controller', type: :request do let(:topic) { Fabricate(:topic, posts: [first_post]) } let(:admin) { Fabricate(:admin) } let(:category) { Fabricate(:category) } + let(:category2) { Fabricate(:category) } let(:tag) { Fabricate(:tag) } + let(:channel) { DiscourseChat::Channel.create(provider:'dummy') } include_context "dummy provider" @@ -44,17 +46,17 @@ describe 'Chat Controller', type: :request do json = JSON.parse(response.body) - expect(json['providers'].size).to eq(1) + expect(json['providers'].size).to eq(2) expect(json['providers'][0]).to eq('name'=> 'dummy', 'id'=> 'dummy', - 'channel_regex'=> nil + 'channel_parameters'=> [] ) end end end - describe 'testing providers' do + describe 'testing channels' do include_examples 'admin constraints', 'get', '/admin/plugins/chat/test.json' context 'when signed in as an admin' do @@ -63,23 +65,23 @@ describe 'Chat Controller', type: :request do end it 'should return the right response' do - post '/admin/plugins/chat/test.json', provider: 'dummy', channel: '#general', topic_id: topic.id + post '/admin/plugins/chat/test.json', channel_id: channel.id, topic_id: topic.id expect(response).to be_success json = JSON.parse(response.body) end - it 'should fail for invalid provider' do - post '/admin/plugins/chat/test.json', provider: 'someprovider', channel: '#general', topic_id: topic.id + it 'should fail for invalid channel' do + post '/admin/plugins/chat/test.json', channel_id: 999, topic_id: topic.id expect(response).not_to be_success end end end - describe 'viewing rules' do - include_examples 'admin constraints', 'get', '/admin/plugins/chat/rules.json' + describe 'viewing channels' do + include_examples 'admin constraints', 'get', '/admin/plugins/chat/channels.json' context 'when signed in as an admin' do before do @@ -87,29 +89,26 @@ describe 'Chat Controller', type: :request do end it 'should return the right response' do - rule = DiscourseChat::Rule.create({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]}) + rule = DiscourseChat::Rule.create(channel: channel, filter:'follow', category_id:category.id, tags:[tag.name]) - get '/admin/plugins/chat/rules.json', provider:'dummy' + get '/admin/plugins/chat/channels.json', provider:'dummy' expect(response).to be_success - rules = JSON.parse(response.body)['rules'] + channels = JSON.parse(response.body)['channels'] - expect(rules.count).to eq(1) + expect(channels.count).to eq(1) - expect(rules.first).to eq( - "channel" => "#general", - "category_id" => category.id, - "tags" => [tag.name], - "filter" => "follow", - "error_key" => nil, - "id" => rule.id, - "provider" => 'dummy' + expect(channels.first).to eq( + "id" => channel.id, + "provider" => 'dummy', + "data" => {}, + "rules" => [{"id" => rule.id, "filter" => "follow", "channel_id" => channel.id, "category_id" => category.id, "tags" => [tag.name], "error_key" => nil}] ) end it 'should fail for invalid provider' do - get '/admin/plugins/chat/rules.json', provider:'someprovider' + get '/admin/plugins/chat/channels.json', provider:'someprovider' expect(response).not_to be_success end @@ -117,6 +116,98 @@ describe 'Chat Controller', type: :request do end end + describe 'adding a channel' do + include_examples 'admin constraints', 'post', '/admin/plugins/chat/channels.json' + + context 'as an admin' do + + before do + sign_in(admin) + end + + it 'should be able to add a new channel' do + post '/admin/plugins/chat/channels.json', + channel:{ + provider: 'dummy', + data: {} + } + + expect(response).to be_success + + channel = DiscourseChat::Channel.all.first + + expect(channel.provider).to eq('dummy') + end + + it 'should fail for invalid params' do + post '/admin/plugins/chat/channels.json', + channel:{ + provider: 'dummy2', + data: {val: 'something with whitespace'} + } + + expect(response).not_to be_success + + end + end + end + + describe 'updating a channel' do + let(:channel){DiscourseChat::Channel.create(provider:'dummy2', data:{val:"something"})} + + include_examples 'admin constraints', 'put', "/admin/plugins/chat/channels/1.json" + + context 'as an admin' do + + before do + sign_in(admin) + end + + it 'should be able update a channel' do + put "/admin/plugins/chat/channels/#{channel.id}.json", + channel:{ + data: {val: "something-else"} + } + + expect(response).to be_success + + channel = DiscourseChat::Channel.all.first + expect(channel.data).to eq({"val" => "something-else"}) + end + + it 'should fail for invalid params' do + put "/admin/plugins/chat/channels/#{channel.id}.json", + channel:{ + data: {val: "something with whitespace"} + } + + expect(response).not_to be_success + + end + end + end + + describe 'deleting a channel' do + let(:channel){DiscourseChat::Channel.create(provider:'dummy', data:{})} + + include_examples 'admin constraints', 'delete', "/admin/plugins/chat/channels/1.json" + + context 'as an admin' do + + before do + sign_in(admin) + end + + it 'should be able delete a channel' do + delete "/admin/plugins/chat/channels/#{channel.id}.json" + + expect(response).to be_success + + expect(DiscourseChat::Channel.all.size).to eq(0) + end + end + end + describe 'adding a rule' do include_examples 'admin constraints', 'put', '/admin/plugins/chat/rules.json' @@ -127,10 +218,9 @@ describe 'Chat Controller', type: :request do end it 'should be able to add a new rule' do - put '/admin/plugins/chat/rules.json', + post '/admin/plugins/chat/rules.json', rule:{ - provider: 'dummy', - channel: '#general', + channel_id: channel.id, category_id: category.id, filter: 'watch', tags: [tag.name] @@ -140,8 +230,7 @@ describe 'Chat Controller', type: :request do rule = DiscourseChat::Rule.all.first - expect(rule.provider).to eq('dummy') - expect(rule.channel).to eq('#general') + expect(rule.channel_id).to eq(channel.id) expect(rule.category_id).to eq(category.id) expect(rule.filter).to eq('watch') expect(rule.tags).to eq([tag.name]) @@ -149,10 +238,9 @@ describe 'Chat Controller', type: :request do end it 'should fail for invalid params' do - put '/admin/plugins/chat/rules.json', + post '/admin/plugins/chat/rules.json', rule:{ - provider: 'dummy', - channel: '#general', + channel_id: channel.id, category_id: category.id, filter: 'watch', tags: ['somenonexistanttag'] @@ -165,7 +253,7 @@ describe 'Chat Controller', type: :request do end describe 'updating a rule' do - let(:rule){DiscourseChat::Rule.create({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]})} + let(:rule){DiscourseChat::Rule.create(channel: channel, filter:'follow', category_id:category.id, tags:[tag.name])} include_examples 'admin constraints', 'put', "/admin/plugins/chat/rules/1.json" @@ -178,9 +266,8 @@ describe 'Chat Controller', type: :request do it 'should be able update a rule' do put "/admin/plugins/chat/rules/#{rule.id}.json", rule:{ - provider: rule.provider, - channel: '#random', - category_id: rule.category_id, + channel_id: channel.id, + category_id: category2.id, filter: rule.filter, tags: rule.tags } @@ -188,20 +275,13 @@ describe 'Chat Controller', type: :request do expect(response).to be_success rule = DiscourseChat::Rule.all.first - - expect(rule.provider).to eq('dummy') - expect(rule.channel).to eq('#random') - expect(rule.category_id).to eq(category.id) - expect(rule.filter).to eq('follow') - expect(rule.tags).to eq([tag.name]) - + expect(rule.category_id).to eq(category2.id) end it 'should fail for invalid params' do put "/admin/plugins/chat/rules/#{rule.id}.json", rule:{ - provider: 'dummy', - channel: '#general', + channel_id: channel.id, category_id: category.id, filter: 'watch', tags: ['somenonexistanttag'] @@ -214,7 +294,7 @@ describe 'Chat Controller', type: :request do end describe 'deleting a rule' do - let(:rule){DiscourseChat::Rule.create({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]})} + let(:rule){DiscourseChat::Rule.create(channel_id: channel.id, filter:'follow', category_id:category.id, tags:[tag.name])} include_examples 'admin constraints', 'delete', "/admin/plugins/chat/rules/1.json" diff --git a/spec/dummy_provider.rb b/spec/dummy_provider.rb index 7a6c521..f16da02 100644 --- a/spec/dummy_provider.rb +++ b/spec/dummy_provider.rb @@ -48,7 +48,7 @@ RSpec.shared_context "validated dummy provider" do PROVIDER_NAME = "dummy2".freeze PROVIDER_ENABLED_SETTING = :chat_integration_enabled # Tie to main plugin enabled setting CHANNEL_PARAMETERS = [ - {key: "val", regex: '\S+'} + {key: "val", regex: '^\S+$'} ] @@sent_messages = [] From 277285620127f2f476c79383d6255e487d5fe1ae Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Jul 2017 20:42:00 +0100 Subject: [PATCH 11/15] Update qunit tests Have removed a lot of them until the UI is finalised, so as not to waste time constantly updating them. --- .../admin-plugins-chat-edit-channel.js.es6 | 9 +- .../modal/admin-plugins-chat-edit-channel.hbs | 2 +- .../acceptance/chat-integration-test.js.es6 | 115 ++++++++++++------ 3 files changed, 82 insertions(+), 44 deletions(-) diff --git a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-channel.js.es6 b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-channel.js.es6 index b215fba..ca11b66 100644 --- a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-channel.js.es6 +++ b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-channel.js.es6 @@ -6,11 +6,6 @@ import InputValidation from 'discourse/models/input-validation'; export default Ember.Controller.extend(ModalFunctionality, { - initThing: function(){ - console.log("Initialising controller"); - console.log(this.get('model.data')); - }.on('init'), - // The validation property must be defined at runtime since the possible parameters vary by provider setupValidations: function(){ if(this.get('model.provider')){ @@ -24,6 +19,10 @@ export default Ember.Controller.extend(ModalFunctionality, { var regex = new RegExp(regString); var val = this.get('model.channel.data.'+parameter.key); + if(val==undefined){ + val = ""; + } + if(val == ""){ // Fail silently if field blank return InputValidation.create({ failed: true, diff --git a/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-channel.hbs b/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-channel.hbs index 9c3399d..579d071 100644 --- a/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-channel.hbs +++ b/assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-channel.hbs @@ -42,7 +42,7 @@
- {{d-button id="save_rule" class='btn-primary btn-large' action="save" title="chat_integration.edit_channel_modal.save" label="chat_integration.edit_channel_modal.save" disabled=saveDisabled}} + {{d-button id="save_channel" class='btn-primary btn-large' action="save" title="chat_integration.edit_channel_modal.save" label="chat_integration.edit_channel_modal.save" disabled=saveDisabled}} {{d-button class="btn-large" action="cancel" title="chat_integration.edit_channel_modal.cancel" label="chat_integration.edit_channel_modal.cancel"}} diff --git a/test/javascripts/acceptance/chat-integration-test.js.es6 b/test/javascripts/acceptance/chat-integration-test.js.es6 index 604cf6d..580fc8d 100644 --- a/test/javascripts/acceptance/chat-integration-test.js.es6 +++ b/test/javascripts/acceptance/chat-integration-test.js.es6 @@ -11,12 +11,21 @@ acceptance("Chat Integration", { ]; }; server.get('/admin/plugins/chat/providers', () => { - return response({ providers: [{name: 'dummy', id:'dummy',channel_regex:null}] }); + return response({ providers: [{name: 'dummy', id:'dummy',channel_parameters:[{key:'somekey', regex:"^\\S+$"}]}] }); }); - server.get('/admin/plugins/chat/rules', () => { - return response({ rules: [{"id":11,"provider":"dummy","channel":"#general","category_id":2,"tags":null,"filter":"follow","error_key":null}] }); + server.get('/admin/plugins/chat/channels', () => { + return response({"channels":[{"id":97,"provider":"dummy","data":{val:"#general"},"rules":[{"id":98,"channel_id":97,"category_id":null,"tags":[],"filter":"watch","error_key":null}]}]}); }); - server.put('/admin/plugins/chat/rules', () => { + server.post('/admin/plugins/chat/channels', () => { + return response({ }); + }); + server.put('/admin/plugins/chat/channels/:id', () => { + return response({ }); + }); + server.delete('/admin/plugins/chat/channels/:id', () => { + return response({ }); + }); + server.post('/admin/plugins/chat/rules', () => { return response({ }); }); server.put('/admin/plugins/chat/rules/:id', () => { @@ -38,78 +47,108 @@ test("Rules load successfully", assert => { andThen(() => { assert.ok(exists('#admin-plugin-chat table'), "it shows the table of rules"); - assert.equal(find('#admin-plugin-chat table tr td').eq(0).text().trim(), '#general', 'rule displayed'); + assert.equal(find('#admin-plugin-chat table tr td').eq(0).text().trim(), 'All posts and replies', 'rule displayed'); }); }); -test("Create rule works", assert => { +test("Create channel works", assert => { visit("/admin/plugins/chat"); andThen(() => { - click('#create_rule'); + click('#create_channel'); }); andThen(() => { - assert.ok(exists('#chat_integration_edit_rule_modal'), 'it displays the modal'); - assert.ok(find('#save_rule').prop('disabled'), 'it disables the save button'); - fillIn('#channel-field', '#general'); - assert.ok(find('#save_rule').prop('disabled'), 'it enables the save button'); + assert.ok(exists('#chat_integration_edit_channel_modal'), 'it displays the modal'); + assert.ok(find('#save_channel').prop('disabled'), 'it disables the save button'); + fillIn('#chat_integration_edit_channel_modal input', '#general'); }); - click('#save_rule'); + andThen(() => { + assert.ok(!find('#save_channel').prop('disabled'), 'it enables the save button'); + }) andThen(() => { - assert.ok(!exists('#chat_integration_edit_rule_modal'), 'modal closes on save'); + click('#save_channel'); + }); + + andThen(() => { + assert.ok(!exists('#chat_integration_edit_channel_modal'), 'modal closes on save'); }) }); -test("Edit rule works", assert => { - visit("/admin/plugins/chat"); +// test("Edit channel works", assert => { +// visit("/admin/plugins/chat"); - andThen(() => { - assert.ok(exists('.edit:first'), 'edit button is displayed'); - }); +// andThen(() => { +// click('#create_channel'); +// }); - click('.edit:first'); +// andThen(() => { +// assert.ok(exists('#chat_integration_edit_channel_modal'), 'it displays the modal'); +// assert.ok(find('#save_channel').prop('disabled'), 'it disables the save button'); +// fillIn('#chat_integration_edit_channel_modal input', '#general'); +// }); - andThen(() => { - assert.ok(exists('#chat_integration_edit_rule_modal'), 'modal opens on edit'); - assert.ok(!find('#save_rule').prop('disabled'), 'it enables the save button'); - }); +// andThen(() => { +// assert.ok(!find('#save_channel').prop('disabled'), 'it enables the save button'); +// }) - click('#save_rule'); +// andThen(() => { +// click('#save_channel'); +// }); - andThen(() => { - assert.ok(!exists('#chat_integration_edit_rule_modal'), 'modal closes on save'); - }); -}); +// andThen(() => { +// assert.ok(!exists('#chat_integration_edit_channel_modal'), 'modal closes on save'); +// }) -test("Delete rule works", function(assert) { - visit("/admin/plugins/chat"); +// }); - andThen(() => { - assert.ok(exists('.delete:first')); - click('.delete:first'); - }); -}); +// test("Edit rule works", assert => { +// visit("/admin/plugins/chat"); + +// andThen(() => { +// assert.ok(exists('.edit:first'), 'edit button is displayed'); +// }); + +// click('.edit:first'); + +// andThen(() => { +// assert.ok(exists('#chat_integration_edit_rule_modal'), 'modal opens on edit'); +// assert.ok(!find('#save_rule').prop('disabled'), 'it enables the save button'); +// }); + +// click('#save_rule'); + +// andThen(() => { +// assert.ok(!exists('#chat_integration_edit_rule_modal'), 'modal closes on save'); +// }); +// }); + +// test("Delete rule works", function(assert) { +// visit("/admin/plugins/chat"); + +// andThen(() => { +// assert.ok(exists('.delete:first')); +// click('.delete:first'); +// }); +// }); test("Test provider works", assert => { visit("/admin/plugins/chat"); andThen(() => { - click('#test_provider'); + click('.fa-rocket'); }); andThen(() => { assert.ok(exists('#chat_integration_test_modal'), 'it displays the modal'); assert.ok(find('#send_test').prop('disabled'), 'it disables the send button'); - fillIn('#channel-field', '#general'); fillIn('#choose-topic-title', '9318'); }); andThen(() => { - debugger; click('#chat_integration_test_modal .radio:first'); }); From 97c45929501ab70de696ee053c908cf80ed0b376 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Jul 2017 21:21:09 +0100 Subject: [PATCH 12/15] Display provider-specific data for each channel in the UI --- .../templates/admin/plugins-chat-provider.hbs | 9 ++++++++- .../templates/components/channel-details.hbs | 11 ++++++++++- assets/stylesheets/chat-integration-admin.scss | 6 ++---- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs b/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs index 86d57c4..7dc72fa 100644 --- a/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs +++ b/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs @@ -7,7 +7,14 @@ {{/if}} {{# each model.channels as |channel|}} - {{channel-details channel=channel provider=provider store=store refresh='refresh' edit='editChannel' test='testChannel'}} + {{channel-details + channel=channel + provider=model.provider + store=store + refresh='refresh' + edit='editChannel' + test='testChannel' + }} {{/each}} diff --git a/assets/javascripts/discourse/templates/components/channel-details.hbs b/assets/javascripts/discourse/templates/components/channel-details.hbs index ed9a5d1..d3ce21d 100644 --- a/assets/javascripts/discourse/templates/components/channel-details.hbs +++ b/assets/javascripts/discourse/templates/components/channel-details.hbs @@ -1,5 +1,4 @@
- {{channel.data.identifier}}
{{d-button action="edit" actionParam=channel icon="pencil" title="chat_integration.edit_channel" label="chat_integration.edit_channel"}} @@ -7,6 +6,16 @@ {{d-button class='cancel' action="delete" actionParam=channel icon="trash" title="chat_integration.delete_channel" label="chat_integration.delete_channel"}}
+ + + {{# each provider.channel_parameters as |param|}} + {{i18n (concat 'chat_integration.provider.' channel.provider '.param.' param.key '.title')}}: + {{get channel.data param.key}}
+ + {{/each}} + + +
- {{i18n (concat 'chat_integration.provider.' model.provider.id '.title')}} -
- {{text-field - name="channel" - value=model.channel - autofocus="autofocus" - id="channel-field"}} - - {{!--  {{input-tip validation=channelValidation}} --}} -
diff --git a/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs b/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs index 399e99a..86d57c4 100644 --- a/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs +++ b/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs @@ -6,70 +6,13 @@ {{/if}} - - - - - - - - {{#if siteSettings.tagging_enabled}} - - {{/if}} - - - - - - - - {{#each model.rules as |rule|}} - - - - - - - - - {{#if siteSettings.tagging_enabled}} - - {{/if}} - - - - - {{/each}} - -
{{i18n "chat_integration.rule_table.channel"}}{{i18n "chat_integration.rule_table.filter"}}{{i18n "chat_integration.rule_table.category"}}{{i18n "chat_integration.rule_table.tags"}}
- {{#if rule.error_key}} - - {{d-button action="showError" actionParam=rule.error_key class="delete btn-danger" icon="exclamation-triangle"}} - - {{/if}} - - {{rule.channel}} - {{rule.filterName}} - {{#if rule.category}} - {{category-link rule.category allowUncategorized="true" link="false"}} - {{else}} - {{i18n "chat_integration.all_categories"}} - {{/if}} - - {{#if rule.tags}} - {{rule.tags}} - {{else}} - {{i18n "chat_integration.all_tags"}} - {{/if}} - - {{d-button action="edit" actionParam=rule icon="pencil" class="edit" title="chat_integration.rule_table.edit_rule"}} - {{d-button action="delete" actionParam=rule icon="trash-o" class="delete" title="chat_integration.rule_table.delete_rule"}} -
+ {{# each model.channels as |channel|}} + {{channel-details channel=channel provider=provider store=store refresh='refresh' edit='editChannel' test='testChannel'}} + + {{/each}}
- {{d-button id="test_provider" action="test" actionParam=model.provider icon="rocket" title="chat_integration.test_provider" label="chat_integration.test_provider"}} -
- {{d-button id="create_rule" action="create" actionParam=model.provider icon="plus" title="chat_integration.create_rule" label="chat_integration.create_rule"}} + {{d-button id="create_channel" action="createChannel" actionParam=model.provider icon="plus" title="chat_integration.create_channel" label="chat_integration.create_channel"}}
\ No newline at end of file diff --git a/assets/javascripts/discourse/templates/components/channel-details.hbs b/assets/javascripts/discourse/templates/components/channel-details.hbs new file mode 100644 index 0000000..ed9a5d1 --- /dev/null +++ b/assets/javascripts/discourse/templates/components/channel-details.hbs @@ -0,0 +1,40 @@ +
+ {{channel.data.identifier}} +
+ {{d-button action="edit" actionParam=channel icon="pencil" title="chat_integration.edit_channel" label="chat_integration.edit_channel"}} + + {{d-button action="test" actionParam=channel icon="rocket" title="chat_integration.test_channel" label="chat_integration.test_channel"}} + + {{d-button class='cancel' action="delete" actionParam=channel icon="trash" title="chat_integration.delete_channel" label="chat_integration.delete_channel"}} +
+
+
+ + + {{!-- --}} + + + + + {{#if siteSettings.tagging_enabled}} + + {{/if}} + + + + + + + + {{#each channel.rules as |rule|}} + {{rule-row rule=rule refresh=refresh}} + + {{/each}} + +
{{i18n "chat_integration.rule_table.filter"}}{{i18n "chat_integration.rule_table.category"}}{{i18n "chat_integration.rule_table.tags"}}
+
+
+
+ {{d-button action="createRule" actionParam=channel icon="plus" title="chat_integration.create_rule" label="chat_integration.create_rule"}} +
+
\ No newline at end of file diff --git a/assets/javascripts/discourse/templates/components/rule-row.hbs b/assets/javascripts/discourse/templates/components/rule-row.hbs new file mode 100644 index 0000000..9af5d24 --- /dev/null +++ b/assets/javascripts/discourse/templates/components/rule-row.hbs @@ -0,0 +1,53 @@ + +		 + {{#if editing}} + {{combo-box name="filter" content=rule.available_filters value=rule.filter}} + {{else}} + {{rule.filterName}} + {{/if}} + + {{#if editing}} + {{category-chooser + name="category" + value=rule.category_id + rootNoneLabel="chat_integration.all_categories" + rootNone=true + overrideWidths=false + }} + {{else}} + {{#if rule.category}} + {{category-link rule.category allowUncategorized="true" link="false"}} + {{else}} + {{i18n "chat_integration.all_categories"}} + {{/if}} + {{/if}} + + {{#if editing}} + {{tag-chooser placeholderKey="chat_integration.all_tags" name="tags" tags=rule.tags}} + {{else}} + {{#if rule.tags}} + {{rule.tags}} + {{else}} + {{i18n "chat_integration.all_tags"}} + {{/if}} + {{/if}} + + {{#if editing}} + {{d-button action="save" actionParam=rule icon="check" class="ok" title="chat_integration.rule_table.save_rule"}} + {{d-button action="cancel" actionParam=rule icon="times" class="cancel" title="chat_integration.rule_table.cancel_edit"}} + {{else}} + {{d-button action="edit" actionParam=rule icon="pencil" class="edit" title="chat_integration.rule_table.edit_rule"}} + {{d-button action="delete" actionParam=rule icon="trash-o" class="delete" title="chat_integration.rule_table.delete_rule"}} + {{/if}} +
diff --git a/assets/stylesheets/chat-integration-admin.scss b/assets/stylesheets/chat-integration-admin.scss index c9b7a71..60e45b2 100644 --- a/assets/stylesheets/chat-integration-admin.scss +++ b/assets/stylesheets/chat-integration-admin.scss @@ -6,12 +6,12 @@ white-space:nowrap; } td:not(:last-child){ - width: 25%; + width: 30%; } } div.table-footer{ - margin-top: 10px; + margin: 10px; } div.error { @@ -22,8 +22,6 @@ margin-bottom: 10px; background-color: dark-light-diff($quaternary, $secondary, 70%, -70%); padding: 15px; - - } div.channel-details{ From d1d333523f851bd4fea41fa33cd02e9e9497e724 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Jul 2017 21:36:07 +0100 Subject: [PATCH 13/15] Order rules by precedence in the UI and slash commands --- app/helpers/helper.rb | 4 ++-- app/models/rule.rb | 6 ++++++ app/serializers/channel_serializer.rb | 2 +- spec/models/rule_spec.rb | 10 ++++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/app/helpers/helper.rb b/app/helpers/helper.rb index d04b4e9..50c0479 100644 --- a/app/helpers/helper.rb +++ b/app/helpers/helper.rb @@ -3,7 +3,7 @@ module DiscourseChat # Produce a string with a list of all rules associated with a channel def self.status_for_channel(channel) - rules = channel.rules + rules = channel.rules.order_by_precedence provider = channel.provider text = I18n.t("chat_integration.provider.#{provider}.status.header") + "\n" @@ -45,7 +45,7 @@ module DiscourseChat # Delete a rule based on its (1 based) index as seen in the # status_for_channel function def self.delete_by_index(channel, index) - rules = DiscourseChat::Rule.with_channel(channel) + rules = channel.rules.order_by_precedence return false if index < 1 or index > rules.size diff --git a/app/models/rule.rb b/app/models/rule.rb index 0357b3f..79baa64 100644 --- a/app/models/rule.rb +++ b/app/models/rule.rb @@ -80,4 +80,10 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel 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 :order_by_precedence, ->{ order("CASE + WHEN value::json->>'filter' = 'mute' THEN 1 + WHEN value::json->>'filter' = 'watch' THEN 2 + WHEN value::json->>'filter' = 'follow' THEN 3 + END") } + end \ No newline at end of file diff --git a/app/serializers/channel_serializer.rb b/app/serializers/channel_serializer.rb index 4022976..2317434 100644 --- a/app/serializers/channel_serializer.rb +++ b/app/serializers/channel_serializer.rb @@ -4,7 +4,7 @@ class DiscourseChat::ChannelSerializer < ApplicationSerializer attributes :id, :provider, :data, :rules def rules - object.rules.map do |rule| + object.rules.order_by_precedence.map do |rule| DiscourseChat::RuleSerializer.new(rule, root:false) end end diff --git a/spec/models/rule_spec.rb b/spec/models/rule_spec.rb index 0c218ad..f292741 100644 --- a/spec/models/rule_spec.rb +++ b/spec/models/rule_spec.rb @@ -110,6 +110,16 @@ RSpec.describe DiscourseChat::Rule do expect(DiscourseChat::Rule.with_category_id(1).length).to eq(2) expect(DiscourseChat::Rule.with_category_id(nil).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') + rule4 = DiscourseChat::Rule.create(channel:channel, filter:'mute') + + expect(DiscourseChat::Rule.all.length).to eq(4) + + expect(DiscourseChat::Rule.all.order_by_precedence.map(&:filter)).to eq(["mute", "mute", "watch", "follow"]) + end end describe 'validations' do From c5fdebd1bc4c5b8088745eb22dd2d356c1ee44de Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Jul 2017 23:08:06 +0100 Subject: [PATCH 14/15] Store errors per-channel rather than per-rule, and update UI to match --- app/controllers/chat_controller.rb | 3 +- app/models/channel.rb | 2 +- app/models/rule.rb | 6 +-- app/serializers/channel_serializer.rb | 2 +- app/serializers/rule_serializer.rb | 2 +- app/services/manager.rb | 48 +++++++++---------- .../admin/components/channel-details.js.es6 | 6 ++- .../admin-plugins-chat-provider.js.es6 | 5 -- .../templates/components/channel-details.hbs | 4 ++ spec/controllers/chat_controller_spec.rb | 3 +- spec/services/manager_spec.rb | 6 +-- 11 files changed, 42 insertions(+), 45 deletions(-) diff --git a/app/controllers/chat_controller.rb b/app/controllers/chat_controller.rb index a605a53..63e9b9e 100644 --- a/app/controllers/chat_controller.rb +++ b/app/controllers/chat_controller.rb @@ -91,7 +91,7 @@ class DiscourseChat::ChatController < ApplicationController def update_channel begin channel = DiscourseChat::Channel.find(params[:id].to_i) - # rule.error_key = nil # Reset any error on the rule + channel.error_key = nil # Reset any error on the rule allowed_keys = DiscourseChat::Provider.get_by_name(channel.provider)::CHANNEL_PARAMETERS.map{|p| p[:key].to_sym} @@ -134,7 +134,6 @@ class DiscourseChat::ChatController < ApplicationController def update_rule begin rule = DiscourseChat::Rule.find(params[:id].to_i) - rule.error_key = nil # Reset any error on the rule hash = params.require(:rule).permit(:filter, :category_id, tags:[]) if not rule.update(hash) diff --git a/app/models/channel.rb b/app/models/channel.rb index a3238d0..2337ade 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -2,7 +2,7 @@ class DiscourseChat::Channel < DiscourseChat::PluginModel KEY_PREFIX = 'channel:' # Setup ActiveRecord::Store to use the JSON field to read/write these values - store :value, accessors: [ :provider, :data ], coder: JSON + store :value, accessors: [ :provider, :error_key, :data ], coder: JSON after_initialize :init_data diff --git a/app/models/rule.rb b/app/models/rule.rb index 79baa64..6ccd4f4 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, :error_key ], coder: JSON + store :value, accessors: [ :channel_id, :category_id, :tags, :filter ], coder: JSON after_initialize :init_filter @@ -67,9 +67,9 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel end # Mock foreign key - # Could raise RecordNotFound + # Could return nil def channel - DiscourseChat::Channel.find(channel_id) + DiscourseChat::Channel.find_by(id:channel_id) end def channel=(val) self.channel_id = val.id diff --git a/app/serializers/channel_serializer.rb b/app/serializers/channel_serializer.rb index 2317434..e24831f 100644 --- a/app/serializers/channel_serializer.rb +++ b/app/serializers/channel_serializer.rb @@ -1,7 +1,7 @@ require_relative './rule_serializer' class DiscourseChat::ChannelSerializer < ApplicationSerializer - attributes :id, :provider, :data, :rules + attributes :id, :provider, :error_key, :data, :rules def rules object.rules.order_by_precedence.map do |rule| diff --git a/app/serializers/rule_serializer.rb b/app/serializers/rule_serializer.rb index d8b3f77..57a0581 100644 --- a/app/serializers/rule_serializer.rb +++ b/app/serializers/rule_serializer.rb @@ -1,3 +1,3 @@ class DiscourseChat::RuleSerializer < ApplicationSerializer - attributes :id, :channel_id, :category_id, :tags, :filter, :error_key + attributes :id, :channel_id, :category_id, :tags, :filter end \ No newline at end of file diff --git a/app/services/manager.rb b/app/services/manager.rb index ce49d5f..78bee22 100644 --- a/app/services/manager.rb +++ b/app/services/manager.rb @@ -61,35 +61,31 @@ module DiscourseChat # Loop through each rule, and trigger appropriate notifications matching_rules.each do |rule| - channel = rule.channel - provider = ::DiscourseChat::Provider.get_by_name(channel.provider) - is_enabled = ::DiscourseChat::Provider.is_enabled(provider) + # If there are any issues, skip to the next rule + next unless channel = rule.channel + next unless provider = ::DiscourseChat::Provider.get_by_name(channel.provider) + next unless is_enabled = ::DiscourseChat::Provider.is_enabled(provider) - if provider and is_enabled - begin - provider.trigger_notification(post, rule.channel) - rule.update_attribute('error_key', nil) if rule.error_key - rescue => e - if e.class == DiscourseChat::ProviderError and e.info.key?(:error_key) and !e.info[:error_key].nil? - rule.update_attribute('error_key', e.info[:error_key]) - else - rule.update_attribute('error_key','chat_integration.rule_exception') - end - - # Log the error - Discourse.handle_job_exception(e, - message: "Triggering notifications failed", - extra: { provider_name: provider::PROVIDER_NAME, - channel: rule.channel, - post_id: post.id, - error_info: e.class == DiscourseChat::ProviderError ? e.info : nil } - ) + begin + provider.trigger_notification(post, channel) + channel.update_attribute('error_key', nil) if channel.error_key + rescue => e + if e.class == DiscourseChat::ProviderError and e.info.key?(:error_key) and !e.info[:error_key].nil? + channel.update_attribute('error_key', e.info[:error_key]) + else + channel.update_attribute('error_key','chat_integration.channel_exception') end - elsif provider - # Provider is disabled, don't do anything - else - # TODO: Handle when the provider does not exist + + # Log the error + Discourse.handle_job_exception(e, + message: "Triggering notifications failed", + extra: { provider_name: provider::PROVIDER_NAME, + channel: rule.channel, + post_id: post.id, + error_info: e.class == DiscourseChat::ProviderError ? e.info : nil } + ) end + end end diff --git a/assets/javascripts/admin/components/channel-details.js.es6 b/assets/javascripts/admin/components/channel-details.js.es6 index 6e3be76..08fcd1b 100644 --- a/assets/javascripts/admin/components/channel-details.js.es6 +++ b/assets/javascripts/admin/components/channel-details.js.es6 @@ -28,9 +28,11 @@ export default Ember.Component.extend({ createRule(channel){ var newRule = this.get('store').createRecord('rule',{channel_id: channel.id}); channel.rules.pushObject(newRule) - } - + }, + showError(error_key){ + bootbox.alert(I18n.t(error_key)); + }, } }); \ No newline at end of file diff --git a/assets/javascripts/admin/controllers/admin-plugins-chat-provider.js.es6 b/assets/javascripts/admin/controllers/admin-plugins-chat-provider.js.es6 index 335d400..f52965c 100644 --- a/assets/javascripts/admin/controllers/admin-plugins-chat-provider.js.es6 +++ b/assets/javascripts/admin/controllers/admin-plugins-chat-provider.js.es6 @@ -34,11 +34,6 @@ export default Ember.Controller.extend({ var model = {channel:channel} showModal('admin-plugins-chat-test', { model: model, admin: true }); }, - showError(error_key){ - bootbox.alert(I18n.t(error_key)); - }, - - } diff --git a/assets/javascripts/discourse/templates/components/channel-details.hbs b/assets/javascripts/discourse/templates/components/channel-details.hbs index d3ce21d..204fd02 100644 --- a/assets/javascripts/discourse/templates/components/channel-details.hbs +++ b/assets/javascripts/discourse/templates/components/channel-details.hbs @@ -7,6 +7,10 @@ {{d-button class='cancel' action="delete" actionParam=channel icon="trash" title="chat_integration.delete_channel" label="chat_integration.delete_channel"}} + {{#if channel.error_key}} + {{d-button action="showError" actionParam=channel.error_key class="delete btn-danger" icon="exclamation-triangle"}} + {{/if}} + {{# each provider.channel_parameters as |param|}} {{i18n (concat 'chat_integration.provider.' channel.provider '.param.' param.key '.title')}}: diff --git a/spec/controllers/chat_controller_spec.rb b/spec/controllers/chat_controller_spec.rb index a8f4e08..c94e518 100644 --- a/spec/controllers/chat_controller_spec.rb +++ b/spec/controllers/chat_controller_spec.rb @@ -103,7 +103,8 @@ describe 'Chat Controller', type: :request do "id" => channel.id, "provider" => 'dummy', "data" => {}, - "rules" => [{"id" => rule.id, "filter" => "follow", "channel_id" => channel.id, "category_id" => category.id, "tags" => [tag.name], "error_key" => nil}] + "error_key" => nil, + "rules" => [{"id" => rule.id, "filter" => "follow", "channel_id" => channel.id, "category_id" => category.id, "tags" => [tag.name]}] ) end diff --git a/spec/services/manager_spec.rb b/spec/services/manager_spec.rb index 0055960..2180c04 100644 --- a/spec/services/manager_spec.rb +++ b/spec/services/manager_spec.rb @@ -28,18 +28,18 @@ RSpec.describe DiscourseChat::Manager do provider.set_raise_exception(DiscourseChat::ProviderError.new info: {error_key:"hello"}) manager.trigger_notifications(first_post.id) expect(provider.sent_to_channel_ids).to contain_exactly() - expect(DiscourseChat::Rule.all.first.error_key).to eq('hello') + expect(DiscourseChat::Channel.all.first.error_key).to eq('hello') # Triggering a different error should set the error_key to a generic message provider.set_raise_exception(StandardError.new "hello") manager.trigger_notifications(first_post.id) expect(provider.sent_to_channel_ids).to contain_exactly() - expect(DiscourseChat::Rule.all.first.error_key).to eq('chat_integration.rule_exception') + expect(DiscourseChat::Channel.all.first.error_key).to eq('chat_integration.channel_exception') provider.set_raise_exception(nil) manager.trigger_notifications(first_post.id) - expect(DiscourseChat::Rule.all.first.error_key.nil?).to be true + expect(DiscourseChat::Channel.all.first.error_key.nil?).to be true end it "should not send notifications when provider is disabled" do From 5d18e1e44479447386f739300117ac8f9a453ab4 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Jul 2017 23:15:56 +0100 Subject: [PATCH 15/15] Update language strings for new error message --- .../discourse/templates/admin/plugins-chat-provider.hbs | 2 +- config/locales/client.en.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs b/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs index 7dc72fa..e4b2946 100644 --- a/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs +++ b/assets/javascripts/discourse/templates/admin/plugins-chat-provider.hbs @@ -2,7 +2,7 @@ {{#if anyErrors}}
- {{i18n "chat_integration.rules_with_errors"}} + {{i18n "chat_integration.channels_with_errors"}}
{{/if}} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 8afedca..63ac6ce 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4,8 +4,8 @@ en: menu_title: "Chat Integrations" settings: "Settings" no_providers: "You need to enable some providers in the plugin settings" - rules_with_errors: "Some rules for this provider failed last time they were triggered. Click the error icon(s) to learn more." - rule_exception: "An unknown error occured when this rule was last triggered. Check the site logs for more information." + channels_with_errors: "Some channels for this provider failed last time messages were sent. Click the error icon(s) to learn more." + channel_exception: "An unknown error occured when a message was last sent to this channel. Check the site logs for more information." all_categories: "(all categories)" all_tags: "(all tags)" create_rule: "Create Rule"