Skip to content

Commit

Permalink
FEATURE: Allow to reassign to same user
Browse files Browse the repository at this point in the history
Currently, when only one user is available for the random auto-assign
and if that user was already assigned, then the assign will fail.

This patch addresses this issue by allowing to reassign a user who’s
already assigned. A new parameter (`allow_self_reassign`) has been added
to `Assigner#assign` with a default value of `false`.
  • Loading branch information
Flink committed Nov 27, 2023
1 parent eb073fe commit cac9a76
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 25 deletions.
23 changes: 18 additions & 5 deletions lib/assigner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ def first_post
topic.posts.where(post_number: 1).first
end

def forbidden_reasons(assign_to:, type:, note:, status:)
def forbidden_reasons(assign_to:, type:, note:, status:, allow_self_reassign:)
case
when assign_to.is_a?(User) && !can_assignee_see_target?(assign_to)
if topic.private_message?
Expand All @@ -211,7 +211,7 @@ def forbidden_reasons(assign_to:, type:, note:, status:)
end
when !can_be_assigned?(assign_to)
assign_to.is_a?(User) ? :forbidden_assign_to : :forbidden_group_assign_to
when already_assigned?(assign_to, type, note, status)
when !allow_self_reassign && already_assigned?(assign_to, type, note, status)
assign_to.is_a?(User) ? :already_assigned : :group_already_assigned
when Assignment.where(topic: topic, active: true).count >= ASSIGNMENTS_PER_TOPIC_LIMIT &&
!reassign?
Expand Down Expand Up @@ -252,15 +252,27 @@ def update_details(assign_to, note, status, skip_small_action_post: false)
{ success: true }
end

def assign(assign_to, note: nil, skip_small_action_post: false, status: nil)
def assign(
assign_to,
note: nil,
skip_small_action_post: false,
status: nil,
allow_self_reassign: false
)
assigned_to_type = assign_to.is_a?(User) ? "User" : "Group"

if topic.private_message? && SiteSetting.invite_on_assign
assigned_to_type == "Group" ? invite_group(assign_to) : invite_user(assign_to)
end

forbidden_reason =
forbidden_reasons(assign_to: assign_to, type: assigned_to_type, note: note, status: status)
forbidden_reasons(
assign_to: assign_to,
type: assigned_to_type,
note: note,
status: status,
allow_self_reassign: allow_self_reassign,
)
return { success: false, reason: forbidden_reason } if forbidden_reason

if no_assignee_change?(assign_to) && details_change?(note, status)
Expand All @@ -271,7 +283,8 @@ def assign(assign_to, note: nil, skip_small_action_post: false, status: nil)
action_code[:user] = topic.assignment.present? ? "reassigned" : "assigned"
action_code[:group] = topic.assignment.present? ? "reassigned_group" : "assigned_group"

skip_small_action_post = skip_small_action_post || no_assignee_change?(assign_to)
skip_small_action_post =
skip_small_action_post || (!allow_self_reassign && no_assignee_change?(assign_to))

if @target.assignment
Jobs.enqueue(
Expand Down
4 changes: 2 additions & 2 deletions lib/random_assign_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def assign_user!
return create_post_template if post_template
Assigner
.new(topic, Discourse.system_user)
.assign(assigned_user)
.assign(assigned_user, allow_self_reassign: true)
.then do |result|
next if result[:success]
no_one!
Expand All @@ -104,7 +104,7 @@ def create_post_template
).create!
Assigner
.new(post, Discourse.system_user)
.assign(assigned_user)
.assign(assigned_user, allow_self_reassign: true)
.then do |result|
next if result[:success]
PostDestroyer.new(Discourse.system_user, post).destroy
Expand Down
50 changes: 32 additions & 18 deletions spec/lib/assigner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -264,33 +264,47 @@ def assigned_to?(assignee)
expect(second_assign[:success]).to eq(true)
end

it "fails to assign when the assigned user and note is the same" do
assigner = described_class.new(topic, moderator_2)
assigner.assign(moderator, note: "note me down")
context "when 'allow_self_reassign' is false" do
subject(:assign) do
assigner.assign(moderator, note: other_note, allow_self_reassign: self_reassign)
end

assign = assigner.assign(moderator, note: "note me down")
let(:self_reassign) { false }
let(:assigner) { described_class.new(topic, moderator_2) }
let(:note) { "note me down" }

expect(assign[:success]).to eq(false)
expect(assign[:reason]).to eq(:already_assigned)
end
before { assigner.assign(moderator, note: note) }

it "fails to assign when the assigned user and note is the same" do
assigner = described_class.new(post, moderator_2)
assigner.assign(moderator, note: "note me down")
context "when the assigned user and the note is the same" do
let(:other_note) { note }

assign = assigner.assign(moderator, note: "note me down")
it "fails to assign" do
expect(assign).to match(success: false, reason: :already_assigned)
end
end

expect(assign[:success]).to eq(false)
expect(assign[:reason]).to eq(:already_assigned)
context "when the assigned user is the same but the note is different" do
let(:other_note) { "note me down again" }

it "allows assignment" do
expect(assign).to match(success: true)
end
end
end

it "allows assign when the assigned user is same but note is different" do
assigner = described_class.new(topic, moderator_2)
assigner.assign(moderator, note: "note me down")
context "when 'allow_self_reassign' is true" do
subject(:assign) { assigner.assign(moderator, allow_self_reassign: self_reassign) }

assign = assigner.assign(moderator, note: "note me down again")
let(:self_reassign) { true }
let(:assigner) { described_class.new(topic, moderator_2) }

expect(assign[:success]).to eq(true)
context "when the assigned user is the same" do
before { assigner.assign(moderator) }

it "allows assignment" do
expect(assign).to match(success: true)
end
end
end

it "fails to assign when the assigned user cannot view the pm" do
Expand Down
22 changes: 22 additions & 0 deletions spec/lib/random_assign_utils_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,28 @@
end
end

context "when in a group of one person" do
let(:fields) do
{
"assignees_group" => {
"value" => group_1.id,
},
"assigned_topic" => {
"value" => topic_1.id,
},
}
end

context "when user is already assigned" do
before { described_class.automation_script!(ctx, fields, automation) }

it "reassigns them" do
expect { auto_assign }.to change { topic_1.reload.assignment.id }
expect(topic_1.assignment.assigned_to).to eq(user_1)
end
end
end

context "when assignees_group is not provided" do
let(:fields) { { "assigned_topic" => { "value" => topic_1.id } } }

Expand Down

0 comments on commit cac9a76

Please sign in to comment.