Manage Flows #1137
Manage Flows #1137
Conversation
…ws. Added unit and integration tests for the same.
…ate and get assets of the flows. Added unit and integration tests for the same.
WalkthroughWalkthroughThe recent update introduces API endpoints and backend functionalities for managing WhatsApp flows, including operations like adding, editing, previewing, retrieving, deleting, and publishing flows. It also involves validation of flow requests and handling file uploads in HTTP requests. A new Changes
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.
Review Status
Actionable comments generated: 20
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
tests/testing_data/flow/sample_flow.jsonis excluded by:!**/*.json
Files selected for processing (6)
- kairon/api/app/routers/bot/channels.py (2 hunks)
- kairon/shared/channels/whatsapp/bsp/dialog360.py (1 hunks)
- kairon/shared/constants.py (2 hunks)
- kairon/shared/utils.py (3 hunks)
- tests/integration_test/services_test.py (2 hunks)
- tests/unit_test/channels/bsp_test.py (2 hunks)
Additional comments: 21
kairon/shared/constants.py (2)
- 62-62: Ensure the addition of
flow_creationtoUserActivityTypeenum follows the existing naming convention and is correctly placed in the enum's logical order.- 130-138: The
FlowCategoriesenum addition is correctly defined and follows the naming conventions. Each category is clearly named, which should facilitate easy identification and use throughout the application.kairon/api/app/routers/bot/channels.py (7)
- 110-122: The endpoint for editing WhatsApp flows correctly handles file uploads and sets appropriate parameters. Ensure that the file handling process is secure and that the file content is validated before processing.
- 143-155: The preview endpoint is correctly defined and uses the GET method, which is appropriate for retrieving data. Ensure that the flow ID is validated to prevent unauthorized access.
- 158-170: The endpoint for getting WhatsApp flow assets is correctly implemented. Verify that the flow ID is validated and that the assets returned are properly sanitized to prevent any security issues.
- 173-185: The deprecation endpoint uses the POST method correctly. Ensure that the flow ID is validated and that appropriate permissions are checked before allowing a flow to be deprecated.
- 188-201: The retrieval endpoint for WhatsApp flows correctly uses the GET method and query parameters for filtering. Ensure that the filters are properly sanitized and validated to prevent injection attacks.
- 204-216: The delete endpoint for WhatsApp flows is correctly implemented using the DELETE method. Ensure that the flow ID is validated and that permissions are checked before deletion.
- 219-231: The publish endpoint for WhatsApp flows correctly uses the POST method. Verify that the flow ID is validated and that appropriate permissions are checked before publishing a flow.
kairon/shared/channels/whatsapp/bsp/dialog360.py (8)
- 94-109: The method for adding WhatsApp flows correctly validates the request data and constructs the HTTP request. Ensure that the data validation is comprehensive and that error handling is robust to cover all potential failure scenarios.
- 110-129: The method for editing WhatsApp flows handles file uploads. Verify that the file content is validated and sanitized before being sent to the external service to prevent any security vulnerabilities.
- 131-142: The preview method correctly constructs the HTTP request to preview a WhatsApp flow. Ensure that the flow ID is validated and that the response from the external service is properly sanitized before being returned to the client.
- 143-154: The method for getting WhatsApp flow assets correctly constructs the HTTP request. Verify that the flow ID is validated and that the assets returned are properly sanitized to prevent any security issues.
- 155-166: The deprecation method uses the POST method correctly. Ensure that the flow ID is validated and that appropriate permissions are checked before allowing a flow to be deprecated.
- 167-180: The method for listing WhatsApp flows correctly constructs the HTTP request with optional fields for filtering. Ensure that the filters are properly sanitized and validated to prevent injection attacks.
- 181-192: The method for deleting a WhatsApp flow is correctly implemented. Verify that the flow ID is validated and that permissions are checked before deletion to prevent unauthorized access.
- 193-203: The method for publishing a WhatsApp flow correctly constructs the HTTP request. Verify that the flow ID is validated and that appropriate permissions are checked before publishing a flow.
tests/unit_test/channels/bsp_test.py (1)
- 791-791: In
test_publish_flow, the success response is asserted but the flow's publish status is not verified in the system or database.Verify the flow's publish status in the system or database after the publish operation to ensure it reflects the expected state.
kairon/shared/utils.py (1)
- 1444-1453: The update to
execute_http_requestto handle file uploads is correct. Ensure that thefilesparameter is documented in the method's docstring to clarify its usage and expected format.tests/integration_test/services_test.py (2)
- 14227-14238: Ensure consistent error handling across all API endpoints. If "Channel not found!" is a common error, consider centralizing its handling.
- 14269-14280: The pattern of testing error scenarios with a mocked response indicating "Channel not found!" is consistent. Ensure that these tests cover all relevant error cases effectively.
| @staticmethod | ||
| def validate_add_flow_request(data: Dict): | ||
| required_keys = ['name', 'categories'] | ||
| 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') | ||
| invalid_categories = [category for category in categories | ||
| if category not in [flow_category.value for flow_category in FlowCategories]] | ||
| if invalid_categories: | ||
| raise AppException(f'Invalid categories {", ".join(invalid_categories)} in request body!') |
There was a problem hiding this comment.
The method validate_add_flow_request correctly checks for required keys and validates categories against the FlowCategories enum. However, consider handling the case where categories is not provided in the request data, which would cause a TypeError when attempting to iterate over None.
if categories is not None:
invalid_categories = [category for category in categories
if category not in [flow_category.value for flow_category in FlowCategories]]
if invalid_categories:
raise AppException(f'Invalid categories {", ".join(invalid_categories)} in request body!')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.
| @staticmethod | |
| def validate_add_flow_request(data: Dict): | |
| required_keys = ['name', 'categories'] | |
| 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') | |
| invalid_categories = [category for category in categories | |
| if category not in [flow_category.value for flow_category in FlowCategories]] | |
| if invalid_categories: | |
| raise AppException(f'Invalid categories {", ".join(invalid_categories)} in request body!') | |
| @staticmethod | |
| def validate_add_flow_request(data: Dict): | |
| required_keys = ['name', 'categories'] | |
| 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') | |
| if categories is not None: | |
| invalid_categories = [category for category in categories | |
| if category not in [flow_category.value for flow_category in FlowCategories]] | |
| if invalid_categories: | |
| raise AppException(f'Invalid categories {", ".join(invalid_categories)} in request body!') |
| @responses.activate | ||
| def test_add_whatsapp_flow_with_missing_keys(self): | ||
| bot = "62bc24b493a0d6b7a46328ff" | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "clone_flow_id": "9070429474112345" | ||
| } | ||
| with pytest.raises(AppException, match="Missing categories in request body!"): | ||
| BSP360Dialog(bot, "test").add_whatsapp_flow(data, bot, "test") |
There was a problem hiding this comment.
Missing unit test for successful scenario in test_add_whatsapp_flow_with_missing_keys.
Add a test case to cover the successful addition of a WhatsApp flow to ensure full coverage.
| @responses.activate | ||
| def test_add_whatsapp_flow_with_invalid_category(self): | ||
| bot = "62bc24b493a0d6b7a46328ff" | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY", "FLOW"] | ||
| } | ||
| with pytest.raises(AppException, match="Invalid categories FLOW in request body!"): | ||
| BSP360Dialog(bot, "test").add_whatsapp_flow(data, bot, "test") |
There was a problem hiding this comment.
Missing unit test for successful scenario in test_add_whatsapp_flow_with_invalid_category.
Add a test case to cover the successful addition of a WhatsApp flow with valid categories to ensure full coverage.
| def test_add_whatsapp_flow_error(self): | ||
| bot = "62bc24b493a0d6b7a46328fg" | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY"] | ||
| } | ||
|
|
||
| with pytest.raises(AppException, match="Channel not found!"): | ||
| BSP360Dialog(bot, "user").add_whatsapp_flow(data, bot, "user") |
There was a problem hiding this comment.
Missing unit test for successful scenario in test_add_whatsapp_flow_error.
Add a test case to cover the successful addition of a WhatsApp flow to ensure full coverage.
| @responses.activate | ||
| def test_add_whatsapp_flow_failure(self, monkeypatch): | ||
| with mock.patch.dict(Utility.environment, {'channels': {"360dialog": {"partner_id": "new_partner_id"}}}): | ||
| bot = "62bc24b493a0d6b7a46328ff" | ||
| partner_id = "new_partner_id" | ||
| waba_account_id = "Cyih7GWA" | ||
| data = { | ||
| "name": "flow with multiple categories", | ||
| "categories": ["APPOINTMENT_BOOKING", "OTHER", "SURVEY"] | ||
| } | ||
| def _get_partners_auth_token(*args, **kwargs): | ||
| return "Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIs.ImtpZCI6Ik1EZEZOVFk1UVVVMU9FSXhPRGN3UVVZME9EUTFRVFJDT1.RSRU9VUTVNVGhDTURWRk9UUTNPQSJ9" | ||
|
|
||
| monkeypatch.setattr(BSP360Dialog, 'get_partner_auth_token', _get_partners_auth_token) | ||
| base_url = Utility.system_metadata["channels"]["whatsapp"]["business_providers"]["360dialog"]["hub_base_url"] | ||
| url = f"{base_url}/api/v2/partners/{partner_id}/waba_accounts/{waba_account_id}/flows" | ||
| responses.add("POST", json={}, url=url, status=500) | ||
|
|
||
| with pytest.raises(AppException, match=r"Failed to add flow: *"): | ||
| BSP360Dialog(bot, "user").add_whatsapp_flow(data, bot, "user") |
There was a problem hiding this comment.
Missing unit test for successful scenario in test_add_whatsapp_flow_failure.
Add a test case to cover the successful addition of a WhatsApp flow to ensure full coverage.
| assert actual["error_code"] == 0 | ||
| assert actual["data"] == api_response | ||
|
|
||
|
|
There was a problem hiding this comment.
Missing implementation details for test_get_channel_endpoint. Ensure the test is fully implemented and covers the intended scenarios.
Would you like assistance in completing this test?
|
|
||
| response = BSP360Dialog(bot, "user").publish_flow(flow_id) | ||
| assert response == {"success": True} | ||
|
|
There was a problem hiding this comment.
In test_publish_flow, there's no validation of the flow's state before and after publishing to ensure the flow's status changes as expected.
Add assertions to verify the flow's status before and after the publish operation to ensure the flow is actually published.
| count = AuditLogData.objects(attributes=[{"key": "bot", "value": bot}], user="test", action="activity", | ||
| entity="template_creation").count() |
There was a problem hiding this comment.
In test_add_template, the assertion for the audit log count is outside the scope of the test's main functionality.
Consider moving audit log assertions to a separate test focused on auditing, or ensure the setup includes a clear state for audit logs to make this assertion meaningful.
| with mock.patch.dict(Utility.environment, {'channels': {"360dialog": {"partner_id": "new_partner_id"}}}): | ||
| bot = "62bc24b493a0d6b7a46328ff" | ||
| flow_id = "test_id" | ||
| partner_id = "new_partner_id" | ||
| waba_account_id = "Cyih7GWA" | ||
|
|
||
| def _get_partners_auth_token(*args, **kwargs): | ||
| return "Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIs.ImtpZCI6Ik1EZEZOVFk1UVVVMU9FSXhPRGN3UVVZME9EUTFRVFJDT1.RSRU9VUTVNVGhDTURWRk9UUTNPQSJ9" | ||
|
|
||
| monkeypatch.setattr(BSP360Dialog, 'get_partner_auth_token', _get_partners_auth_token) | ||
| base_url = Utility.system_metadata["channels"]["whatsapp"]["business_providers"]["360dialog"][ | ||
| "hub_base_url"] | ||
| url = f"{base_url}/api/v2/partners/{partner_id}/waba_accounts/{waba_account_id}/flows/{flow_id}" | ||
| responses.add("DELETE", json={"success": True}, url=url) | ||
|
|
||
| response = BSP360Dialog(bot, "user").delete_flow(flow_id) | ||
| assert response == {"success": True} |
There was a problem hiding this comment.
In test_delete_flow, there's no verification that the flow was actually deleted.
Add a step to verify the flow no longer exists after the delete operation to ensure the flow is actually removed.
| bot = "62bc24b493a0d6b7a46328ff" | ||
| flow_id = "test_id" | ||
| partner_id = "new_partner_id" | ||
| waba_account_id = "Cyih7GWA" | ||
|
|
||
| def _get_partners_auth_token(*args, **kwargs): | ||
| return "Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIs.ImtpZCI6Ik1EZEZOVFk1UVVVMU9FSXhPRGN3UVVZME9EUTFRVFJDT1.RSRU9VUTVNVGhDTURWRk9UUTNPQSJ9" | ||
|
|
||
| monkeypatch.setattr(BSP360Dialog, 'get_partner_auth_token', _get_partners_auth_token) | ||
| base_url = Utility.system_metadata["channels"]["whatsapp"]["business_providers"]["360dialog"][ | ||
| "hub_base_url"] | ||
| url = f"{base_url}/api/v2/partners/{partner_id}/waba_accounts/{waba_account_id}/flows/{flow_id}/publish" | ||
| responses.add("POST", json={"success": True}, url=url) | ||
|
|
||
| response = BSP360Dialog(bot, "user").publish_flow(flow_id) | ||
| assert response == {"success": True} |
There was a problem hiding this comment.
In test_publish_flow, there's no verification of the flow's state before and after publishing.
Add steps to verify the flow's status before and after the publish operation to ensure the flow is actually published.
Added APIs to create, update, retrieve and delete and publish, deprecate and get assets of the flows. Added unit and integration tests for the same.
Summary by CodeRabbit
FlowCategoriesenum to classify WhatsApp flows into categories such as SIGN_UP, SIGN_IN, APPOINTMENT_BOOKING, LEAD_GENERATION, CONTACT_US, CUSTOMER_SUPPORT, SURVEY, and OTHER.dialog360.pyfor better interaction with external services.