Skip to content

Commit

Permalink
Merge f00033f into 0ac60aa
Browse files Browse the repository at this point in the history
  • Loading branch information
jcoyne committed Oct 4, 2018
2 parents 0ac60aa + f00033f commit 817c4e3
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 58 deletions.
15 changes: 0 additions & 15 deletions .rubocop_todo.yml
Expand Up @@ -190,21 +190,6 @@ Lint/ShadowingOuterLocalVariable:
- 'spec/models/blacklight/configuration_spec.rb'
- 'spec/models/blacklight/solr/search_builder_spec.rb'

# Offense count: 30
Lint/UselessAssignment:
Exclude:
- 'app/controllers/concerns/blacklight/search_context.rb'
- 'app/helpers/blacklight/facets_helper_behavior.rb'
- 'app/models/bookmark.rb'
- 'lib/blacklight/routes/exportable.rb'
- 'lib/blacklight/routes/searchable.rb'
- 'lib/railties/blacklight.rake'
- 'spec/helpers/blacklight/configuration_helper_behavior_spec.rb'
- 'spec/helpers/catalog_helper_spec.rb'
- 'spec/models/blacklight/solr/search_builder_spec.rb'
- 'spec/services/blacklight/search_service_spec.rb'
- 'spec/support/features/session_helpers.rb'

# Offense count: 1
Lint/UselessComparison:
Exclude:
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/blacklight/search_context.rb
Expand Up @@ -84,7 +84,7 @@ def find_or_initialize_search_session_from_params params

saved_search = searches_from_history.find { |x| x.query_params == params_copy }

saved_search ||= Search.create(query_params: params_copy).tap do |s|
saved_search || Search.create(query_params: params_copy).tap do |s|
add_to_search_history(s)
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/blacklight/facets_helper_behavior.rb
Expand Up @@ -105,7 +105,7 @@ def facet_partial_name(display_facet = nil)
config = facet_configuration_for_field(display_facet.name)
name = config.try(:partial)
name ||= "facet_pivot" if config.pivot
name ||= "facet_limit"
name || "facet_limit"
end

##
Expand Down
2 changes: 1 addition & 1 deletion app/models/bookmark.rb
Expand Up @@ -13,7 +13,7 @@ def document
def document_type
value = super if defined?(super)
value &&= value.constantize
value ||= default_document_type
value || default_document_type
end

def default_document_type
Expand Down
4 changes: 1 addition & 3 deletions lib/blacklight/routes/exportable.rb
Expand Up @@ -6,9 +6,7 @@ def initialize(defaults = {})
@defaults = defaults
end

def call(mapper, options = {})
options = @defaults.merge(options)

def call(mapper, _options = {})
mapper.member do
mapper.match 'email', via: [:get, :post]
mapper.match 'sms', via: [:get, :post]
Expand Down
4 changes: 1 addition & 3 deletions lib/blacklight/routes/searchable.rb
Expand Up @@ -6,9 +6,7 @@ def initialize(defaults = {})
@defaults = defaults
end

def call(mapper, options = {})
options = @defaults.merge(options)

def call(mapper, _options = {})
mapper.match '/', action: 'index', as: 'search', via: [:get, :post]

mapper.post ":id/track", action: 'track', as: 'track'
Expand Down
10 changes: 5 additions & 5 deletions spec/helpers/blacklight/configuration_helper_behavior_spec.rb
Expand Up @@ -131,22 +131,22 @@

describe "#field_label" do
it "looks up the label as an i18n string" do
allow(helper).to receive(:t).with(:some_key, default: []).and_return "my label"
expect(helper).to receive(:t).with(:some_key, default: []).and_return "my label"
label = helper.field_label :some_key

expect(label).to eq "my label"
end

it "passes the provided i18n keys to I18n.t" do
allow(helper).to receive(:t).with(:key_a, default: [:key_b, "default text"])
expect(helper).to receive(:t).with(:key_a, default: [:key_b, "default text"])

label = helper.field_label :key_a, :key_b, "default text"
helper.field_label :key_a, :key_b, "default text"
end

it "compacts nil keys (fixes rails/rails#19419)" do
allow(helper).to receive(:t).with(:key_a, default: [:key_b])
expect(helper).to receive(:t).with(:key_a, default: [:key_b])

