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

Deactivating Leases from Manage Leases dashboard tab does not work #4516

Closed
mcdowney opened this issue Sep 11, 2020 · 2 comments · Fixed by #4538
Closed

Deactivating Leases from Manage Leases dashboard tab does not work #4516

mcdowney opened this issue Sep 11, 2020 · 2 comments · Fixed by #4538
Assignees

Comments

@mcdowney
Copy link

Descriptive summary

Attempting to deactivate expired leases from the Manage Leases section of the dashboard UI does not succeed in deactivating the leases.

This is happening on 3.0.0.pre.rc2 in Chrome, Safari and Firefox.

Rationale

This seems to have been introduced into the 3.0 release candidate since the most recent round of testing, and was discovered pursuant to test RA_1.19 in the testing script.

Expected behavior

Selecting one or more Expired Active Leases from the "Expired Active Leases" tab and clicking on "Deactivate Leases for Selected" should remove the embargoes from the selected work(s).

Actual behavior

The green modal appears, advising user to "Select something first", but the works remain in the list of Expired Active Leases. Works scheduled to revert to private after lease expiration remain public.

Steps to reproduce the behavior

  1. As an admin user, navigate to Dashboard > Tasks > Manage Leases.
  2. Click on "Expired Active Leases"
  3. Select one or more expired leases to deactivate.
  4. Click on "Deactivate Leases for Selected"
  5. Find the works still in the Expired Active Leases list

Related work

Link to related tickets or prior related work here.

jeremyf added a commit that referenced this issue Oct 7, 2020
Prior to this commit, when a user changed the Embargo or Lease in the
UI (and by extension via the ActorStack), Hyrax would not persist those
changes.

With this change, we ensure that when the visibility changes, hyrax
updates the embargo and lease.

In consultation with @no_reply, we believe that this should be handled
further upstream (e.g. within `Hydra::AccessControls`).  We also believe
that there was an upstream change that broke the previous correct
behavior.

We are attempting to track that down.

Closes #4534, #4517, #4516, #4515
@jeremyf jeremyf self-assigned this Oct 7, 2020
jeremyf added a commit that referenced this issue Oct 7, 2020
In 241cbed we introduced changes to
address accessibility issues reported in #3966.  However, the introduced
change broke the HTML form field names and the assumed parameters for
the [batch processing][1].

This commit reverts that change, but it does not address the
accessibility concern raised in #3966.  That is for another commit, as I
don't know if that other commit is the correct path forward.

Closes #4516
Closes #4515

[1]:https://github.com/samvera/hyrax/blob/3c7258c95c1fe6f0fe1537078f3f5f458f211989/app/controllers/concerns/hyrax/collections/accepts_batches.rb#L20-L28
jeremyf added a commit that referenced this issue Oct 7, 2020
Prior to this commit, when a user changed the Embargo or Lease in the
UI (and by extension via the ActorStack), Hyrax would not persist those
changes.

With this change, we ensure that when the visibility changes, hyrax
updates the embargo and lease.

In consultation with @no_reply, we believe that this should be handled
further upstream (e.g. within `Hydra::AccessControls`).  We also believe
that there was an upstream change that broke the previous correct
behavior.

We are attempting to track that down.

Closes #4534, #4517, #4516, #4515
jeremyf added a commit that referenced this issue Oct 7, 2020
In 241cbed we introduced changes to
address accessibility issues reported in #3966.  However, the introduced
change broke the HTML form field names and the assumed parameters for
the [batch processing][1].

This commit reverts that change, but it does not address the
accessibility concern raised in #3966.  That is for another commit, as I
don't know if that other commit is the correct path forward.

Closes #4516
Closes #4515

[1]:https://github.com/samvera/hyrax/blob/3c7258c95c1fe6f0fe1537078f3f5f458f211989/app/controllers/concerns/hyrax/collections/accepts_batches.rb#L20-L28
@rjkati
Copy link

rjkati commented Mar 25, 2021

Deactivating leases using instructions above now displays an error message in iOS Safari and Chrome:

Image from iOS (1)

https://nurax-dev.curationexperts.com/leases?locale=en

@no-reply
Copy link
Member

this latest seems like a new bug. can we open a new ticket for it?

no-reply pushed a commit that referenced this issue Mar 25, 2021
in a follow-up to #4516, it became clear that in some cases objects that are
neither `WorkBehavior` nor `Hyrax::Resource` are being passed to the visibility
propagator.

when, and what kinds of objects? this should help us find out.
no-reply pushed a commit that referenced this issue Mar 25, 2021
in a follow-up to #4516, it became clear that in some cases objects that are
neither `WorkBehavior` nor `Hyrax::Resource` are being passed to the visibility
propagator.

when, and what kinds of objects? this should help us find out.
no-reply pushed a commit that referenced this issue Mar 25, 2021
in a follow-up to #4516, it became clear that in some cases objects that are
neither `WorkBehavior` nor `Hyrax::Resource` are being passed to the visibility
propagator.

when, and what kinds of objects? this should help us find out.
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 a pull request may close this issue.

4 participants