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 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.
This commit is contained in:
Loïc Guitaut 2023-11-15 13:58:05 +01:00 committed by Loïc Guitaut
parent 4ce9ae97f4
commit fbd1fa3794
2 changed files with 386 additions and 362 deletions

View File

@ -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("Group(#{group_id}) not found")
end
end
unless group = Group.find_by(id: group_id)
raise_error(automation, "Group(#{group_id}) not found")
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
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),
def recently_assigned_users_ids(from)
usernames =
PostCustomField
.joins(:post)
.where(
name: "action_code_who",
posts: {
topic: topic,
action_code: %w[assigned reassigned assigned_to_post],
},
)
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)
.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
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) }
private
if group_users_ids.empty?
RandomAssignUtils.no_one!(topic_id, group.name)
return
end
def assigned_user
@assigned_user ||=
begin
group_users_ids = group_users.pluck(:id)
return if group_users_ids.empty?
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)
last_assignees_ids = recently_assigned_users_ids(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)
recently_assigned_users_ids = recently_assigned_users_ids(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
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.find { |user_id| RandomAssignUtils.in_working_hours?(user_id) }
assign_to_user_id = users_ids.shuffle.detect { |user_id| 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
User.find(assign_to_user_id)
end
end
assign_to = User.find(assign_to_user_id)
result = nil
if raw = fields.dig("post_template", "value").presence
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: raw,
raw: post_template,
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)
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
RandomAssignUtils.no_one!(topic_id, group.name) if !result[:success]
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 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)
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)
def raise_error(message)
raise("[discourse-automation id=#{automation.id}] #{message}.")
end
def self.user_tzinfo(user_id)
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

View File

@ -3,35 +3,40 @@
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) }
before do
group_1.add(user_1)
UserCustomField.create!(name: "on_holiday", value: "t", user_id: user_1.id)
around do |example|
orig_logger = Rails.logger
Rails.logger = FakeLogger.new
example.run
Rails.logger = orig_logger
end
it "creates post on the topic" do
described_class.automation_script!(
{},
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,
@ -39,28 +44,21 @@ describe RandomAssignUtils do
"assigned_topic" => {
"value" => topic_1.id,
},
},
automation,
)
expect(topic_1.posts.first.raw).to match(
}
end
before { UserCustomField.create!(name: "on_holiday", value: "t", user_id: user_1.id) }
it "creates post on the topic" do
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)
end
it "creates post on the topic" do
described_class.automation_script!(
{},
let(:fields) do
{
"assignees_group" => {
"value" => group_1.id,
@ -68,25 +66,35 @@ describe RandomAssignUtils do
"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),
)
}
end
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) }
before { group_1.add(user_1) }
it "creates post on the topic" do
described_class.automation_script!(
{},
let(:fields) do
{
"assignees_group" => {
"value" => group_1.id,
@ -94,29 +102,22 @@ describe RandomAssignUtils do
"assigned_topic" => {
"value" => topic_1.id,
},
},
automation,
)
expect(topic_1.posts.first.raw).to match(
}
end
before { SiteSetting.assign_allowed_on_groups = "" }
it "creates post on the topic" do
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!(
{},
let(:fields) do
{
"post_template" => {
"value" => "this is a post template",
@ -127,19 +128,17 @@ describe RandomAssignUtils do
"assigned_topic" => {
"value" => topic_1.id,
},
},
automation,
)
expect(topic_1.posts.first.raw).to match("this is a post template")
}
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
fab!(:post_1) { Fabricate(:post, topic: topic_1) }
it "assigns the user to the topic" do
described_class.automation_script!(
{},
let(:fields) do
{
"assignees_group" => {
"value" => group_1.id,
@ -147,28 +146,18 @@ describe RandomAssignUtils do
"assigned_topic" => {
"value" => topic_1.id,
},
},
automation,
)
}
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
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,
@ -179,73 +168,67 @@ describe RandomAssignUtils do
"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),
)
}
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,19 +249,7 @@ 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)
end
it "creates post on the topic if all users are new" do
described_class.automation_script!(
{},
let(:fields) do
{
"assignees_group" => {
"value" => group_1.id,
@ -300,17 +260,18 @@ describe RandomAssignUtils do
"skip_new_users_for_days" => {
"value" => "10",
},
},
automation,
)
}
end
it "creates post on the topic if all users are new" do
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",
},
}
end
it "assigns topic" do
auto_assign
expect(topic_1.assignment).not_to be_nil
end
end
end
end
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,
},
automation,
)
"assigned_topic" => {
"value" => post.topic.id,
},
}
end
let(:post) { Fabricate(:post) }
let(:assign_allowed_group) { Group.find_by(name: "staff") }
expect(topic_1.assignment).not_to eq(nil)
end
end
end
describe ".recently_assigned_users_ids" do
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