From e07a4da46036f21f12a680b14fb2a7647efab9f6 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 13 Jul 2017 16:09:34 +0100 Subject: [PATCH] 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