braintree / braintree_node

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

Inconsistent response objects #178

Closed fightbulc closed 3 years ago

fightbulc commented 3 years ago

Hey there, I was fetching all addOns objects. The documentation states "Returns a collection of AddOn objects." But then the code reads: gateway.addOn.all().then(result => { const addOns = result.addOns; });. So it does not return a collection of objects. Is that intended?

For instance for fetching all plans you also describe it by saying "Returns a collection of Plan objects.". This time the code aligns with this: const plans = gateway.plan.all().then(result => { });

Would appreciate to see if the description for "addOns" is correct. Same accounts for "disputes".

I am asking because I would like to fix this issue for braintree's @typed definitions.

hollabaq86 commented 3 years ago

👋 @fightbulc Thanks for reaching out. Both of these calls return result objects, where result.addOns and result.plans are collections of addOns and Plans, respectively.

I'll make a note to update our docs so this is more clear.

Which disputes call are you looking at?

hollabaq86 commented 3 years ago

Update: Node is the only SDK that is returning a result object right now, the other server SDKs are all returning collections (which would explain the discrepancy in the docs).

We're going to update the Node SDK so that we're returning a collection of these objects, too. This way all our SDKs are in parity and the docs are correct: collections are returned. We'll also keep the old result object properties to maintain backwards compatibility.

In the next major version, we'll update the SDK so that we return a collection only.

I'll leave this issue open until we release this update- I'm still curious which disputes description you're reviewing in the docs.

fightbulc commented 3 years ago

Hey @hollabaq86 and thanks for the quick reply. Looking forward to see these changes. Sorry regarding "disputes" I ment to write "discounts". It behaves the same as "addOns".

fightbulc commented 3 years ago

@hollabaq86 looks like you deployed a fix. Thanks!

One object response which is still off is the one for fetching all plans:

const plans = await getBraintreeGateway().plan.all(); // { plans: Plan[] }
console.log(plans.plans)

To align with the others it should return directly all plan objects:

const plans = await getBraintreeGateway().plan.all(); // Plan[]
crookedneighbor commented 3 years ago

@fightbulc that's very strange, because we definitely applied the same fix there:

https://github.com/braintree/braintree_node/blob/55365bda1ef8508f27ac89c304a940e6f4510122/lib/braintree/plan_gateway.js#L14-L33

Note that preserve backwards compatibility, we needed to attach a plans attribute to the returned plans array. Are you certain the underlying plans object is not an Array? Are you on the latest version?

fightbulc commented 3 years ago

@crookedneighbor thanks for the quick reply. I was relying on braintree's @types but I assume that you guys didn't update these definitions. Ignoring the types I def can work directly with the response.

crookedneighbor commented 3 years ago

Correct, we don't maintain those type definitions. See https://github.com/braintree/braintree_node/issues/176#issuecomment-709583623