From 7e37d441c54b42afeb1d5f92530ad625b49f9018 Mon Sep 17 00:00:00 2001 From: Monique Rio Date: Tue, 4 Jun 2024 13:18:28 -0400 Subject: [PATCH 1/3] Adds field as an option for browse methods --- lib/utilities/browse_solr_client.rb | 12 +++---- spec/utilities/browse_solr_client_spec.rb | 44 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/lib/utilities/browse_solr_client.rb b/lib/utilities/browse_solr_client.rb index 8df99a3..776ed3b 100644 --- a/lib/utilities/browse_solr_client.rb +++ b/lib/utilities/browse_solr_client.rb @@ -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 diff --git a/spec/utilities/browse_solr_client_spec.rb b/spec/utilities/browse_solr_client_spec.rb index 8d0df28..daa1a58 100644 --- a/spec/utilities/browse_solr_client_spec.rb +++ b/spec/utilities/browse_solr_client_spec.rb @@ -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")) From d392fe3cc6565d0cffdc09f4675d698b33355469 Mon Sep 17 00:00:00 2001 From: Monique Rio Date: Tue, 4 Jun 2024 13:55:59 -0400 Subject: [PATCH 2/3] adds match_field BrowseList inputs This enables the first page of results to use a different field for fq and sort. --- lib/models/author_list.rb | 1 + lib/models/browse_list.rb | 14 ++++++++++---- lib/models/call_number_list.rb | 3 ++- lib/models/subject_list.rb | 1 + spec/models/browse_list_spec.rb | 11 ++++++++++- spec/requests_spec.rb | 18 +++++++++--------- 6 files changed, 33 insertions(+), 15 deletions(-) diff --git a/lib/models/author_list.rb b/lib/models/author_list.rb index bf6ef25..3ecc648 100644 --- a/lib/models/author_list.rb +++ b/lib/models/author_list.rb @@ -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: "term", browse_solr_client: BrowseSolrClient.new(core: S.authority_collection, match_field: "term", q: "browse_field:name") ) diff --git a/lib/models/browse_list.rb b/lib/models/browse_list.rb index c20348c..d268ea2 100644 --- a/lib/models/browse_list.rb +++ b/lib/models/browse_list.rb @@ -1,9 +1,15 @@ 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) @@ -32,8 +38,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( diff --git a/lib/models/call_number_list.rb b/lib/models/call_number_list.rb index 5cd7738..b9424c7 100644 --- a/lib/models/call_number_list.rb +++ b/lib/models/call_number_list.rb @@ -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"] } diff --git a/lib/models/subject_list.rb b/lib/models/subject_list.rb index 64825fc..58a7076 100644 --- a/lib/models/subject_list.rb +++ b/lib/models/subject_list.rb @@ -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: "term", browse_solr_client: BrowseSolrClient.new(core: S.authority_collection, match_field: "term", q: "browse_field:subject") ) diff --git a/spec/models/browse_list_spec.rb b/spec/models/browse_list_spec.rb index a4e705d..1669e58 100644 --- a/spec/models/browse_list_spec.rb +++ b/spec/models/browse_list_spec.rb @@ -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 @@ -19,11 +20,17 @@ @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 @@ -31,6 +38,8 @@ 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) diff --git a/spec/requests_spec.rb b/spec/requests_spec.rb index 0549211..e0c4f7a 100644 --- a/spec/requests_spec.rb +++ b/spec/requests_spec.rb @@ -14,15 +14,15 @@ 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 @@ -30,14 +30,14 @@ 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({sort: "term desc"}), output: fixture("author_results.json")) + stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'term:["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({sort: "term desc"}), no_return: true).to_timeout get "/author", {query: "Thing"} expect(last_response.status).to eq(200) end @@ -45,14 +45,14 @@ 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({sort: "term desc"}), output: fixture("subject_results.json")) + stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'term:["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({sort: "term desc"}), no_return: true).to_timeout get "/subject", {query: "Thing"} expect(last_response.status).to eq(200) end From 4676eac05476adde3e9689e1b7560ac86badf032 Mon Sep 17 00:00:00 2001 From: Monique Rio Date: Tue, 4 Jun 2024 14:46:55 -0400 Subject: [PATCH 3/3] reverted to using id for authority browse --- lib/models/author_list.rb | 2 +- lib/models/browse_list.rb | 1 - lib/models/subject_list.rb | 2 +- spec/requests_spec.rb | 21 +++++++++++---------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/models/author_list.rb b/lib/models/author_list.rb index 3ecc648..36cc050 100644 --- a/lib/models/author_list.rb +++ b/lib/models/author_list.rb @@ -6,7 +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: "term", + match_field: "id", browse_solr_client: BrowseSolrClient.new(core: S.authority_collection, match_field: "term", q: "browse_field:name") ) diff --git a/lib/models/browse_list.rb b/lib/models/browse_list.rb index d268ea2..4a35490 100644 --- a/lib/models/browse_list.rb +++ b/lib/models/browse_list.rb @@ -16,7 +16,6 @@ def self.for( 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, diff --git a/lib/models/subject_list.rb b/lib/models/subject_list.rb index 58a7076..c68d0d7 100644 --- a/lib/models/subject_list.rb +++ b/lib/models/subject_list.rb @@ -6,7 +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: "term", + match_field: "id", browse_solr_client: BrowseSolrClient.new(core: S.authority_collection, match_field: "term", q: "browse_field:subject") ) diff --git a/spec/requests_spec.rb b/spec/requests_spec.rb index e0c4f7a..36af24c 100644 --- a/spec/requests_spec.rb +++ b/spec/requests_spec.rb @@ -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 @@ -29,30 +30,30 @@ 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: "term desc"}), output: fixture("author_results.json")) - stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'term:["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: "term 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: "term desc"}), output: fixture("subject_results.json")) - stub_solr_get_request(url: "#{@authority_collection}/select", query: hash_including({fq: 'term:["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: "term 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