Support specifying customer in POST /api/v1/messages endpoint (#723)

* Support specifying a customer_id in POST /api/v1/messages

* Improve error handling

lib/chat_api/customers.ex

@@ -45,7 +45,7 @@ defmodule ChatApi.Customers do
     |> Repo.paginate(pagination_params)
   end
 
-  @spec get_customer!(binary(), atom() | list(atom()) | keyword()) :: Customer.t() | nil
+  @spec get_customer!(binary(), atom() | list(atom()) | keyword()) :: Customer.t()
   def get_customer!(id, preloads \\ [:company, :tags]) do
     Customer
     |> Repo.get!(id)

lib/chat_api_web/controllers/fallback_controller.ex

@@ -35,4 +35,26 @@ defmodule ChatApiWeb.FallbackController do
       }
     })
   end
+
+  def call(conn, {:error, :unprocessable_entity, message}) do
+    conn
+    |> put_status(422)
+    |> json(%{
+      error: %{
+        status: 422,
+        message: message
+      }
+    })
+  end
+
+  def call(conn, {:error, :forbidden, message}) do
+    conn
+    |> put_status(403)
+    |> json(%{
+      error: %{
+        status: 403,
+        message: message
+      }
+    })
+  end
 end

lib/chat_api_web/controllers/message_controller.ex

@@ -7,13 +7,13 @@ defmodule ChatApiWeb.MessageController do
 
   action_fallback(ChatApiWeb.FallbackController)
 
-  plug :authorize when action in [:show, :update, :delete]
+  plug(:authorize when action in [:show, :update, :delete])
 
   defp authorize(conn, _) do
     id = conn.path_params["id"]
 
-    with %{id: user_id, account_id: account_id} <- conn.assigns.current_user,
-         message = %{account_id: ^account_id, user_id: ^user_id} <- Messages.get_message!(id) do
+    with %{id: _user_id, account_id: account_id} <- conn.assigns.current_user,
+         message = %{account_id: ^account_id} <- Messages.get_message!(id) do
       assign(conn, :current_message, message)
     else
       _ -> ChatApiWeb.FallbackController.call(conn, {:error, :not_found}) |> halt()
@@ -92,11 +92,8 @@ defmodule ChatApiWeb.MessageController do
 
   @spec create(Plug.Conn.t(), map()) :: Plug.Conn.t()
   def create(conn, %{"message" => message_params}) do
-    with %{id: user_id, account_id: account_id} <- conn.assigns.current_user,
-         {:ok, %Message{} = msg} <-
-           message_params
-           |> Map.merge(%{"user_id" => user_id, "account_id" => account_id})
-           |> Messages.create_message(),
+    with {:ok, params} <- sanitize_new_message_params(conn, message_params),
+         {:ok, %Message{} = msg} <- Messages.create_message(params),
          message <-
            Messages.get_message!(msg.id) do
       broadcast_new_message(message)
@@ -146,6 +143,41 @@ defmodule ChatApiWeb.MessageController do
     end
   end
 
+  @spec sanitize_new_message_params(Plug.Conn.t(), map()) ::
+          {:ok, map()} | {:error, atom(), binary()}
+  defp sanitize_new_message_params(
+         _conn,
+         %{"customer_id" => customer_id, "user_id" => user_id}
+       )
+       when is_binary(customer_id) and not is_nil(user_id) do
+    {:error, :unprocessable_entity,
+     "A message should not have both a `user_id` and `customer_id`"}
+  end
+
+  defp sanitize_new_message_params(conn, %{"customer_id" => customer_id} = params)
+       when is_binary(customer_id) do
+    account_id = conn.assigns.current_user.account_id
+    customer = ChatApi.Customers.get_customer!(customer_id, [])
+
+    case customer do
+      %{account_id: ^account_id} ->
+        {:ok, Map.merge(params, %{"account_id" => account_id})}
+
+      _ ->
+        {:error, :forbidden, "Forbidden: invalid `customer_id`"}
+    end
+  end
+
+  defp sanitize_new_message_params(conn, params) do
+    case conn.assigns.current_user do
+      %{id: user_id, account_id: account_id} ->
+        {:ok, Map.merge(params, %{"user_id" => user_id, "account_id" => account_id})}
+
+      _ ->
+        {:error, :unauthorized, "Access denied"}
+    end
+  end
+
   defp broadcast_new_message(message) do
     message
     |> Messages.Notification.broadcast_to_customer!()

