Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD] Upgrade to Blacklight 8 #4230

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

mjgiarlo
Copy link
Member

@mjgiarlo mjgiarlo commented Oct 11, 2023

Supersedes #2857
Closes #2857

TO-DO

  • Fix missing pagination component on show page. After running a search, click a result and notice that the << Previous | 1 of ... | Next >> component is rendering as a couple buttons now1
  • Fix some busted modals (e.g., tag editing and Manage Release on the item show page)
  • Successfully run integration tests. All pass but one2
  • Manually test this in a deployed env
  • Figure out if the behavior we're altering in a Blacklight.Modal.modalAjaxClickLink monkeypatch is a bug in Blacklight's modal JS or something we've got mucked up
    • Does not block merging this PR. It's a small monkeypatch and we can prioritize iterating on that change.
  • Explore whether we can remove custom EditModalComponent now that we're on Blacklight 8
    • No, our custom modal component operates via Turbo, unlike Blacklight's built-in modal component.
  • Figure out why the document_presenter helper override isn't functioning as expected
    • Fixed.

Why was this change made?

Keep pace with dependencies.

How was this change tested?

CI, local

Footnotes

  1. Screenshot from 2023-10-11 14-20-01

  2. Here is the test that fails

    1) Argo rights changes result in correct Access Rights facet value is expected to have text "Items successfully registered."       
    Failure/Error: select view, from: 'item_view_access'                                                                            
    Capybara::ElementNotFound:
       Unable to find select box "item_view_access" that is not disabled within #<Capybara::Node::Element tag="turbo-frame" path="/HTML/BODY[1]/MAIN[1]/DIV[2]/SECTION[1]/DIV[2]/DIV[2]/DIV[1]/TABLE[1]/TBODY[1]/TR[5]/TD[1]/TURBO-FRAME[1]"> and Unable to find input box with datalist completion "item_view_access" that is not disabled within #<Capybara::Node::Element tag="turbo-frame" path="/HTML/BODY[1]/MAIN[1]/DIV[2]/SECTION[1]/DIV[2]/DIV[2]/DIV[1]/TABLE[1]/TBODY[1]/TR[5]/TD[1]/TURBO-FRAME[1]">
     # ...
     # ./spec/features/access_indexing_spec.rb:95:in `block in choose_rights'
     # ./spec/features/access_indexing_spec.rb:93:in `choose_rights'
     # ./spec/features/access_indexing_spec.rb:40:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # Capybara::ElementNotFound:
     #   Unable to find select box "item_view_access" that is not disabled within #<Capybara::Node::Element tag="turbo-frame" path="/HTML/BODY[1]/MAIN[1]/DIV[2]/SECTION[1]/DIV[2]/DIV[2]/DIV[1]/TABLE[1]/TBODY[1]/TR[5]/TD[1]/TURBO-FRAME[1]">
     #   /home/mjg/.rvm/gems/ruby-3.2.2@ii/gems/capybara-3.39.2/lib/capybara/node/finders.rb:312:in `block in synced_resolve'
    
     Finished in 1 minute 7.04 seconds (files took 0.72034 seconds to load)
     1 example, 1 failure
    
     Failed examples:
    
     rspec ./spec/features/access_indexing_spec.rb:16 # Argo rights changes result in correct Access Rights facet value is expected to have text "Items successfully registered."
    

@mjgiarlo mjgiarlo force-pushed the blacklight8-take-two-backup branch 6 times, most recently from b6b4fbc to 3231c16 Compare October 11, 2023 21:00
@mjgiarlo mjgiarlo changed the title Test rebase [HOLD] Upgrade to Blacklight 8 Oct 11, 2023
@mjgiarlo mjgiarlo marked this pull request as ready for review October 11, 2023 21:06
@mjgiarlo mjgiarlo mentioned this pull request Oct 11, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant