Use real ActiveRecord for the “Rule” model by extending the PluginStoreRow class.

This gives us all the activerecord awesomeness for free, like validations, safe form params etc.
This commit is contained in:
David Taylor 2017-07-12 18:28:45 +01:00
parent a724463f7c
commit 56da3639ab
8 changed files with 132 additions and 210 deletions

View File

@ -3,7 +3,7 @@ module DiscourseChat
# Produce a string with a list of all rules associated with a channel
def self.status_for_channel(provider, channel)
rules = DiscourseChat::Rule.all_for_channel(provider, channel)
rules = DiscourseChat::Rule.with_channel(provider, channel)
text = I18n.t("chat_integration.provider.#{provider}.status.header") + "\n"
@ -44,7 +44,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(provider, channel, index)
rules = DiscourseChat::Rule.all_for_channel(provider, channel)
rules = DiscourseChat::Rule.with_channel(provider, channel)
return false if index < 1 or index > rules.size
@ -59,7 +59,7 @@ module DiscourseChat
# :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.all_for_channel(provider, channel)
existing_rules = DiscourseChat::Rule.with_channel(provider, channel)
# Select the ones that have the same category
same_category = existing_rules.select { |rule| rule.category_id == category_id }

View File

@ -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.all_for_category(topic.category_id)
matching_rules = DiscourseChat::Rule.with_category(topic.category_id)
if topic.category # Also load the rules for the wildcard category
matching_rules += DiscourseChat::Rule.all_for_category(nil)
matching_rules += DiscourseChat::Rule.with_category(nil)
end
# If tagging is enabled, thow away rules that don't apply to this topic
@ -67,14 +67,13 @@ module DiscourseChat
if provider and is_enabled
begin
provider.trigger_notification(post, rule.channel)
rule.update({error_key: nil}, false) if rule.error_key
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.error_key = e.info[:error_key]
rule.update_attribute('error_key', e.info[:error_key])
else
rule.error_key = 'chat_integration.rule_exception'
rule.update_attribute('error_key','chat_integration.rule_exception')
end
rule.save(false) # Save without validations
# Log the error
Discourse.handle_job_exception(e,

View File

@ -21,6 +21,10 @@ module DiscourseChat
end
end
def self.provider_names
self.providers.map {|x| x::PROVIDER_NAME}
end
def self.enabled_provider_names
self.enabled_providers.map {|x| x::PROVIDER_NAME}
end

View File

@ -1,186 +1,105 @@
# Similar to an ActiveRecord class, but uses PluginStore for storage instead. Adapted from discourse-data-explorer
# Using this means we can use a standard serializer for sending JSON to the client, and also have convenient save/update/delete methods
# Since this is now being used in two plugins, maybe it should be built into core somehow
class DiscourseChat::Rule
attr_accessor :id, :provider, :channel, :category_id, :tags, :filter, :error_key
class DiscourseChat::Rule < PluginStoreRow
PLUGIN_NAME = 'discourse-chat-integration'
KEY_PREFIX = 'rule:'
# Restrict the scope to JSON PluginStoreRows which are for this plugin, and this model
default_scope { where(type_name: 'JSON')
.where(plugin_name: PLUGIN_NAME)
.where("key like?", "#{KEY_PREFIX}%")
}
def initialize(h={})
@provider = ''
@channel = ''
@category_id = nil
@tags = nil
@filter = 'watch'
@error_key = nil
h.each {|k,v| public_send("#{k}=",v)}
# 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
before_save :set_key
after_initialize :init
def init
self.filter ||= 'watch'
end
validates :filter, :inclusion => { :in => %w(watch follow mute),
:message => "%{value} is not a valid filter" }
validate :provider_and_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
def tags=(array)
if array.nil? or array.empty?
@tags = nil
else
@tags = array
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
end
end
def category_id=(val)
if val.nil? or val.blank?
@category_id = nil
else
@category_id = val.to_i
def category_valid?
# Validate category
if not (category_id.nil? or Category.where(id: category_id).exists?)
errors.add(:category_id, "#{category_id} is not a valid category id")
end
end
def tags_valid?
# Validate tags
return if tags.nil?
tags.each do |tag|
if not Tag.where(name: tag).exists?
errors.add(:tags, "#{tag} is not a valid tag")
end
end
end
# saving/loading functions
def self.alloc_id
DistributedMutex.synchronize('discourse-chat_rule-id') do
max_id = DiscourseChat.pstore_get("rule:_id")
# We never want an empty array, set it to nil instead
def tags=(array)
if array.nil? or array.empty?
super(nil)
else
super(array)
end
end
# Don't want this to end up as anything other than an integer
def category_id=(val)
if val.nil? or val.blank?
super(nil)
else
super(val.to_i)
end
end
scope :with_provider, ->(provider) { where("value::json->>'provider'=?", provider)}
scope :with_channel, ->(provider, channel) { with_provider(provider).where("value::json->>'channel'=?", channel)}
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)}
private
def set_key
self.key ||= alloc_key
end
def alloc_key
DistributedMutex.synchronize("#{PLUGIN_NAME}_#{KEY_PREFIX}_id") do
max_id = PluginStore.get(PLUGIN_NAME, "#{KEY_PREFIX}_id")
max_id = 1 unless max_id
DiscourseChat.pstore_set("rule:_id", max_id + 1)
max_id
PluginStore.set(PLUGIN_NAME, "#{KEY_PREFIX}_id", max_id + 1)
"#{KEY_PREFIX}#{max_id}"
end
end
def self.from_hash(h)
rule = DiscourseChat::Rule.new
[:provider, :channel, :category_id, :tags, :filter, :error_key].each do |sym|
rule.send("#{sym}=", h[sym]) if h[sym]
end
if h[:id]
rule.id = h[:id].to_i
end
rule
end
def to_hash
{
id: @id,
provider: @provider,
channel: @channel,
category_id: @category_id,
tags: @tags,
filter: @filter,
error_key: @error_key,
}
end
def self.find(id, opts={})
hash = DiscourseChat.pstore_get("rule:#{id}")
unless hash
return DiscourseChat::Rule.new if opts[:ignore_deleted]
raise Discourse::NotFound
end
from_hash hash
end
def update(h, validate=true)
[:provider, :channel, :category_id, :tags, :filter, :error_key].each do |sym|
public_send("#{sym}=", h[sym]) if h.key?(sym)
end
save(validate)
end
def save(validate=true)
if validate
return false if not valid?
end
unless @id && @id > 0
@id = self.class.alloc_id
end
DiscourseChat.pstore_set "rule:#{id}", to_hash
return self
end
def save!(validate=true)
if not save(validate)
raise 'Rule not valid'
end
return self
end
def valid?
# Validate provider
return false if not ::DiscourseChat::Provider.providers.map {|x| x::PROVIDER_NAME}.include? @provider
# Validate channel
return false if @channel.blank?
provider = ::DiscourseChat::Provider.get_by_name(@provider)
if defined? provider::PROVIDER_CHANNEL_REGEX
channel_regex = Regexp.new provider::PROVIDER_CHANNEL_REGEX
return false if not channel_regex.match?(@channel)
end
# Validate category
return false if not (@category_id.nil? or Category.where(id: @category_id).exists?)
# Validate tags
if not @tags.nil?
@tags.each do |tag|
return false if not Tag.where(name: tag).exists?
end
end
# Validate filter
return false if not ['watch','follow','mute'].include? @filter
return true
end
def destroy
DiscourseChat.pstore_delete "rule:#{id}"
end
def read_attribute_for_serialization(attr)
self.send(attr)
end
def self.all_for_provider(provider)
self.where("value::json->>'provider'=?", provider)
end
def self.all_for_channel(provider, channel)
self.where("value::json->>'provider'=? AND value::json->>'channel'=?", provider, channel)
end
def self.all_for_category(category_id)
if category_id.nil?
self.where("json_typeof(value::json->'category_id')='null'")
else
self.where("value::json->>'category_id'=?", category_id.to_s)
end
end
# Use JSON selectors like this:
# Rule.where("value::json->>'provider'=?", "slack")
def self.where(*args)
rows = self._all_raw.where(*args)
self._from_psr_rows(rows)
end
def self.all
self._from_psr_rows(self._all_raw)
end
def self._all_raw
PluginStoreRow.where(plugin_name: DiscourseChat.plugin_name)
.where("key LIKE 'rule:%'")
.where("key != 'rule:_id'")
end
def self._from_psr_rows(raw)
rules = raw.map do |psr|
from_hash PluginStore.cast_value(psr.type_name, psr.value)
end
filter_order = ["mute", "watch", "follow"]
rules = rules.sort_by{ |r| [r.channel, r.category_id.nil? ? 0 : r.category_id, filter_order.index(r.filter)] }
return rules
end
def self.destroy_all
self._all_raw().destroy_all
end
end
end

