From f20e3c0795d96d86933cee36e340d5e08a769857 Mon Sep 17 00:00:00 2001 From: Ondrej Prazak Date: Tue, 22 Dec 2020 11:46:49 +0100 Subject: [PATCH] Fixes #156 - Deprecate "shout" event Deprecate "shout" event for conversation channel in favor of "message:created". --- lib/chat_api/messages/notification.ex | 6 +-- .../channels/conversation_channel.ex | 46 +++++++++++++------ .../channels/notification_channel.ex | 43 +++++++++++------ .../channels/conversation_channel_test.exs | 6 +++ .../channels/notification_channel_test.exs | 16 +++++++ 5 files changed, 84 insertions(+), 33 deletions(-) diff --git a/lib/chat_api/messages/notification.ex b/lib/chat_api/messages/notification.ex index 4ed149852..afc4f6707 100644 --- a/lib/chat_api/messages/notification.ex +++ b/lib/chat_api/messages/notification.ex @@ -16,11 +16,11 @@ defmodule ChatApi.Messages.Notification do }) end - @spec broadcast_to_conversation!(Message.t()) :: Message.t() - def broadcast_to_conversation!(%Message{} = message) do + @spec broadcast_to_conversation!(Message.t(), String.t()) :: Message.t() + def broadcast_to_conversation!(%Message{} = message, event_name \\ "shout") do message |> Helpers.get_conversation_topic() - |> ChatApiWeb.Endpoint.broadcast!("shout", Helpers.format(message)) + |> ChatApiWeb.Endpoint.broadcast!(event_name, Helpers.format(message)) message end diff --git a/lib/chat_api_web/channels/conversation_channel.ex b/lib/chat_api_web/channels/conversation_channel.ex index a59a78f7c..bc2f70dc8 100644 --- a/lib/chat_api_web/channels/conversation_channel.ex +++ b/lib/chat_api_web/channels/conversation_channel.ex @@ -3,6 +3,7 @@ defmodule ChatApiWeb.ConversationChannel do alias ChatApiWeb.Presence alias ChatApi.{Messages, Conversations} + require Logger @impl true def join("conversation:lobby", payload, socket) do @@ -79,20 +80,16 @@ defmodule ChatApiWeb.ConversationChannel do end def handle_in("shout", payload, socket) do - with %{conversation: conversation} <- socket.assigns, - %{id: conversation_id, account_id: account_id} <- conversation, - {:ok, message} <- - payload - |> Map.merge(%{"conversation_id" => conversation_id, "account_id" => account_id}) - |> Messages.create_message(), - message <- Messages.get_message!(message.id) do - broadcast_new_message(socket, message) - else - _ -> - broadcast(socket, "shout", payload) - end + Logger.warn( + "'shout' is deprecated as event name on a new message and will be removed in a future version. Please migrate to a newer version of a client." + ) - {:noreply, socket} + handle_incoming_message("shout", payload, socket) + end + + @impl true + def handle_in("message:created", payload, socket) do + handle_incoming_message("message:created", payload, socket) end def handle_in("messages:seen", _payload, socket) do @@ -119,9 +116,9 @@ defmodule ChatApiWeb.ConversationChannel do }) end - defp broadcast_new_message(socket, message) do + defp broadcast_new_message(socket, event_name, message) do broadcast_conversation_update(message) - broadcast(socket, "shout", Messages.Helpers.format(message)) + broadcast(socket, event_name, Messages.Helpers.format(message)) message |> Messages.Notification.notify(:slack) @@ -143,4 +140,23 @@ defmodule ChatApiWeb.ConversationChannel do _ -> false end end + + # It is also common to receive messages from the client and + # broadcast to everyone in the current topic (conversation:lobby). + defp handle_incoming_message(event_name, payload, socket) do + with %{conversation: conversation} <- socket.assigns, + %{id: conversation_id, account_id: account_id} <- conversation, + {:ok, message} <- + payload + |> Map.merge(%{"conversation_id" => conversation_id, "account_id" => account_id}) + |> Messages.create_message(), + message <- Messages.get_message!(message.id) do + broadcast_new_message(socket, event_name, message) + else + _ -> + broadcast(socket, event_name, payload) + end + + {:noreply, socket} + end end diff --git a/lib/chat_api_web/channels/notification_channel.ex b/lib/chat_api_web/channels/notification_channel.ex index 5bf89782d..60dd48468 100644 --- a/lib/chat_api_web/channels/notification_channel.ex +++ b/lib/chat_api_web/channels/notification_channel.ex @@ -51,21 +51,16 @@ defmodule ChatApiWeb.NotificationChannel do end def handle_in("shout", payload, socket) do - with %{current_user: current_user} <- socket.assigns, - %{id: user_id, account_id: account_id} <- current_user do - {:ok, message} = - payload - |> Map.merge(%{"user_id" => user_id, "account_id" => account_id}) - |> Messages.create_message() + Logger.warn( + "'shout' is deprecated as event name on a new message and will be removed in a future version. Please migrate to a newer version of a client." + ) - message - |> Map.get(:id) - |> Messages.get_message!() - |> broadcast_new_message() - |> Messages.Helpers.handle_post_creation_conversation_updates() - end + handle_incoming_message("shout", payload, socket) + end - {:reply, :ok, socket} + @impl true + def handle_in("message:created", payload, socket) do + handle_incoming_message("message:created", payload, socket) end @impl true @@ -105,9 +100,9 @@ defmodule ChatApiWeb.NotificationChannel do {:noreply, socket} end - defp broadcast_new_message(message) do + defp broadcast_new_message(message, event_name) do message - |> Messages.Notification.broadcast_to_conversation!() + |> Messages.Notification.broadcast_to_conversation!(event_name) |> Messages.Notification.notify(:slack) |> Messages.Notification.notify(:slack_support_channel) |> Messages.Notification.notify(:slack_company_channel) @@ -137,4 +132,22 @@ defmodule ChatApiWeb.NotificationChannel do _ -> false end end + + defp handle_incoming_message(event_name, payload, socket) do + with %{current_user: current_user} <- socket.assigns, + %{id: user_id, account_id: account_id} <- current_user do + {:ok, message} = + payload + |> Map.merge(%{"user_id" => user_id, "account_id" => account_id}) + |> Messages.create_message() + + message + |> Map.get(:id) + |> Messages.get_message!() + |> broadcast_new_message(event_name) + |> Messages.Helpers.handle_post_creation_conversation_updates() + end + + {:reply, :ok, socket} + end end diff --git a/test/chat_api_web/channels/conversation_channel_test.exs b/test/chat_api_web/channels/conversation_channel_test.exs index 06d550687..d64ef58f9 100644 --- a/test/chat_api_web/channels/conversation_channel_test.exs +++ b/test/chat_api_web/channels/conversation_channel_test.exs @@ -25,6 +25,12 @@ defmodule ChatApiWeb.ConversationChannelTest do assert_broadcast "shout", _msg end + test "message:created broadcasts to conversation:lobby", %{socket: socket, account: account} do + msg = %{body: "Hello world!", account_id: account.id} + push(socket, "message:created", msg) + assert_broadcast "message:created", _msg + end + test "broadcasts are pushed to the client", %{socket: socket} do broadcast_from!(socket, "broadcast", %{"some" => "data"}) assert_push "broadcast", %{"some" => "data"} diff --git a/test/chat_api_web/channels/notification_channel_test.exs b/test/chat_api_web/channels/notification_channel_test.exs index 881d5383c..c147d3574 100644 --- a/test/chat_api_web/channels/notification_channel_test.exs +++ b/test/chat_api_web/channels/notification_channel_test.exs @@ -40,6 +40,22 @@ defmodule ChatApiWeb.NotificationChannelTest do assert_push("shout", _msg) end + test "message:created broadcasts to notification:lobby", %{ + socket: socket, + account: account, + conversation: conversation + } do + msg = %{ + body: "Message created!", + account_id: account.id, + conversation_id: conversation.id + } + + push(socket, "message:created", msg) + + assert_push("message:created", _msg) + end + test "broadcasts are pushed to the client", %{socket: socket} do broadcast_from!(socket, "broadcast", %{"some" => "data"}) assert_push "broadcast", %{"some" => "data"}