Manage Whatsapp Flows #1224
Manage Whatsapp Flows #1224
Conversation
2. Modified Add and Update Whatsapp flows APIs. 3. Added and fixed related test cases both unit and integration test cases.
WalkthroughThe recent updates introduce comprehensive WhatsApp flow management capabilities to the Kairon API. New endpoints have been added for creating, editing, previewing, retrieving, deprecating, deleting, and publishing WhatsApp flows. These functionalities are supported by backend changes, including new methods in the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant API
participant BSPFactory
participant Dialog360
User->>API: POST /add_whatsapp_flow
API->>BSPFactory: Create instance
BSPFactory->>Dialog360: add_whatsapp_flow(request_data)
Dialog360-->>BSPFactory: Flow added response
BSPFactory-->>API: Flow added response
API-->>User: Flow added response
sequenceDiagram
participant User
participant API
participant BSPFactory
participant Dialog360
User->>API: GET /preview_whatsapp_flow
API->>BSPFactory: Create instance
BSPFactory->>Dialog360: preview_whatsapp_flow(flow_id)
Dialog360-->>BSPFactory: Flow preview data
BSPFactory-->>API: Flow preview data
API-->>User: Flow preview data
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![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 (3)
tests/unit_test/channels/bsp_test.py (1)
Line range hint
19-19: Remove unused import ofmongomock.MongoClient.- from mongomock import MongoClientTools
Ruff
615-615: Local variable
flow_idis assigned to but never used
624-624: Local variable
flow_idis assigned to but never used
644-644: Local variable
flow_idis assigned to but never used
673-673: Local variable
flow_idis assigned to but never usedkairon/shared/utils.py (1)
Line range hint
992-992: Unused local variablee.The variable
ein theexecute_http_requestmethod is assigned but never used. Consider removing it or using it as needed.- except Exception as e: + except Exception:tests/integration_test/services_test.py (1)
19012-19029: Consider adding more detailed assertions to verify the structure and content of the error responses.Adding assertions to check specific fields in the error response can help ensure that the API is returning all expected information correctly.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- kairon/api/app/routers/bot/channels.py (1 hunks)
- kairon/shared/channels/whatsapp/bsp/dialog360.py (2 hunks)
- kairon/shared/constants.py (2 hunks)
- kairon/shared/utils.py (3 hunks)
- metadata/flows/default_meta_flows.json (1 hunks)
- metadata/flows/flow_json.json (1 hunks)
- tests/integration_test/services_test.py (2 hunks)
- tests/testing_data/flow/sample_flow.json (1 hunks)
- tests/unit_test/channels/bsp_test.py (2 hunks)
Files not summarized due to errors (1)
- metadata/flows/default_meta_flows.json: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
- metadata/flows/flow_json.json
- tests/testing_data/flow/sample_flow.json
Additional context used
Ruff
tests/unit_test/channels/bsp_test.py
19-19:
mongomock.MongoClientimported but unused
615-615: Local variable
flow_idis assigned to but never used
624-624: Local variable
flow_idis assigned to but never used
644-644: Local variable
flow_idis assigned to but never used
673-673: Local variable
flow_idis assigned to but never used
1007-1007: Local variable
partner_idis assigned to but never used
1008-1008: Local variable
channel_idis assigned to but never used
1152-1152: Local variable
partner_idis assigned to but never used
1153-1153: Local variable
account_idis assigned to but never used
1215-1215: Local variable
partner_idis assigned to but never used
1216-1216: Local variable
account_idis assigned to but never usedkairon/shared/utils.py
992-992: Local variable
eis assigned to but never usedtests/integration_test/services_test.py
835-835: f-string without any placeholders
845-845: f-string without any placeholders
2460-2460: Local variable
response_threeis assigned to but never used
2684-2684: Comparison to
Noneshould becond is None
5509-5509: Avoid equality comparisons to
True; useif actual["success"]:for truth checks
5718-5718: Do not compare types, use
isinstance()
8543-8543: f-string without any placeholders
9083-9083: Local variable
response_oneis assigned to but never used
9087-9087: Local variable
response_twois assigned to but never used
9091-9091: Local variable
response_threeis assigned to but never used
9114-9114: f-string without any placeholders
9257-9257: f-string without any placeholders
9737-9737: Local variable
response_delete_story_oneis assigned to but never used
9742-9742: Local variable
response_delete_story_twois assigned to but never used
10430-10430: Local variable
payload_responseis assigned to but never used
12528-12528: f-string without any placeholders
14048-14048: Comparison to
Noneshould becond is None
14094-14094: Comparison to
Noneshould becond is None
16911-16911: f-string without any placeholders
16922-16922: f-string without any placeholders
16933-16933: f-string without any placeholders
16946-16946: f-string without any placeholders
16956-16956: f-string without any placeholders
16965-16965: f-string without any placeholders
16982-16982: f-string without any placeholders
16988-16988: f-string without any placeholders
16994-16994: f-string without any placeholders
17014-17014: f-string without any placeholders
17018-17018: f-string without any placeholders
17024-17024: f-string without any placeholders
17035-17035: f-string without any placeholders
17052-17052: f-string without any placeholders
17065-17065: f-string without any placeholders
17078-17078: f-string without any placeholders
17092-17092: f-string without any placeholders
17124-17124: f-string without any placeholders
18943-18943: Comparison to
Noneshould becond is None
18984-18984: Comparison to
Noneshould becond is None
18998-18998: Comparison to
Noneshould becond is None
19525-19525: Local variable
datais assigned to but never used
19550-19550: Local variable
datais assigned to but never used
19578-19578: Local variable
datais assigned to but never used
19597-19597: Local variable
datais assigned to but never used
19616-19616: Local variable
datais assigned to but never used
19645-19645: Local variable
datais assigned to but never used
19845-19845: f-string without any placeholders
21142-21142: Local variable
mockis assigned to but never used
22499-22499: f-string without any placeholders
22534-22534: Comparison to
Noneshould becond is None
22623-22623: Comparison to
Noneshould becond is None
22649-22649: Comparison to
Noneshould becond is None
22889-22889: Avoid equality comparisons to
False; useif not actual["data"]["logs"][1]["translate_responses"]:for false checks
22890-22890: Avoid equality comparisons to
False; useif not actual["data"]["logs"][1]["translate_actions"]:for false checks
22906-22906: f-string without any placeholders
23152-23152: f-string without any placeholders
23204-23204: f-string without any placeholders
23862-23862: f-string without any placeholders
23876-23876: f-string without any placeholders
23969-23969: Local variable
passwrd_change_responseis assigned to but never used
24049-24049: Local variable
regsiter_responseis assigned to but never used
24081-24081: Local variable
regsiter_responseis assigned to but never used
24286-24286: Avoid equality comparisons to
False; useif not actual["success"]:for false checks
24290-24290: f-string without any placeholders
Additional comments not posted (54)
kairon/shared/constants.py (2)
65-65: Added new enum value forflow_creationunderUserActivityType.This addition aligns with the new functionalities introduced for managing WhatsApp flows, ensuring that user activities related to flow creation are properly logged.
135-154: Introduced new enumsFlowCategoriesandFlowTemplates.These enums are essential for categorizing and templating the new WhatsApp flows. This structured approach enhances maintainability and scalability of flow management.
kairon/api/app/routers/bot/channels.py (8)
109-121: Added new endpoint for adding WhatsApp flows.This endpoint correctly implements the functionality to add new WhatsApp flows as drafts, which is crucial for the dynamic management of flows in a business service provider context.
124-137: Added new endpoint for editing WhatsApp flows.This endpoint provides the functionality to edit existing WhatsApp flows, which is necessary for maintaining and updating the flows as per business requirements.
140-152: Added new endpoint for previewing WhatsApp flows.This endpoint facilitates the previewing of WhatsApp flows, allowing users to review changes before they are finalized, which enhances the user experience and reduces errors.
155-167: Added new endpoint for retrieving WhatsApp flow assets.This endpoint is well-implemented to fetch all assets related to a specific flow, supporting richer content management within flows.
170-182: Added new endpoint for deprecating WhatsApp flows.This functionality is crucial for managing the lifecycle of WhatsApp flows, allowing for the deprecation of outdated or irrelevant flows.
185-198: Added new endpoint for retrieving all WhatsApp flows.This endpoint effectively retrieves all WhatsApp flows, with support for filtering, which is essential for managing a large number of flows.
201-213: Added new endpoint for deleting WhatsApp flows.Properly implemented functionality for deleting WhatsApp flows, which is necessary for effective flow management and cleanup.
216-228: Added new endpoint for publishing WhatsApp flows.This endpoint enables the publishing of WhatsApp flows, transitioning them from draft to active status, which is a critical step in the flow management process.
kairon/shared/channels/whatsapp/bsp/dialog360.py (11)
86-93: Added methodget_flow_endpoint_urlto construct the URL for flow operations.This method is crucial for encapsulating the construction of URLs used in various flow operations, promoting code reuse and maintainability.
95-112: Added methodadd_whatsapp_flowto handle the addition of new WhatsApp flows.This method correctly implements the addition of new flows, including validation and logging, which are essential for robust flow management.
117-126: Added methodget_flow_json_from_templateto fetch flow JSON based on a template.This method enhances the flexibility of flow creation by allowing flows to be initialized from templates, which can streamline the flow setup process.
128-131: Added methodwrite_flow_json_into_fileto save flow JSON to a file.This utility method is essential for persisting flow configurations, which is important for auditability and version control of flow definitions.
133-152: Added methodedit_whatsapp_flowto handle the editing of existing WhatsApp flows.This method is well-implemented to support the editing of flows, including handling file uploads, which is crucial for managing complex flow configurations.
160-167: Added methodpreview_whatsapp_flowto fetch a preview of a WhatsApp flow.This method provides a crucial functionality for previewing flows, which can help in quality assurance and user acceptance testing before flows go live.
172-179: Added methodget_whatsapp_flow_assetsto retrieve assets for a specific flow.This method effectively fetches assets associated with a flow, supporting enhanced content management within the flow.
184-191: Added methoddeprecate_whatsapp_flowto mark a flow as deprecated.This method is essential for lifecycle management of flows, allowing outdated or irrelevant flows to be marked accordingly.
196-205: Added methodlist_whatsapp_flowsto retrieve all flows with optional fields.This method provides comprehensive functionality to list all flows, with support for field-based filtering, which is useful for managing a large number of flows.
210-217: Added methoddelete_flowto remove a specific WhatsApp flow.This method correctly implements the deletion of flows, which is necessary for effective flow management and cleanup.
222-229: Added methodpublish_flowto make a WhatsApp flow active.This method enables the transition of a flow from draft to active status, which is a critical functionality for making flows available to end-users.
tests/unit_test/channels/bsp_test.py (28)
294-304: Ensure proper error handling for missing keys in the request body.
305-315: Validate categories properly to avoid runtime errors.
316-326: Check for valid templates to prevent incorrect flow creation.
327-337: Handle the scenario where the channel is not found more gracefully.
338-359: Improve error handling when adding a WhatsApp flow fails.
360-396: Ensure that the successful addition of a WhatsApp flow is handled correctly.
397-405: Handle the scenario where the channel is not found during flow editing.
406-426: Improve error handling when editing a WhatsApp flow fails.
427-451: Ensure that the successful editing of a WhatsApp flow is handled correctly.
452-459: Handle the scenario where the channel is not found during asset retrieval.
460-479: Improve error handling when retrieving WhatsApp flow assets fails.
480-509: Ensure that the successful retrieval of WhatsApp flow assets is handled correctly.
510-517: Handle the scenario where the channel is not found during flow deprecation.
518-537: Improve error handling when deprecating a WhatsApp flow fails.
538-557: Ensure that the successful deprecation of a WhatsApp flow is handled correctly.
558-565: Handle the scenario where the channel is not found during flow preview.
566-585: Improve error handling when previewing a WhatsApp flow fails.
586-611: Ensure that the successful preview of a WhatsApp flow is handled correctly.
612-619: Handle the scenario where the channel is not found during flow listing.Tools
Ruff
615-615: Local variable
flow_idis assigned to but never used
620-639: Improve error handling when listing WhatsApp flows fails.Tools
Ruff
624-624: Local variable
flow_idis assigned to but never used
640-668: Ensure that the successful listing of WhatsApp flows with query parameters is handled correctly.Tools
Ruff
644-644: Local variable
flow_idis assigned to but never used
669-709: Ensure that the successful listing of WhatsApp flows is handled correctly.Tools
Ruff
673-673: Local variable
flow_idis assigned to but never used
710-717: Handle the scenario where the channel is not found during flow deletion.
718-737: Improve error handling when deleting a WhatsApp flow fails.
738-757: Ensure that the successful deletion of a WhatsApp flow is handled correctly.
758-765: Handle the scenario where the channel is not found during flow publication.
766-785: Improve error handling when publishing a WhatsApp flow fails.
786-805: Ensure that the successful publication of a WhatsApp flow is handled correctly.metadata/flows/default_meta_flows.json (4)
3-106: The JSON structure for WhatsApp flow categories and templates is well-formed and aligns with the new features described in the PR.
3-37: The categories defined in the JSON match the newFlowCategoriesenum additions and are correctly structured.
38-105: All templates have valid JSON strings forflow_jsonand theis_endpoint_configuredflags are used appropriately.
38-105: All templates necessary for the new WhatsApp flow functionalities are present and correctly defined.tests/integration_test/services_test.py (1)
19012-19029: Ensure consistent error handling across tests.Verification successful
The verification script has confirmed that the error handling in the tests is consistent, with multiple instances of assertions checking for the error code
422.
- The pattern
assert actual["error_code"] == 422is used consistently across the test file.- This indicates that the error handling follows a uniform pattern.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all error handling in tests is consistent and follows the same pattern. # Test: Search for error handling patterns in tests. Expect: Consistent usage of assertions for error codes and messages. rg --type python $'assert.*"error_code".*422' tests/integration_test/services_test.pyLength of output: 117
Script:
#!/bin/bash # Description: Verify that all error handling in tests is consistent and follows the same pattern. # Test: Search for error handling patterns in tests. Expect: Consistent usage of assertions for error codes and messages. rg --type py $'assert.*"error_code".*422' tests/integration_test/services_test.pyLength of output: 13237
| @patch("kairon.shared.channels.whatsapp.bsp.dialog360.BSP360Dialog.get_partner_auth_token", autospec=True) | ||
| def test_add_whatsapp_flow_error(mock_get_partner_auth_token): | ||
| mock_get_partner_auth_token.return_value = "Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIs.ImtpZCI6Ik1EZEZOVFk1UVVVMU9FSXhPRGN3UVVZME9EUTFRVFJDT1.RSRU9VUTVNVGhDTURWRk9UUTNPQSJ9" | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY"], | ||
| "template": "FLOWS_OFFSITE_CALL_TO_ACTION", | ||
| } | ||
| response = client.post( | ||
| f"/api/bot/{pytest.bot}/channels/whatsapp/flows/360dialog", | ||
| json={'data': data}, | ||
| headers={"Authorization": pytest.token_type + " " + pytest.access_token}, | ||
| ) | ||
| actual = response.json() | ||
| assert not actual["success"] | ||
| assert actual["error_code"] == 422 | ||
| assert actual["message"] == "Channel not found!" | ||
| assert actual["data"] is None |
There was a problem hiding this comment.
Refactor repeated mock setup code to a shared fixture or helper function.
The repeated setup code for mock_get_partner_auth_token can be refactored into a pytest fixture or a helper function to reduce duplication and improve maintainability.
| "name": "my first flow" | ||
| } | ||
| ] | ||
| mock_list_whatsapp_flows.return_value = api_response | ||
|
|
||
| response = client.get( | ||
| f"/api/bot/{pytest.bot}/channels/whatsapp/flows/360dialog", | ||
| headers={"Authorization": pytest.token_type + " " + pytest.access_token}, | ||
| ) | ||
| actual = response.json() | ||
| assert actual["success"] | ||
| assert actual["error_code"] == 0 | ||
| assert actual["data"]['flows'] == api_response | ||
|
|
||
|
|
||
| @patch("kairon.shared.channels.whatsapp.bsp.dialog360.BSP360Dialog.delete_flow", autospec=True) | ||
| def test_delete_whatsapp_flow(mock_delete_flow): | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY"] | ||
| } | ||
| api_response = {"success": True} | ||
| mock_delete_flow.return_value = api_response | ||
|
|
||
| response = client.delete( | ||
| f"/api/bot/{pytest.bot}/channels/whatsapp/flows/360dialog/test_flow_id", | ||
| headers={"Authorization": pytest.token_type + " " + pytest.access_token}, | ||
| ) | ||
| actual = response.json() | ||
| assert actual["success"] | ||
| assert actual["error_code"] == 0 | ||
| assert actual["data"] == api_response | ||
|
|
||
|
|
||
| @patch("kairon.shared.channels.whatsapp.bsp.dialog360.BSP360Dialog.publish_flow", autospec=True) | ||
| def test_publish_whatsapp_flow(mock_publish_flow): | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY"] | ||
| } | ||
| api_response = {"success": True} | ||
| mock_publish_flow.return_value = api_response | ||
|
|
||
| response = client.post( | ||
| f"/api/bot/{pytest.bot}/channels/whatsapp/flows/360dialog/test_flow_id/publish", | ||
| headers={"Authorization": pytest.token_type + " " + pytest.access_token}, | ||
| ) | ||
| actual = response.json() | ||
| assert actual["success"] | ||
| assert actual["error_code"] == 0 | ||
| assert actual["data"] == api_response | ||
|
|
||
|
|
||
| @patch("kairon.shared.channels.whatsapp.bsp.dialog360.BSP360Dialog.get_whatsapp_flow_assets", autospec=True) | ||
| def test_get_whatsapp_flow_assets(mock_get_whatsapp_flow_assets): | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY"] | ||
| } | ||
| api_response = { | ||
| "assets": [ | ||
| { | ||
| "asset_type": "FLOW_JSON", | ||
| "download_url": "https://mmg.whatsapp.net/m1/v/t24/An8n-Ot0L5sxj8lupzxfUTfHYhsdsdsdsRyqAZRySBcATvtUGgPjP76UJKS0wMopyj6SNTmNmf_F1pLAt04wbP3B9kFCIpvy7oOG6CM3HK4wFY61Z7TiLDxjGvzEgXjdog6A?ccb=10-5&oh=01_AdTq_Njj-foFgD0-KlMXq4AbdhrqLoNm6_CtlsxZxC03rA&oe=65F414CD&_nc_sid=471a72", | ||
| "name": "flow.json" | ||
| } | ||
| ], | ||
| "count": 1, | ||
| "total": 1 | ||
| } | ||
| mock_get_whatsapp_flow_assets.return_value = api_response | ||
|
|
||
| response = client.get( | ||
| f"/api/bot/{pytest.bot}/channels/whatsapp/flows/360dialog/test_flow_id/assets", | ||
| headers={"Authorization": pytest.token_type + " " + pytest.access_token}, | ||
| ) | ||
| actual = response.json() | ||
| assert actual["success"] | ||
| assert actual["error_code"] == 0 | ||
| assert actual["data"] == api_response | ||
|
|
||
|
|
||
| @patch("kairon.shared.channels.whatsapp.bsp.dialog360.BSP360Dialog.deprecate_whatsapp_flow", autospec=True) | ||
| def test_deprecate_whatsapp_flow(mock_deprecate_whatsapp_flow): | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY"] | ||
| } | ||
| api_response = {"success": True} | ||
| mock_deprecate_whatsapp_flow.return_value = api_response | ||
|
|
||
| response = client.post( | ||
| f"/api/bot/{pytest.bot}/channels/whatsapp/flows/360dialog/test_flow_id/deprecate", | ||
| headers={"Authorization": pytest.token_type + " " + pytest.access_token}, | ||
| ) | ||
| actual = response.json() | ||
| assert actual["success"] | ||
| assert actual["error_code"] == 0 | ||
| assert actual["data"] == api_response | ||
|
|
There was a problem hiding this comment.
Optimize test setup by reusing data structures.
Many tests are setting up similar data structures. Consider using a shared fixture or helper function to define these data structures, reducing redundancy and making the tests easier to maintain.
Tools
Ruff
19525-19525: Local variable
datais assigned to but never used
19550-19550: Local variable
datais assigned to but never used
19578-19578: Local variable
datais assigned to but never used
19597-19597: Local variable
datais assigned to but never used
19616-19616: Local variable
datais assigned to but never used
19645-19645: Local variable
datais assigned to but never used
|
|
||
| @patch("kairon.shared.channels.whatsapp.bsp.dialog360.BSP360Dialog.preview_whatsapp_flow", autospec=True) | ||
| def test_preview_whatsapp_flow(mock_preview_whatsapp_flow): | ||
| data = { |
There was a problem hiding this comment.
Remove unused local variables as indicated by static analysis.
- data = {
- "name": "flow with multiple categories",
- "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY"]
- }These data variables are defined but never used in the respective test functions, leading to unnecessary code clutter.
Also applies to: 19550-19550, 19578-19578, 19597-19597, 19616-19616, 19645-19645
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.
| data = { |
Tools
Ruff
19525-19525: Local variable
datais assigned to but never used
| def validate_add_flow_request(data: Dict): | ||
| required_keys = ['name', 'categories', 'template'] | ||
| missing_keys = [key for key in required_keys if key not in data] | ||
| if missing_keys: | ||
| raise AppException(f'Missing {", ".join(missing_keys)} in request body!') | ||
| categories = data.get('categories') | ||
| template = data.get('template') | ||
| invalid_categories = [category for category in categories | ||
| if category not in [flow_category.value for flow_category in FlowCategories]] | ||
| invalid_template = template if template not in [flow_template.value for flow_template in FlowTemplates] else "" | ||
| if invalid_categories: | ||
| raise AppException(f'Invalid categories {", ".join(invalid_categories)} in request body!') | ||
| if invalid_template: | ||
| raise AppException(f'Invalid template {template} in request body!') |
There was a problem hiding this comment.
Ensure proper validation of categories and template in validate_add_flow_request.
The method validate_add_flow_request checks for missing keys, invalid categories, and templates. However, it does not handle cases where categories or template might be empty or have incorrect types. Consider adding checks to ensure these fields are not empty and are of the expected type.
+ if not categories or not isinstance(categories, list):
+ raise AppException('Categories must be a non-empty list!')
+ if not template or not isinstance(template, str):
+ raise AppException('Template must be a non-empty string!')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.
| def validate_add_flow_request(data: Dict): | |
| required_keys = ['name', 'categories', 'template'] | |
| missing_keys = [key for key in required_keys if key not in data] | |
| if missing_keys: | |
| raise AppException(f'Missing {", ".join(missing_keys)} in request body!') | |
| categories = data.get('categories') | |
| template = data.get('template') | |
| invalid_categories = [category for category in categories | |
| if category not in [flow_category.value for flow_category in FlowCategories]] | |
| invalid_template = template if template not in [flow_template.value for flow_template in FlowTemplates] else "" | |
| if invalid_categories: | |
| raise AppException(f'Invalid categories {", ".join(invalid_categories)} in request body!') | |
| if invalid_template: | |
| raise AppException(f'Invalid template {template} in request body!') | |
| def validate_add_flow_request(data: Dict): | |
| required_keys = ['name', 'categories', 'template'] | |
| missing_keys = [key for key in required_keys if key not in data] | |
| if missing_keys: | |
| raise AppException(f'Missing {", ".join(missing_keys)} in request body!') | |
| categories = data.get('categories') | |
| template = data.get('template') | |
| if not categories or not isinstance(categories, list): | |
| raise AppException('Categories must be a non-empty list!') | |
| if not template or not isinstance(template, str): | |
| raise AppException('Template must be a non-empty string!') | |
| invalid_categories = [category for category in categories | |
| if category not in [flow_category.value for flow_category in FlowCategories]] | |
| invalid_template = template if template not in [flow_template.value for flow_template in FlowTemplates] else "" | |
| if invalid_categories: | |
| raise AppException(f'Invalid categories {", ".join(invalid_categories)} in request body!') | |
| if invalid_template: | |
| raise AppException(f'Invalid template {template} in request body!') |
| if kwargs.get('files'): | ||
| response = session.request( | ||
| request_method.upper(), http_url, data=request_body, headers=headers, | ||
| timeout=kwargs.get('timeout'), files=kwargs.get('files') | ||
| ) | ||
| else: | ||
| response = session.request( | ||
| request_method.upper(), http_url, json=request_body, headers=headers, | ||
| timeout=kwargs.get('timeout') | ||
| ) |
There was a problem hiding this comment.
Handle file uploads in HTTP requests more robustly.
The method execute_http_request handles file uploads but does not check if the files parameter is of the correct type. This could lead to errors if an incorrect type is passed. Consider adding a type check for the files parameter.
+ if kwargs.get('files') and not isinstance(kwargs.get('files'), dict):
+ raise AppException('Files must be provided as a dictionary!')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.
| if kwargs.get('files'): | |
| response = session.request( | |
| request_method.upper(), http_url, data=request_body, headers=headers, | |
| timeout=kwargs.get('timeout'), files=kwargs.get('files') | |
| ) | |
| else: | |
| response = session.request( | |
| request_method.upper(), http_url, json=request_body, headers=headers, | |
| timeout=kwargs.get('timeout') | |
| ) | |
| if kwargs.get('files'): | |
| if not isinstance(kwargs.get('files'), dict): | |
| raise AppException('Files must be provided as a dictionary!') | |
| response = session.request( | |
| request_method.upper(), http_url, data=request_body, headers=headers, | |
| timeout=kwargs.get('timeout'), files=kwargs.get('files') | |
| ) | |
| else: | |
| response = session.request( | |
| request_method.upper(), http_url, json=request_body, headers=headers, | |
| timeout=kwargs.get('timeout') | |
| ) |
Summary by CodeRabbit
New Features
Tests
Bug Fixes