API page: https://api.drupal.org/api/drupal/modules!user!user.module/function/user...

The $roles parameter is optional. It would be fair to assume that if omitted, you get data for all roles -- after all, this is how lots of Drupal functions work.

That is however not the case. You get nothing. I'm not even sure why the param is optional, as you can nothing without it. It might be a remnant from when this function would have had a $reset parameter too.

Anyway, we probably shouldn't change it on D7 now, so we should change the docs to say that there's no point calling it without $roles.

CommentFileSizeAuthor
#12 user_role_permissions_without_parameter-2158383-12.patch758 byteslauriii
#10 user_role_permissions_without_parameter-2158383-10.patch762 byteslauriii
#8 user_role_permissions_without_parameter-2158383-8.patch762 byteslauriii
#6 user_role_permissions_without_parameter-2158383-5.patch758 bytes-enzo-

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Closed (works as designed)

I don't see any documentation there saying $roles is optional? I see it has a default value of array(), but there is no documentation saying $roles is optional or that if you pass in nothing you get all the roles. So I don't see a problem in the docs here.

joachim’s picture

joachim commented

> I see it has a default value of array(),

That means it's optional!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Agreed, the default means it is optional. However, the docs do not say it's optional, so I'm not sure what we can do. If you want to make this a pure 8.x and user.module issue instead of documentation, you could lobby to have that default removed.

joachim’s picture

joachim commented

8.x has the default removed :)

My point is that what happened to me could easily happen to other developers: call the function with no parameter, assuming that'll get you data for all roles because that's how Drupal tends to work. Get nothing back. Scratch head, read code.

Could we just add a note to say there parameter being optional is a bug, and it is in fact necessary?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Closed (works as designed) » Active

OK. :) Make a patch?

-enzo-’s picture

-enzo- commented
Status: Active » Needs review
StatusFileSize
new user_role_permissions_without_parameter-2158383-5.patch758 bytes

I just did an small patch , please review

joachim’s picture

joachim commented
Status: Needs review » Needs work

This issue is just about fixing the documentation.

Unfortunately, we can't change the return value of an API function at this point in the development of 7.x.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Needs review
StatusFileSize
new user_role_permissions_without_parameter-2158383-8.patch762 bytes

Here's a patch for the comment.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Thanks for the patch!

We need to have all of the lines of documentation wrapped into a paragraph, so that each line is close to 80 characters without going over.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new user_role_permissions_without_parameter-2158383-10.patch762 bytes

Now with fixed line lenghts

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented 
+ *   If param $roles is set an array indexed by role ID is returned. Each
+ *   value is an array whose keys are the permission strings for the given
+ *   role ID.
+ *   If param $roles is empty nothing is returned.

Please move the last sentence up to the previous line. Thanks! Also... can we not use the non-word "param" in the documentation text? Just say "If $roles is a non-empty array...". Thanks!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new user_role_permissions_without_parameter-2158383-12.patch758 bytes

Maybe now!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Needs review

The last submitted patch, 10: user_role_permissions_without_parameter-2158383-10.patch, failed testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Thanks! This one looks good to me. I'll get it committed sometime soon.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to 7.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.