Support Blacklight 8

[corylown]

This may have mostly been completed via #2853

Relates to:

• Blacklight 8 builds are not actually using blacklight 8. #3025
• Install fails on Blacklight 8 #2950
• Allow testing with specific versions of blacklight #2949
• Enable setting blacklight-version #3022

Upgrading to Blacklight 8 Guide

• Use response.documents instead of @document_list #3069
• Specify config.search_state_fields for Blacklight 8 #2966
• Address switch from configured partials to DocumentComponent #3070
• Specify bootstrap version ~>4.0 in the template #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: #3047 (comment)

[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.

[hudajkhan]

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

[corylown]

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.

[hudajkhan]

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]

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.

• rspec ./spec/controllers/spotlight/catalog_controller_spec.rb:31 # Spotlight::CatalogController when the user is not authenticated GET show shows the item. Support html-safe (BL7) and non-html-safe (BL8) versions of the title for breadcrumbs #3142
• 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 Support html-safe (BL7) and non-html-safe (BL8) versions of the title for breadcrumbs #3142
• 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 Support html-safe (BL7) and non-html-safe (BL8) versions of the title for breadcrumbs #3142
• 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 Support html-safe (BL7) and non-html-safe (BL8) versions of the title for breadcrumbs #3142
• 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: setting json solr path to nil to prevent browse category query from s… #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: Replacing header nav partial with header component #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: Replacing header nav partial with header component #3136
• rspec ./spec/views/spotlight/pages/show.html.erb_spec.rb:50 # spotlight/pages/show when rendering with layout includes analytics reporting . Related PR: Replacing header nav partial with header component #3136
• 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: ensure string is passed to truncate since errors with integers #3131

[hudajkhan]

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

• Handle missing Blacklight SkipLinkComponent

[hudajkhan]

Reopening just for tracking the items above.