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

Collection Level "Assign role" API error message should specify: User already has this role, and not include "dataset" (which is incorrect) #11191

Open
sbarbosadataverse opened this issue Jan 27, 2025 · 1 comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours. Type: Bug a defect

Comments

@sbarbosadataverse
Copy link

sbarbosadataverse commented Jan 27, 2025

What steps does it take to reproduce the issue?
When using this Native AP

If the user already has the role, the error message reads "{"status":"ERROR","message":"User already has this role for this dataset"}% instead of "collection"

  • When does this issue occur?
    When a user already has the role type in question

  • Which page(s) does it occurs on?

  • What happens?

  • To whom does it occur (all users, curators, superusers)?
    superuser, curators, anyone with permission to run this script on a collection level

  • What did you expect to happen?

correct error message: "User already has this role" remove "datasets" and no need to mention dataverse or collection

Which version of Dataverse are you using?

most recent

Any related open or closed issues to this bug report?
NO

Screenshots:

@sbarbosadataverse sbarbosadataverse added the Type: Bug a defect label Jan 27, 2025
@sbarbosadataverse sbarbosadataverse changed the title Collection Level "Assign role" API error message should specify: User already has this role, and not include "dataset" (which is the bug) Collection Level "Assign role" API error message should specify: User already has this role, and not include "dataset" (which is incorrect) Jan 27, 2025
@pdurbin
Copy link
Member

pdurbin commented Jan 27, 2025

I tested this as of a789342 and can easily reproduce it with the following test by duplicating the call to grant a role on a collection:

% git diff src/test
diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java
index c1273fba9f..0f8b8b79bd 100644
--- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java
+++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java
@@ -299,6 +299,10 @@ public class DatasetsIT {
         Response grantRole = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.DS_CONTRIBUTOR, AuthenticatedUsers.get().getIdentifier(), apiToken);
         grantRole.prettyPrint();
         assertEquals(OK.getStatusCode(), grantRole.getStatusCode());
+        grantRole = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.DS_CONTRIBUTOR, AuthenticatedUsers.get().getIdentifier(), apiToken);
+        grantRole.prettyPrint();
+        assertEquals(OK.getStatusCode(), grantRole.getStatusCode());

This checking for duplicate roles is relatively new, added in 6.3 with this PR (my bad for not catching this in code review):

As @sbarbosadataverse said, the easiest fix is to shorten the message to "User already has this role". One thing to note however, is that in the test above we are assigning a group rather than a user to a role. So "User or group already has this role" is a bit more accurate. We could also vary the messages, of course ("Group already has this role on this file" or whatever), but perhaps that's overkill. We could even simply say "Role has already been granted." 🤷

I'm going to go ahead and give this a size of 3.

@pdurbin pdurbin added the Size: 3 A percentage of a sprint. 2.1 hours. label Jan 27, 2025
@sbarbosadataverse sbarbosadataverse moved this to SPRINT READY in IQSS Dataverse Project Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours. Type: Bug a defect
Projects
Status: SPRINT READY
Development

No branches or pull requests

2 participants