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

custom sort on custom column in index ? #3085

Closed
Epiju opened this issue Apr 17, 2014 · 36 comments · Fixed by #4768
Closed

custom sort on custom column in index ? #3085

Epiju opened this issue Apr 17, 2014 · 36 comments · Fixed by #4768
Assignees

Comments

@Epiju
Copy link

Epiju commented Apr 17, 2014

Hi,

I'm looking for a way to make a custom sort on a custom column in an index page.

ActiveAdmin.register User do
  index do
    id_column
    ...
    column 'Drivers' do |user|
      user.drivers_in_neighborhood.count
    end
  end
end

My User#drivers_in_neighborhood method is in User model and is doing SQL queries.
I just want to make the column "Drivers" sortable.

How to do that?

Thanks!

@seanlinsley
Copy link
Contributor

What database queries does drivers_in_neighborhood perform?

@Epiju
Copy link
Author

Epiju commented Apr 17, 2014

It joins tables, mostly.

Something like :

Address.joins('LEFT JOIN vehicles ON vehicles.id = addresses.addressable_id').
        joins('LEFT JOIN users ON users.id = vehicles.main_owner_id').
        where('(point(addresses.longitude, addresses.latitude) <@> point(?,?)) <= ? AND addressable_type = ?',
              self.longitude, self.latitude, radius * 0.621371192, 'Vehicle').
        where('users.owner_completion >= ?', completion)

@seanlinsley
Copy link
Contributor

Ideally you'd be able to pass raw SQL like this:

column 'Drivers', sortable: 'sum(vehicles.id)'

but that's not currently possible.

@Epiju
Copy link
Author

Epiju commented Apr 22, 2014

Yeah, it'd be perfect if I could to that!

So it means that we currently cannot sort a custom column?

There parameter must be an attribute and cannot be a method which do the sort?

Thanks

@dmitry
Copy link
Contributor

dmitry commented Apr 22, 2014

I've made some success locally, but haven't add it as a pull-request yet.

@Epiju
Copy link
Author

Epiju commented Apr 22, 2014

Cool @dmitry ! Could you put it in a gist or tell me how to do this? It'd be great!

@Epiju
Copy link
Author

Epiju commented Apr 23, 2014

@dmitry Could you help me on that?

@dmitry
Copy link
Contributor

dmitry commented Apr 23, 2014

@Epiju look to the ActiveAdmin::OrderClause. You need to change /^([\w\_\.]+)(->'\w+')?_(desc|asc)$/ regexp.

@timoschilling
Copy link
Member

related to #3173

@seanlinsley
Copy link
Contributor

Ultimately there's no regular expression that could match every possible query. We're going to have to remove the validation entirely.

@timoschilling
Copy link
Member

I thing we could have a sql injection problem then!

We should rewrite the sorting.

column :foo, sorting: 'bar.foo' do; end

In that case we give bar.foo_desc over the url into .order(...), thats makes a SQL injection possible.

We should pass only the column name and sort order through the url and make a lookup to generate the order query.

@seanlinsley
Copy link
Contributor

We should pass only the column name and sort order through the url and make a lookup to generate the order query.

👍

@timoschilling timoschilling self-assigned this Oct 21, 2014
@gkop
Copy link

gkop commented Jan 30, 2015

+1 @timoschilling , wanting to sort by count(...) but running into this wall.

@timoschilling
Copy link
Member

@gkop depend on what kind of count do you want to do, take a look on:
http://guides.rubyonrails.org/association_basics.html#belongs-to-association-reference (4.1.2)

@gkop
Copy link

gkop commented Jan 30, 2015

Thanks @timoschilling , you're referring to a counter cache eh? My scenario is habtm with conditions. I don't think you can use the counter cache with conditions - it gets messy - IE a column of an associated row may change, which should affect our count, so then activerecord must not only observe the lifecycle of the joiner rows but also the associated records.

@timoschilling
Copy link
Member

@gkop yes, I think it don't work for your case too. But I many "I want to sort by count" cases it helps

@kaelumania
Copy link

I also wanna do something like:

column :foo, sortable: 'lower(foo)'

for case-insensitive sorting!

@Fivell
Copy link
Member

Fivell commented May 7, 2015

guys, how you can override apply_sorting on controller level

sort of

controller do 
  def apply_sorting(chain)
      order_clause = ActiveAdmin::OrderClause.new params[:order]
      if order_clause.field == 'lower_foo'
        chain.reorder("lower(foo)  #{order_clause.order}")
      else
        super
      end
    end
end

and then describe column like

column :foo, sortable: :lower_foo

@Fivell
Copy link
Member

