Stripe.Relay.Product + Converter returns incorrect struct

[bobwaycott]

Hey, been working with stripity-stripe for a bit on a new product, and I've encountered unexpected functionality and/or a bug. If I understand the library correctly, it appears a user is expected to use Stripe.Product for subscription products, and Stripe.Relay.Product for ordered products.

Unfortunately, it looks like Converter.convert_stripe_object returns an incorrect struct when working with a Stripe.Relay.Product. Because Converter is reading the object key, it thinks it should return a Stripe.Product, even when the product was not created by the Stripe.Product module.

It seems a Stripe.Product should only be returned when :object is "product" and :type == "service". When the product :type == "good", it should be returning a Stripe.Relay.Product struct (at least, according to matching up Stripe docs to the @specs on the modules).

This can be easily reproduced like so:

# when creating
iex> Stripe.Relay.Product.create(%{name: "My cool product", type: "good"})
%Stripe.Product{...}

# when retrieving
iex> Stripe.Relay.Product.retrieve("prod_randomId")
%Stripe.Product{...}

I would happily submit a pull request, but didn't want to do so without bringing it up, as the type argument does not appear to be available to the Converter or Util modules when determining the proper object to return.

PS: I realize the typespec for both modules is identical. It could be this is expected behavior, but it is at least counterintuitive based on function @specs and the code itself.

[snewcomer]

@bobwaycott Yep that is a good find and we don't have a test for it here

Here is a somewhat similar PR I did for file_upload./, but not exactly. Would ❤️ a PR is you have time!

[bobwaycott]

Hey @snewcomer, happy to submit a PR.

Quick question--I notice the existing Stripe.Relay.ProductTest module specifically is both using and matching on Stripe.Product for all module functions. Should I take that to be intentional, or do you agree that Stripe.Relay.Product functions should always use/return a Stripe.Relay.Product?

Edit: It also appears that all Stripe.Relay.Product tests are passing type: "service", when I believe they should all be type: "good".

I suppose what I'm getting at in seeing that the existing tests are using Stripe.Product specifically for all function calls is what the idea behind the separate Stripe.Relay.Product is exactly. The two modules have different function @specs, but identical typedefs. However, because they're going through the same API endpoint, I don't want to assume the wrong thing here.

[bobwaycott]

Going to provide a little more context/info in a separate comment (instead of continuing to edit the same comment):

Stripe has two notions of a Product -- a good or a service. They both use the same API endpoint. There are a few key differences that apply based on the type of Product:

• Product.type == good is expected to have SKUs attached to provide pricing, and those SKUs are expected to be attached to Orders.
• Product.type == service is expected to have Plans attached to provide pricing, and those Plans are attached to Subscriptions.

There are different expectations for valid params when creating/updating a good vs a service Product. These are mostly reflected in the function @specs of Stripe.Product and Stripe.Relay.Product, but it doesn't look like stripity_stripe is really enforcing Stripe's expectations strongly (as in, enforcing & testing that a Stripe.Product is of type: "service", and a Stripe.Relay.Product is of type: "good".

[github-actions[bot]]

This issue has been automatically marked as "stale:discard". If this issue still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment.

[github-actions[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still relevant, feel free to re-open the issue. Thank you!