Merge pull request #722 from projectblacklight/proposed-backports-to-…

…4.6.3

Proposed backports to 4.6.3

LICENSE

@@ -1,5 +1,6 @@
 ##########################################################################
-# Copyright 2008 Rector and Visitors of the University of Virginia
+# Copyright 2008-2014 Rector and Visitors of the University of Virginia, The Board of Trustees of the Leland Stanford Junior University, Johns Hopkins Universities, and Data Curation Experts
+# Additional copyright may be held by others, as reflected in the commit log
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.

VERSION

@@ -1 +1 @@
-4.6.2
+4.6.3

app/controllers/feedback_controller.rb

@@ -1,6 +1,9 @@
 # -*- encoding : utf-8 -*-
 class FeedbackController < ApplicationController
 
+  extend Deprecation
+  self.deprecation_horizon = 'Blacklight 5.x'
+
   # http://expressica.com/simple_captcha/
   # include SimpleCaptcha::ControllerHelpers
 
@@ -14,6 +17,7 @@ def show
       end
     end
   end
+  deprecation_deprecate :show
 
   protected
 

app/helpers/blacklight/blacklight_helper_behavior.rb

@@ -368,14 +368,18 @@ def field_value_separator
     ', '
   end
 
-  def document_index_view_type
-    if blacklight_config.document_index_view_types.include? params[:view]
-      params[:view]
+  def document_index_view_type query_params=params
+    if blacklight_config.document_index_view_types.include? query_params[:view]
+      query_params[:view]
     else
-      blacklight_config.document_index_view_types.first
+      default_document_index_view_type
     end
   end
 
+  def default_document_index_view_type
+    blacklight_config.document_index_view_types.first
+  end
+
   def render_document_index documents = nil, locals = {}
     documents ||= @document_list
     render_document_index_with_view(document_index_view_type, documents)
@@ -464,6 +468,17 @@ def link_to_query(query)
     link_to(query, link_url)
   end
 
+  ##
+  # Get the path to the search action with any parameters (e.g. view type)
+  # that should be persisted across search sessions.
+  def start_over_path query_params = params
+    h = { }
+    current_index_view_type = document_index_view_type(query_params)
+    h[:view] = current_index_view_type unless current_index_view_type == default_document_index_view_type
+
+    search_action_url(h)
+  end
+
   def render_document_index_label doc, opts
     label = nil
     label ||= doc.get(opts[:label], :sep => nil) if opts[:label].instance_of? Symbol

app/helpers/blacklight/catalog_helper_behavior.rb

@@ -19,6 +19,7 @@ def paginate_params(response)
   # kaminari paginate, passed on through.
   # will output HTML pagination controls.
   def paginate_rsolr_response(response, options = {}, &block)
+    Deprecation.warn Blacklight::CatalogHelperBehavior, "#paginate_rsolr_response is deprecated; the original response object is Kaminari-compatible"
     paginate response, options, &block
   end
 

app/helpers/blacklight/facets_helper_behavior.rb

@@ -58,7 +58,23 @@ def render_facet_limit(display_facet, options = {})
   # @param [Blacklight::SolrResponse::Facets::FacetField] display_facet 
   def should_render_facet? display_facet
     # display when show is nil or true
-    display = facet_configuration_for_field(display_facet.name).show != false
+    facet_config = facet_configuration_for_field(display_facet.name)
+
+    display = case facet_config.show
+    when Symbol
+      arity = method(facet_config.show).arity
+
+      if arity == 0
+        send(facet_config.show)
+      else 
+        send(facet_config.show, display_facet)
+      end
+    when Proc
+      facet_config.show.call self, facet_config, display_facet
+    else
+      facet_config.show
+    end
+
     return display && display_facet.items.present?
   end
 

app/views/catalog/_constraints.html.erb

@@ -2,7 +2,7 @@
       <div id="appliedParams" class="constraints-container">
         <span class="constraints-label"><%= t('blacklight.search.filters.title') %></span>
 
-        <%=link_to t('blacklight.search.start_over'), url_for(:action=>'index'), :class => "catalog_startOverLink", :id=>"startOverLink" %>
+          <%=link_to t('blacklight.search.start_over'), start_over_path, :class => "catalog_startOverLink btn btn-sm btn-text", :id=>"startOverLink" %>
         <%= render_constraints(params) %>  
 
       </div>

app/views/catalog/_previous_next_doc.html.erb

@@ -10,7 +10,7 @@
 <div class="pull-right">
       <%= link_back_to_catalog %>
       |
-      <%=link_to "#{t('blacklight.search.start_over')}", catalog_index_path, :id=>"startOverLink" %>
+      <%=link_to "#{t('blacklight.search.start_over')}", start_over_path(current_search_session.try(:query_params) || {}), :id=>"startOverLink" %>
   </div>
 	</div>
 <% end %>

lib/blacklight/configuration/facet_field.rb

@@ -7,6 +7,8 @@ def normalize! blacklight_config = nil
         self.tag = "#{self.field}_single"
         self.ex = "#{self.field}_single"
       end
+
+      self.show = true if self.show.nil?
       super
     end
   end

lib/blacklight/routes.rb

@@ -2,6 +2,26 @@
 module Blacklight
   class Routes
 
+    # adds as class and instance level accessors, default_route_sets
+    # returns an array of symbols for method names that define routes. 
+    # Order is important:.  (e.g. /catalog/email precedes /catalog/:id)
+    #
+    # Add-ons that want to add routes into default routing might
+    # monkey-patch Blacklight::Routes, say:
+    #
+    #     module MyWidget::Routes
+    #       extend ActiveSupport::Concern
+    #       included do |klass|
+    #         klass.default_route_sets += [:widget_routing]
+    #       end
+    #       def widget_routing(primary_resource)
+    #         get "#{primary_resource}/widget", "#{primary_resource}#widget"
+    #       end
+    #     end
+    #     Blacklight::Routes.send(:include, MyWidget::Routes)
+    class_attribute :default_route_sets
+    self.default_route_sets = [:bookmarks, :search_history, :saved_searches, :catalog, :solr_document, :feedback]
+
     def initialize(router, options)
       @router = router
       @options = options
