Add upsert_person method to ActionNetwork #734
Conversation
…o add_person and update_person
| @@ -200,8 +204,25 @@ def add_person(self, email_address=None, given_name=None, family_name=None, tags | |||
| logger.info(f"Entry {person_id} successfully added to people.") | |||
There was a problem hiding this comment.
Not a huge deal, but should probably change this language to reflect the upsert functionality.
| Updates a person's data in Action Network. WARNING: this endpoint has been deprecated in | ||
| favor of upsert_person and may not work properly. | ||
|
|
There was a problem hiding this comment.
Now that we know that the person-specific endpoint does work with PUT requests, do we still think that we want to deprecate this? As an alternative, we could just remove the tagging functionality. For instance, NGPVAN has both upsert and update methods. On the other hand, maybe keeping things simpler is a better approach?
There was a problem hiding this comment.
Do you think the person-specific endpoint is easier to use? If so, we should probably keep it.
There was a problem hiding this comment.
I have no reason to believe it is, but I'm afraid I also don't have any reason to believe it isn't. The only thing I know for sure is that the method we're changing to upsert (i.e. the one that uses the helper endpoint) is definitely used, and I have no notion of whether update_person() is used at all. Honestly, I think it matters very little, so I was hoping you'd have a structural, conventional, or even esthetic reason to help make the call.
There was a problem hiding this comment.
Hmm, yeah. I think conventionally/aesthetically you minimize duplicate calls, but otoh we don't want to break things for people. Not sure which matters more.
Also adds deprecation warnings to update_person and add_person.
Addresses #706
cc @Maggiedp