feat(whatsapp): upgrade SDK to 2.0.0, add pytest unit + integration tests#333
feat(whatsapp): upgrade SDK to 2.0.0, add pytest unit + integration tests#333Tram-Nguyen87 wants to merge 5 commits into
Conversation
…ests Converts context.fetch() callers to FetchResponse.data, replaces error ActionResults with ActionError, bumps config.json to 2.0.0, and replaces the legacy asyncio testbed with pytest suites (118 unit tests covering every branch + 8 integration tests with destructive marker). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
🔍 Integration Validation ResultsCommit: Changed directories:
✅ Structure Check output✅ Code Check output✅ Tests Check output✅ README Check output✅ Version Check output |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d5f1ca482
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ves as a package The previous sys.path entry pointed at whatsapp/, which made `import whatsapp` resolve to whatsapp/whatsapp.py (a module), breaking `from whatsapp.whatsapp import ...` in the test suites with "whatsapp is not a package". Pointing at the repo root makes whatsapp/ load as a package via its __init__.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@Tram-Nguyen87 ran the full live e2e tests locally — 9/9 integration tests pass (read-only |
TheRealAgentK
left a comment
There was a problem hiding this comment.
Nice SDK v2 migration overall. I left a few comments on aligning the template language default and trimming docs/catalog wording to the actual action surface.
| @@ -121,67 +105,42 @@ async def execute(self, inputs: Dict[str, Any], context: ExecutionContext): | |||
| language_code = inputs.get("language_code", "en") | |||
There was a problem hiding this comment.
send_template_message defaults language_code to "en", but the live destructive test harness defaults WHATSAPP_TEMPLATE_LANG to "en_US" for the default hello_world template. That means the tested live path and the production omitted-input path can send different payloads. Can we align code/config/README/tests here — either default to en_US everywhere, or make language_code required so callers must provide the approved locale?
| @@ -51,9 +49,7 @@ Configure the integration within Autohive using platform authentication for What | |||
| - `language_code` (optional): Template language code (default: "en") | |||
There was a problem hiding this comment.
Please update this default to match the code/config decision above. Right now the docs say omitted language_code means "en", but the live template test path uses "en_US".
There was a problem hiding this comment.
The README/config descriptions still say the integration can “manage conversations”, but the current action surface only sends messages/templates/media and checks phone-number health. Since this PR is refreshing the public docs, can we trim this wording so the README/catalog matches the actual actions? I’d do the same for the config description.
There was a problem hiding this comment.
Same default mismatch as the action code: config advertises "en" for send_template_message.language_code, while the live test default is "en_US". Please keep the schema default in sync with whichever behavior we want to support.
Make language_code a required input on send_template_message so callers explicitly pass the locale matching their approved template. WhatsApp templates are approved per-locale on Meta (e.g. the stock hello_world template only exists in en_US), so no default is universally correct; the previous "en" default silently mismatched the live test path that uses en_US. - whatsapp.py: read language_code as a required input (no fallback) - config.json: drop default, add language_code to required[], reword description to flag the per-locale approval constraint - README.md: mark language_code (required), update example to en_US - test_whatsapp_unit.py: replace default-is-"en" test with a validation-error test (SDK 2.0 rejects missing required inputs at the validator); pass language_code on all other template tests - test_whatsapp_integration.py: clarify TEMPLATE_LANG env-var doc Also trim README + config descriptions to match the actual action surface (send text/template/media, read phone-number health) instead of the inaccurate "manage conversations" / "contact validation" wording from the previous draft. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Long inline dict literals from adding language_code to template tests exceeded 120 cols; ruff format breaks them across multiple lines. Also wraps the WHATSAPP_TEMPLATE_LANG docstring line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed all four threads and here is a general summary:
|
Summary
autohive-integrations-sdkto~=2.0.0and converts allcontext.fetch()callers inwhatsapp.pyto the newFetchResponseshape (response.data).ActionResult(data={"error": ...})withActionError(message=...)and stripserror/resultproperties fromconfig.jsonoutput schemas.test_whatsapp.py) with a full pytest suite: 118 unit tests (mocked fetch viaFetchResponse) and 9 integration tests (live Graph API viaaiohttp) with adestructivemarker on write tests.README.mdto drop v1-eraresult: bool/error: stroutput rows, replace the legacy test runner with the pytest pattern, and clarify thatphone_number_idis a per-action input (not an auth field).Changes
SDK 2.0.0 upgrade
whatsapp/requirements.txtbumped toautohive-integrations-sdk~=2.0.0whatsapp/config.jsonversion bumped to2.0.0context.fetch()call sites inwhatsapp.pynow useresponse.datafor the bodyActionError(message=...)(validation, API error envelopes, exception handlers, missing-token paths)config.jsontightened:error/resultfields removed; only documented success fields remain (message_id,status,quality_rating)Tests
tests/context.pyand the legacytests/test_whatsapp.pytests/conftest.pyforsys.pathsetuptests/test_whatsapp_unit.py— 118 tests covering:get_whatsapp_creds,_extract_api_error,validate_phone_number,validate_phone_number_id,validate_media_url) with parametrized edge cases200for success,400/404for error envelopes){"auth_type": "PlatformOauth2", "credentials": {...}}tests/test_whatsapp_integration.py— 9 live tests:get_phone_number_health(status + quality rating, invalid ID, nonexistent ID), plus input-validation paths forsend_messageandsend_media_messagethat short-circuit before fetch@pytest.mark.destructive: one test each forsend_message,send_template_message,send_media_message(sends real messages toWHATSAPP_RECIPIENT_PHONE— review before running)real_fetchuses realresp.status+resp.headers, passes throughparams=, and falls back toresp.text()if the body isn't JSONDocs
whatsapp/README.md— added global "Error handling" note explainingActionError; removedsuccess/erroroutput rows from every action; corrected provider name (WhatsApp→WhatsApp Business); rewrote Authentication section to clarifyphone_number_idis a per-action input rather than an auth field; replaced legacypython tests/test_whatsapp.pyinstructions with the pytest unit + integration pattern (safe command first, destructive command second with warning).env.example— addedWHATSAPP_*section with the 6 variables the integration tests read (ACCESS_TOKEN,PHONE_NUMBER_ID,RECIPIENT_PHONE,TEMPLATE_NAME,TEMPLATE_LANG,MEDIA_URL)Validation
Local build pipeline (all green):
python ../autohive-integrations-tooling/scripts/validate_integration.py whatsapp→ 0 errors, 0 warningspython ../autohive-integrations-tooling/scripts/check_code.py whatsapp→ all 9 sub-checks pass (syntax, imports, JSON, ruff lint, ruff format, bandit, pip-audit, config-code sync, fetch patterns)python -m pytest whatsapp/ -v→ 118 passedruff check+ruff format --config ../autohive-integrations-tooling/ruff.toml whatsapp→ cleanTest plan
pytest whatsapp/tests/test_whatsapp_unit.py -vpasses 118/118pytest whatsapp/tests/test_whatsapp_integration.py -m "integration and not destructive"passes with a validWHATSAPP_ACCESS_TOKEN+WHATSAPP_PHONE_NUMBER_IDpytest whatsapp/tests/test_whatsapp_integration.py -m "integration and destructive"passes after settingWHATSAPP_RECIPIENT_PHONE(sends real messages — review first)