stripe-elixir icon indicating copy to clipboard operation
stripe-elixir copied to clipboard

Documentation

Open aphillipo opened this issue 8 years ago • 10 comments
trafficstars

In terms of documentation what do you think about following the Official Guides where possible, I've largely removed sections I feel are not relevant:

  • Payments
    • Quickstart
    • Checkout
    • Charges
      • Creating Charges
      • Declines & Failed Payments
      • Disputes
      • Refunds
      • Receiving Payouts
      • Supported Currencies
    • Products & Orders
      • Orders
      • Tax Integrations
      • Shipping Integrations
  • Subscriptions
    • Quickstart
    • Creating Subscriptions
  • Connect
    • Overview
      • On Demand Apps
      • Store Builders
    • Connecting to Accounts
    • Standalone Accounts
    • Managed Accounts
    • Creating Charges
    • Debiting Managed Accounts
    • Using Subscriptions
    • Multiple Currencies
    • Account Balances
    • Testing
  • Handling Webhooks

I'm happy to get a PR done stubbing these out and starting on them. Thanks for this library, it looks amazing and seems very complete. Just be good to have examples for the above I think, with links to the original docs.

aphillipo avatar May 15 '17 08:05 aphillipo

@aphillipo thanks for the suggest. I would love to spend time to help improve documentation.

The reason I have not done anything detailed like that is because our API is 99% the same as the official ruby/JS API. We also have the Hex.pm doc where you can look up the modules and function definitions as needed.

Did you run into a situation where the current Hex docs + code won't let you figure out an API? If not I think the best way now is to keep improving the Hex docs instead of duplicating the official stripe docs here. Let me know what you think.

sikanhe avatar May 16 '17 13:05 sikanhe

I see what you mean; however top of my list I'd like to see a few examples at least of the data structures I need to pass to this API. At a first glance I don't know until I test it if I should pass Atoms or Strings in the map keys (guess strings) and looking at the code there is no Account.save or some such from the ruby api, an :update is a request!

Finally I'm not certain how to document methods generated by the macros...

aphillipo avatar May 29 '17 12:05 aphillipo

@aphillipo I might be migrating away from the macro way of doing things and just write out all the functions explicitly. It was done the current way because I needed to get to v1 quickly without much writing and repetitive testing.

As for what you take for post data they can either be anything - String/Atoms, in either Maps/KeywordLists. All of the following should work since they are just converted to query params, you can see all of those types handled in the utils file(https://github.com/sikanhe/stripe-elixir/blob/master/lib/stripe/utils.ex#L10).

Stripe.charge(customer: "123")
Stripe.charge([{"customer", "123"}]) 
Stripe.charge(%{customer: "123"})
Stripe.charge(%{"customer" => "123"})

I could definitely put a note of this in the readme.

sikanhe avatar May 30 '17 20:05 sikanhe

Seems like something Elixir should support though; @docformacro :update "etc... example, doc tests etc". The way you have done it looks good to me.

aphillipo avatar May 30 '17 22:05 aphillipo

I would have to agree the way you currently have it seems correct. However writing out the function does have the benefit of being very explicit and easier to understand for new people coming into the library, and lowers the barrier of entry for contributions.

As for the documentation I would like to second the idea of examples. For most people examples are the easiest way to learn a new library. Also you may have people from Haskel, C, PHP, Rust or any number of other languages who have never used the Ruby/JS library.

One example where the hex docs fall down is the subscription create action. It is not very clear what "form" is and how it uses it to create a subscription, or what is required to initiate a create. At first glance it would see you feed it the entire form and its params from the page, but if you read the code its more like the other functions. This is mirrored by many of the create methods. They would greatly benefit from usage examples or clarification of the argument name, and most likely both.

sublimecoder avatar Jun 02 '17 19:06 sublimecoder

https://hexdocs.pm/elixir/Module.html#add_doc/6

Could prove useful for doing this... sorry about closing the issue, should probably stay open?

aphillipo avatar Jun 26 '17 08:06 aphillipo

@aphillipo @sublimecoder Hey I am looking to completely do away with use for 1.0 and replace it with behaviors. Take a look at https://github.com/sikanhe/stripe-elixir/pull/10 let me know your thoughts on it.

Additionally I am also playing with having API modules only generate a %Stripe.Request{} struct and have it pipe into Stripe.request/1 function to make it more functional. So you could do something like:

{:ok, cus} = Stripe.Customer.retrieve("cus_id") 
                    |> Stripe.request(stripe_account: "some_acct_id",
                                      api_key: "api_key_on_demand",
                                ...other request related options) 

This allows for nice a separation of business logic arguments/options and request args/options. It will also simplify the code base a lot by removing the need for the last opts argument for each function.

sikanhe avatar Jun 26 '17 20:06 sikanhe

I think thats a much more elixir-ish solution. I would recommend to have multiple versions of request with different arity so you can just pipe directly into request for a more simplistic syntax depending on how you need to use it. So the usage would look something like this. I think that's what you're going for right?

{:ok, cus} = Stripe.Customer.retrieve("cus_id") |> Stripe.request() 
{:ok, cus} = Stripe.Customer.update("cus_id") |> Stripe.request(options)
{:ok, cus} = Stripe.Customer.update("cus_id") |> Stripe.request(options, stripe_account: "acct_id", api_key: api_key) 

I think that would give increased flexibility. Only thing I'm against is having the explicitly declare stripe account and api_key on every request, especially if we can default it to config or env vars, then allow the user to override where it makes sense.

sublimecoder avatar Jun 27 '17 08:06 sublimecoder

@sublimecoder Stripe.request will only take one argument and its the request options. What "options" are you referring to here for first argument of Stripe.request in your Customer.update examples? For updates it would look like this under the new api

{:ok, cus} = Stripe.Customer.update("cus_id", %{metadata: %{some: "value"}}) |> Stripe.request(api_key: "...") 

stripe_account and api_keyare optional, they are just shown as examples as what you can put in the request options.

sikanhe avatar Jun 27 '17 13:06 sikanhe

@sikanhe Ah I see. I think that's a decent way of doing it. Sorry for the delayed response here.

sublimecoder avatar Aug 09 '17 23:08 sublimecoder