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

fix count display for collections with GROUP BY #897

Merged
merged 2 commits into from
Jan 19, 2012

Conversation

comboy
Copy link
Contributor

@comboy comboy commented Jan 7, 2012

When used with something with GROUP BY as collection, size method will not return number, but hash instead. So instead of having some "Displaying all 123 posts", we get "Displaying all {"bar"=> 34, "baz" => 54} posts". Using total_count in case of a single page same as it is used for many pages, fixes the problem.

@pduersteler
Copy link

+1, experienced just now.

@pcreux
Copy link
Contributor

pcreux commented Jan 18, 2012

Thanks for this fix. Please add specs to "paginated_collection_spec.rb".

@comboy
Copy link
Contributor Author

comboy commented Jan 18, 2012

spec added, but I did not dive much into the test suite, I've created 3 objects in database, and I'm not sure if I should care about cleaning them up

@pcreux
Copy link
Contributor

pcreux commented Jan 19, 2012

@comboy That's perfect thanks!

pcreux added a commit that referenced this pull request Jan 19, 2012
---

When used with something with GROUP BY as collection, size method will not return number, but hash instead. So instead of having some "Displaying all 123 posts", we get "Displaying all {"bar"=> 34, "baz" => 54} posts". Using total_count in case of a single page same as it is used for many pages, fixes the problem.
@pcreux pcreux merged commit 0afef46 into activeadmin:master Jan 19, 2012
@comboy
Copy link
Contributor Author

comboy commented Jan 20, 2012

Thanks for quick merge :)

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.

3 participants