feat(whatsapp): implement official WhatsApp Cloud API provider#3599
feat(whatsapp): implement official WhatsApp Cloud API provider#3599MeenalSinha wants to merge 2 commits intoohcnetwork:developfrom
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request establishes a new messaging module for Django. It includes app configuration, an abstract base provider class for messaging integrations, and a concrete WhatsApp Graph API provider implementation with configuration validation and webhook handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR introduces a new Key issues:
Confidence Score: 1/5Not safe to merge — two P0 issues will cause ImportError and AttributeError before any message can be sent. The PR cannot function at all in its current state: the base class it inherits from does not exist (causing an ImportError on import), and the Django settings it reads at instantiation time are not defined anywhere (causing an AttributeError). These are not theoretical concerns — they are guaranteed runtime failures. Tests are also missing, which is a stated requirement for review.
Important Files Changed
Reviews (1): Last reviewed commit: "feat(whatsapp): implement official Whats..." | Re-trigger Greptile |
| self.api_url = f"https://graph.facebook.com/v17.0/{settings.WHATSAPP_PHONE_NUMBER_ID}/messages" | ||
| self.headers = { | ||
| "Authorization": f"Bearer {settings.WHATSAPP_ACCESS_TOKEN}", | ||
| "Content-Type": "application/json", |
There was a problem hiding this comment.
Undefined Django settings cause
AttributeError at instantiation
Neither WHATSAPP_PHONE_NUMBER_ID nor WHATSAPP_ACCESS_TOKEN is defined in any settings file (config/settings/base.py, production.py, deployment.py, etc.). Direct attribute access on the settings object will raise AttributeError: 'Settings' object has no attribute 'WHATSAPP_ACCESS_TOKEN' as soon as WhatsAppProvider() is constructed.
The existing SnsBackend pattern uses getattr(settings, "SNS_ACCESS_KEY", None) and raises ImproperlyConfigured with a descriptive message when a required value is absent. This PR should follow the same approach:
- Add
WHATSAPP_PHONE_NUMBER_IDandWHATSAPP_ACCESS_TOKEN(defaulting toNone) toconfig/settings/base.py. - Guard access in
__init__withgetattr+ImproperlyConfigured, asSnsBackenddoes, rather than accessing the attributes directly.
|
|
||
| class WhatsAppProvider(BaseMessagingProvider): | ||
| def __init__(self): | ||
| self.api_url = f"https://graph.facebook.com/v17.0/{settings.WHATSAPP_PHONE_NUMBER_ID}/messages" |
There was a problem hiding this comment.
Hardcoded Graph API version
v17.0
Meta regularly deprecates older Graph API versions. Hardcoding v17.0 directly in the URL means this will break when Meta sunsets that version without any code change triggering a visible warning. Consider making the version configurable via a setting (e.g., WHATSAPP_API_VERSION, defaulting to the current stable version) so it can be bumped without a code deploy.
… timeouts, and API versioning
|
Please delete all existing MR's or you will be banned from contributing to care. |
Proposed Changes
WhatsAppProviderclass utilizing the Official WhatsApp Cloud API (Meta).BaseMessagingProviderabstraction for future service modularity.Associated Issue
Architecture changes
care/messaging/providers/whatsapp.py.Merge Checklist
/docsWHATSAPP_ACCESS_TOKEN).Only PR's with test cases included and passing lint and test pipelines will be reviewed
@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins
Summary by CodeRabbit