Conversation
Adds a new messaging provider type for integrating with the Meta Cloud API (WhatsApp Business Platform) directly, as an alternative to Twilio/Turn.io. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a Meta Cloud API provider is selected, the WhatsApp channel form now calls the Phone Number Management API to resolve the user-provided phone number to its phone number ID and stores it in extra_data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…annels Adds a from_identifier property to WhatsappChannel that returns the phone_number_id from extra_data when present (Meta Cloud API), falling back to the phone number for Twilio/Turn.io providers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add MetaCloudAPIMessage dataclass for parsing Meta webhook payloads - Add handle_meta_cloud_api_message Celery task - Implement new_meta_cloud_api_message view with webhook verification and message dispatching by phone_number_id - Add verify_token field to MetaCloudAPIMessagingConfigForm - Set webhook_url for meta_cloud_api channels - Remove stub handle_whatsapp_message task and hardcoded constants - Update Meta API base URL to v25.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add app_secret field to MetaCloudAPIMessagingConfigForm and service - Obfuscate access_token, app_secret, and verify_token in the form - Verify HMAC-SHA256 signature on POST requests before processing - Tries each unique app_secret from configured Meta Cloud API providers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract all message-bearing values upfront, use the first phone_number_id to look up the channel and its app_secret, verify the signature once for the entire payload, then dispatch tasks for each value. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The phone_number_id is already passed separately to the task handler for channel lookup, so it doesn't need to be on the message dataclass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add JSONDecodeError handling for malformed payloads
- Verify X-Hub-Signature-256 before dispatching tasks
- Extract MetaCloudAPIWebhook helper class from view
- Unify MetaCloudAPIMessage as alias for TurnWhatsappMessage
- Add select_related("messaging_provider") to get_experiment_channel
- Add webhook view tests (signature, verification, POST flow)
- Fix send_voice_to_user docstring referencing Twilio
- Add challenge None check in verify_webhook
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… ExperimentChannel Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis pull request introduces comprehensive Meta Cloud API (WhatsApp) support to the system. It adds a new messaging provider type with corresponding form configuration, webhook verification and message handling, an HTTP service for API interactions, async task processing, and refactors the WhatsappChannel to support phone number ID resolution. The implementation includes database migrations, security features for webhook signature validation, and extensive test coverage across multiple layers. Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/channels/tests/test_meta_cloud_api_webhook.py (1)
142-193: Consider adding multi-channel integration test for signature verification routing.While the current tests validate signature verification logic in isolation, there's no integration test confirming that when multiple Meta Cloud API channels exist with different
app_secretvalues, the system correctly routes each webhook to the right channel's secret for verification. The_post()and_meta_webhook_payload()helpers already support this—create a second channel with a different secret and phone_number_id, then verify that a webhook signed for that second channel is accepted and dispatched correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/channels/tests/test_meta_cloud_api_webhook.py` around lines 142 - 193, Add an integration test that creates a second Meta Cloud API channel with a different app_secret and phone_number_id, then post a webhook signed with that second channel's secret and assert routing/verification works: inside TestNewMetaCloudApiMessage (use _post, _make_signature, and _meta_webhook_payload helpers) create the second channel, patch apps.channels.tasks.handle_meta_cloud_api_message.delay, call self._post with the second channel's app_secret, assert response.status_code == 200 and mock_delay.assert_called_once_with(phone_number_id=<second id>, message_data=_meta_webhook_payload()["entry"][0]["changes"][0]["value"]); ensure this verifies the webhook is accepted and dispatched to the correct channel via new_meta_cloud_api_message.apps/channels/tests/test_forms.py (1)
89-90: Movehttpxto module-level imports.Both new tests use local
httpximports without a circular-import/startup justification. Move them to the top-level import section for consistency with project standards.♻️ Proposed cleanup
from unittest.mock import Mock, PropertyMock, patch +import httpx import pytest from django.forms.widgets import HiddenInput, Select @@ def test_whatsapp_form_meta_cloud_api_resolves_phone_number_id(mock_httpx_get, messaging_provider, experiment): """Test that the phone number ID is fetched from Meta API and stored in extra_data""" - import httpx - @@ def test_whatsapp_form_meta_cloud_api_rejects_unknown_number(mock_httpx_get, messaging_provider, experiment): """Test that form validation fails when the phone number is not found in the Meta Business Account""" - import httpx -As per coding guidelines,
**/*.py: Don't use local imports for any reason other than to avoid circular imports or as a means to reduce startup time.Also applies to: 132-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/channels/tests/test_forms.py` around lines 89 - 90, Move the local "httpx" imports to the module-level import block: add "import httpx" with the other top-level imports and remove the two in-function/local imports (the ones currently added near the two tests). Update any tests referencing those local imports to use the module-level name; this keeps imports consistent and avoids unnecessary local imports.apps/channels/views.py (1)
449-456: Consider indexingextra_data->phone_number_idfor webhook lookup throughput.This endpoint does a per-request JSON key lookup on
ExperimentChannel.extra_data. At scale, add a DB index (e.g., functional index on the JSON key) to avoid table scans on inbound webhook traffic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/channels/views.py` around lines 449 - 456, Add a DB index on the JSON key used for webhook lookups to avoid table scans: create a new Django migration that adds an Index on ExperimentChannel.extra_data for the "phone_number_id" key (use KeyTextTransform('phone_number_id', 'extra_data') in the Index expression) and apply it; update the ExperimentChannel model Meta.indexes to include Index(KeyTextTransform('phone_number_id', 'extra_data'), name='experimentchannel_phone_number_id_idx') so the migration can be generated and run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/channels/forms.py`:
- Around line 238-246: When calling service.get_phone_number_id(number) inside
the branch where self.messaging_provider.type ==
MessagingProviderType.meta_cloud_api, wrap the call in a try/except to catch
network/HTTP/timeout errors (e.g., any exception from the service client) and
convert them into a forms.ValidationError with a user-facing message (include
the original error detail for debugging if helpful), otherwise continue to check
the falsy phone_number_id and set self._phone_number_id when present; reference
the get_phone_number_id call and the self._phone_number_id assignment to locate
where to add the try/except and the forms.ValidationError raise.
In `@apps/channels/meta_webhook.py`:
- Around line 17-18: The current conditional in meta_webhook.py appends any
payload with a "messages" key even when it's empty; update the filter used
before values.append(value) so it checks that value.get("messages") is
truthy/non-empty in addition to value.get("metadata", {}).get("phone_number_id")
— e.g., require value.get("messages") and value.get("messages") != [] or
len(value.get("messages", [])) > 0 — so only payloads with actual messages are
enqueued.
In `@apps/channels/views.py`:
- Around line 458-459: The log call exposes a raw provider identifier
(first_phone_number_id); replace it with a non-reversible or partially masked
representation before logging. Locate the log.info call that references
first_phone_number_id in apps/channels/views.py and change it to log a masked
value (e.g., only last 4 chars prefixed with stars) or a hashed representation
(e.g., SHA-256 of first_phone_number_id) so the original id cannot be recovered;
ensure you compute this masked_or_hashed value in the same scope and log that
variable instead of first_phone_number_id.
In `@apps/service_providers/messaging_service.py`:
- Around line 363-364: MetaCloudAPIService makes outbound httpx requests without
timeouts (e.g., the httpx.get call in the method that assigns response =
httpx.get(url, headers=self._headers, params={"fields":
"id,display_phone_number"}) and the other httpx calls in the class), which can
hang workers; add explicit timeouts to these calls by passing a timeout
parameter (or a configurable instance attribute like self._timeout) to
httpx.get/httpx.post so each request includes a sensible timeout (e.g.,
connect/read limits), and update any related tests or callers to use the new
configurable timeout if introduced.
---
Nitpick comments:
In `@apps/channels/tests/test_forms.py`:
- Around line 89-90: Move the local "httpx" imports to the module-level import
block: add "import httpx" with the other top-level imports and remove the two
in-function/local imports (the ones currently added near the two tests). Update
any tests referencing those local imports to use the module-level name; this
keeps imports consistent and avoids unnecessary local imports.
In `@apps/channels/tests/test_meta_cloud_api_webhook.py`:
- Around line 142-193: Add an integration test that creates a second Meta Cloud
API channel with a different app_secret and phone_number_id, then post a webhook
signed with that second channel's secret and assert routing/verification works:
inside TestNewMetaCloudApiMessage (use _post, _make_signature, and
_meta_webhook_payload helpers) create the second channel, patch
apps.channels.tasks.handle_meta_cloud_api_message.delay, call self._post with
the second channel's app_secret, assert response.status_code == 200 and
mock_delay.assert_called_once_with(phone_number_id=<second id>,
message_data=_meta_webhook_payload()["entry"][0]["changes"][0]["value"]); ensure
this verifies the webhook is accepted and dispatched to the correct channel via
new_meta_cloud_api_message.
In `@apps/channels/views.py`:
- Around line 449-456: Add a DB index on the JSON key used for webhook lookups
to avoid table scans: create a new Django migration that adds an Index on
ExperimentChannel.extra_data for the "phone_number_id" key (use
KeyTextTransform('phone_number_id', 'extra_data') in the Index expression) and
apply it; update the ExperimentChannel model Meta.indexes to include
Index(KeyTextTransform('phone_number_id', 'extra_data'),
name='experimentchannel_phone_number_id_idx') so the migration can be generated
and run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c71a8005-8b2b-4beb-b303-20b42081ade9
📒 Files selected for processing (15)
apps/channels/datamodels.pyapps/channels/forms.pyapps/channels/meta_webhook.pyapps/channels/models.pyapps/channels/tasks.pyapps/channels/tests/test_forms.pyapps/channels/tests/test_meta_cloud_api_webhook.pyapps/channels/urls.pyapps/channels/views.pyapps/chat/channels.pyapps/service_providers/forms.pyapps/service_providers/messaging_service.pyapps/service_providers/migrations/0043_alter_messagingprovider_type.pyapps/service_providers/models.pyapps/service_providers/tests/test_messaging_providers.py
… char limit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request introduces Meta Cloud API (WhatsApp) as a new messaging provider. The implementation adds a MetaCloudAPIService class for API communication, a MetaCloudAPIWebhook handler for webhook verification and message extraction, and integrates these components through existing channel forms, models, views, and task handlers. A new WhatsappChannelForm.post_save hook persists phone_number_id metadata, while a new view endpoint accepts incoming messages via POST. The changes span models, services, forms, views, tasks, and corresponding test coverage without modifying existing provider functionality. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
apps/channels/forms.py (1)
238-246:⚠️ Potential issue | 🟠 MajorConvert Meta lookup failures into
ValidationError.If
get_phone_number_idraises (HTTP/network/timeout), this path returns a server error instead of a form error.🛠️ Proposed fix
if self.messaging_provider.type == MessagingProviderType.meta_cloud_api: - phone_number_id = service.get_phone_number_id(number) + try: + phone_number_id = service.get_phone_number_id(number) + except Exception: + logger.exception("Failed to resolve Meta Cloud API phone_number_id") + raise forms.ValidationError( + "Could not validate this number with Meta right now. Please try again." + ) from None if not phone_number_id: raise forms.ValidationError( f"{number} was not found in the WhatsApp Business Account. " "Please verify the number is registered with your business." )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/channels/forms.py` around lines 238 - 246, Wrap the call to service.get_phone_number_id inside the Meta branch (where self.messaging_provider.type == MessagingProviderType.meta_cloud_api) in a try/except that catches network/HTTP/timeout errors and converts them into a forms.ValidationError (instead of letting them bubble up); only assign self._phone_number_id if the call succeeds, and include a clear validation message (e.g., number not found or lookup failed) when raising the ValidationError.apps/service_providers/messaging_service.py (1)
363-364:⚠️ Potential issue | 🟠 MajorAdd explicit timeouts to Meta API HTTP calls.
Both Meta outbound requests still omit
timeout, so transient network stalls can hang workers.🛠️ Proposed fix
class MetaCloudAPIService(MessagingService): @@ META_API_BASE_URL: ClassVar[str] = "https://graph.facebook.com/v25.0" + REQUEST_TIMEOUT: ClassVar[float] = 10.0 @@ - response = httpx.get(url, headers=self._headers, params={"fields": "id,display_phone_number"}) + response = httpx.get( + url, + headers=self._headers, + params={"fields": "id,display_phone_number"}, + timeout=self.REQUEST_TIMEOUT, + ) @@ - response = httpx.post(url, headers=self._headers, json=data) + response = httpx.post(url, headers=self._headers, json=data, timeout=self.REQUEST_TIMEOUT)#!/bin/bash # Verify MetaCloudAPIService HTTP calls and whether timeout is set. rg -nP 'class MetaCloudAPIService|httpx\.(get|post)\(' apps/service_providers/messaging_service.py -C3Also applies to: 384-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/service_providers/messaging_service.py` around lines 363 - 364, The MetaCloudAPIService HTTP calls (the httpx.get and httpx.post usages in the MetaCloudAPIService class) lack explicit timeouts and can hang; update both calls (the GET that sets params {"fields": "id,display_phone_number"} and the other POST/GET around the 384-385 area) to include a sensible timeout parameter (e.g., timeout=self._timeout or timeout=10) so requests never block indefinitely, and keep response.raise_for_status() as-is; if the class has no _timeout attribute, add one (or use a module-level constant) and use it for both httpx calls for consistency.apps/channels/meta_webhook.py (1)
17-18:⚠️ Potential issue | 🟡 MinorFilter out empty
messagespayloads before enqueue path.Line 17 currently accepts any payload that merely contains a
messageskey, including empty arrays.✅ Proposed fix
- if "messages" in value and value.get("metadata", {}).get("phone_number_id"): + messages = value.get("messages") or [] + if messages and value.get("metadata", {}).get("phone_number_id"): values.append(value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/channels/meta_webhook.py` around lines 17 - 18, The current condition around values.append(value) accepts payloads with a "messages" key even when it's an empty list; update the conditional that includes values.append(value) (the if using "messages" in value and value.get("metadata", {}).get("phone_number_id")) to explicitly check that value.get("messages") is a non-empty list (e.g., truthy and isinstance(list) and len(...) > 0) or otherwise contains at least one non-empty message before appending, so empty messages payloads are filtered out prior to enqueue.apps/channels/views.py (1)
458-458:⚠️ Potential issue | 🟠 MajorRemove clear-text logging of
phone_number_id.Line 458 logs a sensitive identifier directly.
🔒 Proposed fix
- log.info("No channel found for phone_number_id: %s", first_phone_number_id) + log.info("No channel found for incoming Meta Cloud API webhook payload")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/channels/views.py` at line 458, The log at the log.info call that prints first_phone_number_id in apps/channels/views.py should not emit the raw identifier; instead redact or pseudonymize it before logging (e.g., replace with a fixed placeholder, masked string, or a short hash/fingerprint) so the log only indicates "no channel found" plus a non-reversible token; update the log.info invocation that references first_phone_number_id to use the redacted/hashed value (or omit the ID entirely) and, if you add a helper, implement it near the view (e.g., a redact_or_hash utility) and use that helper when logging.
🧹 Nitpick comments (2)
apps/channels/tests/test_forms.py (1)
89-89: Movehttpximport to module scope.Line 89 and Line 132 use local imports without a circular-import/startup-time reason.
As per coding guidelines `**/*.py`: Don't use local imports for any reason other than to avoid circular imports or as a means to reduce startup time.♻️ Proposed fix
from unittest.mock import Mock, PropertyMock, patch +import httpx import pytest from django.forms.widgets import HiddenInput, Select @@ def test_whatsapp_form_meta_cloud_api_resolves_phone_number_id(mock_httpx_get, messaging_provider, experiment): """Test that the phone number ID is fetched from Meta API and stored in extra_data""" - import httpx - @@ def test_whatsapp_form_meta_cloud_api_rejects_unknown_number(mock_httpx_get, messaging_provider, experiment): """Test that form validation fails when the phone number is not found in the Meta Business Account""" - import httpx -Also applies to: 132-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/channels/tests/test_forms.py` at line 89, Two local imports of the httpx package were added inside test functions (at the locations shown) even though there is no circular-import or startup-time reason; move the httpx import to module scope at the top of the file and remove the local imports so tests reference the module-level httpx import (update the test functions that currently import httpx to use the top-level import).apps/channels/tests/test_meta_cloud_api_webhook.py (1)
157-168: Add a regression test for mixedphone_number_idbatches.Current dispatch coverage is single-value only. Please add a case where the first value is unknown and a later value is known, so ordering bugs don’t silently drop valid messages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/channels/tests/test_meta_cloud_api_webhook.py` around lines 157 - 168, Add a new test that posts a payload containing a batch with mixed phone_number_id values (first an unknown id, then a known id "12345") to exercise ordering; use the existing helper _meta_webhook_payload() or construct a payload with two entries/changes where entry[0].changes[0].value.phone_number_id is unknown and entry[1].changes[0].value.phone_number_id == "12345", call self._post(...) in the test (decorated with `@patch`("apps.channels.tasks.handle_meta_cloud_api_message.delay")), then assert that handle_meta_cloud_api_message.delay was called exactly once with phone_number_id="12345" and message_data equal to the corresponding value from the payload, and do not expect a call for the unknown id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/channels/tasks.py`:
- Line 203: The log statement using log.info that prints the raw phone_number_id
should be changed to avoid emitting sensitive identifiers; update the log call
in the tasks module (the log.info referencing phone_number_id) to either remove
the identifier entirely or log a redacted/masked form (e.g., hash or show only
last 4 characters) or replace with a fixed placeholder like
"<redacted_phone_number_id>" so no raw provider identifier is written to logs.
In `@apps/channels/views.py`:
- Around line 447-456: The code currently uses first_phone_number_id from
message_values[0] to find an ExperimentChannel, which fails for batched payloads
where other messages may have different phone_number_ids; instead, collect all
phone_number_ids from message_values, query ExperimentChannel with
extra_data__phone_number_id__in=<collected_ids> and
messaging_provider__type=MessagingProviderType.meta_cloud_api, then map returned
channels by their extra_data phone_number_id so each incoming message can be
matched to its correct channel (use the message_values list and the channel map
rather than the single variable first_phone_number_id); update the logic around
ExperimentChannel.objects.filter(...).first() to use the set-based query and
per-message lookup.
In `@apps/chat/channels.py`:
- Around line 1157-1162: The from_identifier property currently falls back to
extra_data["number"] when "phone_number_id" is missing, which causes invalid
Meta Graph API paths; update the from_identifier property (on the class that
accesses experiment_channel.extra_data) to check the channel/provider type
(e.g., Meta) and if the provider is Meta, require "phone_number_id" to exist —
raise a clear exception when it's missing instead of returning the plain
"number"; for non-Meta providers continue to return extra_data["number"] as
before.
---
Duplicate comments:
In `@apps/channels/forms.py`:
- Around line 238-246: Wrap the call to service.get_phone_number_id inside the
Meta branch (where self.messaging_provider.type ==
MessagingProviderType.meta_cloud_api) in a try/except that catches
network/HTTP/timeout errors and converts them into a forms.ValidationError
(instead of letting them bubble up); only assign self._phone_number_id if the
call succeeds, and include a clear validation message (e.g., number not found or
lookup failed) when raising the ValidationError.
In `@apps/channels/meta_webhook.py`:
- Around line 17-18: The current condition around values.append(value) accepts
payloads with a "messages" key even when it's an empty list; update the
conditional that includes values.append(value) (the if using "messages" in value
and value.get("metadata", {}).get("phone_number_id")) to explicitly check that
value.get("messages") is a non-empty list (e.g., truthy and isinstance(list) and
len(...) > 0) or otherwise contains at least one non-empty message before
appending, so empty messages payloads are filtered out prior to enqueue.
In `@apps/channels/views.py`:
- Line 458: The log at the log.info call that prints first_phone_number_id in
apps/channels/views.py should not emit the raw identifier; instead redact or
pseudonymize it before logging (e.g., replace with a fixed placeholder, masked
string, or a short hash/fingerprint) so the log only indicates "no channel
found" plus a non-reversible token; update the log.info invocation that
references first_phone_number_id to use the redacted/hashed value (or omit the
ID entirely) and, if you add a helper, implement it near the view (e.g., a
redact_or_hash utility) and use that helper when logging.
In `@apps/service_providers/messaging_service.py`:
- Around line 363-364: The MetaCloudAPIService HTTP calls (the httpx.get and
httpx.post usages in the MetaCloudAPIService class) lack explicit timeouts and
can hang; update both calls (the GET that sets params {"fields":
"id,display_phone_number"} and the other POST/GET around the 384-385 area) to
include a sensible timeout parameter (e.g., timeout=self._timeout or timeout=10)
so requests never block indefinitely, and keep response.raise_for_status()
as-is; if the class has no _timeout attribute, add one (or use a module-level
constant) and use it for both httpx calls for consistency.
---
Nitpick comments:
In `@apps/channels/tests/test_forms.py`:
- Line 89: Two local imports of the httpx package were added inside test
functions (at the locations shown) even though there is no circular-import or
startup-time reason; move the httpx import to module scope at the top of the
file and remove the local imports so tests reference the module-level httpx
import (update the test functions that currently import httpx to use the
top-level import).
In `@apps/channels/tests/test_meta_cloud_api_webhook.py`:
- Around line 157-168: Add a new test that posts a payload containing a batch
with mixed phone_number_id values (first an unknown id, then a known id "12345")
to exercise ordering; use the existing helper _meta_webhook_payload() or
construct a payload with two entries/changes where
entry[0].changes[0].value.phone_number_id is unknown and
entry[1].changes[0].value.phone_number_id == "12345", call self._post(...) in
the test (decorated with
`@patch`("apps.channels.tasks.handle_meta_cloud_api_message.delay")), then assert
that handle_meta_cloud_api_message.delay was called exactly once with
phone_number_id="12345" and message_data equal to the corresponding value from
the payload, and do not expect a call for the unknown id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f3640f13-cee7-4cd4-ac2b-f1d5ba65a5a5
📒 Files selected for processing (15)
apps/channels/datamodels.pyapps/channels/forms.pyapps/channels/meta_webhook.pyapps/channels/models.pyapps/channels/tasks.pyapps/channels/tests/test_forms.pyapps/channels/tests/test_meta_cloud_api_webhook.pyapps/channels/urls.pyapps/channels/views.pyapps/chat/channels.pyapps/service_providers/forms.pyapps/service_providers/messaging_service.pyapps/service_providers/migrations/0043_alter_messagingprovider_type.pyapps/service_providers/models.pyapps/service_providers/tests/test_messaging_providers.py
…ation Add extra_data JSONField to MessagingProvider to store non-encrypted queryable data. For Meta Cloud API providers, a SHA-256 hash of the verify_token is stored on save, allowing verify_webhook to filter directly in the DB instead of iterating and decrypting all providers. Also fixes test expecting 400 for invalid signature when the view intentionally returns 200 to prevent Meta retries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Convert MetaCloudAPIWebhook static methods to module-level functions - Add view-level GET verification tests - Add test for missing X-Hub-Signature-256 header - Use Django's constant_time_compare instead of hmac.compare_digest - Add comments explaining signature verification placement and multi-phone-number payload safety Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Filter out empty messages payloads in extract_message_values - Handle Meta API lookup failures with user-friendly validation error - Add explicit 30s timeouts for outbound Meta API httpx calls - Redact phone_number_id from log messages - Fail fast in WhatsappChannel.from_identifier when phone_number_id is missing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
apps/channels/urls.py
Outdated
| name="new_sureadhere_message", | ||
| ), | ||
| path("whatsapp/turn/<uuid:experiment_id>/incoming_message", views.new_turn_message, name="new_turn_message"), | ||
| path("whatsapp/meta/incoming_message", views.new_meta_cloud_api_message, name="new_meta_cloud_api_message"), |
There was a problem hiding this comment.
We could have done whatsapp/meta/incoming_message/<uiud:channel_external_id> which will help us find the channel immediately, but since multiple numbers can be linked to a business account, we probably want to keep support for multiple numbers
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
Documents each MetaCloudAPIMessagingConfigForm parameter (verify_token, app_secret, business_id, access_token), explaining when and how each is used in the webhook verification, signature checking, phone number resolution, and message sending flows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convert new_meta_cloud_api_message function into MetaCloudAPIWebhookView CBV, splitting GET (webhook verification) and POST (message handling) into separate methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| type = models.CharField(max_length=255, choices=MessagingProviderType.choices) | ||
| name = models.CharField(max_length=255) | ||
| config = encrypt(models.JSONField(default=dict)) | ||
| extra_data = models.JSONField(default=dict, blank=True) |
There was a problem hiding this comment.
See https://github.com/dimagi/open-chat-studio/blob/e09de8e6ae433ccc03cc4da9b4d4411922c01808/docs/developer_guides/meta_cloud_api_integration.md#verify_token--webhook-verification on how this field is used.
TLDR:
The verify token is stored in an encrypted column, but since we cannot filter encrypted columns, we take the hash of the secret and add it in this column instead, making it filterable.
This was done as an optimization step where we have to find the messaging provider based on the verify_token (sent in a header). Instead of iterating over all messaging providers to check if there is one with this token, we now filter using the hash value.
apps/channels/forms.py
Outdated
| number = phonenumbers.format_number(number_obj, phonenumbers.PhoneNumberFormat.E164) | ||
| service = self.messaging_provider.get_messaging_service() | ||
| if not service.is_valid_number(number): | ||
| if self.messaging_provider.type == MessagingProviderType.meta_cloud_api: |
There was a problem hiding this comment.
can't this be moved into MessagingService.is_valid_number
There was a problem hiding this comment.
Fixed in 4a5ca39. Added is_valid_number to MetaCloudAPIService which wraps the phone number ID lookup. The form's clean method now calls service.is_valid_number(number) uniformly for all providers — no provider-type check needed for validation.
apps/channels/forms.py
Outdated
| f"{number} was not found in the WhatsApp Business Account. " | ||
| "Please verify the number is registered with your business." | ||
| ) | ||
| self._phone_number_id = phone_number_id |
There was a problem hiding this comment.
This feels hacky - what about doing basic phone number validation here (make sure it's a correctly formatted phone number) and then moving the service level check to the clean method and putting the phone_number_id into form.cleaned_data.
I think that would follow normal Django patterns better and you won't need to rely on a temporary field on the form.
There was a problem hiding this comment.
Fixed in 4a5ca39. clean_number now only does phone number format validation. The service-level check (is_valid_number) moved to clean, and phone_number_id goes into cleaned_data directly — no more temp attribute or post_save override.
apps/channels/tasks.py
Outdated
| experiment_channel = get_experiment_channel( | ||
| ChannelPlatform.WHATSAPP, | ||
| extra_data__phone_number_id=phone_number_id, | ||
| messaging_provider__type=MessagingProviderType.meta_cloud_api, | ||
| ) | ||
| if not experiment_channel: | ||
| log.info("No experiment channel found for incoming Meta Cloud API message") | ||
| return |
There was a problem hiding this comment.
I think it's better to handle this in the view and pass through the channel ID in the task.
This also allows passing through the team slug in the task which is useful for debugging.
It also means we don't ever have to fire off a task if there is no channel.
There was a problem hiding this comment.
Fixed in 4a5ca39. The view now passes channel_id and team_slug to the task instead of phone_number_id. The task looks up by ID directly — no redundant filter query, and tasks are never fired for missing channels.
Co-authored-by: Simon Kelly <skelly@dimagi.com>
- Refactor WhatsappChannelForm: clean_number does format validation only, clean handles service-level validation via is_valid_number and stores phone_number_id in cleaned_data for Meta channels - Add is_valid_number and get_phone_number_id to MetaCloudAPIService so the form delegates validation to the service layer - Pass channel_id and team_slug from view to task instead of phone_number_id, avoiding redundant DB lookup in the task - Add comment explaining the whatsapp_business_account object check with link to Meta docs - Add debug logging to webhook GET verification handler Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Debug-level logs for early exits (invalid JSON, non-whatsapp object, no messages, missing fields), info for missing channel, warning for signature verification failure, and debug for successful dispatch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test failed intermittently because get_slug_for_team() caches slugs in Django's cache, and when test transactions roll back, DB IDs can be reused with different slugs while the cache retains stale values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
A single Meta webhook payload can contain messages for multiple phone numbers (e.g. a WhatsApp Business Account with several registered numbers). Previously all messages were dispatched to the channel of the first phone number, silently misrouting any subsequent messages. Now all unique phone_number_ids in the payload are fetched in a single bulk query and each message is routed to its correct channel. Messages with no matching channel are logged and skipped. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
apps/channels/views.py
Outdated
| .select_related("experiment", "team", "messaging_provider") | ||
| .first() | ||
| ) | ||
| if not channel: |
There was a problem hiding this comment.
this validation logic doesn't match with logic later that supports multiple phone numbers
An attacker with a legitimate channel could forge messages for a victim's phone number by including it in a payload signed with their own app_secret. Fix _payload_has_valid_signature to reject any payload whose channels span more than one messaging provider (distinct app_secret). Also adds a test covering the cross-provider forgery scenario. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Technical Description
Adds support for Meta Cloud API as a new WhatsApp messaging provider, enabling direct integration with the WhatsApp Business Platform without requiring a third-party intermediary like Turn.io.
This is phase 1 of the integration where only text messages are supported. Audio messages are coming in a followup PR.
Key changes:
meta_cloud_api): Registered as aMessagingProviderTypewith its own config form (MetaCloudAPIMessagingConfigForm) supportingbusiness_id,access_token,app_secret, andverify_tokenfieldsMetaCloudAPIService: NewMessagingServiceimplementation that sends text messages via the Meta Graph API (v25.0) and resolves phone number IDs from E.164 numbers using the WhatsApp Business Account Phone Number Management API/channels/whatsapp/meta/incoming_message): Handles both the Meta GET verification handshake and incoming POST message payloads. VerifiesX-Hub-Signature-256HMAC signatures using the configuredapp_secretphone_number_idinextra_data. This ID is used for routing incoming webhooks and as thefromidentifier when sending messagesWhatsappChannelrefactor: Extracted afrom_identifierproperty that returns eitherphone_number_id(Meta Cloud API) ornumber(other providers), removing duplicatedfrom_number/to_numberlogic acrosssend_text_to_user,send_voice_to_user, andsend_file_to_userMetaCloudAPIMessage = TurnWhatsappMessage) since both use the same WhatsApp Business API payload structureI'll write up some user docs for this as well
Migrations
Migration
0043_alter_messagingprovider_typeaddsmeta_cloud_apito theMessagingProviderTypechoices. This is backwards compatible — existing code will simply not encounter the new type.Demo
To set up:
/channels/whatsapp/meta/incoming_messageDocs and Changelog
Walkthrough guide (AI guide)
## OverviewThis PR adds Meta Cloud API as a new WhatsApp messaging provider, enabling direct integration with the WhatsApp Business Platform without a third-party intermediary like Turn.io. Phase 1 supports text messages only (audio coming in a follow-up PR).
Size: Large (781 additions, 27 deletions, 18 files changed)
Key areas:
apps/service_providers/(provider type, config, service),apps/channels/(webhook, views, forms, tasks),apps/chat/channels.py(WhatsappChannel refactor)Architecture Impact
MessagingProviderType→form_cls→get_messaging_servicedispatch. Clean extension, no existing patterns broken.phone_number_id(unlike Turn.io's per-experiment URLs). New routing pattern in the codebase.app_secretlives in encrypted provider config.from_identifierproperty on WhatsappChannel abstracts Meta vs. others, consolidating duplicatedfrom_numberlogic.Review Comments Summary
CodeRabbit flagged docstring coverage below 80% threshold. Author addressed review comments across multiple commits. Key iterations: webhook signature verification, verify_token hashing, class-based view refactor, module-level function extraction.
Recommended Reading Order
Step 1 of 8: Provider Type & Migration
Files:
apps/service_providers/models.py,apps/service_providers/migrations/0044_alter_messagingprovider_type.pyWhy first: Everything else depends on the new enum value and the
extra_datafield onMessagingProvider.New enum value:
Dispatch wiring:
New field on
MessagingProvider:Migration adds the enum choice and the
extra_dataJSONField. Backwards compatible.What to notice:
extra_datais a plain JSONField (not encrypted) — storesverify_token_hashfor efficient DB lookups. Sensitive values stay in encryptedconfig.Architecture: Follows the established provider extension pattern: add enum → wire
form_cls→ wireget_messaging_service. No new patterns at this layer.Step 2 of 8: Provider Config Form
File:
apps/service_providers/forms.pyWhy here: Defines what fields are collected when creating a Meta Cloud API provider, and how the verify token hash gets persisted.
What to notice:
access_token,app_secret,verify_token), butbusiness_idis not — just an account identifier.save()hashes verify_token with SHA-256, stores inextra_data. Enables webhook verification endpoint to do a DB lookup by hash.save()overwritesextra_dataentirely (not merging). Fine since verify_token_hash is the only extra_data for this provider type.Step 3 of 8: MetaCloudAPIService
File:
apps/service_providers/messaging_service.pyWhy here: Core domain logic — how we talk to Meta's Graph API. Phone number resolution and message sending live here.
Class definition:
Phone number resolution:
Sending messages:
What to notice:
is_valid_numberhas a side effect: caches resolvedphone_number_idin_phone_number_id. The form later callsget_phone_number_id()to retrieve it. Avoids a second API call but couples the two methods.from_insend_text_messageis thephone_number_id, not a phone number. URL is/{phone_number_id}/messages.smart_splitat 4096 chars — WhatsApp's per-message limit.TEXTinsupported_message_types— voice/file support coming in phase 2._fetch_phone_number_idfetches ALL phone numbers and iterates. Pagination may be needed for accounts with many numbers.Step 4 of 8: Channel Form & Data Model
Files:
apps/channels/forms.py,apps/channels/datamodels.pyWhy here: The form is where
phone_number_idresolution happens at channel creation. The data model shows message format reuse.Data model — simple type alias:
Form refactor —
clean_numbersplit intoclean_number(parse) +clean(validate):class WhatsappChannelForm(WebhookUrlFormBase): def clean_number(self): try: number_obj = phonenumbers.parse(self.cleaned_data["number"]) - number = phonenumbers.format_number(number_obj, phonenumbers.PhoneNumberFormat.E164) - service = self.messaging_provider.get_messaging_service() + return phonenumbers.format_number(number_obj, phonenumbers.PhoneNumberFormat.E164) + except phonenumbers.NumberParseException: + raise forms.ValidationError("Enter a valid phone number (e.g. +12125552368).") from None + + def clean(self): + cleaned_data = super().clean() + number = cleaned_data.get("number") + if not number or not self.messaging_provider: + return cleaned_data + + service = self.messaging_provider.get_messaging_service() + try: if not service.is_valid_number(number): self.warning_message = ( f"{number} was not found at the provider. Please make sure it is there before proceeding" ) - return number - except phonenumbers.NumberParseException: - raise forms.ValidationError("Enter a valid phone number (e.g. +12125552368).") from None + except ValueError as e: + self.add_error("number", str(e)) + return cleaned_data + except Exception: + self.add_error("number", "Could not validate this number right now. Please try again.") + return cleaned_data + + if self.messaging_provider.type == MessagingProviderType.meta_cloud_api: + cleaned_data["phone_number_id"] = service.get_phone_number_id() + return cleaned_dataWhat to notice:
clean_numbertocleanis better Django practice — cross-field validation belongs inclean().is_valid_numberraisesValueError(hard error, no phone_number_id = can't function). Turn.io/Twilio returnFalse(soft warning).phone_number_idflows intocleaned_data→ChannelFormWrapper.save()→channel.extra_dataautomatically.Exceptioncatch is a resilience measure — if the Meta API is down, the form gracefully errors.Step 5 of 8: Webhook Helpers
File:
apps/channels/meta_webhook.py(NEW FILE)Why here: Security-critical building blocks used by the view. Three pure module-level functions.
What to notice:
constant_time_compareprevents timing attacks on HMAC. Good security practice.extract_message_valuesfilters for entries with bothmessagesANDphone_number_id— skips delivery receipts (statuses) that come through the same webhook.Step 6 of 8: Webhook View & URL Routing
Files:
apps/channels/views.py,apps/channels/urls.py,apps/channels/models.pyWhy here: HTTP integration layer that ties everything together.
URL registration:
Webhook URL property on
ExperimentChannel:The view:
What to notice:
phone_number_idin payload.app_secretis in encrypted provider config so we need the channel first.csrf_exempt+waf_allow(NoUserAgent_HEADER)— Meta webhooks have no CSRF tokens or standard user-agent headers.select_related.Architecture: New routing pattern. Turn.io uses per-experiment URLs (
/turn/<experiment_id>/). Meta uses a single endpoint with payload-based routing. Both coexist cleanly.Step 7 of 8: WhatsappChannel Refactor & Task
Files:
apps/chat/channels.py,apps/channels/tasks.pyWhy here: Where message processing happens after webhook dispatches. The
from_identifierrefactor is the key DRY improvement.New
from_identifierproperty:Simplified send methods (same pattern for all three):
New Celery task:
Also added
messaging_providertoselect_relatedin sharedget_experiment_channel:What to notice:
from_identifiereliminates duplication across 3 send methods. Future providers benefit from this abstraction too.channel_iddirectly — view already resolved the channel, avoiding redundant queries.messaging_providertoselect_relatedinget_experiment_channelbenefits all providers.Step 8 of 8: Tests
Files:
apps/channels/tests/test_meta_cloud_api_webhook.py(new, 242 lines),apps/channels/tests/test_forms.py,apps/service_providers/tests/test_messaging_providers.pyWebhook test fixtures:
Signature verification tests:
POST flow tests:
Phone number ID resolution tests (5 parametrized cases):
Form tests:
What to notice:
test_chatbot_tables.py(clearing cached team slugs).Potential Concerns
is_valid_numberside-effect coupling — The validation method cachesphone_number_idas a side effect. Could be clearer as a method that returns the ID directly._fetch_phone_number_idfetches all numbers without handling pagination. May fail for large business accounts.send_voice_messageandsend_file_to_usernot implemented. Calling them on a Meta channel would hit the base class..first()when looking up channels. If multiple channels share the samephone_number_id, only the first will receive messages.