ANW-1563 aplha sort facets without breaking facet link

public/app/controllers/concerns/handle_faceting.rb

@@ -19,14 +19,13 @@ def fetch_only_facets(query, facets_array, include_zero)
 
   # strip out: facets with counts less than input minimum or equal to the total hits, facets of form "ead/ arch*"
   # returns a hash with the text of the facet as the key, count as the value
-  def strip_facets(facets_array, min, total_hits = nil)
-    facets = {}
-    facets_array.each_slice(2) do |t, ct|
-      next if ct < min
-      next if total_hits && ct == total_hits
-      next if t.start_with?("ead/ archdesc/ ")
-      facets[t] = ct
+  def strip_facets(facets_array, min, type, total_hits = nil)
+    facets = []
+    facets_array.each_slice(2) do |facet_key, facet_count|
+      facets << Facet.new(type, facet_key, facet_count)
     end
+    facets.reject! { |f| f.count < min || f.count == total_hits }
+    facets.sort_by! { |f| f.label } if AppConfig[:pui_display_facets_alpha]
     facets
   end
 

public/app/controllers/concerns/searchable.rb

@@ -189,11 +189,11 @@ def process_search_results(base="/search")
     hits = Integer(@results['total_hits'])
     if !@results['facets'].blank?
       @results['facets']['facet_fields'].keys.each do |type|
-        facet_hash = strip_facets( @results['facets']['facet_fields'][type], 1, hits)
-        if facet_hash.present?
-          @facets[type] = facet_hash
+        facet_group = strip_facets( @results['facets']['facet_fields'][type], 1, type, hits)
+        if facet_group.present?
+          @facets[type] = facet_group
           if type == 'repository'
-            @facets['repository'].delete('global')
+            @facets['repository'].reject! { |f| f.key == 'global' }
           end
         end
       end

public/app/models/facet.rb

@@ -0,0 +1,11 @@
+class Facet < Struct.new(:type, :key, :count, :label)
+  extend HandleFaceting
+  extend ManipulateNode
+
+  def initialize(type, key, count)
+    self.type = type
+    self.key = key
+    self.count = count
+    self.label = self.class.get_pretty_facet_value(self.type, self.key)
+  end
+end

public/app/views/shared/_only_facets.html.erb

@@ -18,47 +18,27 @@
   %>
 
   <% ordered_facet_types.each do |type| %>
-    <% h = @facets.fetch(type, false) %>
+    <% facet_group = @facets.fetch(type, false) %>
 
-    <%# ANW-340: If we are sorting facets in alphabetical order, do the re-sort and facet name translation %>
-    <% if AppConfig[:pui_display_facets_alpha] %>
-      <% alpha_order = {} %>
-      <% h.each do |k, v| %>
-        <% alpha_order[get_pretty_facet_value(type, k)] = v %>
-      <% end %>
-
-      <% h = alpha_order.sort_by {|k, v| k}.to_h %>
-
-    <%# If not, just do the translation %>
-    <% else %>
-      <% translated = {} %>
-      <% h.each do |k, v| %>
-        <% translated[get_pretty_facet_value(type, k)] = v %>
-      <% end %>
-      <% h = translated %>
-    <% end %>
-
-    <% next unless h %>
+    <% next if facet_group.empty? %>
     <dt><%= t("search_results.filter.#{type}") %></dt>
-    <% facet_count = 0 %>
-    <% h.each do |v, ct| %>
-      <% if facet_count == 5 %>
+    <% facet_group.each_with_index do |facet, i| %>
+      <% if i == 5 %>
         <div class="more-facets">
           <span class="more btn">&or; <%= t('search_results.more_facets') %></span>
           <div class="below-the-fold">
       <% end %>
-      <% facet_count += 1 %>
       <dd>
-        <a href="<%= app_prefix("#{@page_search}&filter_fields[]=#{type}&filter_values[]=#{CGI.escape(v)}") %>"
+        <a href="<%= app_prefix("#{@page_search}&filter_fields[]=#{type}&filter_values[]=#{CGI.escape(facet.key)}") %>"
            rel="nofollow"
