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.

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

Allow to pass params to autoPagingIterator method #1409

Closed
wants to merge 1 commit into from
Closed

Allow to pass params to autoPagingIterator method #1409

wants to merge 1 commit into from

Conversation

driesvints
Copy link

This PR resolves #1400

By allowing to pass params to the underlying nextPage or previousPage methods we can expand data for the resources that are retrieved.

Example:

$stripe = new \Stripe\StripeClient('stripe-secret');

$invoice = $stripe->invoices->retrieve('in_xxx', [
    'expand' => ['lines.data.tax_amounts.tax_rate'],
]);

foreach ($invoice->lines->autoPagingIterator(['expand' => ['data.tax_amounts.tax_rate']) as $line) {
    foreach ($line->tax_amounts as $tax_amount) {
        $tax_rate = $tax_amount->tax_rate;
        echo $tax_rate->display_name . ' ' . $tax_rate->percentage . '%' . PHP_EOL;
    }
}

@remi-stripe
Copy link
Contributor

Hey @driesvints Thanks a lot for the PR!

In theory the parameters should be kept during pagination and we've had similar issues in other libraries such as stripe/stripe-java#451

We'll have a look and see what is causing this and whether this fix is the right one!

@pakrym-stripe
Copy link
Contributor

@driesvints I filed an internal request to see if we can fix the service to carry expand parameter over to next pages of lines.

If you don't mind I'll hold off merging this PR until we hear from the service team.

@richardm-stripe
Copy link
Contributor

Hello @driesvints. We've changed the behavior on the API side so that merging this PR is no longer necessary (we closed out stripe/stripe-java#451, but missed this one.)

Please feel free to re-open if you notice any issues!

@driesvints driesvints deleted the patch-1 branch January 10, 2023 08:46
@driesvints
Copy link
Author

Amazing, thanks!

@driesvints
Copy link
Author

@richardm-stripe seems this isn't resolved yet: #1422

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If the invoice has more than 10 items, from item number 11 tax_amounts does not include expanded tax_rate.
4 participants