label = helper.field_label :key_a, nil, :key_b
helper.field_label :key_a, nil, :key_b
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/helpers/catalog_helper_spec.rb
Expand Up @@ -254,7 +254,7 @@ def render_grouped_response?

it "calls thumbnail presenter with provided values" do
expect(thumbnail_presenter).to receive(:thumbnail_tag).with({}, suppress_link: true)
result = helper.render_thumbnail_tag document, {}, suppress_link: true
helper.render_thumbnail_tag document, {}, suppress_link: true
end
end

Expand Down
19 changes: 8 additions & 11 deletions spec/models/blacklight/solr/search_builder_spec.rb
Expand Up @@ -453,7 +453,7 @@

describe "#add_solr_fields_to_query" do
let(:blacklight_config) do
config = Blacklight::Configuration.new do |config|
Blacklight::Configuration.new do |config|
config.add_index_field 'an_index_field', solr_params: { 'hl.alternativeField' => 'field_x' }
config.add_show_field 'a_show_field', solr_params: { 'hl.alternativeField' => 'field_y' }
config.add_field_configuration_to_solr_request!
Expand All @@ -476,15 +476,13 @@

describe "#add_facetting_to_solr" do
let(:blacklight_config) do
config = Blacklight::Configuration.new

config.add_facet_field 'test_field', sort: 'count'
config.add_facet_field 'some-query', query: { 'x' => { fq: 'some:query' } }, ex: 'xyz'
config.add_facet_field 'some-pivot', pivot: %w[a b], ex: 'xyz'
config.add_facet_field 'some-field', solr_params: { 'facet.mincount' => 15 }
config.add_facet_fields_to_solr_request!

config
Blacklight::Configuration.new do |config|
config.add_facet_field 'test_field', sort: 'count'
config.add_facet_field 'some-query', query: { 'x' => { fq: 'some:query' } }, ex: 'xyz'
config.add_facet_field 'some-pivot', pivot: %w[a b], ex: 'xyz'
config.add_facet_field 'some-field', solr_params: { 'facet.mincount' => 15 }
config.add_facet_fields_to_solr_request!
end
end

let(:solr_parameters) do
Expand Down Expand Up @@ -674,7 +672,6 @@
end

it "does not add an !ex local parameter if it isn't configured" do
mock_field = double
expect(subject.with_ex_local_param(nil, "some-value")).to eq "some-value"
end
end
Expand Down
32 changes: 16 additions & 16 deletions spec/services/blacklight/search_service_spec.rb
Expand Up @@ -153,7 +153,7 @@
let(:user_params) { { q: all_docs_query } }

before do
(solr_response, document_list) = service.search_results
(solr_response,) = service.search_results
@facets = solr_response.aggregations
end

Expand Down Expand Up @@ -195,7 +195,7 @@
let(:user_params) { { q: all_docs_query } }

it 'starts with first results by default' do
(solr_response, document_list) = service.search_results
(solr_response,) = service.search_results
expect(solr_response.params[:start].to_i).to eq 0
end
it 'has number of results (per page) set in initializer, by default' do
Expand Down Expand Up @@ -231,7 +231,7 @@
let(:page) { 3 }

it 'skips appropriate number of results when requested - default per page' do
(solr_response2, document_list2) = service.search_results
(solr_response2,) = service.search_results
expect(solr_response2.params[:start].to_i).to eq blacklight_config[:default_solr_params][:rows] * (page - 1)
end
end
Expand All @@ -243,7 +243,7 @@

it 'skips appropriate number of results when requested - non-default per page' do
num_results = 3
(solr_response2a, document_list2a) = service.search_results
(solr_response2a,) = service.search_results
expect(solr_response2a.params[:start].to_i).to eq num_results * (page - 1)
end
end
Expand All @@ -267,7 +267,7 @@
it 'shows first results when prompted for page before first result' do
# FIXME: should it show first results, or should it throw an error for view to deal w?
# Solr throws an error for a negative start value
(solr_response4, document_list4) = service.search_results
(solr_response4,) = service.search_results
expect(solr_response4.params[:start].to_i).to eq 0
end
end
Expand Down Expand Up @@ -320,7 +320,7 @@
let(:user_params) { { q: 'boo' } }

