Refactor plugin.

app/models/channel.rb

@@ -4,66 +4,66 @@ class DiscourseChat::Channel < DiscourseChat::PluginModel
   # Setup ActiveRecord::Store to use the JSON field to read/write these values
   store :value, accessors: [ :provider, :error_key, :data ], coder: JSON
 
+  scope :with_provider, ->(provider) { where("value::json->>'provider'=?", provider) }
+  scope :with_data_value, ->(key, value) { where("(value::json->>'data')::json->>?=?", key.to_s, value.to_s) }
+
   after_initialize :init_data
-
-  def init_data
-    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?
-    # Validate provider
-    if not ::DiscourseChat::Provider.provider_names.include? provider
-      errors.add(:provider, "#{provider} is not a valid provider")
-      return
-    end
-  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.map { |p| p[:key] }.sort == data.keys.sort
-      errors.add(:data, "data does not match the required structure for provider #{provider}")
-      return
-    end
-
-    check_unique = false
-    matching_channels = DiscourseChat::Channel.with_provider(provider).where.not(id: id)
-
-    data.each do |key, value|
-      regex_string = params.find { |p| p[:key] == key }[:regex]
-      if !Regexp.new(regex_string).match(value)
-        errors.add(:data, "data.#{key} is invalid")
-      end
-
-      unique = params.find { |p| p[:key] == key }[:unique]
-      if unique
-        check_unique = true
-        matching_channels = matching_channels.with_data_value(key, value)
-      end
-    end
-
-    if check_unique && matching_channels.exists?
-      errors.add(:data, "matches an existing channel")
-    end
-
-  end
-
   def rules
     DiscourseChat::Rule.with_channel_id(id).order_by_precedence
   end
 
-  scope :with_provider, ->(provider) { where("value::json->>'provider'=?", provider) }
+  private
 
-  scope :with_data_value, ->(key, value) { where("(value::json->>'data')::json->>?=?", key.to_s, value.to_s) }
+    def init_data
+      self.data = {} if self.data.nil?
+    end
 
+    def destroy_rules
+      rules.destroy_all
+    end
+
+    def provider_valid?
+      # Validate provider
+      if not ::DiscourseChat::Provider.provider_names.include? provider
+        errors.add(:provider, "#{provider} is not a valid provider")
+        return
+      end
+    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.map { |p| p[:key] }.sort == data.keys.sort
+        errors.add(:data, "data does not match the required structure for provider #{provider}")
+        return
+      end
+
+      check_unique = false
+      matching_channels = DiscourseChat::Channel.with_provider(provider).where.not(id: id)
+
+      data.each do |key, value|
+        regex_string = params.find { |p| p[:key] == key }[:regex]
+        if !Regexp.new(regex_string).match(value)
+          errors.add(:data, "data.#{key} is invalid")
+        end
+
+        unique = params.find { |p| p[:key] == key }[:unique]
+        if unique
+          check_unique = true
+          matching_channels = matching_channels.with_data_value(key, value)
+        end
+      end
+
+      if check_unique && matching_channels.exists?
+        errors.add(:data, "matches an existing channel")
+      end
+
+    end
 end

app/models/plugin_model.rb

@@ -2,13 +2,10 @@ class DiscourseChat::PluginModel < PluginStoreRow
   PLUGIN_NAME = 'discourse-chat-integration'
   KEY_PREFIX = 'unimplemented'
 
-  after_initialize :init_plugin_model
   default_scope { self.default_scope }
 
-  def init_plugin_model
+  after_initialize :init_plugin_model
-    self.type_name ||= 'JSON'
+  before_save :set_key
-    self.plugin_name ||= PLUGIN_NAME
-  end
 
   # Restrict the scope to JSON PluginStoreRows which are for this plugin, and this model
   def self.default_scope
@@ -17,14 +14,17 @@ class DiscourseChat::PluginModel < PluginStoreRow
       .where("key LIKE ?", "#{self::KEY_PREFIX}%")
   end
 
-  before_save :set_key
-
   private
 
     def set_key
       self.key ||= self.class.alloc_key
     end
 
+    def init_plugin_model
+      self.type_name ||= 'JSON'
+      self.plugin_name ||= PLUGIN_NAME
+    end
+
     def self.alloc_key
       raise "KEY_PREFIX must be defined" if self::KEY_PREFIX == 'unimplemented'
       DistributedMutex.synchronize("#{self::PLUGIN_NAME}_#{self::KEY_PREFIX}_id") do

app/models/rule.rb

@@ -4,12 +4,24 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel
   # Setup ActiveRecord::Store to use the JSON field to read/write these values
   store :value, accessors: [ :channel_id, :type, :group_id, :category_id, :tags, :filter ], coder: JSON
 
-  after_initialize :init_filter
+  scope :with_type, ->(type) { where("value::json->>'type'=?", type.to_s) }
+  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_id, ->(category_id) { category_id.nil? ? where("(value::json->'category_id') IS NULL OR json_typeof(value::json->'category_id')='null'") : where("value::json->>'category_id'=?", category_id.to_s) }
+  scope :with_group_ids, ->(group_id) { where("value::json->>'group_id' IN (?)", group_id.map(&:to_s)) }
 
