Make "Roles" a tab of "admin/people" instead of "admin/people/permissions"

+100

Other than not being able to find Roles when I first went looking in D7, I now have to do an extra click to get to it.

I know that I mentioned this to @Bojhan once, and he had what seemed at the time to be a reasonable explanation for the change, so tagging.

I agree with this one too.

Same problems as #1

Forgot to change the menu item argument index... and that's what happens when you upload w/o testing :)

I couldn't agree more with moving the Roles tab. The patch worked great for me, and the path "admin/people/permissions/roles" is nowhere to be found in the tree after patching. I'd say this is done.

Still needs usability review.

I think this needs some testing, we have few to no data around how people use these screens. We also know that secondary navigation has serious visibility issues that might have contributed to this.

Marked #1316942: Move "Roles" LOCAL_ACTION to MENU_TAB as duplicate

Ah. I have another duplicate of this around somewhere. Edit: #884086: Roles are quite hidden at the moment; closing that one as a duplicate.

"Roles" is not logically a child of "permissions." I think the explanation in #11 is quite apt. Also, as David Rothstein pointed out in the previous issue, you can specify roles that can view a block on block administration, and it has nothing to do with permissions. Similarly, access control modules will configure access rules for roles independently of permissions.

Usablity testing is a good idea, but I feel strongly that this is a very good change.

Here is a reroll of Everett's patch for the new directory structure; no other changes.

Screenshot of the UI with this patch applied:

• Roles are not "secondary" to permissions as folks have already explained in this issue. Both UI-wise and technically, roles are not a subset/subtask of permissions. Most of the time, you need to create the roles first before it even makes sense to start trying to configure permissions. Even if secondary navigation was actually visible ;) that's still not the right place to put roles.
• There's no downside to making 'Roles' more visible as a primary navigation link. It's not like 'People' is overflowing with primary links.
• The patch in #15 is good, no visual problems upon review, bot is happy.
• I can't imagine usability testing is going to contradict the assertion that people will be more able to properly configure permissions and roles if they can more obviously find the Roles page. I don't see why this needs formal usability testing.

Therefore, RTBC. ;)

Bah, patch no longer applies (not that surprising). I'll do a quick re-roll. Stay tuned.

Bot's still happy. I'm not RTBC'ing my own patch here... it's really just xjm's patch with new line numbers and such. ;)

Still needs testing, we don't just test for visibility of one setting in a test - otherwise we can just put it as a button on the front page :). One thing more important, might make another thing less important.

I am open to letting it - without testing, as this is something we can easily test/adjust after the fact and we can't afford to hold up tiny issues like these atm.

I think this is a big improvement and would like to commit it. The patch doesn't apply though.

vortex:drupal-head dries$ git apply ../f.patch
error: core/modules/user/user.test: No such file or directory

The tests were moved to /core/modules/user/lib/Drupal/user/.

#19: 1262812-19.move-roles-to-primary-tab.patch queued for re-testing.

The patch in #884086: Roles are quite hidden at the moment did:

List -> Permissions -> Roles

The screenshot above shows:

List -> Roles -> Permissions

Which order should they actually be in?

The first seems better to me since it puts the more frequently used page ahead of the less frequently used one (and also it's alphabetical :). However, the second is a little more logical in terms of the relationship between the tabs (users have roles, but roles have permissions).

Definitly the first, useage triumps conceptual relationship in this case.

user.test got PSR-0'ed, that's why this no longer applied. Fixed that.

Also, the menu stuff was borked b/c we had left the MENU_DEFAULT_LOCAL_TASK in there (which we no longer need) and the Roles tab had a weight -5. Fixed that, too. ;)

Rerolled because of #935062: Change role id to machine name.

This patch can also be found in the platform-role-1262812-klausi branch of the platform sandbox http://drupal.org/sandbox/sun/1255586

Codewise the patch is a straightforward replacement of the old path, and the web test coverage for role CRUD still works. I applied the patch locally and confirmed that no instances of the old path were missed. It would be good to have screenshots in the issue for the UI change, which klausi should be adding right about now. :)

I also tested the patch manually and it works as advertised.

Screenshot before:

Screenshot after:

Thanks klausi!

Asking for a re-test. I think the system tests have moved.

#28: 1262812-role-tab-28.patch queued for re-testing.

Rerolled.

git fetch origin
git fetch platform
git checkout platform-role-1262812-klausi
git merge origin/8.x
git diff origin/8.x > 1262812-role-tab-35.patch

I love you git. Truly.

lala, status

Agreed with the RTBC. Patch is still good on visual inspection, bot is happy, and behold:

% cd drupal-8
% git pull
Current branch 8.x is up to date.
% find . | xargs egrep -i admin/people/permissions/roles | wc
      38     734    8016
% find . | xargs egrep -i admin/people/permissions/list | wc 
       2       8     163
% wget http://drupal.org/files/1262812-role-tab-35.patch
...
% git apply 1262812-role-tab-35.patch
% mv 1262812-role-tab-35.patch ..
% find . | xargs egrep -i admin/people/permissions/roles | wc  
       0       0       0
% find . | xargs egrep -i admin/people/permissions/list | wc  
       0       0       0

That pretty much says it all. ;) And if Bojhan says it's RTBC, I don't think this needs any usability testing...

Cheers,
-Derek

Committed to 8.x. Finally! Thanks for all the re-rolls along the way. :)

This is a won't fix, checked it with Angie.

However, please do make a contrib module or something for this :)

Changing menu router paths is definitely an API change, so at minimum, tagging this.

Not sure whether this needs a change notice. Perhaps we should have a generic/summarized one for all router paths that have changed between D7 and D8?

just stumbled on this... was going to suggest it!
awesome =)

Fixed the URL for the screenshot.