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

added search customer by email api #124

Merged
merged 4 commits into from
Dec 6, 2022
Merged

Conversation

PFadel
Copy link
Contributor

@PFadel PFadel commented Nov 21, 2022

See the example:
getCustomersByEmail.js

Features:

Added get method
Url encode emails

updates from: #89

lib/api.ts Outdated
@@ -51,6 +52,14 @@ export class APIClient {
return this.request.post(`${this.apiRoot}/send/email`, req.message);
}

getCustomersByEmail(email: string) {
if (typeof email !== 'string' || !email) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer the isEmpty util to the ! operator to catch cases where the value is an empty string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

t.truthy((t.context.client.request.get as SinonStub).calledWith(`${RegionUS.apiUrl}/customers?email=${email}`));
})

test('#getCustomersByEmail: should throw error when email is empty', (t) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another test should be added for cases where null/undefined or a non-string object are used for non-typescript clients

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@PFadel PFadel requested a review from mike-engel November 22, 2022 17:13
@PFadel
Copy link
Contributor Author

PFadel commented Nov 28, 2022

@mike-engel could you take another look?

Copy link
Collaborator

@mike-engel mike-engel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @PFadel! One last thing I forgot—can you add documentation to the readme?

@PFadel
Copy link
Contributor Author

PFadel commented Nov 30, 2022

Thanks @PFadel! One last thing I forgot—can you add documentation to the readme?

@mike-engel done!

@PFadel PFadel requested a review from mike-engel November 30, 2022 20:02
@PFadel
Copy link
Contributor Author

PFadel commented Dec 5, 2022

@mike-engel could you take another look? <3

@mike-engel mike-engel merged commit bf0033a into customerio:main Dec 6, 2022
@mike-engel
Copy link
Collaborator

Thanks for your patience @PFadel. This was included with 3.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants