new permissions for list sharing

Hi solipsist,

Igor hired me to fix this issue for him.

This is patch which adds share own list of watched posts permission. Roles with this permission disabled don't see Share my list of watched posts at theirs Watcher user settings.

Users with access others lists of watched posts permission are able to see other's users permission if this users have 1) share own list of watched posts permission enabled and also Share my list of watched posts enabled at theirs Watcher user settings.

Thats all. Three lines of code changed/added...

Talk to you soon
Vojta

Thanks for your patch.

Igor requested I "fixed" this but I do not consider it a bug. My reasons for thinking so follow:

The setting means: "Allow those who have sufficient permission to view my list of watched posts", which is true regardless of whether the current user may view others' lists of watched posts. These are independent of each other. It would be strange if you could only allow others to view your posts if you were allowed to see theirs. Further, it would complicate the setting of default permissions as it would require the module to calculate user settings based on the permission of each user. Given the number of access modules and the ACL chain, a check would have to be done per-user, and not inferred from roles alone. Looking at your patch I can see it doesn't work with default user settings.

Hi solipsist,

I understand what you mean with the settings originally. You have symmetrical permission which allows users to see each others lists. My patch allows both "asymmetrical" and "symetrical" relationship: When you assign both permisions to the role, relationship will become symmetric for this role.

What do you mean with ACL chain? In the patch I added user_access('share own list of watched posts', $account) to the binder_access function and it works as supposed:

// check access to list of the $account, $user can be the author - same account - or other user
// I added user_access('share own list of watched posts', $account) to check if the account's role even allows that
// and after that there is a check of the per user setting if the user personally wants to allow that
return (
   $user->uid == 1 ||
   ($user->uid == $account->uid && user_access('use watcher')) ||
   (user_access('access others lists of watched posts') && user_access('share own list of watched posts', $account) && _watcher_user_settings_load('watcher_share_binder', $account->uid))
   );

But the root of the problem for Igor is that users see the setting Share my list of watched posts even in case they have not the permission access others lists of watched posts and anyone don't have it. So they can be confused about that.

So if asymmetric relationship sounds strange to you, I think at least the visibility to this settings should be based on the user access.

   //The line in patch which checks user access to the *Share my list of watched posts* per user setting ...
   '#access' => user_access('share own list of watched posts', $account),

   // ... can be changed to work with original set of permissions ...
   '#access' => user_access('access others lists of watched posts', $account),

   // ... or we can make some setting in the Watcher adminstration and check it.
   '#access' => variable_get('watcher_enable_list_sharing', TRUE),

Do you see some problem with this?

Personally I don't see why this relationship needs to be "symmetrical". I think it's just confusing to make it symmetrical. It is intuitively asymmetrical. Further, it's not just unintuitive, it's programmatically complex.

Your patch only solves half the problem, that is for users who choose to customize their settings. Look in the settings screen for Watcher and you'll see there's something called "Default settings". With a "symmetrical" relationship, updating default permissions becomes complex since you need to check the permission you just introduced for *every* user before setting the shared binder permission or not. Joining the query with the roles table based on what roles have what permissions won't suffice as permissions are chained, other access modules interfere, so it must be done in code which is expensive and must be batched if the site has lots of users.

I was thinking that you thinks it has to be "symmetrical" in a sense like allow to see my list to all roles which has "access others lists of watched posts" permission.

I think that the batch update of default setting could be same as now. You don't need to check the user permission when updating the default settings, you will need to check the permission only when user wants to access other user list or the own user setting.

If this still sounds complicated to you, there is possible solution #2:

Forget for now about the new permission and only imagine situation that you don't want to allow any user to share list on the site, so no roles has assigned "access others lists of watched posts" permisson. But when one of these users goes to own Watcher user settings tab, they still see setting Share my list of watched posts. This setting is still present there but without any real effect(!) and it is confusing to users. This is the main point of this "issue".

Maybe we can solve this by some global Watcher setting at the admin level: "Allow users to share their list" or something like that?

In case it will be disabled, setting on the User settings will be replaced with a form type=value with default value (so user can't modify the setting, but it will be present in form values). So you will don't need to modify user settings form submit handler.

There will be only two modifications
1) + one setting on the admin settings form and
2) + one if /else on the user settings form
.. and nothing else...