Permission to "approve other suggestions only"

Good feature idea. I'll look into implementing this later if nobody beats me to it. It needs some thought on the integration with Organic Groups as well, since it only provides one level of permissions (either you are a group member or an admin).

This is quite easy to game though with two user accounts. Also I assume you expect people to play fair :)

This is a feature request, I think a good one. Maybe somebody will pick it up? I might but fixing bugs seems to have higher priority

The problem with "Approve other suggestions only" is, that the code gets uglier, because user_access("Approve other suggestions only") return TRUE for the user1, which is kind of unlogical.

Here is a patch which does only the check with two user access(suggester and approver). It needs more testing

A couple of remarks:

• We should use lower case permission names
• We should drop submit translations and approve suggestions in favor of the more granular submit suggestions, approve own suggestions and approve suggestions. That way, we could also create ‘reviewer only’ accounts.

Agreed with @kkaefer!

I also like the idea
I will have time this afternoon to do it.

Here is a more complete patch I just received from @kkaefer yesterday. This is broken out of the bigger patch at #563128: Rework the translation UI. I did a deep reading of this patch, and found the following issues:

- minor: l10n_community_string_suggestions() could use cleaner variable assingment to use if ($perm) instead of using ternary operators with empty second values
- l10n_community_import_form() checks for L10N_PERM_MODERATE_OWN, but people can use that form to submit .po files in the name of the group (which is then not attributed to their name but to the "multiple contributors" user); in that case, their act of clicking this button would require the L10N_PERM_MODERATE_OTHERS permission. So I'd say we should display that checkbox if either moderation permissions are given and then either have a description and/or JS to disable/enable that checkbox depending on the permission levels and handle it on the server side as well accordingly.
- I'm seeing L10N_PERM_STABILIZE_OTHERS and L10N_PERM_STABILIZE_OWN in the patch, which are then not handled (given that this patch is not about supporting that feature); I'd suggest to remove this, and would say that adding them later as bigger numbers should not confuse the listing (string permissions will not be exactly close by in terms of bits, but that is fine IMHO).

I'll look into these and the test failures that Konstantin mentioned.

The patch did not apply anymore due to changes to the moderation page filter handling and the import screen, as well as import tests added. Fixed where it did not apply and rerolled as a first step.

Attached patch fixes some issues:

- implemented conditional wrapper instead of ternaries in l10n_community_string_suggestions()
- found out that l10n_community_string_ajax_suggestion() was only accepting approval/decline actions, if the user had both permission to approve own and approve other suggestions; needed to use the same logic as in the builder function
- fixed permission check on top of l10n_community_import_page(), since checking for IMPORT permission is late enough here; we should use the moderation permissions instead
- removed the stabilization constants
- reverted to the old permission checking code in l10n_community_block_help(); I could not figure out why did you try to use NULL as a language code to ask for permissions, that should theoretically never happen
- added detailed comments on l10n_community_moderation_form() on who's responsibility is to check permissions ahead
- fixed l10n_community_pick_go_submit() to use the suggest permission, since import is not required when jumping to the edit or view page (previous patch used the more generic contribute permission)
- added a bunch of @todo fixme comments with explanation on what still needs to be done; mostly description changes about what a user has permission to do

Running this through our testsuite in the module says 934 passes, 4 fails, and 0 exceptions
Looks like those 4 failures are due to individual approve and decline actions failing.

subscribe

@hass: any better suggestions for naming?

Updated patch now passes tests. The ajax submission callback handler was not using $suggestion->language but $langcode, which was nonexistent in that context.

Fixed use of l10n_community_get_permission() to use $perm consistently. One place used $permissions for example, which is elsewhere used for the whole array of permissions for all users and languages.

Also found an issue with permissions for the moderate page, which requires review access to be used, but in fact, it would work with either moderation access right. Added a todo for now.

@hass: again, any better suggestions for naming?

I suggest "moderate suggestions from others" or "moderate suggestions by others" for permission name. (Not sure which one's best, but both should be reasonably clear.)

Used "moderate suggestions from others" in the updated attached patch. Still needs to fix the @todo's.

Worked more on this patch. Fixed @todos regarding help text in the main module and group module, title for import screen and access permission to moderate page.

There is still one @todo left for option of direct translation import on import screen, which is tricky due to how it turns out depending on you filling the form whether we need one permission or another to validate your submission. Might need some tricky JS to help validate this, as well as more robust checking on the PHP side.

So still needs work. Help is still welcome, so I'm not a lone crusader :)

Worked even more on this patch. Tried to test different scenarios. With l10n_groups enabled, I found that even if I'm the creator of a language group, I do not get any contribution permissions. This is not right. also, if I also enable og_user_roles (which will eventually be used to grant these roles to members), I get even more stranger results with the Overview page saying I have "63" permissions (all bits active), while only the Browse and Export tabs work ("33") and the other tabs show on Overview but not on Browse or Export. Very strange. Anyway, that might be an og_user_roles issue, but it still does not work with the regular l10n_groups, which runs on l.d.o, so it is not a go yet. (And it still has a @todo).

Worked on the remaining @todo on the import screen. Did not implement convenient JS helper for import form, but server side submission validation takes permissions into account now. Tested with OG again, and it turned out to be a configuration issue on my part, not an issue with the patch or OG. So committing this one.

It would still be cool to add tests. I'm not deploying this just yet on localize.drupal.org. For this to make sense there, we'd still need to have og_user_roles set up there and our l10n_groups og permission handling removed. So then group managers could hand out permissions to members appropriately. As it is right now, if you are using groups like l.d.o does, the advantages of finer grained permissions is annulled, since the og permission handler can only distinguish admins and members, and these still apply the same permissions as before. If group members would be full user admins, it would equally be useful, but that is not going to happen on l.d.o. Also, approval rights in one language should not mean approval rights in another automatically as you join, so we truly need og_user_roles support.

Realized update function was missing. Here it is, committed also.

Followup to migrate to og_user_roles submitted at #652038: Migrate permissions to og_user_roles, instead of our own code.

#652038: Migrate permissions to og_user_roles, instead of our own code is now committed and deployed. News post of localize.drupal.org deployment at http://localize.drupal.org/node/616