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 <[email protected]>

---------

Co-authored-by: David Taylor <[email protected]>

Author: Grubba27

app/controllers/discourse_assign/assign_controller.rb

@@ -43,6 +43,8 @@ def assign
       group_name = params.permit(:group_name)["group_name"]
       note = params.permit(:note)["note"].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 =
         (
@@ -58,7 +60,13 @@ def assign
       target = target_type.constantize.where(id: target_id).first
       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]
         render json: success_json

lib/assigner.rb

@@ -221,7 +221,7 @@ def forbidden_reasons(assign_to:, type:, note:, status:, allow_self_reassign:)
     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
     when note.present? && status.present? && @target.assignment.note != note &&
            @target.assignment.status != status
@@ -240,7 +240,7 @@ def update_details(assign_to, note, status, skip_small_action_post: false)
     end
 
     @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)
 
     # email is skipped, for now
@@ -258,12 +258,17 @@ def assign(
     note: nil,
     skip_small_action_post: false,
     status: nil,
-    allow_self_reassign: false
+    allow_self_reassign: false,
+    should_notify: true
   )
     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)
+      if assigned_to_type == "Group"
+        invite_group(assign_to, should_notify)
+      else
+        invite_user(assign_to)
+      end
     end
 
     forbidden_reason =
@@ -277,7 +282,15 @@ def assign(
     return { success: false, reason: forbidden_reason } if forbidden_reason
 
     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
 
     action_code = {}
@@ -308,7 +321,7 @@ def assign(
       )
 
     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)
 
     if assignment.assigned_to_user?
@@ -452,7 +465,7 @@ def invite_user(user)
     topic.invite(@assigned_by, user.username)
   end
 
-  def invite_group(group)
+  def invite_group(group, should_notify)
     return if topic.topic_allowed_groups.exists?(group_id: group.id)
     if topic
          .all_allowed_users
@@ -463,7 +476,7 @@ def invite_group(group)
     end
 
     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
 
   def guardian

spec/lib/assigner_spec.rb

@@ -465,11 +465,17 @@ def assigned_to?(assignee)
       it "queues notification" do
         assigner.assign(moderator)
 
+        expect(job_enqueued?(job: :assign_notification)).to eq(true)
         expect_enqueued_with(job: :assign_notification) do
           assigner.assign(moderator, status: "Done")
         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
         assigner.assign(moderator)
 
@@ -756,16 +762,38 @@ def assigned_to?(assignee)
       expect(topic.allowed_users).not_to include(user)
     end
 
-    it "invites group to the PM" do
+    it "invites group to the PM and notifies users" 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)
       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
 
     it "doesn't invite group if all members have access to the PM already" do

spec/requests/assign_controller_spec.rb

@@ -259,6 +259,62 @@ def assign_user_to_post
       )
     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
       topic = Fabricate(:topic, category: Fabricate(:private_category, group: Fabricate(:group)))
       another_user = Fabricate(:user)