View File

@ -119,7 +119,7 @@ after_initialize do
requested_provider = params[:provider]
if providers.include? requested_provider
rules = DiscourseChat::Rule.all_for_provider(requested_provider)
rules = DiscourseChat::Rule.with_provider(requested_provider)
else
raise Discourse::NotFound
end
@ -129,10 +129,11 @@ after_initialize do
def create_rule
begin
rule = DiscourseChat::Rule.new()
hash = params.require(:rule)
hash = params.require(:rule).permit(:provider, :channel, :filter, :category_id, tags:[])
if not rule.update(hash)
rule = DiscourseChat::Rule.new(hash)
if not rule.save(hash)
raise Discourse::InvalidParameters, 'Rule is not valid'
end
@ -146,8 +147,8 @@ after_initialize do
begin
rule = DiscourseChat::Rule.find(params[:id].to_i)
rule.error_key = nil # Reset any error on the rule
hash = params.require(:rule)
hash = params.require(:rule).permit(:provider, :channel, :filter, :category_id, tags:[])
if not rule.update(hash)
raise Discourse::InvalidParameters, 'Rule is not valid'
end

View File

@ -87,7 +87,7 @@ describe 'Chat Controller', type: :request do
end
it 'should return the right response' do
rule = DiscourseChat::Rule.new({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]}).save!
rule = DiscourseChat::Rule.create({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]})
get '/admin/plugins/chat/rules.json', provider:'dummy'
@ -165,7 +165,7 @@ describe 'Chat Controller', type: :request do
end
describe 'updating a rule' do
let(:rule){DiscourseChat::Rule.new({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]}).save!}
let(:rule){DiscourseChat::Rule.create({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]})}
include_examples 'admin constraints', 'put', "/admin/plugins/chat/rules/1.json"
@ -214,7 +214,7 @@ describe 'Chat Controller', type: :request do
end
describe 'deleting a rule' do
let(:rule){DiscourseChat::Rule.new({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]}).save!}
let(:rule){DiscourseChat::Rule.create({provider: 'dummy', channel: '#general', filter:'follow', category_id:category.id, tags:[tag.name]})}
include_examples 'admin constraints', 'delete', "/admin/plugins/chat/rules/1.json"

