Use new DiscourseChat::Rule model throughout the backend code

assets/javascripts/admin/controllers/modals/admin-plugins-chat-edit-rule.js.es6

@@ -1,14 +1,32 @@
 import Rule from 'discourse/plugins/discourse-chat/admin/models/rule'
+import ModalFunctionality from 'discourse/mixins/modal-functionality';
 import { ajax } from 'discourse/lib/ajax';
+import { extractError } from 'discourse/lib/ajax-error';
 
-export default Ember.Controller.extend({
+export default Ember.Controller.extend(ModalFunctionality, {
 
   model: Rule.create({}),
 
   actions: {
     cancel: function(){
-
       this.send('closeModal');
     },
+
+    save: function(){
+
+    	const self = this;
+
+    	this.get('model').update().then(function(result) {
+        self.send('closeModal');
+        if (result.responseJson.success) {
+          self.transitionToRoute('tags.show', result.responseJson.tag.id);
+        } else {
+          self.flash(extractError(result.responseJson.errors[0]), 'error');
+        }
+      }).catch(function(error) {
+        self.flash(extractError(error), 'error');
+      });
+
+    }
   }
 });

assets/javascripts/admin/models/rule.js.es6

@@ -10,21 +10,26 @@ export default RestModel.extend({
   ],
 
   category_id: null,
+  tags: null,
   provider: '',
   channel: '',
   filter: null,
 
   @computed('category_id')
-  categoryName(categoryId) {
+  category(categoryId) {
-    if (categoryId)
+    if (categoryId){
-      return Category.findById(categoryId).get('name');
+      return Category.findById(categoryId);
-    else {
+    }else {
-      return I18n.t('slack.choose.all_categories');
+      return false;
     }
   },
 
   @computed('filter')
   filterName(filter) {
     return I18n.t(`slack.present.${filter}`);
+  },
+
+  updateProperties() {
+    return ['category_id','provider','channel', 'tags','filter'];
   }
 });

assets/javascripts/admin/templates/modal/admin-plugins-chat-edit-rule.hbs

@@ -6,9 +6,11 @@
       <label for="category">{{i18n "chat.category"}}</label>
       {{category-chooser
       			name="category"
-            value=model.categoryId
+            value=model.category_id
             rootNoneLabel="slack.choose.all_categories"
-            rootNone=true}} 
+            rootNone=true
+            allowUncategorized="true"
+            }} 
     </div>
 
     {{#if siteSettings.tagging_enabled}}

lib/manager.rb

@@ -1,50 +1,10 @@
 module DiscourseChat
   module Manager
-    KEY_PREFIX = 'category_'.freeze
 
     def self.guardian
       Guardian.new(User.find_by(username: SiteSetting.chat_discourse_username))
     end
 
-    def self.get_store_key(cat_id = nil)
-      "#{KEY_PREFIX}#{cat_id.present? ? cat_id : '*'}"
-    end
-
-    def self.get_all_rules
-      rules = []
-      PluginStoreRow.where(plugin_name: DiscourseChat::PLUGIN_NAME)
-        .where("key ~* :pattern", pattern: "^#{DiscourseChat::Manager::KEY_PREFIX}.*")
-        .each do |row|
-        PluginStore.cast_value(row.type_name, row.value).each_with_index do |rule, thisIndex|
-          category_id =
-            if row.key == DiscourseChat::Manager.get_store_key
-              nil
-            else
-              row.key.gsub!(DiscourseChat::Manager::KEY_PREFIX, '')
-              row.key
-            end
-
-          rules << {
-            id: "#{(category_id || 'all')}_#{thisIndex}",
-            provider: rule[:provider],
-            channel: rule[:channel],
-            filter: rule[:filter],
-            category_id: category_id,
-            tags: rule[:tags]
-          }
-        end
-      end
-      return rules
-    end
-
-    def self.get_rules_for_provider(provider)
-      get_all_rules.select { |rule| rule[:provider] == provider }
-    end
-
-    def self.get_rules_for_category(cat_id = nil)      
-      PluginStore.get(DiscourseChat::PLUGIN_NAME, get_store_key(cat_id)) || []
-    end
-
     def self.trigger_notifications(post_id)
       Rails.logger.info("Triggering chat notifications for post #{post_id}")
 
@@ -62,37 +22,37 @@ module DiscourseChat
       return if topic.blank? || topic.archetype == Archetype.private_message
 
       # Load all the rules that apply to this topic's category
-      matching_rules = get_rules_for_category(topic.category_id)
+      matching_rules = DiscourseChat::Rule.all_for_category(topic.category_id)
 
       if topic.category # Also load the rules for the wildcard category
-        matching_rules += get_rules_for_category(nil)
+        matching_rules += DiscourseChat::Rule.all_for_category(nil)
       end
 
       # If tagging is enabled, thow away rules that don't apply to this topic
       if SiteSetting.tagging_enabled
         topic_tags = topic.tags.present? ? topic.tags.pluck(:name) : []
         matching_rules = matching_rules.select do |rule|
-          next true if rule[:tags].nil? or rule[:tags].empty? # Filter has no tags specified
+          next true if rule.tags.nil? or rule.tags.empty? # Filter has no tags specified
-          any_tags_match = !((rule[:tags] & topic_tags).empty?)
+          any_tags_match = !((rule.tags & topic_tags).empty?)
           next any_tags_match # If any tags match, keep this filter, otherwise throw away
         end
       end
 
       # Sort by order of precedence (mute always wins; watch beats follow)
       precedence = { 'mute' => 0, 'watch' => 1, 'follow' => 2}
-      sort_func = proc { |a, b| precedence[a[:filter]] <=> precedence[b[:filter]] }
+      sort_func = proc { |a, b| precedence[a.filter] <=> precedence[b.filter] }
       matching_rules = matching_rules.sort(&sort_func)
 
       # Take the first rule for each channel
-      uniq_func = proc { |rule| rule.values_at(:provider, :channel) }
+      uniq_func = proc { |rule| [rule.provider, rule.channel] }
       matching_rules = matching_rules.uniq(&uniq_func)
 
       # If a matching rule is set to mute, we can discard it now
-      matching_rules = matching_rules.select { |rule| rule[:filter] != "mute" }
+      matching_rules = matching_rules.select { |rule| rule.filter != "mute" }
 
       # If this is not the first post, discard all "follow" rules
       if not post.is_first_post?
-        matching_rules = matching_rules.select { |rule| rule[:filter] != "follow" }
+        matching_rules = matching_rules.select { |rule| rule.filter != "follow" }
       end
 
       # All remaining rules now require a notification to be sent
@@ -101,10 +61,10 @@ module DiscourseChat
 
       # Loop through each rule, and trigger appropriate notifications
       matching_rules.each do |rule|
-        Rails.logger.info("Sending notification to provider #{rule[:provider]}, channel #{rule[:channel]}")
+        Rails.logger.info("Sending notification to provider #{rule.provider}, channel #{rule.channel}")
-        provider = ::DiscourseChat::Provider.get_by_name(rule[:provider])
+        provider = ::DiscourseChat::Provider.get_by_name(rule.provider)
         if provider
-          provider.trigger_notification(post, rule[:channel])
+          provider.trigger_notification(post, rule.channel)
         else
           puts "Can't find provider"
           # TODO: Handle when the provider does not exist
@@ -113,16 +73,6 @@ module DiscourseChat
 
     end
 
-    def self.create_rule(provider, channel, filter, category_id, tags)
-      raise "Invalid filter" if !['mute','follow','watch'].include?(filter)
-
-      data = get_rules_for_category(category_id)
-      tags = Tag.where(name: tags).pluck(:name)
-      tags = nil if tags.blank?
-
-      data.push(provider: provider, channel: channel, filter: filter, tags: tags)
-      PluginStore.set(DiscourseChat::PLUGIN_NAME, get_store_key(category_id), data)
-    end
 
   end
 end

lib/rule.rb

@@ -79,7 +79,7 @@
     end
 
     # Use JSON selectors like this:
-    #    Rule.where("value::json->>'provider'=?", "telegram")
+    #    Rule.where("value::json->>'provider'=?", "slack")
     def self.where(*args)
       rows = self._all_raw.where(*args)
       self._from_psr_rows(rows)

plugin.rb

@@ -5,6 +5,7 @@
 
 enabled_site_setting :chat_enabled
 
+
 after_initialize do
 
   module ::DiscourseChat
@@ -60,19 +61,24 @@ after_initialize do
       requested_provider = params[:provider]
 
       if requested_provider.nil?
-        rules = DiscourseChat::Manager.get_all_rules()
+        rules = DiscourseChat::Rule.all
       elsif providers.include? requested_provider
-        rules = DiscourseChat::Manager.get_rules_for_provider(requested_provider)
+        rules = DiscourseChat::Rule.all_for_provider(requested_provider)
       else
         raise Discourse::NotFound
       end
 
-      render json: rules, root: 'rules'
+      render_serialized rules, DiscourseChat::RuleSerializer, root: 'rules'
     end
 
+    def update_rule
+      render json: {success: false}
+    end
   end
 
-
+  class DiscourseChat::RuleSerializer < ActiveModel::Serializer
+    attributes :id, :provider, :channel, :category_id, :tags, :filter
+  end
 
   require_dependency 'admin_constraint'
 
@@ -82,7 +88,9 @@ after_initialize do
   DiscourseChat::Engine.routes.draw do
     get "" => "chat#respond"
     get '/providers' => "chat#list_providers"
+    
     get '/rules' => "chat#list_rules"
+    put 'rules/:id' => "chat#update_rule"
 
     get "/:provider" => "chat#respond"
   end

spec/lib/manager_spec.rb

@@ -31,10 +31,15 @@ RSpec.describe DiscourseChat::Manager do
 
     let(:provider) {::DiscourseChat::Provider::DummyProvider}
 
+    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
+    end
+
     it "should send a notification to watched and following channels for new topic" do
-      manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
+
-      manager.create_rule('dummy', 'chan2', 'follow', category.id, nil)
+      create_rule('dummy', 'chan1', 'watch', category.id, nil)
-      manager.create_rule('dummy', 'chan3', 'mute', category.id, nil)
+      create_rule('dummy', 'chan2', 'follow', category.id, nil)
+      create_rule('dummy', 'chan3', 'mute', category.id, nil)
 
       manager.trigger_notifications(first_post.id)
 
@@ -42,9 +47,9 @@ RSpec.describe DiscourseChat::Manager do
     end
 
     it "should send a notification only to watched for reply" do
-      manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
+      create_rule('dummy', 'chan1', 'watch', category.id, nil)
-      manager.create_rule('dummy', 'chan2', 'follow', category.id, nil)
+      create_rule('dummy', 'chan2', 'follow', category.id, nil)
-      manager.create_rule('dummy', 'chan3', 'mute', category.id, nil)
+      create_rule('dummy', 'chan3', 'mute', category.id, nil)
 
       manager.trigger_notifications(second_post.id)
 
@@ -52,7 +57,7 @@ RSpec.describe DiscourseChat::Manager do
     end
 
     it "should respect wildcard category settings" do
-      manager.create_rule('dummy', 'chan1', 'watch', nil, nil)
+      create_rule('dummy', 'chan1', 'watch', nil, nil)
 
       manager.trigger_notifications(first_post.id)
 
@@ -60,8 +65,8 @@ RSpec.describe DiscourseChat::Manager do
     end
 
     it "should respect mute over watch" do
-      manager.create_rule('dummy', 'chan1', 'watch', nil, nil) # Wildcard watch
+      create_rule('dummy', 'chan1', 'watch', nil, nil) # Wildcard watch
-      manager.create_rule('dummy', 'chan1', 'mute', category.id, nil) # Specific mute
+      create_rule('dummy', 'chan1', 'mute', category.id, nil) # Specific mute
 
       manager.trigger_notifications(first_post.id)
 
@@ -69,8 +74,8 @@ RSpec.describe DiscourseChat::Manager do
     end
 
     it "should respect watch over follow" do
-      manager.create_rule('dummy', 'chan1', 'follow', nil, nil)
+      create_rule('dummy', 'chan1', 'follow', nil, nil)
-      manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
+      create_rule('dummy', 'chan1', 'watch', category.id, nil)
 
       manager.trigger_notifications(second_post.id)
 
@@ -78,7 +83,7 @@ RSpec.describe DiscourseChat::Manager do
     end
 
     it "should not notify about private messages" do
-      manager.create_rule('dummy', 'chan1', 'watch', nil, nil)
+      create_rule('dummy', 'chan1', 'watch', nil, nil)
       private_post = Fabricate(:private_message_post)
 
       manager.trigger_notifications(private_post.id)
@@ -87,7 +92,7 @@ RSpec.describe DiscourseChat::Manager do
     end
 
     it "should not notify about private messages" do
-      manager.create_rule('dummy', 'chan1', 'watch', nil, nil)
+      create_rule('dummy', 'chan1', 'watch', nil, nil)
       private_post = Fabricate(:private_message_post)
 
       manager.trigger_notifications(private_post.id)
@@ -96,7 +101,7 @@ RSpec.describe DiscourseChat::Manager do
     end
 
     it "should not notify about posts the chat_user cannot see" do
-      manager.create_rule('dummy', 'chan1', 'watch', nil, nil)
+      create_rule('dummy', 'chan1', 'watch', nil, nil)
 
       # Create a group & user
       group = Fabricate(:group, name: "friends")
@@ -139,7 +144,7 @@ RSpec.describe DiscourseChat::Manager do
       end
 
       it 'should still work for rules without any tags specified' do
-        manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
+        create_rule('dummy', 'chan1', 'watch', category.id, nil)
 
         manager.trigger_notifications(first_post.id)
         manager.trigger_notifications(tagged_first_post.id)
@@ -148,7 +153,7 @@ RSpec.describe DiscourseChat::Manager do
       end
 
       it 'should only match tagged topics when rule has tags' do
-        manager.create_rule('dummy', 'chan1', 'watch', category.id, [tag.name])
+        create_rule('dummy', 'chan1', 'watch', category.id, [tag.name])
 
         manager.trigger_notifications(first_post.id)
         manager.trigger_notifications(tagged_first_post.id)
@@ -159,36 +164,4 @@ RSpec.describe DiscourseChat::Manager do
     end
   end
 
-
-  describe '.create_rule' do
-    it 'should add new rule correctly' do
-      expect do
-        manager.create_rule('dummy', 'chan1', 'watch', category.id, nil)
-      end.to change { manager.get_rules_for_category(category.id).length }.by(1)
-
-      expect do
-        manager.create_rule('dummy', 'chan2', 'follow', category.id, nil)
-      end.to change { manager.get_rules_for_category(category.id).length }.by(1)
-    end
-
-    it 'should accept tags correctly' do
-      tag = Fabricate(:tag)
-      expect do
-        manager.create_rule('dummy', 'chan1', 'watch', category.id, [tag.name, 'faketag'])
-      end.to change { manager.get_rules_for_category(category.id).length }.by(1)
-
-      expect(manager.get_rules_for_category(category.id).first[:tags]).to contain_exactly(tag.name)
-
-    end
-
-    it 'should error on invalid filter strings' do
-      expect do
-        manager.create_rule('dummy', 'chan1', 'invalid_filter', category.id, nil)
-      end.to raise_error(RuntimeError, "Invalid filter")
-    end
-
-
-  end
-
-
 end