From fbd1fa3794d8d1c8c202975431cc77f96cbef9ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= <loic@discourse.org>
Date: Wed, 15 Nov 2023 13:58:05 +0100
Subject: [PATCH] FEATURE: Add a fallback to auto-assign
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 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           | 294 +++++++++--------
 spec/lib/random_assign_utils_spec.rb | 454 +++++++++++++--------------
 2 files changed, 386 insertions(+), 362 deletions(-)

diff --git a/lib/random_assign_utils.rb b/lib/random_assign_utils.rb
index fc0edaa..badfacd 100644
--- a/lib/random_assign_utils.rb
+++ b/lib/random_assign_utils.rb
@@ -1,130 +1,190 @@
 # frozen_string_literal: true
 
 class RandomAssignUtils
-  def self.raise_error(automation, message)
-    raise("[discourse-automation id=#{automation.id}] #{message}.")
+  attr_reader :context, :fields, :automation, :topic, :group
+
+  def self.automation_script!(...)
+    new(...).automation_script!
   end
 
-  def self.log_info(automation, message)
-    Rails.logger.info("[discourse-automation id=#{automation.id}] #{message}.")
-  end
-
-  def self.automation_script!(context, fields, automation)
-    raise_error(automation, "discourse-assign is not enabled") unless SiteSetting.assign_enabled?
+  def initialize(context, fields, automation)
+    @context = context
+    @fields = fields
+    @automation = automation
 
+    raise_error("discourse-assign is not enabled") unless SiteSetting.assign_enabled?
     unless topic_id = fields.dig("assigned_topic", "value")
-      raise_error(automation, "`assigned_topic` not provided")
+      raise_error("`assigned_topic` not provided")
     end
-
-    unless topic = Topic.find_by(id: topic_id)
-      raise_error(automation, "Topic(#{topic_id}) not found")
-    end
-
-    min_hours = fields.dig("minimum_time_between_assignments", "value").presence
-    if min_hours &&
-         TopicCustomField
-           .where(name: "assigned_to_id", topic_id: topic_id)
-           .where("created_at < ?", min_hours.to_i.hours.ago)
-           .exists?
-      log_info(automation, "Topic(#{topic_id}) has already been assigned recently")
-      return
+    unless @topic = Topic.find_by(id: topic_id)
+      raise_error("Topic(#{topic_id}) not found")
     end
 
     unless group_id = fields.dig("assignees_group", "value")
-      raise_error(automation, "`assignees_group` not provided")
+      raise_error("`assignees_group` not provided")
     end
-
-    unless group = Group.find_by(id: group_id)
-      raise_error(automation, "Group(#{group_id}) not found")
+    unless @group = Group.find_by(id: group_id)
+      raise_error("Group(#{group_id}) not found")
     end
-
-    assignable_user_ids = User.assign_allowed.pluck(:id)
-    users_on_holiday =
-      Set.new(
-        User.where(
-          id: UserCustomField.where(name: "on_holiday", value: "t").select(:user_id),
-        ).pluck(:id),
-      )
-
-    group_users = group.group_users.joins(:user)
-    if skip_new_users_for_days = fields.dig("skip_new_users_for_days", "value").presence
-      group_users = group_users.where("users.created_at < ?", skip_new_users_for_days.to_i.days.ago)
-    end
-
-    group_users_ids =
-      group_users
-        .pluck("users.id")
-        .filter { |user_id| assignable_user_ids.include?(user_id) }
-        .reject { |user_id| users_on_holiday.include?(user_id) }
-
-    if group_users_ids.empty?
-      RandomAssignUtils.no_one!(topic_id, group.name)
-      return
-    end
-
-    max_recently_assigned_days =
-      (fields.dig("max_recently_assigned_days", "value").presence || 180).to_i.days.ago
-    last_assignees_ids =
-      RandomAssignUtils.recently_assigned_users_ids(topic_id, max_recently_assigned_days)
-    users_ids = group_users_ids - last_assignees_ids
-    if users_ids.blank?
-      min_recently_assigned_days =
-        (fields.dig("min_recently_assigned_days", "value").presence || 14).to_i.days.ago
-      recently_assigned_users_ids =
-        RandomAssignUtils.recently_assigned_users_ids(topic_id, min_recently_assigned_days)
-      users_ids = group_users_ids - recently_assigned_users_ids
-    end
-
-    if users_ids.blank?
-      RandomAssignUtils.no_one!(topic_id, group.name)
-      return
-    end
-
-    if fields.dig("in_working_hours", "value")
-      assign_to_user_id =
-        users_ids.shuffle.find { |user_id| RandomAssignUtils.in_working_hours?(user_id) }
-    end
-
-    assign_to_user_id ||= users_ids.sample
-    if assign_to_user_id.blank?
-      RandomAssignUtils.no_one!(topic_id, group.name)
-      return
-    end
-
-    assign_to = User.find(assign_to_user_id)
-    result = nil
-    if raw = fields.dig("post_template", "value").presence
-      post =
-        PostCreator.new(
-          Discourse.system_user,
-          raw: raw,
-          skip_validations: true,
-          topic_id: topic.id,
-        ).create!
-
-      result = Assigner.new(post, Discourse.system_user).assign(assign_to)
-
-      PostDestroyer.new(Discourse.system_user, post).destroy if !result[:success]
-    else
-      result = Assigner.new(topic, Discourse.system_user).assign(assign_to)
-    end
-
-    RandomAssignUtils.no_one!(topic_id, group.name) if !result[:success]
   end
 
