From f68e79f1bd61c38a733298805482e1377c117909 Mon Sep 17 00:00:00 2001 From: Mackenzie Halliday Date: Wed, 5 Feb 2025 14:19:29 -0500 Subject: [PATCH] #2172 - Revert payload passed into celery back to dict, correct testing as needed --- app/celery/provider_tasks.py | 1 - app/va/vetext/client.py | 38 ++++++++++++++++--------- tests/app/celery/test_provider_tasks.py | 37 ++++++++++++++++++------ 3 files changed, 53 insertions(+), 23 deletions(-) diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index a284a24fab..89270ff5b7 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -273,7 +273,6 @@ def _handle_delivery_failure( @notify_celery.task( bind=True, - serializer='pickle', name='deliver_push', throws=(AutoRetryException,), autoretry_for=(AutoRetryException,), diff --git a/app/va/vetext/client.py b/app/va/vetext/client.py index 9f2ad35739..408a72f77b 100644 --- a/app/va/vetext/client.py +++ b/app/va/vetext/client.py @@ -1,4 +1,3 @@ -from dataclasses import asdict from logging import Logger from time import monotonic from typing_extensions import TypedDict @@ -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() @@ -80,18 +92,18 @@ def send_push_notification( elif e.response.status_code == 400: self._decode_bad_request_response(e) else: - payload_dict['icn'] = '' + payload['icn'] = '' 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'] = '' + payload['icn'] = '' 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 diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 39615bb50f..f37c0b2d3e 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -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 @@ -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( @@ -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( @@ -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( @@ -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)