Merge pull request activeadmin#4768 from Fivell/order_clause

Fivell · web-flow · commit ab72c3351a5e · 2017-01-30T17:12:02.000+02:00
order clause refactoring, allow to use custom sql ordering strategies
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,7 @@
 
 #### Minor
 
+* Support for custom sorting strategies [#4768][] by [@Fivell][]
 * Stream CSV downloads as they're generated [#3038][] by [@craigmcnamara][]
   * Disable streaming in development for easier debugging [#3535][] by [@seanlinsley][]
 * Improved code reloading [#3783][] by [@chancancode][]
diff --git a/docs/3-index-pages/index-as-table.md b/docs/3-index-pages/index-as-table.md
@@ -164,6 +164,24 @@ index do
 end
 ```
 
+## Custom sorting 
+
+It is also possible to use database specific expressions and options for sorting by column
+
+```ruby
+order_by(:title) do |order_clause|
+   if order_clause.order == 'desc'
+     [order_clause.to_sql, 'NULLS LAST'].join(' ')
+   else
+     [order_clause.to_sql, 'NULLS FIRST'].join(' ')
+   end
+end
+
+index do
+  column :title
+end
+```
+
 ## Associated Sorting
 
 You're normally able to sort columns alphabetically, but by default you
@@ -187,6 +205,7 @@ index do
 end
 ```
 
+
 ## Showing and Hiding Columns
 
 The entire index block is rendered within the context of the view, so you can
diff --git a/lib/active_admin/application.rb b/lib/active_admin/application.rb
@@ -107,6 +107,9 @@ def initialize
     # Whether to display 'Current Filters' on search screen
     inheritable_setting :current_filters, true
 
+    # class to handle ordering
+    inheritable_setting :order_clause, ActiveAdmin::OrderClause
+
     # Request parameters that are permitted by default
     inheritable_setting :permitted_params, [
       :utf8, :_method, :authenticity_token, :commit, :id
diff --git a/lib/active_admin/order_clause.rb b/lib/active_admin/order_clause.rb
@@ -1,26 +1,48 @@
 module ActiveAdmin
   class OrderClause
-    attr_reader :field, :order
+    attr_reader :field, :order, :active_admin_config
 
-    def initialize(clause)
+    def initialize(active_admin_config, clause)
       clause =~ /^([\w\_\.]+)(->'\w+')?_(desc|asc)$/
       @column = $1
       @op = $2
       @order = $3
-
+      @active_admin_config = active_admin_config
       @field = [@column, @op].compact.join
     end
 
     def valid?
       @field.present? && @order.present?
     end
 
-    def to_sql(active_admin_config)
-      table = active_admin_config.resource_column_names.include?(@column) ? active_admin_config.resource_table_name : nil
-      table_column = (@column =~ /\./) ? @column :
-        [table, active_admin_config.resource_quoted_column_name(@column)].compact.join(".")
+    def apply(chain)
+      chain.reorder(sql)
+    end
 
+    def to_sql
       [table_column, @op, ' ', @order].compact.join
     end
+
+    def table
+      active_admin_config.resource_column_names.include?(@column) ? active_admin_config.resource_table_name : nil
+    end
+
+    def table_column
+      (@column =~ /\./) ? @column :
+        [table, active_admin_config.resource_quoted_column_name(@column)].compact.join(".")
+    end
+
+    def sql
+      custom_sql || to_sql
+    end
+
+    protected
+
+    def custom_sql
+      if active_admin_config.ordering[@column].present?
+        active_admin_config.ordering[@column].call(self)
+      end
+    end
+
   end
 end
diff --git a/lib/active_admin/resource.rb b/lib/active_admin/resource.rb
@@ -10,6 +10,7 @@
 require 'active_admin/resource/scope_to'
 require 'active_admin/resource/sidebars'
 require 'active_admin/resource/belongs_to'
+require 'active_admin/resource/ordering'
 
 module ActiveAdmin
 
@@ -50,6 +51,9 @@ def sort_order
     # Set breadcrumb builder
     attr_writer :breadcrumb
 
+    #Set order clause
+    attr_writer :order_clause
+
     # Store a reference to the DSL so that we can dereference it during garbage collection.
     attr_accessor :dsl
 
@@ -82,6 +86,7 @@ def initialize(namespace, resource_class, options = {})
     include ScopeTo
     include Sidebars
     include Routes
+    include Ordering
 
     # The class this resource wraps. If you register the Post model, Resource#resource_class
     # will point to the Post class
@@ -144,6 +149,10 @@ def breadcrumb
       instance_variable_defined?(:@breadcrumb) ? @breadcrumb : namespace.breadcrumb
     end
 
+    def order_clause
+      @order_clause || namespace.order_clause
+    end
+
     def find_resource(id)
       resource = resource_class.public_send *method_for_find(id)
       (decorator_class && resource) ? decorator_class.new(resource) : resource
diff --git a/lib/active_admin/resource/ordering.rb b/lib/active_admin/resource/ordering.rb
@@ -0,0 +1,11 @@
+module ActiveAdmin
+  class Resource
+    module Ordering
+
+      def ordering
+        @ordering ||= {}.with_indifferent_access
+      end
+
+    end
+  end
+end
diff --git a/lib/active_admin/resource_controller/data_access.rb b/lib/active_admin/resource_controller/data_access.rb
@@ -209,11 +209,10 @@ def apply_authorization_scope(collection)
 
       def apply_sorting(chain)
         params[:order] ||= active_admin_config.sort_order
-
-        order_clause = OrderClause.new params[:order]
+        order_clause = active_admin_config.order_clause.new(active_admin_config, params[:order])
 
         if order_clause.valid?
-          chain.reorder(order_clause.to_sql(active_admin_config))
+          order_clause.apply(chain)
         else
           chain # just return the chain
         end
diff --git a/lib/active_admin/resource_dsl.rb b/lib/active_admin/resource_dsl.rb
@@ -8,6 +8,25 @@ def initialize(config, resource_class)
 
     private
 
+    # Redefine sort behaviour for column
+    #
+    # For example:
+    #
+    #   # nulls last
+    #   order_by(:age) do |order_clause|
+    #     [order_clause.to_sql, 'NULLS LAST'].join(' ')  if order_clause.order == 'desc'
+    #   end
+    #
+    #   # by last_name but in the case that there is no last name, by first_name.
+    #   order_by(:full_name) do |order_clause|
+    #     ['COALESCE(NULLIF(last_name, ''), first_name), first_name', order_clause.order].join(' ')
+    #   end
+    #
+    #
+    def order_by(column, &block)
+      config.ordering[column] = block
+    end
+
     def belongs_to(target, options = {})
       config.belongs_to(target, options)
     end
diff --git a/lib/active_admin/views/components/table_for.rb b/lib/active_admin/views/components/table_for.rb
@@ -109,7 +109,7 @@ def build_table_cell(col, resource)
       #   current_sort[1] #=> asc | desc
       def current_sort
         @current_sort ||= begin
-          order_clause = OrderClause.new params[:order]
+          order_clause = active_admin_config.order_clause.new(active_admin_config, params[:order])
 
           if order_clause.valid?
             [order_clause.field, order_clause.order]
diff --git a/lib/generators/active_admin/install/templates/active_admin.rb.erb b/lib/generators/active_admin/install/templates/active_admin.rb.erb
@@ -276,4 +276,11 @@ ActiveAdmin.setup do |config|
   # override the content of the footer here.
   #
   # config.footer = 'my custom footer text'
+
+  # == Sorting
+  #
+  # By default ActiveAdmin::OrderClause is used for sorting logic
+  # You can inherit it with own class and inject it for all resources
+  #
+  # config.order_clause = MyOrderClause
 end
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
@@ -91,6 +91,10 @@
     expect(application.comments).to eq true
   end
 
+  it "should have default order clause class" do
+    expect(application.order_clause).to eq ActiveAdmin::OrderClause
+  end
+
   describe "authentication settings" do
 
     it "should have no default current_user_method" do
diff --git a/spec/unit/order_clause_spec.rb b/spec/unit/order_clause_spec.rb
@@ -1,11 +1,11 @@
 require 'rails_helper'
 
 describe ActiveAdmin::OrderClause do
-  subject { described_class.new clause }
+  subject { described_class.new(config, clause) }
 
   let(:application) { ActiveAdmin::Application.new }
-  let(:namespace)   { ActiveAdmin::Namespace.new application, :admin }
-  let(:config)      { ActiveAdmin::Resource.new namespace, Post }
+  let(:namespace) { ActiveAdmin::Namespace.new application, :admin }
+  let(:config) { ActiveAdmin::Resource.new namespace, Post }
 
   describe 'id_asc (existing column)' do
     let(:clause) { 'id_asc' }
@@ -23,7 +23,7 @@
     end
 
     specify '#to_sql prepends table name' do
-      expect(subject.to_sql(config)).to eq '"posts"."id" asc'
+      expect(subject.to_sql).to eq '"posts"."id" asc'
     end
   end
 
@@ -43,7 +43,7 @@
     end
 
     specify '#to_sql' do
-      expect(subject.to_sql(config)).to eq '"virtual_column" asc'
+      expect(subject.to_sql).to eq '"virtual_column" asc'
     end
   end
 
@@ -63,7 +63,7 @@
     end
 
     it 'converts to sql' do
-      expect(subject.to_sql(config)).to eq %Q("hstore_col"->'field' desc)
+      expect(subject.to_sql).to eq %Q("hstore_col"->'field' desc)
     end
   end
 
diff --git a/spec/unit/resource/ordering_spec.rb b/spec/unit/resource/ordering_spec.rb
@@ -0,0 +1,38 @@
+require 'rails_helper'
+
+module ActiveAdmin
+  describe Resource, "Ordering" do
+    describe "#order_by" do
+
+      let(:application) { ActiveAdmin::Application.new }
+      let(:namespace) { ActiveAdmin::Namespace.new application, :admin }
+      let(:resource_config) { ActiveAdmin::Resource.new namespace, Post }
+      let(:dsl){ ActiveAdmin::ResourceDSL.new(resource_config, Post) }
+
+      it "should register the ordering in the config" do
+        dsl.run_registration_block do
+          order_by(:age) do |order_clause|
+            if order_clause.order == 'desc'
+              [order_clause.to_sql, 'NULLS LAST'].join(' ')
+            end
+          end
+        end
+        expect(resource_config.ordering.size).to eq(1)
+      end
+
+
+      it "should allow to setup custom ordering class" do
+        MyOrderClause = Class.new(ActiveAdmin::OrderClause)
+        dsl.run_registration_block do
+          config.order_clause = MyOrderClause
+        end
+        expect(resource_config.order_clause).to eq(MyOrderClause)
+        expect(application.order_clause).to eq(ActiveAdmin::OrderClause)
+
+      end
+
+    end
+  end
+end
+
+
diff --git a/spec/unit/resource_controller/data_access_spec.rb b/spec/unit/resource_controller/data_access_spec.rb
@@ -60,6 +60,35 @@
       end
     end
 
+    context "custom strategy" do
+      before do
+        expect(controller.send(:active_admin_config)).to receive(:ordering).twice.and_return(
+          {
+            published_date: proc do |order_clause|
+              [order_clause.to_sql, 'NULLS LAST'].join(' ') if order_clause.order == 'desc'
+            end
+          }.with_indifferent_access
+        )
+      end
+
+      context "when params applicable" do
+        let(:params) {{ order: "published_date_desc" }}
+        it "reorders chain" do
+          chain = double "ChainObj"
+          expect(chain).to receive(:reorder).with('"posts"."published_date" desc NULLS LAST').once.and_return(Post.search)
+          controller.send :apply_sorting, chain
+        end
+      end
+      context "when params not applicable" do
+        let(:params) {{ order: "published_date_asc" }}
+        it "reorders chain" do
+          chain = double "ChainObj"
+          expect(chain).to receive(:reorder).with('"posts"."published_date" asc').once.and_return(Post.search)
+          controller.send :apply_sorting, chain
+        end
+      end
+    end
+
   end
 
   describe "scoping" do
