adding unlink to ui and matching link permissions #10689

stevenwinship · 2024-07-16T15:47:51Z

What this PR does / why we need it: Provide dataset unlink functionality to same level of permission/role as dataset link ["Publish Dataset"/Curator]

Which issue(s) this PR closes: #10583

Closes #10583

Special notes for your reviewer:

Suggestions on how to test this: See description in Issue #10583

Does this PR introduce a user interface change? If mockups are available, please link/include them here: Yes

Is there a release notes update needed for this change?: Yes (to be included)

Additional documentation:

coveralls · 2024-07-16T15:52:21Z

coverage: 20.77% (+0.03%) from 20.741%
when pulling ad8cd72 on 10583-dataset-unlink-functionality-same-permission-as-link
into 0d27957 on develop.

DS-INRAE · 2024-07-16T16:08:17Z

Hi and thanks for implementing this feature 🥳
Could it be possible to have a dropdown list of the "allowed" Dataverse. It would be very helpful for users that don't manage a lot dataverse collections.
Example of suggested selector from the metadata editing form:

stevenwinship · 2024-07-16T18:12:17Z

Could it be possible to have a dropdown list of the "allowed" Dataverse.

The dialog is designed to match the Link Dataset dialog. It does show a dropdown of Dataverses that the user has access to but requires the user to enter a character or two to start the filtering.
Additionally the UnLink dropdown will only show the Dataverses that this dataset was linked to.

DS-INRAE · 2024-07-17T10:00:25Z

Understood, thanks 🙂
It would be helpful to change both in the longer run to reduce typing from the user

pdurbin

The screenshots are great. Overall, looks good. I left a few comments and I'm especially wondering about the new "preAuthorized" boolean. 🤔

doc/release-notes/10583-dataset-unlink-functionality-same-permission-as-link.md

pdurbin · 2024-08-01T20:48:28Z

src/main/java/edu/harvard/iq/dataverse/DatasetPage.java

