Groups - Improve Group Identifiers in Permissions UI

[mheppler]

Cleanup Group identifiers on assign role autocomplete input dropdown. Some examples we found in demo: “&explicit/161-”, “&shib/1”.

It would be neat if we had a format like “&group-identifier”, like we have for users, “@user-identifier”.

Is it possible to have unique identifier for all users and groups?

[eaquigley]

@mheppler I checked in demo and am no longer seeing this, it also appears that we have moved away from & and are now doing : for group identifiers (can be seen in screenshot)

@kcondon I'm passing this to you to close as it no longer seems to be an issue.

[pdurbin]

can be seen in screenshot

The only group type seen in the screenshot above is the one with the colon but the ugly ones are still with us (such as "&explicit/1-firstGroup" below):

[eaquigley]

@pdurbin Gross. Why are two different punctuation marks (: and &) being used for groups?

[pdurbin]

That is a question for @michbarsinai . The code looks like this:

public RoleAssignee getRoleAssignee(String identifier) {
    switch (identifier.charAt(0)) {
        case ':':
            return predefinedRoleAssignees.get(identifier);
        case '@':
            return authSvc.getAuthenticatedUser(identifier.substring(1));
        case '&':
            return groupSvc.getGroup(identifier.substring(1));
        default:
            throw new IllegalArgumentException("Unsupported assignee identifier '" + identifier + "'");
    }
}

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

Line 57 in e6df62a

public RoleAssignee getRoleAssignee(String identifier) {

[eaquigley]

then i shall pass this back to @michbarsinai, who was originally assigned

[michbarsinai]

That's a design decision. Assignees starting with : are built-in ones, such as :guest, :authenticated-users etc. &XXX are user-defined groups, and @YYY are users.
This also solves the issue of preventing names of user-defined assignees from clashing with built-in ones. For example, a user cannot call herself :guest. She can only be @guest.

[michbarsinai]

Summary:

1. We can remove the : prefix, but then face another issue of name clashes (@scolapasta ruled in favor of keeping :, if memory serves).
2. Current group name is composed of & Group Provider / Group-id. We can replace this structure with &``GroupName and add another lookup table, connecting GroupName with the provider and it's internal id. That's another database call, though.

Over to Liz as this is a requirement issue.

[pdurbin]

I can get behind @pdurbin meaning a person or organization (@IQSS) because we see it here on GitHub and also on Twitter (huh, and Facebook apparently). Using an ampersand for a group is unique to Dataverse, I believe. It's a little ugly in my opinion but my only suggestion is to use an "@" sign for both users and groups (and maybe this is a terrible idea). Or maybe internally (and in the API) groups could continue to have an "&" (if necessary) but the user never sees it in the GUI ("don't make me think"). If we're going to continue showing ampersands in the GUI we should explain they're for groups since it's uncommon and non-obvious.

[posixeleni]

👍 for @pdurbin

[pdurbin]

"we did discuss several issues of not displaying the @ (easy to strip out) and handling the other prefixes (& and :) in a different manner" -- @mcrosas in #2046

[michbarsinai]

Some comments:

• The RoleAssignee interface defines a method of getting a RoleAssigneeDisplayInfo object[1]. We might need to re-define its fields a bit, but the basic infrastructure of displaying different types of role assignees (group or users) is there.
• I think & makes sense for groups, as it connects parties ("Phil & Ted", "Joe & Co." etc.).
• Role assignee identifiers currently hold all the information needed to retrieve the role assignee. Beautifying them in a way that results in another database call is understandable, but we need to consider the performance impact.
• But we may not need to display the identifier, if we use the RoleAssigneeDisplayInfo objects.

Maybe we should let @mheppler design a "dream" interface about role assignees, and then see if we can adapt RoleAssigneeDisplayInfo to support it?

[1] https://github.com/IQSS/dataverse/blob/4.3/src/main/java/edu/harvard/iq/dataverse/authorization/RoleAssignee.java#L26

[mheppler]

Closing as a "design decision" was made and implemented. No other comments have been gathered on this since it has been released.