Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: Allow to reassign the same user #532

Merged
merged 1 commit into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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