I believe so, but I don’t know for sure. Please test and let me know.
Alright, thank you for your quick response and have a wonderful weekend!
Cheers! 🙂
Same to you! Please do let me know what you find out. I believe it should work fine for each sub-site.
Hi @helgatheviking !
Thanks a bunch for your answers!! 🙂
Sorry for my very slow response! (been quite busy)
I might ask you nicely while i have you on the line..
Any update planned soon btw? The plugin is getting bit old by now..(seven months+..
In regards to WPMU..:
I have it running on a few sub-sites and so far it seems to work. I have not network activted it. But per website…
Cheers! 🙂
Well that’s good to hear. There are no planned updates since the plugin is still working just fine for most people. If it ain’t broke….
Thank you @helgatheviking !
I wish you a wonderful day.
Cheers!
AngryWarrior
Hi @helgatheviking ,
I just tested your plugin, and unfortunately, it does not differentiate between users of different sites in a multi site installation. So any logged in user (of the respective access level) of any site can see any restricted navigation entry.
Is this a desired behavior? If not, is this something that can be fixed?
Best,
Fiech
-
This reply was modified 8 years, 10 months ago by
Fiech.
It isn’t desired, it’s just I don’t know that much about multi-site. It appears I may need to user current_user_can_for_blog() instead of current_user_can() here: https://github.com/helgatheviking/Nav-Menu-Roles/blob/master/nav-menu-roles.php#L464
I’m open to pull requests if you want to look into it, otherwise I won’t have the time to look into this until possibly September.
Hey, thanks for the quick reply.
Ok, so I looked into it a bit, and I have to correct myself: If I select a minimum role, it works as desired, because current_user_can() already only looks up the current blog.
The problem occurs, if one only selects “Logged in” as a requirement. Depending on whether this is a desired behavior, it can be easily fixed by changing line 455 to:
$visible = ( is_user_logged_in() && is_user_member_of_blog() ) ? true : false;
I don’t know, if this warrants a PR, but I can open one, if you prefer.
Well that’s great. What happens with is_user_member_of_blog() when *not* a multisite? Is there anything that needs to happen when “logged out” is selected?
Should
$visible = ! is_user_logged_in() ? true : false;
become
$visible = ! is_user_logged_in() || ! is_user_member_of_blog() ? true : false;
If you can clear that up, I can probably push an update without a PR.
Ok, I did some further testing:
a) The code and the changes all work as expected in a single site installation too.
b) You’re right about the logged-out case. There, the fix is also needed because otherwise users from different blogs won’t see external links, when logged in.
c) The function current_user_can() in line 464 respects super-admin rights. So to make this consistent with the logged-in/logged-out parts, you’ll need the following fixes:
line 455: $visible = $visible = is_user_logged_in() && ( is_user_member_of_blog() || is_super_admin() ) ? true : false;
line 458: $visible = ! is_user_logged_in() || ! is_user_member_of_blog() && ! is_super_admin() ? true : false;
Doesn’t is_user_logged_in() return true if the super admin is logged in? So is the check for super admin necessary?
The thing is, with the fix, the user has additionally be a member of the current blog. But is_user_member_of_blog() does not return automatically true for super admins. OTOH current_user_can() will return true for super admins in every case.
But you can streamline the code, because both is_user_member_of_blog() as well as is_super_admin() return false (surprise, I know, but I checked for errors, nonetheless), if current user is not logged in at all.
So, you can simplify the code:
line 455: $visible = is_user_member_of_blog() || is_super_admin() ? true : false;:
For the logged in condition to work, the user has to be a logged in member of the current blog or be a logged in super user.
line 458: $visible = ! is_user_member_of_blog() && ! is_super_admin() ? true : false;:
For the logged out condition to work, the user has to be either logged out or not be a member of of the current blog. But they also may not be a super admin, because logged in super admins should see the internal stuff, not the external.
This would be consistent with the code in lines 461-467, where current_user_can() always returns true for super admins.
I also checked this code to work with a non-multi site installation.
Coming back to it: Should I put up a pull-request on the weekend?
Please do. I don’t know quite when I’ll get to check it out, but I will eventually. Thanks!
