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

Wrong invoices count (always 4) #807

Closed
vedmant opened this issue Nov 19, 2019 · 6 comments · Fixed by #879
Closed

Wrong invoices count (always 4) #807

vedmant opened this issue Nov 19, 2019 · 6 comments · Fixed by #879
Assignees

Comments

@vedmant
Copy link

vedmant commented Nov 19, 2019

I have an issue with following:

\Stripe\Stripe::setApiKey($_ENV['STRIPE_KEY']);
$invoices = \Stripe\Invoice::all(['limit' => 100]);

$invoices->count() always returns 4 even if when I iterate over results it clearly shows more records. count($invoices->data) returns correct number of 100.

@remi-stripe
Copy link
Contributor

@vedmant I think this is expected behaviour. The code for count on ApiResource here confirms that we don't override count for collections. The correct way to get to the number of invoices in your list is to use count($invoices->data).

Assigning to @ob-stripe in case this is something we should reconsider.

@vedmant
Copy link
Author

vedmant commented Nov 19, 2019

@remi-stripe I see, that's misleading, usually first thing how anyone would check count is to $invoices->count(); it's a collection and logically to try to get results count from it, or count($invoices) as it's implements Countable.

@ob-stripe
Copy link
Contributor

@brandur-stripe Any thoughts on this? We have the same issue in stripe-ruby and stripe-python (and possibly others).

On the one hand, changing the behavior of count() on list objects would make it inconsistent with the behavior on "normal" Stripe objects, and could even be considered a breaking change. On the other hand, we've gone to great lengths to make iterable semantics work on list objects, so adding countable semantics would not be a big stretch.

@brandur-stripe
Copy link
Contributor

@ob-stripe Good question. I think I'd usually feel adverse to overloading objects to make them seem more like objects they're not — technically the count result of 4 is correct, and there may be cases where you really do want to examine the list object rather than the data it's encapsulating.

That said, yep, we're pretty bar down the road of making list objects look like lists already, so I can see the argument for. I'm a little worried that we'd keep finding little holes like this one in the facade, but there's a reasonable chance that count would be the last one.

So all in all, I probably wouldn't jump on implementing this, but wouldn't put up any opposition either.

@nomadme
Copy link

nomadme commented Feb 14, 2020

I also wanted to chime in to this issue. This is true on all list objects.

Logically, I would expect that $list->count() would show me how many items in the data property right away.

@ob-stripe
Copy link
Contributor

Fixed in 7.25.0.

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 a pull request may close this issue.

5 participants