diff --git a/CHANGELOG.md b/CHANGELOG.md index be5863520..c26acddf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ New entries in this file should aim to provide a meaningful amount of informatio ### Added - Migrate Webpacker to Esbuild [PR#3320](https://github.com/ualbertalib/jupiter/pull/3320) +### Removed +- Remove fully enabled feature flags that have been enabled for years [PR#3375](https://github.com/ualbertalib/jupiter/pull/3375) + ### Chores - Update Bundler to v2.5.5 to match production [PR#3374](https://github.com/ualbertalib/jupiter/pull/3374) diff --git a/Gemfile.lock b/Gemfile.lock index 9b84143e7..1f3c71ef5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -162,7 +162,7 @@ GEM connection_pool (2.4.1) cork (0.3.0) colored2 (~> 3.1) - crack (0.4.6) + crack (1.0.0) bigdecimal rexml crass (1.0.6) @@ -386,7 +386,7 @@ GEM activesupport (>= 3.0.0) raabro (1.4.0) racc (1.7.3) - rack (2.2.8) + rack (2.2.8.1) rack-protection (3.1.0) rack (~> 2.2, >= 2.2.4) rack-test (2.1.0) @@ -460,7 +460,7 @@ GEM rack (>= 1.4) retriable (3.1.2) rexml (3.2.6) - rollbar (3.5.1) + rollbar (3.5.2) rouge (4.1.3) rsolr (2.5.0) builder (>= 2.1.2) @@ -509,14 +509,14 @@ GEM seed_dump (3.3.1) activerecord (>= 4) activesupport (>= 4) - selenium-webdriver (4.17.0) + selenium-webdriver (4.18.1) base64 (~> 0.2) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) shoulda-matchers (6.1.0) activesupport (>= 5.2.0) - sidekiq (7.2.1) + sidekiq (7.2.2) concurrent-ruby (< 2) connection_pool (>= 2.3.0) rack (>= 2.2.4) @@ -525,7 +525,7 @@ GEM fugit (~> 1.8) globalid (>= 1.0.1) sidekiq (>= 6) - sidekiq-unique-jobs (8.0.9) + sidekiq-unique-jobs (8.0.10) concurrent-ruby (~> 1.0, >= 1.0.5) sidekiq (>= 7.0.0, < 8.0.0) thor (>= 1.0, < 3.0) @@ -589,7 +589,7 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) - webmock (3.20.0) + webmock (3.22.0) addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) diff --git a/app/controllers/collections_controller.rb b/app/controllers/collections_controller.rb index 7ae36e1ac..2991e98e3 100644 --- a/app/controllers/collections_controller.rb +++ b/app/controllers/collections_controller.rb @@ -8,8 +8,7 @@ def show base_restriction_key: Item.solr_exporter_class.solr_name_for(:member_of_paths, role: :pathing), value: @collection.path, params:, - current_user:, - fulltext: Flipper.enabled?(:fulltext_search, current_user) + current_user: ) @results = search_query_index.results @search_models = search_query_index.search_models diff --git a/app/controllers/profile_controller.rb b/app/controllers/profile_controller.rb index 6d68f9c6d..41ff79aca 100644 --- a/app/controllers/profile_controller.rb +++ b/app/controllers/profile_controller.rb @@ -10,8 +10,7 @@ def index base_restriction_key: Item.solr_exporter_class.solr_name_for(:owner, role: :exact_match), value: @user.id, params:, - current_user: @user, - fulltext: Flipper.enabled?(:fulltext_search, current_user) + current_user: @user ) @results = search_query_index.results @search_models = search_query_index.search_models diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index b5f06f549..57a872b17 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -19,8 +19,7 @@ def index search_query_index = UserSearchService.new( search_models: models, params:, - current_user:, - fulltext: Flipper.enabled?(:fulltext_search, current_user) + current_user: ) @results = search_query_index.results @search_models = search_query_index.search_models diff --git a/app/decorators/facets/default_facet_decorator.rb b/app/decorators/facets/default_facet_decorator.rb index 5c6ed3a17..a0d741a2b 100644 --- a/app/decorators/facets/default_facet_decorator.rb +++ b/app/decorators/facets/default_facet_decorator.rb @@ -29,11 +29,7 @@ def facet_search_link end def display - if Flipper.enabled?(:facet_badge_category_name) - "#{@category_name}: #{display_value}" - else - display_value - end + "#{@category_name}: #{display_value}" end def display_value diff --git a/app/models/jupiter_core/search.rb b/app/models/jupiter_core/search.rb index 42ef1fc3b..a016f034b 100644 --- a/app/models/jupiter_core/search.rb +++ b/app/models/jupiter_core/search.rb @@ -37,13 +37,7 @@ def self.faceted_search(q: '', facets: [], ranges: [], models: [], as: nil, full base_query << q if q.present? facets.each do |key, values| - if Flipper.enabled?(:or_facets) - fq << %Q(#{key}:\(#{values.collect { |value| "\"#{value}\"" }.join(' OR ')}\)) - else - values.each do |value| - fq << %Q(#{key}:"#{value}") - end - end + fq << %Q(#{key}:\(#{values.collect { |value| "\"#{value}\"" }.join(' OR ')}\)) end ranges.each do |key, value| fq << "#{key}:[#{value[:begin]} TO #{value[:end]}]" diff --git a/app/services/user_search_service.rb b/app/services/user_search_service.rb index f780e8f90..779705a16 100644 --- a/app/services/user_search_service.rb +++ b/app/services/user_search_service.rb @@ -6,8 +6,8 @@ class UserSearchService attr_reader :search_models, :invalid_date_range - def initialize(current_user:, params:, base_restriction_key: nil, value: nil, # rubocop:disable Metrics/ParameterLists - search_models: [Item, Thesis], fulltext: false) + def initialize(current_user:, params:, base_restriction_key: nil, value: nil, + search_models: [Item, Thesis]) if base_restriction_key.present? && value.blank? raise ArgumentError, 'Must supply both a base_restriction_key and a value' end @@ -18,7 +18,6 @@ def initialize(current_user:, params:, base_restriction_key: nil, value: nil, # @search_models = search_models @search_params = search_params(params) @current_user = current_user - @fulltext = fulltext end def results @@ -31,7 +30,7 @@ def results search_options = { q: query, models: search_models, as: @current_user, facets:, ranges: @search_params[:ranges], - fulltext: @fulltext } + fulltext: true } # Sort by relevance if a search term is present and no explicit sort field has been chosen sort_fields = @search_params[:sort] diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb index a9d5cc55b..280bd0012 100644 --- a/app/views/layouts/admin.html.erb +++ b/app/views/layouts/admin.html.erb @@ -23,11 +23,9 @@ - <% if Flipper.enabled?(:batch_ingest, current_user) %> - - <% end %> + <%= yield %> diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb index c7962c7a5..289f600f0 100644 --- a/test/controllers/search_controller_test.rb +++ b/test/controllers/search_controller_test.rb @@ -69,24 +69,12 @@ class SearchControllerTest < ActionDispatch::IntegrationTest assert_no_match(//, response.body) end - test 'should NOT render highlights on search result page by default' do - get search_url, params: { search: 'French' } - - assert_response :success - - assert_no_match(/French<\/mark>/, response.body) - end - - test 'should render highlights on search result page when feature flag is on' do - Flipper.enable(:fulltext_search) - + test 'should render highlights on search result page by default' do get search_url, params: { search: 'French' } assert_response :success assert_match(/French<\/mark>/, response.body) - - Flipper.disable(:fulltext_search) end test 'search with invalid date range shows alert' do diff --git a/test/integration/solr_bad_request_test.rb b/test/integration/solr_bad_request_test.rb index 4e787eb05..92aef1229 100644 --- a/test/integration/solr_bad_request_test.rb +++ b/test/integration/solr_bad_request_test.rb @@ -3,24 +3,10 @@ class SolrBadRequestTest < ActionDispatch::IntegrationTest test 'request in which solr gives 400 gives a proper response' do - get search_url, params: { facets: { all_subjects_sim: ['Antimicrobial', '"blown-pack"'] } } - - assert_response :bad_request - - get search_url, params: { facets: { all_subjects_sim: ['"A Most Extraordinary Case"'] } } - - assert_response :bad_request - - get search_url, params: { facets: { all_subjects_sim: ['"291" (Gallery)'], - departments_sim: ['Department of Art and Design'], - item_type_with_status_sim: ['thesis'], - member_of_paths_dpsim: ['db9a4e71-f809-4385-a274-048f28eb6814'] } } - - assert_response :bad_request - - get search_url, params: { direction: 'asc', - facets: { all_subjects_sim: ['Meat spoilage', "\"\\'Bacteriocins", '"blown-pack"'] }, - sort: 'title' } + # Special characters can cause solr to give a 400 error, so let's test that we are handling that properly + get search_url, params: { + facets: { all_subjects_sim: ["\"\\'Bacteriocins"] } + } assert_response :bad_request end diff --git a/test/models/search_test.rb b/test/models/search_test.rb index 4138dcaaf..f3467997b 100644 --- a/test/models/search_test.rb +++ b/test/models/search_test.rb @@ -2,22 +2,7 @@ class SearchTest < ActiveSupport::TestCase - test 'default behaviour within a facet is AND and between facets is AND' do - results = JupiterCore::Search.faceted_search(models: [Item], - facets: { - all_subjects_sim: ['Fancy things', - 'Practical things'], - all_contributors_sim: ['Joe Blow'] - }) - - # rubocop:disable Layout/LineLength - assert_includes results.criteria[:fq], - 'all_subjects_sim:"Fancy things" AND all_subjects_sim:"Practical things" AND all_contributors_sim:"Joe Blow"' - # rubocop:enable Layout/LineLength - end - - test 'default behaviour within a facet is OR and between facets is AND when feature flag is on' do - Flipper.enable(:or_facets) + test 'default behaviour within a facet is OR and between facets is AND' do item1 = items(:item_fancy) item2 = items(:item_practical) @@ -34,7 +19,6 @@ class SearchTest < ActiveSupport::TestCase assert_includes results.criteria[:fq], 'all_subjects_sim:("Fancy things" OR "Practical things") AND all_contributors_sim:("Joe Blow")' - Flipper.disable(:or_facets) end end diff --git a/test/services/user_search_service_test.rb b/test/services/user_search_service_test.rb index 2eed774b1..679b02e7b 100644 --- a/test/services/user_search_service_test.rb +++ b/test/services/user_search_service_test.rb @@ -79,8 +79,7 @@ class UserSearchServiceTest < ActiveSupport::TestCase search = UserSearchService.new( current_user:, - params:, - fulltext: true + params: ) # Only Admin item has "French" in its description diff --git a/test/system/search_test.rb b/test/system/search_test.rb index fa5336487..32f98d853 100644 --- a/test/system/search_test.rb +++ b/test/system/search_test.rb @@ -204,8 +204,7 @@ class SearchTest < ApplicationSystemTestCase badge.assert_selector 'span.badge', text: 'Fancy Collection 1' end - test 'facet badge should have category when flipped' do - Flipper.enable(:facet_badge_category_name) + test 'facet badge should have category' do visit root_path fill_in name: 'search', with: 'Fancy' click_button 'Search' @@ -219,8 +218,6 @@ class SearchTest < ApplicationSystemTestCase href: search_path(search: 'Fancy')) badge.assert_selector 'span.badge', text: 'Collections: Fancy Community/Fancy Collection 1' - - Flipper.disable(:facet_badge_category_name) end test 'anybody should be able to view community/collection hits via tabs' do