Make it possible to sync an entire Slack thread (#561)

* Improvements to how we handle syncing replies to bot messages (WIP)

* Get everything working with syncing slack threads (WIP)

* Play around with shuffling things around (WIP)

assets/src/components/conversations/ConversationsProvider.tsx

@@ -4,7 +4,11 @@ import {throttle} from 'lodash';
 import * as API from '../../api';
 import {notification} from '../common';
 import {Account, Conversation, Message, User} from '../../types';
-import {isWindowHidden, updateQueryParams} from '../../utils';
+import {
+  isWindowHidden,
+  sortConversationMessages,
+  updateQueryParams,
+} from '../../utils';
 import {SOCKET_URL} from '../../socket';
 import logger from '../../logger';
 
@@ -523,10 +527,7 @@ export class ConversationsProvider extends React.Component<Props, State> {
 
         return {
           ...acc,
-          [conv.id]: messages.sort(
-            (a: Message, b: Message) =>
-              +new Date(a.created_at) - +new Date(b.created_at)
-          ),
+          [conv.id]: sortConversationMessages(messages),
         };
       },
       {}

assets/src/components/conversations/SharedConversation.tsx

@@ -4,6 +4,7 @@ import {Box, Flex} from 'theme-ui';
 import qs from 'query-string';
 import {colors} from '../common';
 import ConversationMessages from './ConversationMessages';
+import {sortConversationMessages} from '../../utils';
 import * as API from '../../api';
 import {Message} from '../../types';
 import logger from '../../logger';
@@ -35,10 +36,7 @@ class SharedConversationContainer extends React.Component<Props, State> {
       );
 
       this.setState({
-        messages: messages.sort(
-          (a: Message, b: Message) =>
-            +new Date(a.created_at) - +new Date(b.created_at)
-        ),
+        messages: sortConversationMessages(messages),
         loading: false,
       });
     } catch (err) {

assets/src/utils.ts

@@ -1,5 +1,9 @@
 import dayjs from 'dayjs';
+import utc from 'dayjs/plugin/utc';
 import qs from 'query-string';
+import {Message} from './types';
+
+dayjs.extend(utc);
 
 const {REACT_APP_STRIPE_PUBLIC_KEY} = process.env;
 
@@ -94,6 +98,22 @@ export const isValidUuid = (id: any) => {
   return regex.test(id);
 };
 
+export const sortConversationMessages = (messages: Array<Message>) => {
+  return messages.sort((a: Message, b: Message) => {
+    // NB: `created_at` is stored as UTC implicitly, whereas `sent_at` is stored
+    // as UTC explicitly. This means that we have to convert `created_at` to a
+    // UTC date on the frontend first in order to compare the two properly.
+    const dateA = a.sent_at
+      ? new Date(a.sent_at)
+      : dayjs.utc(a.created_at).toDate();
+    const dateB = b.sent_at
+      ? new Date(b.sent_at)
+      : dayjs.utc(b.created_at).toDate();
+
+    return +dateA - +dateB;
+  });
+};
+
 export const updateQueryParams = (query: Record<any, any>) => {
   if (window.history.pushState) {
     window.history.pushState(

lib/chat_api/customers.ex

@@ -58,6 +58,44 @@ defmodule ChatApi.Customers do
     external_id |> to_string() |> find_by_external_id(account_id, filters)
   end
 
+  @spec find_or_create_by_external_id(binary() | nil, binary(), map()) ::
+          {:ok, Customer.t()} | {:error, Ecto.Changeset.t()} | {:error, atom()}
+  def find_or_create_by_external_id(external_id, account_id, attrs \\ %{})
+  def find_or_create_by_external_id(nil, _account_id, _attrs), do: {:error, :external_id_required}
+
+  def find_or_create_by_external_id(external_id, account_id, attrs) do
+    case find_by_external_id(external_id, account_id) do
+      nil ->
+        get_default_params()
+        |> Map.merge(attrs)
+        |> Map.merge(%{external_id: external_id, account_id: account_id})
+        |> create_customer()
+
+      customer ->
+        {:ok, customer}
+    end
+  end
+
+  @spec create_or_update_by_external_id(binary() | nil, binary(), map()) ::
+          {:ok, Customer.t()} | {:error, Ecto.Changeset.t()} | {:error, atom()}
+  def create_or_update_by_external_id(external_id, account_id, attrs \\ %{})
+
+  def create_or_update_by_external_id(nil, _account_id, _attrs),
+    do: {:error, :external_id_required}
+
+  def create_or_update_by_external_id(external_id, account_id, attrs) do
+    case find_by_external_id(external_id, account_id) do
+      nil ->
+        get_default_params()
+        |> Map.merge(attrs)
+        |> Map.merge(%{external_id: external_id, account_id: account_id})
+        |> create_customer()
+
+      customer ->
+        update_customer(customer, attrs)
+    end
+  end
+
   @spec find_by_email(binary() | nil, binary()) :: Customer.t() | nil
   def find_by_email(nil, _account_id), do: nil
 
@@ -77,14 +115,7 @@ defmodule ChatApi.Customers do
   def find_or_create_by_email(email, account_id, attrs) do
     case find_by_email(email, account_id) do
       nil ->
-        %{
-          # Defaults
-          first_seen: DateTime.utc_now(),
-          last_seen: DateTime.utc_now(),
-          # TODO: last_seen is stored as a date, while last_seen_at is stored as
-          # a datetime -- we should opt for datetime values whenever possible
-          last_seen_at: DateTime.utc_now()
-        }
+        get_default_params()
         |> Map.merge(attrs)
         |> Map.merge(%{email: email, account_id: account_id})
         |> create_customer()
@@ -102,14 +133,7 @@ defmodule ChatApi.Customers do
   def create_or_update_by_email(email, account_id, attrs) do
     case find_by_email(email, account_id) do
       nil ->
-        %{
-          # Defaults
-          first_seen: DateTime.utc_now(),
-          last_seen: DateTime.utc_now(),
-          # TODO: last_seen is stored as a date, while last_seen_at is stored as
-          # a datetime -- we should opt for datetime values whenever possible
-          last_seen_at: DateTime.utc_now()
-        }
+        get_default_params()
         |> Map.merge(attrs)
         |> Map.merge(%{email: email, account_id: account_id})
         |> create_customer()
@@ -141,6 +165,22 @@ defmodule ChatApi.Customers do
     |> Repo.update()
   end
 
+  # Ideally these would be set at the DB level, but this should be fine for now
+  @spec get_default_params(map()) :: map()
+  def get_default_params(overrides \\ %{}) do
+    Map.merge(
+      %{
+        # Defaults
+        first_seen: DateTime.utc_now(),
+        last_seen: DateTime.utc_now(),
+        # TODO: last_seen is stored as a date, while last_seen_at is stored as
+        # a datetime -- we should opt for datetime values whenever possible
+        last_seen_at: DateTime.utc_now()
+      },
+      overrides
+    )
+  end
+
   # TODO: figure out if any of this can be done in the changeset, or if there's
   # a better way to handle this in general
   @spec sanitize_metadata(map()) :: map()

lib/chat_api/messages/notification.ex

@@ -34,21 +34,33 @@ defmodule ChatApi.Messages.Notification do
     message
   end
 
-  @spec notify(Message.t(), atom()) :: Message.t()
+  @spec notify(Message.t(), atom(), keyword()) :: Message.t()
+  def notify(message, type, opts \\ [])
+
   def notify(
         %Message{body: _body, conversation_id: _conversation_id} = message,
-        :slack
+        :slack,
+        opts
       ) do
     Logger.info("Sending notification: :slack")
 
-    Task.start(fn ->
-      ChatApi.Slack.Notification.notify_primary_channel(message)
-    end)
+    case opts do
+      [metadata: %{"send_to_reply_channel" => false}] ->
+        nil
+
+      [async: false] ->
+        ChatApi.Slack.Notification.notify_primary_channel(message)
+
+      _ ->
+        Task.start(fn ->
+          ChatApi.Slack.Notification.notify_primary_channel(message)
+        end)
+    end
 
     message
   end
 
-  def notify(%Message{account_id: account_id} = message, :webhooks) do
+  def notify(%Message{account_id: account_id} = message, :webhooks, _opts) do
     Logger.info("Sending notification: :webhooks")
     # TODO: how should we handle errors/retry logic?
     Task.start(fn ->
@@ -58,7 +70,7 @@ defmodule ChatApi.Messages.Notification do
     message
   end
 
-  def notify(%Message{} = message, :new_message_email) do
+  def notify(%Message{} = message, :new_message_email, _opts) do
     Logger.info("Sending notification: :new_message_email")
     # TODO: how should we handle errors/retry logic?
     Task.start(fn ->
@@ -68,7 +80,7 @@ defmodule ChatApi.Messages.Notification do
     message
   end
 
-  def notify(%Message{} = message, :conversation_reply_email) do
+  def notify(%Message{} = message, :conversation_reply_email, _opts) do
     Logger.info("Sending notification: :conversation_reply_email")
     # 20 minutes (TODO: make this configurable?)
     schedule_in = 20 * 60
@@ -85,7 +97,7 @@ defmodule ChatApi.Messages.Notification do
     message
   end
 
-  def notify(%Message{} = message, :slack_company_channel) do
+  def notify(%Message{} = message, :slack_company_channel, _opts) do
     Logger.info("Sending notification: :slack_company_channel")
 
     Task.start(fn ->
@@ -96,7 +108,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) do
+  def notify(%Message{} = message, :slack_support_channel, _opts) do
     Logger.info("Sending notification: :slack_support_channel")
 
     Task.start(fn ->
@@ -106,28 +118,11 @@ defmodule ChatApi.Messages.Notification do
     message
   end
 
-  def notify(message, type) do
+  def notify(message, type, _opts) do
     Logger.error(
       "Unrecognized notification type #{inspect(type)} for message #{inspect(message)}"
     )
 
     message
   end
-
-  @spec notify(Message.t(), atom(), map()) :: Message.t()
-  def notify(%Message{} = message, :slack, metadata) do
-    # NB: we currently use the Slack authorization metadata to handle one-off cases
-    # where a user might not want to broadcast to the "reply"-type Slack integration
-    # under certain circumstances.
-    if send_to_slack_reply_channel?(metadata),
-      do: notify(message, :slack),
-      else: message
-  end
-
-  def notify(%Message{} = message, type, _metadata) do
-    notify(message, type)
-  end
-
-  defp send_to_slack_reply_channel?(%{"send_to_reply_channel" => false}), do: false
-  defp send_to_slack_reply_channel?(_), do: true
 end

lib/chat_api/slack.ex

@@ -2,4 +2,53 @@ defmodule ChatApi.Slack do
   @moduledoc """
   A module to handle sending Slack notifications.
   """
+
+  # TODO: play around with using structs to format Slack API responses and webhook events
+
+  defmodule Message do
+    @enforce_keys [:text]
+
+    defstruct [
+      :blocks,
+      :bot_id,
+      :client_msg_id,
+      :subtype,
+      :team,
+      :thread_ts,
+      :ts,
+      :user,
+      text: "",
+      type: "message"
+    ]
+
+    def from_json(json) do
+      params = Map.new(json, fn {key, value} -> {String.to_atom(key), value} end)
+
+      struct(Message, params)
+    end
+  end
+
+  defmodule MessageEvent do
+    defstruct [
+      :blocks,
+      :bot_id,
+      :client_msg_id,
+      :channel,
+      :channel_type,
+      :event_ts,
+      :subtype,
+      :team,
+      :thread_ts,
+      :ts,
+      :user,
+      text: "",
+      type: "message"
+    ]
+
+    def from_json(json) do
+      params = Map.new(json, fn {key, value} -> {String.to_atom(key), value} end)
+
+      struct(MessageEvent, params)
+    end
+  end
 end

lib/chat_api/slack/client.ex

@@ -104,6 +104,22 @@ defmodule ChatApi.Slack.Client do
     end
   end
 
+  @spec retrieve_bot_info(binary(), binary()) :: {:ok, nil} | Tesla.Env.result()
+  def retrieve_bot_info(bot_id, access_token) do
+    if should_execute?(access_token) do
+      get("/bots.info",
+        query: [bot: bot_id],
+        headers: [
+          {"Authorization", "Bearer " <> access_token}
+        ]
+      )
+    else
+      Logger.info("Invalid access token")
+
+      {:ok, nil}
+    end
+  end
+
   @spec list_channels(binary()) :: {:ok, nil} | Tesla.Env.result()
   def list_channels(access_token) do
     # TODO: we need channels:read scope to access this