Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Twilio webhook handler #693

Merged
merged 14 commits into from
Apr 1, 2021
14 changes: 14 additions & 0 deletions lib/chat_api/conversations.ex
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,17 @@ defmodule ChatApi.Conversations do
|> Repo.all()
end

@spec find_latest_conversation(binary(), map()) :: Conversation.t()
def find_latest_conversation(account_id, filters) do
Conversation
|> where(^filter_where(filters))
|> where(account_id: ^account_id)
|> where([c], is_nil(c.archived_at))
|> order_by(desc: :inserted_at)
|> first()
|> Repo.one()
end

# Used internally in dashboard
@spec list_recent_by_customer(binary(), binary(), integer()) :: [Conversation.t()]
def list_recent_by_customer(customer_id, account_id, limit \\ 5) do
Expand Down Expand Up @@ -519,6 +530,9 @@ defmodule ChatApi.Conversations do
{"account_id", value}, dynamic ->
dynamic([p], ^dynamic and p.account_id == ^value)

{"source", value}, dynamic ->
dynamic([p], ^dynamic and p.source == ^value)

{_, _}, dynamic ->
# Not a where parameter
dynamic
Expand Down
2 changes: 1 addition & 1 deletion lib/chat_api/conversations/conversation.ex
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ defmodule ChatApi.Conversations.Conversation do
:metadata
])
|> validate_required([:status, :account_id, :customer_id])
|> validate_inclusion(:source, ["chat", "slack", "email"])
|> validate_inclusion(:source, ["chat", "slack", "email", "sms"])
|> put_closed_at()
|> foreign_key_constraint(:account_id)
|> foreign_key_constraint(:customer_id)
Expand Down
53 changes: 53 additions & 0 deletions lib/chat_api/twilio.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ defmodule ChatApi.Twilio do
alias ChatApi.Repo

alias ChatApi.Twilio.TwilioAuthorization
alias ChatApi.Conversations
alias ChatApi.Customers

@spec list_twilio_authorizations() :: [TwilioAuthorization.t()]
def list_twilio_authorizations() do
Expand Down Expand Up @@ -82,6 +84,57 @@ defmodule ChatApi.Twilio do
|> Repo.one()
end

@spec find_or_create_customer_and_conversation(String.t(), String.t()) ::
{:ok, Customers.Customer.t(), Conversations.Conversation.t()}
| {:error, Ecto.Changeset.t()}
def find_or_create_customer_and_conversation(account_id, phone) do
fmterrorf marked this conversation as resolved.
Show resolved Hide resolved
now = DateTime.utc_now()
sms_source = "sms"

case Customers.list_customers(account_id, %{phone: phone}) do
[] ->
with {:ok, customer} <-
Customers.create_customer(%{
phone: phone,
account_id: account_id,
first_seen: now,
last_seen: now
}),
{:ok, conversation} <-
Conversations.create_conversation(%{
account_id: account_id,
customer_id: customer.id,
source: sms_source
}) do
{:ok, customer, conversation}
else
error ->
error
end

[customer] ->
with nil <-
Conversations.find_latest_conversation(account_id, %{
customer_id: customer.id,
source: sms_source
}),
{:ok, conversation} <-
Conversations.create_conversation(%{
account_id: account_id,
customer_id: customer.id,
source: sms_source
}) do
{:ok, customer, conversation}
else
%Conversations.Conversation{} = conversation ->
{:ok, customer, conversation}

error ->
error
end
end
end

