Merge branch 'master' into webpacker-migration

ualbertalib · Feb 22, 2024 · d108700 · d108700
2 parents 9e8408d + 9c270b1
commit d108700
Show file tree

Hide file tree

Showing 14 changed files with 29 additions and 88 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -23,11 +23,9 @@
       <li class="nav-item">
         <%= link_to t('.feature_flags'), flipper_path, class: 'nav-link' %>
       </li>
-      <% if Flipper.enabled?(:batch_ingest, current_user) %>
-        <li class="nav-item">
-          <%= active_link_to t('admin.batch_ingests.index.header'), admin_batch_ingests_path, class: 'nav-link', active: :exclusive %>
-        </li>
-      <% end %>
+      <li class="nav-item">
+        <%= active_link_to t('admin.batch_ingests.index.header'), admin_batch_ingests_path, class: 'nav-link', active: :exclusive %>
+      </li>
     </ul>
     <%= yield %>
   </div>

diff --git 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(/<a href="\/items\/#{@item4.id}">/, 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(/<mark>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(/<mark>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
@@ -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
@@ -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
@@ -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
@@ -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