-  def self.recently_assigned_users_ids(topic_id, from)
-    posts =
-      Post
-        .joins(:user)
-        .where(topic_id: topic_id, action_code: %w[assigned reassigned assigned_to_post])
-        .where("posts.created_at > ?", from)
-        .order(created_at: :desc)
+  def automation_script!
+    return log_info("Topic(#{topic.id}) has already been assigned recently") if assigned_recently?
+    return no_one! unless assigned_user
+    assign_user!
+  end
+
+  def recently_assigned_users_ids(from)
     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)
+      PostCustomField
+        .joins(:post)
+        .where(
+          name: "action_code_who",
+          posts: {
+            topic: topic,
+            action_code: %w[assigned reassigned assigned_to_post],
+          },
+        )
+        .where("posts.created_at > ?", from)
+        .order("posts.created_at DESC")
+        .pluck(:value)
+        .uniq
+    User
+      .where(username: usernames)
+      .joins(
+        "JOIN unnest('{#{usernames.join(",")}}'::text[]) WITH ORDINALITY t(username, ord) USING(username)",
+      )
+      .limit(100)
+      .order("ord")
+      .pluck(:id)
   end
 
-  def self.user_tzinfo(user_id)
+  private
+
+  def assigned_user
+    @assigned_user ||=
+      begin
+        group_users_ids = group_users.pluck(:id)
+        return if group_users_ids.empty?
+
+        last_assignees_ids = recently_assigned_users_ids(max_recently_assigned_days)
+        users_ids = group_users_ids - last_assignees_ids
+        if users_ids.blank?
+          recently_assigned_users_ids = recently_assigned_users_ids(min_recently_assigned_days)
+          users_ids = group_users_ids - recently_assigned_users_ids
+        end
+        users_ids << last_assignees_ids.intersection(group_users_ids).last if users_ids.blank?
+        if fields.dig("in_working_hours", "value")
+          assign_to_user_id = users_ids.shuffle.detect { |user_id| in_working_hours?(user_id) }
+        end
+        assign_to_user_id ||= users_ids.sample
+
+        User.find(assign_to_user_id)
+      end
+  end
+
+  def assign_user!
+    return create_post_template if post_template
+    Assigner
+      .new(topic, Discourse.system_user)
+      .assign(assigned_user)
+      .then do |result|
+        next if result[:success]
+        no_one!
+      end
+  end
+
+  def create_post_template
+    post =
+      PostCreator.new(
+        Discourse.system_user,
+        raw: post_template,
+        skip_validations: true,
+        topic_id: topic.id,
+      ).create!
+    Assigner
+      .new(post, Discourse.system_user)
+      .assign(assigned_user)
+      .then do |result|
+        next if result[:success]
+        PostDestroyer.new(Discourse.system_user, post).destroy
+        no_one!
+      end
+  end
+
+  def group_users
+    users =
+      group
+        .users
+        .where(id: User.assign_allowed.select(:id))
+        .where.not(
+          id:
+            User
+              .joins(:_custom_fields)
+              .where(user_custom_fields: { name: "on_holiday", value: "t" })
+              .select(:id),
+        )
+    return users unless skip_new_users_for_days
+    users.where("users.created_at < ?", skip_new_users_for_days)
+  end
+
+  def raise_error(message)
+    raise("[discourse-automation id=#{automation.id}] #{message}.")
+  end
+
+  def log_info(message)
+    Rails.logger.info("[discourse-automation id=#{automation.id}] #{message}.")
+  end
+
+  def no_one!
+    PostCreator.create!(
+      Discourse.system_user,
+      topic_id: topic.id,
+      raw: I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group.name),
+      validate: false,
+    )
+  end
+
+  def assigned_recently?
+    return unless min_hours
+    TopicCustomField
+      .where(name: "assigned_to_id", topic: topic)
+      .where("created_at < ?", min_hours)
+      .exists?
+  end
+
+  def skip_new_users_for_days
+    days = fields.dig("skip_new_users_for_days", "value").presence
+    return unless days
+    days.to_i.days.ago
+  end
+
+  def max_recently_assigned_days
+    @max_days ||= (fields.dig("max_recently_assigned_days", "value").presence || 180).to_i.days.ago
+  end
+
+  def min_recently_assigned_days
+    @min_days ||= (fields.dig("min_recently_assigned_days", "value").presence || 14).to_i.days.ago
+  end
+
+  def post_template
+    @post_template ||= fields.dig("post_template", "value").presence
+  end
+
+  def min_hours
+    hours = fields.dig("minimum_time_between_assignments", "value").presence
+    return unless hours
+    hours.to_i.hours.ago
+  end
+
+  def in_working_hours?(user_id)
+    tzinfo = user_tzinfo(user_id)
+    tztime = tzinfo.now
+
+    !tztime.saturday? && !tztime.sunday? && tztime.hour > 7 && tztime.hour < 11
+  end
+
+  def user_tzinfo(user_id)
     timezone = UserOption.where(user_id: user_id).pluck(:timezone).first || "UTC"
 
     tzinfo = nil
