From 456064ff72ef8ba8829efc4acd7dd5f16ce08b81 Mon Sep 17 00:00:00 2001 From: maheshsattala <59285563+maheshsattala@users.noreply.github.com> Date: Thu, 3 Oct 2024 19:34:24 +0530 Subject: [PATCH] Recurring Broadcast Fix (#1563) * Recursive broadcast test case fix * Recursive broadcast test case fix * Recursive broadcast test case fix * Recursive broadcast test case fix * Fixed resend broadcast for the recurring case and added and fixed test cases for the same. * Fixed resend broadcast for the recurring case and added and fixed test cases for the same. * Fixed resend broadcast for the recurring case and added and fixed test cases for the same. * Fixed resend broadcast for the recurring case and added and fixed test cases for the same. --- .../events/definitions/message_broadcast.py | 1 + kairon/shared/chat/broadcast/processor.py | 10 ++- kairon/shared/chat/models.py | 6 ++ tests/integration_test/services_test.py | 86 +++++++++++++++++++ .../channels/broadcast_processor_test.py | 62 +++++++++++++ tests/unit_test/events/events_test.py | 23 ----- 6 files changed, 161 insertions(+), 27 deletions(-) diff --git a/kairon/events/definitions/message_broadcast.py b/kairon/events/definitions/message_broadcast.py index 56adb1bd4..eaa7128e8 100644 --- a/kairon/events/definitions/message_broadcast.py +++ b/kairon/events/definitions/message_broadcast.py @@ -65,6 +65,7 @@ def execute(self, event_id: Text, **kwargs): if is_resend: config = broadcast.resend_broadcast() else: + config = MessageBroadcastProcessor.update_retry_count(event_id, self.bot, self.user, retry_count=0) recipients = broadcast.get_recipients() broadcast.send(recipients) status = EVENT_STATUS.COMPLETED.value diff --git a/kairon/shared/chat/broadcast/processor.py b/kairon/shared/chat/broadcast/processor.py index 63d84c835..c7ebcf986 100644 --- a/kairon/shared/chat/broadcast/processor.py +++ b/kairon/shared/chat/broadcast/processor.py @@ -20,8 +20,7 @@ class MessageBroadcastProcessor: @staticmethod def get_settings(notification_id: Text, bot: Text, **kwargs): try: - status = not kwargs.get("is_resend", False) - settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=status).get() + settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot).get() settings = settings.to_mongo().to_dict() settings["_id"] = settings["_id"].__str__() return settings @@ -116,7 +115,10 @@ def get_broadcast_logs(bot: Text, start_idx: int = 0, page_size: int = 10, **kwa @staticmethod def get_reference_id_from_broadcasting_logs(event_id): - log = MessageBroadcastLogs.objects(event_id=event_id, log_type=MessageBroadcastLogType.common.value).get() + log = MessageBroadcastLogs.objects( + event_id=event_id, + log_type=MessageBroadcastLogType.common.value + ).order_by('-timestamp').first() return log.reference_id @staticmethod @@ -202,7 +204,7 @@ def __add_broadcast_logs_status_and_errors(reference_id: Text, campaign_name: Te @staticmethod def update_retry_count(notification_id: Text, bot: Text, user: Text, retry_count: int = 0): try: - settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=False).get() + settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot).get() settings.retry_count = retry_count settings.user = user settings.timestamp = datetime.utcnow() diff --git a/kairon/shared/chat/models.py b/kairon/shared/chat/models.py index 4205fa099..876eadaab 100644 --- a/kairon/shared/chat/models.py +++ b/kairon/shared/chat/models.py @@ -93,6 +93,12 @@ def validate_request(cls, values): "recipients_config and template_config is required for static broadcasts!" ) + if values.get("broadcast_type") == MessageBroadcastType.dynamic: + if not values.get("template_name"): + raise ValueError("template_name is required for dynamic broadcasts!") + if not values.get("language_code"): + raise ValueError("language_code is required for dynamic broadcasts!") + pyscript = values.get("pyscript") if values.get( "broadcast_type" diff --git a/tests/integration_test/services_test.py b/tests/integration_test/services_test.py index 999109bd3..54f646a11 100644 --- a/tests/integration_test/services_test.py +++ b/tests/integration_test/services_test.py @@ -1296,6 +1296,61 @@ def test_get_llm_metadata_bot_specific_model_exists(): LLMSecret.objects.delete() +@patch("kairon.shared.utils.Utility.request_event_server", autospec=True) +def test_add_scheduled_broadcast_with_no_template_name(mock_event_server): + config = { + "name": "first_scheduler_dynamic", + "broadcast_type": "dynamic", + "connector_type": "whatsapp", + "scheduler_config": { + "expression_type": "cron", + "schedule": "21 11 * * *", + "timezone": "Asia/Kolkata", + }, + "pyscript": "send_msg('template_name', '9876543210')", + } + response = client.post( + f"/api/bot/{pytest.bot}/channels/broadcast/message", + json=config, + headers={"Authorization": pytest.token_type + " " + pytest.access_token}, + ) + actual = response.json() + print(actual) + assert not actual["success"] + assert actual["error_code"] == 422 + assert actual["message"] == [{'loc': ['body', '__root__'], + 'msg': 'template_name is required for dynamic broadcasts!', 'type': 'value_error'}] + assert not actual["data"] + + +@patch("kairon.shared.utils.Utility.request_event_server", autospec=True) +def test_add_scheduled_broadcast_with_no_language_code(mock_event_server): + config = { + "name": "first_scheduler_dynamic", + "broadcast_type": "dynamic", + "connector_type": "whatsapp", + "template_name": "consent", + "scheduler_config": { + "expression_type": "cron", + "schedule": "21 11 * * *", + "timezone": "Asia/Kolkata", + }, + "pyscript": "send_msg('template_name', '9876543210')", + } + response = client.post( + f"/api/bot/{pytest.bot}/channels/broadcast/message", + json=config, + headers={"Authorization": pytest.token_type + " " + pytest.access_token}, + ) + actual = response.json() + print(actual) + assert not actual["success"] + assert actual["error_code"] == 422 + assert actual["message"] == [{'loc': ['body', '__root__'], + 'msg': 'language_code is required for dynamic broadcasts!', 'type': 'value_error'}] + assert not actual["data"] + + @responses.activate def test_default_values(): response = client.get( @@ -23288,6 +23343,8 @@ def test_broadcast_config_error(): "timezone": "UTC", } config["broadcast_type"] = "dynamic" + config["template_name"] = "consent" + config["language_code"] = "en" response = client.post( f"/api/bot/{pytest.bot}/channels/broadcast/message", json=config, @@ -23329,6 +23386,8 @@ def test_update_broadcast(mock_event_server): "name": "first_scheduler_dynamic", "broadcast_type": "dynamic", "connector_type": "whatsapp", + "template_name": "consent", + "language_code": "en", "scheduler_config": { "expression_type": "cron", "schedule": "21 11 * * *", @@ -23624,6 +23683,33 @@ def test_get_bot_settings(): 'retry_broadcasting_limit': 3} +@patch("kairon.shared.utils.Utility.request_event_server", autospec=True) +def test_add_scheduled_dynamic_broadcast(mock_event_server): + config = { + "name": "first_scheduler_dynamic", + "broadcast_type": "dynamic", + "connector_type": "whatsapp", + "template_name": "consent", + "language_code": "en", + "scheduler_config": { + "expression_type": "cron", + "schedule": "21 11 * * *", + "timezone": "Asia/Kolkata", + }, + "pyscript": "send_msg('template_name', '9876543210')", + } + response = client.post( + f"/api/bot/{pytest.bot}/channels/broadcast/message", + json=config, + headers={"Authorization": pytest.token_type + " " + pytest.access_token}, + ) + actual = response.json() + assert actual["success"] + assert actual["error_code"] == 0 + assert actual["message"] == 'Broadcast added!' + assert actual["data"] + + def test_update_analytics_settings_with_empty_value(): bot_settings = { "analytics": {'fallback_intent': ''}, diff --git a/tests/unit_test/channels/broadcast_processor_test.py b/tests/unit_test/channels/broadcast_processor_test.py index da528460c..3757e6194 100644 --- a/tests/unit_test/channels/broadcast_processor_test.py +++ b/tests/unit_test/channels/broadcast_processor_test.py @@ -266,3 +266,65 @@ def test_delete_schedule_not_exists(self): with pytest.raises(AppException, match="Notification settings not found!"): MessageBroadcastProcessor.delete_task(ObjectId().__str__(), bot, True) + + @patch("kairon.shared.utils.Utility.is_exist", autospec=True) + def test_add_scheduled_broadcast_task(self, mock_channel_config): + bot = "test_schedule" + user = "test_user" + config = { + "name": "schedule_broadcast", "broadcast_type": "dynamic", + "connector_type": "whatsapp", + "scheduler_config": { + "expression_type": "cron", + "schedule": "57 22 * * *", + "timezone": "Asia/Kolkata" + }, + "pyscript": "send_msg('template_name', '9876543210')" + } + message_broadcast_id = MessageBroadcastProcessor.add_scheduled_task(bot, user, config) + assert message_broadcast_id + + def test_get_broadcast_settings_scheduled(self): + bot = "test_schedule" + settings = list(MessageBroadcastProcessor.list_settings(bot)) + config_id = settings[0].pop("_id") + assert isinstance(config_id, str) + settings[0].pop("timestamp") + assert settings == [ + { + 'name': 'schedule_broadcast', + 'connector_type': 'whatsapp', + "broadcast_type": "dynamic", + 'retry_count': 0, + 'scheduler_config': { + 'expression_type': 'cron', + 'schedule': '57 22 * * *', + "timezone": "Asia/Kolkata" + }, + "pyscript": "send_msg('template_name', '9876543210')", + "template_config": [], + 'bot': 'test_schedule', + 'user': 'test_user', + 'status': True + } + ] + + setting = MessageBroadcastProcessor.get_settings(config_id, bot) + assert isinstance(setting.pop("_id"), str) + setting.pop("timestamp") + assert setting == { + 'name': 'schedule_broadcast', + 'connector_type': 'whatsapp', + "broadcast_type": "dynamic", + 'retry_count': 0, + 'scheduler_config': { + 'expression_type': 'cron', + 'schedule': '57 22 * * *', + "timezone": "Asia/Kolkata" + }, + "pyscript": "send_msg('template_name', '9876543210')", + "template_config": [], + 'bot': 'test_schedule', + 'user': 'test_user', + 'status': True + } diff --git a/tests/unit_test/events/events_test.py b/tests/unit_test/events/events_test.py index 2ed2385c9..85f4f2695 100644 --- a/tests/unit_test/events/events_test.py +++ b/tests/unit_test/events/events_test.py @@ -1406,9 +1406,6 @@ def test_execute_message_broadcast_with_static_values(self, mock_is_exist, mock_ 'template_name': 'brochure_pdf', 'language_code': 'hi', 'namespace': None, 'retry_count': 0} - with pytest.raises(AppException, match="Notification settings not found!"): - MessageBroadcastProcessor.get_settings(event_id, bot) - settings = list(MessageBroadcastProcessor.list_settings(bot, status=False, name="one_time_schedule")) assert len(settings) == 1 assert settings[0]["status"] == False @@ -1541,8 +1538,6 @@ def test_execute_message_broadcast_with_dynamic_values(self, mock_is_exist, mock 'filename': 'Brochure.pdf'}}]}], "template": template, 'template_name': 'brochure_pdf', 'language_code': 'hi', 'namespace': None, 'retry_count': 0} - with pytest.raises(AppException, match="Notification settings not found!"): - MessageBroadcastProcessor.get_settings(event_id, bot) assert mock_send.call_args[0][1] == 'brochure_pdf' assert mock_send.call_args[0][2] in ['876543212345', '9876543210'] @@ -1609,9 +1604,6 @@ def test_execute_message_broadcast_with_recipient_evaluation_failure(self, mock_ 'exception': "Failed to evaluate recipients: 'recipients'" } - with pytest.raises(AppException, match="Notification settings not found!"): - MessageBroadcastProcessor.get_settings(event_id, bot) - @responses.activate @patch("kairon.shared.chat.processor.ChatDataProcessor.get_channel_config") @patch("kairon.shared.data.processor.MongoProcessor.get_bot_settings") @@ -1710,9 +1702,6 @@ def test_execute_message_broadcast_with_channel_deleted(self, mock_is_exist, moc assert exception.startswith("Whatsapp channel config not found!") assert logs[0][0] == {"event_id": event_id, 'log_type': 'common', 'bot': bot, 'status': 'Fail', 'user': user, 'recipients': ['918958030541']} - with pytest.raises(AppException, match="Notification settings not found!"): - MessageBroadcastProcessor.get_settings(event_id, bot) - @responses.activate @mongomock.patch(servers=(('localhost', 27017),)) @patch("kairon.shared.channels.whatsapp.bsp.dialog360.BSP360Dialog.get_partner_auth_token", autospec=True) @@ -1865,9 +1854,6 @@ def test_execute_message_broadcast_evaluate_template_parameters(self, mock_is_ex 'href': 'https://developers.facebook.com/docs/whatsapp/cloud-api/support/error-codes/'} ]} - with pytest.raises(AppException, match="Notification settings not found!"): - MessageBroadcastProcessor.get_settings(event_id, bot) - settings = list(MessageBroadcastProcessor.list_settings(bot, status=False, name="one_time_schedule")) assert len(settings) == 1 assert settings[0]["status"] is False @@ -2016,9 +2002,6 @@ def test_execute_message_broadcast_with_pyscript(self, mock_list_templates, mock assert logged_config.pop("template_config") == [] assert logged_config == config - with pytest.raises(AppException, match="Notification settings not found!"): - MessageBroadcastProcessor.get_settings(event_id, bot) - timestamp = datetime.utcnow() MessageBroadcastLogs( **{ @@ -2222,9 +2205,6 @@ def test_execute_message_broadcast_with_pyscript_failure(self, mock_is_exist, mo assert logs[0][0] == {"event_id": event_id, 'reference_id': reference_id, 'log_type': 'common', 'bot': bot, 'status': 'Fail', 'user': user, "exception": "Script execution error: import of 'os' is unauthorized"} - with pytest.raises(AppException, match="Notification settings not found!"): - MessageBroadcastProcessor.get_settings(event_id, bot) - @responses.activate @patch("kairon.shared.channels.broadcast.whatsapp.json") @patch("kairon.shared.data.processor.MongoProcessor.get_bot_settings") @@ -2281,9 +2261,6 @@ def sleep_for_some_time(*args, **kwargs): assert logs[0][0] == {"event_id": event_id, 'reference_id': reference_id, 'log_type': 'common', 'bot': bot, 'status': 'Fail', 'user': user, 'exception': 'Operation timed out: 1 seconds'} - with pytest.raises(AppException, match="Notification settings not found!"): - MessageBroadcastProcessor.get_settings(event_id, bot) - @responses.activate @mongomock.patch(servers=(('localhost', 27017),)) @patch("kairon.shared.channels.whatsapp.bsp.dialog360.BSP360Dialog.get_partner_auth_token", autospec=True)