Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActionNetwork.update_person() does not update the person #706

Closed
Maggiedp opened this issue Jul 1, 2022 · 7 comments
Closed

ActionNetwork.update_person() does not update the person #706

Maggiedp opened this issue Jul 1, 2022 · 7 comments
Labels
bug Impact - something is currently broken in Parsons and needs to be fixed connector update Work type - additions or changes to the functions of an existing Parsons connector high priority Priority - addressing this is an urgent need for a broad swath of Parsons users

Comments

@Maggiedp
Copy link

Maggiedp commented Jul 1, 2022

When I run update_person(), it returns the person but does not actually update the value for that person.

To Reproduce

from parsons import ActionNetwork
an = ActionNetwork()
person_id='d52040b6-0168-401f-8e7a-b49b2b4e15a1'
person_update=an.update_person(person_id, custom_fields={'2022Letter': 'test'})
person_update=an.update_person(person_id, family_name='test')
person_update['family_name']
person_update['custom_fields']['2022Letter']

person_update['family_name'] returns 'Charles' and person_update['custom_fields']['2022Letter'] returns a keyerror because 2022Letter was not added to the custom fields. It does successfully return the json for the correct person, it just doesn't update the person.

Your Environment

  • Version of Parsons used (if you know): parsons==0.19.0
  • Operating System and version (desktop or mobile): Ubuntu 21.10
  • Is there information about my Action Network group that would be helpful here? I'm just using the key from the API page

Additional Context

I'm using the lower level function api.get_request() with the Action Network connector and it's working fine.

Priority

I think this is a high priority, since it's a bug that makes this function not work but I'm not sure how popular this function is. I'm working on workarounds that don't use parsons for updating people in AN.

@Maggiedp Maggiedp added the bug Impact - something is currently broken in Parsons and needs to be fixed label Jul 1, 2022
@Maggiedp
Copy link
Author

Maggiedp commented Jul 1, 2022

I think the json for PUT needs to go into the data parameter of the request. Right now it's going in the json parameter

person_update = an.api.put_request(url=f"{an.api_url}/people/{person_id}", data=json.dumps(data), success_codes=[204, 201, 200]) seems to work correctly

@SorenSpicknall SorenSpicknall added high priority Priority - addressing this is an urgent need for a broad swath of Parsons users connector update Work type - additions or changes to the functions of an existing Parsons connector labels Jul 8, 2022
@SorenSpicknall
Copy link
Collaborator

Thanks for reporting this, @Maggiedp - we're trying to get some eyes on it within the contributor community. Feel free to put in a PR for a fix yourself, too, if you want!

@ydamit
Copy link
Contributor

ydamit commented Aug 17, 2022

Chiming in to report that I was just told by support that the "right" way to update a record is to use the same "helper" endpoint we're already using in add_person(). This supposedly only works with POST requests, but if you supply an email or mobile number of an existing record, it'll update that record instead of creating a new one.

In my investigation, it did seem that it was possible to update some data with a PUT request to the record-specific endpoint. Specifically, I was able to update the value of a custom field (that already had a value on the record) this way. And, interestingly, I was able to do that without nesting custom_field inside of a person item.

I'm not 100% sure what the best approach for this connector is. Maybe we just need to get rid of update_person() and rename add_person() to something like upsert_person() à la the NGPVAN method. Alternatively, update_person() could just be modified to be a convenience method that does practically the same thing add_person() does.

@ydamit
Copy link
Contributor

ydamit commented Aug 18, 2022

UPDATE: I learned that you can use PUT on the record-specific endpoint to make changes to native and custom fields, but that the POST request to the generic ("helper") endpoint is required for anything to do with tags.

@shaunagm
Copy link
Collaborator

shaunagm commented Aug 25, 2022

Thanks for doing this research, @ydamit! I think adopting upsert_person while leaving add_person and update_person with deprecation warnings for now is the best way forward. I'm going to open up a PR for this, I'll @ you when I do.

@alxmrs
Copy link
Contributor

alxmrs commented Sep 18, 2022

Can we mark this issue as closed?

@shaunagm
Copy link
Collaborator

Yup, good catch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Impact - something is currently broken in Parsons and needs to be fixed connector update Work type - additions or changes to the functions of an existing Parsons connector high priority Priority - addressing this is an urgent need for a broad swath of Parsons users
Projects
None yet
Development

No branches or pull requests

5 participants