From 386c3e9fa7d07c4e454c17da4aacf59b91c914d1 Mon Sep 17 00:00:00 2001 From: Molly Struve Date: Mon, 27 Jul 2020 11:16:41 -0500 Subject: [PATCH] [deploy] Optimization/Bug Fix:Only Send Mentioned User Notifications, not all for every message (#9493) --- app/controllers/messages_controller.rb | 13 +++++-------- spec/requests/messages_spec.rb | 13 ++++++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 4f8e879ec..45d3dd8be 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -13,8 +13,7 @@ class MessagesController < ApplicationController pusher_message_created(true, @message, @temp_message_id) if @message.save pusher_message_created(false, @message, @temp_message_id) - notify_users(@message.chat_channel.channel_users_ids, "all") - notify_users(@mentioned_users_id, "mention") + notify_mentioned_users(@mentioned_users_id) render json: { status: "success", message: { temp_id: @temp_message_id, id: @message.id } }, status: :created else render json: { @@ -104,17 +103,15 @@ class MessagesController < ApplicationController end end - def notify_users(user_ids, type) + def notify_mentioned_users(user_ids) + # If @all is mentioned then we get an array of all of the channel's users IDs from the channel + # https://github.com/forem/forem/blob/9bdef4d4ae0b41612001a62c2409121b654bf71f/app/javascript/chat/chat.jsx#L1562 return unless user_ids message_json = create_pusher_payload(@message, @temp_message_id) user_ids.each do |id| - if type == "mention" - Pusher.trigger("private-message-notifications-#{id}", "mentioned", message_json) - else - Pusher.trigger("private-message-notifications-#{id}", "message-created", message_json) - end + Pusher.trigger("private-message-notifications-#{id}", "mentioned", message_json) end end end diff --git a/spec/requests/messages_spec.rb b/spec/requests/messages_spec.rb index fcee6c749..73c124a33 100644 --- a/spec/requests/messages_spec.rb +++ b/spec/requests/messages_spec.rb @@ -23,16 +23,19 @@ RSpec.describe "Messages", type: :request do before do allow(Pusher).to receive(:trigger).and_return(true) sign_in user + end + + it "triggers pusher, returns json response and 201 status upon success", :aggregate_failures do post "/messages", params: { message: new_message } - end - - it "returns 201 upon success" do - allow(Pusher).to receive(:trigger).and_return(true) expect(response.status).to eq(201) + expect(response.media_type).to eq("application/json") + expect(Pusher).to have_received(:trigger).twice end - it "returns in json" do + it "sends mention notifications when mentioned_users_id present" do + post "/messages", params: { message: new_message.merge(mentioned_users_id: [99]) } expect(response.media_type).to eq("application/json") + expect(Pusher).to have_received(:trigger).exactly(3).times end end