spotlight icon indicating copy to clipboard operation
spotlight copied to clipboard
verified publisher icon projectblacklight

Support Blacklight 8

Open corylown opened this issue 1 year ago • 7 comments

This may have mostly been completed via https://github.com/projectblacklight/spotlight/pull/2853

Relates to:

  • https://github.com/projectblacklight/spotlight/issues/3025
  • https://github.com/projectblacklight/spotlight/issues/2950
  • https://github.com/projectblacklight/spotlight/pull/2949
  • https://github.com/projectblacklight/spotlight/pull/3022

Upgrading to Blacklight 8 Guide

  • [ ] #3069
  • [x] https://github.com/projectblacklight/spotlight/issues/2966
  • [ ] #3070
  • [ ] https://github.com/projectblacklight/spotlight/pull/2922
  • [ ] Convert admin partials to components? (not sure if this is strictly required): https://github.com/search?q=repo%3Aprojectblacklight%2Fspotlight+blacklight_config.view.admin_table&type=code
  • [ ] Figure out if we need to adapt how we're handling constraints: see https://github.com/projectblacklight/blacklight/wiki/Upgrading-to-Blacklight-8#adding-elements-to-constraints and also: https://github.com/search?q=repo%3Aprojectblacklight%2Fspotlight+render_constraints&type=code

Things we can drop if we stop supporting Blacklight 7:

  • [ ] Blacklight::SearchService#fetch returns just the documents, not the solr response and the documents: https://github.com/search?q=repo%3Aprojectblacklight%2Fspotlight+search_service.fetch&type=code
  • [ ] convert_to_search_state has been removed. Just use search_state: https://github.com/search?q=repo%3Aprojectblacklight%2Fspotlight+convert_to_search_state&type=code

Update: Look at this comment for current state: https://github.com/projectblacklight/spotlight/issues/3047#issuecomment-2274034424

corylown avatar Jul 08 '24 18:07 corylown

Known issues with Blacklight 8 & Spotlight:

  • app/views/layouts/spotlight/base.html.erb and app/views/shared/_header_navbar.html.erb both call Blacklight's container_classes helper that in Blacklight 8 calls blacklight_config. Seeing some test failures complaining that blacklight_config is not defined.
  • The only way I've been able to get spotlight to work with Blacklight 8 is to generate spotlight with Blacklight 7 and then switch to version 8 via the gemfile, likely due to different default settings for asset handling in BL7 vs 8.
  • Had to add to app/assets/config/manifest.js
//= link blacklight/manifest.js
  • The public/private checkbox control in the items admin view is broken. When you click the checkbox you get an error: Unable to save the bookmark at this time
  • The shared/flash_msg is complaining about blacklight_config being undefined.
  • _solr_documents_features_block.html.erb is trying to use the truncate helper and I think it's weirdly finding the ActiveRecord::ConnectionAdapters::DatabaseStatements.truncate instead which has a different method signature.

corylown avatar Aug 07 '24 18:08 corylown

I will start picking up specific pieces of this issue. I'm not sure if we want the open tasks in the ready column as well but just indicating I am beginning to look at this. Starting with #3069

hudajkhan avatar Sep 11 '24 00:09 hudajkhan

Thanks @hudajkhan! I think the work for this ticket is described by my Aug 7 comment. We know that Blacklight 8 mostly works with Spotlight 4 because DLME is on Blacklight 8. The goal here is mostly to get the tests passing and address any lingering incompatibilities. We can talk more after stand-up, but there are a few different approaches we could take as other work to get CI and the generators working with Blacklight 8.

corylown avatar Sep 11 '24 11:09 corylown

