Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add "OnBehalfOf" to subscriptions #700

Merged
merged 1 commit into from
Oct 3, 2018
Merged

Add "OnBehalfOf" to subscriptions #700

merged 1 commit into from
Oct 3, 2018

Conversation

klauspost
Copy link
Contributor

It seems like OnBehalfOf was added to SubscriptionParams but not the subscription objects themselves.

@remi-stripe
Copy link
Contributor

@klauspost Thank you for the pull request! Since this field represents an Account id (acct_123) it can be Expanded and return an Account object too. Would you be up for changing your PR to support this? You can see how we do this on the Charge resource here for destination. This makes me realize we should also support on_behalf_of on the Charge resource too.

Let me know if it'd be easier for us to do the changes!

@klauspost
Copy link
Contributor Author

@klauspost Thank you for the pull request! Since this field represents an Account id (acct_123) it can be Expanded and return an Account object too.

@remi-stripe I don't really see how that works. Unless you always "expand", I can't really figure out how it falls back to the ID.

Let me know if it'd be easier for us to do the changes!

Thanks - I would appreciate that since I don't really get how the expand functionality works.

@remi-stripe
Copy link
Contributor

@klauspost This idea is that the library will automatically deserialize the value, if it's a string it will only set OnBehalfOf.Id and if it's an object it will deserialize all the Account properties automatically. You only need to replace string by *Account in your PR and then update the tests.

Let me know if that clarifies how to do this. Otherwise I can take over in a separate PR instead to show you!

@klauspost
Copy link
Contributor Author

Ahh.. The ParseID in the Account unmarhal! 👍 I'll give it a shot :)

@remi-stripe
Copy link
Contributor

Correct, it's a bit of magic we put on all resources that can be expanded so that it "just works".

@klauspost
Add "OnBehalfOf" to subscription and charge
74c5e6e 
It seems like `OnBehalfOf` was added to SubscriptionParams but not the subscription objects themselves.

Added to `Subscription` and `Charge`.
@klauspost klauspost force-pushed the add-on-behalf-of-subscriptions branch from c6c1d8a to 74c5e6e Compare October 3, 2018 14:04
@klauspost
Copy link
Contributor Author

@remi-stripe Updated :)

remi-stripe
remi-stripe approved these changes Oct 3, 2018
View reviewed changes
Copy link
Contributor

@remi-stripe remi-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you for the quick work

@remi-stripe remi-stripe merged commit 400c8b5 into stripe:master Oct 3, 2018
@remi-stripe
Copy link
Contributor

@klauspost Released as 51.1.0. Thanks for working on this, that's awesome!

@klauspost klauspost deleted the add-on-behalf-of-subscriptions branch October 3, 2018 14:20
nadaismail-stripe pushed a commit that referenced this pull request Oct 18, 2024
@mbianco-stripe
Add metadata and period information to proration line item (#700)
f14d080 
* Set metadata on add_invoice_items and proration data

* Tests for built-in metadata and proration settings

* Missing types

* typing fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@remi-stripe remi-stripe remi-stripe approved these changes

Assignees

@klauspost klauspost

Labels
approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@klauspost @remi-stripe @stripe-ci