DEV: use group ids to allow assign on groups (#38)

* DEV: use group ids to allow assign on groups

* FIX: Dinamically sets the default value to support older versions

config/settings.yml

@@ -31,6 +31,6 @@ plugins:
     client: true
     type: group_list
     list_type: compact
-    default: 'staff'
+    default: ''
     allow_any: false
     refresh: true

jobs/scheduled/enqueue_reminders.rb

@@ -16,8 +16,7 @@ module Jobs
     end
 
     def allowed_group_ids
-      allowed_groups = SiteSetting.assign_allowed_on_groups.split('|')
+      Group.assign_allowed_groups.pluck(:id).join(',')
-      Group.where(name: allowed_groups).pluck(:id).join(',')
     end
 
     def user_ids

lib/topic_assigner.rb

@@ -96,7 +96,7 @@ class ::TopicAssigner
       allowed_groups = SiteSetting.assign_allowed_on_groups.split('|')
 
       User.human_users
-        .joins(:groups).where(groups: { name: allowed_groups })
+        .assign_allowed
         .where('username_lower IN (?)', mentions.map(&:downcase))
         .first
     end

plugin.rb

@@ -48,11 +48,29 @@ after_initialize do
 =end
   reviewable_api_enabled = defined?(Reviewable)
 
+  # TODO: Remove this once 2.4 becomes the new stable.
+  current_version = ActiveRecord::Migrator.current_version
+  min_version = 201_907_081_533_31
+  above_min_version = current_version >= min_version
+
+  # Dinamically sets the default value, supports older versions.
+  default_allowed_group_value = above_min_version ? Group::AUTO_GROUPS[:staff] : 'staff'
+  allowed_group_values = SiteSetting.assign_allowed_on_groups.split('|')
+  allowed_group_values << default_allowed_group_value if allowed_group_values.empty?
+  SiteSetting.assign_allowed_on_groups = allowed_group_values.join('|')
+
+  attribute = above_min_version ? 'id' : 'name'
+
+  add_class_method(:group, :assign_allowed_groups) do
+    allowed_groups = SiteSetting.assign_allowed_on_groups.split('|')
+    where("groups.#{attribute} IN (?)", allowed_groups)
+  end
+
   add_to_class(:user, :can_assign?) do
     @can_assign ||=
       begin
         allowed_groups = SiteSetting.assign_allowed_on_groups.split('|').compact
-        allowed_groups.present? && groups.where('name in (?)', allowed_groups).exists? ?
+        allowed_groups.present? && groups.where("groups.#{attribute} in (?)", allowed_groups).exists? ?
           :true : :false
       end
     @can_assign == :true
@@ -62,21 +80,22 @@ after_initialize do
 
   add_class_method(:user, :assign_allowed) do
     allowed_groups = SiteSetting.assign_allowed_on_groups.split('|')
-    where('users.id IN (
+    where("users.id IN (
       SELECT user_id FROM group_users
       INNER JOIN groups ON group_users.group_id = groups.id
-      WHERE groups.name IN (?)
+      WHERE groups.#{attribute} IN (?)
-    )', allowed_groups)
+    )", allowed_groups)
   end
 
   add_model_callback(Group, :before_update) do
-    if name_changed?
+    if !above_min_version && name_changed?
       SiteSetting.assign_allowed_on_groups = SiteSetting.assign_allowed_on_groups.gsub(name_was, name)
     end
   end
 
   add_model_callback(Group, :before_destroy) do
-    new_setting = SiteSetting.assign_allowed_on_groups.gsub(/#{name}[|]?/, '')
+    to_remove = above_min_version ? id : name
+    new_setting = SiteSetting.assign_allowed_on_groups.gsub(/#{to_remove}[|]?/, '')
     new_setting = new_setting.chomp('|') if new_setting.ends_with?('|')
     SiteSetting.assign_allowed_on_groups = new_setting
   end

spec/models/group_spec.rb

@@ -10,19 +10,17 @@ RSpec.describe Group do
       SiteSetting.assign_enabled = true
     end
 
-    it 'updates the site setting when the group name changes' do
+    let(:above_min_version) do
-      SiteSetting.assign_allowed_on_groups = "#{group.name}|staff|moderators"
+      current_version = ActiveRecord::Migrator.current_version
-      different_name = 'different_name'
+      min_version = 201_907_081_533_31
-
+      current_version >= min_version
-      group.update!(name: different_name)
-
-      expect(SiteSetting.assign_allowed_on_groups).to eq "#{different_name}|staff|moderators"
     end
 
-    let(:removed_group_setting) { 'staff|moderators' }
+    let(:removed_group_setting) { above_min_version ? '3|4' : 'staff|moderators' }
+    let(:group_attribute) { above_min_version ? group.id : group.name }
 
     it 'removes the group from the setting when the group gets destroyed' do
-      SiteSetting.assign_allowed_on_groups = "#{group.name}|staff|moderators"
+      SiteSetting.assign_allowed_on_groups = "#{group_attribute}|#{removed_group_setting}"
 
       group.destroy!
 
@@ -30,7 +28,7 @@ RSpec.describe Group do
     end
 
     it 'removes the group from the setting when this is the last one on the list' do
-      SiteSetting.assign_allowed_on_groups = "staff|moderators|#{group.name}"
+      SiteSetting.assign_allowed_on_groups = "#{removed_group_setting}|#{group_attribute}"
 
       group.destroy!
 
@@ -38,7 +36,8 @@ RSpec.describe Group do
     end
 
     it 'removes the group from the list when it is on the middle of the list' do
-      SiteSetting.assign_allowed_on_groups = "staff|#{group.name}|moderators"
+      allowed_groups = above_min_version ? "3|#{group_attribute}|4" : "staff|#{group_attribute}|moderators"
+      SiteSetting.assign_allowed_on_groups = allowed_groups
 
       group.destroy!
 

spec/requests/assign_controller_spec.rb

@@ -11,6 +11,12 @@ RSpec.describe DiscourseAssign::AssignController do
   let(:post) { Fabricate(:post) }
   let(:user2) { Fabricate(:active_user) }
 
+  let(:above_min_version) do
+    current_version = ActiveRecord::Migrator.current_version
+    min_version = 201_907_081_533_31
+    current_version >= min_version
+  end
+
   describe 'only allow users from allowed groups' do
     before { sign_in(user2) }
 
@@ -31,7 +37,13 @@ RSpec.describe DiscourseAssign::AssignController do
         allowed_group = Group.find_by(name: 'everyone')
         user2.groups << allowed_group
         user2.groups << default_allowed_group
-        SiteSetting.assign_allowed_on_groups = 'staff|everyone'
+        defaults = if above_min_version
+          "#{default_allowed_group.id}|#{allowed_group.id}"
+        else
+          "#{default_allowed_group.name}|#{allowed_group.name}"
+        end
+
+        SiteSetting.assign_allowed_on_groups = defaults
         TopicAssigner.new(post.topic, user).assign(user2)
 
         get '/assign/suggestions.json'
@@ -43,7 +55,7 @@ RSpec.describe DiscourseAssign::AssignController do
       it 'does not include users from disallowed groups' do
         allowed_group = Group.find_by(name: 'everyone')
         user2.groups << allowed_group
-        SiteSetting.assign_allowed_on_groups = 'staff'
+        SiteSetting.assign_allowed_on_groups = above_min_version ? default_allowed_group.id.to_s : default_allowed_group.name
         TopicAssigner.new(post.topic, user).assign(user2)
 
         get '/assign/suggestions.json'