From e2ab88b714cbdabea319ad950699b8d1faa888d4 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Thu, 23 Jan 2014 13:09:17 -0500 Subject: [PATCH 1/7] Make Blacklight::Routes.default_route_sets settable Useful for add-ons that want to add more routes to Blacklight::Routes, such as blacklight_range_limit --- lib/blacklight/routes.rb | 24 ++++++++++++++++++++---- spec/routing/routes_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 spec/routing/routes_spec.rb diff --git a/lib/blacklight/routes.rb b/lib/blacklight/routes.rb index f4446ddeba..6a1896cdf7 100644 --- a/lib/blacklight/routes.rb +++ b/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| diff --git a/spec/routing/routes_spec.rb b/spec/routing/routes_spec.rb new file mode 100644 index 0000000000..b451e618dd --- /dev/null +++ b/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 \ No newline at end of file From f9abdf01fbfcfe3aece98cebe4eefe4178902efa Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Fri, 17 Jan 2014 09:32:45 -0800 Subject: [PATCH 2/7] update LICENSE file --- LICENSE | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index c5c4255343..d48015cb0e 100644 --- a/LICENSE +++ b/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. From 21928e8f3a2cc459754f007e38259df08d0a5a41 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Wed, 29 Jan 2014 08:21:40 -0800 Subject: [PATCH 3/7] 'Start Over' links should preserve the current view type across search sessions Fixes #714 --- .../blacklight/blacklight_helper_behavior.rb | 23 ++++++++++--- app/views/catalog/_constraints.html.erb | 2 +- app/views/catalog/_previous_next_doc.html.erb | 2 +- spec/helpers/blacklight_helper_spec.rb | 20 +++++++++++ .../catalog/_constraints.html.erb_spec.rb | 33 +++++++++++++++++++ 5 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 spec/views/catalog/_constraints.html.erb_spec.rb diff --git a/app/helpers/blacklight/blacklight_helper_behavior.rb b/app/helpers/blacklight/blacklight_helper_behavior.rb index 226cfe17a3..818fa06ea8 100644 --- a/app/helpers/blacklight/blacklight_helper_behavior.rb +++ b/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 diff --git a/app/views/catalog/_constraints.html.erb b/app/views/catalog/_constraints.html.erb index 6f354914cc..c341e74413 100644 --- a/app/views/catalog/_constraints.html.erb +++ b/app/views/catalog/_constraints.html.erb @@ -2,7 +2,7 @@
<%= t('blacklight.search.filters.title') %> - <%=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) %>
diff --git a/app/views/catalog/_previous_next_doc.html.erb b/app/views/catalog/_previous_next_doc.html.erb index 10ad7d6416..fe9015d908 100644 --- a/app/views/catalog/_previous_next_doc.html.erb +++ b/app/views/catalog/_previous_next_doc.html.erb @@ -10,7 +10,7 @@
<%= 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" %>
<% end %> \ No newline at end of file diff --git a/spec/helpers/blacklight_helper_spec.rb b/spec/helpers/blacklight_helper_spec.rb index 3da3c534dc..f194974bc0 100644 --- a/spec/helpers/blacklight_helper_spec.rb +++ b/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 diff --git a/spec/views/catalog/_constraints.html.erb_spec.rb b/spec/views/catalog/_constraints.html.erb_spec.rb new file mode 100644 index 0000000000..cce00705ac --- /dev/null +++ b/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 \ No newline at end of file From 2977de2d5109653759d8d76b99cb992a8eb56693 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Wed, 29 Jan 2014 10:20:15 -0800 Subject: [PATCH 4/7] Provide more flexibility for deciding whether to render facet fields Add configuration options to `add_facet_field 'field', :show => ` to allow "run-time" configuration when the solr data and request context are available, with a couple different options: - helper method (with no arguments), e.g.: ``` config.add_facet_field 'field', :show => :should_render_this_field? ... \# helper def should_render_this_field? current_user.admin? end ``` - helper method with the solr data, e.g.: ``` config.add_facet_field 'field', :show => :should_render_field? ... \# helper def should_render_field? display_field display_field.items.length > 1 end ``` - inline Proc/lambda, e.g.: ``` config.add_facet_field 'field', :show => lambda { |ctx, facet_config, display_field| true } ``` Fixes #717 --- .../blacklight/facets_helper_behavior.rb | 18 +++++++++++- lib/blacklight/configuration/facet_field.rb | 2 ++ spec/helpers/facets_helper_spec.rb | 28 ++++++++++++++++++- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/app/helpers/blacklight/facets_helper_behavior.rb b/app/helpers/blacklight/facets_helper_behavior.rb index 373aae28ad..2131f76df7 100644 --- a/app/helpers/blacklight/facets_helper_behavior.rb +++ b/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 diff --git a/lib/blacklight/configuration/facet_field.rb b/lib/blacklight/configuration/facet_field.rb index c0e90ed3b5..e11ef0bfd5 100644 --- a/lib/blacklight/configuration/facet_field.rb +++ b/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 diff --git a/spec/helpers/facets_helper_spec.rb b/spec/helpers/facets_helper_spec.rb index 7c95286c17..c8ffcd3d2c 100644 --- a/spec/helpers/facets_helper_spec.rb +++ b/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 From 96f8dc60fe99e9cb256a31796caefe51cb0cc6dd Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Wed, 29 Jan 2014 17:31:13 -0800 Subject: [PATCH 5/7] deprecate FeedbackController --- app/controllers/feedback_controller.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/controllers/feedback_controller.rb b/app/controllers/feedback_controller.rb index 64eb4c0e85..2ce9f2c0ae 100644 --- a/app/controllers/feedback_controller.rb +++ b/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 From bc94cbaaaab24cfeca9662913f6338c0a90dced3 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Thu, 30 Jan 2014 08:28:51 -0800 Subject: [PATCH 6/7] deprecate #paginate_rsolr_response helper --- app/helpers/blacklight/catalog_helper_behavior.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/helpers/blacklight/catalog_helper_behavior.rb b/app/helpers/blacklight/catalog_helper_behavior.rb index 5564142b1f..04790dcf4c 100644 --- a/app/helpers/blacklight/catalog_helper_behavior.rb +++ b/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 From e80e966b0fd61aeacd0bf82ec2eca449f7032a15 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Wed, 29 Jan 2014 17:34:08 -0800 Subject: [PATCH 7/7] bump to 4.6.3 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 3208b090ce..7962f0fc07 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -4.6.2 \ No newline at end of file +4.6.3 \ No newline at end of file