Add Twilio webhook handler #693
Conversation
|
|
||
| test "returns 200 when message is successfuly created", | ||
| %{authed_conn: authed_conn} do | ||
| with_mocks([ |
There was a problem hiding this comment.
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
|
cc @reichert621 |
|
I noticed we changed some of the same files. I'm fine to handle conflicts since my PR is newer! |
|
Rebase troubles? 😛 |
Haha :D yeah i rebased to master 🤦 TIL about |
|
I did the exact same thing the other day but I caught it before force-pushing so I only had to hard reset to origin |
|
Nice. I'm so used to merge commits 😄 Trying to get a hold of the rebase workflow |
There was a problem hiding this comment.
nice @fmterrorf 🎉 thanks for writing all these tests too :)
just a couple comments, but looks great so far!
| now = DateTime.utc_now() | ||
| sms_source = "sms" | ||
|
|
||
| case Customers.list_customers(account_id, %{phone: phone}) do |
There was a problem hiding this comment.
might be nice to set up a Customers.find_or_create_by_phone method to abstract over some of this logic -- what do you think?
There was a problem hiding this comment.
If if I create a Customers.find_or_create_by_phone, It will always return a customer.
The current code right contains this logic
if the customer is not present
creates a customer + conversation
return {customer, conversation}
else
find conversation
if no conversation found
create one
else
return {customer, conversation}
If the customer is not present it creates the customer + conversation right away, without having to do a find by conversation
There was a problem hiding this comment.
hmm so wouldn't it still work if Customers.find_or_create_by_phone just indicated whether the returned customer was found or created?
maybe something like...
case Customers.find_or_create_by_phone(account, phone) do
{:ok, :found, customer} ->
case Conversations.find_latest_conversation_by_customer(customer.id) do
nil -> Conversations.create_conversation(...)
conversation -> {:ok, conversation}
end
{:ok, :created, customer} ->
Conversations.create_conversation(...)
error ->
error
enddoes that make sense?
There was a problem hiding this comment.
Yeah that makes sense!
* Added logic * Added tests for context * Added controller tests * Pattern match param * Removed whitespace * Add inet6 back * Add typespec * Wrap test with describe * Moved find_or_create_customer_and_conversation to Conversations context * Mix format * Cleanup typespec * Log if twilio account is not found * Log warn message if twilio account is not found * Fix twilio controller test
* Added logic * Added tests for context * Added controller tests * Pattern match param * Removed whitespace * Add inet6 back * Add typespec * Wrap test with describe * Moved find_or_create_customer_and_conversation to Conversations context * Mix format * Cleanup typespec * Log if twilio account is not found * Log warn message if twilio account is not found * Fix twilio controller test
* Set up Twilio authorization flow (#671) * Set up boilerplate for Twilio client * Add migration for twilio authorizations * Set up boilerplate UI for twilio authorization flow * Improve error handling * Add todos, fix warning * Hide todo comment * Send SMS notification via twilio (#695) * Send SMS notification via twilio * Log error if twilio sms fails * Switch error to info * Remove unused from test * Add Twilio webhook handler (#693) * Added logic * Added tests for context * Added controller tests * Pattern match param * Removed whitespace * Add inet6 back * Add typespec * Wrap test with describe * Moved find_or_create_customer_and_conversation to Conversations context * Mix format * Cleanup typespec * Log if twilio account is not found * Log warn message if twilio account is not found * Fix twilio controller test * Fix tests * Improve logging a bit Co-authored-by: Andy Tran <[email protected]> Co-authored-by: Daven <[email protected]>
Description
Issue
Fixes #675
Screenshot
Here's a screenshot of a new conversation
I used ngrok to test this locally
Checklist
mix testmix format