Merge pull request #303 from fluxility/master

Make permissions in dynamic item lazy evaluated

AUTHORS

@@ -52,6 +52,7 @@ Kishor Kunal Raj <https://github.com/kishorkunal-raj>
 Ben Finney <https://github.com/bignose-debian>
 witwar <https://github.com/witwar>
 Jon Kiparsky <https://github.com/jonkiparsky>
+Jeffrey de Lange <https://github.com/jgadelange>
 
 
 Translators

sitetree/tests/test_utils.py

@@ -97,17 +97,41 @@ def test_perm_obj_permissions(self):
         i1 = item('root', 'url', access_by_perms=perm)
         assert i1.permissions == [perm]
 
-    def test_bad_string_permissions(self):
-        from sitetree.toolbox import item
+    def test_bad_string_permissions(self, template_context, template_render_tag):
+        from sitetree.toolbox import register_dynamic_trees, tree, item, compose_dynamic_tree
 
-        with pytest.raises(ValueError):
-            item('root', 'url', access_by_perms='bad name')
+        register_dynamic_trees(compose_dynamic_tree([tree('bad', items=[
+            item('root', 'url', access_by_perms='bad name'),
+        ])]), reset_cache=True)
 
         with pytest.raises(ValueError):
-            item('root', 'url', access_by_perms='unknown.name')
+            template_render_tag(
+                'sitetree', f'sitetree_page_title from "bad"',
+                template_context(request='/'))
+
+    def test_unknown_name_permissions(self, template_context, template_render_tag):
+        from sitetree.toolbox import register_dynamic_trees, tree, item, compose_dynamic_tree
+
+        register_dynamic_trees(compose_dynamic_tree([tree('unknown', items=[
+            item('root', 'url', access_by_perms='unknown.name'),
+        ])]), reset_cache=True)
 
         with pytest.raises(ValueError):
-            item('root', 'url', access_by_perms=42.2)
+            template_render_tag(
+                'sitetree', f'sitetree_page_title from "unknown"',
+                template_context(request='/'))
+
+    def test_float_permissions(self, template_context, template_render_tag):
+        from sitetree.toolbox import register_dynamic_trees, tree, item, compose_dynamic_tree
+
+        register_dynamic_trees(compose_dynamic_tree([tree('fortytwodottwo', items=[
+            item('root', 'url', access_by_perms=42.2),
+        ])]), reset_cache=True)
+
+        with pytest.raises(ValueError) as e:
+            template_render_tag(
+                'sitetree', f'sitetree_page_title from "fortytwodottwo"',
+                template_context(request='/'))
 
     def test_access_restricted(self):
         from sitetree.toolbox import item

sitetree/utils.py

@@ -5,6 +5,7 @@
 from django.apps import apps
 from django.contrib.auth.models import Permission
 from django.core.exceptions import ImproperlyConfigured
+from django.utils.functional import SimpleLazyObject
 from django.utils.module_loading import module_has_submodule
 
 from . import settings
@@ -51,6 +52,39 @@ def traverse(items):
     return tree_obj
 
 
+def clean_permission(permission: TypePermission) -> Union[int, Permission]:
+    if isinstance(permission, str):
+        # Get permission object from string
+        try:
+            app, codename = permission.split('.')
+        except ValueError:
+            raise ValueError(
+                f'Wrong permission string format: supplied - `{permission}`; '
+                'expected - `<app_name>.<permission_name>`.')
+        try:
+            return Permission.objects.get(codename=codename, content_type__app_label=app)
+        except Permission.DoesNotExist:
+            raise ValueError(f'Permission `{app}.{codename}` does not exist.')
+    elif not isinstance(permission, (int, Permission)):
+        raise ValueError('Permissions must be given as strings, ints, or `Permission` instances.')
+
+    return permission
+
+
+def clean_permissions(permissions: Union[TypePermission, List[TypePermission]]) -> List[Permission]:
+    if permissions is None:
+        return []
+
+    # Make permissions a list if currently a single object
+    if not isinstance(permissions, list):
+        permissions = [permissions]
+
+    return [
+        clean_permission(permission)
+        for permission in permissions
+    ]
+
+
 def item(
         title: str,
         url: str,
@@ -118,37 +152,10 @@ def item(
     item_obj.is_dynamic = True
     item_obj.dynamic_children = []
 
-    cleaned_permissions = []
-    if access_by_perms:
-        # Make permissions a list if currently a single object
-        if not isinstance(access_by_perms, list):
-            access_by_perms = [access_by_perms]
-
-        for perm in access_by_perms:
-            if isinstance(perm, str):
-                # Get permission object from string
-                try:
-                    app, codename = perm.split('.')
-                except ValueError:
-                    raise ValueError(
-                        f'Wrong permission string format: supplied - `{perm}`; '
-                        'expected - `<app_name>.<permission_name>`.')
-
-                try:
-                    perm = Permission.objects.get(codename=codename, content_type__app_label=app)
-
-                except Permission.DoesNotExist:
-                    raise ValueError(f'Permission `{app}.{codename}` does not exist.')
-
-            elif not isinstance(perm, (int, Permission)):
-                raise ValueError('Permissions must be given as strings, ints, or `Permission` instances.')
-
-            cleaned_permissions.append(perm)
-
-    item_obj.permissions = cleaned_permissions or []
+    item_obj.permissions = SimpleLazyObject(lambda: clean_permissions(access_by_perms))
     item_obj.access_perm_type = item_obj.PERM_TYPE_ALL if perms_mode_all else item_obj.PERM_TYPE_ANY
 
-    if item_obj.permissions:
+    if access_by_perms:
         item_obj.access_restricted = True
 
     if children is not None: