FEATURE: Add skip notification option to group invite to topic (#29741)

* FEATURE: Add skip notification option to group invite to topic

* DEV: rename `skip_notification` to `should_notify`

* DEV: update `should_notify` param to be default `true` in controllers

* DEV: update spec to use `greater than` instead of `equal to` to prevent flakiness

* Update app/controllers/topics_controller.rb

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

* DEV: merged two `#invite_group` specs into one

* DEV: Added test case for `invite-group` in requests spec

---------

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

app/controllers/topics_controller.rb

@@ -757,7 +757,13 @@ class TopicsController < ApplicationController
 
     if topic.private_message?
       guardian.ensure_can_invite_group_to_private_message!(group, topic)
-      topic.invite_group(current_user, group)
+      should_notify =
+        if params[:should_notify].blank?
+          true
+        else
+          params[:should_notify].to_s == "true"
+        end
+      topic.invite_group(current_user, group, should_notify: should_notify)
       render_json_dump BasicGroupSerializer.new(group, scope: guardian, root: "group")
     else
       render json: failed_json, status: 422

app/models/topic.rb

@@ -1186,16 +1186,18 @@ class Topic < ActiveRecord::Base
       SiteSetting.max_allowed_message_recipients
   end
 
-  def invite_group(user, group)
+  def invite_group(user, group, should_notify: true)
     TopicAllowedGroup.create!(topic_id: self.id, group_id: group.id)
     self.allowed_groups.reload
 
     last_post =
       self.posts.order("post_number desc").where("not hidden AND posts.deleted_at IS NULL").first
     if last_post
-      Jobs.enqueue(:post_alert, post_id: last_post.id)
       add_small_action(user, "invited_group", group.name)
-      Jobs.enqueue(:group_pm_alert, user_id: user.id, group_id: group.id, post_id: last_post.id)
+      if should_notify
+        Jobs.enqueue(:post_alert, post_id: last_post.id)
+        Jobs.enqueue(:group_pm_alert, user_id: user.id, group_id: group.id, post_id: last_post.id)
+      end
     end
 
     # If the group invited includes the OP of the topic as one of is members,

spec/models/topic_spec.rb

@@ -1239,6 +1239,23 @@ RSpec.describe Topic do
             expect(notification.notification_type).to eq(Notification.types[:group_message_summary])
           end
 
+          it "does not create notifications if invite is set to skip notifications" do
+            Fabricate(:post, topic: topic)
+            user_watching = Fabricate(:user)
+
+            group.add(topic.user)
+            group.add(user_watching)
+
+            set_state!(group, topic.user, :watching)
+            set_state!(group, user_watching, :watching)
+
+            Notification.delete_all
+            Jobs.run_immediately!
+            topic.invite_group(topic.user, group, should_notify: false)
+
+            expect(Notification.count).to eq(0)
+          end
+
           it "removes users in topic_allowed_users who are part of the added group" do
             admins = Group[:admins]
             admins.update!(messageable_level: Group::ALIAS_LEVELS[:everyone])

spec/requests/api/topics_spec.rb

@@ -372,6 +372,62 @@ RSpec.describe "topics" do
     end
   end
 
+  path "/t/{id}/invite-group.json" do
+    post "Invite group to topic" do
+      tags "Topics", "Invites"
+      operationId "inviteGroupToTopic"
+      consumes "application/json"
+      parameter name: "Api-Key", in: :header, type: :string, required: true
+      parameter name: "Api-Username", in: :header, type: :string, required: true
+      parameter name: :id, in: :path, schema: { type: :string }
+
+      parameter name: :request_body,
+                in: :body,
+                schema: {
+                  type: :object,
+                  properties: {
+                    group: {
+                      type: :string,
+                      description: "The name of the group to invite",
+                    },
+                    should_notify: {
+                      type: :boolean,
+                      description: "Whether to notify the group, it defaults to true",
+                    },
+                  },
+                }
+
+      produces "application/json"
+      response "200", "invites to a PM" do
+        schema type: :object,
+               properties: {
+                 group: {
+                   type: :object,
+                   properties: {
+                     id: {
+                       type: :integer,
+                     },
+                     name: {
+                       type: :string,
+                     },
+                   },
+                 },
+               }
+
+        let!(:admins) { Group[:admins] }
+        let(:request_body) { { group: admins.name } }
+        let(:pm) { Fabricate(:private_message_topic) }
+        let(:id) { pm.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data["group"]["name"]).to eq(admins.name)
+          expect(pm.allowed_groups.first.id).to eq(admins.id)
+        end
+      end
+    end
+  end
+
   path "/t/{id}/bookmark.json" do
     put "Bookmark topic" do
       tags "Topics"

spec/requests/topics_controller_spec.rb

@@ -3624,29 +3624,6 @@ RSpec.describe TopicsController do
     end
   end
 
-  describe "#invite_group" do
-    let!(:admins) { Group[:admins] }
-
-    before do
-      sign_in(admin)
-      admins.messageable_level = Group::ALIAS_LEVELS[:everyone]
-      admins.save!
-    end
-
-    it "disallows inviting a group to a topic" do
-      post "/t/#{topic.id}/invite-group.json", params: { group: "admins" }
-
-      expect(response.status).to eq(422)
-    end
-
-    it "allows inviting a group to a PM" do
-      post "/t/#{pm.id}/invite-group.json", params: { group: "admins" }
-
-      expect(response.status).to eq(200)
-      expect(pm.allowed_groups.first.id).to eq(admins.id)
-    end
-  end
-
   describe "#make_banner" do
     it "needs you to be a staff member" do
       tl4_topic = Fabricate(:topic, user: sign_in(trust_level_4))
@@ -5272,7 +5249,7 @@ RSpec.describe TopicsController do
     end
   end
 
-  describe "invite_group" do
+  describe "#invite_group" do
     let!(:admins) { Group[:admins] }
 
     def invite_group(topic, expected_status)
@@ -5318,6 +5295,40 @@ RSpec.describe TopicsController do
         invite_group(pm, 200)
         expect(pm.allowed_groups.first.id).to eq(admins.id)
       end
+
+      it "sends a notification to the group" do
+        user = Fabricate(:user)
+        Fabricate(:post, topic: pm)
+        admins.add(user)
+        admins
+          .group_users
+          .find_by(user_id: user.id)
+          .update!(notification_level: NotificationLevels.all[:watching])
+
+        Notification.delete_all
+        Jobs.run_immediately!
+        post "/t/#{pm.id}/invite-group.json", params: { group: "admins" }
+
+        expect(response.status).to eq(200)
+        expect(Notification.count).to be > 0
+      end
+
+      it "allows disabling notifications for that invite" do
+        user = Fabricate(:user)
+        Fabricate(:post, topic: pm)
+        admins.add(user)
+        admins
+          .group_users
+          .find_by(user_id: user.id)
+          .update!(notification_level: NotificationLevels.all[:watching])
+
+        Notification.delete_all
+        Jobs.run_immediately!
+        post "/t/#{pm.id}/invite-group.json", params: { group: "admins", should_notify: false }
+
+        expect(response.status).to eq(200)
+        expect(Notification.count).to eq(0)
+      end
     end
 
     context "when PM has reached maximum allowed numbers of recipients" do