Skip to content

Commit

Permalink
Handle unsuccessful deliveries (#2077)
Browse files Browse the repository at this point in the history
Fixes #2076  

### What changed and Why

This PR extends the same feature we had with Twilio to 360dialog, we:

* Look out for statuses that show failed message delivery that have an
error code of 131_026, which indicates that we cannot message this
recipient for one of several reasons. See issue for a list of them from
Meta.
* When we get this error, we mark the contributor as inactive since we
cannot send them any messages,
* Then, we create a notification to make this visible to the users
* Also, we add some text on the contributor show page with this
information ☝️ ...
 
This is all an attempt to not silently treat these issues. We want to
bring visibility to who we know cannot receive messages and allow the
clients to decide if they would like to work with the contributor to
resolve the issue.
  • Loading branch information
mattwr18 authored Nov 20, 2024
1 parent 60ee87f commit fdbe579
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 39 deletions.
18 changes: 14 additions & 4 deletions app/controllers/whats_app/three_sixty_dialog_webhook_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ class ThreeSixtyDialogWebhookController < ApplicationController
skip_before_action :require_login, :verify_authenticity_token, :user_permitted?
before_action :extract_components, only: :message

UNSUCCESSFUL_DELIVERY = %w[undelivered failed].freeze
INVALID_MESSAGE_RECIPIENT_ERROR_CODE = 131_026 # https://docs.360dialog.com/docs/useful/api-error-message-list#type-message-undeliverable

def message
head :ok
handle_statuses and return if @components[:statuses].present? # TODO: Handle statuses
handle_statuses and return if @components[:statuses].present?
handle_errors(@components[:errors]) and return if @components[:errors].present?

WhatsAppAdapter::ThreeSixtyDialog::ProcessWebhookJob.perform_later(organization_id: @organization.id, components: @components)
Expand Down Expand Up @@ -49,15 +52,22 @@ def message_params
def handle_statuses
statuses = @components[:statuses]
statuses.each do |status|
handle_errors(status[:errors]) if status[:errors]
invalid_recipient_error = status[:errors].select { |error| error[:code].to_i.eql?(INVALID_MESSAGE_RECIPIENT_ERROR_CODE) }
mark_inactive_contributor_inactive(status) if invalid_recipient_error.present?
handle_errors(status[:errors]) if status[:status].in?(UNSUCCESSFUL_DELIVERY)
end
end

def handle_errors(errors)
errors.each do |error|
exception = WhatsAppAdapter::ThreeSixtyDialogError.new(error_code: error[:code], message: error[:title])
ErrorNotifier.report(exception, context: { details: error[:error_data][:details] })
exception = WhatsAppAdapter::ThreeSixtyDialogError.new(error_code: error[:code], message: error[:message])
ErrorNotifier.report(exception, context: { details: error[:error_data][:details], title: error[:title] })
end
end

def mark_inactive_contributor_inactive(status)
contributor = @organization.contributors.find_by(whats_app_phone_number: "+#{status[:recipient_id]}")
MarkInactiveContributorInactiveJob.perform_later(organization_id: @organization.id, contributor_id: contributor.id)
end
end
end
2 changes: 1 addition & 1 deletion config/locales/de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ de:
text: "<strong>%{name}</strong> ist ein <strong>inaktives Mitglied</strong> deiner Community und erhält aktuell keine Recherchefragen über %{project_name}."
possible_reason_whats_app: |
Die Rufnummer wurde möglicherweise nicht bei WhatsApp registriert oder der Empfänger hat die neuen Nutzungsbedingungen und Datenschutzrichtlinien von WhatsApp nicht akzeptiert.
Es ist auch möglich, dass der Empfänger eine alte, nicht unterstützte Version des WhatsApp-Clients für sein Telefon verwendet. Bitte überprüfe dies mit<strong>%{first_name}</strong>.
Es ist auch möglich, dass der Empfänger eine alte, nicht unterstützte Version des WhatsApp-Clients für sein Telefon verwendet. Bitte überprüfe dies mit <strong>%{first_name}</strong>.
possible_reason_signal: "Die Rufnummer wurde möglicherweise nicht bei Signal registriert. Bitte überprüfe dies mit <strong>%{first_name}</strong>."
possible_reason_threema: "Diese Threema-ID wurde möglicherweise von Threema als ungültig gemeldet. Bitte <strong>%{first_name}</strong>, die Threema-ID zu überprüfen."
possible_reason_email: "Diese E-Mail-Adresse wurde vom Server als inaktiv gemeldet. Das bedeutet, es können keine E-Mails an diese Adresse zugestellt werden."
Expand Down
120 changes: 86 additions & 34 deletions spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,29 @@
require 'webmock/rspec'

RSpec.describe WhatsApp::ThreeSixtyDialogWebhookController do
let(:organization) do
create(:organization, whats_app_server_phone_number: '+4915133311445', three_sixty_dialog_client_api_key: 'valid_api_key')
end
let(:params) do
{ entry: [{ id: 'some_external_id',
changes: [{ value: {
messaging_product: 'whatsapp',
metadata: { display_phone_number: '4915133311445', phone_number_id: 'some_valid_id' },
contacts: [{ profile: { name: 'Matthew Rider' },
wa_id: '491511234567' }],
messages: [{ from: '491511234567',
id: 'some_valid_id',
text: { body: 'Hey' },
timestamp: '1692118778',
type: 'text' }]
} }] }] }
end
let(:components) { params[:entry].first[:changes].first[:value] }
describe '#messages' do
subject { -> { post organization_whats_app_three_sixty_dialog_webhook_path(organization), params: params } }

subject { -> { post organization_whats_app_three_sixty_dialog_webhook_path(organization), params: params } }
let(:organization) do
create(:organization, whats_app_server_phone_number: '+4915133311445', three_sixty_dialog_client_api_key: 'valid_api_key')
end
let(:params) do
{ entry: [{ id: 'some_external_id',
changes: [{ value: {
messaging_product: 'whatsapp',
metadata: { display_phone_number: '4915133311445', phone_number_id: 'some_valid_id' },
contacts: [{ profile: { name: 'Matthew Rider' },
wa_id: '491511234567' }],
messages: [{ from: '491511234567',
id: 'some_valid_id',
text: { body: 'Hey' },
timestamp: '1692118778',
type: 'text' }]
} }] }] }
end
let(:components) { params[:entry].first[:changes].first[:value] }
let(:exception) { WhatsAppAdapter::ThreeSixtyDialogError.new(error_code: error_code, message: error_message) }

describe '#messages' do
before do
allow(Sentry).to receive(:capture_exception)
end
Expand All @@ -44,37 +45,88 @@
end

describe 'statuses' do
let(:params) do
{
statuses: [{ id: 'some_valid_id',
message: { recipient_id: '491511234567' },
status: 'read',
timestamp: '1691405467',
type: 'message' }]
}
end
context 'unsuccessful delivery' do
context 'failed delivery' do
let(:user) { create(:user, organizations: [organization]) }
context 'message undeliverable' do
let(:failed_status) do
[{
id: 'valid_external_message_id',
status: 'failed',
timestamp: '1731672268',
recipient_id: '49123456789',
errors: [{
code: 131_026,
title: 'Message undeliverable',
message: 'Message undeliverable',
error_data: {
details: 'Message Undeliverable.'
}
}]
}]
end
let(:error_code) { 131_026 }
let(:error_message) { 'Message undeliverable' }
let!(:contributor) do
create(:contributor, whats_app_phone_number: '+49123456789', organization: organization, email: nil, first_name: 'Johnny')
end
let(:message_explaining_reason_for_being_marked_inactive) do
<<~HELLO
Die Rufnummer wurde möglicherweise nicht bei WhatsApp registriert oder der Empfänger hat die neuen Nutzungsbedingungen und Datenschutzrichtlinien von WhatsApp nicht akzeptiert.
HELLO
end
let(:message_continued) do
<<~HELLO
Es ist auch möglich, dass der Empfänger eine alte, nicht unterstützte Version des WhatsApp-Clients für sein Telefon verwendet. Bitte überprüfe dies mit Johnny
HELLO
end
before { components[:statuses] = failed_status }

it 'ignores statuses' do
expect(WhatsAppAdapter::ThreeSixtyDialogInbound).not_to receive(:new)
it 'reports any errors' do
expect(Sentry).to receive(:capture_exception).with(exception)

subject.call
subject.call
end

it 'marks the contributor as inactive since we are unable to send them a message' do
subject.call
expect(MarkInactiveContributorInactiveJob).to have_been_enqueued.with do |params|
expect(params[:organization_id]).to eq(organization.id)
expect(params[:contributor_id]).to eq(contributor.id)
end
end

it 'displays a message to inform the contributor the potential reason' do
perform_enqueued_jobs(only: MarkInactiveContributorInactiveJob) do
subject.call
get organization_contributor_path(organization, contributor, as: user)
expect(page).to have_content(message_explaining_reason_for_being_marked_inactive.strip)
expect(page).to have_content(message_continued.strip)
end
end
end
end
end
end

describe 'errors' do
let(:exception) { WhatsAppAdapter::ThreeSixtyDialogError.new(error_code: '501', message: 'Unsupported message type') }
let(:error_code) { 501 }
let(:error_message) { 'Unsupported message type' }

before do
components[:errors] = [{
code: 501,
title: 'Unsupported message type',
message: 'Unsupported message type',
error_data: { details: 'Message type is not currently supported' }
}]

allow(ErrorNotifier).to receive(:report)
end

it 'reports the error' do
expect(ErrorNotifier).to receive(:report).with(exception, context: { details: 'Message type is not currently supported' })
context = { details: 'Message type is not currently supported', title: 'Unsupported message type' }
expect(ErrorNotifier).to receive(:report).with(exception, context: context)

subject.call
end
Expand Down

0 comments on commit fdbe579

Please sign in to comment.