npgvan.People json methods do not work as documented

[Jason94]

The NGPVAN people API (documented here) uses camel case for the search parameters. Keeping with Python style, the Parsons VAN API uses underscore casing (which I learned today is called snake case). The people methods that take parameters, such as find_person, do store these arguments in a camel-cased dictionary at ngpvan.people.py:L262.

However, for the methods that take JSON, such as find_person_json, the dictionary passed in is not transformed in any way. So if you pass a dictionary with keys like firstName you will be fine. However if you pass keys like first_name, it will not work because the API is expecting camel case. This is made worse because our documentation for these methods explicitly state that you must include the snake-cased versions of the parameters.

In the process of investigating this, I also found some irregularities with the email field in particular. The API expects emails to be wrapped in a data structure. Again, if you use the find_person method this works fine. The email argument is wrapped in the necessary structure at ngpvan.people.py:L267. However, no such wrapping occurs if you pass a plain email key in your dictionary for the find_person_json method. If you duplicate that structure in your dictionary, then it does work fine when it gets to the VAN API.

I have attached a script I wrote where you can quickly verify these problems, and the results of one time that I ran it. I expect that the same problems could be found with other multi-word arguments to the API, such as additionalEnvelopeName, although I haven't tested with any others.

The two solutions I can see are:

1. Change the documentation so passing camel-cased fields is called out explicitly. Also document how more complex data like emails must be passed. Change the People._valid_search method so that it fails when a JSON object with snake case keys was passed.
2. Change the People._people_search method so that it transforms snake case dictionaries to camel case dictionaries.

Personally I think 2 would be more consistent and has the benefit of not changing the external API.

import os
import requests
from urllib.error import HTTPError
from parsons import VAN

# Load your VAN API key here
VAN_API_KEY = os.getenv('EA_TRAINING_PASSWORD')
# Set to MyCampaign/EveryAction/etc.
VAN_DB = 'EveryAction'

def main():
    van = VAN(api_key=VAN_API_KEY, db=VAN_DB)

    test_person = {
        'first_name': 'Jason',
        'last_name': 'Walker',
        'email': 't@test.com'
    }

    print("Test Json:")
    try:
        test_json = van.find_person_json(test_person)
    except requests.HTTPError:
        test_json = "Not found."
    print(test_json)

    print("Test args:")
    try:
        test_args = van.find_person(
            first_name=test_person['first_name'],
            last_name=test_person['last_name'],
            email=test_person['email']
        )
    except requests.HTTPError:
        test_args = "Not found."
    print(test_args)

    print("Test camel json:")
    try:
        test_camel_json = van.find_person_json({
            'firstName': test_person['first_name'],
            'lastName': test_person['last_name'],
            'email': test_person['email']
        })
    except requests.HTTPError:
        test_camel_json = "Not found."
    print(test_camel_json)

    print("Test email wrapped, camel json:")
    try:
        fixed_json = van.find_person_json({
            'firstName': test_person['first_name'],
            'lastName': test_person['last_name'],
            'emails': [{'email': test_person['email']}]
        })
    except requests.HTTPError:
        fixed_json = "Not found."
    print(fixed_json)

if __name__ ==  '__main__':
    main()

van_search_email_test.txt

[Jason94]

I also noticed that the documentation for update_person_json is out-of-date. It says:

Update a person record based on a provided ID within the match_json dict.

But the function takes a van ID as an argument, so this documentation is probably wrong.

[shaunagm]

It might be helpful to have a utility that transforms camel case to snake case, which can then be called easily by connector methods (and maybe it can be a method on APIConnector, so any given connector method could call self.camel_to_snake() as needed - maybe as part of #736).

For now, though, fixing the documentation seems pretty high priority.

[rdhyee]

@shaunagm As someone new to the project, I'm looking for high priority issues that I can start with. Do you think a newbie like me can try to fix the documentation as you suggested in https://github.com/move-coop/parsons/issues/621#issuecomment-1438835875 ?

Digging a little further into this issue, and reading the comment above by @Jason94 (https://github.com/move-coop/parsons/issues/621#issuecomment-1035141757) that

the documentation for update_person_json is out-of-date

I think I found the line that needs to be changed:

https://github.com/move-coop/parsons/blob/a6e78a4eeb20d3e68b8ae8ac844fc6686976405a/parsons/ngpvan/people.py#L174

Is that correct?

[shaunagm]

Sorry for the delay responding, @rdhyee! Yes, that looks like the right spot.

[rdhyee]

@shaunagm So, I still need to set up my own work environment for working on Parsons code and documentation. I'll do that, make the changes, ensure that when I rebuild the docs, things look right, and then issue a PR.

[shaunagm]

Sounds great! Let me know if you need help with any of that - happy to pair with you on it.

On Thu, Aug 8, 2024 at 1:01 PM Raymond Yee @.***> wrote:

@shaunagm https://github.com/shaunagm So, I still need to set up my own work environment for working on Parsons code and documentation. I'll do that, make the changes, ensure that when I rebuild the docs, things look right, and then issue a PR.

— Reply to this email directly, view it on GitHub https://github.com/move-coop/parsons/issues/621#issuecomment-2276276122, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAI75YWSAM3BVHMTOXOLK53ZQOP6VAVCNFSM6AAAAABL575VRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZWGI3TMMJSGI . You are receiving this because you were mentioned.Message ID: @.***>