diff --git a/app/assets/stylesheets/partials/_search.scss b/app/assets/stylesheets/partials/_search.scss index 0cb9ed3e..daeca2a8 100644 --- a/app/assets/stylesheets/partials/_search.scss +++ b/app/assets/stylesheets/partials/_search.scss @@ -3,7 +3,7 @@ // ------------------------ /* basic search bar */ -.basic-search { +.search-form { background-color: #989898; margin-bottom: 0rem; padding: 2.4rem 2rem 1.6rem 2rem; @@ -114,7 +114,7 @@ } } - .basic-search-submit { + .geo-search-submit { width: 150px; margin-top: 0.8rem; .btn { diff --git a/app/views/basic_search/index.html.erb b/app/views/basic_search/index.html.erb index 1de2af85..7b2c7996 100644 --- a/app/views/basic_search/index.html.erb +++ b/app/views/basic_search/index.html.erb @@ -2,6 +2,6 @@
<%= render partial: "shared/site_title" %> - <%= render partial: "search/form" %> + <%= render partial: "search/form_geo" if Flipflop.enabled?(:gdt) %> <%= render partial: "static/about" if ENV.fetch('ABOUT_APP', nil) %>
diff --git a/app/views/layouts/_site_header.html.erb b/app/views/layouts/_site_header.html.erb index 4792c7c1..a8651f43 100644 --- a/app/views/layouts/_site_header.html.erb +++ b/app/views/layouts/_site_header.html.erb @@ -29,5 +29,6 @@ <% end %> + <%= render partial: 'search/form' unless Flipflop.enabled?(:gdt) %> \ No newline at end of file diff --git a/app/views/search/_form.html.erb b/app/views/search/_form.html.erb index 81f02734..deacab7f 100644 --- a/app/views/search/_form.html.erb +++ b/app/views/search/_form.html.erb @@ -1,185 +1,13 @@ -<% -# Initial form setup is determined by the advanced parameter. Thereafter it is altered by javascript. -advanced_label = "Search by title, author, etc." -search_required = true -if params[:advanced] == "true" - search_required = false -end - -geobox_label = "Geospatial bounding box search" -if params[:geobox] == "true" - geobox_required = true - search_required = false -end - -geodistance_label = "Geospatial distance search" -if params[:geodistance] == "true" - geodistance_required = true - search_required = false -end - -# Placeholder text for the keyword input changes if any of the search panels are open. -keyword_placeholder = search_required ? "Enter your search" : "Keyword anywhere" -%> - - diff --git a/app/views/search/_form_geo.html.erb b/app/views/search/_form_geo.html.erb new file mode 100644 index 00000000..9ac0a6a0 --- /dev/null +++ b/app/views/search/_form_geo.html.erb @@ -0,0 +1,205 @@ +<% +# Initial form setup is determined by the advanced parameter. Thereafter it is altered by javascript. +advanced_label = "Search by title, author, etc." +search_required = true +if params[:advanced] == "true" + search_required = false +end + +geobox_label = "Geospatial bounding box search" +if params[:geobox] == "true" + geobox_required = true + search_required = false +end + +geodistance_label = "Geospatial distance search" +if params[:geodistance] == "true" + geodistance_required = true + search_required = false +end + +# Placeholder text for the keyword input changes if any of the search panels are open. +keyword_placeholder = search_required ? "Enter your search" : "Keyword anywhere" +%> + + + +<% if Flipflop.enabled?(:boolean_picker) %> + +<% end %> + +<%= javascript_include_tag "search_form" %> diff --git a/app/views/search/results.html.erb b/app/views/search/results.html.erb index 09fbdcbc..e4b9253b 100644 --- a/app/views/search/results.html.erb +++ b/app/views/search/results.html.erb @@ -8,7 +8,6 @@ <%= render partial: "shared/site_title" %> - <%= render partial: "form" %> <%= render partial: "search_summary" %> <%= render(partial: 'shared/error', collection: @errors) %> diff --git a/app/views/search/results_geo.html.erb b/app/views/search/results_geo.html.erb index 3ec91925..1d025023 100644 --- a/app/views/search/results_geo.html.erb +++ b/app/views/search/results_geo.html.erb @@ -8,7 +8,7 @@ <%= render partial: "shared/site_title" %> - <%= render partial: "form" %> + <%= render partial: "form_geo" %> <%= render partial: "search_summary_geo" %> <%= render(partial: 'shared/error', collection: @errors) %> diff --git a/test/controllers/search_controller_geo_test.rb b/test/controllers/search_controller_geo_test.rb index 70d71304..6e6908fa 100644 --- a/test/controllers/search_controller_geo_test.rb +++ b/test/controllers/search_controller_geo_test.rb @@ -23,9 +23,45 @@ def setup assert_select 'input#advanced-identifiers', count: 0 end + test 'index shows geo form when GDT is enabled' do + get '/' + + # Should show geo form with all geo elements, no basic form in header + assert_select 'form#search-form', { count: 0 } # No basic form in header when GDT enabled + assert_select 'form#search-form-geo', { count: 1 } # Geo form in main content + assert_select 'details#geobox-search-panel', { count: 1 } + assert_select 'details#geodistance-search-panel', { count: 1 } + assert_select 'details#advanced-search-panel', { count: 1 } + end + + test 'results page shows geo form when GDT is enabled' do + VCR.use_cassette('geobox and geodistance', + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do + query = { + geobox: 'true', + geodistance: 'true', + geoboxMinLongitude: 40.5, + geoboxMinLatitude: 60.0, + geoboxMaxLongitude: 78.2, + geoboxMaxLatitude: 80.0, + geodistanceLatitude: 36.1, + geodistanceLongitude: 62.6, + geodistanceDistance: '50mi' + }.to_query + get "/results?#{query}" + assert_response :success + assert_select 'form#search-form', { count: 0 } # No basic form in header when GDT enabled + assert_select 'form#search-form-geo', { count: 1 } + assert_select 'details#geobox-search-panel', { count: 1 } + assert_select 'details#geodistance-search-panel', { count: 1 } + assert_select 'details#advanced-search-panel', { count: 1 } + end + end + test 'contributors label is renamed to authors in GDT' do get '/' - assert_select 'label', text: "Authors" + assert_select 'label', text: 'Authors' end test 'index shows geobox form, closed by default' do @@ -84,8 +120,8 @@ def setup test 'GDT lists applied geospatial search terms' do VCR.use_cassette('geobox and geodistance', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { geobox: 'true', geodistance: 'true', @@ -113,8 +149,8 @@ def setup test 'can query geobox' do VCR.use_cassette('geobox', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { geobox: 'true', geoboxMinLongitude: 40.5, @@ -130,8 +166,8 @@ def setup test 'can query geodistance' do VCR.use_cassette('geodistance', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { geodistance: 'true', geodistanceLatitude: 36.1, @@ -146,8 +182,8 @@ def setup test 'both geospatial fields can be queried at once' do VCR.use_cassette('geobox and geodistance', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { geobox: 'true', geodistance: 'true', @@ -167,8 +203,8 @@ def setup test 'geo forms are open on results page with URL parameter' do VCR.use_cassette('geobox and geodistance', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { geobox: 'true', geodistance: 'true', @@ -191,8 +227,8 @@ def setup test 'coordinates can include decimals or not' do VCR.use_cassette('geobox and geodistance many decimals', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { geobox: 'true', geodistance: 'true', @@ -213,8 +249,8 @@ def setup end VCR.use_cassette('geobox and geodistance no decimals', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { geobox: 'true', geodistance: 'true', @@ -237,8 +273,8 @@ def setup test 'geospatial arguments retain values in search form with correct data types' do VCR.use_cassette('geobox and geodistance', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { geobox: 'true', geodistance: 'true', @@ -248,7 +284,7 @@ def setup geoboxMaxLatitude: 80.0, geodistanceLatitude: 36.1, geodistanceLongitude: 62.6, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{query}" assert_response :success @@ -266,8 +302,8 @@ def setup test 'geospatial search can combine with basic and advanced inputs' do VCR.use_cassette('geo all', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do query = { q: 'hi', title: 'hey', @@ -280,7 +316,7 @@ def setup geoboxMaxLatitude: 80.0, geodistanceLatitude: 36.1, geodistanceLongitude: 62.6, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{query}" assert_response :success @@ -294,7 +330,7 @@ def setup }.to_query get "/results?#{query}" assert_response :redirect - assert_equal flash[:error], "All bounding box fields are required." + assert_equal flash[:error], 'All bounding box fields are required.' end test 'geodistance arguments are non-nullable' do @@ -303,7 +339,7 @@ def setup }.to_query get "/results?#{query}" assert_response :redirect - assert_equal flash[:error], "All geospatial distance fields are required." + assert_equal flash[:error], 'All geospatial distance fields are required.' end test 'geobox param is required for geobox search' do @@ -315,18 +351,18 @@ def setup }.to_query get "/results?#{query}" assert_response :redirect - assert_equal flash[:error], "A search term is required." + assert_equal flash[:error], 'A search term is required.' end test 'geodistance param is required for geobox search' do query = { geodistanceLatitude: 36.1, geodistanceLongitude: 62.6, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{query}" assert_response :redirect - assert_equal flash[:error], "A search term is required." + assert_equal flash[:error], 'A search term is required.' end test 'geobox lat cannot fall below lower limit' do @@ -339,7 +375,7 @@ def setup }.to_query get "/results?#{low_query}" assert_response :redirect - assert_equal flash[:error], "Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0." + assert_equal flash[:error], 'Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0.' end test 'geobox long cannot fall below lower limit' do @@ -352,7 +388,7 @@ def setup }.to_query get "/results?#{low_query}" assert_response :redirect - assert_equal flash[:error], "Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0." + assert_equal flash[:error], 'Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0.' end test 'geobox lat/long cannot exceed upper limit' do @@ -365,13 +401,13 @@ def setup }.to_query get "/results?#{high_query}" assert_response :redirect - assert_equal flash[:error], "Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0." + assert_equal flash[:error], 'Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0.' end test 'geobox does not error with minimum lat value' do VCR.use_cassette('geobox min lat range limit', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geobox: 'true', geoboxMinLongitude: -45.0, @@ -387,8 +423,8 @@ def setup test 'geobox does not error with minimum long value' do VCR.use_cassette('geobox min long range limit', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geobox: 'true', geoboxMinLongitude: -180.0, @@ -404,8 +440,8 @@ def setup test 'geobox does not error with maximum lat value' do VCR.use_cassette('geobox max lat range limit', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geobox: 'true', geoboxMinLongitude: 45.0, @@ -421,8 +457,8 @@ def setup test 'geobox does not error with maximum long value' do VCR.use_cassette('geobox max long range limit', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geobox: 'true', geoboxMinLongitude: 45.0, @@ -446,7 +482,7 @@ def setup }.to_query get "/results?#{bad_values}" assert_response :redirect - assert_equal flash[:error], "Maximum latitude cannot exceed minimum latitude." + assert_equal flash[:error], 'Maximum latitude cannot exceed minimum latitude.' end test 'geodistance lat cannot fall below lower limit' do @@ -454,11 +490,11 @@ def setup geodistance: 'true', geodistanceLatitude: -90.000001, geodistanceLongitude: -180.0, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{low_query}" assert_response :redirect - assert_equal flash[:error], "Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0." + assert_equal flash[:error], 'Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0.' end test 'geodistance long cannot fall below lower limit' do @@ -466,11 +502,11 @@ def setup geodistance: 'true', geodistanceLatitude: -90.0, geodistanceLongitude: -180.000001, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{low_query}" assert_response :redirect - assert_equal flash[:error], "Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0." + assert_equal flash[:error], 'Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0.' end test 'geodistance lat cannot exceed upper limit' do @@ -478,11 +514,11 @@ def setup geodistance: 'true', geodistanceLatitude: 90.000001, geodistanceLongitude: 180.0, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{high_query}" assert_response :redirect - assert_equal flash[:error], "Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0." + assert_equal flash[:error], 'Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0.' end test 'geodistance long cannot exceed upper limit' do @@ -490,22 +526,22 @@ def setup geodistance: 'true', geodistanceLatitude: 90.0, geodistanceLongitude: 180.000001, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{high_query}" assert_response :redirect - assert_equal flash[:error], "Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0." + assert_equal flash[:error], 'Latitude must be between -90.0 and 90.0, and longitude must be -180.0 and 180.0.' end test 'geodistance does not error with minimum lat value' do VCR.use_cassette('geodistance min lat range limit', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geodistance: 'true', geodistanceLatitude: -90.0, geodistanceLongitude: -43.33, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{acceptable_query}" assert_response :success @@ -515,13 +551,13 @@ def setup test 'geodistance does not error with minimum long value' do VCR.use_cassette('geodistance min long range limit', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geodistance: 'true', geodistanceLatitude: -21.1724, geodistanceLongitude: -180.0, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{acceptable_query}" assert_response :success @@ -531,13 +567,13 @@ def setup test 'geodistance does not error with maximum lat value' do VCR.use_cassette('geodistance max lat range limit', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geodistance: 'true', geodistanceLatitude: 90.0, geodistanceLongitude: -43.33, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{acceptable_query}" assert_response :success @@ -547,30 +583,30 @@ def setup test 'geodistance does not error with maximum long value' do VCR.use_cassette('geodistance max long range limit', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geodistance: 'true', geodistanceLatitude: -21.1724, geodistanceLongitude: 180.0, - geodistanceDistance: '50mi' + geodistanceDistance: '50mi' }.to_query get "/results?#{acceptable_query}" assert_response :success assert_nil flash[:error] end - end + end test 'geodistance cannot be negative' do zero_distance = { geodistance: 'true', geodistanceLatitude: 90.0, geodistanceLongitude: 180.0, - geodistanceDistance: '-50mi' + geodistanceDistance: '-50mi' }.to_query get "/results?#{zero_distance}" assert_response :redirect - assert_equal flash[:error], "Distance must include an integer greater than 0." + assert_equal flash[:error], 'Distance must include an integer greater than 0.' end test 'geodistance cannot be 0' do @@ -578,17 +614,17 @@ def setup geodistance: 'true', geodistanceLatitude: 90.0, geodistanceLongitude: 180.0, - geodistanceDistance: '0mi' + geodistanceDistance: '0mi' }.to_query get "/results?#{negative_distance}" assert_response :redirect - assert_equal flash[:error], "Distance must include an integer greater than 0." + assert_equal flash[:error], 'Distance must include an integer greater than 0.' end test 'geodistance can contain units or not (default is meters)' do VCR.use_cassette('geodistance units', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do acceptable_query = { geodistance: 'true', geodistanceLatitude: -21.1724, @@ -601,8 +637,8 @@ def setup end VCR.use_cassette('geodistance no units', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do another_acceptable_query = { geodistance: 'true', geodistanceLatitude: -21.1724, @@ -624,7 +660,7 @@ def setup }.to_query get "/results?#{bad_units}" assert_response :redirect - assert_equal flash[:error], "Distance units must be one of the following: mi, km, yd, ft, in, m, cm, mm, NM/nmi" + assert_equal flash[:error], 'Distance units must be one of the following: mi, km, yd, ft, in, m, cm, mm, NM/nmi' end test 'view full record link appears as expected for GDT records' do diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb index 3b5cea00..3cbdc1ca 100644 --- a/test/controllers/search_controller_test.rb +++ b/test/controllers/search_controller_test.rb @@ -75,7 +75,7 @@ def mock_timdex_search_success get '/' assert_response :success - assert_select 'form#basic-search', { count: 1 } + assert_select 'form#search-form', { count: 1 } assert_select 'details#advanced-search-panel', count: 0 end @@ -90,6 +90,19 @@ def mock_timdex_search_success end end + test 'index shows basic search form when GDT is disabled' do + # GDT is disabled by default in test setup + get '/' + assert_response :success + + # Should show basic form without geo elements + assert_select 'form#search-form', { count: 1 } + assert_select 'form#search-form-geo', { count: 0 } + assert_select 'details#geobox-search-panel', { count: 0 } + assert_select 'details#geodistance-search-panel', { count: 0 } + assert_select 'details#advanced-search-panel', { count: 0 } + end + test 'advanced search form appears on results page with URL parameter' do if Flipflop.enabled?(:gdt) VCR.use_cassette('advanced', @@ -183,7 +196,7 @@ def mock_timdex_search_success get '/results?q=hallo' assert_response :success - assert_select 'form#basic-search', { count: 1 } + assert_select 'form#search-form', { count: 1 } end test 'timdex results with valid query shows search form' do @@ -191,7 +204,7 @@ def mock_timdex_search_success get '/results?q=hallo&tab=timdex' assert_response :success - assert_select 'form#basic-search', { count: 1 } + assert_select 'form#search-form', { count: 1 } end test 'primo results with valid query populates search form with query' do @@ -210,6 +223,15 @@ def mock_timdex_search_success assert_select '#basic-search-main[value=data]' end + test 'results page shows basic USE search form when GDT is disabled' do + # GDT is disabled by default in test setup + mock_primo_search_success + get '/results?q=test' + assert_response :success + assert_select 'form#search-form', { count: 1 } + assert_select 'form#search-form-geo', { count: 0 } + end + test 'results with valid query has div for filters which is populated' do skip('Filters not implemented in USE UI') VCR.use_cassette('data basic controller',