-           title="<%= t('search_results.filter_by')%> '<%= v %>'">
-          <%= v %>
+           title="<%= t('search_results.filter_by')%> '<%= facet.label %>'">
+          <%= facet.label %>
         </a>
-        <span class="recordnumber"><%= ct %></span>
+        <span class="recordnumber"><%= facet.count %></span>
       </dd>
     <% end %>
-    <% if facet_count > 5 %>
-        <span class="less btn">&and; <%= t('search_results.fewer_facets') %></span>
+    <% if facet_group.size > 5 %>
++        <span class="less btn">&and; <%= t('search_results.fewer_facets') %></span>
       </div>
     <% end %>
     <span class="type-spacer">&nbsp;</span>

public/spec/controllers/search_controller_spec.rb

@@ -62,4 +62,58 @@
     expect(result_data['total_hits']).to eq(1)
   end
 
+  describe 'leaves facets in the order received (hit count) unless config says to sort by label' do
+    it 'sorts facets by hit count' do
+      repos = class_double("Repository").
+                as_stubbed_const
+      allow(repos).to receive(:get_repos) {
+        {
+          "/repositories/3" => { "name" => "Alpha" },
+          "/repositories/2" => { "name" => "Beta" },
+          "/repositories/1" => { "name" => "Delta" }
+        }
+      }
+
+      as_client = instance_double("ArchivesSpaceClient")
+      allow(as_client).to receive(:advanced_search) { |base_search, page, criteria|
+        SolrResults.new({
+                          'total_hits' => 10,
+                          'results' => (0...10).map {|i| {
+                                                       'primary_type' => 'accession',
+                                                       'json' => {'title' => 'TITLE'},
+                                                       'uri' => "/accessions/#{i}"
+                                                     } },
+                          'facets' => {
+                            'facet_fields' => {
+                              'repository' => [
+                                '/repositories/2', 6, '/repositories/1', 3, '/repositories/3', 2
+                              ]
+                            }
+                          }
+                        })
+      }
+      allow(controller).to receive(:archivesspace).and_return(as_client)
+
+      response = get(:search, params: {
+                       :rid => 2,
+                       :q => ['foo'],
+                       :op => ['OR'],
+                       :field => ['']
+                     })
+      facets = assigns(:facets)
+      expect(facets["repository"].map { |f| f.key }).to eq(["/repositories/2", "/repositories/1", "/repositories/3"])
+
+      allow(AppConfig).to receive(:[]).and_call_original
+      allow(AppConfig).to receive(:[]).with(:pui_display_facets_alpha) { true }
+
+      response = get(:search, params: {
+                       :rid => 2,
+                       :q => ['foo'],
+                       :op => ['OR'],
+                       :field => ['']
+                     })
+      facets = assigns(:facets)
+      expect(facets["repository"].map { |f| f.key }).to eq(["/repositories/3", "/repositories/2", "/repositories/1"])
+    end
+  end
 end

public/spec/features/resources_spec.rb

@@ -54,21 +54,4 @@
     click_link 'Resource with Accession'
     expect(page).to have_content('Related Unprocessed Material')
   end
-
-  it 'can display facets in alphabetical order' do
-    visit('/')
-    click_link 'Collections'
-
-    prev = ""
-    page.all('dl#facets dd').each do |dd|
-      next if dd.find("a").text =~ /Term/
-
-      unless prev == ""
-        content = dd.find("a").text
-        expect(prev <=> content).to eq(-1)
-      end
-
-      prev = dd.find("a").text
-    end
-  end
 end

public/spec/spec_helper.rb

@@ -32,7 +32,6 @@
 AppConfig[:backend_url] = $backend
 AppConfig[:pui_hide][:record_badge] = false # we want this for testing
 AppConfig[:arks_enabled] = true # ARKs have to be enabled to be able to test them
-AppConfig[:pui_display_facets_alpha] = true
 
 $backend_start_fn = proc {
   TestUtils::start_backend($backend_port,