@@ -140,20 +200,4 @@ class RandomAssignUtils
 
     tzinfo
   end
-
-  def self.no_one!(topic_id, group)
-    PostCreator.create!(
-      Discourse.system_user,
-      topic_id: topic_id,
-      raw: I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group),
-      validate: false,
-    )
-  end
-
-  def self.in_working_hours?(user_id)
-    tzinfo = RandomAssignUtils.user_tzinfo(user_id)
-    tztime = tzinfo.now
-
-    !tztime.saturday? && !tztime.sunday? && tztime.hour > 7 && tztime.hour < 11
-  end
 end
diff --git a/spec/lib/random_assign_utils_spec.rb b/spec/lib/random_assign_utils_spec.rb
index ca96627..0f54802 100644
--- a/spec/lib/random_assign_utils_spec.rb
+++ b/spec/lib/random_assign_utils_spec.rb
@@ -3,175 +3,124 @@
 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
-  end
-
-  after { Rails.logger = @orig_logger }
-
+RSpec.describe RandomAssignUtils do
   FakeAutomation = Struct.new(:id)
 
-  let(:post) { Fabricate(:post) }
   let!(:automation) { FakeAutomation.new(1) }
 
-  describe ".automation_script!" do
-    context "when all users of group are on holidays" do
-      fab!(:topic_1) { Fabricate(:topic) }
-      fab!(:group_1) { Fabricate(:group) }
-      fab!(:user_1) { Fabricate(:user) }
+  around do |example|
+    orig_logger = Rails.logger
+    Rails.logger = FakeLogger.new
+    example.run
+    Rails.logger = orig_logger
+  end
 
-      before do
-        group_1.add(user_1)
-        UserCustomField.create!(name: "on_holiday", value: "t", user_id: user_1.id)
+  before { SiteSetting.assign_enabled = true }
+
+  describe ".automation_script!" do
+    subject(:auto_assign) { described_class.automation_script!(ctx, fields, automation) }
+
+    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) }
+    fab!(:user_3) { Fabricate(:user) }
+
+    let(:ctx) { {} }
+    let(:fields) { {} }
+
+    before do
+      SiteSetting.assign_allowed_on_groups = group_1.id.to_s
+      group_1.add(user_1)
+    end
+
+    context "when all users of group are on holidays" do
+      let(:fields) do
+        {
+          "assignees_group" => {
+            "value" => group_1.id,
+          },
+          "assigned_topic" => {
+            "value" => topic_1.id,
+          },
+        }
       end
 
