Dataverse: Curator should be able to edit a Dataset that is "In Review"

[sekmiller]

When "In Review" was not considered to be a lock in the same way that File Ingest was a lock(versions prior to the lock redesign), a curator (or someone with dataset publish and edit permission) could edit a dataset that was "In Review". (That is, the edit dataset button was not disabled for them.)

To fix the disabled tag on the edit dataset button so that if the lock reason is in review disabled is false for users with Publish Dataset permission.

[tlchristian]

For administrators, the edit and bulk download is disabled. Curators no longer can see the "in review" datasets. Not sure why this is, but doesn't seem to work for data curation workflows.

[sbarbosadataverse]

Admins/curators should be the only ones with the ability to edit a dataset after it's been submitted for review

Changing as I described above would correct the workflow for Harvard's installation.

[pdurbin]

@pameyer previously you were commenting on #3172 for work in this area, I believe. At this point I think that issue can be closed.

[pdurbin]

Here are my notes from sprint planning on Wednesday afternoon:

"In the command, check for a lock but override and allow editing if user has edit permission and publish permission." I believe someone said it was a certain kind of lock to check for.

[pdurbin]

In 1f1a79e I added an API test to assert the current behavior. A curator can't edit a dataset because it's locked. Oddly, however, the check for a lock is in the SWORD code. I thought the check for a lock would be in the UpdateDatasetCommand. I discussed this with @sekmiller and will keep looking.

[pdurbin]

As I mentioned at standup this morning, I added some tests in 445e1ed that (unfortunately) assert that authors can continue to edit metadata while datasets are in review if the native API is used (heads up to @pameyer about this). I was told that @michbarsinai probably implemented the new logic that prevents authors from editing metadata via GUI so it was probably added in pull request #4116. If there was an issue about preventing authors from editing datasets while in review, I can't find it. Perhaps it's #3172 but it was never put through QA due to scope creep.

[pdurbin]

In 830fd97 I took a look at the Edit button on the dataset page and made it so that people who can publish can still edit the metadata. However, it's a confusing user experience for these people who are editing metadata while it's in review because you see both of these messages at once:

• Success! – The metadata for this dataset has been updated.
• Dataset Locked This dataset has been submitted for review.

Here's a screenshot:

I'm going to flag this for UI/UX attention and try to pull in @mheppler @TaniaSchlatter @dlmurphy or @jggautier . Please note that #3172 is highly related.

Even more problematic than the above is that the rendering of that button is the only thing that's preventing authors from editing metadata. That is to say, if I put a hack in to make is so authors can click the Edit button, authors are also able to edit the metadata just like curators. This makes sense because as I reported in my last comment, in Dataverse 4.8 and 4.8.1 authors are allowed to edit metadata via API but not the GUI.

@djbrooke I could use help with the definition of done on this issue. I feel like a stab was made at #3172 but the job was never quite finished. This issue no longer feels like a 3. Please advise.

[pdurbin]

I just created pull request #4208 and am putting this issue into Code Review at https://waffle.io/IQSS/dataverse

Authors are now prevented from editing metadata at the command level. Somewhat strangely to me, the native API and the GUI use different commands.

If I made a small revision to the API Guide I could put #3777 through QA as well since I was testing metadata edits via the native API (and added a convenience method for testing the API via REST Assured).

[pdurbin]

it's a confusing user experience for these people who are editing metadata while it's in review

@mheppler asked me to create a separate issue for this, so I did: #4209

[pdurbin]

@scolapasta noticed SwordUtil.datasetLockCheck and it's still being called in the SWORD code that:

• deletes a dataset
• deletes a file
• adds files

If not sure if file handling is in scope for this issue or not. (Does locking allow files to be added or removed?) I'm almost afraid to write the API tests.

[pdurbin]

I'm closing #4209 because @dlmurphy and @jggautier came by with a suggestion that seems like an improvement to me, a simple wording change that I implemented in 31043c4. Here's a screenshot:

[pdurbin]

@sekmiller and I have been talking to @landreev and @djbrooke

We're thinking we'd like to keep getting to the more complete solution of allowing curators to have complete control over a dataset in terms of editing metadata and adding/removing files.

To do list:

• [x] On the dataset page, have "Upload Files" button use same render logic that we've been adding. Currently it's disabled for curators.
• [x] On the file page, make sure the files can be edited, such as metadata like description.
• [x] When a dataset is in review a curator should be able to upload a tabular file without kicking the dataset out of the in review state. Fixed as of 5a53fa0