Testing this out. Note: I updated the test template generator to add the //= link blacklight/manifest.js to the internal test app's app/assets/config/manifest.js file. I had to manually update the internal test app's Gemfile to use Blacklight 8.

  • The checkmark to toggle public status of item is still throwing the bookmark error mentioned by @corylown above.
  • 13 rspec failures currently, with a few I saw without Blacklight 8 (around Selenium/chromedriver and not finding a modal). Others include the solr documents features block error looking for a truncate method (as noted above), three tests failing because of not finding blacklight_config, breadcrumbs in catalog controller tests that expect "L'" in the text but instead got "L ' " which implies a difference in encoding, and a browse category search which shows the search matches 3 of 6 items as opposed to the expected 1 of 6 items etc.
  • I am not sure where the flash msg error is happening.

hudajkhan avatar Sep 13 '24 22:09 hudajkhan

Here are the test failures that happen only with Blacklight 8.3 for me. (8.4 is also leading to some errors which I will look at separately). I am creating separate issues for these (as well as 8.4 specific issues). For the feature_page_admin_spec issues, it is possible that those are only local for me but I'll leave them here for reference.

  • [x] rspec ./spec/controllers/spotlight/catalog_controller_spec.rb:31 # Spotlight::CatalogController when the user is not authenticated GET show shows the item. https://github.com/projectblacklight/spotlight/pull/3142
  • [x] rspec ./spec/controllers/spotlight/catalog_controller_spec.rb:38 # Spotlight::CatalogController when the user is not authenticated GET show shows the item with breadcrumbs to the browse page https://github.com/projectblacklight/spotlight/pull/3142
  • [x] rspec ./spec/controllers/spotlight/catalog_controller_spec.rb:49 # Spotlight::CatalogController when the user is not authenticated GET show shows the item with breadcrumbs to the feature page https://github.com/projectblacklight/spotlight/pull/3142
  • [x] rspec ./spec/controllers/spotlight/catalog_controller_spec.rb:60 # Spotlight::CatalogController when the user is not authenticated GET show shows the item with breadcrumbs from the home page https://github.com/projectblacklight/spotlight/pull/3142
  • [x] rspec ./spec/features/browse_category_spec.rb:163 # Browse pages with a search field based browse category conducts a search within the browse category. Related PR: https://github.com/projectblacklight/spotlight/pull/3137
  • [ ] rspec ./spec/features/item_admin_spec.rb:41 # Item Administration admin toggles the 'blacklight-private' label
  • [ ] rspec ./spec/features/javascript/feature_page_admin_spec.rb:57 # Feature Pages Adminstration stays in curation mode if a user has unsaved data
  • [ ] rspec ./spec/features/javascript/feature_page_admin_spec.rb:68 # Feature Pages Adminstration stays in curation mode if a user has unsaved contenteditable data
  • [ ] rspec ./spec/features/javascript/feature_page_admin_spec.rb:81 # Feature Pages Adminstration does not update the pages list when the user has unsaved changes
  • [ ] rspec ./spec/views/shared/_header_navbar.html.erb_spec.rb:8 # shared/_header_navbar has nav links . Related PR: https://github.com/projectblacklight/spotlight/pull/3136
  • [ ] rspec ./spec/views/spotlight/pages/show.html.erb_spec.rb:46 # spotlight/pages/show when rendering with layout does not double-escape HTML entities in the HTML title . Related PR: https://github.com/projectblacklight/spotlight/pull/3136
  • [ ] rspec ./spec/views/spotlight/pages/show.html.erb_spec.rb:50 # spotlight/pages/show when rendering with layout includes analytics reporting . Related PR: https://github.com/projectblacklight/spotlight/pull/3136
  • [x] rspec ./spec/views/spotlight/sir_trevor/blocks/_solr_documents_features_block.html.erb_spec.rb:48 # spotlight/sir_trevor/blocks/_solr_documents_features_block.html.erb without a primary caption falls back to the regular document title for the caption. Related PR: #3131

hudajkhan avatar Sep 18 '24 21:09 hudajkhan

In this comment, I will track Blacklight 8.4 test failures:

  • [ ] Handle missing Blacklight SkipLinkComponent

hudajkhan avatar Sep 18 '24 21:09 hudajkhan

Reopening just for tracking the items above.

hudajkhan avatar Sep 20 '24 20:09 hudajkhan