samvera-labs / nurax-pre2023

Vanilla-plus Hyrax app for testing and tire-kicking
https://nurax-dev.curationexperts.com
Other
8 stars 18 forks source link

Deactivating embargo makes Works private #205

Closed rjkati closed 6 years ago

rjkati commented 6 years ago

Reporting for

Descriptive Summary

I have logged in as a Repository Administrator and navigated to the "Manage Embargoes" page. I would like to deactivate an embargo on a Generic Work, which the "Visibility will change to" column says will change to "Institution" after the embargo is deactivated

manage embargoes page

After clicking the "Deactivate Embargo" button and the "Yes please" button, the visibility of the file is changed to "Private", rather than "Institution". private file

Expected Behavior

Visibility of embargoed work should change to the visibility setting indicated in the "Visibility will change to" column after the embargo is deactivated.

julesies commented 6 years ago

verified.

chrisdaaz commented 6 years ago

moved to https://github.com/samvera/hyrax/issues/2792

laritakr commented 6 years ago

The expected behavior of deactivating a lease or embargo is to remove any future changes to the visibility.

Embargo and Lease deactivation always leaves the work in the current visibility state.

This is not a behavioral change from the current release, and the release candidate works as expected based on my testing. This will not be fixed unless you can show behavior different than I just documented.

julesies commented 6 years ago

@laritakr Here is an example of what is happening now. Pretty sure this was not how it worked in 2.0 and either way it's a problem. Here is the example I'm working on

https://nurax.curationexperts.com/concern/generic_works/b5644r54d?locale=en#?c=0&m=0&s=0&cv=0&xywh=-318%2C-2%2C1273%2C438

Before I deactivated the expired embargo:

screen shot 2018-03-16 at 4 01 08 pm

After I deactivated it: the file went from embargo to institution, but the work became Private screen shot 2018-03-16 at 4 01 45 pm

julesies commented 6 years ago

In addition: this page has changed, but I don't have a 2.0 instance to compare with.

https://nurax.curationexperts.com/embargoes?locale=en#expired

The changes I think are happening:

  1. Now when you visit the page, the "Work" is not preselected, only the files.
  2. The word "File" is appearing at the work level.

screen shot 2018-03-16 at 4 07 33 pm

laritakr commented 6 years ago

Thanks for the additional clarification Julie. This isn't what I found in my testing, but I'll do more testing on Monday after my dates are in the past (I rebuilt my app so I'm unable to get earlier than today).

julesies commented 6 years ago

@rjkati can you retest?

rjkati commented 6 years ago

RA 1.18, which manages expired active embargoes, now passes in Firefox.

RA 1.21, which manages all active embargoes, is still working as described in the initial bug report. Based on the conversation above, I'm not sure if it was fixed. However, the current functionality is confusing from a repo-management standpoint as it takes additional steps to bring an embargoed item to the visibility state listed in the "Visibility will Change to" column on the embargo management page. This may be a problem to address in the next release though.

laritakr commented 6 years ago

@rjkati I'm curious if you can clarify what you mean re: RA 1.21? The text on the screen is a bit confusing to me.

The appropriate behavior is that when you cancel an unexpired embargo or lease, it remains in the "before" state. When the embargo or lease is past the expiry date, it should change to the final state. Is this the behavior you were seeing?

rjkati commented 6 years ago

@laritakr , RA 1.21 was the number of the test for which the bug was reported. It's test 21 on the repository administration tab in the testing spreadsheet.

The behavior you describe is what I am seeing. I do have a use case in which I would need to cancel an unexpired embargo and move the work to the "final" state immediately upon the embargoes expiration, which I thought I could accomplish by clicking the "deactivate embargo" button. It seems that to accomplish this task currently, I would need to change the expiry date, wait for the date to pass, then deactivate the embargo. This does not seem intuitive to me, but perhaps this should be addressed in a later release.

laritakr commented 6 years ago

@rjaki I thought that you could just change to a different state directly, without going through the lease/embargo menus to get rid of the lease or embargo when it isn't expired. If that's not true, I am positive that you can just cancel the lease or embargo, and then go back to the work and change the state directly.

I find the text on the screen to not be clear at all. But if we want the option to just expire the embargo or lease rather than to deactivate it, perhaps a second button should be added to each page as a future enhancement.

julesies commented 6 years ago

I did confirm that deactivating an embargo that is expired now works as expected an the work and the files look good! trying to figure out what ticket to write with above comments. i'll look into it a little.

vantuyls commented 6 years ago

reading through all of this: 1) Do we have a second confirmation (apart from larita's) that this is functioning as she describes? 2) Can we get clarifying text on this page to describe the expected functionality?

if we can do 1 and 2, then i think we can close this as an issue for the purposes of the 2.1 release.

Clarifying Text to place on the work or file level embargo management page: If the embargo expiration date has passed, Deactivating the Embargo will set the work or file's visibility to the post-embargo visibility (ie. it will "lift the embargo")

If the embargo expiration date has not passed, Deactivating the Embargo will set the work or file's visibility to the pre-embargo visibility (ie. it will remove the embargo but keep the current visibility)

elrayle commented 6 years ago

@julesies @rjkati I am not certain that we are all talking about the same steps to reproduce the problem. So I went to look at test RA1.18 and RA1.21 to see what they say to do. The copy of the QA tests that I have does not have these tests, so perhaps I am looking at an outdated version.

https://docs.google.com/spreadsheets/d/1rhHdmVkSnDqrL6i1iQbkUF8SzgZAYiKMBqCnvtm-wQI/edit#gid=63084226

Here is what I think is being tested...

image

This is the page where the clarifying text should be added, right?

One step further would be to add a modal that appears only if the expiration date has not yet passed.

OPTION 1: add clarifying text to this page and stay at the pre-embargo state if it is before the embargo expiration date OPTION 2: warn the user that they are removing the embargo early and that it will stay in the pre-embargo state OPTION 3: warn the user that they are removing the embargo early AND give them the choice of being in the pre-embargo state or post-embargo state.

For 2.1.0, I recommend Option 1 and 2. Option 3 can be added to the wish list of improvements for 2.1.1.

chrisdaaz commented 6 years ago

i think clicking 'deactivate embargo' should update the visibility. i hope that gets addressed at some point in the future.

clarifying text that says something like: "Deactivating the embargo does not change the visibility." would be helpful in the interim.

i don't understand the use-case where someone would want to deactivate an embargo and keep the work in the same visibility state as it was during the embargo. that seems to defeat the purpose of deactivating the embargo in the first place.

elrayle commented 6 years ago

Long term, when the expiration date has not passed, we can show a modal that asks the user which behavior they want.

julesies commented 6 years ago

closing, wrote ticket in hyrax for desired behavior https://github.com/samvera/hyrax/issues/3058