it 'has (multiple) spelling suggestions' do
(solr_response, document_list) = service.search_results
(solr_response,) = service.search_results
expect(solr_response.spelling.words).to include('bon')
expect(solr_response.spelling.words).to include('bod') # for multiple suggestions
end
Expand All @@ -330,7 +330,7 @@
let(:user_params) { { q: 'politica' } }

it 'has multiple spelling suggestions' do
(solr_response, document_list) = service.search_results
(solr_response,) = service.search_results
expect(solr_response.spelling.words).to include('policy') # less freq
expect(solr_response.spelling.words).to include('politics') # more freq
expect(solr_response.spelling.words).to include('political') # more freq
Expand All @@ -345,7 +345,7 @@
let(:user_params) { { q: 'yehudiyam', qt: 'search', "spellcheck.dictionary": "title" } }

it 'has spelling suggestions' do
(solr_response, document_list) = service.search_results
(solr_response,) = service.search_results
expect(solr_response.spelling.words).to include('yehudiyim')
end
end
Expand All @@ -354,7 +354,7 @@
let(:user_params) { { q: 'shirma', qt: 'search', "spellcheck.dictionary": "author" } }

it 'has spelling suggestions' do
(solr_response, document_list) = service.search_results
(solr_response,) = service.search_results
expect(solr_response.spelling.words).to include('sharma')
end
end
Expand All @@ -363,7 +363,7 @@
let(:user_params) { { q: 'wome', qt: 'search', "spellcheck.dictionary": "subject" } }

it 'has spelling suggestions' do
(solr_response, document_list) = service.search_results
(solr_response,) = service.search_results
expect(solr_response.spelling.words).to include('women')
end
end
Expand All @@ -389,40 +389,40 @@
end

it "returns the previous and next documents for a search" do
response, docs = service.previous_and_next_documents_for_search(4, q: '')
_response, docs = service.previous_and_next_documents_for_search(4, q: '')

expect(docs.first.id).to eq @all_docs[3].id
expect(docs.last.id).to eq @all_docs[5].id
end

it "returns only the next document if the counter is 0" do
response, docs = service.previous_and_next_documents_for_search(0, q: '')
_response, docs = service.previous_and_next_documents_for_search(0, q: '')

expect(docs.first).to be_nil
expect(docs.last.id).to eq @all_docs[1].id
end

it "returns only the previous document if the counter is the total number of documents" do
response, docs = service.previous_and_next_documents_for_search(@full_response.total - 1, q: '')
_response, docs = service.previous_and_next_documents_for_search(@full_response.total - 1, q: '')
expect(docs.first.id).to eq @all_docs.slice(-2).id
expect(docs.last).to be_nil
end

it "returns an array of nil values if there is only one result" do
response, docs = service.previous_and_next_documents_for_search(0, q: 'id:2007020969')
_response, docs = service.previous_and_next_documents_for_search(0, q: 'id:2007020969')
expect(docs.last).to be_nil
expect(docs.first).to be_nil
end

it 'returns only the unique key by default' do
response, docs = service.previous_and_next_documents_for_search(0, q: '')
_response, docs = service.previous_and_next_documents_for_search(0, q: '')
expect(docs.last.to_h).to eq 'id' => @all_docs[1].id
end

it 'allows the query parameters to be customized using configuration' do
blacklight_config.document_pagination_params[:fl] = 'id,format'

response, docs = service.previous_and_next_documents_for_search(0, q: '')
_response, docs = service.previous_and_next_documents_for_search(0, q: '')

expect(docs.last.to_h).to eq @all_docs[1].to_h.slice('id', 'format')
end
Expand Down
2 changes: 1 addition & 1 deletion spec/support/features/session_helpers.rb
Expand Up @@ -13,7 +13,7 @@ def sign_up_with(email, password)

def sign_in(login = 'user1')
email = "#{login}@#{login}.com"
user = User.create(email: email, password: "password", password_confirmation: "password")
User.create(email: email, password: "password", password_confirmation: "password")
visit new_user_session_path
fill_in("user_email", with: email)
fill_in("user_password", with: "password")
Expand Down

0 comments on commit 817c4e3

Please sign in to comment.