From f254bff73172e5e60bc79e42fde179da406e73a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Samuli=20Sillanp=C3=A4=C3=A4?=
 <samuli.sillanpaa@helsinki.fi>
Date: Tue, 27 Aug 2019 00:02:16 +0300
Subject: [PATCH] Add configuration options for sorting hierarchical facets
 (homepage, advanced search, sidefacets) (#1389)

---
 config/vufind/facets.ini                      | 39 +++++++--
 .../src/VuFind/AjaxHandler/GetFacetData.php   |  6 +-
 .../src/VuFind/ContentBlock/FacetList.php     | 10 ++-
 .../VuFind/Controller/AbstractSolrSearch.php  | 51 +++++++++--
 .../Search/Solr/HierarchicalFacetHelper.php   | 32 ++++++-
 .../Solr/HierarchicalFacetHelperTest.php      | 87 ++++++++++++++++++-
 .../templates/ContentBlock/FacetList.phtml    |  4 +-
 .../Recommend/SideFacets/facet.phtml          |  2 +-
 8 files changed, 201 insertions(+), 30 deletions(-)

diff --git a/config/vufind/facets.ini b/config/vufind/facets.ini
index be5e7c6fae2..5dfca94afc9 100644
--- a/config/vufind/facets.ini
+++ b/config/vufind/facets.ini
@@ -66,14 +66,29 @@ dateRange[] = publishDate
 ; (see https://wiki.apache.org/solr/HierarchicalFaceting but note that we always
 ; use a trailing slash to avoid ambiguities)
 ;hierarchical[] = building
-; Sort options for hierarchical facets:
-; How hierarchical facets are sorted. Default is result count, but alternative ways
-; can be specified:
-; top = Sort the top level list alphabetically, others by result count (useful e.g.
-;       for a large number of building facets where top level is organization and
-;       second level the library branch)
-; all = Sort all levels alphabetically
+
+; General sort options for hierarchical facets (Home page, Advanced Search and
+; SideFacets).
+;
+; You can set a general default setting with * and set field-specific overrides
+; using field names (see example below).
+;
+; Available options:
+; top   = Sort the top level list alphabetically, others by result count (useful e.g.
+;         for a large number of building facets where top level is organization and
+;         second level the library branch)
+; all   = Sort all levels alphabetically
+; count = Sort all levels by count
+;
+; Note: this section may be overridden for HomePage and Advanced search facets (see
+; hierarchicalFacetSortOptions in HomePage_Settings and Advanced_Settings below).
+;
+; By default, if no settings are configured in this file, the default sort will be
+; 'count' for SideFacets values, 'all' for HomePage values, and 'top' for Advanced
+; values.
+;hierarchicalFacetSortOptions[*] = all
 ;hierarchicalFacetSortOptions[building] = top
+
 ; How hierarchical facet values are displayed in the records:
 ; single = Display only the deepest level (default)
 ; full   = Display full hierarchy for each entry
@@ -208,6 +223,11 @@ translated_facets[] = callnumber-first:CallNumberFirst
 ;delimited_facets[] = author_id_str
 ;delimited_facets[] = "author_id_str|:::"
 
+; Sort overrides for Advanced search hierarchical facets. See the comments
+; above the SpecialFacets > hierarchicalFacetSortOptions setting for details.
+;hierarchicalFacetSortOptions[*] = all
+;hierarchicalFacetSortOptions[building] = top
+
 ; These facets will be displayed on the Home Page when FacetList is turned on in
 ; the content setting of the [HomePage] section of searches.ini. If this section
 ; is omitted, the [Advanced] section will be used instead.
@@ -225,6 +245,11 @@ format           = Format
 ; of the homepage facet lists, we may not display all loaded values for every facet
 facet_limit      = 20
 
+; Sort overrides for HomePage search hierarchical facets. See the comments
+; above the SpecialFacets > hierarchicalFacetSortOptions setting for details.
+;hierarchicalFacetSortOptions[*] = all
+;hierarchicalFacetSortOptions[building] = top
+
 [Visual_Settings]
 ; Which two facetable fields should be used for creating the visual results?
 ; See VisualFacets recommendation module in searches.ini for more details.
diff --git a/module/VuFind/src/VuFind/AjaxHandler/GetFacetData.php b/module/VuFind/src/VuFind/AjaxHandler/GetFacetData.php
index d211fe353fc..69d57c6223b 100644
--- a/module/VuFind/src/VuFind/AjaxHandler/GetFacetData.php
+++ b/module/VuFind/src/VuFind/AjaxHandler/GetFacetData.php
@@ -108,11 +108,7 @@ class GetFacetData extends AbstractBase
             $facets = [];
         } else {
             $facetList = $facets[$facet]['data']['list'];
-
-            if (!empty($sort)) {
-                $this->facetHelper->sortFacetList($facetList, $sort == 'top');
-            }
-
+            $this->facetHelper->sortFacetList($facetList, $sort);
             $facets = $this->facetHelper->buildFacetArray(
                 $facet, $facetList, $results->getUrlQuery(), false
             );
diff --git a/module/VuFind/src/VuFind/ContentBlock/FacetList.php b/module/VuFind/src/VuFind/ContentBlock/FacetList.php
index f8673e00162..a691c9512c3 100644
--- a/module/VuFind/src/VuFind/ContentBlock/FacetList.php
+++ b/module/VuFind/src/VuFind/ContentBlock/FacetList.php
@@ -105,9 +105,17 @@ class FacetList implements ContentBlockInterface
      */
     protected function getHierarchicalFacetSortSettings($facetConfig)
     {
-        return isset($facetConfig->SpecialFacets->hierarchicalFacetSortOptions)
+        $baseConfig
+            = isset($facetConfig->SpecialFacets->hierarchicalFacetSortOptions)
             ? $facetConfig->SpecialFacets->hierarchicalFacetSortOptions->toArray()
             : [];
+        $homepageConfig
+            = isset($facetConfig->HomePage_Settings->hierarchicalFacetSortOptions)
+            ? $facetConfig->HomePage_Settings->hierarchicalFacetSortOptions
+                ->toArray()
+            : [];
+
+        return array_merge($baseConfig, $homepageConfig);
     }
 
     /**
diff --git a/module/VuFind/src/VuFind/Controller/AbstractSolrSearch.php b/module/VuFind/src/VuFind/Controller/AbstractSolrSearch.php
index 9b278161ae7..179cfa2acae 100644
--- a/module/VuFind/src/VuFind/Controller/AbstractSolrSearch.php
+++ b/module/VuFind/src/VuFind/Controller/AbstractSolrSearch.php
@@ -55,8 +55,15 @@ class AbstractSolrSearch extends AbstractSearch
             ->getList('Advanced');
         $view->hierarchicalFacets
             = $this->getHierarchicalFacets($view->options->getFacetsIni());
+        $view->hierarchicalFacetsSortOptions
+            = $this->getAdvancedHierarchicalFacetsSortOptions(
+                $view->options->getFacetsIni()
+            );
         $view->facetList = $this->processAdvancedFacets(
-            $facets, $view->saved, $view->hierarchicalFacets
+            $facets,
+            $view->saved,
+            $view->hierarchicalFacets,
+            $view->hierarchicalFacetsSortOptions
         );
         $specialFacets = $this->parseSpecialFacetsSetting(
             $view->options->getSpecialAdvancedFacets()
@@ -116,14 +123,17 @@ class AbstractSolrSearch extends AbstractSearch
     /**
      * Process the facets to be used as limits on the Advanced Search screen.
      *
-     * @param array  $facetList          The advanced facet values
-     * @param object $searchObject       Saved search object (false if none)
-     * @param array  $hierarchicalFacets Hierarchical facet list (if any)
+     * @param array  $facetList                     The advanced facet values
+     * @param object $searchObject                  Saved search object
+     * (false if none)
+     * @param array  $hierarchicalFacets            Hierarchical facet list (if any)
+     * @param array  $hierarchicalFacetsSortOptions Hierarchical facet sort options
+     * (if any)
      *
-     * @return array               Sorted facets, with selected values flagged.
+     * @return array Sorted facets, with selected values flagged.
      */
     protected function processAdvancedFacets($facetList, $searchObject = false,
-        $hierarchicalFacets = []
+        $hierarchicalFacets = [], $hierarchicalFacetsSortOptions = []
     ) {
         // Process the facets
         $facetHelper = null;
@@ -136,7 +146,11 @@ class AbstractSolrSearch extends AbstractSearch
             // to a flat array according to the hierarchy
             if (in_array($facet, $hierarchicalFacets)) {
                 $tmpList = $list['list'];
-                $facetHelper->sortFacetList($tmpList, true);
+
+                $sort = $hierarchicalFacetsSortOptions[$facet]
+                    ?? $hierarchicalFacetsSortOptions['*'] ?? 'top';
+
+                $facetHelper->sortFacetList($tmpList, $sort);
                 $tmpList = $facetHelper->buildFacetArray(
                     $facet,
                     $tmpList
@@ -181,4 +195,27 @@ class AbstractSolrSearch extends AbstractSearch
             ? $facetConfig->SpecialFacets->hierarchical->toArray()
             : [];
     }
+
+    /**
+     * Get an array of hierarchical facet sort options for Advanced search
+     *
+     * @param string $config Name of facet configuration file to load.
+     *
+     * @return array
+     */
+    protected function getAdvancedHierarchicalFacetsSortOptions($config)
+    {
+        $facetConfig = $this->getConfig($config);
+        $baseConfig
+            = isset($facetConfig->SpecialFacets->hierarchicalFacetSortOptions)
+            ? $facetConfig->SpecialFacets->hierarchicalFacetSortOptions->toArray()
+            : [];
+        $advancedConfig
+            = isset($facetConfig->Advanced_Settings->hierarchicalFacetSortOptions)
+            ? $facetConfig->Advanced_Settings->hierarchicalFacetSortOptions
+                ->toArray()
+            : [];
+
+        return array_merge($baseConfig, $advancedConfig);
+    }
 }
diff --git a/module/VuFind/src/VuFind/Search/Solr/HierarchicalFacetHelper.php b/module/VuFind/src/VuFind/Search/Solr/HierarchicalFacetHelper.php
index 41ded201203..d67c4df5fa1 100644
--- a/module/VuFind/src/VuFind/Search/Solr/HierarchicalFacetHelper.php
+++ b/module/VuFind/src/VuFind/Search/Solr/HierarchicalFacetHelper.php
@@ -45,13 +45,39 @@ class HierarchicalFacetHelper
      * Helper method for building hierarchical facets:
      * Sort a facet list according to the given sort order
      *
-     * @param array $facetList Facet list returned from Solr
-     * @param bool  $topLevel  Whether to sort only top level
+     * @param array          $facetList Facet list returned from Solr
+     * @param boolean|string $order     Sort order:
+     * - true|top  sort top level alphabetically and the rest by count
+     * - false|all sort all levels alphabetically
+     * - count     sort all levels by count
      *
      * @return void
      */
-    public function sortFacetList(&$facetList, $topLevel)
+    public function sortFacetList(&$facetList, $order = null)
     {
+        // We need a boolean flag indicating whether or not to sort only the top
+        // level of the hierarchy. If we received a string configuration option,
+        // we should set the flag accordingly (boolean values of $order are
+        // supported for backward compatibility).
+        $topLevel = $order ?? 'count';
+        if (is_string($topLevel)) {
+            switch (strtolower(trim($topLevel))) {
+            case 'top':
+                $topLevel = true;
+                break;
+            case 'all':
+                $topLevel = false;
+                break;
+            case '':
+            case 'count':
+                // At present, we assume the incoming list is already sorted by
+                // count, so no further action is needed. If in future we need
+                // to support re-sorting an arbitrary list, rather than simply
+                // operating on raw Solr values, we may need to implement logic.
+                return;
+            }
+        }
+
         // Parse level from each facet value so that the sort function
         // can run faster
         foreach ($facetList as &$facetItem) {
diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Search/Solr/HierarchicalFacetHelperTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Search/Solr/HierarchicalFacetHelperTest.php
index 4df48e826fc..4911c02966f 100644
--- a/module/VuFind/tests/unit-tests/src/VuFindTest/Search/Solr/HierarchicalFacetHelperTest.php
+++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Search/Solr/HierarchicalFacetHelperTest.php
@@ -43,6 +43,11 @@ use VuFindTest\Unit\TestCase;
  */
 class HierarchicalFacetHelperTest extends TestCase
 {
+    /**
+     * Test input data.
+     *
+     * @var array
+     */
     protected $facetList = [
         [
             'value' => '0/Book/',
@@ -113,9 +118,39 @@ class HierarchicalFacetHelperTest extends TestCase
     }
 
     /**
-     * Tests for sortFacetList (top level only)
+     * Tests for sortFacetList (default/count sort -- at present these should
+     * make no changes to the input data and can thus both be tested in a single
+     * test method).
+     *
+     * @return void
      */
-    public function testSortFacetListTopLevel()
+    public function testSortFacetListDefault()
+    {
+        $facetList = $this->facetList;
+        $this->helper->sortFacetList($facetList);
+        $this->assertEquals($facetList[0]['value'], '0/Book/');
+        $this->assertEquals($facetList[1]['value'], '0/AV/');
+        $this->assertEquals($facetList[2]['value'], '0/Audio/');
+        $this->assertEquals($facetList[3]['value'], '1/Book/BookPart/');
+        $this->assertEquals($facetList[4]['value'], '1/Book/Section/');
+        $this->assertEquals($facetList[5]['value'], '1/Audio/Spoken/');
+        $this->assertEquals($facetList[6]['value'], '1/Audio/Music/');
+        $this->helper->sortFacetList($facetList, 'count');
+        $this->assertEquals($facetList[0]['value'], '0/Book/');
+        $this->assertEquals($facetList[1]['value'], '0/AV/');
+        $this->assertEquals($facetList[2]['value'], '0/Audio/');
+        $this->assertEquals($facetList[3]['value'], '1/Book/BookPart/');
+        $this->assertEquals($facetList[4]['value'], '1/Book/Section/');
+        $this->assertEquals($facetList[5]['value'], '1/Audio/Spoken/');
+        $this->assertEquals($facetList[6]['value'], '1/Audio/Music/');
+    }
+
+    /**
+     * Tests for sortFacetList (top level only, specified with boolean)
+     *
+     * @return void
+     */
+    public function testSortFacetListTopLevelBooleanTrue()
     {
         $facetList = $this->facetList;
         $this->helper->sortFacetList($facetList, true);
@@ -129,9 +164,29 @@ class HierarchicalFacetHelperTest extends TestCase
     }
 
     /**
-     * Tests for sortFacetList (all levels)
+     * Tests for sortFacetList (top level only, specified with string)
+     *
+     * @return void
      */
-    public function testSortFacetListAllLevels()
+    public function testSortFacetListTopLevelStringConfig()
+    {
+        $facetList = $this->facetList;
+        $this->helper->sortFacetList($facetList, 'top');
+        $this->assertEquals($facetList[0]['value'], '0/AV/');
+        $this->assertEquals($facetList[1]['value'], '0/Book/');
+        $this->assertEquals($facetList[2]['value'], '0/Audio/');
+        $this->assertEquals($facetList[3]['value'], '1/Book/BookPart/');
+        $this->assertEquals($facetList[4]['value'], '1/Book/Section/');
+        $this->assertEquals($facetList[5]['value'], '1/Audio/Spoken/');
+        $this->assertEquals($facetList[6]['value'], '1/Audio/Music/');
+    }
+
+    /**
+     * Tests for sortFacetList (all levels, specified with boolean)
+     *
+     * @return void
+     */
+    public function testSortFacetListAllLevelsBooleanFalse()
     {
         $facetList = $this->facetList;
         $this->helper->sortFacetList($facetList, false);
@@ -144,8 +199,28 @@ class HierarchicalFacetHelperTest extends TestCase
         $this->assertEquals($facetList[6]['value'], '1/Audio/Spoken/');
     }
 
+    /**
+     * Tests for sortFacetList (all levels, specified with string)
+     *
+     * @return void
+     */
+    public function testSortFacetListAllLevelsStringConfig()
+    {
+        $facetList = $this->facetList;
+        $this->helper->sortFacetList($facetList, 'all');
+        $this->assertEquals($facetList[0]['value'], '0/AV/');
+        $this->assertEquals($facetList[1]['value'], '0/Book/');
+        $this->assertEquals($facetList[2]['value'], '0/Audio/');
+        $this->assertEquals($facetList[3]['value'], '1/Book/BookPart/');
+        $this->assertEquals($facetList[4]['value'], '1/Book/Section/');
+        $this->assertEquals($facetList[5]['value'], '1/Audio/Music/');
+        $this->assertEquals($facetList[6]['value'], '1/Audio/Spoken/');
+    }
+
     /**
      * Tests for buildFacetArray
+     *
+     * @return void
      */
     public function testBuildFacetArray()
     {
@@ -183,6 +258,8 @@ class HierarchicalFacetHelperTest extends TestCase
 
     /**
      * Tests for flattenFacetHierarchy
+     *
+     * @return void
      */
     public function testFlattenFacetHierarchy()
     {
@@ -202,6 +279,8 @@ class HierarchicalFacetHelperTest extends TestCase
 
     /**
      * Tests for formatDisplayText
+     *
+     * @return void
      */
     public function testFormatDisplayText()
     {
diff --git a/themes/bootstrap3/templates/ContentBlock/FacetList.phtml b/themes/bootstrap3/templates/ContentBlock/FacetList.phtml
index fe60f9bd34a..a889fb248a7 100644
--- a/themes/bootstrap3/templates/ContentBlock/FacetList.phtml
+++ b/themes/bootstrap3/templates/ContentBlock/FacetList.phtml
@@ -11,7 +11,7 @@
       <?php if ($isHierarchy = in_array($field, $hierarchicalFacets ?? [])):
           $this->headScript()->appendFile('vendor/jsTree/jstree.min.js');
           $this->headScript()->appendFile('facets.js');
-          $sort = $hierarchicalFacetSortOptions[$field] ?? '';
+          $sort = $hierarchicalFacetSortOptions[$field] ?? $hierarchicalFacetSortOptions['*'] ?? 'all';
           $script = <<<JS
 $(document).ready(function() {
   $('#facet_{$this->escapeHtml($field)}_container').removeClass('hide');
@@ -29,7 +29,7 @@ JS;
               data-exclude="0"
               data-operator="AND"
               data-exclude-title="<?=$this->transEsc('exclude_facet')?>"
-              data-sort="all">
+              data-sort="<?=$sort?>">
           </div>
         </div>
         <noscript>
diff --git a/themes/bootstrap3/templates/Recommend/SideFacets/facet.phtml b/themes/bootstrap3/templates/Recommend/SideFacets/facet.phtml
index c8ff7fcfb29..502b89ff9d8 100644
--- a/themes/bootstrap3/templates/Recommend/SideFacets/facet.phtml
+++ b/themes/bootstrap3/templates/Recommend/SideFacets/facet.phtml
@@ -28,7 +28,7 @@
       [
         'allowExclude' => $this->recommend->excludeAllowed($facet),
         'title' => $facet,
-        'sortOptions' => $hierarchicalFacetSortOptions[$facet] ?? '',
+        'sortOptions' => $hierarchicalFacetSortOptions[$facet] ?? $hierarchicalFacetSortOptions['*'] ?? null,
         'collapsedFacets' => $this->collapsedFacets
       ]
     ); ?>
-- 
GitLab