Optimization:Dont Notify of Channel Leavings (#10518)

* Optimization:Dont Notify of Channel Leavings

* Add specs and dont send pusher notifications or mark chat channel as unread for left or removed chat actions

app/controllers/chat_channel_memberships_controller.rb

@@ -276,7 +276,7 @@ class ChatChannelMembershipsController < ApplicationController
     temp_message_id = (0...20).map { ("a".."z").to_a[rand(8)] }.join
     message = Message.create("message_markdown" => message, "user_id" => user.id, "chat_channel_id" => channel_id,
                              "chat_action" => action)
-    pusher_message_created(false, message, temp_message_id)
+    pusher_message_created(false, message, temp_message_id) unless message.left_channel?
   end
 
   def user_not_authorized

app/models/message.rb

@@ -23,6 +23,10 @@ class Message < ApplicationRecord
     chat_channel.users.where.not(id: user.id).first
   end
 
+  def left_channel?
+    chat_action == "removed_from_channel" || chat_action == "left_channel"
+  end
+
   private
 
   def update_chat_channel_last_message_at
@@ -31,6 +35,8 @@ class Message < ApplicationRecord
   end
 
   def update_all_has_unopened_messages_statuses
+    return if left_channel?
+
     chat_channel
       .chat_channel_memberships
       .where("last_opened_at < ?", 10.seconds.ago)

spec/models/message_spec.rb

@@ -147,6 +147,23 @@ RSpec.describe Message, type: :model do
     end
   end
 
+  describe "#left_channel?" do
+    it "returns true if chat_action is removed_from_channel" do
+      message.chat_action = "removed_from_channel"
+      expect(message.left_channel?).to eq(true)
+    end
+
+    it "returns true if chat_action is left_channel" do
+      message.chat_action = "left_channel"
+      expect(message.left_channel?).to eq(true)
+    end
+
+    it "returns false if chat_action is NOT removed_from_channel" do
+      message.chat_action = "joined"
+      expect(message.left_channel?).to eq(false)
+    end
+  end
+
   describe "#after_create" do
     it "enqueues ChatChannels::IndexesMembershipsWorker" do
       chat_channel.add_users([user])
@@ -156,5 +173,23 @@ RSpec.describe Message, type: :model do
 
       expect(ChatChannels::IndexesMembershipsWorker).to have_received(:perform_async)
     end
+
+    context "when chat_action is left_channel" do
+      it "does not update unopened message statuses" do
+        chat_channel.add_users([user, user2])
+        create(:message, chat_channel: chat_channel, user: user, chat_action: "left_channel")
+
+        expect(user2.chat_channel_memberships.pluck(:has_unopened_messages).all?(false)).to eq(true)
+      end
+    end
+
+    context "when chat_action is NOT left_channel" do
+      it "updates unopened message statuses" do
+        chat_channel.add_users([user, user2])
+        create(:message, chat_channel: chat_channel, user: user)
+
+        expect(user2.chat_channel_memberships.pluck(:has_unopened_messages).all?(true)).to eq(true)
+      end
+    end
   end
 end

spec/requests/chat_channel_memberships_spec.rb

@@ -128,6 +128,20 @@ RSpec.describe "ChatChannelMemberships", type: :request do
         expect(response.status).to eq(401)
       end
     end
+
+    it "does not send pusher notifications" do
+      allow(Pusher).to receive(:trigger)
+
+      user.add_role(:super_admin)
+      membership = ChatChannelMembership.find_by(chat_channel_id: chat_channel.id, user_id: user.id)
+
+      post "/chat_channel_memberships/remove_membership", params: {
+        chat_channel_id: chat_channel.id,
+        membership_id: membership.id
+      }
+
+      expect(Pusher).not_to have_received(:trigger)
+    end
   end
 
   describe "PUT /chat_channel_memberships/:id" do
@@ -211,7 +225,7 @@ RSpec.describe "ChatChannelMemberships", type: :request do
     end
   end
 
-  describe "POST/ /leave_membership/:id" do
+  describe "PATCH /leave_membership/:id" do
     context "when user is logged in" do
       it "leaves chat channel" do
         allow(Pusher).to receive(:trigger).and_return(true)
@@ -225,6 +239,18 @@ RSpec.describe "ChatChannelMemberships", type: :request do
         expect(response.status).to eq(200)
         expect(membership.reload.status).to eq("left_channel")
       end
+
+      it "does not send Pusher notifications" do
+        allow(Pusher).to receive(:trigger)
+        chat_channel.add_users([second_user])
+        membership = ChatChannelMembership.last
+
+        sign_in second_user
+
+        patch "/chat_channel_memberships/leave_membership/#{membership.id}"
+
+        expect(Pusher).not_to have_received(:trigger)
+      end
     end
 
     context "when user is not logged in" do