+      before { UserCustomField.create!(name: "on_holiday", value: "t", user_id: user_1.id) }
+
       it "creates post on the topic" do
-        described_class.automation_script!(
-          {},
-          {
-            "assignees_group" => {
-              "value" => group_1.id,
-            },
-            "assigned_topic" => {
-              "value" => topic_1.id,
-            },
-          },
-          automation,
-        )
-        expect(topic_1.posts.first.raw).to match(
+        auto_assign
+        expect(topic_1.posts.last.raw).to match(
           I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name),
         )
       end
     end
 
     context "when all users of group have been assigned recently" do
-      fab!(:topic_1) { Fabricate(:topic) }
-      fab!(:group_1) { Fabricate(:group) }
-      fab!(:user_1) { Fabricate(:user) }
-
-      before do
-        Assigner.new(topic_1, Discourse.system_user).assign(user_1)
-        group_1.add(user_1)
+      let(:fields) do
+        {
+          "assignees_group" => {
+            "value" => group_1.id,
+          },
+          "assigned_topic" => {
+            "value" => topic_1.id,
+          },
+        }
       end
 
-      it "creates post on the topic" do
-        described_class.automation_script!(
-          {},
-          {
-            "assignees_group" => {
-              "value" => group_1.id,
-            },
-            "assigned_topic" => {
-              "value" => topic_1.id,
-            },
-          },
-          automation,
-        )
-        expect(topic_1.posts.first.raw).to match(
-          I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name),
-        )
+      before do
+        group_1.add(user_2)
+        group_1.add(user_3)
+        freeze_time(10.days.ago) { Assigner.new(topic_1, Discourse.system_user).assign(user_1) }
+        freeze_time(5.days.ago) { Assigner.new(topic_1, Discourse.system_user).assign(user_3) }
+        Assigner.new(topic_1, Discourse.system_user).assign(user_2)
+      end
+
+      it "assigns the least recently assigned user to the topic" do
+        expect { auto_assign }.to change { topic_1.reload.assignment.assigned_to }.to(user_1)
+      end
+
+      context "when a user is on holiday" do
+        before do
+          user_1.custom_fields["on_holiday"] = true
+          user_1.save!
+        end
+
+        it "doesn't assign them" do
+          expect { auto_assign }.to change { topic_1.reload.assignment.assigned_to }.to(user_3)
+        end
       end
     end
 
     context "when no users can be assigned because none are members of assign_allowed_on_groups groups" do
-      fab!(:topic_1) { Fabricate(:topic) }
-      fab!(:group_1) { Fabricate(:group) }
-      fab!(:user_1) { Fabricate(:user) }
+      let(:fields) do
+        {
+          "assignees_group" => {
+            "value" => group_1.id,
+          },
+          "assigned_topic" => {
+            "value" => topic_1.id,
+          },
+        }
+      end
 
-      before { group_1.add(user_1) }
+      before { SiteSetting.assign_allowed_on_groups = "" }
 
       it "creates post on the topic" do
-        described_class.automation_script!(
-          {},
-          {
-            "assignees_group" => {
-              "value" => group_1.id,
-            },
-            "assigned_topic" => {
-              "value" => topic_1.id,
-            },
-          },
-          automation,
-        )
-        expect(topic_1.posts.first.raw).to match(
+        auto_assign
+        expect(topic_1.posts.last.raw).to match(
           I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name),
         )
       end
     end
 
     context "when user can be assigned" do
-      fab!(:group_1) { Fabricate(:group) }
-      fab!(:user_1) { Fabricate(:user) }
-      fab!(:topic_1) { Fabricate(:topic) }
-
-      before do
-        SiteSetting.assign_allowed_on_groups = [group_1.id.to_s].join("|")
-        group_1.add(user_1)
-      end
-
       context "when post_template is set" do
