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.
| else { | ||
| foreach ($roles as $key => $item) { | ||
| if (!empty($key)) { | ||
| $str .= $key.','; |
There was a problem hiding this comment.
On the first role, this causes an error as you can't add text to a an undefined variable. Easiest fix to this is to define this above the foreach (e.g. $str = '';)
[!] Warning: Undefined variable $str
in roles_bee_callback() (line 126 of /app/commands/role.bee.inc).
| 'aliases' => array('rcrt'), | ||
| 'bootstrap' => BEE_BOOTSTRAP_FULL, | ||
| 'examples' => array( | ||
| 'bee role-create editor' => bt("Add role 'editor'."), |
There was a problem hiding this comment.
As editor is a default role, perhaps it would be better to have a different role name (e.g. manager)
| /** | ||
| * Make sure that the role-add-perm command works. | ||
| */ | ||
| public function test_role_add_perm_command_works() { |
There was a problem hiding this comment.
This looks odd in the test result.
| public function test_role_add_perm_command_works() { | |
| public function test_role_add_permission_command_works() { |
| /** | ||
| * Make sure that the role-remove-perm command works. | ||
| */ | ||
| public function test_role_remove_perm_command_works() { |
There was a problem hiding this comment.
This looks odd in the test result.
| public function test_role_remove_perm_command_works() { | |
| public function test_role_remove_permission_command_works() { |
| /** | ||
| * Make sure that the role-delete command works. | ||
| */ | ||
| public function test_role_delete_perm_command_works() { |
There was a problem hiding this comment.
This looks odd in the test result.
| public function test_role_delete_perm_command_works() { | |
| public function test_role_delete_permission_command_works() { |
| @@ -15,6 +15,7 @@ | |||
| <!-- There's no Status command test, since that command is used in other tests already. --> | |||
| <file>backdrop/UpdateCommandsTest.php</file> | |||
| <file>backdrop/UserCommandsTest.php</file> | |||
| <file>backdrop/RolesCommandsTest.php</file> | |||
There was a problem hiding this comment.
Please place in the list alphabetically so between Projects and State
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