From 4fd2072e85a87f981d554fbd7bc2a8bcd124b57b Mon Sep 17 00:00:00 2001 From: Matthew Rider Date: Wed, 20 Nov 2024 10:20:47 +0100 Subject: [PATCH 1/2] Handle unsuccessful deliveries report errors, mark inactive contributors inactive --- .../three_sixty_dialog_webhook_controller.rb | 18 ++- config/locales/de.yml | 2 +- .../three_sixty_dialog_webhook_spec.rb | 117 +++++++++++++----- 3 files changed, 99 insertions(+), 38 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..fc9b28b00 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' } }] From 851df889fbb4ba6dbb4b5ac8099282bf1d1f4d64 Mon Sep 17 00:00:00 2001 From: Matthew Rider Date: Wed, 20 Nov 2024 10:29:20 +0100 Subject: [PATCH 2/2] Fix expectation after changing context of error --- spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 fc9b28b00..5d3e637fb 100644 --- a/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb +++ b/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb @@ -125,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