Add customer management #7
Conversation
Kustomer API lets finding a customer via their Kustomer UUID, email and externalId. This change implements the missing find_customer_by_email. It also changes the behaviour of the find_customer method by making it attempt to find the customer first via their kustomer_id (if present), then the Spree::User email, and eventually via their Spree::User id. The migration is needed to cache the kustomer_id on the user record.
nirebu
force-pushed
the
nirebu/add-customer-management
branch
from
e439081
to
75ab58a
Compare
This change implements methods to create and update customers on Kustomer
nirebu
force-pushed
the
nirebu/add-customer-management
branch
from
75ab58a
to
0182f83
Compare
This change enables automatic creation and update of customers' data on Kustomer via after_commit callbacks on the Spree.user_class.
nirebu
force-pushed
the
nirebu/add-customer-management
branch
from
8936eac
to
5b1fed0
Compare
| # @param user [Spree::User] the customer's UUID we're looking for | ||
| # | ||
| # @return [String] the customer UUID or nil if not present | ||
| def find_customer_id(user) |
There was a problem hiding this comment.
You should rename this method to make it clear that it doesn't just find a customer, it also sets the Kustomer ID on the respective Spree::User record.
There was a problem hiding this comment.
I've gone with a descriptive yet longish name, such as find_customer_id_and_update_user_if_necessary. I don't like it very much but it's explicit.
| it 'retrieves the customer via their UUID' do | ||
| kustomer = described_class.new(api_key: 'my_api_key') | ||
| user = create(:user, kustomer_id: 'the_customer_UUID') | ||
| allow(kustomer).to receive(:find_customer_by_uuid) |
There was a problem hiding this comment.
You never stub the subject under test. The fact that the class relies on find_customer_by_uuid and the other methods is an internal detail. What you should stub here are the external interactions of the method.
The same goes for the other tests.
| def create_customer_now(user) | ||
| kustomer_client.create_customer(user) | ||
| end | ||
|
|
||
| def create_customer_later(user) | ||
| SolidusKustomer::CreateCustomerJob.perform_later(user) | ||
| end | ||
|
|
||
| def update_customer_now(user) | ||
| kustomer_client.update_customer(user) | ||
| end | ||
|
|
||
| def update_customer_later(user) | ||
| SolidusKustomer::UpdateCustomerJob.perform_later(user) | ||
| end |
There was a problem hiding this comment.
I see you're trying to replicate the API of solidus_tracking and solidus_klaviyo here. It may be useful, but if you add more methods to the client, you'd have to keep adding methods here, which is not ideal.
solidus_tracking and solidus_klaviyo have that API because they only expose a single piece of functionality and because those methods are expected to be called from the outside.
In your case, you expose multiple features, but these two methods in particular are already called by your callbacks, so I don't think you need to expose them like this. You can just call the client/background jobs directly where you need to use them.
|
|
||
| def update_customer_on_update | ||
| return unless SolidusKustomer.configuration.update_customer_on_user_update | ||
|
|
There was a problem hiding this comment.
Do you think it also makes sense to check if any relevant attributes have changed? Otherwise, you'll be calling the Kustomer API every time a user is updated, even when nothing is supposed to change in Kustomer.
This PR adds the core functionality to create and update customer objects on Kustomer.
For accuracy and performance, it also adds a
kustomer_idcolumn toSpree::User, which will be used to not reach to Kustomer's API endpoints to retrieve a user's Kustomer ID every time it's needed. Doing so, this closes #3 .