From 2bfd0f648e78819b99e552b053a147e3c807658b 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". --- .../channels/conversation_channel.ex | 46 +++++++++++++------ .../channels/notification_channel.ex | 39 ++++++++++------ .../channels/conversation_channel_test.exs | 6 +++ .../channels/notification_channel_test.exs | 16 +++++++ 4 files changed, 79 insertions(+), 28 deletions(-) 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 8bbd0b42d..2f9dce2b9 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() - |> maybe_update_conversation_assignee() - end + handle_incoming_message(payload, socket) + end - {:noreply, socket} + @impl true + def handle_in("message:created", payload, socket) do + handle_incoming_message(payload, socket) end @impl true @@ -168,4 +163,22 @@ defmodule ChatApiWeb.NotificationChannel do _ -> false end end + + defp handle_incoming_message(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() + |> maybe_update_conversation_assignee() + end + + {:noreply, 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 b86f1e8be..523517636 100644 --- a/test/chat_api_web/channels/notification_channel_test.exs +++ b/test/chat_api_web/channels/notification_channel_test.exs @@ -38,6 +38,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: "Hello world!", + 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"}