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

Errors and notices missing when nesting collections #2592

Closed
laritakr opened this issue Jan 31, 2018 · 2 comments · Fixed by #2905
Closed

Errors and notices missing when nesting collections #2592

laritakr opened this issue Jan 31, 2018 · 2 comments · Fixed by #2905
Assignees
Labels
bug Collection impacts the Collection part of PCDM Model
Milestone

Comments

@laritakr
Copy link
Contributor

laritakr commented Jan 31, 2018

Descriptive summary

When nesting collections, the notices and errors do not display when adding or removing a parent collection, or adding or removing a subcollection. The notices are filled correctly and passed with the redirect, but are apparently eaten by Turbolinks. (Notices work appropriately for adding a subcollection as a new collection.)

The response after submitModalAjax is:

Turbolinks.clearCache()
Turbolinks.visit(“http://localhost:3000/dashboard/collections/parent_nested?locale=en“, {“action”:“replace”})

Turbolinks needs to be deactivated somehow, to allow the responses to use the controller redirect_to.

Rationale

It is important to have a confirmation of what is happening, particularly in the case where a collection cannot be nested because it would exceed the specified nesting depth. In this situation, nothing happens but there is no message to explain why. At least in the case of a successful nesting, you can see the nested collection on the new view.

Expected behavior

  • In cases of valid nesting, a confirmation notice states that the colllections were nested.
  • In cases of errors, one or more errors state why the collections cannot be nested.

Actual behavior

  • The nesting completes normally through the UI and all specs pass.
  • in /app/controllers/hyrax/dashboard/nest_collections_controller.rb, the appropriate notices and errors are loaded as expected.
  • Confirmation notices do not appear after the redirect.
  • A notice regarding the nesting depth does not appear after the redirect.
    (since the query service was expanded to do appropriate validation, it would be very unlikely that you would be able to produce the other flash errors via the UI without forcing an error in method valid_combined_nesting_depth? in /app/services/hyrax/collections/nested_collection_query_service.rb)

Steps to reproduce the behavior

  • Create several collections of the same nestable collection type.
  • Go to the show page for a collection.
  • Select add to collection, choose a collection, and submit.
  • The page redisplays and shows the newly linked parent.
  • No confirmation notice appears.
  • Select add a subcollection, choose a collection, and submit.
  • The page redisplays and shows the newly linked child.
  • No confirmation notice appears.
  • Chain collections 5 nesting levels deep ("nesting_collection__deepest_nested_depth_isi":5)
  • Select Create new collection as subcollection.
  • Page redisplays, with no changes and no error notice.
@laritakr laritakr added bug Collection impacts the Collection part of PCDM Model labels Jan 31, 2018
@laritakr laritakr self-assigned this Feb 12, 2018
@elrayle elrayle moved this from Ready to In Progress in Collections Extensions Feb 15, 2018
@laritakr laritakr removed their assignment Feb 20, 2018
@laritakr laritakr moved this from In Progress to Ready in Collections Extensions Feb 28, 2018
@elrayle elrayle moved this from Ready to High Priority - targeted for 2.1.0 in Collections Extensions Mar 1, 2018
@elrayle elrayle added this to Blocking - triage in 2.1.0 Release Mar 1, 2018
@elrayle
Copy link
Contributor

elrayle commented Mar 23, 2018

This should also look at addressing messages getting eaten when adding users/groups on the sharing tab. They were recently converted to AJAX processing and no longer display messages.

Related Work: PR #2832

@blancoj blancoj self-assigned this Mar 28, 2018
@elrayle elrayle moved this from Blocking - triage to In Progress - active dev in 2.1.0 Release Mar 29, 2018
@jgondron jgondron self-assigned this Apr 4, 2018
@jgondron
Copy link
Contributor

jgondron commented Apr 4, 2018

I've tracked this down to a collision in behavior with blacklight: projectblacklight/blacklight@4d9290f. The problem that blacklight is solving with this flash.discard isn't a problem in hyrax since turbolinks will override xhr post behavior on the rails and client side to force it to auto follow redirects, which will properly consume flash messages (see https://github.com/turbolinks/turbolinks-rails/blob/master/lib/turbolinks/redirection.rb#L13).

I'm looking into proposing a change to make this behavior configurable in blacklight, since I suspect this will likely become a frequent issue for it with the recent "turbolinks by default" integration in rails.

@blancoj blancoj removed their assignment Apr 4, 2018
jgondron added a commit that referenced this issue Apr 4, 2018
- Added a way to configure blacklight's discard flash behavior and changed the catalog controller template to disable this behavior by default. For applications that don't use turbolinks, they will likely want to remove this config line to restore blacklight's solution to the UX problems mentioned in projectblacklight/blacklight@4d9290f
- Changed NestCollectionsController to inherit from Hyrax::My::CollectionsController. Without this, this controller would not run through the catalog controller's configure_blacklight step and would not get the override for the discard flash behavior
jgondron added a commit that referenced this issue Apr 5, 2018
- Added a way to configure blacklight's discard flash behavior and changed the catalog controller template to disable this behavior by default. For applications that don't use turbolinks, they will likely want to remove this config line to restore blacklight's solution to the UX problems mentioned in projectblacklight/blacklight@4d9290f
- Changed NestCollectionsController to inherit from Hyrax::My::CollectionsController. Without this, this controller would not run through the catalog controller's configure_blacklight step and would not get the override for the discard flash behavior
- Now that the NestCollectionsController inherits from Hyrax::My::CollectionsController, it will redirect to login page if the user is not logged in. Fixed the spec to login as a user before performing any actions.
jgondron added a commit that referenced this issue Apr 9, 2018
- install_generator now adds a skip_after_action to override Blacklight's discard_flash_if_xhr behavior at the ApplicationController level.
@no-reply no-reply added this to the 2.x series milestone Apr 13, 2018
@no-reply no-reply removed this from High Priority - targeted for 2.1.0 in Collections Extensions Apr 13, 2018
@elrayle elrayle moved this from In Progress - active dev to WIP PRs in 2.1.0 Release Apr 17, 2018
2.1.0 Release automation moved this from WIP PRs to Closed Apr 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Collection impacts the Collection part of PCDM Model
Projects
No open projects
2.1.0 Release
  
On nurax
Development

Successfully merging a pull request may close this issue.

5 participants