diff --git a/app/controllers/administrate/application_controller.rb b/app/controllers/administrate/application_controller.rb index 63f486d5ae..2322137f66 100644 --- a/app/controllers/administrate/application_controller.rb +++ b/app/controllers/administrate/application_controller.rb @@ -141,16 +141,18 @@ def order @order ||= Administrate::Order.new( sorting_attribute, sorting_direction, - association_attribute: order_by_field( + sorting_column: sorting_column( dashboard_attribute(sorting_attribute) ) ) end - def order_by_field(dashboard) - return unless dashboard.try(:options) + def sorting_column(dashboard_attribute) + return unless dashboard_attribute.try(:options) - dashboard.options.fetch(:order, nil) + dashboard_attribute.options.fetch(:sorting_column) { + dashboard_attribute.options.fetch(:order, nil) + } end def dashboard_attribute(attribute) diff --git a/app/views/administrate/application/_collection.html.erb b/app/views/administrate/application/_collection.html.erb index 6a61ccc953..e49c1dcc20 100644 --- a/app/views/administrate/application/_collection.html.erb +++ b/app/views/administrate/application/_collection.html.erb @@ -23,25 +23,35 @@ to display a collection of resources in an HTML table. <% collection_presenter.attribute_types.each do |attr_name, attr_type| %> " - scope="col" - aria-sort="<%= sort_order(collection_presenter.ordered_html_class(attr_name)) %>"> - <%= link_to(sanitized_order_params(page, collection_field_name).merge( - collection_presenter.order_params_for(attr_name, key: collection_field_name) - )) do %> - <%= t( - "helpers.label.#{collection_presenter.resource_name}.#{attr_name}", - default: resource_class.human_attribute_name(attr_name).titleize, - ) %> - <% if collection_presenter.ordered_by?(attr_name) %> - - - + cell-label--<%= attr_type.html_class %> + cell-label--<%= collection_presenter.ordered_html_class(attr_name) %> + cell-label--<%= "#{collection_presenter.resource_name}_#{attr_name}" %>" + scope="col" + <% if attr_type.sortable? %> + aria-sort="<%= sort_order(collection_presenter.ordered_html_class(attr_name)) %>" + <% end %> + > + <% if attr_type.sortable? %> + <%= link_to(sanitized_order_params(page, collection_field_name).merge( + collection_presenter.order_params_for(attr_name, key: collection_field_name) + )) do %> + <%= t( + "helpers.label.#{collection_presenter.resource_name}.#{attr_name}", + default: resource_class.human_attribute_name(attr_name).titleize, + ) %> + <% if collection_presenter.ordered_by?(attr_name) %> + + + + <% end %> <% end %> + <% else %> + <%= t( + "helpers.label.#{collection_presenter.resource_name}.#{attr_name}", + default: resource_class.human_attribute_name(attr_name).titleize, + ) %> <% end %> <% end %> diff --git a/lib/administrate/field/base.rb b/lib/administrate/field/base.rb index 65121587c9..e3eebf0a41 100644 --- a/lib/administrate/field/base.rb +++ b/lib/administrate/field/base.rb @@ -24,6 +24,10 @@ def self.searchable? false end + def self.sortable? + true + end + def self.field_type to_s.split("::").last.underscore end diff --git a/lib/administrate/field/deferred.rb b/lib/administrate/field/deferred.rb index ae3a79e851..58db84d053 100644 --- a/lib/administrate/field/deferred.rb +++ b/lib/administrate/field/deferred.rb @@ -56,6 +56,14 @@ def searchable_fields end end + def sortable? + options.fetch(:sortable, deferred_class.sortable?) + end + + def sortable_field + options.fetch(:sortable_field, nil) + end + def permitted_attribute(attr, opts = {}) if options.key?(:foreign_key) Administrate.warn_of_deprecated_option(:foreign_key) diff --git a/lib/administrate/field/has_many.rb b/lib/administrate/field/has_many.rb index 8e14759cb9..806eaae46d 100644 --- a/lib/administrate/field/has_many.rb +++ b/lib/administrate/field/has_many.rb @@ -82,12 +82,21 @@ def data def order_from_params(params) Administrate::Order.new( params.fetch(:order, sort_by), - params.fetch(:direction, direction) + params.fetch(:direction, direction), + sorting_column: sorting_column( + associated_dashboard_attribute(params.fetch(:order, sort_by)) + ) ) end def order - @order ||= Administrate::Order.new(sort_by, direction) + @order ||= Administrate::Order.new( + sort_by, + direction, + sorting_column: sorting_column( + associated_dashboard_attribute(sort_by) + ) + ) end private @@ -109,6 +118,16 @@ def display_candidate_resource(resource) associated_dashboard.display_resource(resource) end + def sorting_column(dashboard_attribute) + return unless dashboard_attribute.try(:options) + + dashboard_attribute.options.fetch(:sorting_column, nil) + end + + def associated_dashboard_attribute(attribute) + associated_dashboard.attribute_types[attribute.to_sym] if attribute + end + def sort_by options[:sort_by] end diff --git a/lib/administrate/field/password.rb b/lib/administrate/field/password.rb index 21fc0be5aa..4355d3f77f 100644 --- a/lib/administrate/field/password.rb +++ b/lib/administrate/field/password.rb @@ -7,6 +7,10 @@ def self.searchable? false end + def self.sortable? + false + end + def truncate data.to_s.gsub(/./, character)[0...truncation_length] end diff --git a/lib/administrate/order.rb b/lib/administrate/order.rb index 7800976c3e..bea06b7805 100644 --- a/lib/administrate/order.rb +++ b/lib/administrate/order.rb @@ -1,19 +1,20 @@ module Administrate class Order - def initialize(attribute = nil, direction = nil, association_attribute: nil) + def initialize(attribute = nil, direction = nil, association_attribute: nil, sorting_column: nil) @attribute = attribute @direction = sanitize_direction(direction) - @association_attribute = association_attribute + # @association_attribute = association_attribute + @sorting_column = sorting_column || attribute end def apply(relation) return order_by_association(relation) unless reflect_association(relation).nil? - order = relation.arel_table[attribute].public_send(direction) + order = relation.arel_table[sorting_column].public_send(direction) return relation.reorder(order) if - column_exist?(relation, attribute) + column_exist?(relation, sorting_column) relation end @@ -33,7 +34,7 @@ def order_params_for(attr) private - attr_reader :attribute, :association_attribute + attr_reader :attribute, :association_attribute, :sorting_column def sanitize_direction(direction) %w[asc desc].include?(direction.to_s) ? direction.to_sym : :asc @@ -94,9 +95,9 @@ def order_by_id(relation) end def ordering_by_association_column?(relation) - association_attribute && + (attribute != sorting_column) && column_exist?( - reflect_association(relation).klass, association_attribute.to_sym + reflect_association(relation).klass, sorting_column.to_sym ) end @@ -113,7 +114,7 @@ def order_by_association_id(relation) end def order_by_association_attribute(relation) - order_by_association_column(relation, association_attribute) + order_by_association_column(relation, sorting_column) end def order_by_association_column(relation, column_name) diff --git a/spec/dashboards/customer_dashboard_spec.rb b/spec/dashboards/customer_dashboard_spec.rb index 79cedf2a39..26db8becea 100644 --- a/spec/dashboards/customer_dashboard_spec.rb +++ b/spec/dashboards/customer_dashboard_spec.rb @@ -20,7 +20,7 @@ expect(fields[:name]).to eq(Field::String) expect(fields[:email]).to eq(Field::Email) expect(fields[:lifetime_value]) - .to eq(Field::Number.with_options(prefix: "$", decimals: 2)) + .to eq(Field::Number.with_options(prefix: "$", decimals: 2, sortable: false)) end end diff --git a/spec/example_app/app/dashboards/customer_dashboard.rb b/spec/example_app/app/dashboards/customer_dashboard.rb index 79786fa4f7..f52506b9d6 100644 --- a/spec/example_app/app/dashboards/customer_dashboard.rb +++ b/spec/example_app/app/dashboards/customer_dashboard.rb @@ -6,7 +6,7 @@ class CustomerDashboard < Administrate::BaseDashboard created_at: Field::DateTime, email: Field::Email, email_subscriber: Field::Boolean, - lifetime_value: Field::Number.with_options(prefix: "$", decimals: 2), + lifetime_value: Field::Number.with_options(prefix: "$", decimals: 2, sortable: false), name: Field::String, orders: Field::HasMany.with_options(limit: 2, sort_by: :id), log_entries: Field::HasManyVariant.with_options(limit: 2, sort_by: :id), diff --git a/spec/example_app/app/dashboards/line_item_dashboard.rb b/spec/example_app/app/dashboards/line_item_dashboard.rb index 4a7c29402c..78f8e325c6 100644 --- a/spec/example_app/app/dashboards/line_item_dashboard.rb +++ b/spec/example_app/app/dashboards/line_item_dashboard.rb @@ -12,7 +12,7 @@ class LineItemDashboard < Administrate::BaseDashboard created_at: Field::DateTime, updated_at: Field::DateTime, order: Field::BelongsTo, - product: Field::BelongsTo, + product: Field::BelongsTo.with_options(sorting_column: :name), quantity: Field::Number, total_price: Field::Number.with_options(prefix: "$", decimals: 2), unit_price: Field::Number.with_options(prefix: "$", decimals: 2) diff --git a/spec/example_app/app/dashboards/order_dashboard.rb b/spec/example_app/app/dashboards/order_dashboard.rb index 631572e217..bc028096f9 100644 --- a/spec/example_app/app/dashboards/order_dashboard.rb +++ b/spec/example_app/app/dashboards/order_dashboard.rb @@ -20,13 +20,14 @@ class OrderDashboard < Administrate::BaseDashboard r.address_state, r.address_zip ].compact.join("\n") - } + }, + sorting_column: :address_zip ), customer: Field::BelongsTo.with_options(order: "name"), line_items: Field::HasMany.with_options( collection_attributes: %i[product quantity unit_price total_price] ), - total_price: Field::Number.with_options(prefix: "$", decimals: 2), + total_price: Field::Number.with_options(prefix: "$", decimals: 2, sortable: false), shipped_at: Field::DateTime, payments: Field::HasMany } diff --git a/spec/features/sort_spec.rb b/spec/features/sort_spec.rb new file mode 100644 index 0000000000..cff660db48 --- /dev/null +++ b/spec/features/sort_spec.rb @@ -0,0 +1,83 @@ +require "rails_helper" + +feature "Sort" do + describe "sortable" do + context "when the field is sortable" do + it "is a link to sort by that field" do + visit admin_customers_path + expect(page).to have_link("Name") + end + end + + context "when the field is NOT sortable" do + it "is not a link" do + visit admin_customers_path + expect(page).not_to have_link("Password") + end + end + end + + scenario "admin sorts customers by name" do + dan = create(:customer, name: "Dan Croak") + bob = create(:customer, name: "Bob") + alice = create(:customer, name: "Alice") + + visit admin_customers_path + click_on "Name" + + expect(page).to have_css("table tr:nth-child(1)", text: alice.name) + expect(page).to have_css("table tr:nth-child(2)", text: bob.name) + expect(page).to have_css("table tr:nth-child(3)", text: dan.name) + end + + scenario "admin sorts orders by full_address (virtual field)" do + create(:order, address_zip: "651", address_state: "UT") + create(:order, address_zip: "7", address_state: "WV") + create(:order, address_zip: "59543-0366", address_state: "NJ") + + visit admin_orders_path + click_on "Full Address" + + expect(page).to have_css("table tr:nth-child(1)", text: "NJ 59543-0366") + expect(page).to have_css("table tr:nth-child(2)", text: "UT 651") + expect(page).to have_css("table tr:nth-child(3)", text: "WV 7") + end + + scenario "admin sorts customer's orders by full_address (virtual field)" do + customer = create(:customer, name: "Alice") + customer.orders << create(:order, address_zip: "651", address_state: "UT") + customer.orders << create(:order, address_zip: "7", address_state: "WV") + customer.orders << create(:order, address_zip: "59543-0366", address_state: "NJ") + + visit admin_customer_path(customer) + click_on "Full Address" + + within(table_for_attribute(:orders)) do + expect(page).to have_css("tr:nth-child(1)", text: "NJ 59543-0366") + end + end + + scenario "admin sorts order's list_items by product name" do + product_1 = create(:product, name: "Monopoly 2") + product_2 = create(:product, name: "Monopoly 3") + product_3 = create(:product, name: "Monopoly 1") + customer = create(:customer) + order = create(:order, customer: customer) + order.line_items << create(:line_item, product: product_1) + order.line_items << create(:line_item, product: product_2) + order.line_items << create(:line_item, product: product_3) + + visit admin_order_path(order) + click_on "Product" + + within(table_for_attribute(:line_items)) do + expect(page).to have_css("tr:nth-child(1)", text: "Monopoly 1") + expect(page).to have_css("tr:nth-child(2)", text: "Monopoly 2") + expect(page).to have_css("tr:nth-child(3)", text: "Monopoly 3") + end + end + + def table_for_attribute(attr_name) + find("table[aria-labelledby=#{attr_name}]") + end +end diff --git a/spec/lib/administrate/order_spec.rb b/spec/lib/administrate/order_spec.rb index b7143bb282..15949a5d75 100644 --- a/spec/lib/administrate/order_spec.rb +++ b/spec/lib/administrate/order_spec.rb @@ -118,7 +118,7 @@ order = Administrate::Order.new( :user, nil, - association_attribute: "name" + sorting_column: "name" ) relation = relation_with_association( :belongs_to, @@ -144,7 +144,7 @@ order = Administrate::Order.new( :user, nil, - association_attribute: "invalid_column_name" + sorting_column: "invalid_column_name" ) relation = relation_with_association( :belongs_to, @@ -192,7 +192,7 @@ order = Administrate::Order.new( :user, nil, - association_attribute: "name" + sorting_column: "name" ) relation = relation_with_association( :has_one, @@ -218,7 +218,7 @@ order = Administrate::Order.new( :user, nil, - association_attribute: "invalid_column_name" + sorting_column: "invalid_column_name" ) relation = relation_with_association( :has_one, diff --git a/spec/lib/fields/deferred_spec.rb b/spec/lib/fields/deferred_spec.rb index c8b5514330..31751244bb 100644 --- a/spec/lib/fields/deferred_spec.rb +++ b/spec/lib/fields/deferred_spec.rb @@ -98,6 +98,38 @@ end end + describe "#sortable?" do + context "when given a `sortable` option" do + it "returns the value given" do + sortable_deferred = Administrate::Field::Deferred.new( + double(sortable?: false), + sortable: true + ) + unsortable_deferred = Administrate::Field::Deferred.new( + double(sortable?: true), + sortable: false + ) + + expect(sortable_deferred.sortable?).to eq(true) + expect(unsortable_deferred.sortable?).to eq(false) + end + end + + context "when not given a `sortable` option" do + it "falls back to the default of the deferred class" do + sortable_deferred = Administrate::Field::Deferred.new( + double(sortable?: true) + ) + unsortable_deferred = Administrate::Field::Deferred.new( + double(sortable?: false) + ) + + expect(sortable_deferred.sortable?).to eq(true) + expect(unsortable_deferred.sortable?).to eq(false) + end + end + end + describe "#==" do it "returns false for different deferred classes" do one = Administrate::Field::Deferred.new(String)