Add support for Subscription Schedules #590
Conversation
remi-stripe
force-pushed
the
remi-add-subscription-schedules
branch
from
528d54b to
2c1355f
Compare
| /** | ||
| * @return string The API URL for this Subscription Schedule Revision. | ||
| */ | ||
| public function instanceUrl() |
There was a problem hiding this comment.
OOC do we really need this?
There was a problem hiding this comment.
Not really: SubscriptionScheduleRevision doesn't have any instance methods, so this is never called. It's fine to leave it though.
| public function instanceUrl() | ||
| { | ||
| $id = $this['id']; | ||
| $schedule = $this['schedule']; |
There was a problem hiding this comment.
Soooo I just realized we do a lot of magic in the library to find the parent id. And I worry a liiiiittle bit that this is not documented anywhere (internally at Stripe). And that one day we would rename this to subscription_schedule because it seems more consistent and then break half the libraries out there.
I wonder if settling on parent for the name across all parent resources would be safer. Just food for thoughts
| * | ||
| * @throws \Stripe\Error\InvalidRequest | ||
| */ | ||
| public static function all($params = null, $opts = null) |
There was a problem hiding this comment.
Weirdly this was not verified in Person.php. Should it?
There was a problem hiding this comment.
I'm not convinced those methods that immediately raise an exception are very useful, but it's better to be consistent, so I'd say yes.
remi-stripe
force-pushed
the
remi-add-subscription-schedules
branch
from
2c1355f to
dabd5f9
Compare
|
@ob-stripe I retried the tests numerous times to no avail. Looks like the hhvm builds are failing but I don't think it's related to my changes. Can you double check and if it's all good review and release? |
Yeah, this is unrelated. HHVM 4.0 was released yesterday, and it's no longer compatible with PHP. We'll have to remove HHVM from the CI, or maybe pin it to the last 3.x version. |
| /** | ||
| * @return string The API URL for this Subscription Schedule Revision. | ||
| */ | ||
| public function instanceUrl() |
There was a problem hiding this comment.
Not really: SubscriptionScheduleRevision doesn't have any instance methods, so this is never called. It's fine to leave it though.
| * | ||
| * @throws \Stripe\Error\InvalidRequest | ||
| */ | ||
| public static function all($params = null, $opts = null) |
There was a problem hiding this comment.
I'm not convinced those methods that immediately raise an exception are very useful, but it's better to be consistent, so I'd say yes.
* Add support for Subscription Schedule * Add support for Subscription Schedule Revision * Upgrade stripe-mock to 0.44 * Update resources and tests impacted by the Payment Intent changes
remi-stripe
force-pushed
the
remi-add-subscription-schedules
branch
from
dabd5f9 to
3e7e4d3
Compare
|
@ob-stripe Fixed! PTAL |
This adds two new resources: Subscription Schedule and Subscription Schedule Revision. The latter is nested under the former.
This should be ready even though we won't merge until all libraries are ready and stripe-mock supports those new endpoints publicly.
r? @ob-stripe
cc @stripe/api-libraries @alexander-stripe