customerio / customerio-ruby

A ruby client for the Customer.io event API.
https://customer.io/docs/api/
MIT License
64 stars 75 forks source link

Allow customer_id to be specified in identify URL #111

Closed richdawe-cio closed 7 months ago

richdawe-cio commented 7 months ago

This is based on work by @jrbeck in https://github.com/customerio/customerio-ruby/pull/102 and @trwalzer in https://github.com/customerio/customerio-ruby/pull/88 and updated to work with the main branch. After some discussion within Customer.io Engineering, the functionality into the identify method, with a new customer_id attribute on that. It works in the same way as when it was a separate method.

These changes are necessary to allow identify operations where the person is identified by their email address, not id or cio_id. It updates identify so the identifier can be specified separately from the attributes, using the customer_id attribute. So you can e.g.: select a person based on their email address and set up their id and other attributes. See for example this use-case.

Notes to CIO reviewers:

richdawe-cio commented 7 months ago

@jeremyw @jrbeck, please let me know if you have any feedback on the current version of the PR?

After some feedback within CIO Engineering, I've moved the support for customer_id into the identify method, e.g.: identify(:customer_id => email, :other_attr => other_val). We would prefer not to add an additional method. I believe the current PR supports your use-cases, but it means you can't set the customer_id attribute on a person in CIO.

We also talked internally about making a breaking change in the API, to make the base identify call support the customer id, e.g.: identify(customer_id, attributes). This would be more inline with our other client libraries. But I didn't want to break your integrations without warning.

jeremyw commented 7 months ago

@richdawe-cio This makes sense to me. My initial thought when I saw the bug was to do something similar, but I had considered naming the customer_id argument identifier to match the API documentation. This totally works though, has a smaller impact on clients, and makes more sense if it's consistent with your other client adapters.

Thanks for the work on this! I'm optimistic that we can get this changed rolled out to production early this week if 5.3.0 is published. Already have a branch ready to go based on these changes.

richdawe-cio commented 7 months ago

@jeremyw I've just published 5.3.0 to RubyGems. Thanks for your help and feedback!

jeremyw commented 7 months ago

@richdawe-cio We are live in production and this definitely looks to have resolved the problem! Thank you!

richdawe-cio commented 7 months ago

@jeremyw I'm glad to hear that. Thanks for letting us know.