From 246f2dddaf6bc23f0b574113166171d4e2786e9e Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Mon, 5 Oct 2015 15:29:52 +0100
Subject: [PATCH] Bug 12478: Display facet terms ordered by number of
 occurrences

By default ES returns the facet terms ordered by most used, which makes
sense.

This patch removes resort done in the scripts (catalogue/search.pl and
opac/opac-search.pl) and moves it to the module.

For Zebra it's now done in C4::Search::getRecords, and there is no
change to expect (still alphabetically).

On the Elastic search side, we could imagine to let the library define
the order of the facets. The facet terms are now sorted by most used.

To test easily this change, turn on the displayFacetCount pref.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
---
 C4/Search.pm                              |  8 +++++++
 Koha/SearchEngine/Elasticsearch/Search.pm | 26 +++++++++++++----------
 catalogue/search.pl                       |  6 ------
 opac/opac-search.pl                       |  8 -------
 4 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/C4/Search.pm b/C4/Search.pm
index 1e5142786c..7dd7c2c6fa 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -641,6 +641,14 @@ sub GetFacets {
         $facets = _get_facets_from_records( $rs );
     }
 
+    # This sorts the facets into alphabetical order
+    if ($facets && @$facets) {
+        foreach my $f (@$facets) {
+            $f->{facets} = [ sort { uc($a->{facet_label_value}) cmp uc($b->{facet_label_value}) } @{ $f->{facets} } ];
+        }
+        @$facets = sort {$a->{expand} cmp $b->{expand}} @$facets;
+    }
+
     return $facets;
 }
 
diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm
index 22182ea884..2d67d0be32 100644
--- a/Koha/SearchEngine/Elasticsearch/Search.pm
+++ b/Koha/SearchEngine/Elasticsearch/Search.pm
@@ -390,13 +390,14 @@ sub _convert_facets {
 
     # These should correspond to the ES field names, as opposed to the CCL
     # things that zebra uses.
+    # TODO let the library define the order using the interface.
     my %type_to_label = (
-        author   => 'Authors',
-        location => 'Location',
-        itype    => 'ItemTypes',
-        se       => 'Series',
-        subject  => 'Topics',
-        'su-geo' => 'Places',
+        author   => { order => 1, label => 'Authors', },
+        itype    => { order => 2, label => 'ItemTypes', },
+        location => { order => 3, label => 'Location', },
+        'su-geo' => { order => 4, label => 'Places', },
+        se       => { order => 5, label => 'Series', },
+        subject  => { order => 6, label => 'Topics', },
     );
 
     # We also have some special cases, e.g. itypes that need to show the
@@ -408,20 +409,21 @@ sub _convert_facets {
         itype    => { map { $_->itemtype         => $_->description } @itypes },
         location => { map { $_->authorised_value => ( $opac ? ( $_->lib_opac || $_->lib ) : $_->lib ) } @locations },
     );
-    my @res;
+    my @facets;
     $exp_facet //= '';
     while ( ( $type, $data ) = each %$es ) {
         next if !exists( $type_to_label{$type} );
 
-        # We restrict to the most popular $limit results
+        # We restrict to the most popular $limit !results
         my $limit = ( $type eq $exp_facet ) ? 10 : 5;
         my $facet = {
             type_id    => $type . '_id',
             expand     => $type,
             expandable => ( $type ne $exp_facet )
               && ( @{ $data->{terms} } > $limit ),
-            "type_label_$type_to_label{$type}" => 1,
+            "type_label_$type_to_label{$type}{label}" => 1,
             type_link_value                    => $type,
+            order      => $type_to_label{$type}{order},
         };
         $limit = @{ $data->{terms} } if ( $limit > @{ $data->{terms} } );
         foreach my $term ( @{ $data->{terms} }[ 0 .. $limit - 1 ] ) {
@@ -442,9 +444,11 @@ sub _convert_facets {
                 type_link_value => $type,
             };
         }
-        push @res, $facet if exists $facet->{facets};
+        push @facets, $facet if exists $facet->{facets};
     }
-    return \@res;
+
+    @facets = sort { $a->{order} cmp $b->{order} } @facets;
+    return \@facets;
 }
 
 
diff --git a/catalogue/search.pl b/catalogue/search.pl
index 06e2b1f116..653ce4c3b7 100755
--- a/catalogue/search.pl
+++ b/catalogue/search.pl
@@ -531,12 +531,6 @@ eval {
     );
 };
 
-# This sorts the facets into alphabetical order
-if ($facets) {
-    foreach my $f (@$facets) {
-        $f->{facets} = [ sort { uc($a->{facet_label_value}) cmp uc($b->{facet_label_value}) } @{ $f->{facets} } ];
-    }
-}
 if ($@ || $error) {
     $template->param(query_error => $error.$@);
     output_html_with_http_headers $cgi, $cookie, $template->output;
diff --git a/opac/opac-search.pl b/opac/opac-search.pl
index f951ef7d38..c61bbbd438 100755
--- a/opac/opac-search.pl
+++ b/opac/opac-search.pl
@@ -620,14 +620,6 @@ if ($tag) {
 };
 }
 
-# This sorts the facets into alphabetical order
-if ($facets && @$facets) {
-    foreach my $f (@$facets) {
-        $f->{facets} = [ sort { uc($a->{facet_label_value}) cmp uc($b->{facet_label_value}) } @{ $f->{facets} } ];
-    }
-    @$facets = sort {$a->{expand} cmp $b->{expand}} @$facets;
-}
-
 # use Data::Dumper; print STDERR "-" x 25, "\n", Dumper($results_hashref);
 if ($@ || $error) {
     $template->param(query_error => $error.$@);
-- 
2.39.5