Standalone login ux

[artfulrobot]

Small steps forward on providing a way to administer users, roles.

This PR provides a SK/FB for listing/editing users. It also reworks the entities for RBAC; moving from separate Role and UserRole entities to a compound roles field on User using a pseudoconstant.

Test updated.

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[artfulrobot]

Also see civicrm/civicrm-buildkit#787

[eileenmcnaughton]

@totten I assume you have seen this

@artfulrobot this is wholly contained within the standalone extension so it's up to you if you want to self-merge this now (or I can) or whether you are wanting to get some pre-merge collaboration

[totten]

@artfulrobot Yeah, mapping-entities like UserRole and RolePermission leave me with some ambivalence. They can handle some edges (vis-a-vis search/batch-update/concurrency) better -- but we don't really need those for Users and Roles. And they come with some cost -- longer incantations, more split-up model.

So I definitely see the case for converting entity UserRole to the field User.roles. 👍

I'm not as keen about converting Role+RolePermission to OptionGroup+OptionValue+RolePermission.

It seems to me that the model would be more intuitive if you had:

• Entity User with field roles (with pseudoconstant pointing to civicrm_role)
• Entity Role with field permissions (with a pseudoconstant derived from a callback function - ultimately hook_permissionList)

Aside: I think there's an interesting conversation to have about defaults and packaging for roles/permissions... But that maybe a separate issue/chat.

[totten]

Aside: I think there's an interesting conversation to have about defaults and packaging for roles/permissions... But that maybe a separate issue/chat.

OK, so how it relates here: suppose you're packaging-up a Role to use on other sites. With most tooling (*.mgd.php, configitems, *.sqldata.php, etc):

• It should be pretty easy to package one Role record with a permissions property.
• It would be more work to package an OptionValue record with a list of related RolePermission records.

[seamuslee001]

@artfulrobot does this support the idea a user could have multiple roles at once?

[artfulrobot]

@artfulrobot does this support the idea a user could have multiple roles at once?

@seamuslee001 yep.

[artfulrobot]

@totten

I hadn't envisaged packaging a role. Can we package CMS roles? I suppose it provides a way to ship sensible defaults, but I always think permissions are best considered case by case.

Entity User with field roles (with pseudoconstant pointing to civicrm_role)

Currently the values from the optionValue stored in User.roles are (string-encoded) integers. I guess this would be the same but pointing to civicrm_role.id.

To confirm my understanding: I don't think this one has any advantage in terms of packaging roles; I think we can package an optionvalue just the same as a role.

Entity Role with field permissions (with a pseudoconstant derived from a callback function - ultimately hook_permissionList)

Permissions are strings. So this means civicrm_role.permissions would become a long bookended value string containing the permissions granted to that role.

Therefore this is simpler to package because the role's permissions aren't in a separate, dependent table with FKs to the newly created role.

Makes sense. OK I'll try moving to that model.

I also like it from an API permissions POV, as it's easier to apply permissions to an entire entity (Role) than it is to a subset of optionvalues (those that belong to the Role optiongroup).

One thing I'm not looking forward to is replacing the role-permissions edit UI. I believe I could do a multiselect (like in the screenshot of user-roles in my previous comment) but that would be really ugly for a huge list that needs explaining. Would you advise that I do a separate angular app for handling that UX or do you think this is something afform can handle (and if so I'm definitely going to need some pointers).

[artfulrobot]

@totten Have a look at this latest, which I think, does things as you suggested.

Everything should be working (ha ha!). And we have a role list + edit functionality now.

I'd like to merge this, if you approve, and work on smaller prs next.

[totten]

@artfulrobot Cool, yes, I think this is a more consistent schema. And I agree about preferring to merge.

FWIW, I also spent some time playing around with it. You probably have a list of notes in your head for things to tackle, but (just in case it helps) here are some of the things that popped out at me:

• "Edit User"
  • Editing various seemed to work. Woot.
  • On the post-submit redirect, the URL got munged a little (dropped the HTTP port). I feel like there was a parallel PR on similar theme, though.
  • "Timezone" field is unclear.
• "Add User"
  • The "Enabled" should be on by default?
  • After submitting, it didn't actually create the user.
• "Edit Role"
  • Huzzah. I edited the label and permissions.
  • On the fence about whether it should allow one to rename an existing role. It probably works. OTOH, trying to think of comparable
    entities/fields... the civicrm_custom_group.name and civicrm_group.name are both initialized from the title (and then become read-only).
• On civicrm/admin/users, there's a checkbox to filter by "Active". If you toggle and watch the data, it's a bit funny. (Perhaps because checkbox
  are visually two options -- On|Off -- but the "Active" filter is logically three options - Any|Active|Inactive).
• Nav Menu
  • Titles are little inconsistent. Maybe call it "Users" and "Roles"?
    * There is a pre-existing submenu for "Users and Permissions" - maybe put the links in there?

[artfulrobot]

Thanks @totten yes I'm aware about those.

#26685

• timezone, UX fix is in the works but bigger issue ←. language also
• add user not working, yeah my FB skills are not there yet, no idea why. (fixed and related pr)
• active by default on new users possibly related afform bug?
• edit role name: meh, not a priority.
• SK active filter, yes I'll remove til I figure out SK better!
• nav menu: agree, good suggestions
• also a small matter of limiting permission to access these screens...!
• edit user needs password setting, changing skills, nicer perms ux

[colemanw]

@artfulrobot I'm not sure if "X" is a very good name for an afform...

[artfulrobot]

Good point! Someone might think we're leaning towards megalomania.

I'll update that in my next standalone pr.

[colemanw]

... or a social network LOL

[colemanw]

@artfulrobot just wait until #27553 is merged or it will conflict.

[colemanw]

@artfulrobot ok merged! I would tweet about it but... you know...