Fivell commented May 7, 2015

Just an Idea, would be nice to have something like

ActiveAdmin.register User do
   config.order_clause_class = ActiveAdmin::UserOrderClause
end

in this case we can extend ActiveAdmin::OrderClause with our own class to support custom sortings

@smtheard
Copy link

smtheard commented Mar 7, 2016

What is the status of this issue? I am coming across the same problem as the original poster. Is there a workaround?

@gkop
Copy link

gkop commented Mar 7, 2016

@smtheard I gave up without discovering a workaround.

Instead I have a numeric filter on the counts, which required custom scopes.

@seanlinsley
Copy link
Contributor

We could use existing aliases:

controller do
  def scoped_collection
    super.includes(:authors).select '*, count(authors.*) as author_count'
  end
end

index do
  column(:authors, sortable: 'author_count') { |b| b.authors.count }
end

Or we could allow sortable to be set with custom SQL as long as an alias is defined in the string:

controller do
  def scoped_collection
    super.includes(:authors)
  end
end

index do
  column(:authors, sortable: 'count(authors.*) as author_count') { |b| b.authors.count }
end

In both cases, we'd use the alias in the HTML, instead of the SQL fragment itself.

@seanlinsley
Copy link
Contributor

I'd probably prefer to only support one of these, to reduce complexity. What does everyone think?

@seanlinsley
Copy link
Contributor

The first option would better suit @Epiju's needs. It could also allow you to do this:

controller do
  def scoped_collection
    super.includes(:authors).select '*, count(authors.*) as author_count'
  end
end

index do
  column :author_count, sortable: true
end

@aruncsengr
Copy link

Guys and @seanlinsley, i tried to implement the following approach but facing a issue in it. Here goes the code snippet:

class FailureGroup < ActiveRecord::Base
  has_many :jobs, -> { where(status: Job::STATUS_FAILED).with_exception }
end
index do
  column :jobs_count, sortable: true do |failure_group|
    link_to failure_group.jobs.count, admin_failure_group_jobs_path(failure_group)
  end
end

controller do
  def scoped_collection
    super.includes(:jobs).select '*, count(jobs.*) as jobs_count'
  end
end

I get the following error:
PG::UndefinedTable - ERROR: missing FROM-clause entry for table "jobs"
LINE 1: SELECT , count(jobs.) as jobs_count FROM "failure_groups"...

It would be really helpful if someone support me in resolving this issue!

@dmitry
Copy link
Contributor

dmitry commented May 6, 2016

You need eager_load(:jobs)

@aruncsengr
Copy link

aruncsengr commented May 6, 2016

Thanks @dmitry i tried with that, then i faced the error:

PG::GroupingError - ERROR: column "failure_groups.id" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: SELECT , count(jobs.) as jobs_count, "failure_groups"."id"...

So changed my code to:
super.eager_load(:jobs).select('*, count(jobs.*) as jobs_count').group("failure_groups.id")
but again it leads to another grouping error:

PG::GroupingError - ERROR: column "jobs.id" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: SELECT , count(jobs.) as jobs_count, "failure_groups"."id"...

Any idea how to proceed further?

@krishnasrihari
Copy link
Contributor

I have tried @seanlinsley solution but this is not working as include doesn't include any has_many relation column. I have come up with new solution here

super.joins("LEFT JOIN invites ON profiles.id = invites.profile_id").select("profiles.*, COUNT(invites.id) as invite_count").group("profiles.id")

column :invite_count, sortable: true

@aruncsengr
Copy link

aruncsengr commented May 9, 2016

Cool, thank u so much @krishnasrihari, i have analyzed it and would like to suggest a minor change to resolve sorting URL issue.,
column :invite_count, sortable: :invite_count

Finally, here goes the working solution to get revealed so that it'll be helpful for others too.,

class FailureGroup < ActiveRecord::Base
  has_many :jobs, -> { where(status: Job::STATUS_FAILED).with_exception }
end
index do
  column :job_count, sortable: :job_count do |failure_group|
    link_to failure_group.jobs.count, admin_failure_group_jobs_path(failure_group)
  end
end

controller do
  def scoped_collection
    super.joins(
      %(LEFT JOIN "jobs" ON "jobs"."failure_group_id" = "failure_groups"."id"
        AND "jobs"."status" = #{Job::STATUS_FAILED}
        AND ("jobs"."exception" IS NOT NULL)))
      .select('failure_groups.*, COUNT(jobs.id) as job_count')
      .group('failure_groups.id')
  end
end

In my case, the condition is needed :) However the general solution will be.,

