diff --git a/lib/chat_api/messages/notification.ex b/lib/chat_api/messages/notification.ex index 27878f98f..12c08ffb9 100644 --- a/lib/chat_api/messages/notification.ex +++ b/lib/chat_api/messages/notification.ex @@ -17,7 +17,7 @@ defmodule ChatApi.Messages.Notification do end @spec broadcast_to_customer!(Message.t()) :: Message.t() - def broadcast_to_customer!(%Message{} = message) do + def broadcast_to_customer!(%Message{private: false} = message) do message |> Helpers.get_conversation_topic() |> ChatApiWeb.Endpoint.broadcast!("shout", Helpers.format(message)) @@ -25,6 +25,8 @@ defmodule ChatApi.Messages.Notification do message end + def broadcast_to_customer!(message), do: message + @spec broadcast_to_admin!(Message.t()) :: Message.t() def broadcast_to_admin!(%Message{} = message) do message @@ -80,7 +82,7 @@ defmodule ChatApi.Messages.Notification do message end - def notify(%Message{} = message, :conversation_reply_email, _opts) do + def notify(%Message{private: false} = message, :conversation_reply_email, _opts) do Logger.info("Sending notification: :conversation_reply_email") # 20 minutes (TODO: make this configurable?) schedule_in = 20 * 60 @@ -97,7 +99,7 @@ defmodule ChatApi.Messages.Notification do message end - def notify(%Message{} = message, :slack_company_channel, _opts) do + def notify(%Message{private: false} = message, :slack_company_channel, _opts) do Logger.info("Sending notification: :slack_company_channel") Task.start(fn -> @@ -108,7 +110,7 @@ defmodule ChatApi.Messages.Notification do end # TODO: come up with a better name... it's not super clear what `slack_support_channel` means! - def notify(%Message{} = message, :slack_support_channel, _opts) do + def notify(%Message{private: false} = message, :slack_support_channel, _opts) do Logger.info("Sending notification: :slack_support_channel") Task.start(fn -> @@ -118,6 +120,14 @@ defmodule ChatApi.Messages.Notification do message end + def notify(%Message{private: true} = message, type, _opts) do + Logger.debug( + "Skipping notification type #{inspect(type)} for private message #{inspect(message)}" + ) + + message + end + def notify(message, type, _opts) do Logger.error( "Unrecognized notification type #{inspect(type)} for message #{inspect(message)}" diff --git a/lib/chat_api/slack/event.ex b/lib/chat_api/slack/event.ex index 7eda697b9..413caa0ef 100644 --- a/lib/chat_api/slack/event.ex +++ b/lib/chat_api/slack/event.ex @@ -58,7 +58,9 @@ defmodule ChatApi.Slack.Event do primary_reply_authorization <- SlackAuthorizations.get_authorization_by_account(account_id, %{type: "reply"}) do if Slack.Helpers.is_primary_channel?(primary_reply_authorization, slack_channel_id) do - %{ + text + |> Slack.Helpers.parse_message_type_params() + |> Map.merge(%{ "body" => Slack.Helpers.sanitize_slack_message(text, primary_reply_authorization), "conversation_id" => conversation_id, "account_id" => account_id, @@ -70,7 +72,7 @@ defmodule ChatApi.Slack.Event do slack_user_id, conversation.assignee_id ) - } + }) |> Messages.create_and_fetch!() |> Messages.Notification.broadcast_to_customer!() |> Messages.Notification.broadcast_to_admin!() diff --git a/lib/chat_api/slack/helpers.ex b/lib/chat_api/slack/helpers.ex index 27ca7905d..5e861d3ce 100644 --- a/lib/chat_api/slack/helpers.ex +++ b/lib/chat_api/slack/helpers.ex @@ -586,6 +586,7 @@ defmodule ChatApi.Slack.Helpers do |> sanitize_slack_user_ids(access_token) |> sanitize_slack_links() |> sanitize_slack_mailto_links() + |> sanitize_private_note() end @spec get_slack_message_metadata(binary()) :: map() | nil @@ -683,6 +684,27 @@ defmodule ChatApi.Slack.Helpers do end end + @private_note_prefix_v1 ~S(\\ ) + @private_note_prefix_v2 ~S(;; ) + @private_note_prefix_regex_v1 ~r/^\\\\ / + @private_note_prefix_regex_v2 ~r/^;; / + + @spec sanitize_private_note(binary()) :: binary() + def sanitize_private_note(text) do + text + |> String.replace(@private_note_prefix_regex_v1, "") + |> String.replace(@private_note_prefix_regex_v2, "") + end + + @spec parse_message_type_params(binary()) :: map() + def parse_message_type_params(text) do + case text do + @private_note_prefix_v1 <> _note -> %{"private" => true, "type" => "note"} + @private_note_prefix_v2 <> _note -> %{"private" => true, "type" => "note"} + _ -> %{} + end + end + @spec slack_link_to_markdown(binary()) :: binary() def slack_link_to_markdown(text) do text @@ -712,13 +734,8 @@ defmodule ChatApi.Slack.Helpers do # Formatters ##################### - @spec get_message_text(map()) :: binary() - def get_message_text(%{ - conversation: %Conversation{id: conversation_id, customer: %Customer{} = customer}, - message: %Message{body: text}, - authorization: _authorization, - thread: nil - }) do + @spec get_dashboard_conversation_url(binary()) :: binary() + def get_dashboard_conversation_url(conversation_id) do url = System.get_env("BACKEND_URL") || "" base = @@ -728,22 +745,57 @@ defmodule ChatApi.Slack.Helpers do "https://" <> url end - url = base <> "/conversations/all?cid=" <> conversation_id - dashboard = "<#{url}|dashboard>" + "#{base}/conversations/all?cid=#{conversation_id}" + end - # TODO: this isn't always a "customer" -- with proactive messaging, it would be an agent - "*:wave: #{identify_customer(customer)} says*: #{text}" <> - "\n\nReply to this thread to start chatting, or view in the #{dashboard} :rocket:" + @spec get_message_text(map()) :: binary() + def get_message_text(%{ + conversation: %Conversation{customer: %Customer{}} = conversation, + message: %Message{body: text} = message, + authorization: _authorization, + thread: nil + }) do + dashboard_link = "<#{get_dashboard_conversation_url(conversation.id)}|dashboard>" + + primary_message_text = + case message do + %Message{user: %User{} = user} -> + "*:female-technologist: #{Slack.Notification.format_user_name(user)}*: #{text}" + + %Message{customer: %Customer{} = customer} -> + "*:wave: #{identify_customer(customer)}*: #{text}" + + %Message{customer_id: nil, user_id: user_id} when not is_nil(user_id) -> + "*:female-technologist: Agent*: #{text}" + + %Message{customer_id: customer_id, user_id: nil} when not is_nil(customer_id) -> + "*:wave: #{identify_customer(conversation.customer)}*: #{text}" + + _ -> + Logger.error("Unrecognized message format: #{inspect(message)}") + + text + end + + primary_message_text <> + "\n\n" <> + "Reply to this thread to start chatting, or view in the #{dashboard_link} :rocket:" end @slack_chat_write_customize_scope "chat:write.customize" def get_message_text(%{ conversation: %Conversation{} = conversation, - message: %Message{body: text} = message, + message: %Message{body: body} = message, authorization: %SlackAuthorization{} = authorization, thread: %SlackConversationThread{} }) do + text = + case message do + %Message{private: true, type: "note"} -> "\\\\ _#{body}_" + _ -> body + end + if SlackAuthorizations.has_authorization_scope?( authorization, @slack_chat_write_customize_scope diff --git a/lib/chat_api_web/channels/notification_channel.ex b/lib/chat_api_web/channels/notification_channel.ex index cab79a9fd..9f61ca5d3 100644 --- a/lib/chat_api_web/channels/notification_channel.ex +++ b/lib/chat_api_web/channels/notification_channel.ex @@ -95,12 +95,14 @@ defmodule ChatApiWeb.NotificationChannel do @spec broadcast_new_message(Message.t(), any()) :: Message.t() defp broadcast_new_message(%Message{private: true} = message, socket) do - # For private messages, we only need to broadcast back to the admin channel - # (We avoid broadcasting to the customer channel or Slack or email) - # TODO: broadcast to webhooks and internal Slack channel? + # For private messages, we only need to broadcast back to the admin channel, + # the internal Slack channel, and webhooks. (We avoid broadcasting to the + # customer channel or any public Slack channel or email.) broadcast(socket, "shout", Messages.Helpers.format(message)) message + |> Messages.Notification.notify(:slack) + |> Messages.Notification.notify(:webhooks) end defp broadcast_new_message(message, socket) do diff --git a/test/chat_api/slack_test.exs b/test/chat_api/slack_test.exs index 1434619e0..ec6e1fe40 100644 --- a/test/chat_api/slack_test.exs +++ b/test/chat_api/slack_test.exs @@ -869,6 +869,29 @@ defmodule ChatApi.SlackTest do end) end + test "Helpers.sanitize_slack_message/2 removes private note indicator prefixes", + %{account: account} do + authorization = insert(:slack_authorization, account: account) + + assert "reply" = Slack.Helpers.sanitize_slack_message("reply", authorization) + assert "note" = Slack.Helpers.sanitize_slack_message("\\\\ note", authorization) + assert "note" = Slack.Helpers.sanitize_slack_message(~S(\\ note), authorization) + assert "note" = Slack.Helpers.sanitize_slack_message(";; note", authorization) + end + + test "Helpers.parse_message_type_params/1 removes private note indicator prefixes" do + assert Slack.Helpers.parse_message_type_params("reply") == %{} + + assert %{"private" => true, "type" => "note"} = + Slack.Helpers.parse_message_type_params("\\\\ note") + + assert %{"private" => true, "type" => "note"} = + Slack.Helpers.parse_message_type_params(~S(\\ note)) + + assert %{"private" => true, "type" => "note"} = + Slack.Helpers.parse_message_type_params(";; note") + end + test "Helpers.find_slack_user_mentions/1 extracts @mentions in a Slack message" do assert ["<@UABC123>"] = Slack.Helpers.find_slack_user_mentions("Hi <@UABC123>! How can we help you?") diff --git a/test/chat_api_web/controllers/slack_controller_test.exs b/test/chat_api_web/controllers/slack_controller_test.exs index 8f9b238c4..6ce188762 100644 --- a/test/chat_api_web/controllers/slack_controller_test.exs +++ b/test/chat_api_web/controllers/slack_controller_test.exs @@ -106,6 +106,56 @@ defmodule ChatApiWeb.SlackControllerTest do assert body == event_params["text"] end + test "sending a new thread message event to the webhook from the primary channel as a private note", + %{ + conn: conn, + auth: auth, + thread: thread + } do + account_id = thread.account_id + + post(conn, Routes.slack_path(conn, :webhook), %{ + "event" => %{ + "type" => "message", + "text" => ~S(\\ This should be private), + "ts" => "12345", + "thread_ts" => thread.slack_thread_ts, + "channel" => @slack_channel, + "user" => auth.authed_user_id + } + }) + + assert [%{body: body, source: "slack", type: "note", private: true}] = + Messages.list_messages(account_id) + + assert body == "This should be private" + end + + test "sending a new thread message event to the webhook from the primary channel as a private note (alternative)", + %{ + conn: conn, + auth: auth, + thread: thread + } do + account_id = thread.account_id + + post(conn, Routes.slack_path(conn, :webhook), %{ + "event" => %{ + "type" => "message", + "text" => ";; This should be private", + "ts" => "12345", + "thread_ts" => thread.slack_thread_ts, + "channel" => @slack_channel, + "user" => auth.authed_user_id + } + }) + + assert [%{body: body, source: "slack", type: "note", private: true}] = + Messages.list_messages(account_id) + + assert body == "This should be private" + end + test "updates the conversation with the assignee after the first agent reply", %{ conn: conn, auth: auth,