braintree / braintree_node

Braintree Node.js library
https://developer.paypal.com/braintree/docs/start/overview
MIT License
334 stars 104 forks source link

Phone number validation when update customer phone #212

Closed RowanG1Bilue closed 1 year ago

RowanG1Bilue commented 1 year ago

General information

Issue description

I am unit testing the update of the Customer object, for the phone field. When looking at the Braintree spec, the phone should be 10-14 characters long, and no special characters. However, regardless of which string I enter for the phone number, the customer update results in success.

saralvasquez commented 1 year ago

I think we're going to need some additional information. What spec are you looking at specifically?

From what the code and the docs say (https://developer.paypal.com/braintree/docs/reference/request/customer/update#phone), the phone value should be, like you said, 10-14 characters and can contain numbers, dashes, parentheses, and periods. Is this inconsistent with what you've been seeing?

RowanG1Bilue commented 1 year ago

When I pass any value for phone number, even empty string, the update succeeds.

On Tue, 27 Sept 2022, 03:12 Sara Vasquez, @.***> wrote:

I think we're going to need some additional information. What spec are you looking at specifically?

From what the code and the docs say ( https://developer.paypal.com/braintree/docs/reference/request/customer/update#phone), the phone value should be, like you said, 10-14 characters and can contain numbers, dashes, parentheses, and periods. Is this inconsistent with what you've been seeing?

— Reply to this email directly, view it on GitHub https://github.com/braintree/braintree_node/issues/212#issuecomment-1258359965, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQPM5PO56KHL426Y77W4PKLWAHKO5ANCNFSM6AAAAAAQRQXSQE . You are receiving this because you authored the thread.Message ID: @.***>

saralvasquez commented 1 year ago

Okay so you said you're unit testing the Customer object. Are you hand writing unit tests for the customer object in the SDK?

Also are you trying to test the SDK or the API, because the SDK doesn't perform any input validation. The SDK is only responsible for putting together an API request based on your input and getting the API's responses back to you. So, as long as you give the SDK parameter keys that are expected and not deprecated, it will successfully create the request no matter the value.

RowanG1Bilue commented 1 year ago

I was actually doing an integration test, so hitting the actual API.

On Wed, 28 Sept 2022, 07:44 Sara Vasquez, @.***> wrote:

Okay so you said you're unit testing the Customer object. Are you hand writing unit tests for the customer object in the SDK?

Also are you trying to test the SDK or the API, because the SDK doesn't perform any input validation. The SDK is only responsible for putting together an API request based on your input and getting the API's responses back to you. So, as long as you give the SDK parameter keys that are expected and not deprecated, it will successfully create the request no matter the value.

— Reply to this email directly, view it on GitHub https://github.com/braintree/braintree_node/issues/212#issuecomment-1260086631, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQPM5PJLYS5BSC4MUNSMJNDWANTD3ANCNFSM6AAAAAAQRQXSQE . You are receiving this because you authored the thread.Message ID: @.***>

saralvasquez commented 1 year ago

Ah okay. In that case, there isn't anything that we can do since there isn't a direct problem with the SDK. For what it's worth, it seems that the API is functioning as intended according to the documentation, but if you want to talk to someone on the API side I can try and point you in the right direction

saralvasquez commented 1 year ago

Actually change of plans! I verified this behavior and I'll reach out to the appropriate team to get some clarity on this behavior. Thanks for bringing this to our attention!

saralvasquez commented 1 year ago

Update: We did get confirmation that there is only a check for max number of characters for the phone number since it's not a mandatory field. The team that owns this is discussing it further and I'll update this issue when they come to a consensus on what they want to do.

saralvasquez commented 1 year ago

Closing this issues since this involves the API and not the SDK. It is also working as designed