From b82eca689e7374cdafbb3435c4168579850649ac Mon Sep 17 00:00:00 2001 From: Phil Hodgson Date: Mon, 6 Oct 2014 17:56:06 -0400 Subject: [PATCH] 3618: allow for orders to be parameterized for showing records; proof of concept here for project tabs, sorting Data Collections by name --- .../app/controllers/application_controller.rb | 17 ++++++++++++++++- apps/workbench/app/models/arvados_api_client.rb | 2 +- .../projects/_show_data_collections.html.erb | 3 ++- .../views/projects/_show_tab_contents.html.erb | 3 ++- .../controllers/arvados/v1/groups_controller.rb | 14 +++++++++++++- services/api/lib/load_param.rb | 4 +++- 6 files changed, 37 insertions(+), 6 deletions(-) diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb index 4f5d8fdcd2..a027e2ea18 100644 --- a/apps/workbench/app/controllers/application_controller.rb +++ b/apps/workbench/app/controllers/application_controller.rb @@ -105,8 +105,23 @@ class ApplicationController < ActionController::Base end end + # params[:order]: + # + # The order can be unspecified, or it can be a column name. + # Column names should always be qualified by a table name and a direction is optional, defaulting to asc + # (e.g. "collections.name" or "collections.name desc"). + # If a column name is specified, that table will be sorted by that column. + # If there are objects from different models that will be shown (such as in Jobs and Pipelines tab), + # then a sort column name can optionally be specified for each model, passed as an array (e.g. "['jobs.description', 'pipeline_instances.name']") + # Currently only one sort column name and direction can be specified for each model. def load_filters_and_paging_params - @order = params[:order] || 'created_at desc' + if params[:order].blank? + @order = 'created_at desc' + elsif params[:order].starts_with? '[' + @order = Oj.load params[:order] + else + @order = params[:order] + end @order = [@order] unless @order.is_a? Array @limit ||= 200 diff --git a/apps/workbench/app/models/arvados_api_client.rb b/apps/workbench/app/models/arvados_api_client.rb index 78d3beef3c..7076799f83 100644 --- a/apps/workbench/app/models/arvados_api_client.rb +++ b/apps/workbench/app/models/arvados_api_client.rb @@ -124,7 +124,7 @@ class ArvadosApiClient header = {"Accept" => "application/json"} - profile_checkpoint { "Prepare request #{url} #{query[:uuid]} #{query[:where]} #{query[:filters]}" } + profile_checkpoint { "Prepare request #{url} #{query[:uuid]} #{query[:where]} #{query[:filters]} #{query[:order]}" } msg = @client_mtx.synchronize do begin @api_client.post(url, query, header: header) diff --git a/apps/workbench/app/views/projects/_show_data_collections.html.erb b/apps/workbench/app/views/projects/_show_data_collections.html.erb index e56321dde8..a2df49ede2 100644 --- a/apps/workbench/app/views/projects/_show_data_collections.html.erb +++ b/apps/workbench/app/views/projects/_show_data_collections.html.erb @@ -1,3 +1,4 @@ <%= render_pane 'tab_contents', to_string: true, locals: { - filters: [['uuid', 'is_a', "arvados#collection"]] + filters: [['uuid', 'is_a', "arvados#collection"]], + order: 'collections.name' }.merge(local_assigns) %> diff --git a/apps/workbench/app/views/projects/_show_tab_contents.html.erb b/apps/workbench/app/views/projects/_show_tab_contents.html.erb index 0f9901aa0a..52fd0a3db0 100644 --- a/apps/workbench/app/views/projects/_show_tab_contents.html.erb +++ b/apps/workbench/app/views/projects/_show_tab_contents.html.erb @@ -1,3 +1,4 @@ +<% order = nil if local_assigns[:order].nil? %>
@@ -75,7 +76,7 @@ - + diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb index 9fca207dd2..be873fb7c9 100644 --- a/services/api/app/controllers/arvados/v1/groups_controller.rb +++ b/services/api/app/controllers/arvados/v1/groups_controller.rb @@ -71,6 +71,10 @@ class Arvados::V1::GroupsController < ApplicationController # aggregate set. limit_all = @limit offset_all = @offset + # save the orders from the current request as determined by load_param, + # but otherwise discard them because we're going to be getting objects + # from many models + request_orders = @orders.clone @orders = [] [Group, @@ -92,7 +96,15 @@ class Arvados::V1::GroupsController < ApplicationController end end - @objects = @objects.order("#{klass.table_name}.uuid") + # If the currently requested orders specifically match the table_name for the current klass, apply the order + request_order = request_orders && request_orders.find{ |r| r =~ /^#{klass.table_name}\./i } + if request_order + @objects = @objects.order(request_order) + else + # default to UUID, ignoring any currently requested ordering because it doesn't apply to this klass + @objects = @objects.order("#{klass.table_name}.uuid") + end + @limit = limit_all - all_objects.count apply_where_limit_order_params klass klass_items_available = @objects. diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb index 71678cd223..fb580f237c 100644 --- a/services/api/lib/load_param.rb +++ b/services/api/lib/load_param.rb @@ -70,7 +70,7 @@ module LoadParam end @orders = [] - if params[:order] + if (params[:order].is_a?(Array) && !params[:order].empty?) || !params[:order].blank? od = [] (case params[:order] when String @@ -93,6 +93,8 @@ module LoadParam model_class.columns.collect(&:name).index(attr) and ['asc','desc'].index direction.downcase @orders << "#{table_name}.#{attr} #{direction.downcase}" + elsif attr.match /^([a-z][_a-z0-9]+)\.([a-z][_a-z0-9]+)$/ + @orders << "#{attr} #{direction.downcase}" end end end -- 2.30.2