From c1babd7444d1a631f3555345d5cc8cf60d114cb0 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Mon, 6 Nov 2023 16:22:04 -0500 Subject: [PATCH 1/6] Patch in an rspec-rails stub_helper that works in Rails 7.1, in rspec-rails that does not yet include it https://github.com/rspec/rspec-rails/commit/4d65bea0619955acb15023b9c3f57a3a53183da8 https://github.com/rspec/rspec-rails/issues/2696 --- spec/spec_helper.rb | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ba566119b0..5ca96f9afe 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -118,3 +118,31 @@ # as the one that triggered the failure. Kernel.srand config.seed end + +# RSpec's stub_template method needs a differnet implementation for Rails 7.1, that +# isn't yet in an rspec-rails release. +# +# First rspec-rails tried this: +# https://github.com/rspec/rspec-rails/commit/4d65bea0619955acb15023b9c3f57a3a53183da8 +# +# But it was subject to this problem: +# https://github.com/rspec/rspec-rails/issues/2696 +# +# Below implementation appears to work for our purposes here, so we will patch it in +# if we are on Rails 7.1+, and not yet rspec-rails 6.1 which we expect to have it. + +if ::Rails.version.to_f >= 7.1 && Gem.loaded_specs["rspec-rails"].version.release < Gem::Version.new('6.1') + + module RSpec + module Rails + module ViewExampleGroup + module ExampleMethods + def stub_template(hash) + controller.prepend_view_path(StubResolverCache.resolver_for(hash)) + end + end + end + end + end + +end From f3f38d755b596895bf8fcc844f6f5bf041ee4229 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Mon, 6 Nov 2023 16:33:02 -0500 Subject: [PATCH 2/6] fix custom one-off inline stub_template definitions to use new Rails 7.1 compat implementation I guess we had to implement these custom inline to use in places other than view specs, with the right method path to get to the view_context? We needed to provide an alternate implementation that works for new rspec-rails way of doing this that is compat with Rails 7.1: https://github.com/rspec/rspec-rails/commit/4d65bea0619955acb15023b9c3f57a3a53183da8 --- .../components/blacklight/facet_component_spec.rb | 12 +++++++++++- .../server_applied_params_component_spec.rb | 12 +++++++++++- spec/helpers/blacklight_helper_spec.rb | 15 ++++++++++----- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/spec/components/blacklight/facet_component_spec.rb b/spec/components/blacklight/facet_component_spec.rb index 5676ae25a5..ec89341331 100644 --- a/spec/components/blacklight/facet_component_spec.rb +++ b/spec/components/blacklight/facet_component_spec.rb @@ -45,8 +45,18 @@ def call Blacklight::Configuration::FacetField.new(key: 'field', partial: 'catalog/facet_partial').normalize! end + # Not sure why we need to re-implement rspec's stub_template, but + # we already were, and need a Rails 7.1+ safe alternate too + # https://github.com/rspec/rspec-rails/commit/4d65bea0619955acb15023b9c3f57a3a53183da8 + # https://github.com/rspec/rspec-rails/issues/2696 before do - controller.view_context.view_paths.unshift(RSpec::Rails::ViewExampleGroup::StubResolverCache.resolver_for('catalog/_facet_partial.html.erb' => 'facet partial')) + replace_hash = { 'catalog/_facet_partial.html.erb' => 'facet partial' } + + if ::Rails.version.to_f >= 7.1 + controller.prepend_view_path(RSpec::Rails::ViewExampleGroup::StubResolverCache.resolver_for(replace_hash)) + else + controller.view_context.view_paths.unshift(RSpec::Rails::ViewExampleGroup::StubResolverCache.resolver_for(replace_hash)) + end end it 'renders the partial' do diff --git a/spec/components/blacklight/search_context/server_applied_params_component_spec.rb b/spec/components/blacklight/search_context/server_applied_params_component_spec.rb index ff21468e7f..f074ca01fd 100644 --- a/spec/components/blacklight/search_context/server_applied_params_component_spec.rb +++ b/spec/components/blacklight/search_context/server_applied_params_component_spec.rb @@ -10,7 +10,17 @@ let(:view_context) { controller.view_context } before do - view_context.view_paths.unshift(RSpec::Rails::ViewExampleGroup::StubResolverCache.resolver_for('application/_start_over.html.erb' => 'start over')) + # Not sure why we need to re-implement rspec's stub_template, but + # we already were, and need a Rails 7.1+ safe alternate too + # https://github.com/rspec/rspec-rails/commit/4d65bea0619955acb15023b9c3f57a3a53183da8 + # https://github.com/rspec/rspec-rails/issues/2696 + replace_hash = { 'application/_start_over.html.erb' => 'start over' } + if ::Rails.version.to_f >= 7.1 + controller.prepend_view_path(RSpec::Rails::ViewExampleGroup::StubResolverCache.resolver_for(replace_hash)) + else + view_context.view_paths.unshift(RSpec::Rails::ViewExampleGroup::StubResolverCache.resolver_for(replace_hash)) + end + allow(view_context).to receive(:current_search_session).and_return current_search_session allow(view_context).to receive(:link_back_to_catalog).with(any_args) end diff --git a/spec/helpers/blacklight_helper_spec.rb b/spec/helpers/blacklight_helper_spec.rb index 5c2f80afa9..cd5b4cf381 100644 --- a/spec/helpers/blacklight_helper_spec.rb +++ b/spec/helpers/blacklight_helper_spec.rb @@ -185,12 +185,17 @@ blacklight_config.view.gallery(template: '/my/partial') end - def stub_template(hash) - view.view_paths.unshift(ActionView::FixtureResolver.new(hash)) - end - it 'renders that template' do - stub_template 'my/_partial.html.erb' => 'some content' + # Not sure why we need to re-implement rspec's stub_template, but + # we already were, and need a Rails 7.1+ safe alternate too + # https://github.com/rspec/rspec-rails/commit/4d65bea0619955acb15023b9c3f57a3a53183da8 + # https://github.com/rspec/rspec-rails/issues/2696 + replace_hash = { 'my/_partial.html.erb' => 'some content' } + if ::Rails.version.to_f >= 7.1 + controller.prepend_view_path(RSpec::Rails::ViewExampleGroup::StubResolverCache.resolver_for(replace_hash)) + else + view.view_paths.unshift(ActionView::FixtureResolver.new(replace_hash)) + end response = helper.render_document_index_with_view :gallery, [obj1, obj1] From 73505f8abdd276c562a06bb2dc3737e2c9e9af45 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Mon, 6 Nov 2023 16:36:49 -0500 Subject: [PATCH 3/6] For Rails 7.1+, Blacklight::Document::ActiveModelShim needs a #composite_primary_key? and #has_query_constraints? --- .../concerns/blacklight/document/active_model_shim.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/models/concerns/blacklight/document/active_model_shim.rb b/app/models/concerns/blacklight/document/active_model_shim.rb index 53bdef5a10..b0f82ecb40 100644 --- a/app/models/concerns/blacklight/document/active_model_shim.rb +++ b/app/models/concerns/blacklight/document/active_model_shim.rb @@ -29,6 +29,16 @@ def repository def find id repository.find(id).documents.first end + + # In Rails 7.1+, needs this method + def composite_primary_key? + false + end + + # In Rails 7.1+, needs this method + def has_query_constraints? + false + end end ## From 3311c3daf8f8dda51199db4b0069ab2c0fef1dec Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Tue, 7 Nov 2023 12:11:47 -0500 Subject: [PATCH 4/6] avoid deprecation warning in Rails 7.1 ActiveRecord #serialize --- app/models/search.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/search.rb b/app/models/search.rb index c01e0a9ed3..cea9010150 100644 --- a/app/models/search.rb +++ b/app/models/search.rb @@ -3,7 +3,12 @@ class Search < ApplicationRecord belongs_to :user, optional: true - serialize :query_params, Blacklight::SearchParamsYamlCoder + if ::Rails.version.to_f >= 7.1 + # non-deprecated coder: keyword arg for Rails 7.1+ + serialize :query_params, coder: Blacklight::SearchParamsYamlCoder + else + serialize :query_params, Blacklight::SearchParamsYamlCoder + end # A Search instance is considered a saved search if it has a user_id. def saved? From a97dcc52c6cc91a4549ef09642bd2d32a541a1ba Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Tue, 7 Nov 2023 12:14:49 -0500 Subject: [PATCH 5/6] Github Actions CI build on Rails 7.1.1 --- .github/workflows/ruby.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 30321a37e9..c72ff8c8fc 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -41,6 +41,8 @@ jobs: additional_engine_cart_rails_options: [''] additional_name: [''] include: + - ruby: '3.2.2' + rails_version: '7.1.1' - ruby: '3.2.0' rails_version: '7.0.4' - ruby: '3.2.0' From e45b762017c6baa47c6e540cb4cba808cbe1a4c6 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Tue, 7 Nov 2023 13:01:59 -0500 Subject: [PATCH 6/6] configure zeitwerk to be okay with our generators in local generated app ./lib/generators Not sure why we are generating a generator INTO the local test app. But rather than figure this out and change it, we want to make zeitwerk okay with it. We are telling zeitwerk to ignore the WHOLE RAILS_ROOT/lib/generators directory, just to deal with the one test_app_generator.rb file we are putting there. But hopefully that is okay. --- lib/blacklight/engine.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/blacklight/engine.rb b/lib/blacklight/engine.rb index d36fd1de2d..0a10408ff6 100644 --- a/lib/blacklight/engine.rb +++ b/lib/blacklight/engine.rb @@ -6,6 +6,18 @@ module Blacklight class Engine < Rails::Engine engine_name "blacklight" + config.before_configuration do + # see https://github.com/fxn/zeitwerk#for_gem + # Blacklight puts a generator into LOCAL APP lib/generators, so tell + # zeitwerk to ignore the whole directory? If we're using a recent + # enough version of Rails to have zeitwerk config + # + # See: https://github.com/cbeer/engine_cart/issues/117 + if Rails.try(:autoloaders).try(:main).respond_to?(:ignore) + Rails.autoloaders.main.ignore(Rails.root.join('lib/generators')) + end + end + config.after_initialize do Blacklight::Configuration.initialize_default_configuration end