[djbrooke]

Thanks @pdurbin and @sekmiller for the continued work on this. Editing the metadata is the big piece of this and we are OK rolling that out independently, but if we want to keep investigating that's OK as well.

[pdurbin]

@michbarsinai clued us in that he's already make the change from OneToOne to OneToMany in pull request #4165:

Here's a link to the code: https://github.com/IQSS/dataverse/pull/4165/files#r145413886

I just updated the to do list above with the plan for who's doing what. @sekmiller is looking into render logic and I plan to make a new combo branch.

[pdurbin]

As discussed in standup, there are merged conflicts between branch 4124-workflow-clean-rollback (pull request #4165) and branch 4139-curator-edits-in-review (pull request #4208) and I have just resolved conflicts between the two branches and created a new branch called 4124-4139-locks that is now pull request #4216. Both #4124 and #4139 will be tested against this new pull request. I'll close the old pull requests. There was a bug in pull request #4165 where the lock was not being removed when a dataset was returned to the author but this is fixed in the new pull request.

@sekmiller and I plan to poke around at the new branch a bit and then move both issues to QA. The todo list above is done but I did find one issue:

• [ ] When a dataset is in review and a curator uploads a tabular file, the blue "Ingest in progress" never goes away. You have to refresh the page.

I'm not sure if we can live with this issue or not. I haven't dug into it yet.

[pdurbin]

@sekmiller and I are done hacking on pull request #4216 so I'm moving this issue to QA.

We decided to punt on the "ingest in progress" issue since there's already an issue tracking it: #3491

We discussed how a ConcurrentModificationException is possible but it feel like such an edge case that for now we simply wrote a test to exercise it at fa3bc31 if/when we need to address it.

I'd like to remind everyone that #3172 is quite related so it wouldn't hurt to read what was asked for there.

Above, @tlchristian mentioned "bulk download" but this issue is more about curators being able to edit so we didn't work on this at all. It felt out of scope.

To test, have an author put a dataset in review. The dataset should be locked from edits by the author (#3172 uses the term "freeze" for this) from both API and UI. From both API and UI, a curator should be able to:

• edit dataset metadata (SWORD and native but see #3777)
• add files (SWORD and native)
• delete files (SWORD only, see #3419)
• change file metadata (we didn't test the API)
• delete the dataset draft ( @sekmiller asked @sbarbosadataverse about this)

Please note that the SWORD code previously checked for locks but this was removed to fall through to the centralized logic that was added to commands.

Finally, if pull request #4195 gets merged first, there's a good chance there will be merge conflicts, so please ping a developer to resolve them, ideally before testing begins.

Moving to Code Review at https://waffle.io/IQSS/dataverse

[sekmiller]

@pdurbin Actually, I fixed the Bulk Download issue. It was caught up in the same "is InReview really a lock for Curators?" so I applied the same render logic to the download button as the edit buttons.

[kcondon]

Issues:
[x] Once dataset is submitted, contributor can download individual files but not download all.
[x] Once dataset is submitted, add+edit metadata button on file landing page is not grayed out for contributor (but correctly does nothing)
[x] Once dataset is submitted, edit terms requirements is disabled for curator.
[x] Ingest in progress notice does not disappear, spinner continues periodically until page refresh, if upload tabular file as curator while in review. Works when not in review (this seems different than #3491 since in progress was removed in that ticket, only explore button did not appear).
[x] Minor UI: batch edit select list works but contents is grayed out when curator and dataset is in review.
[x] Set thumbnail for image file not working, changes image to default icon.
[x] Ingest spinner runs during in review for curator, even when nothing new/ingestible was uploaded. Some extra logging every time spinner runs and spinner never goes away even when an ingest finishes.
[ ] Spinner happens a couple times after page load when curator and dataset in review even without a file ingesting

[sekmiller]

All of the issues above have been addressed in my latest check-in except for the second one and the ingest spinner thing.
Mike is taking a look at why disabling the button does not gray it out.

[djbrooke]

Nice! Thanks @mheppler and @sekmiller!

[mheppler]

Gave @sekmiller a code snippet to render a disable style class into the h:outputLink.

[kcondon]

OK, I've retested the fixes and updated the list above. Please review.