Skip to content

Commit 0c7bc61

Browse files
committed
Remove Sections::List#sort_reverse because of having very limited use case
Closes #1181
1 parent 0c9f8b3 commit 0c7bc61

4 files changed

Lines changed: 19 additions & 29 deletions

File tree

app/controllers/rails_admin/main_controller.rb

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,14 @@ def back_or_index
6666

6767
def get_sort_hash(model_config)
6868
abstract_model = model_config.abstract_model
69-
params[:sort] = params[:sort_reverse] = nil unless model_config.list.fields.collect { |f| f.name.to_s }.include? params[:sort]
70-
params[:sort] ||= model_config.list.sort_by.to_s
71-
params[:sort_reverse] ||= 'false'
72-
7369
field = model_config.list.fields.detect { |f| f.name.to_s == params[:sort] }
70+
7471
column =
75-
if field.nil? || field.sortable == true # use params[:sort] on the base table
76-
"#{abstract_model.table_name}.#{params[:sort]}"
77-
elsif field.sortable == false # use default sort, asked field is not sortable
72+
if field.nil? || field.sortable == false # use default sort, asked field does not exist or is not sortable
73+
field = model_config.list.possible_fields.detect { |f| f.name == model_config.list.sort_by.to_sym }
7874
"#{abstract_model.table_name}.#{model_config.list.sort_by}"
75+
elsif field.sortable == true # use the given field
76+
"#{abstract_model.table_name}.#{field.name}"
7977
elsif (field.sortable.is_a?(String) || field.sortable.is_a?(Symbol)) && field.sortable.to_s.include?('.') # just provide sortable, don't do anything smart
8078
field.sortable
8179
elsif field.sortable.is_a?(Hash) # just join sortable hash, don't do anything smart
@@ -86,8 +84,8 @@ def get_sort_hash(model_config)
8684
"#{abstract_model.table_name}.#{field.sortable}"
8785
end
8886

89-
reversed_sort = (field ? field.sort_reverse? : model_config.list.sort_reverse?)
90-
{sort: column, sort_reverse: (params[:sort_reverse] == reversed_sort.to_s)}
87+
params[:sort_reverse] ||= 'false'
88+
{sort: column, sort_reverse: (params[:sort_reverse] == (field&.sort_reverse&.to_s || 'true'))}
9189
end
9290

9391
def redirect_to_on_success

lib/rails_admin/config/has_fields.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,10 @@ def visible_fields
116116
all_fields.collect { |f| f.with(bindings) }.select(&:visible?).sort_by { |f| [f.order, i += 1] } # stable sort, damn
117117
end
118118

119+
def possible_fields
120+
_fields(true)
121+
end
122+
119123
protected
120124

121125
# Raw fields.

lib/rails_admin/config/sections/list.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@ class List < RailsAdmin::Config::Sections::Base
3232
parent.abstract_model.primary_key
3333
end
3434

35-
register_instance_option :sort_reverse? do
36-
true # By default show latest first
37-
end
38-
3935
register_instance_option :scopes do
4036
[]
4137
end
@@ -51,6 +47,10 @@ class List < RailsAdmin::Config::Sections::Base
5147
def fields_for_table
5248
visible_fields.partition(&:sticky?).flatten
5349
end
50+
51+
register_deprecated_instance_option :sort_reverse do
52+
ActiveSupport::Deprecation.warn('The sort_reverse configuration option is deprecated and has no effect.')
53+
end
5454
end
5555
end
5656
end

spec/integration/actions/index_spec.rb

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -795,20 +795,6 @@ def visit_page(page)
795795

796796
before { @players = players.collect { |h| Player.create(h) } }
797797

798-
it 'is configurable per model' do
799-
RailsAdmin.config Player do
800-
list do
801-
sort_by :created_at
802-
sort_reverse true
803-
field :name
804-
end
805-
end
806-
visit index_path(model_name: 'player')
807-
player_names_by_date.reverse.each_with_index do |name, i|
808-
expect(find("tbody tr:nth-child(#{i + 1})")).to have_content(name)
809-
end
810-
end
811-
812798
it 'has reverse direction by default' do
813799
RailsAdmin.config Player do
814800
list do
@@ -822,11 +808,13 @@ def visit_page(page)
822808
end
823809
end
824810

825-
it 'allows change default direction' do
811+
it 'allows change direction by using field configuration' do
826812
RailsAdmin.config Player do
827813
list do
828814
sort_by :created_at
829-
sort_reverse false
815+
configure :created_at do
816+
sort_reverse false
817+
end
830818
field :name
831819
end
832820
end

0 commit comments

Comments
 (0)