-        it "creates a post with the template and assign the user" do
-          described_class.automation_script!(
-            {},
-            {
-              "post_template" => {
-                "value" => "this is a post template",
-              },
-              "assignees_group" => {
-                "value" => group_1.id,
-              },
-              "assigned_topic" => {
-                "value" => topic_1.id,
-              },
-            },
-            automation,
-          )
-          expect(topic_1.posts.first.raw).to match("this is a post template")
-        end
-      end
-
-      context "when post_template is not set" do
-        fab!(:post_1) { Fabricate(:post, topic: topic_1) }
-
-        it "assigns the user to the topic" do
-          described_class.automation_script!(
-            {},
-            {
-              "assignees_group" => {
-                "value" => group_1.id,
-              },
-              "assigned_topic" => {
-                "value" => topic_1.id,
-              },
-            },
-            automation,
-          )
-          expect(topic_1.assignment.assigned_to_id).to eq(user_1.id)
-        end
-      end
-    end
-
-    context "when all users are in working hours" do
-      fab!(:topic_1) { Fabricate(:topic) }
-      fab!(:group_1) { Fabricate(:group) }
-      fab!(:user_1) { Fabricate(:user) }
-
-      before do
-        freeze_time("2022-10-01 02:00")
-        UserOption.find_by(user_id: user_1.id).update(timezone: "Europe/Paris")
-        group_1.add(user_1)
-      end
-
-      it "creates post on the topic" do
-        described_class.automation_script!(
-          {},
+        let(:fields) do
           {
-            "in_working_hours" => {
-              "value" => true,
+            "post_template" => {
+              "value" => "this is a post template",
             },
             "assignees_group" => {
               "value" => group_1.id,
@@ -179,73 +128,107 @@ describe RandomAssignUtils do
             "assigned_topic" => {
               "value" => topic_1.id,
             },
+          }
+        end
+
+        it "creates a post with the template and assign the user" do
+          auto_assign
+          expect(topic_1.posts.second.raw).to match("this is a post template")
+        end
+      end
+
+      context "when post_template is not set" do
+        let(:fields) do
+          {
+            "assignees_group" => {
+              "value" => group_1.id,
+            },
+            "assigned_topic" => {
+              "value" => topic_1.id,
+            },
+          }
+        end
+
+        it "assigns the user to the topic" do
+          auto_assign
+          expect(topic_1.assignment.assigned_to_id).to eq(user_1.id)
+        end
+      end
+    end
+
+    context "when all users are in working hours" do
+      let(:fields) do
+        {
+          "in_working_hours" => {
+            "value" => true,
           },
-          automation,
-        )
-        expect(topic_1.posts.first.raw).to match(
-          I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name),
-        )
+          "assignees_group" => {
+            "value" => group_1.id,
+          },
+          "assigned_topic" => {
+            "value" => topic_1.id,
+          },
+        }
+      end
+
+      before do
+        freeze_time("2022-10-01 02:00")
+        UserOption.find_by(user_id: user_1.id).update(timezone: "Europe/Paris")
+      end
+
+      it "assigns the user to the topic" do
+        auto_assign
+        expect(topic_1.assignment.assigned_to).to eq(user_1)
       end
     end
 
     context "when assignees_group is not provided" do
-      fab!(:topic_1) { Fabricate(:topic) }
+      let(:fields) { { "assigned_topic" => { "value" => topic_1.id } } }
 
       it "raises an error" do
-        expect {
-          described_class.automation_script!(
-            {},
-            { "assigned_topic" => { "value" => topic_1.id } },
-            automation,
-          )
-        }.to raise_error(/`assignees_group` not provided/)
+        expect { auto_assign }.to raise_error(/`assignees_group` not provided/)
       end
     end
 
     context "when assignees_group not found" do
-      fab!(:topic_1) { Fabricate(:topic) }
+      let(:fields) do
+        { "assigned_topic" => { "value" => topic_1.id }, "assignees_group" => { "value" => -1 } }
+      end
 
       it "raises an error" do
-        expect {
-          described_class.automation_script!(
-            {},
-            {
-              "assigned_topic" => {
-                "value" => topic_1.id,
-              },
-              "assignees_group" => {
-                "value" => -1,
-              },
-            },
-            automation,
-          )
-        }.to raise_error(/Group\(-1\) not found/)
+        expect { auto_assign }.to raise_error(/Group\(-1\) not found/)
       end
     end
 
     context "when assigned_topic not provided" do
       it "raises an error" do
-        expect { described_class.automation_script!({}, {}, automation) }.to raise_error(
-          /`assigned_topic` not provided/,
-        )
+        expect { auto_assign }.to raise_error(/`assigned_topic` not provided/)
       end
     end
 
     context "when assigned_topic is not found" do
+      let(:fields) { { "assigned_topic" => { "value" => 1 } } }
+
       it "raises an error" do
-        expect {
-          described_class.automation_script!(
-            {},
-            { "assigned_topic" => { "value" => 1 } },
-            automation,
-          )
-        }.to raise_error(/Topic\(1\) not found/)
+        expect { auto_assign }.to raise_error(/Topic\(1\) not found/)
       end
     end
 
     context "when minimum_time_between_assignments is set" do
       context "when the topic has been assigned recently" do
-        fab!(:topic_1) { Fabricate(:topic) }
+        let(:fields) do
+          {
+            "assignees_group" => {
+              "value" => group_1.id,
+            },
+            "assigned_topic" => {
+              "value" => topic_1.id,
+            },
+            "minimum_time_between_assignments" => {
+              "value" => 10,
+            },
+          }
+        end
 
         before do
           freeze_time
@@ -257,18 +240,7 @@ describe RandomAssignUtils do
         end
 
         it "logs a warning" do
-          described_class.automation_script!(
-            {},
-            {
-              "assigned_topic" => {
-                "value" => topic_1.id,
-              },
-              "minimum_time_between_assignments" => {
-                "value" => 10,
-              },
-            },
-            automation,
-          )
+          auto_assign
           expect(Rails.logger.infos.first).to match(
             /Topic\(#{topic_1.id}\) has already been assigned recently/,
           )
@@ -277,40 +249,29 @@ describe RandomAssignUtils do
     end
 
     context "when skip_new_users_for_days is set" do
-      fab!(:topic_1) { Fabricate(:topic) }
-      fab!(:post_1) { Fabricate(:post, topic: topic_1) }
-      fab!(:group_1) { Fabricate(:group) }
-      fab!(:user_1) { Fabricate(:user) }
-
-      before do
-        SiteSetting.assign_allowed_on_groups = "#{group_1.id}"
-        group_1.add(user_1)
+      let(:fields) do
+        {
+          "assignees_group" => {
+            "value" => group_1.id,
+          },
+          "assigned_topic" => {
+            "value" => topic_1.id,
+          },
+          "skip_new_users_for_days" => {
+            "value" => "10",
+          },
+        }
       end
 
       it "creates post on the topic if all users are new" do
-        described_class.automation_script!(
-          {},
-          {
-            "assignees_group" => {
-              "value" => group_1.id,
-            },
-            "assigned_topic" => {
-              "value" => topic_1.id,
-            },
-            "skip_new_users_for_days" => {
-              "value" => "10",
-            },
-          },
-          automation,
-        )
+        auto_assign
         expect(topic_1.posts.last.raw).to match(
           I18n.t("discourse_automation.scriptables.random_assign.no_one", group: group_1.name),
         )
       end
 
-      it "assign topic if all users are old" do
-        described_class.automation_script!(
-          {},
+      context "when all users are old" do
+        let(:fields) do
           {
             "assignees_group" => {
               "value" => group_1.id,
@@ -321,46 +282,65 @@ describe RandomAssignUtils do
             "skip_new_users_for_days" => {
               "value" => "0",
             },
-          },
-          automation,
-        )
+          }
+        end
 
-        expect(topic_1.assignment).not_to eq(nil)
+        it "assigns topic" do
+          auto_assign
+          expect(topic_1.assignment).not_to be_nil
+        end
       end
     end
   end
 
-  describe ".recently_assigned_users_ids" do
+  describe "#recently_assigned_users_ids" do
+    subject(:assignees) { utils.recently_assigned_users_ids(2.months.ago) }
+
+    let(:utils) { described_class.new({}, fields, automation) }
+    let(:fields) do
+      {
+        "assignees_group" => {
+          "value" => assign_allowed_group.id,
+        },
+        "assigned_topic" => {
+          "value" => post.topic.id,
+        },
+      }
+    end
+    let(:post) { Fabricate(:post) }
+    let(:assign_allowed_group) { Group.find_by(name: "staff") }
+
     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)
-          Assigner.new(post.topic, admin).assign(user_2)
-          Assigner.new(post_2, admin).assign(user_4)
-        end
-
+      before do
         freeze_time 3.months.ago do
           Assigner.new(post.topic, admin).assign(user_3)
         end
+        freeze_time 45.days.ago do
+          Assigner.new(post_2, admin).assign(user_4)
+        end
+        freeze_time 30.days.ago do
+          Assigner.new(post.topic, admin).assign(user_1)
+        end
+        freeze_time 15.days.ago do
+          Assigner.new(post.topic, admin).assign(user_2)
+        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