Resend Broadcast #1267
Resend Broadcast #1267
Conversation
Fixed and added test cases related to the same.
WalkthroughA new feature to resend scheduled message broadcasts has been integrated into the system. Changes include modifying multiple classes and methods across the code to accommodate the resend functionality. Key additions involve a new API endpoint, extended parameters in existing methods, and new constants to manage the broadcasting logic. Moreover, comprehensive tests have been added to ensure the reliability of the new feature. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Auth
participant BroadcastProcessor
participant BroadcastService
Client->>API: POST /resend_broadcast
API->>Auth: Validate user
Auth-->>API: User validated
API->>BroadcastProcessor: Fetch broadcast details
BroadcastProcessor-->>API: Broadcast details
API->>BroadcastService: Trigger resend (is_resend=True)
BroadcastService-->>API: Resend initiated
API-->>Client: Resend confirmation
Poem
Tip AI model upgrade
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (8)
kairon/events/definitions/scheduled_base.py (1)
Line range hint
11-11: Replacetyping.Textwithstrand optimize dictionary key checkThe use of
typing.Textis deprecated, and usingstris recommended. Also, checking for a key in a dictionary should be done directly without using.keys()for better performance.- from typing import Text, Dict + from typing import Dict - def enqueue(self, event_request_type: Text, **kwargs): + def enqueue(self, event_request_type: str, **kwargs): - def _resend_broadcast(self, msg_broadcast_id: Text, config: Dict): + def _resend_broadcast(self, msg_broadcast_id: str, config: Dict): - if event_request_type not in request_implementation.keys(): + if event_request_type not in request_implementation:Also applies to: 21-21, 35-35, 39-39, 43-43
Tools
Ruff
21-21: Use
key not in dictinstead ofkey not in dict.keys()(SIM118)Remove
.keys()tests/unit_test/events/events_test.py (7)
Line range hint
11-11: Replace deprecatedmockimport withunittest.mock.The
mocklibrary is now part of theunittestmodule in the standard library. Update the import statement to reflect this change.- import mock + from unittest.mock import patch, MagicMockTools
Ruff
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
26-26: Module level import not at top of file (E402)
27-27: Module level import not at top of file (E402)
28-28: Module level import not at top of file (E402)
Line range hint
21-21: Remove unused import.The import
kairon.shared.channels.broadcast.whatsapp.WhatsappBroadcastis not used in this file.- from kairon.shared.channels.broadcast.whatsapp import WhatsappBroadcastTools
Ruff
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
26-26: Module level import not at top of file (E402)
27-27: Module level import not at top of file (E402)
28-28: Module level import not at top of file (E402)
Line range hint
26-44: Reorganize imports to the top of the file.Python convention dictates that all imports should be at the top of the file unless there is a specific reason for a delayed import.
+ from kairon.shared.chat.broadcast.processor import MessageBroadcastProcessor + from kairon.events.definitions.data_importer import TrainingDataImporterEvent + from kairon.events.definitions.faq_importer import FaqDataImporterEvent + from kairon.events.definitions.history_delete import DeleteHistoryEvent + from kairon.events.definitions.message_broadcast import MessageBroadcastEvent + from kairon.events.definitions.model_testing import ModelTestingEvent + from kairon.events.definitions.model_training import ModelTrainingEvent + from kairon.events.definitions.scheduled_base import ScheduledEventsBase + from kairon.exceptions import AppException + from kairon.shared.chat.broadcast.processor import MessageBroadcastProcessor + from kairon.shared.constants import EventClass, EventRequestType, ChannelTypes + from kairon.shared.data.constant import EVENT_STATUS, REQUIREMENTS + from kairon.shared.data.data_objects import Configs, BotSettings + from kairon.shared.data.history_log_processor import HistoryDeletionLogProcessor + from kairon.shared.data.processor import MongoProcessor + from kairon.shared.importer.processor import DataImporterLogProcessor + from kairon.shared.test.processor import ModelTestingLogProcessor + from kairon.shared.utils import Utility + from kairon.test.test_models import ModelTesterTools
Ruff
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
26-26: Module level import not at top of file (E402)
27-27: Module level import not at top of file (E402)
28-28: Module level import not at top of file (E402)
Line range hint
53-53: Use capitalized environment variable name.It's a common convention to use uppercase for environment variable names.
- os.environ["system_file"] = "./tests/testing_data/system.yaml" + os.environ["SYSTEM_FILE"] = "./tests/testing_data/system.yaml"Tools
Ruff
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
26-26: Module level import not at top of file (E402)
27-27: Module level import not at top of file (E402)
28-28: Module level import not at top of file (E402)
Line range hint
853-853: Rewrite as bytes literal.The use of the
encodemethod here is unnecessary. You can directly define the data as a bytes literal.- faq = "Questions,Answer,\nWhat is Digite?, IT Company,\nHow are you?, I am good,\nWhat day is it?, It is Thursday,\nWhat day is it?, It is Thursday,\n".encode() + faq = b"Questions,Answer,\nWhat is Digite?, IT Company,\nHow are you?, I am good,\nWhat day is it?, It is Thursday,\nWhat day is it?, It is Thursday,\n"Tools
Ruff
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
26-26: Module level import not at top of file (E402)
27-27: Module level import not at top of file (E402)
28-28: Module level import not at top of file (E402)
Line range hint
1398-1398: Use direct boolean checks.In Python, it's more idiomatic to use direct boolean checks rather than comparing to
False.- if settings[0]["status"] == False: + if not settings[0]["status"]:Tools
Ruff
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
26-26: Module level import not at top of file (E402)
27-27: Module level import not at top of file (E402)
28-28: Module level import not at top of file (E402)
Line range hint
1851-1851: Remove unnecessaryfprefix.The f-string here does not contain any placeholders, making the
fprefix unnecessary.- f"/api/events/execute/{EventClass.data_importer}" + "/api/events/execute/{EventClass.data_importer}"Tools
Ruff
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
26-26: Module level import not at top of file (E402)
27-27: Module level import not at top of file (E402)
28-28: Module level import not at top of file (E402)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- kairon/api/app/routers/bot/channels.py (1 hunks)
- kairon/events/definitions/message_broadcast.py (3 hunks)
- kairon/events/definitions/scheduled_base.py (2 hunks)
- kairon/shared/channels/broadcast/whatsapp.py (6 hunks)
- kairon/shared/chat/broadcast/constants.py (1 hunks)
- kairon/shared/chat/broadcast/processor.py (4 hunks)
- kairon/shared/constants.py (1 hunks)
- tests/integration_test/services_test.py (1 hunks)
- tests/unit_test/events/events_test.py (2 hunks)
Additional context used
Ruff
kairon/events/definitions/scheduled_base.py
11-11:
typing.Textis deprecated, usestr(UP019)Replace with
str
21-21: Use
key not in dictinstead ofkey not in dict.keys()(SIM118)Remove
.keys()
35-35:
typing.Textis deprecated, usestr(UP019)Replace with
str
39-39:
typing.Textis deprecated, usestr(UP019)Replace with
str
43-43:
typing.Textis deprecated, usestr(UP019)Replace with
strkairon/events/definitions/message_broadcast.py
25-25:
typing.Textis deprecated, usestr(UP019)Replace with
str
25-25:
typing.Textis deprecated, usestr(UP019)Replace with
str
29-29: Use
super()instead ofsuper(__class__, self)(UP008)Remove
__super__parameters
45-45:
typing.Textis deprecated, usestr(UP019)Replace with
str
105-105: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
107-107:
typing.Textis deprecated, usestr(UP019)Replace with
str
117-117:
typing.Textis deprecated, usestr(UP019)Replace with
str
137-137:
typing.Textis deprecated, usestr(UP019)Replace with
str
148-148:
typing.Textis deprecated, usestr(UP019)Replace with
strkairon/shared/channels/broadcast/whatsapp.py
36-36: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
65-65:
typing.Textis deprecated, usestr(UP019)Replace with
str
65-65:
typing.Textis deprecated, usestr(UP019)Replace with
str
65-65:
typing.Textis deprecated, usestr(UP019)Replace with
str
172-172: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
172-172: f-string without any placeholders (F541)
Remove extraneous
fprefix
174-174:
typing.Textis deprecated, usestr(UP019)Replace with
str
174-174:
typing.Textis deprecated, usestr(UP019)Replace with
strkairon/shared/chat/broadcast/processor.py
21-21:
typing.Textis deprecated, usestr(UP019)Replace with
str
21-21:
typing.Textis deprecated, usestr(UP019)Replace with
str
23-23: Use
not ...instead ofFalse if ... else True(SIM211)Replace with
not ...
29-29: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
32-32:
typing.Textis deprecated, usestr(UP019)Replace with
str
40-40:
typing.Textis deprecated, usestr(UP019)Replace with
str
40-40:
typing.Textis deprecated, usestr(UP019)Replace with
str
51-51:
typing.Textis deprecated, usestr(UP019)Replace with
str
51-51:
typing.Textis deprecated, usestr(UP019)Replace with
str
51-51:
typing.Textis deprecated, usestr(UP019)Replace with
str
68-68: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
71-71:
typing.Textis deprecated, usestr(UP019)Replace with
str
71-71:
typing.Textis deprecated, usestr(UP019)Replace with
str
81-81: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
84-84:
typing.Textis deprecated, usestr(UP019)Replace with
str
84-84:
typing.Textis deprecated, usestr(UP019)Replace with
str
84-84:
typing.Textis deprecated, usestr(UP019)Replace with
str
84-84:
typing.Textis deprecated, usestr(UP019)Replace with
str
105-105:
typing.Textis deprecated, usestr(UP019)Replace with
str
121-121:
typing.Textis deprecated, usestr(UP019)Replace with
str
136-136:
typing.Textis deprecated, usestr(UP019)Replace with
str
149-149:
typing.Textis deprecated, usestr(UP019)Replace with
str
160-160:
typing.Textis deprecated, usestr(UP019)Replace with
str
160-160:
typing.Textis deprecated, usestr(UP019)Replace with
str
160-160:
typing.Textis deprecated, usestr(UP019)Replace with
str
182-182:
typing.Textis deprecated, usestr(UP019)Replace with
str
182-182:
typing.Textis deprecated, usestr(UP019)Replace with
str
190-190:
typing.Textis deprecated, usestr(UP019)Replace with
str
190-190:
typing.Textis deprecated, usestr(UP019)Replace with
str
200-200:
typing.Textis deprecated, usestr(UP019)Replace with
strkairon/api/app/routers/bot/channels.py
25-25: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
38-38: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
48-48: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
60-60: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
71-71: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
83-83: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
98-98: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
113-113: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
128-128: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
142-142: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
156-156: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
170-170: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
188-188: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
202-202: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
214-214: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
226-226: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
239-239: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
252-252: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)tests/unit_test/events/events_test.py
11-11:
mockis deprecated, useunittest.mock(UP026)Import from
unittest.mockinstead
21-21:
kairon.shared.channels.broadcast.whatsapp.WhatsappBroadcastimported but unused (F401)Remove unused import:
kairon.shared.channels.broadcast.whatsapp.WhatsappBroadcast
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
26-26: Module level import not at top of file (E402)
27-27: Module level import not at top of file (E402)
28-28: Module level import not at top of file (E402)
29-29: Module level import not at top of file (E402)
30-30: Module level import not at top of file (E402)
31-31: Module level import not at top of file (E402)
32-32: Module level import not at top of file (E402)
33-33: Module level import not at top of file (E402)
34-34: Module level import not at top of file (E402)
35-35: Module level import not at top of file (E402)
35-35: Redefinition of unused
MessageBroadcastProcessorfrom line 26 (F811)Remove definition:
MessageBroadcastProcessor
36-36: Module level import not at top of file (E402)
37-37: Module level import not at top of file (E402)
38-38: Module level import not at top of file (E402)
39-39: Module level import not at top of file (E402)
40-40: Module level import not at top of file (E402)
41-41: Module level import not at top of file (E402)
42-42: Module level import not at top of file (E402)
43-43: Module level import not at top of file (E402)
44-44: Module level import not at top of file (E402)
46-46: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
53-53: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
853-853: Unnecessary call to
encodeas UTF-8 (UP012)Rewrite as bytes literal
883-883: Unnecessary call to
encodeas UTF-8 (UP012)Rewrite as bytes literal
929-929: Unnecessary call to
encodeas UTF-8 (UP012)Rewrite as bytes literal
1398-1398: Avoid equality comparisons to
False; useif not settings[0]["status"]:for false checks (E712)Replace with
not settings[0]["status"]
1851-1851: f-string without any placeholders (F541)
Remove extraneous
fprefix
1856-1856:
pytest.raises(Exception)should be considered evil (B017)
2340-2340: Local variable
reference_idis assigned to but never used (F841)Remove assignment to unused variable
reference_idtests/integration_test/services_test.py
9-9:
mockis deprecated, useunittest.mock(UP026)Import from
unittest.mockinstead
13-13:
mockis deprecated, useunittest.mock(UP026)Import from
unittest.mockinstead
76-76: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
85-85: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
233-237: Use a single
withstatement with multiple contexts instead of nestedwithstatements (SIM117)
690-690: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
747-747: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
793-793: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
808-808: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
835-835: f-string without any placeholders (F541)
Remove extraneous
fprefix
845-845: f-string without any placeholders (F541)
Remove extraneous
fprefix
1181-1181: Use context handler for opening files (SIM115)
1257-1257: Use context handler for opening files (SIM115)
1261-1261: Use context handler for opening files (SIM115)
1265-1265: Use context handler for opening files (SIM115)
1269-1269: Use context handler for opening files (SIM115)
1275-1275: Use context handler for opening files (SIM115)
1282-1282: Use context handler for opening files (SIM115)
1360-1360: Use context handler for opening files (SIM115)
1364-1364: Use context handler for opening files (SIM115)
1368-1368: Use context handler for opening files (SIM115)
1372-1372: Use context handler for opening files (SIM115)
1378-1378: Use context handler for opening files (SIM115)
1385-1385: Use context handler for opening files (SIM115)
1470-1470: Use context handler for opening files (SIM115)
1474-1474: Use context handler for opening files (SIM115)
1478-1478: Use context handler for opening files (SIM115)
1482-1482: Use context handler for opening files (SIM115)
1488-1488: Use context handler for opening files (SIM115)
1495-1495: Use context handler for opening files (SIM115)
2584-2584: Local variable
response_threeis assigned to but never used (F841)Remove assignment to unused variable
response_three
2808-2808: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
4586-4586: Use context handler for opening files (SIM115)
4610-4610: Use context handler for opening files (SIM115)
4644-4644: Use context handler for opening files (SIM115)
4648-4648: Use context handler for opening files (SIM115)
4652-4652: Use context handler for opening files (SIM115)
4656-4656: Use context handler for opening files (SIM115)
4662-4662: Use context handler for opening files (SIM115)
4669-4669: Use context handler for opening files (SIM115)
4703-4703: Use context handler for opening files (SIM115)
4707-4707: Use context handler for opening files (SIM115)
4713-4713: Use context handler for opening files (SIM115)
4718-4718: Use context handler for opening files (SIM115)
4722-4722: Use context handler for opening files (SIM115)
4804-4804: Use context handler for opening files (SIM115)
4836-4836: Use context handler for opening files (SIM115)
4843-4843: Use context handler for opening files (SIM115)
4850-4850: Use context handler for opening files (SIM115)
4859-4859: Use context handler for opening files (SIM115)
4866-4866: Use context handler for opening files (SIM115)
4921-4921: Use context handler for opening files (SIM115)
4928-4928: Use context handler for opening files (SIM115)
4935-4935: Use context handler for opening files (SIM115)
4944-4944: Use context handler for opening files (SIM115)
4951-4951: Use context handler for opening files (SIM115)
5062-5063: Replace
yieldoverforloop withyield from(UP028)Replace with
yield from
5209-5209: Use context handler for opening files (SIM115)
5267-5267: Use context handler for opening files (SIM115)
5271-5271: Use context handler for opening files (SIM115)
5275-5275: Use context handler for opening files (SIM115)
5279-5279: Use context handler for opening files (SIM115)
5285-5285: Use context handler for opening files (SIM115)
5319-5319: Use context handler for opening files (SIM115)
5323-5323: Use context handler for opening files (SIM115)
5327-5327: Use context handler for opening files (SIM115)
5331-5331: Use context handler for opening files (SIM115)
5633-5633: Avoid equality comparisons to
True; useif actual["success"]:for truth checks (E712)Replace with
actual["success"]
5842-5842: Do not compare types, use
isinstance()(E721)
8723-8723: f-string without any placeholders (F541)
Remove extraneous
fprefix
8919-8919: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
9263-9263: Local variable
response_oneis assigned to but never used (F841)Remove assignment to unused variable
response_one
9267-9267: Local variable
response_twois assigned to but never used (F841)Remove assignment to unused variable
response_two
9271-9271: Local variable
response_threeis assigned to but never used (F841)Remove assignment to unused variable
response_three
9288-9289: Replace
yieldoverforloop withyield from(UP028)Replace with
yield from
9294-9294: f-string without any placeholders (F541)
Remove extraneous
fprefix
9353-9353: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
9400-9400: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
9437-9437: f-string without any placeholders (F541)
Remove extraneous
fprefix
9917-9917: Local variable
response_delete_story_oneis assigned to but never used (F841)Remove assignment to unused variable
response_delete_story_one
9922-9922: Local variable
response_delete_story_twois assigned to but never used (F841)Remove assignment to unused variable
response_delete_story_two
10610-10610: Local variable
payload_responseis assigned to but never used (F841)Remove assignment to unused variable
payload_response
12399-12399: Use context handler for opening files (SIM115)
12423-12423: Use context handler for opening files (SIM115)
12444-12444: Use context handler for opening files (SIM115)
12708-12708: f-string without any placeholders (F541)
Remove extraneous
fprefix
13179-13179: Use context handler for opening files (SIM115)
13186-13186: Use context handler for opening files (SIM115)
13193-13193: Use context handler for opening files (SIM115)
13227-13227: Use context handler for opening files (SIM115)
13231-13231: Use context handler for opening files (SIM115)
13235-13235: Use context handler for opening files (SIM115)
13260-13260: Use context handler for opening files (SIM115)
13303-13303: Use context handler for opening files (SIM115)
13310-13310: Use context handler for opening files (SIM115)
13383-13383: Use context handler for opening files (SIM115)
13390-13390: Use context handler for opening files (SIM115)
13397-13397: Use context handler for opening files (SIM115)
13404-13404: Use context handler for opening files (SIM115)
13411-13411: Use context handler for opening files (SIM115)
13418-13418: Use context handler for opening files (SIM115)
13427-13427: Use context handler for opening files (SIM115)
13434-13434: Use context handler for opening files (SIM115)
13699-13699: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
13722-13722: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
13836-13836: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
14188-14188: Use context handler for opening files (SIM115)
14230-14230: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
14230-14230: Yoda conditions are discouraged, use
actual.get("data").get("whitelist") == Noneinstead (SIM300)Replace Yoda condition with
actual.get("data").get("whitelist") == None
14276-14276: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
14276-14276: Yoda conditions are discouraged, use
actual.get("data").get("whitelist") == Noneinstead (SIM300)Replace Yoda condition with
actual.get("data").get("whitelist") == None
14723-14723: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
14756-14756: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
16837-16837: Yoda conditions are discouraged (SIM300)
Replace Yoda condition
16838-16838: Yoda conditions are discouraged (SIM300)
Replace Yoda condition
16870-16870: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
16904-16904: Yoda conditions are discouraged, use
item["lookup"] == "case_insensitive_lookup"instead (SIM300)Replace Yoda condition with
item["lookup"] == "case_insensitive_lookup"
16912-16912: Yoda conditions are discouraged, use
item["lookup"] == "case_insensitive_lookup"instead (SIM300)Replace Yoda condition with
item["lookup"] == "case_insensitive_lookup"
17093-17093: f-string without any placeholders (F541)
Remove extraneous
fprefix
17104-17104: f-string without any placeholders (F541)
Remove extraneous
fprefix
17115-17115: f-string without any placeholders (F541)
Remove extraneous
fprefix
17128-17128: f-string without any placeholders (F541)
Remove extraneous
fprefix
17138-17138: f-string without any placeholders (F541)
Remove extraneous
fprefix
17147-17147: f-string without any placeholders (F541)
Remove extraneous
fprefix
17164-17164: f-string without any placeholders (F541)
Remove extraneous
fprefix
17170-17170: f-string without any placeholders (F541)
Remove extraneous
fprefix
17176-17176: f-string without any placeholders (F541)
Remove extraneous
fprefix
17196-17196: f-string without any placeholders (F541)
Remove extraneous
fprefix
17200-17200: f-string without any placeholders (F541)
Remove extraneous
fprefix
17206-17206: f-string without any placeholders (F541)
Remove extraneous
fprefix
17217-17217: f-string without any placeholders (F541)
Remove extraneous
fprefix
17234-17234: f-string without any placeholders (F541)
Remove extraneous
fprefix
17247-17247: f-string without any placeholders (F541)
Remove extraneous
fprefix
17252-17252: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
17260-17260: f-string without any placeholders (F541)
Remove extraneous
fprefix
17266-17266: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
17274-17274: f-string without any placeholders (F541)
Remove extraneous
fprefix
17280-17280: Use
bool(...)instead ofTrue if ... else False(SIM210)Replace with `bool(...)
17306-17306: f-string without any placeholders (F541)
Remove extraneous
fprefix
19125-19125: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
19166-19166: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
19180-19180: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
19721-19721: f-string without any placeholders (F541)
Remove extraneous
fprefix
21018-21018: Local variable
mockis assigned to but never used (F841)Remove assignment to unused variable
mock
21713-21713: Use context handler for opening files (SIM115)
21756-21756: Use context handler for opening files (SIM115)
21962-21962: Use context handler for opening files (SIM115)
22029-22029: Use context handler for opening files (SIM115)
22064-22064: Use context handler for opening files (SIM115)
22085-22085: Use context handler for opening files (SIM115)
22090-22090: Use context handler for opening files (SIM115)
22210-22210: Local variable
payloadis assigned to but never used (F841)Remove assignment to unused variable
payload
22311-22311: Loop control variable
inot used within loop body (B007)Rename unused
ito_i
22394-22394: f-string without any placeholders (F541)
Remove extraneous
fprefix
22429-22429: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
22429-22429: Yoda conditions are discouraged, use
actual.get("data").get("whitelist") == Noneinstead (SIM300)Replace Yoda condition with
actual.get("data").get("whitelist") == None
22484-22484: Use context handler for opening files (SIM115)
22518-22518: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
22518-22518: Yoda conditions are discouraged, use
actual.get("data").get("whitelist") == Noneinstead (SIM300)Replace Yoda condition with
actual.get("data").get("whitelist") == None
22544-22544: Comparison to
Noneshould becond is None(E711)Replace with
cond is None
22544-22544: Yoda conditions are discouraged, use
actual.get("data").get("whitelist") == Noneinstead (SIM300)Replace Yoda condition with
actual.get("data").get("whitelist") == None
22549-22549: Use context handler for opening files (SIM115)
22784-22784: Avoid equality comparisons to
False; useif not actual["data"]["logs"][1]["translate_responses"]:for false checks (E712)Replace with
not actual["data"]["logs"][1]["translate_responses"]
22785-22785: Avoid equality comparisons to
False; useif not actual["data"]["logs"][1]["translate_actions"]:for false checks (E712)Replace with
not actual["data"]["logs"][1]["translate_actions"]
22801-22801: f-string without any placeholders (F541)
Remove extraneous
fprefix
23047-23047: f-string without any placeholders (F541)
Remove extraneous
fprefix
23099-23099: f-string without any placeholders (F541)
Remove extraneous
fprefix
23492-23492: Unnecessary call to
encodeas UTF-8 (UP012)Rewrite as bytes literal
23523-23523: Unnecessary call to
encodeas UTF-8 (UP012)Rewrite as bytes literal
23634-23635: Replace
yieldoverforloop withyield from(UP028)Replace with
yield from
23757-23757: f-string without any placeholders (F541)
Remove extraneous
fprefix
23771-23771: f-string without any placeholders (F541)
Remove extraneous
fprefix
23864-23864: Local variable
passwrd_change_responseis assigned to but never used (F841)Remove assignment to unused variable
passwrd_change_response
23944-23944: Local variable
regsiter_responseis assigned to but never used (F841)Remove assignment to unused variable
regsiter_response
23976-23976: Local variable
regsiter_responseis assigned to but never used (F841)Remove assignment to unused variable
regsiter_response
24181-24181: Avoid equality comparisons to
False; useif not actual["success"]:for false checks (E712)Replace with
not actual["success"]
24185-24185: f-string without any placeholders (F541)
Remove extraneous
fprefix
Gitleaks
kairon/shared/constants.py
51-52: Identified a HashiCorp Terraform password field, risking unauthorized infrastructure configuration and security breaches. (hashicorp-tf-password)
tests/integration_test/services_test.py
235-235: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
9054-9054: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
9098-9098: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
9753-9753: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
10082-10082: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
10099-10099: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
17289-17289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
18635-18635: Identified a Slack Bot token, which may compromise bot integrations and communication channel security. (slack-bot-token)
18681-18681: Identified a Slack Bot token, which may compromise bot integrations and communication channel security. (slack-bot-token)
18703-18703: Identified a Slack Bot token, which may compromise bot integrations and communication channel security. (slack-bot-token)
18636-18636: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
18638-18638: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
18659-18659: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
18704-18704: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
18706-18706: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
18854-18854: Identified a Slack Bot token, which may compromise bot integrations and communication channel security. (slack-bot-token)
18855-18855: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
18857-18857: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
19572-19572: Identified a Slack Bot token, which may compromise bot integrations and communication channel security. (slack-bot-token)
21851-21851: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
21879-21879: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
21930-21930: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
21958-21958: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
21996-21996: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
22023-22023: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
23252-23252: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
23898-23898: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
Additional comments not posted (21)
kairon/shared/chat/broadcast/constants.py (2)
7-7: New enum value added:resendThe addition of
resendtoMessageBroadcastLogTypealigns with the new functionality for resending broadcasts. This is a necessary update to support logging for the new feature.
17-20: New error codes added toMetaErrorCodesThe addition of specific error codes (
message_undeliverable,recipient_sender_same,payment_error) is appropriate for handling distinct error scenarios in message broadcasting, particularly in the context of resending messages. These codes will facilitate more precise error handling and reporting.kairon/events/definitions/scheduled_base.py (2)
18-19: Added handling forresend_broadcastevent typeThe addition of
resend_broadcastto the dictionary of request implementations is necessary for the new feature to function correctly. This update ensures that the system can handle resend requests appropriately.
39-40: Abstract method_resend_broadcastaddedThis new abstract method is essential for subclasses to implement the resend functionality. It's a good practice to define such behaviors in the base class, ensuring consistency across different implementations.
Tools
Ruff
39-39:
typing.Textis deprecated, usestr(UP019)Replace with
strkairon/shared/constants.py (1)
85-85: New event request type added:resend_broadcastThe addition of
resend_broadcasttoEventRequestTypeis crucial for supporting the new resend functionality. This ensures that the application can correctly recognize and handle resend broadcast requests.kairon/events/definitions/message_broadcast.py (5)
53-53: Added handling foris_resendparameter inexecutemethodThe introduction of the
is_resendparameter allows the method to differentiate between initial sends and resends, which is crucial for the new functionality. This change is well-implemented and necessary for the feature.
57-61: Conditional handling based onis_resendThis logic correctly handles the flow for resending broadcasts versus sending new ones. It's a good implementation that ensures the correct method is called based on the
is_resendstatus.
68-69: Logging status after broadcastingThe insertion of the status into the webhook and logging the event are crucial for tracking the outcome of the broadcast, especially in the context of resends. This is a necessary update.
107-112: Implementing resend logic in_resend_broadcastThis method provides the necessary logic to handle resending of broadcasts. The use of
Utility.request_event_serverto trigger the resend is appropriate and aligns with existing patterns.Tools
Ruff
107-107:
typing.Textis deprecated, usestr(UP019)Replace with
str
148-157: Retrieving configuration for resendingThe method
__retrieve_confighas been updated to handle the resend scenario, which involves fetching specific configurations and logging the status. This is an essential part of the resend functionality.Tools
Ruff
148-148:
typing.Textis deprecated, usestr(UP019)Replace with
strkairon/shared/channels/broadcast/whatsapp.py (2)
11-11: Updated imports to includeMetaErrorCodesThe addition of
MetaErrorCodesto the imports is necessary for handling specific error scenarios related to message broadcasting on WhatsApp. This change is relevant and well-placed.
46-48: Comprehensive updates to support resend functionalityThe changes in methods like
send,__send_using_pyscript,__send_using_configuration, andresend_broadcastare extensive and crucial for implementing the resend functionality on the WhatsApp channel. These updates ensure that messages can be resent under the appropriate conditions and that errors are handled correctly.Also applies to: 50-63, 96-127, 136-161
kairon/shared/chat/broadcast/processor.py (3)
116-118: Method implementation is correct.The method
get_reference_id_from_broadcasting_logscorrectly retrieves the reference ID from broadcasting logs based on the given event ID.
121-125: Update to consideris_resendis appropriate.The updates to
extract_message_ids_from_broadcast_logsto consider theis_resendflag for settinglog_typeare correctly implemented. This ensures that the method can handle different scenarios based on whether the message is being resent or not.Tools
Ruff
121-121:
typing.Textis deprecated, usestr(UP019)Replace with
str
182-185: Correct implementation ofis_resendhandling in webhook status insertion.The method
insert_status_received_on_channel_webhookcorrectly uses theis_resendflag to process broadcast logs. This is an essential update for handling different broadcast scenarios.Tools
Ruff
182-182:
typing.Textis deprecated, usestr(UP019)Replace with
str
182-182:
typing.Textis deprecated, usestr(UP019)Replace with
strkairon/api/app/routers/bot/channels.py (1)
185-196: New endpoint for resending broadcasts is correctly implemented.The new endpoint
resend_message_broadcast_eventcorrectly handles the resending of message broadcasts by enqueuing the appropriate event type. This is a crucial addition for the resend functionality.Tools
Ruff
188-188: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)tests/integration_test/services_test.py (5)
2033-2056: Function implementation looks good and is well-documented.
2059-2081: Resend functionality test is comprehensive and checks all necessary conditions.
2083-2107: Listing broadcast messages test is implemented correctly.
2110-2119: Deletion functionality test is thorough and correctly implemented.
2121-2121: Function to test client configuration retrieval based on IP is well-implemented.
| is_resend = kwargs.pop("is_resend", False) | ||
| event_completion_states = [] if is_resend else [EVENT_STATUS.FAIL.value, EVENT_STATUS.COMPLETED.value] | ||
| is_new_log = log_type in {MessageBroadcastLogType.send.value, | ||
| MessageBroadcastLogType.resend.value, | ||
| MessageBroadcastLogType.self.value} or kwargs.pop("is_new_log", None) | ||
| try: | ||
| if is_new_log: | ||
| if is_new_log and not is_resend: |
There was a problem hiding this comment.
Refactor the event logging logic and improve exception handling.
The method can be simplified by directly using set operations and improving the clarity of exception handling:
- is_new_log = log_type in {MessageBroadcastLogType.send.value,
- MessageBroadcastLogType.resend.value,
- MessageBroadcastLogType.self.value} or kwargs.pop("is_new_log", None)
+ is_new_log = log_type in {MessageBroadcastLogType.send.value, MessageBroadcastLogType.resend.value, MessageBroadcastLogType.self.value} or kwargs.get("is_new_log", False)
- if is_new_log and not is_resend:
- raise DoesNotExist()
+ if not is_new_log or is_resend:
+ raise DoesNotExist() from NoneThis change clarifies the conditions under which the exception is raised and uses the correct form of raising exceptions with context.
Committable suggestion was skipped due to low confidence.
| def get_settings(notification_id: Text, bot: Text, **kwargs): | ||
| try: | ||
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=True).get() | ||
| status = False if kwargs.get("is_resend", False) else True | ||
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=status).get() |
There was a problem hiding this comment.
Optimize the condition check for is_resend.
Instead of using a ternary operation to set the status, use direct negation:
- status = False if kwargs.get("is_resend", False) else True
+ status = not kwargs.get("is_resend", False)This simplifies the logic and improves readability.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_settings(notification_id: Text, bot: Text, **kwargs): | |
| try: | |
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=True).get() | |
| status = False if kwargs.get("is_resend", False) else True | |
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=status).get() | |
| 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() |
Tools
Ruff
21-21:
typing.Textis deprecated, usestr(UP019)Replace with
str
21-21:
typing.Textis deprecated, usestr(UP019)Replace with
str
23-23: Use
not ...instead ofFalse if ... else True(SIM211)Replace with
not ...
| logs = MessageBroadcastProcessor.get_broadcast_logs(bot) | ||
| assert len(logs[0]) == logs[1] == 3 | ||
| logs[0][0].pop("timestamp") | ||
| reference_id = logs[0][0].get("reference_id") |
There was a problem hiding this comment.
Remove unused variable.
The variable reference_id is assigned but never used. Consider removing it if it's not needed.
- reference_id = logs[0][0].get("reference_id")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reference_id = logs[0][0].get("reference_id") |
Tools
Ruff
2340-2340: Local variable
reference_idis assigned to but never used (F841)Remove assignment to unused variable
reference_id
| @@ -23,8 +23,6 @@ | |||
|
|
|||
| os.environ["system_file"] = "./tests/testing_data/system.yaml" | |||
There was a problem hiding this comment.
Use capitalized environment variable name.
It's a common convention to use uppercase for environment variable names.
- os.environ["system_file"] = "./tests/testing_data/system.yaml"
+ os.environ["SYSTEM_FILE"] = "./tests/testing2data/system.yaml"Committable suggestion was skipped due to low confidence.
Tools
Ruff
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_file(SIM112)Replace
system_filewithSYSTEM_FILE
| response = client.post( | ||
| f"/api/bot/{pytest.bot}/channels/whatsapp/360dialog/onboarding?clientId=kairon&client=sdfgh5678&channels=['sdfghjk678']", |
There was a problem hiding this comment.
Tip
Codebase Verification
Correct the URL encoding for query parameters.
The channels query parameter in the URL is not correctly URL-encoded. Ensure that special characters like square brackets [ and ] are properly encoded.
- Location:
tests/integration_test/services_test.py, lines around 2018-2019
Suggested Correction:
response = client.post(
f"/api/bot/{pytest.bot}/channels/whatsapp/360dialog/onboarding?clientId=kairon&client=sdfgh5678&channels=%5B'sdfghjk678'%5D",
headers={
"Authorization": pytest.token_type + " " + pytest.access_token
},
)Analysis chain
Verify the correctness of the API endpoint and query parameters.
The URL in the POST request seems to have a typo or incorrect format. Ensure that the query parameters are correctly encoded and the URL is structured properly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the correctness of the API endpoint and query parameters.
# Test: Search for the endpoint usage. Expect: Only occurances of the correct endpoint.
rg --type python $'http://kairon-api.digite.com/api/bot/whatsapp/{pytest.bot}/e'
Length of output: 271
Script:
#!/bin/bash
# Description: Verify the correctness of the API endpoint and query parameters.
# Test: Search for the endpoint usage. Expect: Only occurrences of the correct endpoint.
# Adjusting the search pattern to avoid regex interpretation issues.
rg --type python 'http://kairon-api.digite.com/api/bot/whatsapp/\{pytest.bot\}/e'
Length of output: 114
Script:
#!/bin/bash
# Description: Verify the correctness of the API endpoint and query parameters.
# Test: Search for the endpoint usage. Expect: Only occurrences of the correct endpoint.
# Adjusting the search pattern to avoid regex interpretation issues and correctly filter Python files.
rg 'http://kairon-api.digite.com/api/bot/whatsapp/\{pytest.bot\}/e' --glob '*.py'
Length of output: 419
Script:
#!/bin/bash
# Description: Verify the correctness of the API endpoint and query parameters.
# Display the specific lines containing the endpoint in the test file to verify the format.
rg 'http://kairon-api.digite.com/api/bot/whatsapp/\{pytest.bot\}/e' --glob '*.py' -A 5 -B 5
Length of output: 2560
Script:
#!/bin/bash
# Description: Verify the correctness of the API endpoint and query parameters.
# Search for the POST request in the test file to verify the URL and query parameters.
rg 'client.post' tests/integration_test/services_test.py -A 5 -B 5
Length of output: 201938
| is_resend = kwargs.get('is_resend') | ||
| recipients = kwargs.get('recipients') | ||
|
|
||
| pattern = r"contacts = \[.*?\]" |
| msg_id = log["message_id"] | ||
| broadcast_log = broadcast_logs[msg_id] | ||
| if log['errors']: | ||
| if log['errors'][0].get('code') in [MetaErrorCodes.message_undeliverable.value, |
| numbers_sent.append(broadcast_log['recipient']) | ||
|
|
||
| numbers_to_exclude = numbers_sent + numbers_failed | ||
| numbers_to_send = [number for number in all_numbers if number not in numbers_to_exclude] |
There was a problem hiding this comment.
check any better way to iterate
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range and nitpick comments (1)
kairon/shared/chat/broadcast/processor.py (1)
Line range hint
168-192: Replacetyping.Textwithstr.
typing.Textis deprecated. Usestrinstead.- def __add_broadcast_logs_status_and_errors(reference_id: Text, campaign_name: Text, broadcast_logs: Dict[Text, Document], resend_count: int = 0): + def __add_broadcast_logs_status_and_errors(reference_id: str, campaign_name: str, broadcast_logs: Dict[str, Document], resend_count: int = 0):Tools
Ruff
195-195:
typing.Textis deprecated, usestrReplace with
str(UP019)
195-195:
typing.Textis deprecated, usestrReplace with
str(UP019)
202-202:
typing.Textis deprecated, usestrReplace with
str(UP019)
202-202:
typing.Textis deprecated, usestrReplace with
str(UP019)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- kairon/api/app/routers/bot/channels.py (1 hunks)
- kairon/events/definitions/message_broadcast.py (3 hunks)
- kairon/events/definitions/scheduled_base.py (2 hunks)
- kairon/shared/channels/broadcast/whatsapp.py (3 hunks)
- kairon/shared/chat/broadcast/constants.py (1 hunks)
- kairon/shared/chat/broadcast/processor.py (5 hunks)
- system.yaml (1 hunks)
- tests/testing_data/system.yaml (1 hunks)
- tests/unit_test/events/events_test.py (7 hunks)
Files not summarized due to errors (1)
- tests/unit_test/events/events_test.py: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
- system.yaml
- tests/testing_data/system.yaml
Files skipped from review as they are similar to previous changes (2)
- kairon/shared/channels/broadcast/whatsapp.py
- kairon/shared/chat/broadcast/constants.py
Additional context used
Ruff
kairon/events/definitions/scheduled_base.py
21-21: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
35-35:
typing.Textis deprecated, usestrReplace with
str(UP019)
39-39:
typing.Textis deprecated, usestrReplace with
str(UP019)
43-43:
typing.Textis deprecated, usestrReplace with
str(UP019)
kairon/events/definitions/message_broadcast.py
104-104: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
106-106:
typing.Textis deprecated, usestrReplace with
str(UP019)
116-116:
typing.Textis deprecated, usestrReplace with
str(UP019)
147-147:
typing.Textis deprecated, usestrReplace with
str(UP019)
kairon/shared/chat/broadcast/processor.py
21-21:
typing.Textis deprecated, usestrReplace with
str(UP019)
21-21:
typing.Textis deprecated, usestrReplace with
str(UP019)
84-84:
typing.Textis deprecated, usestrReplace with
str(UP019)
84-84:
typing.Textis deprecated, usestrReplace with
str(UP019)
84-84:
typing.Textis deprecated, usestrReplace with
str(UP019)
84-84:
typing.Textis deprecated, usestrReplace with
str(UP019)
129-129:
typing.Textis deprecated, usestrReplace with
str(UP019)
144-144:
typing.Textis deprecated, usestrReplace with
str(UP019)
168-168:
typing.Textis deprecated, usestrReplace with
str(UP019)
168-168:
typing.Textis deprecated, usestrReplace with
str(UP019)
169-169:
typing.Textis deprecated, usestrReplace with
str(UP019)
195-195:
typing.Textis deprecated, usestrReplace with
str(UP019)
195-195:
typing.Textis deprecated, usestrReplace with
str(UP019)
202-202:
typing.Textis deprecated, usestrReplace with
str(UP019)
202-202:
typing.Textis deprecated, usestrReplace with
str(UP019)
kairon/api/app/routers/bot/channels.py
188-188: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
tests/unit_test/events/events_test.py
24-24: Use capitalized environment variable
SYSTEM_FILEinstead ofsystem_fileReplace
system_filewithSYSTEM_FILE(SIM112)
Additional comments not posted (6)
kairon/shared/chat/broadcast/processor.py (2)
130-141: Replacetyping.Textwithstr.
typing.Textis deprecated. Usestrinstead.- def extract_message_ids_from_broadcast_logs(reference_id: Text): + def extract_message_ids_from_broadcast_logs(reference_id: str):Likely invalid or redundant comment.
115-117: Replacetyping.Textwithstr.
typing.Textis deprecated. Usestrinstead.- def get_reference_id_from_broadcasting_logs(event_id: Text): + def get_reference_id_from_broadcasting_logs(event_id: str):Likely invalid or redundant comment.
tests/unit_test/events/events_test.py (4)
1282-1283: Ensure Consistency in Log DataThe log data for
template_paramsandtemplate_nameshould be consistent with the rest of the log structure. Consider verifying that all logs have these fields correctly populated.
2133-2400: Ensure Proper Handling of Resend BroadcastThe function
test_execute_message_broadcast_with_resend_broadcast_with_static_valuestests the resend broadcast functionality with static values. Ensure that the resend logic is correctly implemented and the logs are properly handled.
2401-2677: Ensure Proper Handling of Resend BroadcastThe function
test_execute_message_broadcast_with_resend_broadcast_with_dynamic_valuestests the resend broadcast functionality with dynamic values. Ensure that the resend logic is correctly implemented and the logs are properly handled.
Line range hint
2678-3030: Ensure Proper Handling of Resend Broadcast with Meta Error CodesThe function
test_execute_message_broadcast_with_resend_broadcast_with_meta_error_codes_to_skiptests the resend broadcast functionality with meta error codes to skip. Ensure that the resend logic is correctly implemented and the logs are properly handled.
| def _resend_broadcast(self, msg_broadcast_id: Text): | ||
| raise NotImplementedError("Provider not implemented") |
There was a problem hiding this comment.
Replace typing.Text with str.
typing.Text is deprecated. Use str instead.
- def _resend_broadcast(self, msg_broadcast_id: Text):
+ def _resend_broadcast(self, msg_broadcast_id: str):Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _resend_broadcast(self, msg_broadcast_id: Text): | |
| raise NotImplementedError("Provider not implemented") | |
| def _resend_broadcast(self, msg_broadcast_id: str): | |
| raise NotImplementedError("Provider not implemented") |
Tools
Ruff
39-39:
typing.Textis deprecated, usestrReplace with
str(UP019)
| EventRequestType.update_schedule.value: self._update_schedule, | ||
| EventRequestType.resend_broadcast.value: self._resend_broadcast |
There was a problem hiding this comment.
Remove .keys() for dictionary lookup.
Use if event_request_type not in request_implementation instead of if event_request_type not in request_implementation.keys() for better performance.
- if event_request_type not in request_implementation.keys():
+ if event_request_type not in request_implementation:Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| EventRequestType.update_schedule.value: self._update_schedule, | |
| EventRequestType.resend_broadcast.value: self._resend_broadcast | |
| EventRequestType.update_schedule.value: self._update_schedule, | |
| EventRequestType.resend_broadcast.value: self._resend_broadcast | |
| if event_request_type not in request_implementation: |
| is_resend = kwargs.get('is_resend', False) | ||
| try: | ||
| config, reference_id = self.__retrieve_config(event_id) | ||
| config, reference_id = self.__retrieve_config(event_id, is_resend) | ||
| broadcast = MessageBroadcastFactory.get_instance(config["connector_type"]).from_config(config, event_id, reference_id) | ||
| recipients = broadcast.get_recipients() | ||
| broadcast.send(recipients) | ||
| if is_resend: | ||
| broadcast.resend_broadcast() |
There was a problem hiding this comment.
Replace typing.Text with str and approve changes.
typing.Text is deprecated. Use str instead.
- def execute(self, event_id: Text, **kwargs):
+ def execute(self, event_id: str, **kwargs):The addition of the is_resend parameter is consistent with the new functionality.
Committable suggestion was skipped due to low confidence.
| def _resend_broadcast(self, msg_broadcast_id: Text): | ||
| try: | ||
| payload = {'bot': self.bot, 'user': self.user, | ||
| "event_id": msg_broadcast_id, "is_resend": True} | ||
| Utility.request_event_server(EventClass.message_broadcast, payload) | ||
| return msg_broadcast_id | ||
| except Exception as e: | ||
| logger.error(e) | ||
| raise e |
There was a problem hiding this comment.
Replace typing.Text with str.
typing.Text is deprecated. Use str instead.
- def _resend_broadcast(self, msg_broadcast_id: Text):
+ def _resend_broadcast(self, msg_broadcast_id: str):Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _resend_broadcast(self, msg_broadcast_id: Text): | |
| try: | |
| payload = {'bot': self.bot, 'user': self.user, | |
| "event_id": msg_broadcast_id, "is_resend": True} | |
| Utility.request_event_server(EventClass.message_broadcast, payload) | |
| return msg_broadcast_id | |
| except Exception as e: | |
| logger.error(e) | |
| raise e | |
| def _resend_broadcast(self, msg_broadcast_id: str): | |
| try: | |
| payload = {'bot': self.bot, 'user': self.user, | |
| "event_id": msg_broadcast_id, "is_resend": True} | |
| Utility.request_event_server(EventClass.message_broadcast, payload) | |
| return msg_broadcast_id | |
| except Exception as e: | |
| logger.error(e) | |
| raise e |
Tools
Ruff
106-106:
typing.Textis deprecated, usestrReplace with
str(UP019)
| def __retrieve_config(self, event_id: Text, is_resend: bool): | ||
|
|
||
| reference_id = MessageBroadcastProcessor.get_reference_id_from_broadcasting_logs(event_id) \ | ||
| if is_resend else ObjectId().__str__() | ||
| config = MessageBroadcastProcessor.get_settings(event_id, self.bot, is_resend=is_resend) | ||
| bot_settings = MongoProcessor.get_bot_settings(self.bot, self.user) | ||
| config["pyscript_timeout"] = bot_settings["dynamic_broadcast_execution_timeout"] | ||
| MessageBroadcastProcessor.add_event_log( | ||
| self.bot, MessageBroadcastLogType.common.value, reference_id, user=self.user, config=config, | ||
| status=EVENT_STATUS.INPROGRESS.value, event_id=event_id, is_new_log=True | ||
| status=EVENT_STATUS.INPROGRESS.value, event_id=event_id, is_new_log=True, is_resend=is_resend |
There was a problem hiding this comment.
Replace typing.Text with str and approve changes.
typing.Text is deprecated. Use str instead.
- def __retrieve_config(self, event_id: Text, is_resend: bool):
+ def __retrieve_config(self, event_id: str, is_resend: bool):The addition of the is_resend parameter is consistent with the new functionality.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __retrieve_config(self, event_id: Text, is_resend: bool): | |
| reference_id = MessageBroadcastProcessor.get_reference_id_from_broadcasting_logs(event_id) \ | |
| if is_resend else ObjectId().__str__() | |
| config = MessageBroadcastProcessor.get_settings(event_id, self.bot, is_resend=is_resend) | |
| bot_settings = MongoProcessor.get_bot_settings(self.bot, self.user) | |
| config["pyscript_timeout"] = bot_settings["dynamic_broadcast_execution_timeout"] | |
| MessageBroadcastProcessor.add_event_log( | |
| self.bot, MessageBroadcastLogType.common.value, reference_id, user=self.user, config=config, | |
| status=EVENT_STATUS.INPROGRESS.value, event_id=event_id, is_new_log=True | |
| status=EVENT_STATUS.INPROGRESS.value, event_id=event_id, is_new_log=True, is_resend=is_resend | |
| def __retrieve_config(self, event_id: str, is_resend: bool): | |
| reference_id = MessageBroadcastProcessor.get_reference_id_from_broadcasting_logs(event_id) \ | |
| if is_resend else ObjectId().__str__() | |
| config = MessageBroadcastProcessor.get_settings(event_id, self.bot, is_resend=is_resend) | |
| bot_settings = MongoProcessor.get_bot_settings(self.bot, self.user) | |
| config["pyscript_timeout"] = bot_settings["dynamic_broadcast_execution_timeout"] | |
| MessageBroadcastProcessor.add_event_log( | |
| self.bot, MessageBroadcastLogType.common.value, reference_id, user=self.user, config=config, | |
| status=EVENT_STATUS.INPROGRESS.value, event_id=event_id, is_new_log=True, is_resend=is_resend |
Tools
Ruff
147-147:
typing.Textis deprecated, usestrReplace with
str(UP019)
| broadcast_logs, resend_count = MessageBroadcastProcessor.extract_message_ids_from_broadcast_logs(reference_id) | ||
| if broadcast_logs: | ||
| MessageBroadcastProcessor.__add_broadcast_logs_status_and_errors(reference_id, broadcast_name, broadcast_logs) | ||
| MessageBroadcastProcessor.__add_broadcast_logs_status_and_errors(reference_id, broadcast_name, | ||
| broadcast_logs, resend_count) |
There was a problem hiding this comment.
Replace typing.Text with str.
typing.Text is deprecated. Use str instead.
- def insert_status_received_on_channel_webhook(reference_id: Text, broadcast_name: Text):
+ def insert_status_received_on_channel_webhook(reference_id: str, broadcast_name: str):Committable suggestion was skipped due to low confidence.
| def get_settings(notification_id: Text, bot: Text, **kwargs): | ||
| try: | ||
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=True).get() | ||
| status = not kwargs.get("is_resend", False) | ||
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=status).get() |
There was a problem hiding this comment.
Replace typing.Text with str and approve changes.
typing.Text is deprecated. Use str instead.
- def get_settings(notification_id: Text, bot: Text, **kwargs):
+ def get_settings(notification_id: str, bot: str, **kwargs):The addition of the is_resend parameter is consistent with the new functionality.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_settings(notification_id: Text, bot: Text, **kwargs): | |
| try: | |
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=True).get() | |
| status = not kwargs.get("is_resend", False) | |
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=status).get() | |
| def get_settings(notification_id: str, bot: str, **kwargs): | |
| try: | |
| status = not kwargs.get("is_resend", False) | |
| settings = MessageBroadcastSettings.objects(id=notification_id, bot=bot, status=status).get() |
Tools
Ruff
21-21:
typing.Textis deprecated, usestrReplace with
str(UP019)
21-21:
typing.Textis deprecated, usestrReplace with
str(UP019)
| def add_event_log(bot: Text, log_type: Text, reference_id: Text = None, status: Text = None, **kwargs): | ||
| event_completion_states = [EVENT_STATUS.FAIL.value, EVENT_STATUS.COMPLETED.value] | ||
| is_new_log = log_type in {MessageBroadcastLogType.send.value, MessageBroadcastLogType.self.value} or kwargs.pop("is_new_log", None) | ||
| is_resend = kwargs.pop("is_resend", False) | ||
| event_completion_states = [] if is_resend else [EVENT_STATUS.FAIL.value, EVENT_STATUS.COMPLETED.value] | ||
| is_new_log = log_type in {MessageBroadcastLogType.send.value, | ||
| MessageBroadcastLogType.self.value} or kwargs.pop("is_new_log", None) |
There was a problem hiding this comment.
Replace typing.Text with str and approve changes.
typing.Text is deprecated. Use str instead.
- def add_event_log(bot: Text, log_type: Text, reference_id: Text = None, status: Text = None, **kwargs):
+ def add_event_log(bot: str, log_type: str, reference_id: str = None, status: str = None, **kwargs):The addition of the is_resend parameter is consistent with the new functionality.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def add_event_log(bot: Text, log_type: Text, reference_id: Text = None, status: Text = None, **kwargs): | |
| event_completion_states = [EVENT_STATUS.FAIL.value, EVENT_STATUS.COMPLETED.value] | |
| is_new_log = log_type in {MessageBroadcastLogType.send.value, MessageBroadcastLogType.self.value} or kwargs.pop("is_new_log", None) | |
| is_resend = kwargs.pop("is_resend", False) | |
| event_completion_states = [] if is_resend else [EVENT_STATUS.FAIL.value, EVENT_STATUS.COMPLETED.value] | |
| is_new_log = log_type in {MessageBroadcastLogType.send.value, | |
| MessageBroadcastLogType.self.value} or kwargs.pop("is_new_log", None) | |
| def add_event_log(bot: str, log_type: str, reference_id: str = None, status: str = None, **kwargs): | |
| is_resend = kwargs.pop("is_resend", False) | |
| event_completion_states = [] if is_resend else [EVENT_STATUS.FAIL.value, EVENT_STATUS.COMPLETED.value] | |
| is_new_log = log_type in {MessageBroadcastLogType.send.value, | |
| MessageBroadcastLogType.self.value} or kwargs.pop("is_new_log", None) |
Tools
Ruff
84-84:
typing.Textis deprecated, usestrReplace with
str(UP019)
84-84:
typing.Textis deprecated, usestrReplace with
str(UP019)
84-84:
typing.Textis deprecated, usestrReplace with
str(UP019)
84-84:
typing.Textis deprecated, usestrReplace with
str(UP019)
| @router.post("/broadcast/message/resend/{msg_broadcast_id}", response_model=Response) | ||
| async def resend_message_broadcast_event( | ||
| msg_broadcast_id: str, | ||
| current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS) |
There was a problem hiding this comment.
Avoid function call Security in argument defaults.
Move the Security function call inside the function.
- current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
+ current_user: User = Depends(lambda: Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS) | |
| current_user: User = Depends(lambda: Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)) |
Tools
Ruff
188-188: Do not perform function call
Securityin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
Added code related to Resend Broadcast.
Fixed and added test cases related to the same.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Tests