From 7627cf53ae1c7b374206d31d9d328ce54a8e1a54 Mon Sep 17 00:00:00 2001 From: muhammadadeeltajamul Date: Fri, 13 Sep 2024 11:06:53 +0500 Subject: [PATCH 1/5] feat: changed message object to dictionary when sending ACE_MESSAGE_SENT signal --- edx_ace/__init__.py | 2 +- edx_ace/delivery.py | 4 ++-- edx_ace/utils/signals.py | 43 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 edx_ace/utils/signals.py diff --git a/edx_ace/__init__.py b/edx_ace/__init__.py index ff21bca6..b18df1b6 100644 --- a/edx_ace/__init__.py +++ b/edx_ace/__init__.py @@ -13,7 +13,7 @@ from .recipient import Recipient from .recipient_resolver import RecipientResolver -__version__ = '1.11.1' +__version__ = '1.11.2' __all__ = [ diff --git a/edx_ace/delivery.py b/edx_ace/delivery.py index 9ae411dc..38eb0112 100644 --- a/edx_ace/delivery.py +++ b/edx_ace/delivery.py @@ -10,8 +10,8 @@ from django.conf import settings from edx_ace.errors import RecoverableChannelDeliveryError -from edx_ace.signals import ACE_MESSAGE_SENT from edx_ace.utils.date import get_current_time +from edx_ace.utils.signals import send_ace_message_sent_signal LOG = logging.getLogger(__name__) @@ -61,7 +61,7 @@ def deliver(channel, rendered_message, message): message.report(f'{channel_type}_delivery_retried', num_seconds) else: message.report(f'{channel_type}_delivery_succeeded', True) - ACE_MESSAGE_SENT.send(sender=channel, message=message) + send_ace_message_sent_signal(channel, message) return delivery_expired_report = f'{channel_type}_delivery_expired' diff --git a/edx_ace/utils/signals.py b/edx_ace/utils/signals.py new file mode 100644 index 00000000..772eb613 --- /dev/null +++ b/edx_ace/utils/signals.py @@ -0,0 +1,43 @@ +""" +Utils for signals. +""" +from edx_ace.signals import ACE_MESSAGE_SENT + + +def make_serializable_object(obj): + """ + Takes a dictionary/list and returns a dictionary/list with all the values converted + to JSON serializable objects. + """ + if isinstance(obj, (int, float, str, bool)) or obj is None: + return obj + elif isinstance(obj, dict): + return {key: make_serializable_object(value) for key, value in obj.items()} + elif isinstance(obj, list): + return [make_serializable_object(element) for element in obj] + return str(obj) + + +def send_ace_message_sent_signal(channel, message): + """ + Creates dictionary from message, makes it JSON serializable and + sends the ACE_MESSAGE_SENT signal. + """ + try: + channel_name = channel.__class__.__name__ + except AttributeError: + channel_name = 'Other' + data = { + 'name': message.name, + 'app_label': message.app_label, + 'recipient': { + 'email': getattr(message.recipient, 'email_address', ''), + 'user_id': getattr(message.recipient, 'lms_user_id', ''), + }, + 'channel': channel_name, + 'context': message.context, + 'options': message.options, + 'uuid': str(message.uuid), + 'send_uuid': str(message.send_uuid), + } + ACE_MESSAGE_SENT.send(sender=channel, message=make_serializable_object(data)) From 974ce35bb34d8ffd11667f7fd019f055abb17036 Mon Sep 17 00:00:00 2001 From: muhammadadeeltajamul Date: Fri, 13 Sep 2024 11:35:55 +0500 Subject: [PATCH 2/5] fix: fixed failing tests --- edx_ace/tests/test_delivery.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/edx_ace/tests/test_delivery.py b/edx_ace/tests/test_delivery.py index 16fecfa1..aedb05b7 100644 --- a/edx_ace/tests/test_delivery.py +++ b/edx_ace/tests/test_delivery.py @@ -34,19 +34,19 @@ def setUp(self): ) self.current_time = datetime.datetime.utcnow().replace(tzinfo=tzutc()) - @patch('edx_ace.delivery.ACE_MESSAGE_SENT.send') + @patch('edx_ace.delivery.send_ace_message_sent_signal') def test_happy_path(self, mock_ace_message_sent): deliver(self.mock_channel, sentinel.rendered_email, self.message) self.mock_channel.deliver.assert_called_once_with(self.message, sentinel.rendered_email) # check if ACE_MESSAGE_SENT is raised - mock_ace_message_sent.assert_called_once_with(sender=self.mock_channel, message=self.message) + mock_ace_message_sent.assert_called_once_with(self.mock_channel, self.message) def test_fatal_error(self): self.mock_channel.deliver.side_effect = FatalChannelDeliveryError('testing') with self.assertRaises(FatalChannelDeliveryError): deliver(self.mock_channel, sentinel.rendered_email, self.message) - @patch('edx_ace.delivery.ACE_MESSAGE_SENT.send') + @patch('edx_ace.delivery.send_ace_message_sent_signal') @patch('edx_ace.delivery.get_current_time') def test_custom_message_expiration(self, mock_get_current_time, mock_ace_message_sent): self.message.expiration_time = self.current_time - datetime.timedelta(seconds=10) @@ -106,7 +106,7 @@ def test_multiple_retries(self, mock_get_current_time, mock_time): assert mock_time.sleep.call_args_list == [call(1), call(1)] assert self.mock_channel.deliver.call_count == 3 - @patch('edx_ace.delivery.ACE_MESSAGE_SENT.send') + @patch('edx_ace.delivery.send_ace_message_sent_signal') def test_message_sent_signal_for_push_channel(self, mock_ace_message_sent): """ Test that ACE_MESSAGE_SENT signal is sent when a message is delivered to a push channel. @@ -117,4 +117,4 @@ def test_message_sent_signal_for_push_channel(self, mock_ace_message_sent): ) deliver(mock_push_channel, sentinel.rendered_email, self.message) # check if ACE_MESSAGE_SENT is raised - mock_ace_message_sent.assert_called_once_with(sender=mock_push_channel, message=self.message) + mock_ace_message_sent.assert_called_once_with(mock_push_channel, self.message) From c20b62b6b06f04486bda22ec5ff6ce7af0eec9e7 Mon Sep 17 00:00:00 2001 From: muhammadadeeltajamul Date: Mon, 16 Sep 2024 13:10:31 +0500 Subject: [PATCH 3/5] test: added unit tests for serialize function --- edx_ace/tests/utils/test_signals.py | 42 +++++++++++++++++++++++++++++ edx_ace/utils/signals.py | 15 ++++++----- 2 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 edx_ace/tests/utils/test_signals.py diff --git a/edx_ace/tests/utils/test_signals.py b/edx_ace/tests/utils/test_signals.py new file mode 100644 index 00000000..10ff8a3e --- /dev/null +++ b/edx_ace/tests/utils/test_signals.py @@ -0,0 +1,42 @@ +from django.test import TestCase +from edx_ace.utils.signals import make_serializable_object + +class TestMakeSerializableObject(TestCase): + def test_primitive_types(self): + self.assertEqual(make_serializable_object(42), 42) + self.assertEqual(make_serializable_object(3.14), 3.14) + self.assertEqual(make_serializable_object("string"), "string") + self.assertEqual(make_serializable_object(True), True) + self.assertEqual(make_serializable_object(None), None) + + def test_dict(self): + input_dict = { + "int": 1, + "float": 2.0, + "str": "test", + "bool": False, + "none": None, + "list": [1, 2, 3], + "nested_dict": {"key": "value"} + } + self.assertEqual(make_serializable_object(input_dict), input_dict) + + def test_list(self): + input_list = [1, 2.0, "test", False, None, [1, 2, 3], {"key": "value"}] + self.assertEqual(make_serializable_object(input_list), input_list) + + def test_non_serializable(self): + class NonSerializable: + pass + + obj = NonSerializable() + self.assertEqual(make_serializable_object(obj), str(obj)) + + def test_non_serializable_list(self): + class NonSerializable: + pass + + obj = NonSerializable() + obj2 = NonSerializable() + obj_list = [obj, obj2] + self.assertEqual(make_serializable_object(obj_list), [str(obj), str(obj2)]) diff --git a/edx_ace/utils/signals.py b/edx_ace/utils/signals.py index 772eb613..68eef7c4 100644 --- a/edx_ace/utils/signals.py +++ b/edx_ace/utils/signals.py @@ -9,12 +9,15 @@ def make_serializable_object(obj): Takes a dictionary/list and returns a dictionary/list with all the values converted to JSON serializable objects. """ - if isinstance(obj, (int, float, str, bool)) or obj is None: - return obj - elif isinstance(obj, dict): - return {key: make_serializable_object(value) for key, value in obj.items()} - elif isinstance(obj, list): - return [make_serializable_object(element) for element in obj] + try: + if isinstance(obj, (int, float, str, bool)) or obj is None: + return obj + elif isinstance(obj, dict): + return {key: make_serializable_object(value) for key, value in obj.items()} + elif isinstance(obj, list): + return [make_serializable_object(element) for element in obj] + except: # pylint: disable=bare-except + pass return str(obj) From b798eb09e6a51499987ada69f298b38d57adc449 Mon Sep 17 00:00:00 2001 From: muhammadadeeltajamul Date: Mon, 16 Sep 2024 14:48:31 +0500 Subject: [PATCH 4/5] fix: fixed lint errors --- edx_ace/tests/utils/test_signals.py | 1 + edx_ace/utils/signals.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/edx_ace/tests/utils/test_signals.py b/edx_ace/tests/utils/test_signals.py index 10ff8a3e..8399c402 100644 --- a/edx_ace/tests/utils/test_signals.py +++ b/edx_ace/tests/utils/test_signals.py @@ -1,6 +1,7 @@ from django.test import TestCase from edx_ace.utils.signals import make_serializable_object + class TestMakeSerializableObject(TestCase): def test_primitive_types(self): self.assertEqual(make_serializable_object(42), 42) diff --git a/edx_ace/utils/signals.py b/edx_ace/utils/signals.py index 68eef7c4..6c4ff324 100644 --- a/edx_ace/utils/signals.py +++ b/edx_ace/utils/signals.py @@ -16,7 +16,7 @@ def make_serializable_object(obj): return {key: make_serializable_object(value) for key, value in obj.items()} elif isinstance(obj, list): return [make_serializable_object(element) for element in obj] - except: # pylint: disable=bare-except + except Exception: # pylint: disable=broad-except pass return str(obj) From 869949b28d27f1aa78b2111606997cfaa2dac28f Mon Sep 17 00:00:00 2001 From: muhammadadeeltajamul Date: Mon, 16 Sep 2024 14:52:33 +0500 Subject: [PATCH 5/5] fix: fixed isort tests --- edx_ace/tests/utils/test_signals.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/edx_ace/tests/utils/test_signals.py b/edx_ace/tests/utils/test_signals.py index 8399c402..61d16fe4 100644 --- a/edx_ace/tests/utils/test_signals.py +++ b/edx_ace/tests/utils/test_signals.py @@ -1,4 +1,8 @@ +""" +Tests for the utils/signals module. +""" from django.test import TestCase + from edx_ace.utils.signals import make_serializable_object