-  def init_filter
+  scope :order_by_precedence, -> { order("CASE
-    self.filter ||= 'watch'
+                                          WHEN value::json->>'type' = 'group_mention' THEN 1
-    self.type ||= 'normal'
+                                          WHEN value::json->>'type' = 'group_message' THEN 2
-  end
+                                          ELSE 3
+                                         END",
+                                        "CASE
+                                          WHEN value::json->>'filter' = 'mute' THEN 1
+                                          WHEN value::json->>'filter' = 'watch' THEN 2
+                                          WHEN value::json->>'filter' = 'follow' THEN 3
+                                         END") }
+
+  after_initialize :init_filter
 
   validates :filter, inclusion: { in: %w(watch follow mute),
                                   message: "%{value} is not a valid filter" }
@@ -19,49 +31,6 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel
 
   validate :channel_valid?, :category_valid?, :group_valid?, :tags_valid?
 
-  def channel_valid?
-    # Validate channel
-    if not (DiscourseChat::Channel.where(id: channel_id).exists?)
-      errors.add(:channel_id, "#{channel_id} is not a valid channel id")
-    end
-  end
-
-  def category_valid?
-    if type != 'normal' && !category_id.nil?
-      errors.add(:category_id, "cannot be specified for that type of rule")
-    end
-
-    return unless type == 'normal'
-
-    # Validate category
-    if not (category_id.nil? || Category.where(id: category_id).exists?)
-      errors.add(:category_id, "#{category_id} is not a valid category id")
-    end
-  end
-
-  def group_valid?
-    if type == 'normal' && !group_id.nil?
-      errors.add(:group_id, "cannot be specified for that type of rule")
-    end
-
-    return if type == 'normal'
-
-    # Validate group
-    if not Group.where(id: group_id).exists?
-      errors.add(:group_id, "#{group_id} is not a valid group 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
-
   # We never want an empty array, set it to nil instead
   def tags=(array)
     if array.nil? || array.empty?
@@ -92,23 +61,49 @@ class DiscourseChat::Rule < DiscourseChat::PluginModel
     self.channel_id = val.id
   end
 
-  scope :with_type, ->(type) { where("value::json->>'type'=?", type.to_s) }
+  private
 
-  scope :with_channel, ->(channel) { with_channel_id(channel.id) }
+    def channel_valid?
-  scope :with_channel_id, ->(channel_id) { where("value::json->>'channel_id'=?", channel_id.to_s) }
+      if !(DiscourseChat::Channel.where(id: channel_id).exists?)
+        errors.add(:channel_id, "#{channel_id} is not a valid channel id")
+      end
+    end
 
-  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) }
+    def category_valid?
-  scope :with_group_ids, ->(group_id) { where("value::json->>'group_id' IN (?)", group_id.map(&:to_s)) }
+      if type != 'normal' && !category_id.nil?
+        errors.add(:category_id, "cannot be specified for that type of rule")
+      end
 
-  scope :order_by_precedence, -> { order("CASE
+      return unless type == 'normal'
-                                          WHEN value::json->>'type' = 'group_mention' THEN 1
-                                          WHEN value::json->>'type' = 'group_message' THEN 2
-                                          ELSE 3
-                                         END",
-                                        "CASE
-                                          WHEN value::json->>'filter' = 'mute' THEN 1
-                                          WHEN value::json->>'filter' = 'watch' THEN 2
-                                          WHEN value::json->>'filter' = 'follow' THEN 3
-                                         END") }
 
+      if !(category_id.nil? || Category.where(id: category_id).exists?)
+        errors.add(:category_id, "#{category_id} is not a valid category id")
+      end
+    end
+
+    def group_valid?
+      if type == 'normal' && !group_id.nil?
+        errors.add(:group_id, "cannot be specified for that type of rule")
+      end
+
+      return if type == 'normal'
+
+      if !Group.where(id: group_id).exists?
+        errors.add(:group_id, "#{group_id} is not a valid group id")
+      end
+    end
+
+    def tags_valid?
+      return if tags.nil?
+      tags.each do |tag|
+        if !Tag.where(name: tag).exists?
+          errors.add(:tags, "#{tag} is not a valid tag")
+        end
+      end
+    end
+
+    def init_filter
+      self.filter ||= 'watch'
+      self.type ||= 'normal'
+    end
 end

app/serializers/rule_serializer.rb

@@ -3,11 +3,10 @@ class DiscourseChat::RuleSerializer < ApplicationSerializer
 
   def group_name
     if object.group_id
-      groups = Group.where(id: object.group_id)
+      if group = Group.find_by(id: object.group_id)
-      if groups.exists?
+        group.name
-        return groups.first.name
       else
-        return I18n.t("chat_integration.deleted_group")
+        I18n.t("chat_integration.deleted_group")
       end
     end
   end