View File

@ -18,7 +18,7 @@ RSpec.describe DiscourseChat::Manager do
end
def create_rule(provider, channel, filter, category_id, tags) # Just shorthand for testing purposes
DiscourseChat::Rule.new({provider: provider, channel: channel, filter:filter, category_id:category_id, tags:tags}).save!
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

View File

@ -1,29 +1,28 @@
require 'rails_helper'
RSpec.describe DiscourseChat::Rule do
let(:tag1){Fabricate(:tag)}
let(:tag2){Fabricate(:tag)}
describe '.alloc_id' do
describe '.alloc_key' do
it 'should return sequential numbers' do
expect( DiscourseChat::Rule.alloc_id ).to eq(1)
expect( DiscourseChat::Rule.alloc_id ).to eq(2)
expect( DiscourseChat::Rule.alloc_id ).to eq(3)
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")
end
end
it 'should save and load successfully' do
expect(DiscourseChat::Rule.all.length).to eq(0)
rule = DiscourseChat::Rule.new({
rule = DiscourseChat::Rule.create({
provider:"slack",
channel: "#general",
category_id: 1,
tags: [tag1.name, tag2.name],
filter: 'watch'
}).save!
})
expect(DiscourseChat::Rule.all.length).to eq(1)
@ -39,12 +38,12 @@ RSpec.describe DiscourseChat::Rule do
describe 'general operations' do
before do
rule = DiscourseChat::Rule.new({
rule = DiscourseChat::Rule.create({
provider:"slack",
channel: "#general",
category_id: 1,
tags: [tag1.name, tag2.name]
}).save!
})
end
it 'can be modified' do
@ -86,8 +85,8 @@ RSpec.describe DiscourseChat::Rule do
expect(DiscourseChat::Rule.all.length).to eq(3)
expect(DiscourseChat::Rule.all_for_provider('slack').length).to eq(2)
expect(DiscourseChat::Rule.all_for_provider('telegram').length).to eq(1)
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
@ -98,8 +97,8 @@ RSpec.describe DiscourseChat::Rule do
expect(DiscourseChat::Rule.all.length).to eq(5)
expect(DiscourseChat::Rule.all_for_channel('slack','#general').length).to eq(3)
expect(DiscourseChat::Rule.all_for_channel('slack', '#blah').length).to eq(1)
expect(DiscourseChat::Rule.with_channel('slack','#general').length).to eq(3)
expect(DiscourseChat::Rule.with_channel('slack', '#blah').length).to eq(1)
end
it 'can be filtered by category' do
@ -108,20 +107,20 @@ RSpec.describe DiscourseChat::Rule do
expect(DiscourseChat::Rule.all.length).to eq(3)
expect(DiscourseChat::Rule.all_for_category(1).length).to eq(2)
expect(DiscourseChat::Rule.all_for_category(nil).length).to eq(1)
expect(DiscourseChat::Rule.with_category(1).length).to eq(2)
expect(DiscourseChat::Rule.with_category(nil).length).to eq(1)
end
end
describe 'validations' do
let(:rule) do
DiscourseChat::Rule.new({
DiscourseChat::Rule.create({
filter: 'watch',
provider:"slack",
channel: "#general",
category_id: 1,
}).save!
})
end
it 'validates provider correctly' do