index do
  column :job_count, sortable: :job_count do |failure_group|
    link_to failure_group.jobs.count, admin_failure_group_jobs_path(failure_group)
  end
end

controller do
  def scoped_collection
    super.joins('LEFT JOIN "jobs" ON "jobs"."failure_group_id" = "failure_groups"."id"')
      .select('failure_groups.*, COUNT(jobs.id) as job_count')
      .group('failure_groups.id')
  end
end

@nicodeceulaer
Copy link

Thanks; this solved my problem.
One small remark, the above example code is missing a colon on invite_count

column :invite_count, sortable: :invite_count**

@aruncsengr
Copy link

@nicodeceulaer Thanks for the heads up! missed it while editing. Updated it now ;)

@nicodeceulaer
Copy link

nicodeceulaer commented Aug 30, 2016

The above solution does not work very well with the pagination logic.
Pagination triggers a count() on scoped_collection. But due to the join/select/group added there, the end result is an SQL query that reads the entire table:

def scoped_collection
  super.joins('LEFT JOIN "debugdumps" ON "debugdumps"."crash_id" = "crashes"."id"')
       .select('crashes.*, COUNT(debugdumps.id) as debugdump_count')
       .group('crashes.id')
end
SELECT COUNT(*) AS count_all, crashes.id AS crashes_id FROM "crashes" LEFT JOIN "debugdumps" ON "debugdumps"."crash_id" = "crashes"."id" GROUP BY crashes.id

Regretfully, the same scoped collection also blocks against the current pagination disable code. So the only way out now is to make my own pagination logic as proposed in stackoverflow

@Fivell
Copy link
Member

Fivell commented Jan 30, 2017

test and feedback are appriciated

@andreydeineko
Copy link

Is there a possibility to sort in index by method (non-db backed values)?

@heridev
Copy link

heridev commented Oct 8, 2018

This is the way I'm sorting by custom values and preventing n + 1 queries

class TechResult < ActiveRecord::Base
  belongs_to :user
end

class User < ActiveRecord::Base
  belongs_to :organization
end

class Organization < ActiveRecord::Base
  has_many :users
end

and then in your tech_results section, you want to render an organization field, in my case the value tech_result.user.organization.name this is the way to sort them out:

ActiveAdmin.register TechResult do
  config.batch_actions = false
  permit_params :user_id

  preserve_default_filters!

  index do
    column :id
    column 'user', sortable: 'users.name' do |tr|
      tr.user.name
    end
    column 'organization', nil, sortable: 'organizations.name' do |tr|
      tr.user.organization.name.titleize if tr.user.organization
    end

  end

  controller do
    def scoped_collection
      super.includes(user: [:organization])
    end
  end
end

@praveenrajt
Copy link

praveenrajt commented Feb 28, 2020

How do I sort store attribute accessor and method

That is, I need my table to be sortable by :status, :another_email_id, :another_phone_number, average

user.rb

class User < ApplicationRecord
has_many :posts
    valivalidates :email , presence: true
    valivalidates :name , presence: true
    valivalidates :phone_number , presence: true
    store :extra_fields, accessors: [:status, :another_email_id, :another_phone_number]

    def average
        ((self.posts.count/Post.all.count)*100).round(2)
    end
end

admin/users.rb

ActiveAdmin.register User do

index do
    column :name
    column :email
    column :phone_number
    column :status
    column :another_phone_number
    column :another_email_id
    column "Average",  &:average 
end

agrobbin added a commit to agrobbin/activeadmin that referenced this issue Nov 30, 2021
Consider sorting that requires a `JOIN` as part of the SQL query (or really anything that was raised in activeadmin#3085):

```ruby
ActiveAdmin.register Foo do
  config.order_clause = FooOrderClause

  index do
    column :latest_bar, sortable: 'bars.created_at' do
      ...
    end
  end
end

class FooOrderClause < ActiveAdmin::OrderClause
  def apply(chain)
    case table_column
    when 'bars.created_at'
      chain.joins(:bars).select(table_column)
    else
      super
    end
  end
end
```

Oftentimes, including that `JOIN` is not a huge deal in terms of performance, however, there have been situations where that table/column can increase the time it takes for a query to run by a couple hundred milliseconds, for various reasons.

Increasing the time to load the page slightly (0.1s) is not a big deal, in an admin, but imagine if you had 7-10 different scopes on that page as well? Because scopes leverage `collection_before_scope`, they will proceed to include the `JOIN` in their query, even though it is only necessary to do sorting of a collection that is not actually being sorted, only counted.

Applying sorting of the collection *after* scoping, instead of before, should have no bearing on the end-result, while ensuring that any custom sorting logic is not needlessly applied to all scope count queries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet