Skip to content

Commit

Permalink
Merge pull request #1750 from tactilenews/1739_programatically_handle…
Browse files Browse the repository at this point in the history
…_twilio_63016_errors
  • Loading branch information
mattwr18 authored Dec 5, 2023
2 parents 757b8a1 + 5a58fbd commit 605d702
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 27 deletions.
16 changes: 8 additions & 8 deletions app/adapters/whats_app_adapter/twilio_outbound.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def send!(message)
if freeform_message_permitted?(recipient)
send_message(recipient, message)
else
send_message_template(recipient, message)
send_message_template!(recipient, message)
end
end

Expand Down Expand Up @@ -50,6 +50,13 @@ def send_resubscribe_error_message!(contributor)
text: I18n.t('adapter.shared.resubscribe.failure'))
end

def send_message_template!(recipient, message)
recipient.update(whats_app_message_template_sent_at: Time.current)
text = I18n.t("adapter.whats_app.request_template.new_request_#{time_of_day}_#{rand(1..3)}", first_name: recipient.first_name,
request_title: message.request.title)
WhatsAppAdapter::Outbound::Text.perform_later(contributor_id: recipient.id, text: text)
end

private

def contributor_can_receive_messages?(recipient)
Expand Down Expand Up @@ -83,13 +90,6 @@ def freeform_message_permitted?(recipient)
responding_to_template_message || latest_message_received_within_last_24_hours
end

def send_message_template(recipient, message)
recipient.update(whats_app_message_template_sent_at: Time.current)
text = I18n.t("adapter.whats_app.request_template.new_request_#{time_of_day}_#{rand(1..3)}", first_name: recipient.first_name,
request_title: message.request.title)
WhatsAppAdapter::Outbound::Text.perform_later(contributor_id: recipient.id, text: text)
end

def send_message(recipient, message)
files = message.files

Expand Down
47 changes: 33 additions & 14 deletions app/controllers/whats_app/webhook_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ class WebhookController < ApplicationController
include WhatsAppHandleCallbacks

skip_before_action :require_login, :verify_authenticity_token
before_action :set_contributor, only: :status

UNSUCCESSFUL_DELIVERY = %w[undelivered failed].freeze
INVALID_MESSAGE_RECIPIENT_ERROR_CODE = 63_024 # https://www.twilio.com/docs/api/errors/63024
FREEFORM_MESSAGE_NOT_ALLOWED_ERROR_CODE = 63_016 # https://www.twilio.com/docs/api/errors/63016

def message
head :ok
Expand Down Expand Up @@ -61,20 +64,7 @@ def errors

def status
head :ok
return unless status_params['MessageStatus'].in?(UNSUCCESSFUL_DELIVERY)

whats_app_phone_number = status_params['To'].split('whatsapp:').last
contributor = Contributor.find_by(whats_app_phone_number: whats_app_phone_number)
return unless contributor

if status_params['ErrorCode'].to_i.eql?(INVALID_MESSAGE_RECIPIENT_ERROR_CODE)
MarkInactiveContributorInactiveJob.perform_later(contributor_id: contributor.id)
return
end
exception = WhatsAppAdapter::MessageDeliveryUnsuccessfulError.new(status: status_params['MessageStatus'],
whats_app_phone_number: whats_app_phone_number,
message: status_params['ErrorMessage'])
ErrorNotifier.report(exception, context: { message_sid: status_params['MessageSid'] })
handle_unsuccessful_delivery if status_params['MessageStatus'].in?(UNSUCCESSFUL_DELIVERY)
end

private
Expand All @@ -95,6 +85,11 @@ def status_params
:EventType, :From, :MessageSid, :MessageStatus, :SmsSid, :SmsStatus, :StructuredMessage, :To)
end

def set_contributor
whats_app_phone_number = status_params['To'].split('whatsapp:').last
@contributor = Contributor.find_by(whats_app_phone_number: whats_app_phone_number)
end

def handle_unknown_contributor(whats_app_phone_number)
exception = WhatsAppAdapter::UnknownContributorError.new(whats_app_phone_number: whats_app_phone_number)
ErrorNotifier.report(exception)
Expand Down Expand Up @@ -124,5 +119,29 @@ def fetch_message_from_twilio(twilio_message_sid)
ErrorNotifier.report(e)
nil
end

def handle_freeform_message_not_allowed_error(contributor, twilio_message_sid)
message_text = fetch_message_from_twilio(twilio_message_sid)
message = Message.find_by(text: message_text)
return unless message

WhatsAppAdapter::TwilioOutbound.send_message_template!(contributor, message)
end

def handle_unsuccessful_delivery
return unless @contributor

if status_params['ErrorCode'].to_i.eql?(INVALID_MESSAGE_RECIPIENT_ERROR_CODE)
MarkInactiveContributorInactiveJob.perform_later(contributor_id: @contributor.id)
return
end
if status_params['ErrorCode'].to_i.eql?(FREEFORM_MESSAGE_NOT_ALLOWED_ERROR_CODE) && status_params['MessageStatus'].eql?('failed')
handle_freeform_message_not_allowed_error(@contributor, status_params['MessageSid'])
end
exception = WhatsAppAdapter::MessageDeliveryUnsuccessfulError.new(status: status_params['MessageStatus'],
whats_app_phone_number: @contributor.whats_app_phone_number,
message: status_params['ErrorMessage'])
ErrorNotifier.report(exception, context: { message_sid: status_params['MessageSid'] })
end
end
end
62 changes: 57 additions & 5 deletions spec/requests/whats_app/webhook_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@
'ChannelInstallSid' => 'someChannelInstallSid',
'ChannelPrefix' => 'whatsapp',
'ChannelToAddress' => whats_app_phone_number.to_s,
'ErrorCode' => '63016',
'ErrorMessage' => freeform_message_not_allowed_error_message,
'ErrorCode' => '60228',
'ErrorMessage' => 'Template was not found',
'From' => "whatsapp:#{Setting.whats_app_server_phone_number}",
'MessageSid' => 'someSid',
'MessageStatus' => 'failed',
Expand All @@ -213,9 +213,7 @@
'To' => "whatsapp:#{whats_app_phone_number}"
}
end
let(:freeform_message_not_allowed_error_message) do
'Twilio Error: Failed to send freeform message because you are outside the allowed window.. Generated new message with sid: someSid'
end

let(:exception) do
WhatsAppAdapter::MessageDeliveryUnsuccessfulError.new(status: params['MessageStatus'],
whats_app_phone_number: whats_app_phone_number, message: params['ErrorMessage'])
Expand Down Expand Up @@ -286,6 +284,60 @@
end
}
end

context 'due to a freeform message not allowed error' do
let!(:request) do
create(:request, title: 'I failed to send', text: 'Hey {{FIRST_NAME}}, because it was sent outside the allowed window')
end
let(:valid_account_sid) { 'VALID_ACCOUNT_SID' }
let(:valid_api_key_sid) { 'VALID_API_KEY_SID' }
let(:valid_api_key_secret) { 'VALID_API_KEY_SECRET' }
let(:mock_twilio_rest_client) { instance_double(Twilio::REST::Client) }
let(:messages_double) { double(Twilio::REST::Api::V2010::AccountContext::MessageInstance, body: body_text) }
let(:body_text) { 'no message with this text saved' }
let(:freeform_message_not_allowed_error_message) do
'Twilio Error: Failed to send freeform message because you are outside the allowed window.'
end

before do
allow(Twilio::REST::Client).to receive(:new).and_return(mock_twilio_rest_client)
allow(mock_twilio_rest_client).to receive(:messages).with('someSid').and_return(messages_double)
allow(messages_double).to receive(:fetch).and_return(messages_double)
params['ErrorCode'] = '63016'
params['ErrorMessage'] = freeform_message_not_allowed_error_message
end

it 'reports it as an error, as we want to track specifics to when this occurs' do
expect(Sentry).to receive(:capture_exception).with(exception)

subject.call
end

it 'given message cannot be found by Twilio message sid body' do
expect { subject.call }.not_to have_enqueued_job(WhatsAppAdapter::Outbound::Text)
expect { subject.call }.not_to raise_error
end

describe 'given a message is found by Twilio message sid body' do
let!(:message) { create(:message, text: body_text, request: request) }
let(:body_text) { "Hey #{contributor.first_name}, because it was sent outside the allowed window" }

it 'enqueues the Text job with WhatsApp template' do
expect { subject.call }.to(have_enqueued_job(WhatsAppAdapter::Outbound::Text).on_queue('default').with do |params|
expect(params[:contributor_id]).to eq(contributor.id)
expect(params[:text]).to include(contributor.first_name)
expect(params[:text]).to include(message.request.title)
end)
end

context 'given an undelivered status' do
before { params['MessageStatus'] = 'undelivered' }
it 'is expected not to schedule a job as it would send out twice, one for undelivered and one for failed' do
expect { subject.call }.not_to have_enqueued_job(WhatsAppAdapter::Outbound::Text)
end
end
end
end
end
end
end
Expand Down

0 comments on commit 605d702

Please sign in to comment.