From fdbe579251abfecbfb1096add20eaee397c1b333 Mon Sep 17 00:00:00 2001 From: Matthew Rider Date: Wed, 20 Nov 2024 19:40:24 +0100 Subject: [PATCH] Handle unsuccessful deliveries (#2077) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../three_sixty_dialog_webhook_controller.rb | 18 ++- config/locales/de.yml | 2 +- .../three_sixty_dialog_webhook_spec.rb | 120 +++++++++++++----- 3 files changed, 101 insertions(+), 39 deletions(-) diff --git a/app/controllers/whats_app/three_sixty_dialog_webhook_controller.rb b/app/controllers/whats_app/three_sixty_dialog_webhook_controller.rb index 1689a03f1..de9c911a7 100644 --- a/app/controllers/whats_app/three_sixty_dialog_webhook_controller.rb +++ b/app/controllers/whats_app/three_sixty_dialog_webhook_controller.rb @@ -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) @@ -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 diff --git a/config/locales/de.yml b/config/locales/de.yml index 69f03c411..9f6f7f7cc 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -212,7 +212,7 @@ de: text: "%{name} ist ein inaktives Mitglied 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%{first_name}. + 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 %{first_name}. possible_reason_signal: "Die Rufnummer wurde möglicherweise nicht bei Signal registriert. Bitte überprüfe dies mit %{first_name}." possible_reason_threema: "Diese Threema-ID wurde möglicherweise von Threema als ungültig gemeldet. Bitte %{first_name}, 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." diff --git a/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb b/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb index b440c0102..5d3e637fb 100644 --- a/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb +++ b/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb @@ -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 @@ -44,29 +45,79 @@ 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' } }] @@ -74,7 +125,8 @@ 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