NYL-22: Add Contact metadata and webhook parity - #485
Conversation
AaronDDM
left a comment
There was a problem hiding this comment.
Good morning! Looks good — models, docstrings, changelog and tests all covered, and the enum already had the contact triggers so the docstring-only change there is right.
One suggestion below. Approving.
AaronDDM
left a comment
There was a problem hiding this comment.
Good morning! Re-reviewed after 6406a73.
The _build_query_params test is exactly what was missing — that's the piece that could have silently regressed. UpdateContactRequest is an alias of CreateContactRequest, so metadata covers both paths. CHANGELOG has an Unreleased entry and the docstrings cover the update semantics.
One non-blocking cross-SDK note below. Approving.
|
|
||
| assert ( | ||
| url | ||
| == "https://test.nylas.com/v3/grants/abc-123/contacts?metadata_pair=key1:sync_eligible" |
There was a problem hiding this comment.
Non-blocking, just noting it: Python emits the colon raw (metadata_pair=key1:sync_eligible) while Node and Ruby percent-encode it (key1%3Async_eligible). Both are valid per RFC 3986 and the API decodes either, so nothing to change here — it's pre-existing serializer behavior in each SDK, not something this PR introduced. Flagging it in case it ever comes up in a support ticket.
Summary
metadata_paircontact filteringcontact.updatedandcontact.deletedwire values and guard against unsupportedcontact.createdTest plan
/Users/zhi.q/nylas-python/.venv/bin/python -m pytest(485 passed, 3 deselected)