Description is available for goods and services

[jamesprior]

Digging into the Stripe Product API I didn't see where description was listed as only for goods - so I moved it out of the validations for goods only.

The tests are unchanged. They were not validating description that I saw and given the focused nature of them I wasn't sure that you wanted to carry a spec to ensure Products with descriptions were still valid.

[tansengming]

Thanks for the head's up. It looks like bigger changes have happened.

I have not used Stripe in a while and it looks like there's been a change from setting up subscriptions with "Products + Plans" to the new way with "Products + Prices". Because of that it looks like stripe has removed the "type" attribute from products too which makes the goods vs services distinction a little outdated.

I can't find the old docs anymore but I imagine that the attributes to be validated here, including "description" where still valid when products were still split between goods and services.

I think I want to keep "description" as it is so that the gem still works with the old way.

Thinking out loud here: I think its a good idea to start a new PR for the new way, which means, maybe getting rid of the type attribute, adding the prices object (thanks!), adding a migration path for people on the old way etc.

You must have been pretty confused looking at how things were set up here!

[jamesprior]

What do you think about switching the validation according to the Stripe API Version? The issue I'm running into is that I would like to set a description on a product in the current API. Since the validation is checking for the absence of description, it looks like the only way I can do that is to set the product type to 'good'. Since they took out product type I suppose I could mark it as a good, set a description, and let it be ignored but it feels clearer to leave off the product type or let it be 'service' while setting description.

[tansengming]

I just realized that the problem with the way validations is written right now is that it presumes that type is either good? or service?. How about re-writing this a little bit more so that it supports another type, no_type? (this is not a good name lol I'll be happy if folks can come up with something better). Basically:

def no_type?
  type.nil?
end

that way it doesn't break the old products with types, but also supports the new way creating products. What do you think? It'll be great if you could update the PR to support it. But I'll understand if you'd rather not.

[jamesprior]

Yeah I can do that! Maybe call it empty_type? Or similar. I’ll write out a few and see what seems to fit.

--James

On Oct 31, 2020, at 10:52 AM, SengMing Tan notifications@github.com wrote:

 I just realized that the problem with the way validations is written right now is that it presumes that type is either good? or service?. How about re-writing this a little bit more so that it supports another type, no_type? (this is not a good name lol I'll be happy if folks can come up with something better). Basically:

def no_type? type.nil? end that way it doesn't break the old products with types, but also supports the new way creating products. What do you think? It'll be great if you could update the PR to support it. But I'll understand if you'd rather not.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[jamesprior]

How's this version look? Rather than checking for unless good or empty it checks for services. That might not quite be the desired behavior but the specs pass :D

[tansengming]

This looks good! Let's roll with it.

There's just one one snag. I tried running the test example to stripe and got this:

$ be rake stripe:prepare
rake aborted!
Stripe::InvalidRequestError: caption is currently only supported for products of type `good`.

Please:

• [ ] fix the example,
• [ ] add the validation for caption, along with the spec.

It should be good to go after that!

[jamesprior]

I'm glad you're following up! Can you post the sample you were using?

Stripe.product :primo do |product|
  product.name = 'PRIMO as a service'
  product.type = 'service'
  product.statement_descriptor = 'PRIMO'
end

works for me locally.

Stripe.product :caption_test do |product|
  # product's name as it will appear on credit card statements
  product.name = 'Unspecified product type with caption'
  product.caption = 'foo'
  product.type = 'service'
end

Raises Stripe::InvalidConfigurationError ({:caption=>["must be blank"]}) before it hits stripe which matches the test I see at https://github.com/jamesprior/stripe-rails/blob/patch-1/test/product_builder_spec.rb#L81

[tansengming]

I was using

        Stripe.product :primo do |product|
          product.name        = 'Acme as a service PRIMO'
          product.active      = true
          product.attributes  = ['size', 'gender']
          product.metadata    = {:number_of_awesome_things => 5}
          product.statement_descriptor = 'PRIMO'
          product.caption     = 'So good it is Primo'
          product.description = 'One step beyond supreme'
          product.shippable   = false
        end

This is from the example you made ontest/product_builder_spec.rb

[jamesprior]

Turns out Stripe will set the product type to 'good' if it is unset in the request. Once that happens, every validation that happens in master now is correct and consistent with Stripe. I don't think there's anything else to do here in that case.

[jamesprior]

Once more - thanks for reviewing this and keeping me honest! Sorry it was a dead end but I'm glad we had this journey together 😄

[tansengming]

oh wow thanks @jamesprior for the detective work. I wish this was documented somewhere!

Just one more note for future reference: when creating a new product without specifying a type, Stripe defaults to a "service". This was tested on https://github.com/tansengming/rails-base/tree/test/pr-198 with

Stripe.product :test_product_no_type do |product|
  product.name        = 'Acme as a service PRIMO'
  product.active      = true
  product.attributes  = ['size', 'gender']
  product.metadata    = {:number_of_awesome_things => 5}
  product.statement_descriptor = 'PRIMO'
  product.description = 'One step beyond supreme'
end

Response

> bundle exec rake stripe:prepare 

[CREATE] - Stripe::Product:{
  "id": "test_product_no_type",
  "object": "product",
  "active": true,
  "attributes": [
    "size",
    "gender"
  ],
  "created": 1606579093,
  "description": "One step beyond supreme",
  "images": [

  ],
  "livemode": false,
  "metadata": {
    "number_of_awesome_things": "5"
  },
  "name": "Acme as a service PRIMO",
  "statement_descriptor": "PRIMO",
  "type": "service",
  "unit_label": null,
  "updated": 1606579093
}