FEATURE: Add a fallback to auto-assign

Currently, when the auto-assign logic can’t find a user to assign, it
will fail saying there was no one to assign. The current logic is this
one:
- Don’t pick anyone who’s been picked in the last 180 days
- If no one has been found, then try the same thing but only for the
  last 14 days.
While this is working relatively well for large enough groups, it
doesn’t work at all with very small groups (like 1 or 2 people) and it
creates unnecessary noise.

This patch addresses this issue by adding a fallback to the current
logic. Now, if the two first rules fail, instead of saying that no one
was assigned, we assign the least recently assigned person. This way,
the logic will continue to work with large groups but will also work
nicely with small groups.

lib/random_assign_utils.rb

@@ -75,10 +75,7 @@ def self.automation_script!(context, fields, automation)
       users_ids = group_users_ids - recently_assigned_users_ids
     end
 
-    if users_ids.blank?
-      RandomAssignUtils.no_one!(topic_id, group.name)
-      return
-    end
+    users_ids << last_assignees_ids.last if users_ids.blank?
 
     if fields.dig("in_working_hours", "value")
       assign_to_user_id =
@@ -119,9 +116,14 @@ def self.recently_assigned_users_ids(topic_id, from)
         .where(topic_id: topic_id, action_code: %w[assigned reassigned assigned_to_post])
         .where("posts.created_at > ?", from)
         .order(created_at: :desc)
-    usernames =
-      Post.custom_fields_for_ids(posts, [:action_code_who]).map { |_, v| v["action_code_who"] }.uniq
-    User.where(username: usernames).limit(100).pluck(:id)
+    usernames = posts.map { _1.custom_fields[:action_code_who] }.uniq
+    User
+      .where(username: usernames)
+      .joins(
+        "JOIN unnest('{#{usernames.join(",")}}'::text[]) WITH ORDINALITY t(username, ord) USING(username)",
+      )
+      .limit(100)
+      .pluck(:id)
   end
 
   def self.user_tzinfo(user_id)

spec/lib/random_assign_utils_spec.rb

@@ -3,15 +3,15 @@
 require "rails_helper"
 require_relative "../support/assign_allowed_group"
 
-describe RandomAssignUtils do
-  before do
-    SiteSetting.assign_enabled = true
-
-    @orig_logger = Rails.logger
-    Rails.logger = @fake_logger = FakeLogger.new
+RSpec.describe RandomAssignUtils do
+  around do |example|
+    orig_logger = Rails.logger
+    Rails.logger = FakeLogger.new
+    example.run
+    Rails.logger = orig_logger
   end
 
-  after { Rails.logger = @orig_logger }
+  before { SiteSetting.assign_enabled = true }
 
   FakeAutomation = Struct.new(:id)
 
@@ -25,6 +25,7 @@
       fab!(:user_1) { Fabricate(:user) }
 
       before do
+        SiteSetting.assign_allowed_on_groups = [group_1.id.to_s].join("|")
         group_1.add(user_1)
         UserCustomField.create!(name: "on_holiday", value: "t", user_id: user_1.id)
       end
@@ -49,16 +50,21 @@
     end
 
     context "when all users of group have been assigned recently" do
-      fab!(:topic_1) { Fabricate(:topic) }
+      fab!(:post_1) { Fabricate(:post) }
+      fab!(:topic_1) { post_1.topic }
       fab!(:group_1) { Fabricate(:group) }
       fab!(:user_1) { Fabricate(:user) }
+      fab!(:user_2) { Fabricate(:user) }
 
       before do
-        Assigner.new(topic_1, Discourse.system_user).assign(user_1)
+        SiteSetting.assign_allowed_on_groups = [group_1.id.to_s].join("|")
         group_1.add(user_1)
+        group_1.add(user_2)
+        freeze_time(10.days.ago) { Assigner.new(topic_1, Discourse.system_user).assign(user_1) }
+        Assigner.new(topic_1, Discourse.system_user).assign(user_2)
       end
 
-      it "creates post on the topic" do
+      it "assigns the least recently assigned user to the topic" do
         described_class.automation_script!(
           {},
           {
@@ -71,9 +77,7 @@
           },
           automation,
         )
-        expect(topic_1.posts.first.raw).to match(
-          I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name),
-        )
+        expect(topic_1.assignment.assigned_to).to eq(user_2)
       end
     end
 
@@ -331,36 +335,40 @@
   end
 
   describe ".recently_assigned_users_ids" do
+    subject(:assignees) { described_class.recently_assigned_users_ids(post.topic_id, 2.months.ago) }
+
     context "when no one has been assigned" do
       it "returns an empty array" do
-        assignees_ids = described_class.recently_assigned_users_ids(post.topic_id, 2.months.ago)
-        expect(assignees_ids).to eq([])
+        expect(assignees).to be_empty
       end
     end
 
     context "when users have been assigned" do
       let(:admin) { Fabricate(:admin) }
       let(:assign_allowed_group) { Group.find_by(name: "staff") }
-      let(:user_1) { Fabricate(:user, groups: [assign_allowed_group]) }
-      let(:user_2) { Fabricate(:user, groups: [assign_allowed_group]) }
-      let(:user_3) { Fabricate(:user, groups: [assign_allowed_group]) }
-      let(:user_4) { Fabricate(:user, groups: [assign_allowed_group]) }
+      let!(:user_1) { Fabricate(:user, groups: [assign_allowed_group]) }
+      let!(:user_2) { Fabricate(:user, groups: [assign_allowed_group]) }
+      let!(:user_3) { Fabricate(:user, groups: [assign_allowed_group]) }
+      let!(:user_4) { Fabricate(:user, groups: [assign_allowed_group]) }
       let(:post_2) { Fabricate(:post, topic: post.topic) }
 
-      it "returns the recently assigned user ids" do
-        freeze_time 1.months.ago do
-          Assigner.new(post.topic, admin).assign(user_1)
+      before do
+        freeze_time 15.days.ago do
           Assigner.new(post.topic, admin).assign(user_2)
+        end
+        freeze_time 30.days.ago do
+          Assigner.new(post.topic, admin).assign(user_1)
+        end
+        freeze_time 45.days.ago do
           Assigner.new(post_2, admin).assign(user_4)
         end
-
         freeze_time 3.months.ago do
           Assigner.new(post.topic, admin).assign(user_3)
         end
+      end
 
-        assignees_ids = described_class.recently_assigned_users_ids(post.topic_id, 2.months.ago)
-
-        expect(assignees_ids).to contain_exactly(user_1.id, user_2.id, user_4.id)
+      it "returns the recently assigned user ids" do
+        expect(assignees).to eq([user_2, user_1, user_4].map(&:id))
       end
     end
   end