Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First page bug #124

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/models/author_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ def self.for(direction:, reference_id:, num_rows_to_display:, original_reference
num_rows_to_display: num_rows_to_display,
original_reference: original_reference,
banner_reference: banner_reference,
match_field: "id",
browse_solr_client: BrowseSolrClient.new(core: S.authority_collection, match_field: "term", q: "browse_field:name")
)

Expand Down
15 changes: 10 additions & 5 deletions lib/models/browse_list.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
class BrowseList
attr_reader :original_reference, :num_rows_to_display, :num_matches, :exact_matches, :banner_reference, :index_docs

def self.for(direction:, reference_id:, num_rows_to_display:, original_reference:,
def self.for(
direction:,
reference_id:,
num_rows_to_display:,
original_reference:,
banner_reference:,
browse_solr_client: BrowseSolrClient.new)
browse_solr_client: BrowseSolrClient.new,
match_field: "PlACEHOLDER"
)

my_banner_reference = banner_reference
exact_matches = browse_solr_client.exact_matches(value: original_reference)
case direction
when "next"
# includes reference in results
# require "byebug"; byebug
index_response = browse_solr_client.browse_reference_on_top(reference_id: reference_id, rows: num_rows_to_display + 2)
BrowseList::ReferenceOnTop.new(
index_response: index_response&.body,
Expand All @@ -32,8 +37,8 @@ def self.for(direction:, reference_id:, num_rows_to_display:, original_reference
else
# index_before:, index_after:
return BrowseList::Empty.new if reference_id.nil? || reference_id == ""
index_before = browse_solr_client.browse_reference_on_bottom(reference_id: reference_id, rows: 3)
index_after = browse_solr_client.browse_reference_on_top(reference_id: reference_id, rows: num_rows_to_display - 1)
index_before = browse_solr_client.browse_reference_on_bottom(reference_id: reference_id, rows: 3, field: match_field)
index_after = browse_solr_client.browse_reference_on_top(reference_id: reference_id, rows: num_rows_to_display - 1, field: match_field)
my_banner_reference = index_after&.body&.dig("response", "docs")&.first&.dig("id")
# need above and below
BrowseList::ReferenceInMiddle.new(
Expand Down
3 changes: 2 additions & 1 deletion lib/models/call_number_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ def self.for(direction:, reference_id:, num_rows_to_display:, original_reference
reference_id: reference_id,
num_rows_to_display: num_rows_to_display,
original_reference: original_reference,
banner_reference: banner_reference
banner_reference: banner_reference,
match_field: "callnumber"
)

bib_ids = browse_list.docs.map { |x| x["bib_id"] }
Expand Down
1 change: 1 addition & 0 deletions lib/models/subject_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ def self.for(direction:, reference_id:, num_rows_to_display:, original_reference
num_rows_to_display: num_rows_to_display,
original_reference: original_reference,
banner_reference: banner_reference,
match_field: "id",
browse_solr_client: BrowseSolrClient.new(core: S.authority_collection, match_field: "term", q: "browse_field:subject")
)

Expand Down
12 changes: 6 additions & 6 deletions lib/utilities/browse_solr_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ def initialize(solr_url: S.solr_url, core: S.call_number_collection, match_field
@q = q
end

def browse_reference_on_top(reference_id:, rows: 20)
def browse_reference_on_top(reference_id:, rows: 20, field: "id")
# square brackets includes reference in return
range = "id:[\"#{reference_id}\" TO *]"
sort = "id asc"
range = "#{field}:[\"#{reference_id}\" TO *]"
sort = "#{field} asc"
browse(rows: rows, sort: sort, range: range)
end

def browse_reference_on_bottom(reference_id:, rows: 20)
def browse_reference_on_bottom(reference_id:, rows: 20, field: "id")
# curly brackets exclues reference in return
range = "id:{* TO \"#{reference_id}\"}"
sort = "id desc"
range = "#{field}:{* TO \"#{reference_id}\"}"
sort = "#{field} desc"
browse(rows: rows, sort: sort, range: range)
end

Expand Down
11 changes: 10 additions & 1 deletion spec/models/browse_list_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
num_rows_to_display: 20,
original_reference: nil,
banner_reference: nil,
browse_solr_client: @browse_client
browse_solr_client: @browse_client,
match_field: "my_match_field"
}
end
subject do
Expand All @@ -19,18 +20,26 @@
@params[:direction] = "next"
allow(@browse_client).to receive(:browse_reference_on_top)
expect(subject.class).to eq(BrowseList::ReferenceOnTop)
expect(@browse_client).to have_received(:browse_reference_on_top).with(
{reference_id: nil, rows: 22}
)
end
it "returns a ReferenceOnTop when direction is previous" do
@params[:direction] = "previous"
allow(@browse_client).to receive(:browse_reference_on_bottom)
expect(subject.class).to eq(BrowseList::ReferenceOnBottom)
expect(@browse_client).to have_received(:browse_reference_on_bottom).with(
{reference_id: nil, rows: 21}
)
end
context "direction is nil" do
it "returns a ReferenceInMiddle if there's a reference_id" do
@params[:reference_id] = "Something"
allow(@browse_client).to receive(:browse_reference_on_top)
allow(@browse_client).to receive(:browse_reference_on_bottom)
expect(subject.class).to eq(BrowseList::ReferenceInMiddle)
expect(@browse_client).to have_received(:browse_reference_on_top).with({field: "my_match_field", reference_id: "Something", rows: 19})
expect(@browse_client).to have_received(:browse_reference_on_bottom).with({field: "my_match_field", reference_id: "Something", rows: 3})
end
it "returns a Empty if there in not a reference_id" do
expect(subject.class).to eq(BrowseList::Empty)
Expand Down
27 changes: 14 additions & 13 deletions spec/requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
before(:each) do
@call_number_collection = S.call_number_collection
@authority_collection = S.authority_collection
@authority_match_field = "id"
end
context "get /" do
it "has status OK" do
Expand All @@ -14,45 +15,45 @@
context "get /callnumber" do
it "for a successful query, returns status OK" do
stub_solr_get_request(url: "#{@call_number_collection}/select", query: hash_including({fq: 'callnumber:"Thing"'}), output: fixture("biblio_results.json"))
stub_solr_get_request(url: "#{@call_number_collection}/select", query: hash_including({sort: "id desc"}), output: fixture("callnumbers_before.json"))
stub_solr_get_request(url: "#{@call_number_collection}/select", query: hash_including({fq: 'id:["Thing" TO *]'}), output: fixture("callnumbers_results.json"))
stub_solr_get_request(url: "#{@call_number_collection}/select", query: hash_including({sort: "callnumber desc"}), output: fixture("callnumbers_before.json"))
stub_solr_get_request(url: "#{@call_number_collection}/select", query: hash_including({fq: 'callnumber:["Thing" TO *]'}), output: fixture("callnumbers_results.json"))
stub_biblio_get_request(url: "biblio/select", query: hash_including({}), output: fixture("biblio_results_middle.json"))
get "/callnumber", {query: "Thing"}
expect(last_response.status).to eq(200)
end
it "for a network error, it still returns a successful response, but with an error message" do
stub_solr_get_request(url: "#{@call_number_collection}/select", query: hash_including({fq: 'callnumber:"Thing"'}), no_return: true).to_timeout
stub_solr_get_request(url: "#{@call_number_collection}/select", query: hash_including({sort: "id desc"}), no_return: true).to_timeout
stub_solr_get_request(url: "#{@call_number_collection}/select", query: hash_including({sort: "callnumber desc"}), no_return: true).to_timeout
get "/callnumber", {query: "Thing"}
expect(last_response.status).to eq(200)
end
end
context "get /author" do
it "returns status OK" do
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'term:"Thing"'}), output: fixture("author_exact_matches.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({sort: "id desc"}), output: fixture("author_results.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'id:["Thing" TO *]'}), output: fixture("author_results.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: "term:\"Thing\""}), output: fixture("author_exact_matches.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({sort: "#{@authority_match_field} desc"}), output: fixture("author_results.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: "#{@authority_match_field}:[\"Thing\" TO *]"}), output: fixture("author_results.json"))
get "/author", {query: "Thing"}
expect(last_response.status).to eq(200)
end
it "for a network error, it still returns a successful response, but with an error message" do
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'term:"Thing"'}), no_return: true).to_timeout
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({sort: "id desc"}), no_return: true).to_timeout
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: "term:\"Thing\""}), no_return: true).to_timeout
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({sort: "#{@authority_match_field} desc"}), no_return: true).to_timeout
get "/author", {query: "Thing"}
expect(last_response.status).to eq(200)
end
end
context "get /subject" do
it "returns status OK" do
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'term:"Thing"'}), output: fixture("author_exact_matches.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({sort: "id desc"}), output: fixture("subject_results.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'id:["Thing" TO *]'}), output: fixture("subject_results.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: "term:\"Thing\""}), output: fixture("author_exact_matches.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({sort: "#{@authority_match_field} desc"}), output: fixture("subject_results.json"))
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: "#{@authority_match_field}:[\"Thing\" TO *]"}), output: fixture("subject_results.json"))
get "/subject", {query: "Thing"}
expect(last_response.status).to eq(200)
end
it "for a network error, it still returns a successful response, but with an error message" do
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'term:"Thing"'}), no_return: true).to_timeout
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({sort: "id desc"}), no_return: true).to_timeout
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: "term:\"Thing\""}), no_return: true).to_timeout
stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({sort: "#{@authority_match_field} desc"}), no_return: true).to_timeout
get "/subject", {query: "Thing"}
expect(last_response.status).to eq(200)
end
Expand Down
44 changes: 44 additions & 0 deletions spec/utilities/browse_solr_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,50 @@
subject do
described_class.new
end
context "#browse_reference_on_top" do
it "uses id as the default range and sort field" do
s = stub_solr_get_request(url: "#{S.call_number_collection}/select", query: hash_including(
{
fq: 'id:["whatever" TO *]',
sort: "id asc"
}
))
subject.browse_reference_on_top(reference_id: "whatever")
expect(s).to have_been_requested
end
it "uses field value as the default range and sort field when given" do
s = stub_solr_get_request(url: "#{S.call_number_collection}/select", query: hash_including(
{
fq: 'some_other_field:["whatever" TO *]',
sort: "some_other_field asc"
}
))
subject.browse_reference_on_top(reference_id: "whatever", field: "some_other_field")
expect(s).to have_been_requested
end
end
context "#browse_reference_on_bottom" do
it "uses id as the default range and sort field" do
s = stub_solr_get_request(url: "#{S.call_number_collection}/select", query: hash_including(
{
fq: 'id:{* TO "whatever"}',
sort: "id desc"
}
))
subject.browse_reference_on_bottom(reference_id: "whatever")
expect(s).to have_been_requested
end
it "uses field value as the default range and sort field when given" do
s = stub_solr_get_request(url: "#{S.call_number_collection}/select", query: hash_including(
{
fq: 'some_other_field:{* TO "whatever"}',
sort: "some_other_field desc"
}
))
subject.browse_reference_on_bottom(reference_id: "whatever", field: "some_other_field")
expect(s).to have_been_requested
end
end
context "#exact_matches" do
it "returns an array of ids for an exact match" do
stub_solr_get_request(url: "#{S.call_number_collection}/select", query: hash_including({fq: 'callnumber:"Thing"'}), output: fixture("biblio_results.json"))
Expand Down
Loading