DOC 48 priv and stripe updates #1184
Conversation
|
Deploy preview for segment-docs-preview ready! Built with commit e53db9d https://deploy-preview-1184--segment-docs-preview.netlify.app |
|
Preview of updated Stripe doc: https://deploy-preview-1184--segment-docs-preview.netlify.app/docs/connections/sources/catalog/cloud-apps/stripe/ |
| @@ -391,58 +442,3 @@ Below are tables outlining the properties included in the collections listed abo | |||
| | `reversed` | Whether the transfer has been fully reversed. If the transfer is only partially reversed, this attribute will still be false | | |||
| | `source_transaction` | ID of the charge or payment that was used to fund the transfer. If null, the transfer was funded from the available balance | | |||
|
|
|||
| ## V2 Changelog | |||
There was a problem hiding this comment.
Curious if the change log is being moved to somewhere else?
There was a problem hiding this comment.
@maggieyu-segment Removed the changelog for now, as it had not been updated in over a year. I think before we re-add it, we need a strategy to make sure it stays up-to-date.
| | `amount` | | | ||
| | `currency` | | | ||
| | `description` | | | ||
|
|
There was a problem hiding this comment.
The transform() in stripe source is not very straightforward. Other than the properties in map[string]interface{}, some properties are added using tr.Flatten(). For example, for order_shipping_methods, there is this line of code:
tr.Flatten(tr.GetMap(method, "delivery_estimate"), "delivery_estimate_", properties)
tr.Flatten() flattens the properties in delivery_estimate. For example, from the Stripe API doc, there is delivery_estimate.date, and this property will be named delivery_estimate_date in the object.
I also noticed that these flattened properties are not captured in some other collections, e.g. accounts. Looks like we may need a wider scope of auditing for the existing doc. (Just pointing this out - feel free to keep the scope of this PR/ ticket to what you intended in the beginning.)
There was a problem hiding this comment.
Thanks for the info. I should be able to expand the scope here and push this out today.
There was a problem hiding this comment.
@maggieyu-segment Also, do we flatten to n levels deep? For example, in charges, we flatten the shipping object. So for things in shipping_address, does that flatten to shipping_address_city etc.?
There was a problem hiding this comment.
There isn't a limitation on the number of levels we flatten to. Flatten() will continue to flatten the child value until the value is no longer a map[string]interface{}. Yes, the charges example you have is correct.
|
|
||
| | Property Name | Description | | ||
| | ------------------------- | ------------------------------------------------------------ | | ||
| | `order_id` | | |
There was a problem hiding this comment.
Will the descriptions be added in a separate PR?
There was a problem hiding this comment.
Still need to add these. Would you recommend using the Stripe API docs to adapt these definitions?
For example, here, I can guess what order_id is used for, but I want to be as accurate as possible.
There was a problem hiding this comment.
Yes, we could use the Stripe API docs for the definitions. They should match for most cases (I can't think of a property that drifts far away from the original property from Stripe. I would say, the ones that have special handling (i.e. not a straightforward prop = obj["id"] or tr.Flatten()) in transform() may require more attention.)
|
Hi @maggieyu-segment, I think we should be good to go here. Can you have a look and hopefully sign off when you have a moment? For the record, rather than list all the flattened properties, in some cases, I describe what has been flattened, provide an example, and link to the relevant section of Stripe's docs. Otherwise, these tables would be massive in spots. Preview here |
|
Thanks @maggieyu-segment. I really appreciate you having a look at this. I'm going to merge as-is, and we can pick up additional updates in another PR. |
Merge timing
Related issues (optional)
DOC-48