Whatsapp media upload2 #1939
Whatsapp media upload2 #1939
Conversation
WalkthroughThis update introduces the capability to handle WhatsApp multimedia messages by capturing and saving media identifiers as part of the message processing flow. A new static method, Changes
Sequence Diagram(s)sequenceDiagram
participant WhatsAppAPI as WhatsApp API
participant ChannelHandler as Whatsapp ChannelHandler
participant UserMedia as UserMedia
participant Processor as AgentProcessor
WhatsAppAPI->>ChannelHandler: Incoming multimedia message (with media ID)
ChannelHandler->>UserMedia: save_whatsapp_media_content(bot, sender_id, media_id, config)
UserMedia-->>ChannelHandler: [media_id]
ChannelHandler->>Processor: handle_channel_message(..., media_ids=[media_id])
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 1
🧹 Nitpick comments (5)
kairon/shared/chat/user_media.py (3)
119-194: Well-structured implementation for downloading WhatsApp mediaThe new
save_whatsapp_media_contentmethod provides a comprehensive solution for downloading media from WhatsApp BSPs. The implementation handles both 360dialog and Meta providers with appropriate error handling and efficient chunked downloads.I especially like the use of:
- Provider-specific logic separation
- Proper HTTP request timeout settings
- Chunked downloads to handle large files efficiently
- Asynchronous task scheduling for non-blocking operation
Consider adding a comment explaining why the 360dialog URL needs replacement on line 143 for better code maintainability.
146-146: Potential typo in filename prefixThere's a typo in the filename string "whataspp_360_" - it should be "whatsapp_360_".
- file_path = f"whataspp_360_{whatsapp_media_id}{extension}" + file_path = f"whatsapp_360_{whatsapp_media_id}{extension}"
162-162: Identical typo in Meta filename prefixThe same "whataspp" typo appears in the Meta filename prefix.
- file_path = f"whataspp_meta_{whatsapp_media_id}{extension}" + file_path = f"whatsapp_meta_{whatsapp_media_id}{extension}"tests/unit_test/chat/user_media_test.py (1)
404-406: Unused helper functionThe
created_corosfunction doesn't appear to be used anywhere in the tests.Consider removing this unused helper function:
-def created_coros(coros): - return coros -kairon/chat/handlers/channels/whatsapp.py (1)
60-70: Consider adding error handling for media download failuresThe current implementation doesn't handle failures that might occur during media download. If an exception is raised by
save_whatsapp_media_content, it will bubble up and be caught by the outer try/except in_handle_user_message, but specific error handling for media download would provide a better user experience.Consider adding specific error handling:
elif message.get("type") in {"image", "audio", "document", "video", "voice"}: if message['type'] == "voice": message['type'] = "audio" text = f"/k_multimedia_msg{{\"{message['type']}\": \"{message[message['type']]['id']}\"}}" - media_ids = UserMedia.save_whatsapp_media_content( - bot=bot, - sender_id=message["from"], - whatsapp_media_id=message[message['type']]['id'], - config=self.config - ) + try: + media_ids = UserMedia.save_whatsapp_media_content( + bot=bot, + sender_id=message["from"], + whatsapp_media_id=message[message['type']]['id'], + config=self.config + ) + except Exception as e: + logger.error(f"Failed to download WhatsApp media: {str(e)}") + # Continue with message processing even if media download fails
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
kairon/chat/handlers/channels/whatsapp.py(6 hunks)kairon/shared/chat/user_media.py(6 hunks)tests/unit_test/channels/whatsapp_test.py(2 hunks)tests/unit_test/chat/user_media_test.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
kairon/shared/chat/user_media.py (3)
tests/unit_test/utility_test.py (1)
bot(3273-3274)tests/unit_test/live_agent_test.py (1)
sender_id(27-28)kairon/shared/models.py (1)
UserMediaUploadType(132-134)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (python)
- GitHub Check: Python CI
🔇 Additional comments (10)
kairon/shared/chat/user_media.py (1)
177-178: Use UUID generation strategy consistent with other methodsThe current implementation uses
uuid7().hexfor generating media IDs. This appears consistent with other methods in the class, but it's worth verifying that this is the intended UUID generation strategy.tests/unit_test/chat/user_media_test.py (4)
327-366: Comprehensive test for 360dialog media downloadThe test effectively covers the happy path for downloading media from 360dialog, including verification of the correct API calls and parameters.
367-402: Good coverage of Meta provider media downloadThe test case properly verifies the functionality with the Meta WhatsApp BSP, including checking proper URL construction and parameter passing.
409-418: Effective error case testing for 360dialogThis test correctly verifies that the method raises an appropriate exception when the API call to 360dialog fails.
421-429: Effective error case testing for MetaThis test case properly verifies exception handling for the Meta API failure scenario.
tests/unit_test/channels/whatsapp_test.py (2)
443-444: Good test isolation with patchingPatching
UserMedia.save_whatsapp_media_contentensures the order message test can focus on message handling logic without requiring actual media downloads.
524-525: Consistent patching approach for attachment message testUsing the same patching approach for the attachment message test maintains consistency in testing methodology.
kairon/chat/handlers/channels/whatsapp.py (3)
15-15: Good module import for media handlingThe addition of the UserMedia import is appropriate for the new functionality.
64-69: Properly integrated media download for WhatsApp messagesThe implementation cleanly integrates the new media handling functionality into the existing message processing flow, calling
save_whatsapp_media_contentwhen relevant media types are detected.
165-165: Complete parameter propagation through the message handling pipelineThe signature updates to
_handle_user_messageandprocess_messageensure media IDs are properly propagated through the entire message handling pipeline.Also applies to: 181-182
| provider = config.get("bsp_type", "meta") | ||
| if provider == '360dialog': | ||
| endpoint = f'https://waba-v2.360dialog.io/{whatsapp_media_id}' | ||
| headers = { | ||
| 'D360-API-KEY': config.get('api_key'), | ||
| } | ||
| resp = requests.get(endpoint, headers=headers, stream=True) | ||
| if resp.status_code != 200: | ||
| raise AppException(f"Failed to download media from 360 dialog: {resp.status_code} - {resp.text}") | ||
| json_resp = resp.json() | ||
| download_url = json_resp.get("url") | ||
| download_url = download_url.replace('https://lookaside.fbsbx.com', 'https://waba-v2.360dialog.io') | ||
| mime_type = json_resp.get("mime_type") | ||
| extension = mimetypes.guess_extension(mime_type) or '' | ||
| file_path = f"whataspp_360_{whatsapp_media_id}{extension}" | ||
| elif provider == 'meta': | ||
| endpoint = f'https://graph.facebook.com/v22.0/{whatsapp_media_id}' | ||
| access_token = config.get('access_token') | ||
| headers = {'Authorization': f'Bearer {access_token}'} | ||
| media_info_resp = requests.get( | ||
| endpoint, | ||
| params={"fields": "url", "access_token": access_token}, | ||
| timeout=10 | ||
| ) | ||
| if media_info_resp.status_code != 200: | ||
| raise AppException(f"Failed to get url from meta for media: {whatsapp_media_id}") | ||
| json_resp = media_info_resp.json() | ||
| download_url = json_resp.get("url") | ||
| mime_type = json_resp.get("mime_type") | ||
| extension = mimetypes.guess_extension(mime_type) or '' | ||
| file_path = f"whataspp_meta_{whatsapp_media_id}{extension}" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for missing provider configuration
The code assumes that the required configuration values (api_key for 360dialog, access_token for Meta) will be present, but doesn't handle the case when they might be missing.
Add validation for required configuration values to provide clearer error messages when configurations are missing:
provider = config.get("bsp_type", "meta")
if provider == '360dialog':
+ api_key = config.get('api_key')
+ if not api_key:
+ raise AppException("Missing api_key in configuration for 360dialog provider")
endpoint = f'https://waba-v2.360dialog.io/{whatsapp_media_id}'
headers = {
- 'D360-API-KEY': config.get('api_key'),
+ 'D360-API-KEY': api_key,
}
# rest of the 360dialog logic...
elif provider == 'meta':
endpoint = f'https://graph.facebook.com/v22.0/{whatsapp_media_id}'
access_token = config.get('access_token')
+ if not access_token:
+ raise AppException("Missing access_token in configuration for Meta provider")
headers = {'Authorization': f'Bearer {access_token}'}
# rest of the Meta logic...
+ else:
+ raise AppException(f"Unsupported WhatsApp BSP provider: {provider}")📝 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.
| provider = config.get("bsp_type", "meta") | |
| if provider == '360dialog': | |
| endpoint = f'https://waba-v2.360dialog.io/{whatsapp_media_id}' | |
| headers = { | |
| 'D360-API-KEY': config.get('api_key'), | |
| } | |
| resp = requests.get(endpoint, headers=headers, stream=True) | |
| if resp.status_code != 200: | |
| raise AppException(f"Failed to download media from 360 dialog: {resp.status_code} - {resp.text}") | |
| json_resp = resp.json() | |
| download_url = json_resp.get("url") | |
| download_url = download_url.replace('https://lookaside.fbsbx.com', 'https://waba-v2.360dialog.io') | |
| mime_type = json_resp.get("mime_type") | |
| extension = mimetypes.guess_extension(mime_type) or '' | |
| file_path = f"whataspp_360_{whatsapp_media_id}{extension}" | |
| elif provider == 'meta': | |
| endpoint = f'https://graph.facebook.com/v22.0/{whatsapp_media_id}' | |
| access_token = config.get('access_token') | |
| headers = {'Authorization': f'Bearer {access_token}'} | |
| media_info_resp = requests.get( | |
| endpoint, | |
| params={"fields": "url", "access_token": access_token}, | |
| timeout=10 | |
| ) | |
| if media_info_resp.status_code != 200: | |
| raise AppException(f"Failed to get url from meta for media: {whatsapp_media_id}") | |
| json_resp = media_info_resp.json() | |
| download_url = json_resp.get("url") | |
| mime_type = json_resp.get("mime_type") | |
| extension = mimetypes.guess_extension(mime_type) or '' | |
| file_path = f"whataspp_meta_{whatsapp_media_id}{extension}" | |
| provider = config.get("bsp_type", "meta") | |
| if provider == '360dialog': | |
| api_key = config.get('api_key') | |
| if not api_key: | |
| raise AppException("Missing api_key in configuration for 360dialog provider") | |
| endpoint = f'https://waba-v2.360dialog.io/{whatsapp_media_id}' | |
| headers = { | |
| 'D360-API-KEY': api_key, | |
| } | |
| resp = requests.get(endpoint, headers=headers, stream=True) | |
| if resp.status_code != 200: | |
| raise AppException(f"Failed to download media from 360 dialog: {resp.status_code} - {resp.text}") | |
| json_resp = resp.json() | |
| download_url = json_resp.get("url") | |
| download_url = download_url.replace('https://lookaside.fbsbx.com', 'https://waba-v2.360dialog.io') | |
| mime_type = json_resp.get("mime_type") | |
| extension = mimetypes.guess_extension(mime_type) or '' | |
| file_path = f"whataspp_360_{whatsapp_media_id}{extension}" | |
| elif provider == 'meta': | |
| endpoint = f'https://graph.facebook.com/v22.0/{whatsapp_media_id}' | |
| access_token = config.get('access_token') | |
| if not access_token: | |
| raise AppException("Missing access_token in configuration for Meta provider") | |
| headers = {'Authorization': f'Bearer {access_token}'} | |
| media_info_resp = requests.get( | |
| endpoint, | |
| params={"fields": "url", "access_token": access_token}, | |
| timeout=10 | |
| ) | |
| if media_info_resp.status_code != 200: | |
| raise AppException(f"Failed to get url from meta for media: {whatsapp_media_id}") | |
| json_resp = media_info_resp.json() | |
| download_url = json_resp.get("url") | |
| mime_type = json_resp.get("mime_type") | |
| extension = mimetypes.guess_extension(mime_type) or '' | |
| file_path = f"whataspp_meta_{whatsapp_media_id}{extension}" | |
| else: | |
| raise AppException(f"Unsupported WhatsApp BSP provider: {provider}") |
1 participant
Summary by CodeRabbit