Fixes #156 - Deprecate "shout" event

Deprecate "shout" event for conversation channel
in favor of "message:created".

Author: xprazak2

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

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

lib/chat_api_web/channels/notification_channel.ex

@@ -51,21 +51,15 @@ 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()
-
-      message
-      |> Map.get(:id)
-      |> Messages.get_message!()
-      |> broadcast_new_message()
-      |> Messages.Helpers.handle_post_creation_conversation_updates()
-    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."
+    )
+    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 +99,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 +131,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

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"}

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"}