+            logger.severe(msg);
+            msg = BundleUtil.getStringFromBundle("dataset.notlinked.msg") + ex;
+            /**
+             * @todo how do we get this message to show up in the GUI?


If we weren't moving to the SPA, I'd ask for any thoughts on this todo. 😄

src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java

src/main/java/propertyFiles/Bundle.properties

pdurbin · 2024-08-01T20:53:01Z

src/main/java/propertyFiles/Bundle.properties

@@ -2538,6 +2545,7 @@ dataset.registered.msg=Your dataset is now registered.
 dataset.notlinked=DatasetNotLinked
 dataset.notlinked.msg=There was a problem linking this dataset to yours:
 dataset.linking.popop.already.linked.note=Note: This dataset is already linked to the following dataverse(s):  
+dataset.linking.popup.not.linked.note=Note: This dataset is not linked to the following dataverse(s):  


Hmm, I'm wondering when this message appears.

It appears on the Unlink dialog. I see the message is a bad copy paste so I fixed it. I'm wondering if it would be better to hide the unlink button if the dataset isn't linked

src/main/webapp/dataset.xhtml

.../java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetLinkingDataverseCommand.java

…ission-as-link.md Co-authored-by: Philip Durbin <[email protected]>

Co-authored-by: Philip Durbin <[email protected]>

…lishDataset permissions

pdurbin

I'm so glad we're using the permission system now! I left a few suggestions for docs.

doc/sphinx-guides/source/admin/dataverses-datasets.rst

doc/release-notes/10583-dataset-unlink-functionality-same-permission-as-link.md

…ission-as-link.md Co-authored-by: Philip Durbin <[email protected]>

Co-authored-by: Philip Durbin <[email protected]>

pdurbin

Overall, this is a nice improvement. There's a new "Unlink button" (previously only possible via API) and unlink via API no longer requires superuser.

I only tested this lightly but API tests are passing and I'm happy with the code. I'm going to approve this, moving it into Ready for QA.

However, I'd like to point out a few UX things we might want to improve in the future, perhaps when we add this feature to the SPA. @jggautier as our UX guru might want to weigh in as well.

Ever-present button

First, the "Unlink Dataset" button is ever-present, taking up a little real estate on the screen, even for datasets that are not linked to anything. It looks like this (or see the screenshots in the PR):

If you click the "Unlink Dataset" button on a dataset that doesn't have any links, you get a popup telling you you can't unlink anything. It looks like this:

Perhaps instead of this UX we should grey out the "Unlink Dataset" button or even hide it (I forget what our guidelines are and couldn't find anything helpful in the Style Guide). Or maybe both "Link Dataset" and "Unlink Dataset" should move under the "Edit Dataset" dropdown? I'm not sure how often we expect these buttons to be clicked.

Don't make me think - suggesting collections to unlink from

Something else I noticed is that when a dataset IS linked, the popup doesn't let you choose from collections to unlink from. Instead you have to type the name of the collection. It looks like this:

Summary

Again, nice feature. I'll take my UX hat off now and move this along! Approved!

sekmiller · 2024-08-26T15:52:21Z

@stevenwinship I think if we just add and (!empty DatasetPage.dataset.datasetLinkingDataverses) to the render logic around line 526 on dataset.xhtml we can hide the unlink button and avoid some of the ui confusion

stevenwinship · 2024-08-26T21:17:35Z

added the empty check to hide the unlink button.

github-actions · 2024-08-26T21:36:10Z

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:10583-dataset-unlink-functionality-same-permission-as-link

ghcr.io/gdcc/configbaker:10583-dataset-unlink-functionality-same-permission-as-link

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

stevenwinship self-assigned this Jul 16, 2024

stevenwinship added FY25 Sprint 1 FY25 Sprint 1 issues Type: Feature a feature request Size: 10 A percentage of a sprint. 7 hours. labels Jul 16, 2024

This comment has been minimized.

Sign in to view

stevenwinship removed their assignment Jul 17, 2024

cmbz added the FY25 Sprint 2 FY25 Sprint 2 issues label Jul 18, 2024

pdurbin self-assigned this Aug 1, 2024

pdurbin reviewed Aug 1, 2024

View reviewed changes

pdurbin assigned stevenwinship Aug 1, 2024

This comment has been minimized.

Sign in to view

stevenwinship and others added 9 commits August 2, 2024 08:26

adding unlink to ui and matching link permissions

4ef4778

add release notes

4dbb048

Update doc/release-notes/10583-dataset-unlink-functionality-same-perm…

57bb583

…ission-as-link.md Co-authored-by: Philip Durbin <[email protected]>

Update doc/release-notes/10583-dataset-unlink-functionality-same-perm…

9bf7d6c

…ission-as-link.md Co-authored-by: Philip Durbin <[email protected]>

Update src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java

be7dafa

Co-authored-by: Philip Durbin <[email protected]>

Update src/main/java/propertyFiles/Bundle.properties

44f1fe5

Co-authored-by: Philip Durbin <[email protected]>

Update src/main/java/propertyFiles/Bundle.properties

1e1063e

Co-authored-by: Philip Durbin <[email protected]>

Update src/main/webapp/dataset.xhtml

5b031bf

Co-authored-by: Philip Durbin <[email protected]>

change preAuthorized to hasPermission

64053b8

This comment has been minimized.

Sign in to view

fix unlink note

29c4e78

This comment has been minimized.

Sign in to view

pdurbin assigned scolapasta and unassigned pdurbin Aug 5, 2024

scolapasta removed their assignment Aug 7, 2024

remove hasPermission paramerter and change delete link to require Pub…

874ae0b

…lishDataset permissions

This comment has been minimized.

Sign in to view

stevenwinship removed their assignment Aug 12, 2024

pdurbin reviewed Aug 12, 2024

View reviewed changes

Update doc/release-notes/10583-dataset-unlink-functionality-same-perm…

fae99e4

…ission-as-link.md Co-authored-by: Philip Durbin <[email protected]>

This comment has been minimized.

Sign in to view

stevenwinship and others added 2 commits August 13, 2024 13:20

Update doc/release-notes/10583-dataset-unlink-functionality-same-perm…

b9514c3

…ission-as-link.md Co-authored-by: Philip Durbin <[email protected]>

Update doc/sphinx-guides/source/admin/dataverses-datasets.rst

5d82a80

Co-authored-by: Philip Durbin <[email protected]>

This comment has been minimized.

Sign in to view

pdurbin self-assigned this Aug 23, 2024

pdurbin approved these changes Aug 23, 2024

View reviewed changes

pdurbin removed their assignment Aug 23, 2024

sekmiller self-assigned this Aug 26, 2024

only show unlink if dataset can be unlinked

ad8cd72

sekmiller merged commit 34bfb1b into develop Aug 27, 2024
13 checks passed

pdurbin added this to the 6.4 milestone Aug 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding unlink to ui and matching link permissions #10689

adding unlink to ui and matching link permissions #10689

stevenwinship commented Jul 16, 2024 •

edited

Loading

coveralls commented Jul 16, 2024 •

edited

Loading

This comment has been minimized.

DS-INRAE commented Jul 16, 2024

stevenwinship commented Jul 16, 2024 •

edited

Loading

This comment has been minimized.

DS-INRAE commented Jul 17, 2024

pdurbin left a comment

pdurbin Aug 1, 2024

pdurbin Aug 1, 2024

stevenwinship Aug 2, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pdurbin left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pdurbin left a comment

sekmiller commented Aug 26, 2024

stevenwinship commented Aug 26, 2024

github-actions bot commented Aug 26, 2024

adding unlink to ui and matching link permissions #10689

adding unlink to ui and matching link permissions #10689

Conversation

stevenwinship commented Jul 16, 2024 • edited Loading

coveralls commented Jul 16, 2024 • edited Loading

This comment has been minimized.

DS-INRAE commented Jul 16, 2024

stevenwinship commented Jul 16, 2024 • edited Loading

This comment has been minimized.

DS-INRAE commented Jul 17, 2024

pdurbin left a comment

Choose a reason for hiding this comment

pdurbin Aug 1, 2024

Choose a reason for hiding this comment

pdurbin Aug 1, 2024

Choose a reason for hiding this comment

stevenwinship Aug 2, 2024

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pdurbin left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pdurbin left a comment

Choose a reason for hiding this comment

Ever-present button

Don't make me think - suggesting collections to unlink from

Summary

sekmiller commented Aug 26, 2024

stevenwinship commented Aug 26, 2024

github-actions bot commented Aug 26, 2024

stevenwinship commented Jul 16, 2024 •

edited

Loading

coveralls commented Jul 16, 2024 •

edited

Loading

stevenwinship commented Jul 16, 2024 •

edited

Loading