From 17302c5fe8dac1b9e1a3ee611e920920145b06a0 Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Tue, 18 Feb 2020 15:06:09 -0500 Subject: [PATCH] Sort by remote selector column when sort-on-link is requested and selector exists Signed-off-by: Mike Rylander Signed-off-by: Galen Charlton --- .../perlmods/lib/OpenILS/Application/Acq/Search.pm | 153 ++++++++++++++------- 1 file changed, 102 insertions(+), 51 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm index 2954c482cf..fde084c88d 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm @@ -219,6 +219,14 @@ sub get_fm_links_by_hint { undef; } +sub get_fm_selector_by_hint { + my ($hint) = @_; + foreach my $field (values %{$Fieldmapper::fieldmap}) { + return $field->{selector} if $field->{hint} eq $hint; + } + undef; +} + sub gen_au_term { my ($value, $n) = @_; my $lc_value = { @@ -478,7 +486,23 @@ sub add_au_joins { $n; } -sub add_acqpro_joins { +sub confirm_join_for_orderby { + my $graft_map = shift; + my $foreign_class = shift; + my $fkey_field = shift; + + for my $key (keys %$graft_map) { + my $clause = $$graft_map{$key}; + return $key if ($$clause{class} eq $foreign_class and $$clause{fkey} eq $fkey_field); + my $there = confirm_join_for_orderby($$clause{join}, $foreign_class, $fkey_field) if (exists($$clause{join})); + return $there if $there; + } + + return undef; +} + +sub add_generic_id_joins { + my $specific = shift; my $graft_map = shift; my $core_hint = shift; @@ -487,23 +511,32 @@ sub add_acqpro_joins { my ($hint, $attr, $num) = @$join; my $start = $graft_map->{$hint}; my $clause = { - "class" => "acqpro", + "class" => $specific, "type" => "left", "field" => "id", "fkey" => $attr, }; if ($hint eq $core_hint) { - $start->{"acqpro$num"} = $clause; + $start->{"$specific$num"} = $clause; } else { $start->{"join"} ||= {}; - $start->{"join"}->{"acqpro$num"} = $clause; + $start->{"join"}->{"$specific$num"} = $clause; } $n++; } $n; } +sub add_acqpo_joins { return add_generic_id_joins( acqpo => @_ ) } +sub add_acqpoi_joins { return add_generic_id_joins( acqpoi => @_ ) } +sub add_acqcr_joins { return add_generic_id_joins( acqcr => @_ ) } +sub add_acqclp_joins { return add_generic_id_joins( acqclp => @_ ) } +sub add_acqpl_joins { return add_generic_id_joins( acqpl => @_ ) } +sub add_acqpro_joins { return add_generic_id_joins( acqpro => @_ ) } +sub add_acqpostlbl_joins { return add_generic_id_joins( acqpostlbl => @_ ) } +sub add_jubstlbl_joins { return add_generic_id_joins( jubstlbl => @_ ) } + sub build_from_clause_and_joins { my ($query, $core, $and_terms, $or_terms) = @_; @@ -625,6 +658,43 @@ sub unified_search { $hint => [{"column" => "id", "transform" => "distinct"}] }; + my $query = {}; + + $query->{"limit"} = $options->{"limit"} if $options->{"limit"}; + + my $graft_map = build_from_clause_and_joins( + $query, $hint, $and_terms, $or_terms + ); + + $and_terms = prepare_terms($and_terms, 1); + $or_terms = prepare_terms($or_terms, 0); + + unless ($options->{au_by_id}) { + my $offset = add_au_joins($graft_map, $hint, prepare_au_terms($and_terms)); + add_au_joins($graft_map, $hint, prepare_au_terms($or_terms, $offset)); + } + + my $offset = add_acqpro_joins($graft_map, $hint, prepare_acqpro_terms($and_terms)); + add_acqpro_joins($graft_map, $hint, prepare_acqpro_terms($or_terms, $offset)); + + # The join to acqmapinv needs to be a left join when present. + if ($query->{from}{$hint}{acqmapinv}) { + $query->{from}{$hint}{acqmapinv}{type} = "left"; + } + + if ($and_terms and $or_terms) { + $query->{"where"} = { + "-" . (lc $conj eq "or" ? "or" : "and") => [$and_terms, $or_terms] + }; + } elsif ($and_terms) { + $query->{"where"} = $and_terms; + } elsif ($or_terms) { + $query->{"where"} = $or_terms; + } else { + $e->disconnect; + return new OpenILS::Event("BAD_PARAMS", "desc" => "No usable terms"); + } + my $attr_from_filter; if ($options->{"order_by"}) { # What's the point of this block? When using ORDER BY in conjuction @@ -650,9 +720,27 @@ q/order_by clause must be of the long form, like: # since the non-distinct column may arbitrarily (via hash keys) # sort to the front of the final SQL, which PG will complain about. $select_clause = { $hint => ["id"] }; + + my ($links, $flink, $fselector); + if ( + ($links = get_fm_links_by_hint($class)) && + ($flink = $links->{$field}) && + ($fselector = get_fm_selector_by_hint($flink->{class})) + ) { # sorting on a linked field, replace with remote selector b/c there is one + my $ob_join_alias = confirm_join_for_orderby($$graft_map{$hint},$flink->{class},$field); + if (!$ob_join_alias) { + no strict 'refs'; + my $func = 'add_'.$flink->{class}.'_joins'; + $func->($graft_map,$hint,[$class,$field,99]); + $ob_join_alias = confirm_join_for_orderby($$graft_map{$hint},$flink->{class},$field); + } + $class = $order_by->{class} = $ob_join_alias; + $field = $order_by->{field} = $fselector; + } + $select_clause->{$class} ||= []; push @{$select_clause->{$class}}, - {column => $field, transform => 'first', aggregate => 1}; + {column => $field, transform => 'min', aggregate => 1}; # when sorting by LI attr values, we have to limit # to a specific type of attr value to sort on. @@ -666,56 +754,15 @@ q/order_by clause must be of the long form, like: "type" => "left", "field" =>"lineitem" }; - # and ensure that the aggregate form of the attribute - # value column is included in the order_by - $order_by->{aggregate} = 1; - $order_by->{transform} = 'first'; } - } - } - - my $query = { - select => $select_clause, - order_by => ($options->{order_by} || {$hint => {id => {}}}), - offset => ($options->{offset} || 0) - }; - - $query->{"limit"} = $options->{"limit"} if $options->{"limit"}; - - my $graft_map = build_from_clause_and_joins( - $query, $hint, $and_terms, $or_terms - ); - - $and_terms = prepare_terms($and_terms, 1); - $or_terms = prepare_terms($or_terms, 0); - - unless ($options->{au_by_id}) { - my $offset = add_au_joins($graft_map, $hint, prepare_au_terms($and_terms)); - add_au_joins($graft_map, $hint, prepare_au_terms($or_terms, $offset)); - } - - my $offset = add_acqpro_joins($graft_map, $hint, prepare_acqpro_terms($and_terms)); - add_acqpro_joins($graft_map, $hint, prepare_acqpro_terms($or_terms, $offset)); - - # The join to acqmapinv needs to be a left join when present. - if ($query->{from}{$hint}{acqmapinv}) { - $query->{from}{$hint}{acqmapinv}{type} = "left"; - } - if ($and_terms and $or_terms) { - $query->{"where"} = { - "-" . (lc $conj eq "or" ? "or" : "and") => [$and_terms, $or_terms] - }; - } elsif ($and_terms) { - $query->{"where"} = $and_terms; - } elsif ($or_terms) { - $query->{"where"} = $or_terms; - } else { - $e->disconnect; - return new OpenILS::Event("BAD_PARAMS", "desc" => "No usable terms"); + # ensure that the aggregate form of the attribute + # value column is included in the order_by + $order_by->{aggregate} = 1; + $order_by->{transform} = 'min'; + } } - # if ordering by acqlia, insert the from clause # filter to limit to one type of attr. if ($attr_from_filter) { @@ -723,6 +770,10 @@ q/order_by clause must be of the long form, like: $query->{from}->{jub}->{acqlia} = $attr_from_filter; } + $$query{select} = $select_clause; + $$query{order_by} = ($options->{order_by} || {$hint => {id => {}}}); + $$query{offset} = ($options->{offset} || 0); + my $results = $e->json_query($query) or return $e->die_event; my @id_list = map { $_->{"id"} } (grep { $_->{"id"} } @$results); -- 2.11.0