FEATURE: Add `should_notify` option to `Assigner#assign` (#604)

* FEATURE: Add `should_notify` option to `Assigner#assign`

This option let's you control whether the added user within a group assignment should be notified or not.

* DEV: stree assign_controller file

* Update app/controllers/discourse_assign/assign_controller.rb

Co-authored-by: David Taylor <david@taylorhq.com>

---------

Co-authored-by: David Taylor <david@taylorhq.com>
This commit is contained in:
Gabriel Grubba 2024-11-14 15:01:02 -03:00 committed by GitHub
parent 964e44a63e
commit 6101ecdba5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 115 additions and 10 deletions

View File

@ -43,6 +43,8 @@ module DiscourseAssign
group_name = params.permit(:group_name)["group_name"] group_name = params.permit(:group_name)["group_name"]
note = params.permit(:note)["note"].presence note = params.permit(:note)["note"].presence
status = params.permit(:status)["status"].presence status = params.permit(:status)["status"].presence
should_notify = params.permit(:should_notify)["should_notify"]
should_notify = (should_notify.present? ? should_notify.to_s == "true" : true)
assign_to = assign_to =
( (
@ -58,7 +60,13 @@ module DiscourseAssign
target = target_type.constantize.where(id: target_id).first target = target_type.constantize.where(id: target_id).first
raise Discourse::NotFound unless target raise Discourse::NotFound unless target
assign = Assigner.new(target, current_user).assign(assign_to, note: note, status: status) assign =
Assigner.new(target, current_user).assign(
assign_to,
note: note,
status: status,
should_notify: should_notify,
)
if assign[:success] if assign[:success]
render json: success_json render json: success_json

View File

@ -221,7 +221,7 @@ class ::Assigner
end end
end end
def update_details(assign_to, note, status, skip_small_action_post: false) def update_details(assign_to, note, status, skip_small_action_post: false, should_notify: true)
case case
when note.present? && status.present? && @target.assignment.note != note && when note.present? && status.present? && @target.assignment.note != note &&
@target.assignment.status != status @target.assignment.status != status
@ -240,7 +240,7 @@ class ::Assigner
end end
@target.assignment.update!(note: note, status: status) @target.assignment.update!(note: note, status: status)
queue_notification(@target.assignment) queue_notification(@target.assignment) if should_notify
publish_assignment(@target.assignment, assign_to, note, status) publish_assignment(@target.assignment, assign_to, note, status)
# email is skipped, for now # email is skipped, for now
@ -258,12 +258,17 @@ class ::Assigner
note: nil, note: nil,
skip_small_action_post: false, skip_small_action_post: false,
status: nil, status: nil,
allow_self_reassign: false allow_self_reassign: false,
should_notify: true
) )
assigned_to_type = assign_to.is_a?(User) ? "User" : "Group" assigned_to_type = assign_to.is_a?(User) ? "User" : "Group"
if topic.private_message? && SiteSetting.invite_on_assign if topic.private_message? && SiteSetting.invite_on_assign
assigned_to_type == "Group" ? invite_group(assign_to) : invite_user(assign_to) if assigned_to_type == "Group"
invite_group(assign_to, should_notify)
else
invite_user(assign_to)
end
end end
forbidden_reason = forbidden_reason =
@ -277,7 +282,15 @@ class ::Assigner
return { success: false, reason: forbidden_reason } if forbidden_reason return { success: false, reason: forbidden_reason } if forbidden_reason
if no_assignee_change?(assign_to) && details_change?(note, status) if no_assignee_change?(assign_to) && details_change?(note, status)
return update_details(assign_to, note, status, skip_small_action_post: skip_small_action_post) return(
update_details(
assign_to,
note,
status,
skip_small_action_post: skip_small_action_post,
should_notify: should_notify,
)
)
end end
action_code = {} action_code = {}
@ -308,7 +321,7 @@ class ::Assigner
) )
first_post.publish_change_to_clients!(:revised, reload_topic: true) first_post.publish_change_to_clients!(:revised, reload_topic: true)
queue_notification(assignment) queue_notification(assignment) if should_notify
publish_assignment(assignment, assign_to, note, status) publish_assignment(assignment, assign_to, note, status)
if assignment.assigned_to_user? if assignment.assigned_to_user?
@ -452,7 +465,7 @@ class ::Assigner
topic.invite(@assigned_by, user.username) topic.invite(@assigned_by, user.username)
end end
def invite_group(group) def invite_group(group, should_notify)
return if topic.topic_allowed_groups.exists?(group_id: group.id) return if topic.topic_allowed_groups.exists?(group_id: group.id)
if topic if topic
.all_allowed_users .all_allowed_users
@ -463,7 +476,7 @@ class ::Assigner
end end
guardian.ensure_can_invite_group_to_private_message!(group, topic) guardian.ensure_can_invite_group_to_private_message!(group, topic)
topic.invite_group(@assigned_by, group) topic.invite_group(@assigned_by, group, should_notify: should_notify)
end end
def guardian def guardian

