Skip to content

Commit

Permalink
#2172 - Revert payload passed into celery back to dict, correct testi…
Browse files Browse the repository at this point in the history
…ng as needed
  • Loading branch information
MackHalliday committed Feb 5, 2025
1 parent f561628 commit f68e79f
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 23 deletions.
1 change: 0 additions & 1 deletion app/celery/provider_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ def _handle_delivery_failure(

@notify_celery.task(
bind=True,
serializer='pickle',
name='deliver_push',
throws=(AutoRetryException,),
autoretry_for=(AutoRetryException,),
Expand Down
38 changes: 25 additions & 13 deletions app/va/vetext/client.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from dataclasses import asdict
from logging import Logger
from time import monotonic
from typing_extensions import TypedDict
Expand Down Expand Up @@ -33,34 +32,47 @@ def init_app(
self.statsd = statsd

@staticmethod
def format_for_vetext(payload: V2PushPayload) -> V2PushPayload:
if payload.personalisation:
def format_for_vetext(payload: V2PushPayload) -> dict[str, str]:
if payload.personalisation is not None:
payload.personalisation = {f'%{k.upper()}%': v for k, v in payload.personalisation.items()}

return payload
if payload.is_broadcast():
formatted_payload = {
'appSid': payload.app_sid,
'templateSid': payload.template_id,
'topicSid': payload.topic_sid,
'personalization': payload.personalisation,
}
else:
formatted_payload = {
'appSid': payload.app_sid,
'icn': payload.icn,
'templateSid': payload.template_id,
'personalization': payload.personalisation,
}
return formatted_payload

def send_push_notification(
self,
payload: V2PushPayload,
payload: dict[str, str],
) -> None:
"""Send the notification to VEText and handle any errors.
Args:
payload (V2PushPayload): The data to send to VEText
payload (dict[str, str]): The data to send to VEText
"""
self.logger.debug('VEText Payload information 2172: %s', payload)
start_time = monotonic()
try:
self.logger.debug('Sending to VEText base url: %s', self.base_url)
payload_dict = asdict(payload)

response = requests.post(
f'{self.base_url}/mobile/push/send', auth=self.auth, json=payload_dict, timeout=self.TIMEOUT
f'{self.base_url}/mobile/push/send', auth=self.auth, json=payload, timeout=self.TIMEOUT
)
self.logger.info(
'VEText response: %s for payload 2172: %s',
response.json() if response.ok else response.status_code,
payload_dict,
payload,
)
self.logger.info('VEText response text 2172: %s', response.text)
response.raise_for_status()
Expand All @@ -80,18 +92,18 @@ def send_push_notification(
elif e.response.status_code == 400:
self._decode_bad_request_response(e)
else:
payload_dict['icn'] = '<redacted>'
payload['icn'] = '<redacted>'
self.logger.exception(
'Status: %s - Not retrying - payload: %s',
e.response.status_code,
payload_dict,
payload,
)
raise NonRetryableException from e
except requests.RequestException as e:
payload_dict['icn'] = '<redacted>'
payload['icn'] = '<redacted>'
self.logger.exception(
'Exception raised sending push notification. Not retrying - payload: %s',
payload_dict,
payload,
)
self.statsd.incr(f'{self.STATSD_KEY}.error.request_exception')
raise NonRetryableException from e
Expand Down
37 changes: 28 additions & 9 deletions tests/app/celery/test_provider_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
)
from app.mobile_app import DEAFULT_MOBILE_APP_TYPE
from app.models import Notification
from app.v2.dataclasses import V2PushPayload
from app.v2.errors import RateLimitError
from collections import namedtuple
from notifications_utils.field import NullValueForNonConditionalPlaceholderException
Expand Down Expand Up @@ -508,10 +507,15 @@ def test_deliver_push_happy_path_icn(
json={'message': 'success'},
status_code=201,
)
payload = V2PushPayload(DEAFULT_MOBILE_APP_TYPE, 'any_template_id', icn='some_icn')
formatted_payload = {
'appSid': DEAFULT_MOBILE_APP_TYPE,
'templateSid': '2222',
'icn': '3333',
'personalization': {'%MSG_ID': '4444'},
}

# Should run without exceptions
deliver_push(payload)
deliver_push(formatted_payload)


def test_deliver_push_happy_path_topic(
Expand All @@ -524,10 +528,15 @@ def test_deliver_push_happy_path_topic(
json={'message': 'success'},
status_code=201,
)
payload = V2PushPayload(DEAFULT_MOBILE_APP_TYPE, 'any_template_id', topic_sid='some_topic_sid')
formatted_payload = {
'appSid': DEAFULT_MOBILE_APP_TYPE,
'templateSid': '2222',
'topicSid': '3333',
'personalization': {'%MSG_ID': '4444'},
}

# Should run without exceptions
deliver_push(payload)
deliver_push(formatted_payload)


@pytest.mark.parametrize(
Expand All @@ -554,10 +563,15 @@ def test_deliver_push_retryable_exception(
f'{client.application.config["VETEXT_URL"]}/mobile/push/send',
exc=test_exception,
)
payload = V2PushPayload(DEAFULT_MOBILE_APP_TYPE, 'any_template_id', 'some_icn')
formatted_payload = {
'appSid': DEAFULT_MOBILE_APP_TYPE,
'templateSid': '2222',
'icn': '3333',
'personalization': {'%MSG_ID': '4444'},
}

with pytest.raises(AutoRetryException):
deliver_push(payload)
deliver_push(formatted_payload)


@pytest.mark.parametrize(
Expand All @@ -582,7 +596,12 @@ def test_deliver_push_nonretryable_exception(
f'{client.application.config["VETEXT_URL"]}/mobile/push/send',
exc=test_exception,
)
payload = V2PushPayload(DEAFULT_MOBILE_APP_TYPE, 'any_template_id', 'some_icn')
formatted_payload = {
'appSid': DEAFULT_MOBILE_APP_TYPE,
'templateSid': '2222',
'icn': '3333',
'personalization': {'%MSG_ID': '4444'},
}

with pytest.raises(NonRetryableException):
deliver_push(payload)
deliver_push(formatted_payload)

0 comments on commit f68e79f

Please sign in to comment.