Roles en permissions commands #322
Conversation
Apply typo corrections, fix phpcs errors, and improve wording.
Apply phpcs and wording suggestions
There was a problem hiding this comment.
Hi @nico8948
Firstly, please accept my apologies for leaving this so long. On the whole this looks look but a warning for the roles command, a few minor improvements, and some presentation issues that we need to to address:
permissions
The list of all permissions is a bit unwieldy as a table. Firstly, could we limit it to modules with permissions, and ignore the ones without? Secondly, I think how you have done it when the module option is used is much better. Perhaps, there could be a simple heading for the module followed by this list. Perhaps:
NODE
Permissions: 'bypass node access','administer content types','administer nodes', etc
roles
As above
Tests will probably need amending when these changes are made.
There was a problem hiding this comment.
Hi @nico8948
I've proposed a move away from both message based output and tabular output for permissions and the same will be true for roles .
I've proposed a number of changes that handle different scenarios and will provide the information in an easier to read format.
I've also requested that variables are named in full to be more meaningful rather than shortened words and shortened concatenated phrases. Hopefully this will be clear what's needed in other functions.
Thanks for your patience.
Co-authored-by: Martin Price <[email protected]>
Co-authored-by: Martin Price <[email protected]>
Co-authored-by: Martin Price <[email protected]>
There was a problem hiding this comment.
Hi @nico8948 - please can you apply same principles to roles_bee_callback() that I suggested (and you accepted) for permissions_bee_callback(). Namely:
- output as text rather than table or
bee_message - exception checking - check if role exists, error if not; if no permissions for role, then error and exit
- comments throughout so it is easier to follow
- Will need a corresponding change to
test_roles_command_works()
Thank you. I think we're almost there.
Co-authored-by: Martin Price <[email protected]>
Co-authored-by: Martin Price <[email protected]>
Co-authored-by: Martin Price <[email protected]>
Co-authored-by: Martin Price <[email protected]>
commands/role.bee.inc
Outdated
| } | ||
| bee_message(bt("The !role role has the following permissions granted: !permissions", array( | ||
| '!role' => $options['role'], | ||
| '!permissions' => $permissions, | ||
| ))); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
commands/role.bee.inc
Outdated
| } | ||
| else { | ||
| $output = array(); | ||
| foreach ($roles as $role => $value) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
I really don't know what the issue was? sorry |
@nico8948 - if you look on this page, you can see those comments in context, with the original feedback: |
|
ok? |
Co-authored-by: Martin Price <[email protected]>
|
I'll drink a beer to that... |
Fixes #321
Added role.bee.inc for:
ROLES
permissions List all permissons of the modules.
pls, permissions-list
role-add-perm Grant specified permission(s) to a role.
rap
role-create Add a role.
rcrt
role-delete Delete a role.
rdel
role-remove-perm Remove specified permission(s) from a role.
rrp
roles List all roles with the permissions.
rls, roles-list