search

wagtail / wagtailtrans

A Wagtail add-on for supporting multilingual sites
http://wagtailtrans.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
104 stars 60 forks source link

Strict Translate Permissions #149

Open JohnMoutafis opened 5 years ago

JohnMoutafis commented 5 years ago

Implements initial idea for strict translate permissions as described in issue #147

mikedingjan commented 5 years ago

Lets continue the discussion over here.

I've looked through your PR, still don't agree on having a setting which will "magically" add behavior in restricting which pages someone can see, especially since the languages the user has access to are collected from the groups it's member of (with string matching)..

How to determine which languages the user has access to is for me as well unclear, maybe a intermediate model between groups and languages, this way we can query with the actual language objects (no string splitting / matching). This would also make it possible to define read / write / delete permissions per language per group (for example: translator-be should be enough to allow nl_BE and fr_BE). A custom PermissionsTester can handle these checks for example.

Creating queries with actual Language objects should also perform a lot better, since you can replace language__code__in=[] which saves some joins.

Also checking for group name to determine if a user is "privileged" is greatly discouraged, since group names can change, maybe there is, or need to be, some custom permission to check for? Checking that way it will also be possible to add more "privileged" user groups by adding that permission to the group.

Any thoughts on this?

JohnMoutafis commented 5 years ago

@mikedingjan Your suggestions seem very reasonable. I made this PR mostly to show you what we did as a workaround for our use-case.

Feel free to adjust the code as you see fit (and it goes without saying that I will be happy to help on that issue :smile:)

mikedingjan commented 5 years ago

I know, that's why I wanted to continue the discussion here, so we can all agree on an approach which works for all. :) Do you have any additional requirements with proposed solution?

Any other thoughts about this? @jjanssen maybe?