defp filter_authorizations_where(params) do
Enum.reduce(params, dynamic(true), fn
{:account_id, value}, dynamic ->
Expand Down
42 changes: 29 additions & 13 deletions lib/chat_api_web/controllers/twilio_controller.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
defmodule ChatApiWeb.TwilioController do
use ChatApiWeb, :controller

alias ChatApi.Messages
alias ChatApi.Twilio
alias ChatApi.Twilio.TwilioAuthorization

Expand Down Expand Up @@ -60,20 +61,35 @@ defmodule ChatApiWeb.TwilioController do
end

@spec webhook(Plug.Conn.t(), map()) :: Plug.Conn.t()
def webhook(conn, payload) do
def webhook(
conn,
%{"AccountSid" => account_sid, "To" => to, "From" => from, "Body" => body} = payload
) do
Logger.debug("Payload from Twilio webhook: #{inspect(payload)}")
# TODO: implement me!
#
# When new SMS message comes in...
# - Check if the receiving number matches one of our `twilio_authorizations`
# - If it does, use that to determine the `account_id` (from the `twilio_authorizations` table)
# Next, find or create a conversation for the account (with `source: "sms"`)
# - First, find customer by phone number (implement `Customers.find_by_phone/2`)
# - If no customer exists, create new customer record and new conversation (with `source: "sms"`)
# - If customer exists, fetch latest open conversation (with `source: "sms"`)
# - If open conversation exists, add message to conversation
# - Otherwise, create new conversation (with `source: "sms"`)
send_resp(conn, 200, "")

with %TwilioAuthorization{account_id: account_id} <-
Twilio.find_twilio_authorization(%{
twilio_account_sid: account_sid,
from_phone_number: to
}),
{:ok, customer, conversation} <-
Twilio.find_or_create_customer_and_conversation(account_id, from),
{:ok, _mesage} <-
Messages.create_message(%{
body: body,
account_id: account_id,
customer_id: customer.id,
conversation_id: conversation.id
}) do
send_resp(conn, 200, "")
else
nil ->
send_resp(conn, 404, "Twilio account not found")
fmterrorf marked this conversation as resolved.
Show resolved Hide resolved

error ->
Logger.error(inspect(error))
send_resp(conn, 500, "")
end
end

@spec verify_authorization(map()) :: :ok | {:error, atom(), any()}
Expand Down
63 changes: 63 additions & 0 deletions test/chat_api/twilio_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
defmodule ChatApi.TwilioTest do
use ChatApi.DataCase, async: true
import ChatApi.Factory
alias ChatApi.Twilio

test "find_or_create_customer_and_conversation/2 returns customer and new conversation when customer exists" do
account = insert(:account)
customer = insert(:customer, account: account)

{:ok, found_customer, found_conversation} =
Twilio.find_or_create_customer_and_conversation(account.id, customer.phone)

assert customer.id == found_customer.id

assert customer.id == found_conversation.customer_id
assert "sms" == found_conversation.source
end

test "find_or_create_customer_and_conversation/2 returns new customer and conversation when customer doesn't exist" do
account = insert(:account)
phone_number = "+18675309"

{:ok, found_customer, found_conversation} =
Twilio.find_or_create_customer_and_conversation(account.id, phone_number)

assert phone_number == found_customer.phone
assert account.id == found_customer.account_id
assert account.id == found_conversation.account_id
end

test "find_or_create_customer_and_conversation/2 returns latest conversations when multiple conversations exist" do
account = insert(:account)
customer = insert(:customer, account: account)

insert(:conversation,
account: account,
customer: customer,
inserted_at: ~N[2020-12-01 00:00:00],
source: "sms"
)

insert(:conversation,
account: account,
customer: customer,
inserted_at: ~N[2020-12-01 00:01:00],
source: "sms"
)

latest_conversation =
insert(:conversation,
account: account,
customer: customer,
inserted_at: ~N[2020-12-01 00:02:00],
source: "sms"
)

{:ok, found_customer, found_conversation} =
Twilio.find_or_create_customer_and_conversation(account.id, "+18675309")

assert latest_conversation.id == found_conversation.id
assert customer.id == found_customer.id
end
end
97 changes: 97 additions & 0 deletions test/chat_api_web/controllers/twilio_controller_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
defmodule ChatApiWeb.TwilioControllerTest do
use ChatApiWeb.ConnCase

import Mock
import ChatApi.Factory

alias ChatApi.Twilio
alias ChatApi.Customers.Customer
alias ChatApi.Messages
alias ChatApi.Messages.Message
alias ChatApi.Conversations.Conversation
alias ChatApi.Twilio.TwilioAuthorization

setup %{conn: conn} do
account = insert(:account)
user = insert(:user, account: account)
conn = put_req_header(conn, "accept", "application/json")
authed_conn = Pow.Plug.assign_current_user(conn, user, [])

{:ok, conn: conn, authed_conn: authed_conn, account: account}
end

describe "webhook" do
@request_body %{
"AccountSid" => "1234",
"To" => "1234",
"From" => "1234",
"Body" => "body"
}

test "returns 200 when message is successfuly created",
%{authed_conn: authed_conn} do
with_mocks([
Copy link
Contributor Author

@fmterrorf fmterrorf Mar 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've decided to use mocks when testing in controller so that I don't have to insert actual data to the db. It seems like this makes sense because it separates the concerns between the controller vs context. The context should do all the business logic and the controller is responsible the web layer. Lmk what y'all think

{
Twilio,
[],
[
find_twilio_authorization: fn _ ->
%TwilioAuthorization{account_id: 1}
end,
find_or_create_customer_and_conversation: fn _, __ ->
{:ok, %Customer{id: 1}, %Conversation{id: 1}}
end
]
},
{
Messages,
[],
create_message: fn _ -> {:ok, %Message{id: 1}} end
}
]) do
conn = post(authed_conn, Routes.twilio_path(authed_conn, :webhook), @request_body)

assert response(conn, 200)
end
end

test "returns 404 when twilio account is not found",
%{authed_conn: authed_conn} do
with_mocks([
{
Twilio,
[],
[
find_twilio_authorization: fn _ -> nil end
]
}
]) do
conn = post(authed_conn, Routes.twilio_path(authed_conn, :webhook), @request_body)

assert response(conn, 404)
end
end

test "returns 500 when unexpected error occurs",
%{authed_conn: authed_conn} do
with_mocks([
{
Twilio,
[],
[
find_twilio_authorization: fn _ ->
%TwilioAuthorization{account_id: 1}
end,
find_or_create_customer_and_conversation: fn _, __ ->
{:error, %Ecto.Changeset{}}
end
]
}
]) do
conn = post(authed_conn, Routes.twilio_path(authed_conn, :webhook), @request_body)

assert response(conn, 500)
end
end
end
end