Permalink
Browse files

Merge pull request #1563 from gregbell/fix-collections-with-group-by

Fix collections with group by
  • Loading branch information...
2 parents 3130933 + da6e448 commit ecf5b9789c56fce014aec184446d913d97c0df1e @gregbell gregbell committed Aug 20, 2012
@@ -3,31 +3,31 @@ Feature: Index Scoping
Viewing resources and scoping them
Scenario: Viewing resources with one scope and no default
- Given 10 posts exist
+ Given 3 posts exist
And an index configuration of:
"""
ActiveAdmin.register Post do
scope :all
end
"""
Then I should see the scope "All" not selected
- And I should see the scope "All" with the count 10
- And I should see 10 posts in the table
+ And I should see the scope "All" with the count 3
+ And I should see 3 posts in the table
Scenario: Viewing resources with one scope as the default
- Given 10 posts exist
+ Given 3 posts exist
And an index configuration of:
"""
ActiveAdmin.register Post do
scope :all, :default => true
end
"""
Then I should see the scope "All" selected
- And I should see the scope "All" with the count 10
- And I should see 10 posts in the table
+ And I should see the scope "All" with the count 3
+ And I should see 3 posts in the table
Scenario: Viewing resources with one scope and no results
- Given 10 posts exist
+ Given 3 posts exist
And an index configuration of:
"""
ActiveAdmin.register Post do
@@ -36,15 +36,15 @@ Feature: Index Scoping
end
"""
- When I fill in "Search Title" with "Hello World 17"
+ When I fill in "Search Title" with "Non Existing Post"
And I press "Filter"
And I should not see the scope "All"
When I am on the index page for posts
Then I should see the scope "All" selected
Scenario: Viewing resources with a scope but scope_count turned off
- Given 10 posts exist
+ Given 3 posts exist
And an index configuration of:
"""
ActiveAdmin.register Post do
@@ -54,11 +54,11 @@ Feature: Index Scoping
"""
Then I should see the scope "All" selected
And I should see the scope "All" with no count
- And I should see 10 posts in the table
+ And I should see 3 posts in the table
@scope
Scenario: Viewing resources with a scope and scope count turned off for a single scope
- Given 10 posts exist
+ Given 3 posts exist
And an index configuration of:
"""
ActiveAdmin.register Post do
@@ -67,11 +67,11 @@ Feature: Index Scoping
"""
Then I should see the scope "All" selected
And I should see the scope "All" with no count
- And I should see 10 posts in the table
+ And I should see 3 posts in the table
Scenario: Viewing resources when scoping
- Given 6 posts exist
- And 4 published posts exist
+ Given 2 posts exist
+ And 3 published posts exist
And an index configuration of:
"""
ActiveAdmin.register Post do
@@ -81,15 +81,49 @@ Feature: Index Scoping
end
end
"""
- Then I should see the scope "All" with the count 10
- And I should see 10 posts in the table
- Then I should see the scope "Published" with the count 4
+ Then I should see the scope "All" with the count 5
+ And I should see 5 posts in the table
+ And I should see the scope "Published" with the count 3
When I follow "Published"
Then I should see the scope "Published" selected
- And I should see 4 posts in the table
+ And I should see 3 posts in the table
+
+ Scenario: Viewing resources when scoping and filtering
+ Given 2 posts written by "Daft Punk" exist
+ Given 1 published posts written by "Daft Punk" exist
+
+ Given 1 posts written by "Alfred" exist
+ Given 2 published posts written by "Alfred" exist
+
+ And an index configuration of:
+ """
+ ActiveAdmin.register Post do
+ scope :all, :default => true
+ scope :published do |posts|
+ posts.where("published_at IS NOT NULL")
+ end
+ end
+ """
+ Then I should see the scope "All" with the count 6
+ And I should see the scope "Published" with the count 3
+ And I should see 6 posts in the table
+
+ When I follow "Published"
+ Then I should see the scope "Published" selected
+ And I should see the scope "All" with the count 6
+ And I should see the scope "Published" with the count 3
+ And I should see 3 posts in the table
+
+ When I select "daft_punk" from "Author"
+ And I press "Filter"
+
+ Then I should see the scope "Published" selected
+ And I should see the scope "All" with the count 3
+ And I should see the scope "Published" with the count 1
+ And I should see 1 posts in the table
Scenario: Viewing resources with optional scopes
- Given 10 posts exist
+ Given 3 posts exist
And an index configuration of:
"""
ActiveAdmin.register Post do
@@ -109,10 +143,10 @@ Feature: Index Scoping
And I should not see the scope "All"
And I should not see the scope "Today"
And I should see the scope "Shown"
- And I should see the scope "Default" with the count 10
+ And I should see the scope "Default" with the count 3
Scenario: Viewing resources with multiple scopes as blocks
- Given 10 posts exist
+ Given 3 posts exist
And an index configuration of:
"""
ActiveAdmin.register Post do
@@ -126,17 +160,17 @@ Feature: Index Scoping
"""
Then I should see the scope "Today" selected
And I should see the scope "Tomorrow" not selected
- And I should see the scope "Today" with the count 10
+ And I should see the scope "Today" with the count 3
And I should see the scope "Tomorrow" with the count 0
- And I should see 10 posts in the table
+ And I should see 3 posts in the table
And I should see a link to "Tomorrow"
When I follow "Tomorrow"
Then I should see the scope "Tomorrow" selected
And I should see the scope "Today" not selected
And I should see a link to "Today"
- Scenario: Viewing resources with scopes when a filter is applied
+ Scenario: Viewing resources with scopes when scoping to user
Given 2 posts written by "Daft Punk" exist
And a post with the title "Monkey Wrench" written by "Foo Fighters" exists
And a post with the title "Everlong" written by "Foo Fighters" exists
@@ -145,6 +179,7 @@ Feature: Index Scoping
ActiveAdmin.register Post do
scope_to :current_user
scope :all, :default => true
+
filter :title
controller do
@@ -160,3 +195,49 @@ Feature: Index Scoping
And I press "Filter"
Then I should see the scope "All" not selected
And I should see the scope "All" with the count 1
+
+ Scenario: Viewing resources when scoping and filtering and group bys and stuff
+ Given 2 posts written by "Daft Punk" exist
+ Given 1 published posts written by "Daft Punk" exist
+
+ Given 1 posts written by "Alfred" exist
+
+ And an index configuration of:
+ """
+ ActiveAdmin.register Post do
+ scope :all, :default => true
+ scope :published do |posts|
+ posts.where("published_at IS NOT NULL")
+ end
+
+ index do
+ column :author_id
+ column :count
+ end
+
+ config.sort_order = "author_id_asc"
+
+ controller do
+ def scoped_collection
+ Post.select("author_id, count(*) as count").group("author_id")
+ end
+ end
+ end
+ """
+ Then I should see the scope "All" with the count 2
+ And I should see the scope "Published" with the count 1
+ And I should see 2 posts in the table
+
+ When I follow "Published"
+ Then I should see the scope "Published" selected
+ And I should see the scope "All" with the count 2
+ And I should see the scope "Published" with the count 1
+ And I should see 1 posts in the table
+
+ When I select "daft_punk" from "Author"
+ And I press "Filter"
+
+ Then I should see the scope "Published" selected
+ And I should see the scope "All" with the count 1
+ And I should see the scope "Published" with the count 1
+ And I should see 1 posts in the table
@@ -13,11 +13,11 @@
Post.create! :title => title, :author => author, :published_at => published_at
end
-Given /^(\d+) posts? written by "([^"]*)" exist$/ do |count, author_name|
+Given /^(\d+)( published)? posts? written by "([^"]*)" exist$/ do |count, published, author_name|
first, last = author_name.split(' ')
author = User.find_or_create_by_first_name_and_last_name(first, last, :username => author_name.gsub(' ', '').underscore)
(0...count.to_i).each do |i|
- Post.create! :title => "Hello World #{i}", :author => author
+ Post.create! :title => "Hello World #{i}", :author => author, :published_at => (published ? Time.now : nil)
end
end
@@ -0,0 +1,23 @@
+module ActiveAdmin
+ module Helpers
+ module Collection
+ # Works around this issue: https://github.com/rails/rails/issues/7121
+ #
+ # GROUP BY + COUNT drops SELECT statement. This leads to SQL error when
+ # the ORDER statement mentions a column defined in the SELECT statement.
+ #
+ # We remove the ORDER statement to work around this issue.
+ def collection_size(collection=collection)
+ size = collection.reorder("").count
+ # when GROUP BY is used, AR returns Hash instead of Fixnum for .size
+ size = size.size if size.kind_of?(Hash)
+
+ size
+ end
+
+ def collection_is_empty?(collection=collection)
+ collection_size(collection) == 0
+ end
+ end
+ end
+end
@@ -88,17 +88,20 @@ def active_admin_collection
end
def scope_current_collection(chain)
+ @collection_before_scope = chain
+
if current_scope
- @before_scope_collection = chain
scope_chain(current_scope, chain)
- elsif active_admin_config.scope_to
- @before_scope_collection = scoped_collection
- chain
else
chain
end
end
+ def collection_before_scope
+ @collection_before_scope
+ end
+
+
include ActiveAdmin::ScopeChain
def current_scope
@@ -1,3 +1,5 @@
+require 'active_admin/helpers/collection'
+
module ActiveAdmin
module Views
@@ -85,12 +87,14 @@ def build_download_format_links(formats = [:csv, :xml, :json])
end
end
+ include ::ActiveAdmin::Helpers::Collection
+
# modified from will_paginate
def page_entries_info(options = {})
if options[:entry_name]
entry_name = options[:entry_name]
entries_name = options[:entries_name] || entry_name.pluralize
- elsif collection.empty?
+ elsif collection_is_empty?
entry_name = I18n.translate("active_admin.pagination.entry", :count => 1, :default => 'entry')
entries_name = I18n.translate("active_admin.pagination.entry", :count => 2, :default => 'entries')
else
@@ -99,17 +103,15 @@ def page_entries_info(options = {})
end
if collection.num_pages < 2
- case collection.size
+ case collection_size
when 0; I18n.t('active_admin.pagination.empty', :model => entries_name)
when 1; I18n.t('active_admin.pagination.one', :model => entry_name)
else; I18n.t('active_admin.pagination.one_page', :model => entries_name, :n => collection.total_count)
end
else
- size = collection.size
- size = size.size if size.kind_of? Hash # when GROUP BY is used, AR returns Hash instead of Fixnum for .size
offset = (collection.current_page - 1) * collection.limit_value
total = collection.total_count
- I18n.t('active_admin.pagination.multiple', :model => entries_name, :from => offset + 1, :to => offset + size, :total => total)
+ I18n.t('active_admin.pagination.multiple', :model => entries_name, :from => offset + 1, :to => offset + collection_size, :total => total)
end
end
@@ -1,3 +1,5 @@
+require 'active_admin/helpers/collection'
+
module ActiveAdmin
module Views
@@ -7,6 +9,8 @@ class Scopes < ActiveAdmin::Component
builder_method :scopes_renderer
include ActiveAdmin::ScopeChain
+ include ::ActiveAdmin::Helpers::Collection
+
def default_class_name
"scopes table_tools_segmented_control"
@@ -54,22 +58,13 @@ def current_scope?(scope)
end
def current_filter_search_empty?
- collection.empty? && params.include?(:q)
+ params.include?(:q) && collection_is_empty?
end
# Return the count for the scope passed in.
def get_scope_count(scope)
- if params[:q]
- search(scope_chain(scope, scoping_class)).relation.count
- else
- scope_chain(scope, scoping_class).count
- end
+ collection_size(scope_chain(scope, collection_before_scope))
end
-
- def scoping_class
- assigns[:before_scope_collection] || active_admin_config.resource_class
- end
-
end
end
end
Oops, something went wrong.

0 comments on commit ecf5b97

Please sign in to comment.