From 5c82e4c0b44b7fe5cb71a3cacb56ff1a3ca1a619 Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Wed, 8 Aug 2012 13:50:33 -0400 Subject: [PATCH] Simplified Hold Pull List: Fix several sorting bugs First of all, sorting on most columns was broken due to a bug in the way that the flattener methods of the open-ils.fielder service were constructing their SQL JOINs. We were coming up with way too many joins, and then losing track of which JOIN's alias to refer to when building the ORDER BY clause later. This is fixed. Secondly, the shelving location column now sorts automatically by the shelving location *ordering* values, when avaiable. These are the values that you set up in the drag-and-drop staff client interface titled "Copy Location Order." When these values are not set for the org unit whose pull list you're viewing, the sorting will fall back to alphabetical. Signed-off-by: Lebbeous Fogle-Weekley Signed-off-by: Mike Rylander --- Open-ILS/examples/fm_IDL.xml | 6 +++- .../lib/OpenILS/Application/Fielder.pm | 3 ++ .../lib/OpenILS/Application/Flattener.pm | 31 +++++++++++++++++-- .../src/templates/circ/hold_pull_list.tt2 | 4 +-- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/Open-ILS/examples/fm_IDL.xml b/Open-ILS/examples/fm_IDL.xml index eb75532675..739bf837a5 100644 --- a/Open-ILS/examples/fm_IDL.xml +++ b/Open-ILS/examples/fm_IDL.xml @@ -5031,7 +5031,7 @@ SELECT usr, NOW())) LEFT JOIN config.standing_penalty csp diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Fielder.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Fielder.pm index 343c285aff..610a86b8c6 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Fielder.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Fielder.pm @@ -24,6 +24,9 @@ use XML::LibXML::XPathContext; use XML::LibXSLT; use OpenILS::Application::Flattener; +use Data::Dumper; + +$Data::Dumper::Indent = 0; our %namespace_map = ( oils_persist=> {ns => 'http://open-ils.org/spec/opensrf/IDL/persistence/v1'}, diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Flattener.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Flattener.pm index ff232a9469..8bc8eda3cc 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Flattener.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Flattener.pm @@ -13,6 +13,10 @@ use OpenSRF::Utils::Logger qw/:logger/; use OpenILS::Utils::CStoreEditor q/:funcs/; use OpenSRF::Utils::JSON; +use Data::Dumper; + +$Data::Dumper::Indent = 0; + sub _fm_link_from_class { my ($class, $field) = @_; @@ -255,6 +259,11 @@ sub process_map { join => {} }; + # Here's a hash where we'll keep track of whether we've already provided + # a join to cover a given hash. It seems that without this we build + # redundant joins. + my $join_coverage = {}; + foreach my $k (keys %$map) { my $column = $map->{$k} = _flattened_search_normalize_map_column($map->{$k}); @@ -269,8 +278,23 @@ sub process_map { # For filter or sort columns, we'll need joining. if ($column->{filter} or $column->{sort}) { - my ($clause, $last_join_alias) = - _flattened_search_single_join_clause($k,$hint,$column->{path}); + my @path = @{ $column->{path} }; + pop @path; # discard last part (field) + my $joinkey = join(",", @path); + + my ($clause, $last_join_alias); + + # Skip joins that are already covered. We shouldn't need more than + # one join for the same path + if ($join_coverage->{$joinkey}) { + ($clause, $last_join_alias) = @{ $join_coverage->{$joinkey} }; + } else { + ($clause, $last_join_alias) = + _flattened_search_single_join_clause( + $k, $hint, $column->{path} + ); + $join_coverage->{$joinkey} = [$clause, $last_join_alias]; + } $map->{$k}{last_join_alias} = $last_join_alias; _flattened_search_merge_join_clause($jffolo->{join}, $clause); @@ -334,6 +358,7 @@ sub finish_jffolo { if ($map->{$key}) { my $class = $map->{$key}{last_join_alias} || $core_hint; + push @{ $jffolo->{order_by} }, { class => $class, field => $map->{$key}{path}[-1], @@ -355,7 +380,7 @@ sub process_result { if (not ref $fmobj) { throw OpenSRF::EX::ERROR( - "process_result() was passed an inappropriate second argument" + "process_result() was passed an inappropriate second argument ($fmobj)" ); } diff --git a/Open-ILS/src/templates/circ/hold_pull_list.tt2 b/Open-ILS/src/templates/circ/hold_pull_list.tt2 index fc497b9225..8355ffe2e7 100644 --- a/Open-ILS/src/templates/circ/hold_pull_list.tt2 +++ b/Open-ILS/src/templates/circ/hold_pull_list.tt2 @@ -71,9 +71,9 @@ editStyle="pane" showLoadFilter="true" fmClass="'ahopl'" - defaultSort="['copy_location_sort_order','call_number_sort_key']" + defaultSort="['copy_location_order_position','call_number_sort_key']" mapExtras="map_extras" - sortFieldReMap="{call_number_label: 'call_number_sort_key'}" + sortFieldReMap="{call_number_label:'call_number_sort_key',shelving_loc:'copy_location_order_position'}" fetchLock="true" query="{}"> -- 2.43.2