added typing indicator for whatsapp #2263
Conversation
WalkthroughAdds WhatsApp typing-indicator support end-to-end: new typing_indicator methods at cloud client and bot handler layers, pyscript utility bindings and HTTP helper methods, and accompanying unit and integration tests; integrates typing indicator call into incoming message handling after marking messages as read. Changes
Sequence Diagram(s)sequenceDiagram
participant User as WhatsApp User
participant Handler as WhatsappBot Handler
participant Client as WhatsappCloud Client
participant API as 360dialog API
User->>Handler: inbound message (msg_id)
Handler->>Client: mark_as_read(msg_id)
Client->>API: POST read status
API-->>Client: 200 OK
rect rgb(220, 235, 255)
Note over Handler,Client: New typing-indicator flow
Handler->>Client: typing_indicator(msg_id)
Client->>API: POST typing indicator payload
API-->>Client: 200 OK
end
Handler->>Handler: process message → generate response
Handler->>Client: send_message(response)
Client->>API: POST message
API-->>User: delivered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)kairon/async_callback/utils.py (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
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 |
![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 (6)
kairon/shared/pyscript/shared_pyscript_utils.py (1)
142-152: Shared WhatsApp typing indicator looks correct; consider handling unusedbotand DRYing payloadThe new
typing_indicatorhelper correctly mirrors the 360dialog payload and header structure and aligns with the callback utility version. Thebotparameter is currently unused, though it’s threaded in viapartialfromPyScriptRunner; if it’s only for signature parity, consider either prefixing it (_bot) to silence linters or using it for contextual logging/metrics. You might also reduce duplication by having one of the utilities delegate to the other so the payload stays in a single place if the API changes.kairon/chat/handlers/channels/clients/whatsapp/cloud.py (1)
188-190: Usetimeoutand format the payload dict for readabilityThe new
typing_indicatorhelper is functionally fine, but:
timeoutis unused despite being part of the signature.- The inline dict makes the payload hard to read and maintain.
Consider:
-def typing_indicator(self, msg_id, timeout=None): - payload = { "messaging_product": "whatsapp", "status": "read", "message_id": msg_id, "typing_indicator": { "type": "text" } } - return self.send_action(payload) +def typing_indicator(self, msg_id, timeout=None): + payload = { + "messaging_product": "whatsapp", + "status": "read", + "message_id": msg_id, + "typing_indicator": {"type": "text"}, + } + return self.send_action(payload, timeout=timeout)This resolves the linter warning and keeps the structure maintainable.
kairon/shared/pyscript/callback_pyscript_utils.py (1)
328-348: Exception wrapping and new typing_indicator helper: behavior OK, small polish possible
save_as_pdfcorrectly wraps underlying errors and tests rely on the"encryption failed-{...}"message; if you want better traceability without breaking tests, you could chain the original exception:except Exception as e:raise Exception(f"encryption failed-{str(e)}")
except Exception as e:raise Exception(f"encryption failed-{e!s}") from e
- The new
typing_indicatormirrors the 360dialog payload and header usage used elsewhere, which is good for consistency. Thebotparameter is currently unused; if it’s only for signature parity, consider prefixing it (_bot) or using it in logging/metrics to avoid ARG004 warnings, and possibly delegating to a single shared implementation to keep the payload definition in one place.kairon/chat/handlers/channels/whatsapp.py (1)
185-188: Typing indicator integration is correct; consider isolating failures from message processingHooking
await out_channel.typing_indicator(metadata["id"])and delegating viaWhatsappBot.typing_indicatorcleanly wires the new feature and matches the existingmark_as_readpattern.Two things to consider:
- Both
mark_as_readandtyping_indicatorrun outside the surroundingtry/except, so any HTTP failure there will still prevent the user message from being processed. You may want to either broaden thetryto include these calls or wrap each in its own best-efforttry/exceptthat logs and continues.- These helpers remain synchronous wrappers over
WhatsappCloudrequests, so each incoming message now performs two blocking HTTP calls on the event loop. If this becomes a bottleneck, consider moving them to the async client (send_action_async) or a background actor.Also applies to: 251-257
tests/unit_test/channels/whatsapp_test.py (2)
394-443:test_whatsapp_typing_indicator_end_to_endexercises bot hooks but HTTP assertion is ineffectiveThe test nicely verifies that
_handle_user_messageawaits bothWhatsappBot.mark_as_readandWhatsappBot.typing_indicatorwith the correct message id, which is the key behavioral change.However:
handler.client.postis never called by_handle_user_message, and the final assertionawait_count >= 0is always true, so it doesn’t validate any network behavior.- If you want to ensure no real HTTP is triggered here, assert
await_count == 0; if you instead want to assert that typing_indicator ultimately results in an HTTP call, you’d need to patch the concrete client (WhatsappCloud.typing_indicator/send_action) and assert on that rather than on a raw.post.Also, most of the other tests in this file patch only
WhatsappBot.mark_as_read. Because_handle_user_messagenow also awaitstyping_indicator, consider patching/mockingWhatsappBot.typing_indicatorin those tests as well to avoid accidental real network calls.
670-761: Attachment test refactor looks good; address minor lint/secrets nitsThe refactored
test_valid_attachment_message_requestcorrectly:
- Uses
responsesto stub the Graph URL for the document lookup and media download.- Patches
WhatsappBot.mark_as_readandWhatsapp.process_message.- Asserts the constructed
/k_multimedia_msgtext matches the expected document id.Two small follow‑ups:
args, kwargs = mock_message.call_argsleaveskwargsunused; renaming it to_kwargs(or omitting it) will satisfy RUF059.- Static scanners flag
access_token = "ERTYUIEFDGHGFHJKLFGHJKGHJ"and similar values as potential secrets even though they’re clearly test data. To quiet those tools, consider renaming to something obviously dummy (e.g."dummy_access_token") or annotating with a# nosec/# pragma: allowlist secretstyle comment, depending on your tooling.Also applies to: 828-831
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
kairon/chat/handlers/channels/clients/whatsapp/cloud.py(1 hunks)kairon/chat/handlers/channels/whatsapp.py(2 hunks)kairon/shared/concurrency/actors/pyscript_runner.py(1 hunks)kairon/shared/pyscript/callback_pyscript_utils.py(1 hunks)kairon/shared/pyscript/shared_pyscript_utils.py(1 hunks)tests/unit_test/callback/pyscript_handler_test.py(2 hunks)tests/unit_test/channels/whatsapp_test.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
kairon/chat/handlers/channels/whatsapp.py (3)
kairon/chat/handlers/channels/clients/whatsapp/cloud.py (1)
typing_indicator(188-190)kairon/shared/pyscript/callback_pyscript_utils.py (1)
typing_indicator(341-348)kairon/shared/pyscript/shared_pyscript_utils.py (1)
typing_indicator(145-152)
tests/unit_test/callback/pyscript_handler_test.py (1)
kairon/shared/pyscript/callback_pyscript_utils.py (1)
typing_indicator(341-348)
kairon/shared/pyscript/shared_pyscript_utils.py (3)
kairon/chat/handlers/channels/clients/whatsapp/cloud.py (1)
typing_indicator(188-190)kairon/chat/handlers/channels/whatsapp.py (1)
typing_indicator(251-256)kairon/shared/pyscript/callback_pyscript_utils.py (1)
typing_indicator(341-348)
kairon/chat/handlers/channels/clients/whatsapp/cloud.py (5)
kairon/chat/handlers/channels/whatsapp.py (1)
typing_indicator(251-256)kairon/shared/pyscript/callback_pyscript_utils.py (1)
typing_indicator(341-348)kairon/shared/pyscript/shared_pyscript_utils.py (1)
typing_indicator(145-152)kairon/chat/handlers/channels/clients/whatsapp/dialog360.py (1)
send_action(30-46)kairon/chat/handlers/channels/clients/whatsapp/on_premise.py (1)
send_action(30-46)
kairon/shared/concurrency/actors/pyscript_runner.py (1)
kairon/shared/pyscript/shared_pyscript_utils.py (2)
PyscriptSharedUtility(16-152)typing_indicator(145-152)
tests/unit_test/channels/whatsapp_test.py (1)
kairon/chat/handlers/channels/whatsapp.py (6)
WhatsappBot(210-294)_handle_user_message(181-196)mark_as_read(244-249)typing_indicator(251-256)handle_meta_payload(104-126)
🪛 Gitleaks (8.29.0)
tests/unit_test/channels/whatsapp_test.py
[high] 705-705: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Ruff (0.14.5)
kairon/shared/pyscript/callback_pyscript_utils.py
338-338: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
338-338: Create your own exception
(TRY002)
338-338: Avoid specifying long messages outside the exception class
(TRY003)
338-338: Use explicit conversion flag
Replace with conversion flag
(RUF010)
341-341: Unused static method argument: bot
(ARG004)
tests/unit_test/callback/pyscript_handler_test.py
3909-3909: Redefinition of unused test_typing_indicator_success from line 2050
(F811)
3935-3935: Redefinition of unused test_typing_indicator_http_error_raises_exception from line 2076
(F811)
kairon/shared/pyscript/shared_pyscript_utils.py
145-145: Unused static method argument: bot
(ARG004)
kairon/chat/handlers/channels/clients/whatsapp/cloud.py
188-188: Unused method argument: timeout
(ARG002)
tests/unit_test/channels/whatsapp_test.py
677-677: Possible hardcoded password assigned to: "access_token"
(S105)
756-756: Unpacked variable kwargs is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (python)
- GitHub Check: Python CI
🔇 Additional comments (2)
kairon/shared/concurrency/actors/pyscript_runner.py (1)
48-56: Wiringtyping_indicatorinto PyScript globals is consistent with existing helpersBinding
typing_indicatorviapartial(PyscriptSharedUtility.typing_indicator, bot=bot)matches how other shared utilities are exposed and looks correct for making it available inside restricted scripts.tests/unit_test/callback/pyscript_handler_test.py (1)
2050-2099: typing_indicator tests cover success and error paths wellBoth tests correctly mock
ActionUtility.execute_http_request, validate the exact URL/payload/headers, and assert that exceptions bubble up unchanged. This gives good coverage for the new helper’s behavior.
| def test_typing_indicator_success(): | ||
| from unittest.mock import patch | ||
| message_id = "test_msg_id" | ||
| api_key = "test_api_key" | ||
| bot = "test_bot" | ||
|
|
||
| mock_response = {"status": "ok"} | ||
|
|
||
| with patch.object(ActionUtility, "execute_http_request", return_value=mock_response) as mock_request: | ||
| response = CallbackScriptUtility.typing_indicator(message_id, api_key, bot) | ||
|
|
||
| mock_request.assert_called_once_with( | ||
| "https://waba-v2.360dialog.io/messages", | ||
| "POST", | ||
| { | ||
| "messaging_product": "whatsapp", | ||
| "status": "read", | ||
| "message_id": message_id, | ||
| "typing_indicator": {"type": "text"}, | ||
| }, | ||
| headers={"D360-API-KEY": api_key}, | ||
| ) | ||
|
|
||
| assert response == mock_response | ||
|
|
||
|
|
||
| def test_typing_indicator_http_error_raises_exception(): | ||
| message_id = "test_msg_id" | ||
| api_key = "test_api_key" | ||
| bot = "test_bot" | ||
|
|
||
| inner_err = Exception("HTTP 500") | ||
|
|
||
| with patch.object(ActionUtility, "execute_http_request", side_effect=inner_err) as mock_request: | ||
| with pytest.raises(Exception) as exc: | ||
| CallbackScriptUtility.typing_indicator(message_id, api_key, bot) | ||
|
|
||
| assert str(exc.value) == "HTTP 500" | ||
|
|
||
| mock_request.assert_called_once_with( | ||
| "https://waba-v2.360dialog.io/messages", | ||
| "POST", | ||
| { | ||
| "messaging_product": "whatsapp", | ||
| "status": "read", | ||
| "message_id": message_id, | ||
| "typing_indicator": {"type": "text"}, | ||
| }, | ||
| headers={"D360-API-KEY": api_key}, | ||
| ) |
There was a problem hiding this comment.
Remove duplicate test_typing_indicator_* definitions at end of file
test_typing_indicator_success and test_typing_indicator_http_error_raises_exception are redefined here with identical bodies to the earlier definitions (Lines 2050–2099). This:
- Triggers Ruff F811 (function redefinition).
- Makes it unclear which definition is intended to run.
You can safely drop the second pair:
-def test_typing_indicator_success():
- ...
-
-
-def test_typing_indicator_http_error_raises_exception():
- ...
-
- mock_request.assert_called_once_with(
- "https://waba-v2.360dialog.io/messages",
- "POST",
- {
- "messaging_product": "whatsapp",
- "status": "read",
- "message_id": message_id,
- "typing_indicator": {"type": "text"},
- },
- headers={"D360-API-KEY": api_key},
- )
+# Duplicate typing_indicator tests removed; rely on the earlier definitions above.This keeps the test suite DRY and resolves the linter error.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.5)
3909-3909: Redefinition of unused test_typing_indicator_success from line 2050
(F811)
3935-3935: Redefinition of unused test_typing_indicator_http_error_raises_exception from line 2076
(F811)
🤖 Prompt for AI Agents
In tests/unit_test/callback/pyscript_handler_test.py around lines 3909 to 3958,
there are duplicate definitions of test_typing_indicator_success and
test_typing_indicator_http_error_raises_exception that re-declare earlier
functions (causing Ruff F811); remove the second pair of test function
definitions (or if they were intended to differ, merge the intended differences
into a single canonical definition) so only one definition of each test remains,
run the test suite/linter to confirm the F811 error is resolved.
1 participant
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.