test/chat_api_web/controllers/message_controller_test.exs

@@ -24,7 +24,8 @@ defmodule ChatApiWeb.MessageControllerTest do
      authed_conn: authed_conn,
      account: account,
      message: message,
-     conversation: conversation}
+     conversation: conversation,
+     user: user}
   end
 
   describe "index" do
@@ -59,13 +60,131 @@ defmodule ChatApiWeb.MessageControllerTest do
       assert %{"id" => id} = json_response(conn, 201)["data"]
 
       conn = get(authed_conn, Routes.message_path(authed_conn, :show, id))
+      user_id = authed_conn.assigns.current_user.id
+      account_id = authed_conn.assigns.current_user.account_id
 
       assert %{
                "id" => _id,
                "object" => "message",
-               "body" => "some body"
+               "body" => "some body",
+               "user_id" => ^user_id,
+               "account_id" => ^account_id
              } = json_response(conn, 200)["data"]
     end
+
+    test "defaults to the authed user's id and account_id when none are specified",
+         %{
+           authed_conn: authed_conn,
+           conversation: conversation
+         } do
+      message = %{
+        body: "some body",
+        conversation_id: conversation.id
+      }
+
+      conn = post(authed_conn, Routes.message_path(authed_conn, :create), message: message)
+      assert %{"id" => id} = json_response(conn, 201)["data"]
+
+      conn = get(authed_conn, Routes.message_path(authed_conn, :show, id))
+      user_id = authed_conn.assigns.current_user.id
+      account_id = authed_conn.assigns.current_user.account_id
+
+      assert %{
+               "id" => _id,
+               "object" => "message",
+               "body" => "some body",
+               "user_id" => ^user_id,
+               "account_id" => ^account_id
+             } = json_response(conn, 200)["data"]
+    end
+
+    test "renders message when customer ID is valid",
+         %{
+           authed_conn: authed_conn,
+           account: account,
+           conversation: conversation
+         } do
+      customer = insert(:customer, account: account)
+
+      message = %{
+        body: "some body",
+        account_id: account.id,
+        customer_id: customer.id,
+        conversation_id: conversation.id
+      }
+
+      conn = post(authed_conn, Routes.message_path(authed_conn, :create), message: message)
+      assert %{"id" => id} = json_response(conn, 201)["data"]
+
+      conn = get(authed_conn, Routes.message_path(authed_conn, :show, id))
+      customer_id = customer.id
+      account_id = authed_conn.assigns.current_user.account_id
+
+      assert %{
+               "id" => _id,
+               "object" => "message",
+               "body" => "some body",
+               "customer_id" => ^customer_id,
+               "account_id" => ^account_id
+             } = json_response(conn, 200)["data"]
+    end
+
+    test "returns error when customer ID is not associated with the authenticated account",
+         %{
+           authed_conn: authed_conn,
+           conversation: conversation
+         } do
+      some_other_account = insert(:account)
+      customer = insert(:customer, account: some_other_account)
+
+      message = %{
+        body: "some body",
+        customer_id: customer.id,
+        conversation_id: conversation.id
+      }
+
+      conn = post(authed_conn, Routes.message_path(authed_conn, :create), message: message)
+      assert json_response(conn, 403)["error"]["message"]
+    end
+
+    test "returns error when both a user ID and customer ID are specified",
+         %{
+           authed_conn: authed_conn,
+           account: account,
+           conversation: conversation
+         } do
+      customer = insert(:customer, account: account)
+
+      message = %{
+        body: "some body",
+        account_id: account.id,
+        customer_id: customer.id,
+        user_id: authed_conn.assigns.current_user.id,
+        conversation_id: conversation.id
+      }
+
+      conn = post(authed_conn, Routes.message_path(authed_conn, :create), message: message)
+      assert json_response(conn, 422)["error"]["message"]
+    end
+
+    test "returns error when connection is not authorized",
+         %{
+           conn: conn,
+           account: account,
+           conversation: conversation
+         } do
+      customer = insert(:customer, account: account)
+
+      message = %{
+        body: "some body",
+        account_id: account.id,
+        customer_id: customer.id,
+        conversation_id: conversation.id
+      }
+
+      conn = post(conn, Routes.message_path(conn, :create), message: message)
+      assert json_response(conn, 401)["error"]["message"]
+    end
   end
 
   describe "update message" do