@@ -23,10 +43,6 @@ def route_sets
       (@options[:only] || default_route_sets) - (@options[:except] || [])
     end
 
-    def default_route_sets
-      [:bookmarks, :search_history, :saved_searches, :catalog, :solr_document, :feedback]
-    end
-
     module RouteSets
       def bookmarks
         add_routes do |options|

spec/helpers/blacklight_helper_spec.rb

@@ -334,6 +334,26 @@ def params
        params[:view] = 'not_in_list'
        document_index_view_type.should == 'list'
      end
+
+     it "should pluck values from supplied params" do
+       blacklight_config.stub(:document_index_view_types) { ['list', 'asdf'] }
+       params[:view] = 'asdf'
+       expect(document_index_view_type(:view => 'list')).to eq 'list'
+     end
+   end
+
+   describe "start_over_path" do
+    it 'should be the catalog path with the current view type' do
+      blacklight_config.stub(:document_index_view_types) { ['list', 'abc'] }
+      helper.stub(:blacklight_config => blacklight_config)
+      expect(helper.start_over_path(:view => 'abc')).to eq catalog_index_url(:view => 'abc')
+    end
+
+    it 'should not include the current view type if it is the default' do
+      blacklight_config.stub(:document_index_view_types) { ['list', 'abc'] }
+      helper.stub(:blacklight_config => blacklight_config)
+      expect(helper.start_over_path(:view => 'list')).to eq catalog_index_url
+    end
    end
 
    describe "render_document_index" do

spec/helpers/facets_helper_spec.rb

@@ -34,11 +34,16 @@
     before do
       @config = Blacklight::Configuration.new do |config|
         config.add_facet_field 'basic_field'
-        config.add_facet_field 'no_show', :show=>false
+        config.add_facet_field 'no_show', :show => false
+        config.add_facet_field 'helper_show', :show => :my_helper
+        config.add_facet_field 'helper_with_an_arg_show', :show => :my_helper_with_an_arg
+        config.add_facet_field 'lambda_show', :show => lambda { |context, config, field| true }
+        config.add_facet_field 'lambda_no_show', :show => lambda { |context, config, field| false }
       end
 
       helper.stub(:blacklight_config => @config)
     end
+
     it "should render facets with items" do
       a = double(:items => [1,2], :name=>'basic_field')
       helper.should_render_facet?(a).should == true
@@ -52,6 +57,27 @@
       a = double(:items => [1,2], :name=>'no_show')
       helper.should_render_facet?(a).should ==  false
     end
+
+    it "should call a helper to determine if it should render a field" do
+      helper.stub(:my_helper => true)
+      a = double(:items => [1,2], :name=>'helper_show')
+      expect(helper.should_render_facet?(a)).to be_true
+    end
+
+    it "should call a helper to determine if it should render a field" do
+      a = double(:items => [1,2], :name=>'helper_with_an_arg_show')
+      helper.should_receive(:my_helper_with_an_arg).with(a).and_return(true)
+      expect(helper.should_render_facet?(a)).to be_true
+    end
+
+
+    it "should evaluate a Proc to determine if it should render a field" do
+      a = double(:items => [1,2], :name=>'lambda_show')
+      expect(helper.should_render_facet?(a)).to be_true
+
+      a = double(:items => [1,2], :name=>'lambda_no_show')
+      expect(helper.should_render_facet?(a)).to be_false
+    end
   end
 
   describe "facet_by_field_name" do

spec/routing/routes_spec.rb

@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe "Blacklight::Routes" do
+  describe "default_route_sets" do
+    around do |example|
+      @original = Blacklight::Routes.default_route_sets.dup.freeze
+
+      example.run
+
+      Blacklight::Routes.default_route_sets = @original
+    end
+
+    it "is settable" do
+      Blacklight::Routes.default_route_sets += [:foo]
+
+      # Order DOES matter. 
+      expect(Blacklight::Routes.default_route_sets).to eq(@original + [:foo])
+    end
+  end
+end

spec/views/catalog/_constraints.html.erb_spec.rb

@@ -0,0 +1,33 @@
+require 'spec_helper'
+
+describe "catalog/constraints" do
+  let :blacklight_config do
+    Blacklight::Configuration.new :document_index_view_types => ['list', 'xyz']
+  end
+
+  it "should render nothing if no constraints are set" do
+    view.stub(query_has_constraints?: false)
+    render partial: "catalog/constraints"
+    expect(rendered).to be_empty
+  end
+
+  it "should render a start over link" do
+    view.should_receive(:search_action_url).with({}).and_return('http://xyz')
+    view.stub(query_has_constraints?: true)
+    view.stub(:blacklight_config).and_return(blacklight_config)
+    render partial: "catalog/constraints"
+    expect(rendered).to have_selector("#startOverLink")
+    expect(rendered).to have_link("Start Over", :href => 'http://xyz')
+  end
+
+  it "should render a start over link with the current view type" do
+    view.should_receive(:search_action_url).with(view: 'xyz').and_return('http://xyz?view=xyz')
+    view.stub(query_has_constraints?: true)
+    params[:view] = 'xyz'
+    view.stub(:blacklight_config).and_return(blacklight_config)
+    render partial: "catalog/constraints"
+    expect(rendered).to have_selector("#startOverLink")
+    expect(rendered).to have_link("Start Over", :href => 'http://xyz?view=xyz')
+  end
+
+end