Staff is always allowed to assign (#36)

config/settings.yml

@@ -30,7 +30,7 @@ plugins:
   assign_allowed_on_groups:
     client: true
     type: group_list
+    default: ''
     list_type: compact
-    default: 'staff'
     allow_any: false
     refresh: true

jobs/scheduled/enqueue_reminders.rb

@@ -17,7 +17,16 @@ module Jobs
 
     def allowed_group_ids
       allowed_groups = SiteSetting.assign_allowed_on_groups.split('|')
-      Group.where(name: allowed_groups).pluck(:id).join(',')
+      return [] if allowed_groups.empty?
+
+      Group.where(name: allowed_groups).pluck(:id)
+    end
+
+    def membership_condition
+      group_ids = allowed_group_ids.join(',')
+      condition = 'users.admin OR users.moderator'
+      condition += " OR group_users.group_id IN (#{group_ids})" if group_ids.present?
+      condition
     end
 
     def user_ids
@@ -36,10 +45,11 @@ module Jobs
         ON topic_custom_fields.value::INT = user_frequency.user_id
         AND user_frequency.name = '#{PendingAssignsReminder::REMINDERS_FREQUENCY}'
 
-        INNER JOIN group_users ON topic_custom_fields.value::INT = group_users.user_id
+        INNER JOIN users ON topic_custom_fields.value::INT = users.id
+        LEFT OUTER JOIN group_users ON topic_custom_fields.value::INT = group_users.user_id
         INNER JOIN topics ON topics.id = topic_custom_fields.topic_id AND (topics.deleted_at IS NULL)
 
-        WHERE group_users.group_id IN (#{allowed_group_ids})
+        WHERE (#{membership_condition})
         AND #{frequency} > 0
         AND (
           last_reminder.value IS NULL OR

lib/topic_assigner.rb

@@ -18,8 +18,7 @@ class ::TopicAssigner
       FROM posts p
       JOIN topics t ON t.id = p.topic_id
       LEFT JOIN topic_custom_fields tc ON tc.name = '#{ASSIGNED_TO_ID}' AND tc.topic_id = p.topic_id
-      WHERE p.user_id IN (SELECT id FROM users WHERE moderator OR admin) AND
-        ( #{staff_mention} ) AND tc.value IS NULL AND NOT t.closed AND t.deleted_at IS NULL
+      WHERE ( #{staff_mention} ) AND tc.value IS NULL AND NOT t.closed AND t.deleted_at IS NULL
       GROUP BY p.topic_id
     SQL
 
@@ -93,10 +92,8 @@ class ::TopicAssigner
   def self.mentioned_staff(post)
     mentions = post.raw_mentions
     if mentions.present?
-      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
@@ -116,7 +113,6 @@ class ::TopicAssigner
 
     assigned_total = TopicCustomField
       .joins(:topic)
-      .where(topics: { deleted_at: nil })
       .where(name: ASSIGNED_TO_ID)
       .where(value: user.id)
       .count

plugin.rb

@@ -49,6 +49,8 @@ after_initialize do
   reviewable_api_enabled = defined?(Reviewable)
 
   add_to_class(:user, :can_assign?) do
+    return true if staff?
+
     @can_assign ||=
       begin
         allowed_groups = SiteSetting.assign_allowed_on_groups.split('|').compact
@@ -62,11 +64,18 @@ after_initialize do
 
   add_class_method(:user, :assign_allowed) do
     allowed_groups = SiteSetting.assign_allowed_on_groups.split('|')
-    where('users.id IN (
-      SELECT user_id FROM group_users
-      INNER JOIN groups ON group_users.group_id = groups.id
-      WHERE groups.name IN (?)
-    )', allowed_groups)
+
+    if allowed_groups.present?
+      real.where('
+        users.admin OR users.moderator OR
+        users.id IN (
+          SELECT user_id FROM group_users
+          INNER JOIN groups ON group_users.group_id = groups.id
+          WHERE groups.name IN (?)
+        )', allowed_groups)
+    else
+      real.where('users.admin OR users.moderator')
+    end
   end
 
   add_model_callback(Group, :before_update) do

spec/jobs/scheduled/enqueue_reminders_spec.rb

@@ -3,15 +3,24 @@
 require 'rails_helper'
 
 RSpec.describe Jobs::EnqueueReminders do
-  let(:assign_allowed_group) { Group.find_by(name: 'staff') }
+  let(:assign_allowed_group) { Fabricate(:group) }
   let(:user) { Fabricate(:user, groups: [assign_allowed_group]) }
 
   before do
+    SiteSetting.assign_allowed_on_groups = assign_allowed_group.name
     SiteSetting.remind_assigns_frequency = RemindAssignsFrequencySiteSettings::MONTHLY_MINUTES
     SiteSetting.assign_enabled = true
   end
 
   describe '#execute' do
+    it 'enqueues a reminder when the user is an admin' do
+      SiteSetting.assign_allowed_on_groups = ''
+      admin = Fabricate(:admin)
+      assign_multiple_tasks_to(admin)
+
+      assert_reminders_enqueued(1)
+    end
+
     it 'does not enqueue reminders when there are no assigned tasks' do
       assert_reminders_enqueued(0)
     end

spec/models/user_spec.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe User do
+  let(:group) { Fabricate(:group) }
+
+  before do
+    SiteSetting.assign_enabled = true
+    SiteSetting.assign_allowed_on_groups = group.name
+  end
+
+  describe '.assign_allowed' do
+    it 'retrieves the user when is a member of an allowed group' do
+      user = Fabricate(:user)
+      group.add(user)
+
+      expect(User.assign_allowed).to include(user)
+    end
+
+    it "doesn't retrieve the user when is not a member of an allowed group" do
+      non_assign_allowed_user = Fabricate(:user)
+
+      expect(User.assign_allowed).not_to include(non_assign_allowed_user)
+    end
+
+    it 'retrieves the user if is an admin' do
+      admin = Fabricate(:admin)
+
+      expect(User.assign_allowed).to include(admin)
+    end
+
+    it 'retrieves the user if is an moderator' do
+      moderator = Fabricate(:moderator)
+
+      expect(User.assign_allowed).to include(moderator)
+    end
+  end
+
+  describe '#can_assign?' do
+    it 'allows member of allowed groups to assign' do
+      user = Fabricate.build(:user)
+      group.add(user)
+
+      expect(user.can_assign?).to eq true
+    end
+
+    it "doesn't allow non allowed users to assign" do
+      user = Fabricate.build(:user)
+
+      expect(user.can_assign?).to eq false
+    end
+
+    it 'allows staff members to assign' do
+      admin = Fabricate.build(:admin)
+
+      expect(admin.can_assign?).to eq true
+    end
+  end
+end