View File

@ -465,11 +465,17 @@ RSpec.describe Assigner do
it "queues notification" do it "queues notification" do
assigner.assign(moderator) assigner.assign(moderator)
expect(job_enqueued?(job: :assign_notification)).to eq(true)
expect_enqueued_with(job: :assign_notification) do expect_enqueued_with(job: :assign_notification) do
assigner.assign(moderator, status: "Done") assigner.assign(moderator, status: "Done")
end end
end end
it "does not queue notification if should_notify is set to false" do
assigner.assign(moderator, status: "Done", should_notify: false)
expect(job_enqueued?(job: :assign_notification)).to eq(false)
end
it "publishes topic assignment with note" do it "publishes topic assignment with note" do
assigner.assign(moderator) assigner.assign(moderator)
@ -756,7 +762,7 @@ RSpec.describe Assigner do
expect(topic.allowed_users).not_to include(user) expect(topic.allowed_users).not_to include(user)
end end
it "invites group to the PM" do it "invites group to the PM and notifies users" do
group = group =
Fabricate( Fabricate(
:group, :group,
@ -764,8 +770,30 @@ RSpec.describe Assigner do
messageable_level: Group::ALIAS_LEVELS[:only_admins], messageable_level: Group::ALIAS_LEVELS[:only_admins],
) )
group.add(Fabricate(:user)) group.add(Fabricate(:user))
Notification.delete_all
Jobs.run_immediately!
assigner.assign(group) assigner.assign(group)
expect(topic.allowed_groups).to include(group) expect(topic.allowed_groups).to include(group)
expect(Notification.count).to be > 0
end
it "invites group to the PM and does not notifies users if should_notify is false" do
group =
Fabricate(
:group,
assignable_level: Group::ALIAS_LEVELS[:only_admins],
messageable_level: Group::ALIAS_LEVELS[:only_admins],
)
group.add(Fabricate(:user))
Notification.delete_all
Jobs.run_immediately!
assigner.assign(group, should_notify: false)
expect(topic.allowed_groups).to include(group)
expect(Notification.count).to eq(0)
end end
it "doesn't invite group if all members have access to the PM already" do it "doesn't invite group if all members have access to the PM already" do

View File

@ -259,6 +259,62 @@ RSpec.describe DiscourseAssign::AssignController do
) )
end end
it "notifies the assignee when the topic is assigned to a group" do
admins = Group[:admins]
admins.messageable_level = Group::ALIAS_LEVELS[:everyone]
admins.save!
SiteSetting.invite_on_assign = true
pm = Fabricate(:private_message_post, user: admin).topic
another_user = Fabricate(:user)
admins.add(another_user)
admins
.group_users
.find_by(user_id: another_user.id)
.update!(notification_level: NotificationLevels.all[:watching])
Notification.delete_all
Jobs.run_immediately!
put "/assign/assign.json",
params: {
target_id: pm.id,
target_type: "Topic",
group_name: admins.name,
}
expect(Notification.count).to be > 0
end
it "does not notify the assignee when the topic is assigned to a group if should_notify option is set to false" do
admins = Group[:admins]
admins.messageable_level = Group::ALIAS_LEVELS[:everyone]
admins.save!
SiteSetting.invite_on_assign = true
pm = Fabricate(:private_message_post, user: admin).topic
another_user = Fabricate(:user)
admins.add(another_user)
admins
.group_users
.find_by(user_id: another_user.id)
.update!(notification_level: NotificationLevels.all[:watching])
Notification.delete_all
Jobs.run_immediately!
put "/assign/assign.json",
params: {
target_id: pm.id,
target_type: "Topic",
group_name: admins.name,
should_notify: false,
}
expect(Notification.count).to eq(0)
end
it "fails with a specific error message if the topic is not a PM and the assignee can not see it" do it "fails with a specific error message if the topic is not a PM and the assignee can not see it" do
topic = Fabricate(:topic, category: Fabricate(:private_category, group: Fabricate(:group))) topic = Fabricate(:topic, category: Fabricate(:private_category, group: Fabricate(:group)))
another_user = Fabricate(:user) another_user = Fabricate(:user)