Merge pull request #474 from tutorcruncher/fix-carrier-failed-webhook-status

Fix carrier failed webhook status

Author: HenryTraill

src/schemas/webhooks.py

@@ -4,7 +4,7 @@
 from enum import Enum
 from pydantic import BaseModel, validator
 from pydantic.validators import str_validator
-from typing import List
+from typing import List, Optional
 
 from src.schemas.messages import MessageStatus
 
@@ -98,7 +98,7 @@ class MessageBirdWebHook(BaseWebhook):
     status: MessageBirdMessageStatus
     message_id: IDStr
     error_code: str = None
-    price_amount: float
+    price_amount: Optional[float] = None
 
     def extra_json(self, sort_keys=False):
         return json.dumps({'error_code': self.error_code} if self.error_code else {}, sort_keys=sort_keys)

src/version.py

@@ -1 +1 @@
-VERSION = '1.0.34'
+VERSION = '1.0.35'

src/views/webhooks.py

@@ -2,6 +2,7 @@
 import hashlib
 import hmac
 import json
+import logging
 from fastapi import APIRouter, Form, Header
 from foxglove import glove
 from foxglove.exceptions import HttpBadRequest, HttpForbidden, HttpUnprocessableEntity
@@ -15,6 +16,8 @@
 
 app = APIRouter(route_class=KeepBodyAPIRoute)
 
+logger = logging.getLogger('views.webhooks')
+
 
 @app.post('/test/')
 async def test_webhook_view(m: MandrillSingleWebhook):
@@ -56,6 +59,16 @@ async def messagebird_webhook_view(request: Request):
         event = MessageBirdWebHook(**request.query_params)
     except ValidationError as e:
         raise HttpUnprocessableEntity(e.args[0])
+    if event.error_code is not None:
+        if event.error_code == '104':
+            logger.error(
+                '[webhooks][mesagebird] carrier rejected error',
+                extra={'id': event.message_id, 'datetime': event.ts, 'status': event.status},
+            )
+        else:
+            # Will change this to a warning in the future
+            logger.error('[webhooks][mesagebird] delivery failed with status: %s', event.status)
+
     method = SendMethod.sms_messagebird
     if (test := request.query_params.get('test')) and test.lower() == 'true':
         method = SendMethod.sms_test

src/worker/webhooks.py

@@ -98,11 +98,12 @@ async def update_message_status(ctx, send_method: SendMethod, m: BaseWebhook, lo
             values=Values(message_id=message_id, status=m.status, ts=m.ts, extra=m.extra_json()),
         ),
     ]
-    if isinstance(m, MessageBirdWebHook):
+    if isinstance(m, MessageBirdWebHook) and m.price_amount is not None:
         cost = m.price_amount
         qs.append(
             glove.pg.execute_b('update messages set cost=:cost where id=:message_id', cost=cost, message_id=message_id)
         )
+
     await asyncio.gather(*qs)
 
     return UpdateStatus.added

tests/test_sms.py

@@ -263,6 +263,92 @@ def test_messagebird_webhook_sms_pricing(cli, sync_db: SyncDb, dummy_server, wor
     assert msg['cost'] == 0.07
 
 
+def test_messagebird_webhook_carrier_failed(cli, sync_db: SyncDb, dummy_server, worker, loop):
+    data = {
+        'uid': str(uuid4()),
+        'company_code': 'webhook-test',
+        'method': 'sms-messagebird',
+        'main_template': 'this is a message',
+        'recipients': [{'first_name': 'John', 'last_name': 'Doe', 'user_link': 4321, 'number': '07801234567'}],
+    }
+
+    r = cli.post('/send/sms/', json=data, headers={'Authorization': 'testing-key'})
+    assert r.status_code == 201, r.text
+    assert worker.test_run() == 1
+
+    msg = sync_db.fetchrow_b('select * from messages join message_groups g on g.id = messages.id')
+    assert msg['status'] == 'send'
+    assert msg['to_first_name'] == 'John'
+    assert msg['to_last_name'] == 'Doe'
+    assert msg['to_user_link'] == '4321'
+    assert msg['to_address'] == '+44 7801 234567'
+    assert msg['from_name'] == 'Morpheus'
+    assert msg['body'] == 'this is a message'
+    assert msg['cost'] is None
+    assert len(msg['tags']) == 1  # just group_id
+
+    url_args = {
+        'id': msg['external_id'],
+        'reference': 'morpheus',
+        'recipient': '447801234567',
+        'status': 'delivery_failed',
+        'statusDatetime': '2032-06-06T12:00:00',
+        'statusReason': 'carrier+rejected',
+        'statusErrorCode': 104,
+    }
+
+    r = cli.get(f'/webhook/messagebird/?{urlencode(url_args)}')
+    assert r.status_code == 200, r.text
+    assert worker.test_run() == 2
+
+    msg = sync_db.fetchrow_b('select * from messages')
+    assert msg['status'] == 'delivery_failed'
+    assert msg['cost'] is None
+
+
+def test_messagebird_webhook_other_delivery_failed(cli, sync_db: SyncDb, dummy_server, worker, loop):
+    data = {
+        'uid': str(uuid4()),
+        'company_code': 'webhook-test',
+        'method': 'sms-messagebird',
+        'main_template': 'this is a message',
+        'recipients': [{'first_name': 'John', 'last_name': 'Doe', 'user_link': 4321, 'number': '07801234567'}],
+    }
+
+    r = cli.post('/send/sms/', json=data, headers={'Authorization': 'testing-key'})
+    assert r.status_code == 201, r.text
+    assert worker.test_run() == 1
+
+    msg = sync_db.fetchrow_b('select * from messages join message_groups g on g.id = messages.id')
+    assert msg['status'] == 'send'
+    assert msg['to_first_name'] == 'John'
+    assert msg['to_last_name'] == 'Doe'
+    assert msg['to_user_link'] == '4321'
+    assert msg['to_address'] == '+44 7801 234567'
+    assert msg['from_name'] == 'Morpheus'
+    assert msg['body'] == 'this is a message'
+    assert msg['cost'] is None
+    assert len(msg['tags']) == 1  # just group_id
+
+    url_args = {
+        'id': msg['external_id'],
+        'reference': 'morpheus',
+        'recipient': '447801234567',
+        'status': 'delivery_failed',
+        'statusDatetime': '2032-06-06T12:00:00',
+        'statusReason': 'unknown+subscriber',
+        'statusErrorCode': 27,
+    }
+
+    r = cli.get(f'/webhook/messagebird/?{urlencode(url_args)}')
+    assert r.status_code == 200, r.text
+    assert worker.test_run() == 2
+
+    msg = sync_db.fetchrow_b('select * from messages')
+    assert msg['status'] == 'delivery_failed'
+    assert msg['cost'] is None
+
+
 def test_failed_render(cli, tmpdir, sync_db: SyncDb, worker, loop):
     data = {
         'uid': str(uuid4()),